Message ID | 20250122132821.126600-2-alisa.roman@analog.com (mailing list archive) |
---|---|
State | Changes Requested |
Headers | show |
Series | Add support for AD7191 | expand |
On Wed, Jan 22, 2025 at 03:20:39PM +0200, Alisa-Dariana Roman wrote: > + adi,odr-state: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + Should be present if ODR pins are pin-strapped. Value corresponds to: > + 0: 120 Hz (ODR1=0, ODR2=0) > + 1: 60 Hz (ODR1=0, ODR2=1) > + 2: 50 Hz (ODR1=1, ODR2=0) > + 3: 10 Hz (ODR1=1, ODR2=1) > + If defined, odr-gpios must be absent. > + enum: [0, 1, 2, 3] This should be a property in hertz > + pga-gpios: > + description: | > + PGA1 and PGA2 pins for gain selection. Should be defined if adi,pga-state > + is absent. > + maxItems: 2 > + > + adi,pga-state: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + Should be present if PGA pins are pin-strapped. Value corresponds to: > + 0: Gain 1 (PGA1=0, PGA2=0) > + 1: Gain 8 (PGA1=0, PGA2=1) > + 2: Gain 64 (PGA1=1, PGA2=0) > + 3: Gain 128 (PGA1=1, PGA2=1) > + If defined, pga-gpios must be absent. > + enum: [0, 1, 2, 3] And I think this one should be in units of "gain". > + adi,clksel-state: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + Should be present if CLKSEL is pin-strapped. 0 selects an external clock, > + 1 selects the internal clock. If defined, clksel-gpios must be absent. > + enum: [0, 1] IMO this one should be a string, options of "external" and "internal". 0 & 1 means nothing to a dts reader/author and needs to be cross checked with the binding to obtain the meanings.
On 1/22/25 7:20 AM, Alisa-Dariana Roman wrote: > AD7191 is a pin-programmable, ultralow noise 24-bit sigma-delta ADC > designed for precision bridge sensor measurements. It features two > differential analog input channels, selectable output rates, > programmable gain, internal temperature sensor and simultaneous > 50Hz/60Hz rejection. > > Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com> > --- > .../bindings/iio/adc/adi,ad7191.yaml | 175 ++++++++++++++++++ > MAINTAINERS | 7 + > 2 files changed, 182 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml > new file mode 100644 > index 000000000000..c0a6bed7a9cb > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml > @@ -0,0 +1,175 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +# Copyright 2025 Analog Devices Inc. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/adi,ad7191.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices AD7191 ADC device driver > + > +maintainers: > + - Alisa-Dariana Roman <alisa.roman@analog.com> > + > +description: | > + Bindings for the Analog Devices AD7191 ADC device. Datasheet can be > + found here: > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf If we are not going to have a powerdown-gpios, I think we should mention in the description that it is expected that the PDOWN pin is connected to the SPI controller chip select. > + > +properties: ... > + > + clksel-gpios: Do we really need this one? I don't think we have a chip yet that wants to change the clock at runtime. We don't have this for any of the other similar ADI sigma delta chips that have already been upstreamed. If there is an evaluation board where the pin is wired to a GPIO, we can just use gpio-hog to select the correct state. > + description: | > + Clock source selection pin (internal or external). Should be defined if > + clksel-config is absent. > + maxItems: 1 > + > + adi,clksel-state: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + Should be present if CLKSEL is pin-strapped. 0 selects an external clock, > + 1 selects the internal clock. If defined, clksel-gpios must be absent. > + enum: [0, 1] I don't think we need this one either. If the clocks property is present, then we can assume that CLKSEL is going to be hardwired to indicate an external clock and if the clocks property is absent, then we know it must be hardwired to select the internal clock. > + > +required: > + - compatible > + - reg > + - interrupts > + - avdd-supply > + - dvdd-supply > + - vref-supply > + - spi-cpol > + - spi-cpha > + - temp-gpios > + - chan-gpios > + > +allOf: > + - $ref: /schemas/spi/spi-peripheral-props.yaml# > + - if: > + required: > + - adi,odr-state > + then: > + properties: > + odr-gpios: false > + else: > + required: > + - odr-gpios I think we could simplify these with: - oneOf: - required: - adi,odr-state - required: - odr-gpios > + - if: > + required: > + - adi,pga-state > + then: > + properties: > + pga-gpios: false > + else: > + required: > + - pga-gpios > + - if: > + required: > + - adi,clksel-state > + then: > + properties: > + clksel-gpios: false > + else: > + required: > + - clksel-gpios > + > +unevaluatedProperties: false > +
On Wed, 22 Jan 2025 15:20:39 +0200 Alisa-Dariana Roman <alisadariana@gmail.com> wrote: > AD7191 is a pin-programmable, ultralow noise 24-bit sigma-delta ADC > designed for precision bridge sensor measurements. It features two > differential analog input channels, selectable output rates, > programmable gain, internal temperature sensor and simultaneous > 50Hz/60Hz rejection. > > Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com> > --- > .../bindings/iio/adc/adi,ad7191.yaml | 175 ++++++++++++++++++ > MAINTAINERS | 7 + > 2 files changed, 182 insertions(+) > create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml > > diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml > new file mode 100644 > index 000000000000..c0a6bed7a9cb > --- /dev/null > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml > @@ -0,0 +1,175 @@ > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) > +# Copyright 2025 Analog Devices Inc. > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/iio/adc/adi,ad7191.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Analog Devices AD7191 ADC device driver Not a "device driver" so drop that bit or say 'binding' instead. > + > +maintainers: > + - Alisa-Dariana Roman <alisa.roman@analog.com> > + > +description: | > + Bindings for the Analog Devices AD7191 ADC device. Datasheet can be > + found here: > + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf > + > +properties: > + compatible: > + enum: > + - adi,ad7191 > + > + reg: > + maxItems: 1 > + > + spi-cpol: true > + > + spi-cpha: true > + > + clocks: > + maxItems: 1 > + description: > + Optionally, either a crystal can be attached externally between MCLK1 and > + MCLK2 pins, or an external CMOS-compatible clock can drive the MCLK2 > + pin. If absent, internal 4.92MHz clock is used. > + > + interrupts: > + maxItems: 1 > + > + avdd-supply: > + description: AVdd voltage supply > + > + dvdd-supply: > + description: DVdd voltage supply > + > + vref-supply: > + description: Vref voltage supply > + > + odr-gpios: > + description: | > + ODR1 and ODR2 pins for output data rate selection. Should be defined if > + adi,odr-state is absent. > + maxItems: 2 minItems also 2? i guess we aren't coping with situation of one pin wired until some board designer decides to do that. > + > + adi,odr-state: > + $ref: /schemas/types.yaml#/definitions/uint32 > + description: | > + Should be present if ODR pins are pin-strapped. Value corresponds to: > + 0: 120 Hz (ODR1=0, ODR2=0) > + 1: 60 Hz (ODR1=0, ODR2=1) > + 2: 50 Hz (ODR1=1, ODR2=0) > + 3: 10 Hz (ODR1=1, ODR2=1) > + If defined, odr-gpios must be absent. > + enum: [0, 1, 2, 3] > + > + pga-gpios: > + description: | > + PGA1 and PGA2 pins for gain selection. Should be defined if adi,pga-state > + is absent. > + maxItems: 2 minItems here as well I think. > + Jonathan
diff --git a/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml b/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml new file mode 100644 index 000000000000..c0a6bed7a9cb --- /dev/null +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml @@ -0,0 +1,175 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +# Copyright 2025 Analog Devices Inc. +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/iio/adc/adi,ad7191.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: Analog Devices AD7191 ADC device driver + +maintainers: + - Alisa-Dariana Roman <alisa.roman@analog.com> + +description: | + Bindings for the Analog Devices AD7191 ADC device. Datasheet can be + found here: + https://www.analog.com/media/en/technical-documentation/data-sheets/AD7191.pdf + +properties: + compatible: + enum: + - adi,ad7191 + + reg: + maxItems: 1 + + spi-cpol: true + + spi-cpha: true + + clocks: + maxItems: 1 + description: + Optionally, either a crystal can be attached externally between MCLK1 and + MCLK2 pins, or an external CMOS-compatible clock can drive the MCLK2 + pin. If absent, internal 4.92MHz clock is used. + + interrupts: + maxItems: 1 + + avdd-supply: + description: AVdd voltage supply + + dvdd-supply: + description: DVdd voltage supply + + vref-supply: + description: Vref voltage supply + + odr-gpios: + description: | + ODR1 and ODR2 pins for output data rate selection. Should be defined if + adi,odr-state is absent. + maxItems: 2 + + adi,odr-state: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + Should be present if ODR pins are pin-strapped. Value corresponds to: + 0: 120 Hz (ODR1=0, ODR2=0) + 1: 60 Hz (ODR1=0, ODR2=1) + 2: 50 Hz (ODR1=1, ODR2=0) + 3: 10 Hz (ODR1=1, ODR2=1) + If defined, odr-gpios must be absent. + enum: [0, 1, 2, 3] + + pga-gpios: + description: | + PGA1 and PGA2 pins for gain selection. Should be defined if adi,pga-state + is absent. + maxItems: 2 + + adi,pga-state: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + Should be present if PGA pins are pin-strapped. Value corresponds to: + 0: Gain 1 (PGA1=0, PGA2=0) + 1: Gain 8 (PGA1=0, PGA2=1) + 2: Gain 64 (PGA1=1, PGA2=0) + 3: Gain 128 (PGA1=1, PGA2=1) + If defined, pga-gpios must be absent. + enum: [0, 1, 2, 3] + + temp-gpios: + description: TEMP pin for temperature sensor enable. + maxItems: 1 + + chan-gpios: + description: CHAN pin for input channel selection. + maxItems: 1 + + clksel-gpios: + description: | + Clock source selection pin (internal or external). Should be defined if + clksel-config is absent. + maxItems: 1 + + adi,clksel-state: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + Should be present if CLKSEL is pin-strapped. 0 selects an external clock, + 1 selects the internal clock. If defined, clksel-gpios must be absent. + enum: [0, 1] + +required: + - compatible + - reg + - interrupts + - avdd-supply + - dvdd-supply + - vref-supply + - spi-cpol + - spi-cpha + - temp-gpios + - chan-gpios + +allOf: + - $ref: /schemas/spi/spi-peripheral-props.yaml# + - if: + required: + - adi,odr-state + then: + properties: + odr-gpios: false + else: + required: + - odr-gpios + - if: + required: + - adi,pga-state + then: + properties: + pga-gpios: false + else: + required: + - pga-gpios + - if: + required: + - adi,clksel-state + then: + properties: + clksel-gpios: false + else: + required: + - clksel-gpios + +unevaluatedProperties: false + +examples: + - | + #include <dt-bindings/gpio/gpio.h> + #include <dt-bindings/interrupt-controller/irq.h> + + spi { + #address-cells = <1>; + #size-cells = <0>; + + adc@0 { + compatible = "adi,ad7191"; + reg = <0>; + spi-max-frequency = <1000000>; + spi-cpol; + spi-cpha; + clocks = <&ad7191_mclk>; + interrupts = <25 IRQ_TYPE_EDGE_FALLING>; + interrupt-parent = <&gpio>; + avdd-supply = <&avdd>; + dvdd-supply = <&dvdd>; + vref-supply = <&vref>; + odr-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>, <&gpio 24 GPIO_ACTIVE_HIGH>; + pga-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>, <&gpio 6 GPIO_ACTIVE_HIGH>; + temp-gpios = <&gpio 22 GPIO_ACTIVE_HIGH>; + chan-gpios = <&gpio 27 GPIO_ACTIVE_HIGH>; + clksel-gpios = <&gpio 13 GPIO_ACTIVE_HIGH>; + }; + }; diff --git a/MAINTAINERS b/MAINTAINERS index 98a3c1e46311..262beced3143 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -1302,6 +1302,13 @@ W: http://ez.analog.com/community/linux-device-drivers F: Documentation/devicetree/bindings/iio/adc/adi,ad7091r* F: drivers/iio/adc/ad7091r* +ANALOG DEVICES INC AD7191 DRIVER +M: Alisa-Dariana Roman <alisa.roman@analog.com> +L: linux-iio@vger.kernel.org +S: Supported +W: https://ez.analog.com/linux-software-drivers +F: Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml + ANALOG DEVICES INC AD7192 DRIVER M: Alisa-Dariana Roman <alisa.roman@analog.com> L: linux-iio@vger.kernel.org
AD7191 is a pin-programmable, ultralow noise 24-bit sigma-delta ADC designed for precision bridge sensor measurements. It features two differential analog input channels, selectable output rates, programmable gain, internal temperature sensor and simultaneous 50Hz/60Hz rejection. Signed-off-by: Alisa-Dariana Roman <alisa.roman@analog.com> --- .../bindings/iio/adc/adi,ad7191.yaml | 175 ++++++++++++++++++ MAINTAINERS | 7 + 2 files changed, 182 insertions(+) create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml