diff mbox series

[12/12] dt-bindings: iio: accel: Add ADIS16203 Inclinometer

Message ID 20230123211758.563383-13-jic23@kernel.org (mailing list archive)
State Changes Requested
Headers show
Series staging: iio: accel: adis16203 Inclinometer cleanup and graduation | expand

Commit Message

Jonathan Cameron Jan. 23, 2023, 9:17 p.m. UTC
From: Jonathan Cameron <Jonathan.Cameron@huawei.com>

There has been a driver in staging for quite a while.
Given we are now moving it to the main tree, time to make sure it
has binding documentation.

Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
---
 .../bindings/iio/accel/adi,adis16203.yaml     | 59 +++++++++++++++++++
 1 file changed, 59 insertions(+)

Comments

Rob Herring (Arm) Jan. 24, 2023, 1:22 a.m. UTC | #1
On Mon, 23 Jan 2023 21:17:58 +0000, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> There has been a driver in staging for quite a while.
> Given we are now moving it to the main tree, time to make sure it
> has binding documentation.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  .../bindings/iio/accel/adi,adis16203.yaml     | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 

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/accel/adi,adis16203.example.dtb: accelerometer@0: Unevaluated properties are not allowed ('interrupt-names' was unexpected)
	From schema: /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/iio/accel/adi,adis16201.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230123211758.563383-13-jic23@kernel.org

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.
Krzysztof Kozlowski Jan. 24, 2023, 8:39 a.m. UTC | #2
On 23/01/2023 22:17, Jonathan Cameron wrote:
> From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> There has been a driver in staging for quite a while.
> Given we are now moving it to the main tree, time to make sure it
> has binding documentation.
> 
> Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> ---
>  .../bindings/iio/accel/adi,adis16203.yaml     | 59 +++++++++++++++++++
>  1 file changed, 59 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml
> new file mode 100644
> index 000000000000..05c095247e10
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml
> @@ -0,0 +1,59 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/accel/adi,adis16203.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: ADIS16203 Programmable 360 degree inclinometer
> +
> +maintainers:
> +  - Jonathan Cameron <Jonathan.Cameron@huawei.com>
> +
> +properties:
> +  compatible:
> +    const: adi,adis16203
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 2
> +
> +  interrupt-names:

maxItems (and probably minItems)

> +    description:
> +      Device has two configurable outputs, both of which may be used
> +      as interrupt sources.
> +    enum:

This won't work. It's an list. You need:
items:
  enum:
     .....

> +      - dio0
> +      - dio1
> +
> +  reset-gpios: true

maxItems: 1


> +
> +  vdd-supply: true
> +
> +required:
> +  - compatible
> +  - reg
> +
> +allOf:
> +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    spi {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        accelerometer@0 {
> +            compatible = "adi,adis16201";
> +            reg = <0>;
> +            spi-max-frequency = <2500000>;
> +            interrupt-parent = <&gpio0>;
> +            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +            interrupt-names = "dio0";

You require here two items, according to interrupts. Don't you miss
minItems?

Also... if you tested the binding it would complain here.

> +        };
> +    };
> +...
> +

Best regards,
Krzysztof
Jonathan Cameron Jan. 24, 2023, 9:43 a.m. UTC | #3
On Tue, 24 Jan 2023 09:39:10 +0100
Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:

> On 23/01/2023 22:17, Jonathan Cameron wrote:
> > From: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > 
> > There has been a driver in staging for quite a while.
> > Given we are now moving it to the main tree, time to make sure it
> > has binding documentation.
> > 
> > Signed-off-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > ---
> >  .../bindings/iio/accel/adi,adis16203.yaml     | 59 +++++++++++++++++++
> >  1 file changed, 59 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml
> > new file mode 100644
> > index 000000000000..05c095247e10
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml
> > @@ -0,0 +1,59 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/accel/adi,adis16203.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ADIS16203 Programmable 360 degree inclinometer
> > +
> > +maintainers:
> > +  - Jonathan Cameron <Jonathan.Cameron@huawei.com>
> > +
> > +properties:
> > +  compatible:
> > +    const: adi,adis16203
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  interrupts:
> > +    maxItems: 2
> > +
> > +  interrupt-names:  
> 
> maxItems (and probably minItems
> 
> > +    description:
> > +      Device has two configurable outputs, both of which may be used
> > +      as interrupt sources.
> > +    enum:  
> 
> This won't work. It's an list. You need:
> items:
>   enum:
>      .....

Gah. I'm still rubbish at writing these things. I guess I just
don't do them often enough.

> 
> > +      - dio0
> > +      - dio1
> > +
> > +  reset-gpios: true  
> 
> maxItems: 1
> 
> 
> > +
> > +  vdd-supply: true
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +
> > +allOf:
> > +  - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > +
> > +unevaluatedProperties: false
> > +
> > +examples:
> > +  - |
> > +    #include <dt-bindings/interrupt-controller/irq.h>
> > +    spi {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        accelerometer@0 {
> > +            compatible = "adi,adis16201";
> > +            reg = <0>;
> > +            spi-max-frequency = <2500000>;
> > +            interrupt-parent = <&gpio0>;
> > +            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> > +            interrupt-names = "dio0";  
> 
> You require here two items, according to interrupts. Don't you miss
> minItems?

Indeed.

> 
> Also... if you tested the binding it would complain here.

I thought I had tested it (and fixed a few issues that showed up).
I guess something went wrong with with the final test.  Will investigate.

Thanks and sorry for the waste of time with the silly errors!

Jonathan


> 
> > +        };
> > +    };
> > +...
> > +  
> 
> Best regards,
> Krzysztof
>
Linus Walleij Feb. 1, 2023, 9:29 a.m. UTC | #4
On Tue, Jan 24, 2023 at 10:43 AM Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:

> > This won't work. It's an list. You need:
> > items:
> >   enum:
> >      .....
>
> Gah. I'm still rubbish at writing these things. I guess I just
> don't do them often enough.

No the dtschema YAML is just really hard. Don't beat yourself up about it.

Yours,
Linus Walleij
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml b/Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml
new file mode 100644
index 000000000000..05c095247e10
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/accel/adi,adis16203.yaml
@@ -0,0 +1,59 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/accel/adi,adis16203.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ADIS16203 Programmable 360 degree inclinometer
+
+maintainers:
+  - Jonathan Cameron <Jonathan.Cameron@huawei.com>
+
+properties:
+  compatible:
+    const: adi,adis16203
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 2
+
+  interrupt-names:
+    description:
+      Device has two configurable outputs, both of which may be used
+      as interrupt sources.
+    enum:
+      - dio0
+      - dio1
+
+  reset-gpios: true
+
+  vdd-supply: true
+
+required:
+  - compatible
+  - reg
+
+allOf:
+  - $ref: /schemas/spi/spi-peripheral-props.yaml#
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    spi {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        accelerometer@0 {
+            compatible = "adi,adis16201";
+            reg = <0>;
+            spi-max-frequency = <2500000>;
+            interrupt-parent = <&gpio0>;
+            interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+            interrupt-names = "dio0";
+        };
+    };
+...
+