diff mbox series

[v3,2/2] dt-bindings: iio: adc: add Texas Instruments ADS7924

Message ID 20230113194959.3276433-3-hugo@hugovil.com
State Superseded
Headers show
Series iio: adc: ti-ads7924: add Texas Instruments ADS7924 driver | expand

Commit Message

Hugo Villeneuve Jan. 13, 2023, 7:49 p.m. UTC 
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Add device tree bindings document for the Texas Instruments ADS7924
ADC.

Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
---
 .../bindings/iio/adc/ti,ads7924.yaml          | 112 ++++++++++++++++++
 1 file changed, 112 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml

Comments

Krzysztof Kozlowski Jan. 15, 2023, 2:57 p.m. UTC | #1 
On 13/01/2023 20:49, Hugo Villeneuve wrote:
> From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> 
> Add device tree bindings document for the Texas Instruments ADS7924
> ADC.
> 
> Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> ---
>  .../bindings/iio/adc/ti,ads7924.yaml          | 112 ++++++++++++++++++
>  1 file changed, 112 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml
> new file mode 100644
> index 000000000000..24bbf95383b4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml
> @@ -0,0 +1,112 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,ads7924.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI ADS7924 4 channels 12 bits I2C analog to digital converter
> +
> +maintainers:
> +  - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> +
> +description: |
> +  Texas Instruments ADS7924 4 channels 12 bits I2C analog to digital converter
> +
> +  Specifications:
> +    https://www.ti.com/lit/gpn/ads7924
> +
> +properties:
> +  compatible:
> +    const: ti,ads7924
> +
> +  reg:
> +    maxItems: 1
> +
> +  vref-supply:
> +    description:
> +      The regulator supply for the ADC reference voltage (AVDD)
> +
> +  reset-gpios:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  "#address-cells":
> +    const: 1
> +
> +  "#size-cells":
> +    const: 0
> +
> +  "#io-channel-cells":
> +    const: 1
> +
> +patternProperties:
> +  "^channel@[0-3]+$":
> +    $ref: adc.yaml
> +
> +    description: |
> +      Represents the external channels which are connected to the ADC.
> +
> +    properties:
> +      reg:
> +        description: |
> +          The channel number. It can have up to 4 channels numbered from 0 to 3.
> +        items:
> +          - minimum: 0
> +            maximum: 3
> +
> +      label:
> +        description: |
> +          Unique name to identify the channel.

Drop description, it's coming from adc.yaml. Just "label: true"

> +
> +    required:
> +      - reg
> +
> +    additionalProperties: false

You are not allowing anything else from adc.yaml. Is it on purpose?

> +
> +additionalProperties: false
> +
> +required:
> +  - compatible
> +  - reg
> +  - vref-supply
> +  - "#address-cells"
> +  - "#size-cells"
> +

Best regards,
Krzysztof
Jonathan Cameron Jan. 15, 2023, 4:43 p.m. UTC | #2 
On Sun, 15 Jan 2023 11:22:05 -0500
Hugo Villeneuve <hugo@hugovil.com> wrote:

> On Sun, 15 Jan 2023 15:57:21 +0100
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
> > On 13/01/2023 20:49, Hugo Villeneuve wrote:  
> > > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > 
> > > Add device tree bindings document for the Texas Instruments ADS7924
> > > ADC.
> > > 
> > > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > ---
> > >  .../bindings/iio/adc/ti,ads7924.yaml          | 112 ++++++++++++++++++
> > >  1 file changed, 112 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml
> > > 
> > > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml
> > > new file mode 100644
> > > index 000000000000..24bbf95383b4
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml
> > > @@ -0,0 +1,112 @@
> > > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/iio/adc/ti,ads7924.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: TI ADS7924 4 channels 12 bits I2C analog to digital converter
> > > +
> > > +maintainers:
> > > +  - Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > > +
> > > +description: |
> > > +  Texas Instruments ADS7924 4 channels 12 bits I2C analog to digital converter
> > > +
> > > +  Specifications:
> > > +    https://www.ti.com/lit/gpn/ads7924
> > > +
> > > +properties:
> > > +  compatible:
> > > +    const: ti,ads7924
> > > +
> > > +  reg:
> > > +    maxItems: 1
> > > +
> > > +  vref-supply:
> > > +    description:
> > > +      The regulator supply for the ADC reference voltage (AVDD)
> > > +
> > > +  reset-gpios:
> > > +    maxItems: 1
> > > +
> > > +  interrupts:
> > > +    maxItems: 1
> > > +
> > > +  "#address-cells":
> > > +    const: 1
> > > +
> > > +  "#size-cells":
> > > +    const: 0
> > > +
> > > +  "#io-channel-cells":
> > > +    const: 1
> > > +
> > > +patternProperties:
> > > +  "^channel@[0-3]+$":
> > > +    $ref: adc.yaml
> > > +
> > > +    description: |
> > > +      Represents the external channels which are connected to the ADC.
> > > +
> > > +    properties:
> > > +      reg:
> > > +        description: |
> > > +          The channel number. It can have up to 4 channels numbered from 0 to 3.
> > > +        items:
> > > +          - minimum: 0
> > > +            maximum: 3
> > > +
> > > +      label:
> > > +        description: |
> > > +          Unique name to identify the channel.  
> > 
> > Drop description, it's coming from adc.yaml. Just "label: true"  
> 
> Done.
> 
> > > +
> > > +    required:
> > > +      - reg
> > > +
> > > +    additionalProperties: false  
> > 
> > You are not allowing anything else from adc.yaml. Is it on purpose?  
> 
> I am really not an expert with this Yaml stuff, and reading the documentation makes me probably more confused than before reading it :)
> 
> But one thing that is for sure is that these other properties in adc.yaml are not used in my driver:
> 
>   bipolar
>   diff-channels
>   settling-time-us
>   oversampling-ratio
> 
> So is it Ok then to use "additionalProperties: false"? I think so, but what is your recommandation?

Makes sense to me.  Whilst there are lots of things a channel can support, most
of them are hardware related and not universal.

Jonathan

> 
> Thank you,
> Hugo.
> 
> 
> > > +
> > > +additionalProperties: false
> > > +
> > > +required:
> > > +  - compatible
> > > +  - reg
> > > +  - vref-supply
> > > +  - "#address-cells"
> > > +  - "#size-cells"
> > > +  
> > 
> > Best regards,
> > Krzysztof
> > 
> >   
> 
>
Hugo Villeneuve Jan. 15, 2023, 8:11 p.m. UTC | #3 
On Sun, 15 Jan 2023 20:17:24 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 15/01/2023 17:32, Hugo Villeneuve wrote:
> >>>>> +    required:
> >>>>> +      - reg
> >>>>> +
> >>>>> +    additionalProperties: false  
> >>>>
> >>>> You are not allowing anything else from adc.yaml. Is it on purpose?  
> >>>
> >>> I am really not an expert with this Yaml stuff, and reading the documentation makes me probably more confused than before reading it :)
> >>>
> >>> But one thing that is for sure is that these other properties in adc.yaml are not used in my driver:
> >>>
> >>>   bipolar
> >>>   diff-channels
> >>>   settling-time-us
> >>>   oversampling-ratio
> >>>
> >>> So is it Ok then to use "additionalProperties: false"? I think so, but what is your recommandation?
> >>
> >> Makes sense to me.  Whilst there are lots of things a channel can support, most
> >> of them are hardware related and not universal.
> > 
> > Ok, I think I am finally beginning to see the light here :)
> > 
> > So I will then leave "additionalProperties: false".
> > 
> > I will send a V4 soon with all the latest changes.
> > 
> 
> Just to clarify - we talk about hardware, not your Linux driver. What
> your driver uses or doesn't, should not matter here that much.

Hi,
the following properties are definitely not supported by the hardware:
    bipolar
    diff-channels
    oversampling-ratio

does this means that we should add these lines?
    bipolar: false
    diff-channels: false
    oversampling-ratio: false

as for settling-time-us, I am not sure of its usage and if its related to this hardware.

Hugo.


> Best regards,
> Krzysztof
> 
>
Jonathan Cameron Jan. 16, 2023, 3:21 p.m. UTC | #4 
On Sun, 15 Jan 2023 15:11:39 -0500
Hugo Villeneuve <hugo@hugovil.com> wrote:

> On Sun, 15 Jan 2023 20:17:24 +0100
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
> > On 15/01/2023 17:32, Hugo Villeneuve wrote:  
> > >>>>> +    required:
> > >>>>> +      - reg
> > >>>>> +
> > >>>>> +    additionalProperties: false    
> > >>>>
> > >>>> You are not allowing anything else from adc.yaml. Is it on purpose?    
> > >>>
> > >>> I am really not an expert with this Yaml stuff, and reading the documentation makes me probably more confused than before reading it :)
> > >>>
> > >>> But one thing that is for sure is that these other properties in adc.yaml are not used in my driver:
> > >>>
> > >>>   bipolar
> > >>>   diff-channels
> > >>>   settling-time-us
> > >>>   oversampling-ratio
> > >>>
> > >>> So is it Ok then to use "additionalProperties: false"? I think so, but what is your recommandation?  
> > >>
> > >> Makes sense to me.  Whilst there are lots of things a channel can support, most
> > >> of them are hardware related and not universal.  
> > > 
> > > Ok, I think I am finally beginning to see the light here :)
> > > 
> > > So I will then leave "additionalProperties: false".
> > > 
> > > I will send a V4 soon with all the latest changes.
> > >   
> > 
> > Just to clarify - we talk about hardware, not your Linux driver. What
> > your driver uses or doesn't, should not matter here that much.  

Indeed. The hardware does not support bipolar inputs, differential channels
or oversampling ratios as all of those require specific silicon that
is not in this particular device.

Technically you could emulate oversampling but there is no reason to do
that in kernel and no drivers do so + then it would not be a feature of
the hardware anyway so wouldn't belong in DT.

> 
> Hi,
> the following properties are definitely not supported by the hardware:
>     bipolar
>     diff-channels
>     oversampling-ratio
> 
> does this means that we should add these lines?
>     bipolar: false
>     diff-channels: false
>     oversampling-ratio: false
> 
> as for settling-time-us, I am not sure of its usage and if its related to this hardware.

That's used for devices that will delay their ADC sampling if there is a mux that is changing
which pin is being internally connected to a single shared ADC.  It requires hardware
support, which isn't present in this device.

Jonathan


> 
> Hugo.
> 
> 
> > Best regards,
> > Krzysztof
> > 
> >   
> 
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml
new file mode 100644
index 000000000000..24bbf95383b4
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,ads7924.yaml
@@ -0,0 +1,112 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/ti,ads7924.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI ADS7924 4 channels 12 bits I2C analog to digital converter
+
+maintainers:
+  - Hugo Villeneuve <hvilleneuve@dimonoff.com>
+
+description: |
+  Texas Instruments ADS7924 4 channels 12 bits I2C analog to digital converter
+
+  Specifications:
+    https://www.ti.com/lit/gpn/ads7924
+
+properties:
+  compatible:
+    const: ti,ads7924
+
+  reg:
+    maxItems: 1
+
+  vref-supply:
+    description:
+      The regulator supply for the ADC reference voltage (AVDD)
+
+  reset-gpios:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 0
+
+  "#io-channel-cells":
+    const: 1
+
+patternProperties:
+  "^channel@[0-3]+$":
+    $ref: adc.yaml
+
+    description: |
+      Represents the external channels which are connected to the ADC.
+
+    properties:
+      reg:
+        description: |
+          The channel number. It can have up to 4 channels numbered from 0 to 3.
+        items:
+          - minimum: 0
+            maximum: 3
+
+      label:
+        description: |
+          Unique name to identify the channel.
+
+    required:
+      - reg
+
+    additionalProperties: false
+
+additionalProperties: false
+
+required:
+  - compatible
+  - reg
+  - vref-supply
+  - "#address-cells"
+  - "#size-cells"
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/gpio/gpio.h>
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@48 {
+            compatible = "ti,ads7924";
+            reg = <0x48>;
+            vref-supply = <&ads7924_reg>;
+            reset-gpios = <&gpio 5 GPIO_ACTIVE_LOW>;
+            interrupts = <25 IRQ_TYPE_EDGE_FALLING>;
+            interrupt-parent = <&gpio>;
+            #address-cells = <1>;
+            #size-cells = <0>;
+            channel@0 {
+                reg = <0>;
+                label = "CH0";
+            };
+            channel@1 {
+                reg = <1>;
+                label = "CH1";
+            };
+            channel@2 {
+                reg = <2>;
+                label = "CH2";
+            };
+            channel@3 {
+                reg = <3>;
+                label = "CH3";
+            };
+        };
+    };
+...