diff mbox series

[v2,1/2] dt-bindings: iio: addac: one-bit-adc-dac yaml documentation

Message ID 20220111115919.14645-1-cristian.pop@analog.com
State New
Headers show
Series [v2,1/2] dt-bindings: iio: addac: one-bit-adc-dac yaml documentation | expand

Commit Message

Cristian Pop Jan. 11, 2022, 11:59 a.m. UTC 
This adds device tree bindings for the one-bit-adc-dac.

Signed-off-by: Cristian Pop <cristian.pop@analog.com>
V1->V2                                                                     
 - I am aware of the recommendation of rename/move this driver. Should we  
   consider "drivers/io/gpio.c"?                                           
 - Add .yaml file                                                          
 - Remove blank lines, remove unnecessary coma                             
 - Remove macros for channels                                              
 - Check if channel is input for write_raw                                 
 - Use labels instead of extend_name                                       
 - Fix channel indexing                                                    
 - Use "sizeof(*channels)" in devm_kcalloc()                               
 - Remove assignment: " indio_dev->dev.parent = &pdev->dev;"               
 - Remove "platform_set_drvdata"                                           
 - Remove "adi" from compatible string since is not ADI specific driver.
---
 .../bindings/iio/addac/one-bit-adc-dac.yaml   | 89 +++++++++++++++++++
 1 file changed, 89 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.yaml

Comments

Rob Herring (Arm) Jan. 11, 2022, 3:14 p.m. UTC | #1 
On Tue, 11 Jan 2022 13:59:18 +0200, Cristian Pop wrote:
> This adds device tree bindings for the one-bit-adc-dac.
> 
> Signed-off-by: Cristian Pop <cristian.pop@analog.com>
> V1->V2
>  - I am aware of the recommendation of rename/move this driver. Should we
>    consider "drivers/io/gpio.c"?
>  - Add .yaml file
>  - Remove blank lines, remove unnecessary coma
>  - Remove macros for channels
>  - Check if channel is input for write_raw
>  - Use labels instead of extend_name
>  - Fix channel indexing
>  - Use "sizeof(*channels)" in devm_kcalloc()
>  - Remove assignment: " indio_dev->dev.parent = &pdev->dev;"
>  - Remove "platform_set_drvdata"
>  - Remove "adi" from compatible string since is not ADI specific driver.
> ---
>  .../bindings/iio/addac/one-bit-adc-dac.yaml   | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.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:
Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.example.dts:19.27-47.11: Warning (unit_address_vs_reg): /example-0/one-bit-adc-dac@0: node has a unit name, but no reg or ranges property
Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.example.dt.yaml:0:0: /example-0/one-bit-adc-dac@0: failed to match any schema with compatible: ['one-bit-adc-dac']

doc reference errors (make refcheckdocs):

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

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.
Rob Herring (Arm) Jan. 11, 2022, 10:02 p.m. UTC | #2 
On Tue, Jan 11, 2022 at 01:59:18PM +0200, Cristian Pop wrote:
> This adds device tree bindings for the one-bit-adc-dac.

I have no idea what a one-bit-adc-dac is. Please describe or provide a 
reference to what this h/w looks like.

> 
> Signed-off-by: Cristian Pop <cristian.pop@analog.com>
> V1->V2                                                                     

This belongs below the '---'

>  - I am aware of the recommendation of rename/move this driver. Should we  
>    consider "drivers/io/gpio.c"?                                           
>  - Add .yaml file                                                          
>  - Remove blank lines, remove unnecessary coma                             
>  - Remove macros for channels                                              
>  - Check if channel is input for write_raw                                 
>  - Use labels instead of extend_name                                       
>  - Fix channel indexing                                                    
>  - Use "sizeof(*channels)" in devm_kcalloc()                               
>  - Remove assignment: " indio_dev->dev.parent = &pdev->dev;"               
>  - Remove "platform_set_drvdata"                                           
>  - Remove "adi" from compatible string since is not ADI specific driver.
> ---
>  .../bindings/iio/addac/one-bit-adc-dac.yaml   | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.yaml b/Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.yaml
> new file mode 100644
> index 000000000000..dbed0f3b1ca4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2020 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/addac/one-bit-adc-dac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices one bit ADC DAC driver
> +
> +maintainers:
> +  - Cristian Pop <cristian.pop@analog.com>
> +
> +description: |
> +  One bit ADC DAC driver
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,one-bit-adc-dac
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  in-gpios:
> +    description: Input GPIOs
> +
> +  out-gpios:
> +    description: Output GPIOs

No constraints on how many GPIOs?

> +
> +required:
> +  - compatible
> +  - in-gpios
> +  - out-gpios
> +
> +patternProperties:
> +  "^channel@([0-9]|1[0-5])$":
> +    type: object
> +    description: |
> +      Represents the external channels which are connected to the ADDAC.
> +
> +    properties:
> +      reg:
> +        maxItems: 1
> +        description: |
> +          The channel number.
> +
> +      label:
> +        description: |
> +          Unique name to identify which channel this is.
> +
> +    required:
> +      - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    one-bit-adc-dac@0 {
> +        compatible = "one-bit-adc-dac";
> +
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        in-gpios = <&gpio 17 0>, <&gpio 27 0>;
> +        out-gpios = <&gpio 23 0>, <&gpio 24 0>;
> +
> +        channel@0 {
> +          reg = <0>;

What does '0' correspond to?

> +          label = "i_17";

Why is this needed? 'label' is supposed to correspond to physical 
labelling of ports. IOW, for identification by humans looking at the 
device.

This all looks duplicated from information in in-gpios and out-gpios.

> +        };
> +
> +        channel@1 {
> +          reg = <1>;
> +          label = "i_27";
> +        };
> +
> +        channel@2 {
> +          reg = <2>;
> +          label = "o_23";
> +        };
> +
> +        channel@3 {
> +          reg = <3>;
> +          label = "o_24";
> +        };
> +    };
> -- 
> 2.17.1
> 
>
Jonathan Cameron Jan. 16, 2022, 11:30 a.m. UTC | #3 
On Tue, 11 Jan 2022 13:59:18 +0200
Cristian Pop <cristian.pop@analog.com> wrote:

> This adds device tree bindings for the one-bit-adc-dac.

This series really needs a cover letter where you describe in
general terms what the aim is etc.

> 
> Signed-off-by: Cristian Pop <cristian.pop@analog.com>
> V1->V2                                                                     
>  - I am aware of the recommendation of rename/move this driver. Should we  
>    consider "drivers/io/gpio.c"? 

Probably keep with the naming of the hwmon iio bridge and go with
gpio_iio.c to indicate bridge from gpio to iio.

I'll put more general comments in patch 2 review, but I'm very doubtful
that setting this up via dt is giong to be the way forward.
Shall we say, the iio_hwmon bindings have always been controversial and
we'd probably not get away with them today...

Reason being it's policy not wiring and reflects internal Linux subsystem
constructs, not generic things applicable to all operating systems.

                                          
>  - Add .yaml file                                                          
>  - Remove blank lines, remove unnecessary coma                             
>  - Remove macros for channels                                              
>  - Check if channel is input for write_raw                                 
>  - Use labels instead of extend_name                                       
>  - Fix channel indexing                                                    
>  - Use "sizeof(*channels)" in devm_kcalloc()                               
>  - Remove assignment: " indio_dev->dev.parent = &pdev->dev;"               
>  - Remove "platform_set_drvdata"                               
Not in this patch so shouldn't be in this description.            
>  - Remove "adi" from compatible string since is not ADI specific driver.
> ---

Version log here for stuff in this patch.  Fine to have the log in the
cover letter if the changes tend to go across multiple patches (renames etc).

>  .../bindings/iio/addac/one-bit-adc-dac.yaml   | 89 +++++++++++++++++++
>  1 file changed, 89 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.yaml
> 
> diff --git a/Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.yaml b/Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.yaml
> new file mode 100644
> index 000000000000..dbed0f3b1ca4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.yaml
> @@ -0,0 +1,89 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2020 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/addac/one-bit-adc-dac.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices one bit ADC DAC driver
> +
> +maintainers:
> +  - Cristian Pop <cristian.pop@analog.com>
> +
> +description: |
> +  One bit ADC DAC driver
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,one-bit-adc-dac
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  in-gpios:
> +    description: Input GPIOs
> +
> +  out-gpios:
> +    description: Output GPIOs
> +
> +required:
> +  - compatible
> +  - in-gpios
> +  - out-gpios
> +
> +patternProperties:
> +  "^channel@([0-9]|1[0-5])$":
> +    type: object
> +    description: |
> +      Represents the external channels which are connected to the ADDAC.
> +
> +    properties:
> +      reg:
> +        maxItems: 1
> +        description: |
> +          The channel number.
> +
> +      label:
> +        description: |
> +          Unique name to identify which channel this is.
> +
> +    required:
> +      - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    one-bit-adc-dac@0 {
> +        compatible = "one-bit-adc-dac";
> +
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +
> +        in-gpios = <&gpio 17 0>, <&gpio 27 0>;
> +        out-gpios = <&gpio 23 0>, <&gpio 24 0>;
> +
> +        channel@0 {
> +          reg = <0>;
> +          label = "i_17";
> +        };
> +
> +        channel@1 {
> +          reg = <1>;
> +          label = "i_27";
> +        };
> +
> +        channel@2 {
> +          reg = <2>;
> +          label = "o_23";
> +        };
> +
> +        channel@3 {
> +          reg = <3>;
> +          label = "o_24";
> +        };
> +    };
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.yaml b/Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.yaml
new file mode 100644
index 000000000000..dbed0f3b1ca4
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/addac/one-bit-adc-dac.yaml
@@ -0,0 +1,89 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2020 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/addac/one-bit-adc-dac.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices one bit ADC DAC driver
+
+maintainers:
+  - Cristian Pop <cristian.pop@analog.com>
+
+description: |
+  One bit ADC DAC driver
+
+properties:
+  compatible:
+    enum:
+      - adi,one-bit-adc-dac
+
+  '#address-cells':
+    const: 1
+
+  '#size-cells':
+    const: 0
+
+  in-gpios:
+    description: Input GPIOs
+
+  out-gpios:
+    description: Output GPIOs
+
+required:
+  - compatible
+  - in-gpios
+  - out-gpios
+
+patternProperties:
+  "^channel@([0-9]|1[0-5])$":
+    type: object
+    description: |
+      Represents the external channels which are connected to the ADDAC.
+
+    properties:
+      reg:
+        maxItems: 1
+        description: |
+          The channel number.
+
+      label:
+        description: |
+          Unique name to identify which channel this is.
+
+    required:
+      - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    one-bit-adc-dac@0 {
+        compatible = "one-bit-adc-dac";
+
+        #address-cells = <1>;
+        #size-cells = <0>;
+
+        in-gpios = <&gpio 17 0>, <&gpio 27 0>;
+        out-gpios = <&gpio 23 0>, <&gpio 24 0>;
+
+        channel@0 {
+          reg = <0>;
+          label = "i_17";
+        };
+
+        channel@1 {
+          reg = <1>;
+          label = "i_27";
+        };
+
+        channel@2 {
+          reg = <2>;
+          label = "o_23";
+        };
+
+        channel@3 {
+          reg = <3>;
+          label = "o_24";
+        };
+    };