[0/3] iio: adc: ad7124: Make it work on de10-nano

Message

Uwe Kleine-König Oct. 24, 2024, 5:17 p.m. UTC

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