| Message ID | 20240905082404.119022-8-aardelean@baylibre.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | iio: adc: ad7606: add support for AD7606C-{16,18} parts | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/patch-applied | fail | build log |
On 05/09/2024 10:24, Alexandru Ardelean wrote: > The driver will support the AD7606C-16 and AD7606C-18. > This change adds the compatible strings for these devices. > > The AD7606C-16,18 channels also support these (individually configurable) > types of channels: > - bipolar single-ended > - unipolar single-ended > - bipolar differential > > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Best regards, Krzysztof
On 9/5/24 3:24 AM, Alexandru Ardelean wrote: > The driver will support the AD7606C-16 and AD7606C-18. > This change adds the compatible strings for these devices. > > The AD7606C-16,18 channels also support these (individually configurable) > types of channels: > - bipolar single-ended > - unipolar single-ended > - bipolar differential > > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> > --- > .../bindings/iio/adc/adi,ad7606.yaml | 109 ++++++++++++++++++ > 1 file changed, 109 insertions(+) > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml > index 69408cae3db9..57537ab0ec82 100644 > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml > @@ -14,6 +14,8 @@ description: | > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7605-4.pdf > https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606_7606-6_7606-4.pdf > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7606B.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-16.pdf > + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-18.pdf > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7616.pdf > > properties: > @@ -24,11 +26,19 @@ properties: > - adi,ad7606-6 > - adi,ad7606-8 # Referred to as AD7606 (without -8) in the datasheet > - adi,ad7606b > + - adi,ad7606c-16 > + - adi,ad7606c-18 > - adi,ad7616 > > reg: > maxItems: 1 > > + "#address-cells": > + const: 1 > + > + "#size-cells": > + const: 0 > + > spi-cpha: true > > spi-cpol: true > @@ -114,6 +124,46 @@ properties: > assumed that the pins are hardwired to VDD. > type: boolean > > +patternProperties: > + "^channel@[1-8]$": > + type: object > + $ref: adc.yaml > + unevaluatedProperties: false > + > + properties: > + reg: > + description: > + The channel number, as specified in the datasheet (from 1 to 8). > + minimum: 1 > + maximum: 8 > + > + diff-channels: > + description: > + Each channel can be configured as a differential bipolar channel. > + The ADC uses the same positive and negative inputs for this. > + This property must be specified as 'reg' (or the channel number) for > + both positive and negative inputs (i.e. diff-channels = <reg reg>). > + items: > + minimum: 1 > + maximum: 8 > + > + bipolar: > + description: > + Each channel can be configured as a unipolar or bipolar single-ended. > + When this property is not specified, it's unipolar, so the ADC will > + have only the positive input wired. > + For this ADC the 'diff-channels' & 'bipolar' properties are mutually > + exclusive. > + > + required: > + - reg > + > + oneOf: > + - required: > + - diff-channels > + - required: > + - bipolar The datasheet (ad7606c-18.pdf) lists the following combinations: * Bipolar single-ended * Unipolar single-ended * Bipolar differential The logic in the oneOf: doesn't match this. This I think this would be sufficient: - if: required: [diff-channels] then: required: [bipolar] > + > required: > - compatible > - reg > @@ -170,6 +220,17 @@ allOf: > adi,conversion-start-gpios: > maxItems: 1 > > + - if: > + not: > + properties: > + compatible: > + enum: > + - adi,ad7606c-16 > + - adi,ad7606c-18 > + then: > + patternProperties: > + "^channel@[1-8]$": false > + > unevaluatedProperties: false > > examples: > @@ -202,4 +263,52 @@ examples: > standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>; > }; > }; > + - | > + #include <dt-bindings/gpio/gpio.h> > + #include <dt-bindings/interrupt-controller/irq.h> > + spi { > + #address-cells = <1>; > + #size-cells = <0>; > + > + adc@0 { > + compatible = "adi,ad7606c-18"; > + reg = <0>; > + > + #address-cells = <1>; > + #size-cells = <0>; > + > + spi-max-frequency = <1000000>; > + spi-cpol; > + spi-cpha; > + > + avcc-supply = <&adc_vref>; > + vdrive-supply = <&vdd_supply>; > + > + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; > + interrupt-parent = <&gpio>; > + > + adi,conversion-start-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>; > + reset-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>; > + adi,first-data-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>; > + standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>; > + > + adi,sw-mode; > + > + channel@1 { > + reg = <1>; > + diff-channels = <1 1>; bipolar; > + }; > + > + channel@3 { > + reg = <3>; > + bipolar; > + }; > + > + channel@8 { > + reg = <8>; > + diff-channels = <8 8>; bipolar; > + }; > + > + }; > + }; > ...
On 9/5/24 3:24 AM, Alexandru Ardelean wrote: > The driver will support the AD7606C-16 and AD7606C-18. > This change adds the compatible strings for these devices. > > The AD7606C-16,18 channels also support these (individually configurable) > types of channels: > - bipolar single-ended > - unipolar single-ended > - bipolar differential > > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> > --- ... > @@ -170,6 +220,17 @@ allOf: > adi,conversion-start-gpios: > maxItems: 1 > > + - if: > + not: > + properties: > + compatible: > + enum: > + - adi,ad7606c-16 > + - adi,ad7606c-18 > + then: > + patternProperties: > + "^channel@[1-8]$": false > + Technically, we should also have "^channel@[1-8]$": false if adi,sw-mode is not set (not: required: [adi,sw-mode]).
On Fri, Sep 6, 2024 at 12:54 AM David Lechner <dlechner@baylibre.com> wrote: > > On 9/5/24 3:24 AM, Alexandru Ardelean wrote: > > The driver will support the AD7606C-16 and AD7606C-18. > > This change adds the compatible strings for these devices. > > > > The AD7606C-16,18 channels also support these (individually configurable) > > types of channels: > > - bipolar single-ended > > - unipolar single-ended > > - bipolar differential > > > > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> > > --- > > .../bindings/iio/adc/adi,ad7606.yaml | 109 ++++++++++++++++++ > > 1 file changed, 109 insertions(+) > > > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml > > index 69408cae3db9..57537ab0ec82 100644 > > --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml > > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml > > @@ -14,6 +14,8 @@ description: | > > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7605-4.pdf > > https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606_7606-6_7606-4.pdf > > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7606B.pdf > > + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-16.pdf > > + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-18.pdf > > https://www.analog.com/media/en/technical-documentation/data-sheets/AD7616.pdf > > > > properties: > > @@ -24,11 +26,19 @@ properties: > > - adi,ad7606-6 > > - adi,ad7606-8 # Referred to as AD7606 (without -8) in the datasheet > > - adi,ad7606b > > + - adi,ad7606c-16 > > + - adi,ad7606c-18 > > - adi,ad7616 > > > > reg: > > maxItems: 1 > > > > + "#address-cells": > > + const: 1 > > + > > + "#size-cells": > > + const: 0 > > + > > spi-cpha: true > > > > spi-cpol: true > > @@ -114,6 +124,46 @@ properties: > > assumed that the pins are hardwired to VDD. > > type: boolean > > > > +patternProperties: > > + "^channel@[1-8]$": > > + type: object > > + $ref: adc.yaml > > + unevaluatedProperties: false > > + > > + properties: > > + reg: > > + description: > > + The channel number, as specified in the datasheet (from 1 to 8). > > + minimum: 1 > > + maximum: 8 > > + > > + diff-channels: > > + description: > > + Each channel can be configured as a differential bipolar channel. > > + The ADC uses the same positive and negative inputs for this. > > + This property must be specified as 'reg' (or the channel number) for > > + both positive and negative inputs (i.e. diff-channels = <reg reg>). > > + items: > > + minimum: 1 > > + maximum: 8 > > + > > + bipolar: > > + description: > > + Each channel can be configured as a unipolar or bipolar single-ended. > > + When this property is not specified, it's unipolar, so the ADC will > > + have only the positive input wired. > > + For this ADC the 'diff-channels' & 'bipolar' properties are mutually > > + exclusive. > > + > > + required: > > + - reg > > + > > + oneOf: > > + - required: > > + - diff-channels > > + - required: > > + - bipolar > > The datasheet (ad7606c-18.pdf) lists the following combinations: > > * Bipolar single-ended > * Unipolar single-ended > * Bipolar differential > > The logic in the oneOf: doesn't match this. > > This I think this would be sufficient: > > - if: > required: [diff-channels] > then: > required: [bipolar] So here, I am a bit vague. This makes 'bipolar' mandatory if 'diff-channels' is mandatory, right? But then 'bipolar' (on its own) becomes optional? The way I understood the oneOf case is that: 1. if it's 'diff-channels' then it's specified 'bipolar differential'. 2. if it's 'bipolar' then it's specified as 'bipolar single-ended' 3. otherwise it's unipolar 4. oneOf enforces that at least 'diff-channels' or 'bipolar' is specified if there is a channel node > > > + > > required: > > - compatible > > - reg > > @@ -170,6 +220,17 @@ allOf: > > adi,conversion-start-gpios: > > maxItems: 1 > > > > + - if: > > + not: > > + properties: > > + compatible: > > + enum: > > + - adi,ad7606c-16 > > + - adi,ad7606c-18 > > + then: > > + patternProperties: > > + "^channel@[1-8]$": false > > + > > unevaluatedProperties: false > > > > examples: > > @@ -202,4 +263,52 @@ examples: > > standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>; > > }; > > }; > > + - | > > + #include <dt-bindings/gpio/gpio.h> > > + #include <dt-bindings/interrupt-controller/irq.h> > > + spi { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + adc@0 { > > + compatible = "adi,ad7606c-18"; > > + reg = <0>; > > + > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + spi-max-frequency = <1000000>; > > + spi-cpol; > > + spi-cpha; > > + > > + avcc-supply = <&adc_vref>; > > + vdrive-supply = <&vdd_supply>; > > + > > + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; > > + interrupt-parent = <&gpio>; > > + > > + adi,conversion-start-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>; > > + reset-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>; > > + adi,first-data-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>; > > + standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>; > > + > > + adi,sw-mode; > > + > > + channel@1 { > > + reg = <1>; > > + diff-channels = <1 1>; > > bipolar; > > > + }; > > + > > + channel@3 { > > + reg = <3>; > > + bipolar; > > + }; > > + > > + channel@8 { > > + reg = <8>; > > + diff-channels = <8 8>; > > bipolar; > > > + }; > > + > > + }; > > + }; > > ... >
On Fri, Sep 6, 2024 at 1:02 AM David Lechner <dlechner@baylibre.com> wrote: > > On 9/5/24 3:24 AM, Alexandru Ardelean wrote: > > The driver will support the AD7606C-16 and AD7606C-18. > > This change adds the compatible strings for these devices. > > > > The AD7606C-16,18 channels also support these (individually configurable) > > types of channels: > > - bipolar single-ended > > - unipolar single-ended > > - bipolar differential > > > > Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> > > --- > > ... > > > @@ -170,6 +220,17 @@ allOf: > > adi,conversion-start-gpios: > > maxItems: 1 > > > > + - if: > > + not: > > + properties: > > + compatible: > > + enum: > > + - adi,ad7606c-16 > > + - adi,ad7606c-18 > > + then: > > + patternProperties: > > + "^channel@[1-8]$": false > > + > > Technically, we should also have "^channel@[1-8]$": false > if adi,sw-mode is not set (not: required: [adi,sw-mode]). Yes. That's true. Maybe I'll add another condition here for that.
On 9/6/24 11:59 PM, Alexandru Ardelean wrote: > On Fri, Sep 6, 2024 at 12:54 AM David Lechner <dlechner@baylibre.com> wrote: >> >> On 9/5/24 3:24 AM, Alexandru Ardelean wrote: ... >>> +patternProperties: >>> + "^channel@[1-8]$": >>> + type: object >>> + $ref: adc.yaml >>> + unevaluatedProperties: false >>> + >>> + properties: >>> + reg: >>> + description: >>> + The channel number, as specified in the datasheet (from 1 to 8). >>> + minimum: 1 >>> + maximum: 8 >>> + >>> + diff-channels: >>> + description: >>> + Each channel can be configured as a differential bipolar channel. >>> + The ADC uses the same positive and negative inputs for this. >>> + This property must be specified as 'reg' (or the channel number) for >>> + both positive and negative inputs (i.e. diff-channels = <reg reg>). >>> + items: >>> + minimum: 1 >>> + maximum: 8 >>> + >>> + bipolar: >>> + description: >>> + Each channel can be configured as a unipolar or bipolar single-ended. >>> + When this property is not specified, it's unipolar, so the ADC will >>> + have only the positive input wired. >>> + For this ADC the 'diff-channels' & 'bipolar' properties are mutually >>> + exclusive. >>> + >>> + required: >>> + - reg >>> + >>> + oneOf: >>> + - required: >>> + - diff-channels >>> + - required: >>> + - bipolar >> >> The datasheet (ad7606c-18.pdf) lists the following combinations: >> >> * Bipolar single-ended >> * Unipolar single-ended >> * Bipolar differential >> >> The logic in the oneOf: doesn't match this. >> >> This I think this would be sufficient: >> >> - if: >> required: [diff-channels] >> then: >> required: [bipolar] > > So here, I am a bit vague. > This makes 'bipolar' mandatory if 'diff-channels' is mandatory, right? > But then 'bipolar' (on its own) becomes optional? > The way I understood the oneOf case is that: > 1. if it's 'diff-channels' then it's specified 'bipolar differential'. diff-channels does not imply bipolar in DT, so we need both properties set to specify "bipolar differential". > 2. if it's 'bipolar' then it's specified as 'bipolar single-ended' > 3. otherwise it's unipolar > 4. oneOf enforces that at least 'diff-channels' or 'bipolar' is > specified if there is a channel node > >
On Sat, Sep 7, 2024 at 5:09 PM David Lechner <dlechner@baylibre.com> wrote: > > On 9/6/24 11:59 PM, Alexandru Ardelean wrote: > > On Fri, Sep 6, 2024 at 12:54 AM David Lechner <dlechner@baylibre.com> wrote: > >> > >> On 9/5/24 3:24 AM, Alexandru Ardelean wrote: > > ... > > >>> +patternProperties: > >>> + "^channel@[1-8]$": > >>> + type: object > >>> + $ref: adc.yaml > >>> + unevaluatedProperties: false > >>> + > >>> + properties: > >>> + reg: > >>> + description: > >>> + The channel number, as specified in the datasheet (from 1 to 8). > >>> + minimum: 1 > >>> + maximum: 8 > >>> + > >>> + diff-channels: > >>> + description: > >>> + Each channel can be configured as a differential bipolar channel. > >>> + The ADC uses the same positive and negative inputs for this. > >>> + This property must be specified as 'reg' (or the channel number) for > >>> + both positive and negative inputs (i.e. diff-channels = <reg reg>). > >>> + items: > >>> + minimum: 1 > >>> + maximum: 8 > >>> + > >>> + bipolar: > >>> + description: > >>> + Each channel can be configured as a unipolar or bipolar single-ended. > >>> + When this property is not specified, it's unipolar, so the ADC will > >>> + have only the positive input wired. > >>> + For this ADC the 'diff-channels' & 'bipolar' properties are mutually > >>> + exclusive. > >>> + > >>> + required: > >>> + - reg > >>> + > >>> + oneOf: > >>> + - required: > >>> + - diff-channels > >>> + - required: > >>> + - bipolar > >> > >> The datasheet (ad7606c-18.pdf) lists the following combinations: > >> > >> * Bipolar single-ended > >> * Unipolar single-ended > >> * Bipolar differential > >> > >> The logic in the oneOf: doesn't match this. > >> > >> This I think this would be sufficient: > >> > >> - if: > >> required: [diff-channels] > >> then: > >> required: [bipolar] > > > > So here, I am a bit vague. > > This makes 'bipolar' mandatory if 'diff-channels' is mandatory, right? > > But then 'bipolar' (on its own) becomes optional? > > The way I understood the oneOf case is that: > > 1. if it's 'diff-channels' then it's specified 'bipolar differential'. > > diff-channels does not imply bipolar in DT, so we need both properties > set to specify "bipolar differential". > ack will update then > > 2. if it's 'bipolar' then it's specified as 'bipolar single-ended' > > 3. otherwise it's unipolar > > 4. oneOf enforces that at least 'diff-channels' or 'bipolar' is > > specified if there is a channel node > > > >
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml index 69408cae3db9..57537ab0ec82 100644 --- a/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml @@ -14,6 +14,8 @@ description: | https://www.analog.com/media/en/technical-documentation/data-sheets/AD7605-4.pdf https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606_7606-6_7606-4.pdf https://www.analog.com/media/en/technical-documentation/data-sheets/AD7606B.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-16.pdf + https://www.analog.com/media/en/technical-documentation/data-sheets/ad7606c-18.pdf https://www.analog.com/media/en/technical-documentation/data-sheets/AD7616.pdf properties: @@ -24,11 +26,19 @@ properties: - adi,ad7606-6 - adi,ad7606-8 # Referred to as AD7606 (without -8) in the datasheet - adi,ad7606b + - adi,ad7606c-16 + - adi,ad7606c-18 - adi,ad7616 reg: maxItems: 1 + "#address-cells": + const: 1 + + "#size-cells": + const: 0 + spi-cpha: true spi-cpol: true @@ -114,6 +124,46 @@ properties: assumed that the pins are hardwired to VDD. type: boolean +patternProperties: + "^channel@[1-8]$": + type: object + $ref: adc.yaml + unevaluatedProperties: false + + properties: + reg: + description: + The channel number, as specified in the datasheet (from 1 to 8). + minimum: 1 + maximum: 8 + + diff-channels: + description: + Each channel can be configured as a differential bipolar channel. + The ADC uses the same positive and negative inputs for this. + This property must be specified as 'reg' (or the channel number) for + both positive and negative inputs (i.e. diff-channels = <reg reg>). + items: + minimum: 1 + maximum: 8 + + bipolar: + description: + Each channel can be configured as a unipolar or bipolar single-ended. + When this property is not specified, it's unipolar, so the ADC will + have only the positive input wired. + For this ADC the 'diff-channels' & 'bipolar' properties are mutually + exclusive. + + required: + - reg + + oneOf: + - required: + - diff-channels + - required: + - bipolar + required: - compatible - reg @@ -170,6 +220,17 @@ allOf: adi,conversion-start-gpios: maxItems: 1 + - if: + not: + properties: + compatible: + enum: + - adi,ad7606c-16 + - adi,ad7606c-18 + then: + patternProperties: + "^channel@[1-8]$": false + unevaluatedProperties: false examples: @@ -202,4 +263,52 @@ examples: standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>; }; }; + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + spi { + #address-cells = <1>; + #size-cells = <0>; + + adc@0 { + compatible = "adi,ad7606c-18"; + reg = <0>; + + #address-cells = <1>; + #size-cells = <0>; + + spi-max-frequency = <1000000>; + spi-cpol; + spi-cpha; + + avcc-supply = <&adc_vref>; + vdrive-supply = <&vdd_supply>; + + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; + interrupt-parent = <&gpio>; + + adi,conversion-start-gpios = <&gpio 17 GPIO_ACTIVE_HIGH>; + reset-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>; + adi,first-data-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>; + standby-gpios = <&gpio 24 GPIO_ACTIVE_LOW>; + + adi,sw-mode; + + channel@1 { + reg = <1>; + diff-channels = <1 1>; + }; + + channel@3 { + reg = <3>; + bipolar; + }; + + channel@8 { + reg = <8>; + diff-channels = <8 8>; + }; + + }; + }; ...
The driver will support the AD7606C-16 and AD7606C-18. This change adds the compatible strings for these devices. The AD7606C-16,18 channels also support these (individually configurable) types of channels: - bipolar single-ended - unipolar single-ended - bipolar differential Signed-off-by: Alexandru Ardelean <aardelean@baylibre.com> --- .../bindings/iio/adc/adi,ad7606.yaml | 109 ++++++++++++++++++ 1 file changed, 109 insertions(+)