diff mbox series

dt-bindings: iio: adc: Add TI TWL603X GPADC

Message ID 20230816202614.324457-1-andreas@kemnade.info
State New
Headers show
Series dt-bindings: iio: adc: Add TI TWL603X GPADC | expand

Commit Message

Andreas Kemnade Aug. 16, 2023, 8:26 p.m. UTC 
Document TI TWL603X GPADC devicetree bindings.
A driver is already there, the compatibles are used, but not documented.
Use two separate files to reference only the allowed compatible in
a future YAML version of
Documentation/devicetree/bindings/mfd/twl-family.txt

Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
---
 .../bindings/iio/adc/ti,twl6030-gpadc.yaml    | 42 +++++++++++++++++++
 .../bindings/iio/adc/ti,twl6032-gpadc.yaml    | 42 +++++++++++++++++++
 2 files changed, 84 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,twl6030-gpadc.yaml
 create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,twl6032-gpadc.yaml

Comments

Krzysztof Kozlowski Aug. 19, 2023, 6:35 p.m. UTC | #1 
On 16/08/2023 22:26, Andreas Kemnade wrote:
> Document TI TWL603X GPADC devicetree bindings.
> A driver is already there, the compatibles are used, but not documented.
> Use two separate files to reference only the allowed compatible in
> a future YAML version of
> Documentation/devicetree/bindings/mfd/twl-family.txt
> 
> Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> ---

Thank you for your patch. There is something to discuss/improve.


>  .../bindings/iio/adc/ti,twl6030-gpadc.yaml    | 42 +++++++++++++++++++
>  .../bindings/iio/adc/ti,twl6032-gpadc.yaml    | 42 +++++++++++++++++++
>  2 files changed, 84 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,twl6030-gpadc.yaml
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,twl6032-gpadc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,twl6030-gpadc.yaml b/Documentation/devicetree/bindings/iio/adc/ti,twl6030-gpadc.yaml
> new file mode 100644
> index 000000000000..08bc0468f616
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,twl6030-gpadc.yaml
> @@ -0,0 +1,42 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,twl6030-gpadc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: GPADC subsystem in the TWL6030 power module
> +
> +maintainers:
> +  - Jonathan Cameron <jic23@kernel.org>

This should be rather someone knowing or having or caring about this
particular hardware, not subsystem maintainer.

> +
> +description:
> +  The GPADC subsystem in the TWL6030 consists of a 10-bit ADC
> +  combined with a 15-input analog multiplexer.
> +
> +properties:
> +  compatible:
> +    const: ti,twl6030-gpadc

Devices look fairly similar. Same properties. Why aren't they in one
binding (enum here instead)?

Best regards,
Krzysztof
Andreas Kemnade Aug. 19, 2023, 8:19 p.m. UTC | #2 
Hi,

On Sat, 19 Aug 2023 20:35:27 +0200
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 16/08/2023 22:26, Andreas Kemnade wrote:
> > Document TI TWL603X GPADC devicetree bindings.
> > A driver is already there, the compatibles are used, but not documented.
> > Use two separate files to reference only the allowed compatible in
> > a future YAML version of
> > Documentation/devicetree/bindings/mfd/twl-family.txt
> > 
> > Signed-off-by: Andreas Kemnade <andreas@kemnade.info>
> > ---  
> 
> Thank you for your patch. There is something to discuss/improve.
> 
> 
> >  .../bindings/iio/adc/ti,twl6030-gpadc.yaml    | 42 +++++++++++++++++++
> >  .../bindings/iio/adc/ti,twl6032-gpadc.yaml    | 42 +++++++++++++++++++
> >  2 files changed, 84 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,twl6030-gpadc.yaml
> >  create mode 100644 Documentation/devicetree/bindings/iio/adc/ti,twl6032-gpadc.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/adc/ti,twl6030-gpadc.yaml b/Documentation/devicetree/bindings/iio/adc/ti,twl6030-gpadc.yaml
> > new file mode 100644
> > index 000000000000..08bc0468f616
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/ti,twl6030-gpadc.yaml
> > @@ -0,0 +1,42 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/adc/ti,twl6030-gpadc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: GPADC subsystem in the TWL6030 power module
> > +
> > +maintainers:
> > +  - Jonathan Cameron <jic23@kernel.org>  
> 
> This should be rather someone knowing or having or caring about this
> particular hardware, not subsystem maintainer.
> 
Hmm, I have the twl6032, but not the twl6030. So probably
Tony (OMAP-Maintainer) or me?

> > +
> > +description:
> > +  The GPADC subsystem in the TWL6030 consists of a 10-bit ADC
> > +  combined with a 15-input analog multiplexer.
> > +
> > +properties:
> > +  compatible:
> > +    const: ti,twl6030-gpadc  
> 
> Devices look fairly similar. Same properties. Why aren't they in one
> binding (enum here instead)?
>
I hope it can be done. See commit message. Maybe my reasoning is wrong.

So what I am thinking about:

&i2c {
	twl: pmic@48 {
		compatible = "ti,twl6032;
		adc {
			compatible = "ti,twl6032-gpadc";
		}
	}
}

So the idea was to later enforce that below a "ti,twl6032" no "ti,twl6030-gpadc"
is allowed in a future yaml version of mfd/twl-family.txt by
using a if: ... compatible = "twl,6032" .. $ref ti,twl6032-gpadc.yaml

If there are other possibilities or that can be just ignored for now,
I fully agree to your proposal. 

Regards,
Andreas
Krzysztof Kozlowski Aug. 20, 2023, 6:29 a.m. UTC | #3 
On 19/08/2023 22:19, Andreas Kemnade wrote:
>>> +---
>>> +$id: http://devicetree.org/schemas/iio/adc/ti,twl6030-gpadc.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: GPADC subsystem in the TWL6030 power module
>>> +
>>> +maintainers:
>>> +  - Jonathan Cameron <jic23@kernel.org>  
>>
>> This should be rather someone knowing or having or caring about this
>> particular hardware, not subsystem maintainer.
>>
> Hmm, I have the twl6032, but not the twl6030. So probably
> Tony (OMAP-Maintainer) or me?

Yes. If you have a device, it's even better, but "caring about" or
having datasheet is enough.

> 
>>> +
>>> +description:
>>> +  The GPADC subsystem in the TWL6030 consists of a 10-bit ADC
>>> +  combined with a 15-input analog multiplexer.
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: ti,twl6030-gpadc  
>>
>> Devices look fairly similar. Same properties. Why aren't they in one
>> binding (enum here instead)?
>>
> I hope it can be done. See commit message. Maybe my reasoning is wrong.

The parent device binding can expect the compatible for the child and it
will have the same effect in total as $ref to this binding. The only
difference would be that running dtbs_check on parent binding would not
spot all the issues in the child node. One need to run dtbs_check with
both bindings.

For an example:
Documentation/devicetree/bindings/display/msm/qcom,sm8450-mdss.yaml


Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/adc/ti,twl6030-gpadc.yaml b/Documentation/devicetree/bindings/iio/adc/ti,twl6030-gpadc.yaml
new file mode 100644
index 000000000000..08bc0468f616
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,twl6030-gpadc.yaml
@@ -0,0 +1,42 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/ti,twl6030-gpadc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GPADC subsystem in the TWL6030 power module
+
+maintainers:
+  - Jonathan Cameron <jic23@kernel.org>
+
+description:
+  The GPADC subsystem in the TWL6030 consists of a 10-bit ADC
+  combined with a 15-input analog multiplexer.
+
+properties:
+  compatible:
+    const: ti,twl6030-gpadc
+
+  interrupts:
+    maxItems: 1
+
+  "#io-channel-cells":
+    const: 1
+
+required:
+  - compatible
+  - interrupts
+  - "#io-channel-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    twl {
+        gpadc {
+            compatible = "ti,twl6030-gpadc";
+            interrupts = <3>;
+            #io-channel-cells = <1>;
+        };
+    };
+...
diff --git a/Documentation/devicetree/bindings/iio/adc/ti,twl6032-gpadc.yaml b/Documentation/devicetree/bindings/iio/adc/ti,twl6032-gpadc.yaml
new file mode 100644
index 000000000000..70acec533277
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/ti,twl6032-gpadc.yaml
@@ -0,0 +1,42 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/ti,twl6032-gpadc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: GPADC subsystem in the TWL6032 power module
+
+maintainers:
+  - Jonathan Cameron <jic23@kernel.org>
+
+description:
+  The GPADC subsystem in the TWL6032 consists of a 10-bit ADC
+  combined with a 19-input analog multiplexer.
+
+properties:
+  compatible:
+    const: ti,twl6032-gpadc
+
+  interrupts:
+    maxItems: 1
+
+  "#io-channel-cells":
+    const: 1
+
+required:
+  - compatible
+  - interrupts
+  - "#io-channel-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    twl {
+        gpadc {
+            compatible = "ti,twl6032-gpadc";
+            interrupts = <3>;
+            #io-channel-cells = <1>;
+        };
+    };
+...