diff mbox series

dt-bindings: iio: adc: ti,adc081c: Document the binding

Message ID 20221125220903.8632-1-samuel@sholland.org
State Not Applicable, archived
Headers show
Series dt-bindings: iio: adc: ti,adc081c: Document the binding | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Samuel Holland Nov. 25, 2022, 10:09 p.m. UTC 
Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but
the compatible strings were undocumented. Add a binding for them. The
hardware has an alert interrupt output, but existing ti,adc081c users
do not provide the 'interrupts' property, so leave it as optional.

Signed-off-by: Samuel Holland <samuel@sholland.org>
---

 .../bindings/iio/adc/ti,adc081c.yaml          | 55 +++++++++++++++++++
 1 file changed, 55 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml

Comments

Krzysztof Kozlowski Nov. 27, 2022, 12:51 p.m. UTC | #1 
On 25/11/2022 23:09, Samuel Holland wrote:
> Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but
> the compatible strings were undocumented. Add a binding for them. The
> hardware has an alert interrupt output, but existing ti,adc081c users
> do not provide the 'interrupts' property, so leave it as optional.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 
>  .../bindings/iio/adc/ti,adc081c.yaml          | 55 +++++++++++++++++++
>  1 file changed, 55 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
> new file mode 100644
> index 000000000000..caaad777580c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
> @@ -0,0 +1,55 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,adc081c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: TI Single-channel I2C ADCs
> +
> +maintainers:
> +  - Jonathan Cameron <jic23@kernel.org>
> +  - Lars-Peter Clausen <lars@metafoo.de>
> +
> +description: |
> +  Single-channel ADC supporting 8, 10, or 12-bit samples and high/low alerts.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - ti,adc081c
> +      - ti,adc101c
> +      - ti,adc121c
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +  vref-supply:
> +    description:
> +      Regulator for the combined power supply and voltage reference
> +
> +  "#io-channel-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg

Why not requiring io-channel-cells? If it is an IIO ADC provider, you
need the cells, right?

Best regards,
Krzysztof
Jonathan Cameron Nov. 27, 2022, 5:42 p.m. UTC | #2 
On Sun, 27 Nov 2022 13:51:19 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 25/11/2022 23:09, Samuel Holland wrote:
> > Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but
> > the compatible strings were undocumented. Add a binding for them. The
> > hardware has an alert interrupt output, but existing ti,adc081c users
> > do not provide the 'interrupts' property, so leave it as optional.
> > 
> > Signed-off-by: Samuel Holland <samuel@sholland.org>
> > ---
> > 
> >  .../bindings/iio/adc/ti,adc081c.yaml          | 55 +++++++++++++++++++
> >  1 file changed, 55 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
> > new file mode 100644
> > index 000000000000..caaad777580c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
> > @@ -0,0 +1,55 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/adc/ti,adc081c.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: TI Single-channel I2C ADCs
> > +
> > +maintainers:
> > +  - Jonathan Cameron <jic23@kernel.org>
> > +  - Lars-Peter Clausen <lars@metafoo.de>
> > +
> > +description: |
> > +  Single-channel ADC supporting 8, 10, or 12-bit samples and high/low alerts.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - ti,adc081c
> > +      - ti,adc101c
> > +      - ti,adc121c
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 1
> > +
> > +  vref-supply:
> > +    description:
> > +      Regulator for the combined power supply and voltage reference
> > +
> > +  "#io-channel-cells":
> > +    const: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg  
> 
> Why not requiring io-channel-cells? If it is an IIO ADC provider, you
> need the cells, right?

Only if anyone is using it as a provider.  If it's purely being used via
IIO then there are no consumers registered.

So historically I've left it up to those defining the binding to decide if
they think #io-channel-cells should be required or optional.

It gets a bit non obvious with some of the more complex special ADCs on whether
they will ever be consumed.  This one is generic, so quite likely it will be.

Jonathan

> 
> Best regards,
> Krzysztof
>
Samuel Holland Nov. 27, 2022, 6:01 p.m. UTC | #3 
On 11/27/22 11:42, Jonathan Cameron wrote:
> On Sun, 27 Nov 2022 13:51:19 +0100
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
>> On 25/11/2022 23:09, Samuel Holland wrote:
>>> Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but
>>> the compatible strings were undocumented. Add a binding for them. The
>>> hardware has an alert interrupt output, but existing ti,adc081c users
>>> do not provide the 'interrupts' property, so leave it as optional.
>>>
>>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>>> ---
>>>
>>>  .../bindings/iio/adc/ti,adc081c.yaml          | 55 +++++++++++++++++++
>>>  1 file changed, 55 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
>>> new file mode 100644
>>> index 000000000000..caaad777580c
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
>>> @@ -0,0 +1,55 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/iio/adc/ti,adc081c.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: TI Single-channel I2C ADCs
>>> +
>>> +maintainers:
>>> +  - Jonathan Cameron <jic23@kernel.org>
>>> +  - Lars-Peter Clausen <lars@metafoo.de>
>>> +
>>> +description: |
>>> +  Single-channel ADC supporting 8, 10, or 12-bit samples and high/low alerts.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - ti,adc081c
>>> +      - ti,adc101c
>>> +      - ti,adc121c
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  vref-supply:
>>> +    description:
>>> +      Regulator for the combined power supply and voltage reference
>>> +
>>> +  "#io-channel-cells":
>>> +    const: 1
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg  
>>
>> Why not requiring io-channel-cells? If it is an IIO ADC provider, you
>> need the cells, right?
> 
> Only if anyone is using it as a provider.  If it's purely being used via
> IIO then there are no consumers registered.
> 
> So historically I've left it up to those defining the binding to decide if
> they think #io-channel-cells should be required or optional.
> 
> It gets a bit non obvious with some of the more complex special ADCs on whether
> they will ever be consumed.  This one is generic, so quite likely it will be.

I kept #io-channel-cells optional because there are already a handful of
boards using ti,adc081c without it.

On the board I am adding (Clockwork DevTerm), the ADC is used a
temperature sensor for a thermal printer. So whether or not the ADC is
used as an OF provider depends on how the printer driver gets implemented.

Regards,
Samuel
Krzysztof Kozlowski Nov. 27, 2022, 9:08 p.m. UTC | #4 
On 27/11/2022 19:01, Samuel Holland wrote:
> On 11/27/22 11:42, Jonathan Cameron wrote:
>> On Sun, 27 Nov 2022 13:51:19 +0100
>> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>>
>>> On 25/11/2022 23:09, Samuel Holland wrote:
>>>> Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but
>>>> the compatible strings were undocumented. Add a binding for them. The
>>>> hardware has an alert interrupt output, but existing ti,adc081c users
>>>> do not provide the 'interrupts' property, so leave it as optional.
>>>>
>>>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>>>> ---
>>>>
>>>>  .../bindings/iio/adc/ti,adc081c.yaml          | 55 +++++++++++++++++++
>>>>  1 file changed, 55 insertions(+)
>>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
>>>> new file mode 100644
>>>> index 000000000000..caaad777580c
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
>>>> @@ -0,0 +1,55 @@
>>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/iio/adc/ti,adc081c.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: TI Single-channel I2C ADCs
>>>> +
>>>> +maintainers:
>>>> +  - Jonathan Cameron <jic23@kernel.org>
>>>> +  - Lars-Peter Clausen <lars@metafoo.de>
>>>> +
>>>> +description: |
>>>> +  Single-channel ADC supporting 8, 10, or 12-bit samples and high/low alerts.
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    enum:
>>>> +      - ti,adc081c
>>>> +      - ti,adc101c
>>>> +      - ti,adc121c
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +  interrupts:
>>>> +    maxItems: 1
>>>> +
>>>> +  vref-supply:
>>>> +    description:
>>>> +      Regulator for the combined power supply and voltage reference
>>>> +
>>>> +  "#io-channel-cells":
>>>> +    const: 1
>>>> +
>>>> +required:
>>>> +  - compatible
>>>> +  - reg  
>>>
>>> Why not requiring io-channel-cells? If it is an IIO ADC provider, you
>>> need the cells, right?
>>
>> Only if anyone is using it as a provider.  If it's purely being used via
>> IIO then there are no consumers registered.
>>
>> So historically I've left it up to those defining the binding to decide if
>> they think #io-channel-cells should be required or optional.
>>
>> It gets a bit non obvious with some of the more complex special ADCs on whether
>> they will ever be consumed.  This one is generic, so quite likely it will be.
> 
> I kept #io-channel-cells optional because there are already a handful of
> boards using ti,adc081c without it.
> 
> On the board I am adding (Clockwork DevTerm), the ADC is used a
> temperature sensor for a thermal printer. So whether or not the ADC is
> used as an OF provider depends on how the printer driver gets implemented.

Thanks, it's fine.

Best regards,
Krzysztof
Krzysztof Kozlowski Nov. 27, 2022, 9:09 p.m. UTC | #5 
On 27/11/2022 18:42, Jonathan Cameron wrote:
> On Sun, 27 Nov 2022 13:51:19 +0100
> Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
> 
>> On 25/11/2022 23:09, Samuel Holland wrote:
>>> Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but
>>> the compatible strings were undocumented. Add a binding for them. The
>>> hardware has an alert interrupt output, but existing ti,adc081c users
>>> do not provide the 'interrupts' property, so leave it as optional.
>>>
>>> Signed-off-by: Samuel Holland <samuel@sholland.org>
>>> ---
>>>
>>>  .../bindings/iio/adc/ti,adc081c.yaml          | 55 +++++++++++++++++++
>>>  1 file changed, 55 insertions(+)
>>>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
>>> new file mode 100644
>>> index 000000000000..caaad777580c
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
>>> @@ -0,0 +1,55 @@
>>> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/iio/adc/ti,adc081c.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: TI Single-channel I2C ADCs
>>> +
>>> +maintainers:
>>> +  - Jonathan Cameron <jic23@kernel.org>
>>> +  - Lars-Peter Clausen <lars@metafoo.de>
>>> +
>>> +description: |
>>> +  Single-channel ADC supporting 8, 10, or 12-bit samples and high/low alerts.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    enum:
>>> +      - ti,adc081c
>>> +      - ti,adc101c
>>> +      - ti,adc121c
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +  interrupts:
>>> +    maxItems: 1
>>> +
>>> +  vref-supply:
>>> +    description:
>>> +      Regulator for the combined power supply and voltage reference
>>> +
>>> +  "#io-channel-cells":
>>> +    const: 1
>>> +
>>> +required:
>>> +  - compatible
>>> +  - reg  
>>
>> Why not requiring io-channel-cells? If it is an IIO ADC provider, you
>> need the cells, right?
> 
> Only if anyone is using it as a provider.  If it's purely being used via
> IIO then there are no consumers registered.
> 
> So historically I've left it up to those defining the binding to decide if
> they think #io-channel-cells should be required or optional.
> 
> It gets a bit non obvious with some of the more complex special ADCs on whether
> they will ever be consumed.  This one is generic, so quite likely it will be.

I remember I asked some time ago and got the same answer... need to
write it down into my notes :)

Best regards,
Krzysztof
Krzysztof Kozlowski Nov. 27, 2022, 9:09 p.m. UTC | #6 
On 25/11/2022 23:09, Samuel Holland wrote:
> Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but
> the compatible strings were undocumented. Add a binding for them. The
> hardware has an alert interrupt output, but existing ti,adc081c users
> do not provide the 'interrupts' property, so leave it as optional.
> 
> Signed-off-by: Samuel Holland <samuel@sholland.org>
> ---
> 


Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof
Jonathan Cameron Dec. 3, 2022, 5:41 p.m. UTC | #7 
On Sun, 27 Nov 2022 22:09:52 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 25/11/2022 23:09, Samuel Holland wrote:
> > Linux has a driver for these ADCs at drivers/iio/adc/ti-adc081c.c, but
> > the compatible strings were undocumented. Add a binding for them. The
> > hardware has an alert interrupt output, but existing ti,adc081c users
> > do not provide the 'interrupts' property, so leave it as optional.
> > 
> > Signed-off-by: Samuel Holland <samuel@sholland.org>
> > ---
> >   
> 
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Applied, but now we are so close to merge window, I'm queuing things
for 6.3.  Obviously wouldn't really matter for this, but it is too
late for other things in my tree to get enough time in linux-next
etc.

Jonathan

> 
> Best regards,
> Krzysztof
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
new file mode 100644
index 000000000000..caaad777580c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,adc081c.yaml
@@ -0,0 +1,55 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/ti,adc081c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: TI Single-channel I2C ADCs
+
+maintainers:
+  - Jonathan Cameron <jic23@kernel.org>
+  - Lars-Peter Clausen <lars@metafoo.de>
+
+description: |
+  Single-channel ADC supporting 8, 10, or 12-bit samples and high/low alerts.
+
+properties:
+  compatible:
+    enum:
+      - ti,adc081c
+      - ti,adc101c
+      - ti,adc121c
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+  vref-supply:
+    description:
+      Regulator for the combined power supply and voltage reference
+
+  "#io-channel-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - vref-supply
+
+additionalProperties: false
+
+examples:
+  - |
+    i2c {
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        adc@52 {
+            compatible = "ti,adc081c";
+            reg = <0x52>;
+            vref-supply = <&reg_2p5v>;
+        };
+    };
+...