Re: [PATCH v2 0/4] iio: adc: ad7124: Make it work on de10-nano

[David Lechner <dlechner@baylibre.com>]

On 10/28/24 11:07 AM, Uwe Kleine-König wrote:
> Hello,
> 
> this is iteration v2 to make ad7124 work on de10-nano. (Implicit) v1 is
> available at
> https://lore.kernel.org/linux-iio/20241024171703.201436-5-u.kleine-koenig@baylibre.com.
> 
> The changes since v1:
> 
> - Write 0 instead of 0x0001 to disable channels. While 0x0001 is the
>   reset default value for these registers (apart from the channel 0 one)
>   there is no sensible reason to use that value (i.e.
>   AD7124_CHANNEL_AINP(0) | AD7124_CHANNEL_AINM(1)) as the value is
>   reprogrammed before use anyhow. This addresses the feedback that the
>   magic value 0x0001 should better be constructed using register bit
>   field defintions.
> 
> - Add maxItems: 1 to the new property defined in the binding patch (Krzysztof)
> 
> - Rename property to rdy-gpios (Rob)
> 
> - Use rdy-gpios only for gpio reading and continue using the usual irq
>   defintion for the interrupt (Jonathan). I was surprised I can use both the
>   GPIO as input and the matching irq.
> 
> - patch #1 is new, and use GPIO_ACTIVE_LOW in the gpio descriptor
>   instead of 2.
> 
> Jonathan voiced concerns about the reliability of this solution and
> proposed to implement polling. I'm convinced the solution implemented
> here is robust, so I see no need to implement polling today.
> 
> Still open questions:
> 
>  - Is rdy-gpios the right name. The line is named ̅R̅D̅Y, so maybe nrdy-gpios? Or
>    nRDY-gpios?

The GPIO_ACTIVE_LOW indicates the "bar" for active low, so we don't
typically add the "n" prefix to the name. So rdy-gpios looks correct
to me.

>  - Jonathan wanted some input from ADI about this series and the
>    hardware details.
> 
> Best regards
> Uwe
> 
> Uwe Kleine-König (4):
>   dt-bindings: iio: adc: adi,ad7124: Use symbolic name for interrupt
>     flag
>   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          | 11 +++++-
>  drivers/iio/adc/ad7124.c                      |  3 ++
>  drivers/iio/adc/ad_sigma_delta.c              | 35 ++++++++++++++++---
>  include/linux/iio/adc/ad_sigma_delta.h        |  1 +
>  4 files changed, 44 insertions(+), 6 deletions(-)
> 
> base-commit: 9852d85ec9d492ebef56dc5f229416c925758edc