diff mbox series

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

Message ID 20240815-ad7606_add_iio_backend_support-v1-2-cea3e11b1aa4@baylibre.com (mailing list archive)
State Not Applicable
Headers show
Series Add iio backend compatibility for ad7606 | expand

Commit Message

Guillaume Stols Aug. 15, 2024, 12:11 p.m. UTC
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
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(-)

Comments

Conor Dooley Aug. 15, 2024, 2:38 p.m. UTC | #1
On Thu, Aug 15, 2024 at 12:11:56PM +0000, Guillaume Stols 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
> 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

You need to describe what the pwms are.

> +  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
>    - avcc-supply
>    - vdrive-supply
> -  - interrupts
> -  - adi,conversion-start-gpios
>  
>  # This checks if reg is a chipselect so the device is on an SPI
>  # bus, the if-clause will fail if reg is a tuple such as for a
> @@ -137,6 +152,35 @@ then:
>          - spi-cpol
>  
>  allOf:
> +  # Communication is handled either by the backend or an interrupt.

This comment seems misplaced, but also superfluous?

> +  - if:
> +      properties:
> +        pwms: false
> +    then:
> +      required:
> +        - adi,conversion-start-gpios
> +
> +  - if:
> +      properties:
> +        adi,conversion-start-gpios: false
> +    then:
> +      required:
> +        - pwms
> +
> +  - if:
> +      properties:
> +        interrupts: false
> +    then:
> +      required:
> +        - io-backends
> +
> +  - if:
> +      properties:
> +        io-backends: false
> +    then:
> +      required:
> +        - interrupts
> +
>    - if:
>        properties:
>          compatible:
> @@ -178,12 +222,37 @@ allOf:
>          adi,sw-mode: false
>      else:
>        properties:
> +        pwms:
> +          maxItems: 1
> +        pwm-names:
> +          maxItems: 1
>          adi,conversion-start-gpios:
>            maxItems: 1
>  
>  unevaluatedProperties: false
>  
>  examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    / {
> +        adi_adc {
> +                compatible = "adi,ad7606b";

Just two space indent for examples please.

Cheers,
Conor.

> +
> +                pwms = <&axi_pwm_gen 0 0>;
> +
> +                avcc-supply = <&adc_vref>;
> +                vdrive-supply = <&vdd_supply>;
> +
> +                reset-gpios = <&gpio0 91 GPIO_ACTIVE_HIGH>;
> +                standby-gpios = <&gpio0 90 GPIO_ACTIVE_LOW>;
> +                adi,range-gpios = <&gpio0 89 GPIO_ACTIVE_HIGH>;
> +                adi,oversampling-ratio-gpios = <&gpio0 88 GPIO_ACTIVE_HIGH
> +                                                &gpio0 87 GPIO_ACTIVE_HIGH
> +                                                &gpio0 86 GPIO_ACTIVE_HIGH>;
> +                io-backends = <&iio_backend>;
> +        };
> +    };
> +
>    - |
>      #include <dt-bindings/gpio/gpio.h>
>      #include <dt-bindings/interrupt-controller/irq.h>
> 
> -- 
> 2.34.1
>
Jonathan Cameron Aug. 17, 2024, 3:09 p.m. UTC | #2
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.

>    - avcc-supply
>    - vdrive-supply
David Lechner Sept. 4, 2024, 4:54 p.m. UTC | #3
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.


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

> 
>>    - avcc-supply
>>    - vdrive-supply
> 
> 
>
Jonathan Cameron Sept. 7, 2024, 1:37 p.m. UTC | #4
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  
> > 
> > 
> >   
>
diff mbox series

Patch

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
   - avcc-supply
   - vdrive-supply
-  - interrupts
-  - adi,conversion-start-gpios
 
 # This checks if reg is a chipselect so the device is on an SPI
 # bus, the if-clause will fail if reg is a tuple such as for a
@@ -137,6 +152,35 @@  then:
         - spi-cpol
 
 allOf:
+  # Communication is handled either by the backend or an interrupt.
+  - if:
+      properties:
+        pwms: false
+    then:
+      required:
+        - adi,conversion-start-gpios
+
+  - if:
+      properties:
+        adi,conversion-start-gpios: false
+    then:
+      required:
+        - pwms
+
+  - if:
+      properties:
+        interrupts: false
+    then:
+      required:
+        - io-backends
+
+  - if:
+      properties:
+        io-backends: false
+    then:
+      required:
+        - interrupts
+
   - if:
       properties:
         compatible:
@@ -178,12 +222,37 @@  allOf:
         adi,sw-mode: false
     else:
       properties:
+        pwms:
+          maxItems: 1
+        pwm-names:
+          maxItems: 1
         adi,conversion-start-gpios:
           maxItems: 1
 
 unevaluatedProperties: false
 
 examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    / {
+        adi_adc {
+                compatible = "adi,ad7606b";
+
+                pwms = <&axi_pwm_gen 0 0>;
+
+                avcc-supply = <&adc_vref>;
+                vdrive-supply = <&vdd_supply>;
+
+                reset-gpios = <&gpio0 91 GPIO_ACTIVE_HIGH>;
+                standby-gpios = <&gpio0 90 GPIO_ACTIVE_LOW>;
+                adi,range-gpios = <&gpio0 89 GPIO_ACTIVE_HIGH>;
+                adi,oversampling-ratio-gpios = <&gpio0 88 GPIO_ACTIVE_HIGH
+                                                &gpio0 87 GPIO_ACTIVE_HIGH
+                                                &gpio0 86 GPIO_ACTIVE_HIGH>;
+                io-backends = <&iio_backend>;
+        };
+    };
+
   - |
     #include <dt-bindings/gpio/gpio.h>
     #include <dt-bindings/interrupt-controller/irq.h>