diff mbox series

[1/2] dt-bindings: add adpd188 schema

Message ID 20211008112747.79969-2-andrei.drimbarean@analog.com
State Changes Requested, archived
Headers show
Series ADPD188 linux driver | expand

Checks

Context Check Description
robh/checkpatch success
robh/dt-meta-schema fail build log

Commit Message

andrei.drimbarean@analog.com Oct. 8, 2021, 11:27 a.m. UTC
From: Andrei Drimbarean <andrei.drimbarean@analog.com>

Signed-off-by: Andrei Drimbarean <andrei.drimbarean@analog.com>
---
 .../bindings/iio/light/adi,adpd188.yaml       | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/light/adi,adpd188.yaml

Comments

Jonathan Cameron Oct. 8, 2021, 4:34 p.m. UTC | #1
On Fri, 8 Oct 2021 14:27:46 +0300
<andrei.drimbarean@analog.com> wrote:

> From: Andrei Drimbarean <andrei.drimbarean@analog.com>
Hi Andrei

Welcome to IIO!

Anyhow, now for review.  Comments inline.

All patches need a patch description.  For a binding it's normal to put
a little bit about the device here.

> 
> Signed-off-by: Andrei Drimbarean <andrei.drimbarean@analog.com>
> ---
>  .../bindings/iio/light/adi,adpd188.yaml       | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/adi,adpd188.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/light/adi,adpd188.yaml b/Documentation/devicetree/bindings/iio/light/adi,adpd188.yaml
> new file mode 100644
> index 000000000000..3c08b0904803
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/light/adi,adpd188.yaml
> @@ -0,0 +1,72 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2019 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/light/adi,adpd188.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADPD188 device driver
> +
> +maintainers:
> +  - Andrei Drimbarean <andrei.drimbarean@analog.com>
> +
> +description: |
> +  Bindings for the Analog Devices ADPD188 device. 

What sort of device is it?  Give us a little bit of detail.

> The device support both SPI and I2C

Please keep the line length under 80 characters when it doesn't hurt readability to do so.

> +  interfaces. Datasheet can be found here:
> +    https://www.analog.com/media/en/technical-documentation/data-sheets/adpd188bi.pdf

Blank line after blocks such as here.

> +properties:
> +  compatible:
> +    enum:
> +      - adi,adpd188
> +
> +  reg:
> +    description: SPI chip select number or I2C slave address
> +    maxItems: 1

No real need to describe this as it's very standard.

    reg: true;

is probably enough info for this one.

> +
> +  interrupts:
> +    description: IRQ line for the device or device chain

Device chain? That's unusual enough that we should probably have
some more detail somewhere in this binding on what that means.

> +    maxItems: 1
> +
> +  spi-cpol: true
> +
> +  spi-cpha: true
> +
> +  spi-max-frequency:
> +    maximum: 10000000
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  status:
> +    const: 'okay'

That should not be in a binding document. It's a general property so
if we did list it, we would need it everywhere. It is also perfectly
acceptable to have a status that says the device isn't present.

> +
> +  adi,no-of-devices:
> +    description: Number of daisy-chained devices on an I2C bus
> +      string

More detail needed on this I think.
Also a default (0 or 1 based?)

> +    $ref: "http://devicetree.org/schemas/types.yaml#/definitions/uint8"

Are there any power supplies that should be described somewhere here?

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +
> +examples:
> +  - |
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +      status = "okay";
> +
> +      adpd188@64 {
> +        compatible = "adi,adpd188";
> +        reg = <0x64>;
> +        interrupts = <9 1>;
> +        interrupt-parent = <&gpio>;
> +        adi,no-of-devices = <8>;
> +      };
> +    };
> +
> +additionalProperties: false
additionalProperties belongs above the 'examples' block.
Rob Herring Oct. 8, 2021, 7:48 p.m. UTC | #2
On Fri, 08 Oct 2021 14:27:46 +0300, andrei.drimbarean@analog.com wrote:
> From: Andrei Drimbarean <andrei.drimbarean@analog.com>
> 
> Signed-off-by: Andrei Drimbarean <andrei.drimbarean@analog.com>
> ---
>  .../bindings/iio/light/adi,adpd188.yaml       | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/light/adi,adpd188.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/linux-dt-review/Documentation/devicetree/bindings/iio/light/adi,adpd188.example.dt.yaml: adpd188@64: adi,no-of-devices: missing size tag in [[8]]
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/light/adi,adpd188.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1538303

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

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.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/light/adi,adpd188.yaml b/Documentation/devicetree/bindings/iio/light/adi,adpd188.yaml
new file mode 100644
index 000000000000..3c08b0904803
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/light/adi,adpd188.yaml
@@ -0,0 +1,72 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/light/adi,adpd188.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADPD188 device driver
+
+maintainers:
+  - Andrei Drimbarean <andrei.drimbarean@analog.com>
+
+description: |
+  Bindings for the Analog Devices ADPD188 device. The device support both SPI and I2C
+  interfaces. Datasheet can be found here:
+    https://www.analog.com/media/en/technical-documentation/data-sheets/adpd188bi.pdf
+properties:
+  compatible:
+    enum:
+      - adi,adpd188
+
+  reg:
+    description: SPI chip select number or I2C slave address
+    maxItems: 1
+
+  interrupts:
+    description: IRQ line for the device or device chain
+    maxItems: 1
+
+  spi-cpol: true
+
+  spi-cpha: true
+
+  spi-max-frequency:
+    maximum: 10000000
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  status:
+    const: 'okay'
+
+  adi,no-of-devices:
+    description: Number of daisy-chained devices on an I2C bus
+      string
+    $ref: "http://devicetree.org/schemas/types.yaml#/definitions/uint8"
+
+required:
+  - compatible
+  - reg
+  - interrupts
+
+examples:
+  - |
+    i2c {
+      #address-cells = <1>;
+      #size-cells = <0>;
+      status = "okay";
+
+      adpd188@64 {
+        compatible = "adi,adpd188";
+        reg = <0x64>;
+        interrupts = <9 1>;
+        interrupt-parent = <&gpio>;
+        adi,no-of-devices = <8>;
+      };
+    };
+
+additionalProperties: false