diff mbox series

[v1,2/3] dt-bindings: iio: adc: add AD7191

Message ID 20241221155926.81954-3-alisa.roman@analog.com (mailing list archive)
State New
Headers show
Series Add support for AD7191 | expand

Commit Message

Alisa-Dariana Roman Dec. 21, 2024, 3:56 p.m. UTC
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          | 128 ++++++++++++++++++
 MAINTAINERS                                   |   7 +
 2 files changed, 135 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml

Comments

Conor Dooley Dec. 22, 2024, 2:48 p.m. UTC | #1
On Sat, Dec 21, 2024 at 05:56:01PM +0200, 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          | 128 ++++++++++++++++++
>  MAINTAINERS                                   |   7 +
>  2 files changed, 135 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..f3e596918c22
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml
> @@ -0,0 +1,128 @@
> +# 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.

Without clock-names, how do you tell the difference between driven ctal and
the cmos-compatible clock? That CLKSEL's job?

> +
> +  interrupts:
> +    maxItems: 1
> +
> +  avdd-supply:
> +    description: AVdd voltage supply
> +
> +  dvdd-supply:
> +    description: DVdd voltage supply
> +
> +  vref-supply:
> +    description: Vref voltage supply
> +
> +  odr1-gpios:
> +    description: GPIO connected to ODR1 pin for output data rate selection
> +    maxItems: 1
> +
> +  odr2-gpios:
> +    description: GPIO connected to ODR2 pin for output data rate selection
> +    maxItems: 1
> +
> +  pga1-gpios:
> +    description: GPIO connected to PGA1 pin for gain selection
> +    maxItems: 1
> +
> +  pga2-gpios:
> +    description: GPIO connected to PGA2 pin for gain selection
> +    maxItems: 1
> +
> +  temp-gpios:
> +    description: GPIO connected to TEMP pin for temperature sensor enable
> +    maxItems: 1
> +
> +  chan-gpios:
> +    description: GPIO connected to CHAN pin for input channel selection
> +    maxItems: 1
> +
> +  clksel-gpios:
> +    description: GPIO connected to CLKSEL pin for clock source selection
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - avdd-supply
> +  - dvdd-supply
> +  - vref-supply
> +  - spi-cpol
> +  - spi-cpha

> +  - odr1-gpios
> +  - odr2-gpios
> +  - pga1-gpios
> +  - pga2-gpios

For these 4, since all are required, seems like grouping as odr-pgios
and pga-gpios would be a good idea?
Jonathan Cameron Dec. 22, 2024, 6:18 p.m. UTC | #2
On Sun, 22 Dec 2024 14:48:08 +0000
Conor Dooley <conor@kernel.org> wrote:

> On Sat, Dec 21, 2024 at 05:56:01PM +0200, 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>

Maybe I'm over thinking things, but comments inline about possibility of
pinstrapping holding this device in a particular configuration, without
the GPIOS connected.

> > ---
> >  .../bindings/iio/adc/adi,ad7191.yaml          | 128 ++++++++++++++++++
> >  MAINTAINERS                                   |   7 +
> >  2 files changed, 135 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..f3e596918c22
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml
> > @@ -0,0 +1,128 @@
> > +# 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.  
> 
> Without clock-names, how do you tell the difference between driven ctal and
> the cmos-compatible clock? That CLKSEL's job?

Seems it's an unusual part and there is no config associated with whether we
have a clock or an xtal, so we probably don't need the name.

Related to that, in many cases I'd expect clksel to be tied to always
use the external clock or not (depending on whether one is connected)
not to be a gpio.  So you probably need to take that configuration into
account as well.

Similar may apply for the odr, and pga pins.  Sometimes people
hardwire those things.  There are examples in tree (I can't point at one
right now though) that deal with this. Fairly sure at least 1 ADI part
has a binding to handle that.  (the ad7606 does a bit of this as it needs
a particular pinstrap for sw-mode).

You should be fine with chan and temp always being GPIOs as it would be weird
to buy a part with the extra channels + temperature sensor and not wire it
to be useable.

> 
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  avdd-supply:
> > +    description: AVdd voltage supply
> > +
> > +  dvdd-supply:
> > +    description: DVdd voltage supply
> > +
> > +  vref-supply:
> > +    description: Vref voltage supply
> > +
> > +  odr1-gpios:
> > +    description: GPIO connected to ODR1 pin for output data rate selection
> > +    maxItems: 1
> > +
> > +  odr2-gpios:
> > +    description: GPIO connected to ODR2 pin for output data rate selection
> > +    maxItems: 1
> > +
> > +  pga1-gpios:
> > +    description: GPIO connected to PGA1 pin for gain selection
> > +    maxItems: 1
> > +
> > +  pga2-gpios:
> > +    description: GPIO connected to PGA2 pin for gain selection
> > +    maxItems: 1
> > +
> > +  temp-gpios:
> > +    description: GPIO connected to TEMP pin for temperature sensor enable
> > +    maxItems: 1
> > +
> > +  chan-gpios:
> > +    description: GPIO connected to CHAN pin for input channel selection
> > +    maxItems: 1
> > +
> > +  clksel-gpios:
> > +    description: GPIO connected to CLKSEL pin for clock source selection
> > +    maxItems: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - avdd-supply
> > +  - dvdd-supply
> > +  - vref-supply
> > +  - spi-cpol
> > +  - spi-cpha  
> 
> > +  - odr1-gpios
> > +  - odr2-gpios
> > +  - pga1-gpios
> > +  - pga2-gpios  
> 
> For these 4, since all are required, seems like grouping as odr-pgios
> and pga-gpios would be a good idea?
Agreed except for the annoying option of pin strapping.  Maybe we ignore that
for now.  If it becomes a problem, we can add it safely as a driver predating
that will try to grab the gpios and fail if it sees a DT not providing them.
So will fail safe before we add pinstrapping.  Maybe we will never need to.

Jonathan
diff mbox series

Patch

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..f3e596918c22
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/adi,ad7191.yaml
@@ -0,0 +1,128 @@ 
+# 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
+
+  odr1-gpios:
+    description: GPIO connected to ODR1 pin for output data rate selection
+    maxItems: 1
+
+  odr2-gpios:
+    description: GPIO connected to ODR2 pin for output data rate selection
+    maxItems: 1
+
+  pga1-gpios:
+    description: GPIO connected to PGA1 pin for gain selection
+    maxItems: 1
+
+  pga2-gpios:
+    description: GPIO connected to PGA2 pin for gain selection
+    maxItems: 1
+
+  temp-gpios:
+    description: GPIO connected to TEMP pin for temperature sensor enable
+    maxItems: 1
+
+  chan-gpios:
+    description: GPIO connected to CHAN pin for input channel selection
+    maxItems: 1
+
+  clksel-gpios:
+    description: GPIO connected to CLKSEL pin for clock source selection
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - avdd-supply
+  - dvdd-supply
+  - vref-supply
+  - spi-cpol
+  - spi-cpha
+  - odr1-gpios
+  - odr2-gpios
+  - pga1-gpios
+  - pga2-gpios
+  - temp-gpios
+  - chan-gpios
+  - clksel-gpios
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    spi0 {
+        #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>;
+            odr1-gpios = <&gpio 23 GPIO_ACTIVE_HIGH>;
+            odr2-gpios = <&gpio 24 GPIO_ACTIVE_HIGH>;
+            pga1-gpios = <&gpio 5 GPIO_ACTIVE_HIGH>;
+            pga2-gpios = <&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 8e5167443cea..254fbfe6144f 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