diff mbox series

[v2,2/3] dt-bindings: iio: adc: Add binding documentation for NXP IMX93 ADC

Message ID 1671024939-29322-2-git-send-email-haibo.chen@nxp.com (mailing list archive)
State Changes Requested
Headers show
Series [v2,1/3] iio: adc: add imx93 adc support | expand

Commit Message

Bough Chen Dec. 14, 2022, 1:35 p.m. UTC 
From: Haibo Chen <haibo.chen@nxp.com>

The IMX93 SoC has a new ADC IP, so add binding documentation
for NXP IMX93 ADC.

Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
---
 .../bindings/iio/adc/nxp,imx93-adc.yaml       | 79 +++++++++++++++++++
 1 file changed, 79 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml

Comments

Rob Herring (Arm) Dec. 14, 2022, 2:56 p.m. UTC | #1 
On Wed, 14 Dec 2022 21:35:38 +0800, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> The IMX93 SoC has a new ADC IP, so add binding documentation
> for NXP IMX93 ADC.
> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 79 +++++++++++++++++++
>  1 file changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.example.dtb: adc@44530000: '#io-channel-cells' is a required property
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.example.dtb: adc@44530000: 'clock-names' does not match any of the regexes: 'pinctrl-[0-9]+'
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/1671024939-29322-2-git-send-email-haibo.chen@nxp.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Rob Herring (Arm) Dec. 14, 2022, 3 p.m. UTC | #2 
On Wed, Dec 14, 2022 at 09:35:38PM +0800, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> The IMX93 SoC has a new ADC IP, so add binding documentation
> for NXP IMX93 ADC.
> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 79 +++++++++++++++++++
>  1 file changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> new file mode 100644
> index 000000000000..229bb79e255c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/nxp,imx93-adc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP iMX93 ADC bindings
> +
> +maintainers:
> +  - Haibo Chen <haibo.chen@nxp.com>
> +
> +description:
> +  The ADC on iMX93 is a 8-channel 12-bit 1MS/s ADC with 4 channels
> +  connected to pins. it support normal and inject mode, include
> +  One-Shot and Scan (continuous) conversions. Programmable DMA
> +  enables for each channel  Also this ADC contain alternate analog
> +  watchdog thresholds, select threshold through input ports. And
> +  also has Self-test logic and Software-initiated calibration.
> +
> +properties:
> +  compatible:
> +    const: nxp,imx93-adc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    description:
> +      line 0 for WDGnL (watchdog threshold) interrupt requests.
> +      line 1 for WDGnH (watchdog threshold) interrupt requests.
> +      line 2 for normal conversion, include EOC (End of Conversion)
> +      interrupt request, ECH (End of Chain) interrupt request,
> +      JEOC (End of Injected Conversion mode) interrupt request
> +      and JECH (End of injected Chain) interrupt request.
> +      line 3 for Self-testing Interrupts.
> +    maxItems: 4

Split the descriptions:

items:
  - description: WDGnL ...
  - description: ...

And then drop maxItems

> +
> +  clocks:
> +    maxItems: 1
> +
> +  vref-supply:
> +    description:
> +      The reference voltage which used to establish channel scaling.
> +
> +  "#io-channel-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - vref-supply
> +  - "#io-channel-cells"
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/clock/imx93-clock.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    soc {
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +        adc@44530000 {
> +            compatible = "nxp,imx93-adc";
> +            reg = <0x44530000 0x10000>;
> +            interrupts = <GIC_SPI 217 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>,
> +                         <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>;
> +            clocks = <&clk IMX93_CLK_ADC1_GATE>;
> +            clock-names = "ipg";
> +            vref-supply = <&reg_vref_1v8>;
> +        };
> +    };
> +...
> -- 
> 2.34.1
> 
>
Krzysztof Kozlowski Dec. 15, 2022, 10:11 a.m. UTC | #3 
On 14/12/2022 14:35, haibo.chen@nxp.com wrote:
> From: Haibo Chen <haibo.chen@nxp.com>
> 
> The IMX93 SoC has a new ADC IP, so add binding documentation
> for NXP IMX93 ADC.
> 
> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> ---
>  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 79 +++++++++++++++++++
>  1 file changed, 79 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> new file mode 100644
> index 000000000000..229bb79e255c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml

This was already sent, so I am surprised to see this in worse or the
same state. Don't force us to repeat review, it's a waste of time.

> @@ -0,0 +1,79 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/nxp,imx93-adc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP iMX93 ADC bindings

Drop bindings. How did it appear here? It wasn't in v1.

> +
> +maintainers:
> +  - Haibo Chen <haibo.chen@nxp.com>
> +
> +description:
> +  The ADC on iMX93 is a 8-channel 12-bit 1MS/s ADC with 4 channels
> +  connected to pins. it support normal and inject mode, include
> +  One-Shot and Scan (continuous) conversions. Programmable DMA
> +  enables for each channel  Also this ADC contain alternate analog
> +  watchdog thresholds, select threshold through input ports. And
> +  also has Self-test logic and Software-initiated calibration.
> +
> +properties:
> +  compatible:
> +    const: nxp,imx93-adc
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    description:
> +      line 0 for WDGnL (watchdog threshold) interrupt requests.
> +      line 1 for WDGnH (watchdog threshold) interrupt requests.
> +      line 2 for normal conversion, include EOC (End of Conversion)
> +      interrupt request, ECH (End of Chain) interrupt request,
> +      JEOC (End of Injected Conversion mode) interrupt request
> +      and JECH (End of injected Chain) interrupt request.
> +      line 3 for Self-testing Interrupts.
> +    maxItems: 4
> +
> +  clocks:
> +    maxItems: 1
> +
> +  vref-supply:
> +    description:
> +      The reference voltage which used to establish channel scaling.
> +
> +  "#io-channel-cells":
> +    const: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clocks
> +  - clock-names
> +  - vref-supply
> +  - "#io-channel-cells"
> +
> +additionalProperties: false
> +

Best regards,
Krzysztof
Bough Chen Dec. 19, 2022, 8:52 a.m. UTC | #4 
> -----Original Message-----
> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Sent: 2022年12月15日 18:12
> To: Bough Chen <haibo.chen@nxp.com>; jic23@kernel.org; lars@metafoo.de;
> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de
> Cc: festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
> linux-iio@vger.kernel.org; devicetree@vger.kernel.org;
> linux-arm-kernel@lists.infradead.org
> Subject: Re: [PATCH v2 2/3] dt-bindings: iio: adc: Add binding documentation for
> NXP IMX93 ADC
> 
> On 14/12/2022 14:35, haibo.chen@nxp.com wrote:
> > From: Haibo Chen <haibo.chen@nxp.com>
> >
> > The IMX93 SoC has a new ADC IP, so add binding documentation for NXP
> > IMX93 ADC.
> >
> > Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
> > ---
> >  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 79
> +++++++++++++++++++
> >  1 file changed, 79 insertions(+)
> >  create mode 100644
> > Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> >
> > diff --git
> > a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> > b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> > new file mode 100644
> > index 000000000000..229bb79e255c
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
> 
> This was already sent, so I am surprised to see this in worse or the same state.
> Don't force us to repeat review, it's a waste of time.

Sorry, I'm focus on the driver side, will pay much attention on the yaml binding.

By the way, for your first review comments:

   > +
   > +  clocks:
   > +    maxItems: 1
   > +
   > +  clock-names:
   > +    const: ipg

   No need for clock-names in such case.


How should I handle this case here? I search in this directory(iio/adc), and find many other yaml also write this way.
Do you mean change like this:
     clock-names:  true

If you will, can you help tell where is the yaml guide, I'm not familiar with yaml rule.

Best Regards
Haibo Chen

> 
> > @@ -0,0 +1,79 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> > +---
> > +$id:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fschemas%2Fiio%2Fadc%2Fnxp%2Cimx93-adc.yaml%23&amp;da
> ta=0
> >
> +5%7C01%7Chaibo.chen%40nxp.com%7C0f5dbde6e91b4c8f920c08dade84c2b4
> %7C68
> >
> +6ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C638066959030689942%7C
> Unknown
> >
> +%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haW
> wiLC
> >
> +JXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=iG7kL12EZ17jqPyLPx8X79
> m8Muzaul
> > +CZDuqAl8Ayhdw%3D&amp;reserved=0
> > +$schema:
> > +https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevi
> >
> +cetree.org%2Fmeta-schemas%2Fcore.yaml%23&amp;data=05%7C01%7Chaib
> o.che
> >
> +n%40nxp.com%7C0f5dbde6e91b4c8f920c08dade84c2b4%7C686ea1d3bc2b4c
> 6fa92c
> >
> +d99c5c301635%7C0%7C0%7C638066959030689942%7CUnknown%7CTWFpb
> GZsb3d8eyJ
> >
> +WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7
> C300
> >
> +0%7C%7C%7C&amp;sdata=fXkafmq9%2FLxo2I%2FyIFRcTgwVMLP7yBXYDPrn0
> DJzuV4%
> > +3D&amp;reserved=0
> > +
> > +title: NXP iMX93 ADC bindings
> 
> Drop bindings. How did it appear here? It wasn't in v1.
> 
> > +
> > +maintainers:
> > +  - Haibo Chen <haibo.chen@nxp.com>
> > +
> > +description:
> > +  The ADC on iMX93 is a 8-channel 12-bit 1MS/s ADC with 4 channels
> > +  connected to pins. it support normal and inject mode, include
> > +  One-Shot and Scan (continuous) conversions. Programmable DMA
> > +  enables for each channel  Also this ADC contain alternate analog
> > +  watchdog thresholds, select threshold through input ports. And
> > +  also has Self-test logic and Software-initiated calibration.
> > +
> > +properties:
> > +  compatible:
> > +    const: nxp,imx93-adc
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    description:
> > +      line 0 for WDGnL (watchdog threshold) interrupt requests.
> > +      line 1 for WDGnH (watchdog threshold) interrupt requests.
> > +      line 2 for normal conversion, include EOC (End of Conversion)
> > +      interrupt request, ECH (End of Chain) interrupt request,
> > +      JEOC (End of Injected Conversion mode) interrupt request
> > +      and JECH (End of injected Chain) interrupt request.
> > +      line 3 for Self-testing Interrupts.
> > +    maxItems: 4
> > +
> > +  clocks:
> > +    maxItems: 1
> > +
> > +  vref-supply:
> > +    description:
> > +      The reference voltage which used to establish channel scaling.
> > +
> > +  "#io-channel-cells":
> > +    const: 1
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - interrupts
> > +  - clocks
> > +  - clock-names
> > +  - vref-supply
> > +  - "#io-channel-cells"
> > +
> > +additionalProperties: false
> > +
> 
> Best regards,
> Krzysztof
Krzysztof Kozlowski Dec. 19, 2022, 8:59 a.m. UTC | #5 
On 19/12/2022 09:52, Bough Chen wrote:
>> -----Original Message-----
>> From: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
>> Sent: 2022年12月15日 18:12
>> To: Bough Chen <haibo.chen@nxp.com>; jic23@kernel.org; lars@metafoo.de;
>> robh+dt@kernel.org; krzysztof.kozlowski+dt@linaro.org;
>> shawnguo@kernel.org; s.hauer@pengutronix.de; kernel@pengutronix.de
>> Cc: festevam@gmail.com; dl-linux-imx <linux-imx@nxp.com>;
>> linux-iio@vger.kernel.org; devicetree@vger.kernel.org;
>> linux-arm-kernel@lists.infradead.org
>> Subject: Re: [PATCH v2 2/3] dt-bindings: iio: adc: Add binding documentation for
>> NXP IMX93 ADC
>>
>> On 14/12/2022 14:35, haibo.chen@nxp.com wrote:
>>> From: Haibo Chen <haibo.chen@nxp.com>
>>>
>>> The IMX93 SoC has a new ADC IP, so add binding documentation for NXP
>>> IMX93 ADC.
>>>
>>> Signed-off-by: Haibo Chen <haibo.chen@nxp.com>
>>> ---
>>>  .../bindings/iio/adc/nxp,imx93-adc.yaml       | 79
>> +++++++++++++++++++
>>>  1 file changed, 79 insertions(+)
>>>  create mode 100644
>>> Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>>
>>> diff --git
>>> a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>> b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>> new file mode 100644
>>> index 000000000000..229bb79e255c
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
>>
>> This was already sent, so I am surprised to see this in worse or the same state.
>> Don't force us to repeat review, it's a waste of time.
> 
> Sorry, I'm focus on the driver side, will pay much attention on the yaml binding.
> 
> By the way, for your first review comments:
> 
>    > +
>    > +  clocks:
>    > +    maxItems: 1
>    > +
>    > +  clock-names:
>    > +    const: ipg
> 
>    No need for clock-names in such case.

I think this was for the other patch and recommendation was to drop
entire clock-names. Here you do not have it, so no need to change this.

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
new file mode 100644
index 000000000000..229bb79e255c
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/adc/nxp,imx93-adc.yaml
@@ -0,0 +1,79 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/adc/nxp,imx93-adc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP iMX93 ADC bindings
+
+maintainers:
+  - Haibo Chen <haibo.chen@nxp.com>
+
+description:
+  The ADC on iMX93 is a 8-channel 12-bit 1MS/s ADC with 4 channels
+  connected to pins. it support normal and inject mode, include
+  One-Shot and Scan (continuous) conversions. Programmable DMA
+  enables for each channel  Also this ADC contain alternate analog
+  watchdog thresholds, select threshold through input ports. And
+  also has Self-test logic and Software-initiated calibration.
+
+properties:
+  compatible:
+    const: nxp,imx93-adc
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    description:
+      line 0 for WDGnL (watchdog threshold) interrupt requests.
+      line 1 for WDGnH (watchdog threshold) interrupt requests.
+      line 2 for normal conversion, include EOC (End of Conversion)
+      interrupt request, ECH (End of Chain) interrupt request,
+      JEOC (End of Injected Conversion mode) interrupt request
+      and JECH (End of injected Chain) interrupt request.
+      line 3 for Self-testing Interrupts.
+    maxItems: 4
+
+  clocks:
+    maxItems: 1
+
+  vref-supply:
+    description:
+      The reference voltage which used to establish channel scaling.
+
+  "#io-channel-cells":
+    const: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - vref-supply
+  - "#io-channel-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/clock/imx93-clock.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    soc {
+        #address-cells = <1>;
+        #size-cells = <1>;
+        adc@44530000 {
+            compatible = "nxp,imx93-adc";
+            reg = <0x44530000 0x10000>;
+            interrupts = <GIC_SPI 217 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 218 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 219 IRQ_TYPE_LEVEL_HIGH>,
+                         <GIC_SPI 268 IRQ_TYPE_LEVEL_HIGH>;
+            clocks = <&clk IMX93_CLK_ADC1_GATE>;
+            clock-names = "ipg";
+            vref-supply = <&reg_vref_1v8>;
+        };
+    };
+...