Re: [PATCH 2/8] dt-bindings: iio: adc: ad7606: Add iio backend bindings

[Jonathan Cameron <jic23@kernel.org>]

On Wed, 4 Sep 2024 11:54:30 -0500
David Lechner <dlechner@baylibre.com> wrote:

> On 8/17/24 10:09 AM, Jonathan Cameron wrote:
> > On Thu, 15 Aug 2024 12:11:56 +0000
> > Guillaume Stols <gstols@baylibre.com> wrote:
> >   
> >> Add the required properties for iio-backend support, as well as an
> >> example and the conditions to mutually exclude interruption and
> >> conversion trigger with iio-backend.
> >> The iio-backend's function is to controls the communication, and thus the
> >> interruption pin won't be available anymore.
> >> As a consequence, the conversion pin must be controlled externally since
> >> we will miss information about when every single conversion cycle (i.e
> >> conversion + data transfert) ends, hence a PWM is introduced to trigger  
> > 
> > transfer
> >   
> >> the conversions.
> >>
> >> Signed-off-by: Guillaume Stols <gstols@baylibre.com>
> >> ---
> >>  .../devicetree/bindings/iio/adc/adi,ad7606.yaml    | 75 +++++++++++++++++++++-
> >>  1 file changed, 72 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> >> index c0008d36320f..4b324f7e3207 100644
> >> --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> >> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml
> >> @@ -114,13 +114,28 @@ properties:
> >>        assumed that the pins are hardwired to VDD.
> >>      type: boolean
> >>  
> >> +  pwms:
> >> +    description:
> >> +      In case the conversion is triggered by a PWM instead of a GPIO plugged to
> >> +      the CONVST pin, the PWM must be referenced.
> >> +    minItems: 1
> >> +    maxItems: 2
> >> +
> >> +  pwm-names:
> >> +    minItems: 1
> >> +    maxItems: 2
> >> +
> >> +  io-backends:
> >> +    description:
> >> +      A reference to the iio-backend, which is responsible handling the BUSY
> >> +      pin's falling edge and communication.
> >> +      An example of backend can be found at
> >> +      http://analogdevicesinc.github.io/hdl/library/axi_ad7606x/index.html
> >> +
> >>  required:
> >>    - compatible
> >> -  - reg  
> > 
> > I think we still want a reg, but only to differentiate multiple instances
> > perhaps.  
> 
> In light of the recent discussions on the similar AXI DAC
> support for AD3552R [1], should we consider some of the same
> things here?
> 
> Essentially, the AXI ADC IP block in this series is acting as
> a parallel bus provider for the AD7606 chip. This is used both
> for configuring registers on the chip and "offloading" for high
> speed data capture.
> 
> So this would mean...
> 
> 1. We should add a new compatible string to iio/adc/adi,axi-adc.yaml
>    for the specialized version of the AXI ADC IP that is used with
>    AD7606 and similar ADCs.
> 2. In the .dts, the AXI ADC node should be the parent of the ADC node
>    since the AXI ADC IP is providing the parallel bus to the ADC.

Ah. I'd completely failed to notice this didn't have a separate control
bus.  The existing ad7606 only does reading so I assumed that the
data path couldn't carry configuration data.  Looking at this patch
is that still the case?

If so I think it is less critical to represent the bus given the history
of not doing so in this driver.   It would be a nice to have though.

Jonathan


> 
> 
> [1]: https://lore.kernel.org/linux-iio/20240903203935.358a1423@jic23-huawei/
> 
> >   
> >>    - avcc-supply
> >>    - vdrive-supply  
> > 
> > 
> >   
>