| Message ID | 20240815-ad7606_add_iio_backend_support-v1-2-cea3e11b1aa4@baylibre.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | Add iio backend compatibility for ad7606 | expand |
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 >
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
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 > > >
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 --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>
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(-)