Add support for ADA4355 #2806
Add support for ADA4355 #2806
Conversation
PIoandan
requested review from
nunojsa,
mhennerich,
dbogdan,
btogorean,
chegbeli,
buha,
amiclaus,
rbolboac,
danmois,
CiprianRegus and
machschmitt
as code owners
PIoandan
force-pushed
the
dev_ada4355
branch
3 times, most recently
from
dd3c40b to
7c9e53a
Compare
|
|
||
| config ADA4355 | ||
| tristate "Analog Devices ADA4355 uModule Data Acquisition Module" | ||
| depends on SPI |
There was a problem hiding this comment.
edit: my comment looks into why it was lacking symbols, nuno suggestion is the correct solution.
| depends on SPI | |
| depends on SPI | |
| depends on CF_AXI_ADC |
The driver uses axiadc_write and axiadc_read, exported from cf_axi_adc_core.c, enabled with CF_AXI_ADC.
So the driver depends on this symbol as well.
Without this change, it cannot be compiled as a module without blindly enabling symbols.
This is an issue in other drivers.
For curiosity, we have now a script that resolves the tree of depends in the KConfig based on a target:
$ python3 ci/symbols_depend.py drivers/iio/adc/ada4355.o
Symbols of touched files:
{'ADA4355'}
Resolved symbols:
{'PCI', 'IIO_BUFFER', 'HAS_IOMEM', 'HAVE_PCI', 'ADA4355', 'SPI', 'IIO'}
as committed, it lacks the CF_AXI_ADC symbol.
| #include <linux/iio/iio.h> | ||
| #include <linux/iio/sysfs.h> | ||
|
|
||
| #include "cf_axi_adc.h" |
There was a problem hiding this comment.
Why using the above and not the new IIO backend framework? That's what we want to use for all new drivers interfacing with the AXI ADC IP. This also makes it possible to upstream the driver.
|
|
||
| #include "zynq-zed.dtsi" | ||
| #include "zynq-zed-adv7511.dtsi" | ||
|
|
There was a problem hiding this comment.
Hi, the PR description says the driver is in the process of upstreaming, but I didn't find it in the mailing list so commenting here.
For upstreaming, a dt-binding (.yaml file) will be needed.
And dt maintainers will probably ask to describe the voltage supplies.
Looks like this will have a required 3.3V VCC supply and two (VEA and VED) optional supplies.
| /*PATTERN*/ | ||
| #define ADA4355_ALT_CHECKERBOARD_PATTERN 0x44 | ||
|
|
||
| /*INPUT_SIGNALS*/ |
There was a problem hiding this comment.
These comments don't add much to the driver. I'd say they are not need if the defines are given sane names.
| /*REG TRANSFER 0xFF*/ | ||
| #define ADA4355_OVERRIDE BIT(0) |
There was a problem hiding this comment.
Maybe add REG to the define and drop the comment:
-/*REG TRANSFER 0xFF*/
-#define ADA4355_OVERRIDE BIT(0)
+#define ADA4355_OVERRIDE_REG BIT(0)| /*CHIP ID*/ | ||
| #define ADA4355_CHIP_ID 0x8B |
There was a problem hiding this comment.
The name in the define already says it's the chip ID.
| tmp = (conv->chip_info->scale_table[0][0] * 1000000ULL) >> | ||
| conv->chip_info->channel[0].scan_type.realbits; | ||
| *val = tmp / 1000000; | ||
| *val2 = tmp % 1000000; |
There was a problem hiding this comment.
Is it applicable to use include/linux/units.h here?
e.g. *val = tmp / MICRO;
| static int ada4355_write_raw(struct iio_dev *indio_dev, | ||
| struct iio_chan_spec const *chan, | ||
| int val, int val2, long mask) | ||
| { | ||
| switch (mask) { | ||
| case IIO_CHAN_INFO_SCALE: | ||
| return -EINVAL; | ||
| case IIO_CHAN_INFO_SAMP_FREQ: | ||
| return -EINVAL; | ||
| default: | ||
| return -EINVAL; | ||
| } | ||
|
|
||
| return 0; | ||
| } |
There was a problem hiding this comment.
Why have write_raw if nothing can be written to the device? Need to implement something that writes to the device or drop write_raw.
| .scale_table = ada4355_scale_table, | ||
| .num_scales = ARRAY_SIZE(ada4355_scale_table), | ||
| .num_channels = 1, | ||
| .channel[0] = ADA4355_CHAN(0, 0, 14, 's', 2), |
There was a problem hiding this comment.
Will this driver support devices with more than one channel?
If not, can we assign the channel at probe? Maybe that will only be possible after switching to the IIO backend framework.
Signed-off-by: Pop Ioan Daniel <[email protected]>
Signed-off-by: Pop Ioan Daniel <[email protected]>
Add driver for ADA4355. Signed-off-by: Pop Ioan Daniel <[email protected]>
PR Description
The ADA4355 is a complete, high performance, current input µModule. The 14-bit ADC converts the amplified voltage signal at a rate of up to 125 MSPS and outputs the digitized signals through two serial, low voltage differential signaling (LVDS) data lanes. The data clock output (DCO) operates at frequencies of up to 500 MHz and supports double data rate (DDR) operation.
This driver is also in the process of upstreaming.
PR Type
PR Checklist