Add support for AD7944, AD7985, AD7986

[dlech]

This adds device tree bindings compatible strings, ad_pulsar driver support and ZedBoard device trees for the AD7944, AD7985, and AD7986 chips.

• https://www.analog.com/media/en/technical-documentation/data-sheets/AD7944.pdf
• https://www.analog.com/media/en/technical-documentation/data-sheets/AD7985.pdf
• https://www.analog.com/media/en/technical-documentation/data-sheets/AD7986.pdf

This has not been tested yet since we are waiting on HDL for these.

[nunojsa]

From a code point of view, everything looks good. But I would say, let's wait for real testing before merging

[dlech]

Have the HDL now.

Updated:

• added gpio for TURBO
• added spi_clk

Still waiting on a hardware setup to be able to test.

[dlech]

Not sure this should be required, e.g. the TURBO pin could be hardwired instead of connected to a GPIO.

[dlech]

update:

• rebased on master
• fixed copy/paste mistake for ad7985 in ad_pulsar_of_match
• fixed turbo gpio number in devicetree

We have hardware to test now, but the driver doesn't seem to be communicating with the ADC chip. The SPI transactions time out (which lead to discovering #2287).

[mhennerich]

Hi David,

I was asked why the dt_binding_check failed on this PR:

CHKDT Documentation/devicetree/bindings/processed-schema.json
/home/vsts/work/1/s/Documentation/devicetree/bindings/iio/adc/adi,pulsar.yaml: allOf:1: 'then' is a dependency of 'if'
hint: Keywords must be a subset of known json-schema keywords
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#
/home/vsts/work/1/s/Documentation/devicetree/bindings/iio/adc/adi,pulsar.yaml: allOf:1:if: 'if' is a dependency of 'then'
hint: Keywords must be a subset of known json-schema keywords
from schema $id: http://devicetree.org/meta-schemas/keywords.yaml#

After fixing the if/then indention this PR runs the failing checks successfully.
Please see here: #2290

[dlech]

Update:

• fixed wrong indent in DT bindings

We have the hardware setup working now, so at least superficially this looks like it is working. We can see a sine wave in the IIO oscilloscope.

So the only question left is the desired behavoir of the turbo gpio.

Should we just automatically disable turbo mode if the requested sample rate does not require it? Or do users need more control over this?

[dlech]

I also found an issue where the first sample is always 0, so I will need to fix that. Or is this the expected behavior for the pulsar driver?

[dlech]

Having looked into this more deeply now, I noticed that AD7986/85/44 don't have a config register like AD7682. So should we make the config register optional in ad_pulsar.c or are these chips different enough we should create a separate driver? (I suspect this is what is causing the first sample to be zero as I mentioned in the previous comment.)

[nunojsa]

Should we just automatically disable turbo mode if the requested sample rate does not require it? Or do users need more control over this?

Not know much about the device but if this is something that can be done automatically by the driver I would say it's better. Always better to simplify the ABI. I mean is the turbo meaningful at all for sampling rates not needing it?

I also found an issue where the first sample is always 0, so I will need to fix that. Or is this the expected behavior for the pulsar driver?

Hmm i would be surprised if it's expected :). Those devices have any way to control the interrupt line or are we relying on {enable|disable}_irq()?

[dlech]

Not know much about the device but if this is something that can be done automatically by the driver I would say it's better. Always better to simplify the ABI. I mean is the turbo meaningful at all for sampling rates not needing it?

Not that I know of. Also, there are some configurations, like daisy-chaining multiple chips where turbo mode is not allowed. So keeping it internal to the driver seems like the best option to me.

Hmm i would be surprised if it's expected :). Those devices have any way to control the interrupt line or are we relying on {enable|disable}_irq()?

Having thought about it a bit more, I think it could be a problem with the HDL. The CNV pin is being used as the CS pin but the conversion is triggered on the rising edge. On the first sample there is probably only a falling edge because the CS/CNV line was high since startup.

The HDL needs to make sure there is a low to high transition of the CNV line before each SPI read.

So should we make the config register optional in ad_pulsar.c or are these chips different enough we should create a separate driver?

FYI, I'm looking at making a separate driver for this chips upstream along with the work we have started on getting SPI Engine offload support mainlined.

[dlech]

This has been tested by both BayLibre and ADI now. Do we want to merge as-is and address the outstanding issues separately?

Still TODO:

• ad_pulsar.c assumes all differential ADC channels are bipolar #2313 - this is a bigger issue than just these 3 chips being added.
• Turn on/off TURBO based on requested sample rate.

IMHO, it would be easier to do these if we first split ad_pulsar.c into two drivers as suggested in #2312.

[nunojsa]

This has been tested by both BayLibre and ADI now. Do we want to merge as-is and address the outstanding issues separately?

If you agree, I would just merge this one and add the missing features in future patches... Thoughts?

[dlech]

If you agree, I would just merge this one and add the missing features in future patches... Thoughts?

Yes, I agree.