Hello,
the ad7124 driver was initially brought up on a raspberry pi. I tried to
do the same on a terasIC de10-nano board and that was much more bumpy
and time consuming than expected. There were two issues:
a) The GPIO controller used on the de10-nano detects an edge event on
the monitored line even when the irq is masked. Together with the
"feature" of the ad7124 that the irq is reported on the spi MISO line
this means that nearly every spi transfer results in a pending irq.
So when ad_sigma_delta_single_conversion() calls enable_irq(), the
irq immediately triggers without the ADC having performed its work
and so non-sense data is read.
To fix that, patch #2 adds a possibility to double check in the irq
handler if the DOUT/̅R̅D̅Y line is really low and only report success if
yes. That needs a different representation in the device tree, so the
binding docs need adaption. That's implemented in patch #1.
Note that while it's annoying to handle that sensitivity of the gpio
controller, this is actually a sane behaviour for a gpio controller.
(The problem with the rpi one's is: If the process is preempted long
enough that the ADC is already done when enable_irq() is called, the
event is missed and the measurement times out even though hardware
wise everything is fine.)
b) The reset default for the CHANNEL0 register is 0x8001, that means
that channel is enabled by default. So if the first measurement is on
(say) channel 5, the chip generates two interrupts. One when the
channel 0 measurement is done and another when the channel 5
measurement is done. The driver however only expects a single irq and
so reports the first data read as channel 5 data to the upper layers.
(Maybe the second irq then happens when the irq is currently enabled
and so might trigger another spurious irq.)
To fix that, patch #3 disables all channels at probe time to sync
reality to the driver's expectations.
This doesn't hurt if the first measurement is done on channel 0.
I wondered if I should add a "Fixes:" line for patch #2. Not entirely
sure because it's not needed for every setup. If yes, it would be for
the initial ad7124 commit (i.e. b3af341bbd96 ("iio: adc: Add ad7124
support")). Additionally that patch doesn't cure the symptom without a
device tree change.
I'd recommend backporting at least patch #3 to the stable branches, but
I didn't add the respective footers for that. I let the applying
maintainer decide if they want to add it (and for which patches).
Best regards
Uwe
Uwe Kleine-König (3):
dt-bindings: iio: adc: adi,ad7124: Allow specifications of a gpio for irq line
iio: adc: ad_sigma_delta: Add support for reading irq status using a GPIO
iio: adc: ad7124: Disable all channels at probe time
.../bindings/iio/adc/adi,ad7124.yaml | 13 +++++--
drivers/iio/adc/ad7124.c | 3 ++
drivers/iio/adc/ad_sigma_delta.c | 36 +++++++++++++++----
include/linux/iio/adc/ad_sigma_delta.h | 1 +
4 files changed, 44 insertions(+), 9 deletions(-)
base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc
--
2.45.2
