-
Notifications
You must be signed in to change notification settings - Fork 835
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
drivers: iio: adc : Add ad4695 support #2433
base: main
Are you sure you want to change the base?
Conversation
I'll try to do a proper review in the next few days... One thing that I'll already ask is to rename your driver name. Typically we don't name it with "wildcards" like 'x'. Just pick one of the parts the drivers supports (maybe the one you have) and name the driver against it. |
2f5312b
to
10555f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Finally remembered about this one 😅
description: The regulator supply for ADC reference voltage. | ||
|
||
"#io-channel-cells": | ||
const: 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why the above?
|
||
clock-names: | ||
items: | ||
- const: ref_clk |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just one clock... drop this one
|
||
pwm-names: | ||
items: | ||
- const: cnv |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know that for dma we need the -names property. Is that also true for pwm? Being just one, I would expect not to need it.
Select the pin-oairing option on the specific channel. Valid values | ||
are: | ||
0: Channel paired with REFGND. | ||
1: Channel paired with COM |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's the difference between refgnd and com from a software point of view? Could both be seen as single-ended channels?
are: | ||
0: Channel paired with REFGND. | ||
1: Channel paired with COM | ||
2: Even and odd channels pairing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The above needs more explanations? What's the meaning of odd and even in this case? (yes, not reading the datasheet at this point :))
drivers/iio/adc/ad4696.c
Outdated
reg_data &= ~mask; | ||
reg_data |= data; | ||
|
||
return ad4696_spi_reg_write(st, reg_addr, reg_data); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not safe... needs a lock
drivers/iio/adc/ad4696.c
Outdated
static int ad4696_enable_cnv(struct ad4696_state *st) | ||
{ | ||
return ad4696_set_sampling_freq(st, st->sampling_freq); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
drop the function
drivers/iio/adc/ad4696.c
Outdated
unsigned int vref_mv; | ||
int sampling_freq; | ||
u8 num_channels; | ||
u8 data[3] ____cacheline_aligned; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just use one space... also use the IIO define for DMA safe buffere... This one is not safe for all platforms and hence not the one to use
drivers/iio/adc/ad4696.c
Outdated
const char *name; | ||
enum ad4696_ids dev_id; | ||
int max_sample_rate; | ||
u8 max_num_channels; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: single space
drivers/iio/adc/ad4696.c
Outdated
if (ret) | ||
return ret; | ||
|
||
*reg_data = st->data[2]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, the question is... Can't we use regmap?
10555f2
to
0761652
Compare
Hi @nunojsa, I made the changes requested by you although there is a chance I missed some. Anyway I will mark this PR as a draft since I tested it with the ZedBoard and appears like the buffer isn't working right, I managed to fix a part of it but still the behavior isn't quit the expected one. |
FYI, we recently upstreamed the driver for this family of chips. https://git.kernel.org/pub/scm/linux/kernel/git/jic23/iio.git/tree/drivers/iio/adc/ad4695.c?h=testing And @threexc is working on adding more features upstream. |
0761652
to
6f9cf07
Compare
v2 :
|
@RaduSabau1 please make sure to take care about CI complains... |
d5fa7db
to
ce18e56
Compare
Add driver dependencies from upstream for upcoming AD4695 driver regarding the include files. Signed-off-by: RaduSabau1 <radu.sabau@analog.com>
Add upstream changes dependencies for upcoming AD4695 driver regarding the source files. Signed-off-by: RaduSabau1 <radu.sabau@analog.com>
Add device tree bindings for AD4695 and similar ADCs. Link: https://patchwork.kernel.org/project/linux-iio/patch/20240711-iio-adc-ad4695-v4-1-c31621113b57@baylibre.com/ Reviewed-by: Conor Dooley <conor.dooley@microchip.com> Signed-off-by: David Lechner <dlechner@baylibre.com> Signed-off-by: RaduSabau1 <radu.sabau@analog.com>
This is a new driver for Analog Devices Inc. AD4695 and similar ADCs. The initial driver supports initializing the chip including configuring all possible LDO and reference voltage sources as well as any possible voltage input channel wiring configuration. Only the 4-wire SPI wiring mode where the CNV pin is tied to the CS pin is supported at this time. And reading sample data from the ADC can only be done in direct mode for now. Co-developed-by: Ramona Gradinariu <ramona.gradinariu@analog.com> Signed-off-by: Ramona Gradinariu <ramona.gradinariu@analog.com> Signed-off-by: David Lechner <dlechner@baylibre.com> Signed-off-by: RaduSabau1 <radu.sabau@analog.com>
The Analog Devices Inc. AD4695 (and similar chips) are complex ADCs that will benefit from a detailed driver documentation. This documents the current features supported by the driver. Link: https://patchwork.kernel.org/project/linux-iio/patch/20240711-iio-adc-ad4695-v4-3-c31621113b57@baylibre.com/ Signed-off-by: David Lechner <dlechner@baylibre.com> Signed-off-by: RaduSabau1 <radu.sabau@analog.com>
Imply Ad4695 in Kconfig.adi file in the IIO subsystem. Signed-off-by: RaduSabau1 <radu.sabau@analog.com>
Add devicetree for ad469x_fmcz/zed project Signed-off-by: RaduSabau1 <radu.sabau@analog.com>
ce18e56
to
7c8b82f
Compare
v3:
|
No description provided.