diff mbox series

[2/3] dt-bindings: iio: add ADE9078

Message ID 20220217135140.5658-3-ciprian.hegbeli@analog.com (mailing list archive)
State Changes Requested
Headers show
Series Add ADE9078 Driver | expand

Commit Message

Ciprian Hegbeli Feb. 17, 2022, 1:51 p.m. UTC
Added device tree bindings for the ADE9078

Signed-off-by: chegbeli <ciprian.hegbeli@analog.com>
---
 .../bindings/iio/meter/adi,ade9078.yaml       | 153 ++++++++++++++++++
 include/dt-bindings/iio/meter/adi,ade9078.h   |  21 +++
 2 files changed, 174 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/iio/meter/adi,ade9078.yaml
 create mode 100644 include/dt-bindings/iio/meter/adi,ade9078.h

Comments

Krzysztof Kozlowski Feb. 17, 2022, 3:44 p.m. UTC | #1
On 17/02/2022 14:51, chegbeli wrote:
> Added device tree bindings for the ADE9078
> 
> Signed-off-by: chegbeli <ciprian.hegbeli@analog.com>
> ---
>  .../bindings/iio/meter/adi,ade9078.yaml       | 153 ++++++++++++++++++
>  include/dt-bindings/iio/meter/adi,ade9078.h   |  21 +++
>  2 files changed, 174 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/meter/adi,ade9078.yaml
>  create mode 100644 include/dt-bindings/iio/meter/adi,ade9078.h
> 
> diff --git a/Documentation/devicetree/bindings/iio/meter/adi,ade9078.yaml b/Documentation/devicetree/bindings/iio/meter/adi,ade9078.yaml
> new file mode 100644
> index 000000000000..e27d52e06e32
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/meter/adi,ade9078.yaml
> @@ -0,0 +1,153 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2021 Analog Devices Inc.
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/bindings/iio/addac/adi,ade9078.yaml#

Did you test your schema with dt_binding_check? This should fail.

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Analog Devices ADE9078 High Performance, Polyphase Energy Metering driver
> +
> +mainterners:
> +  -Ciprian Hegbeli <ciprian.hegbeli@analog.com>

Space after '-'.

> +
> +description: |
> +  The ADE9078 1 is a highly accurate, fully integrated energy
> +  metering device. Interfacing with both current transformer
> +  (CT) and Rogowski coil sensors, the ADE9078 enables users to
> +  develop a 3-phase metrology platform, which achieves high
> +  performance for Class 1 up to Class 0.2 meters.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - adi,ade9078
> +
> +    reg:
> +      maxItems: 1
> +
> +    '#address-cells':
> +      const: 1
> +
> +    '#size-cells':
> +      const: 0
> +
> +    spi-max-frequency:
> +      maximum: 1000000
> +
> +    interrupts:
> +      maxItems: 2
> +
> +    reset-gpios:
> +      description: |
> +        Must be the device tree identifier of the RESET pin. As the line is
> +        active low, it should be marked GPIO_ACTIVE_LOW.
> +      maxItems: 1
> +
> +    interrupt-names:
> +      description: |
> +        Names to be attributed to the interrupts of the device. Should be "irq0"
> +        or "irq1"

Skip description but instead list items ( - const: irq0 ...)

> +
> +    adi,wf-cap-sel:
> +      description: |
> +        This bit selects whether the waveform buffer is filled with resampled
> +        data or fixed data rate data
> +        0 - WF_RESAMPLED_DATA
> +        1 - WF_FIXED_DATA_RATE
> +      maxItems: 1
> +      minimum: 0
> +      maximum: 1

1. You need a type definition.
2. This looks like bool.
3. maxItems seems wrong here.
4. Do not describe registers and their bits but a feature of the device.

This applies to fields below as well.

> +
> +    adi,wf-mode:
> +      description: |
> +        Fixed data rate waveforms filling and trigger based modes.
> +        0 - WFB_FULL_MODE (Stop when waveform buffer is full)
> +        1 - WFB_EN_TRIG_MODE (Continuous fill—stop only on enabled trigger events)
> +        2 - WFB_CENTER_EN_TRIG_MODE (Continuous filling—center capture around enabled trigger events)
> +        3 - WFB_SVAE_EN_TRIG_MODE (Continuous fill—save event address of enabled trigger events)
> +      maxItems: 1
> +      minimum: 0
> +      maximum: 3

Everything above + this looks like an enum, so use enum, instead of min/max.

> +
> +    adi,wf-src:
> +      description: |
> +        Waveform buffer source and DREADY, data ready update rate, selection.
> +        0 - WFB_SRC_SINC4 (Sinc4 output, at 16 kSPS)
> +        1 - Reserved
> +        2 - WFB_SRC_SINC4_IIR_LPF (Sinc4 + IIR LPF output, at 4 kSPS)
> +        3 - WFB_SRC_DSP (Current and voltage channel waveform samples,processed by the DSP
> +            (xI_PCF, xV_PCF) at 4 kSPS)
> +      maxItems: 1
> +      minimum: 0
> +      maximum: 3
> +
> +    adi,wf-in-en:
> +      description: |
> +        This setting determines whether the IN waveform samples are read out of
> +        the waveform buffer through SPI.
> +        0 - WFB_IN_DISABLE
> +        1 - WFB_IN_EN
> +      maxItems: 1
> +      minimum: 0
> +      maximum: 1

Also bool.

> +
> +  required:
> +    - compatible
> +    - reg
> +    - reset-gpios
> +    - interrupts
> +    - interrupt-names
> +    - adi,wf-cap-sel
> +    - adi,wf-mode
> +    - adi,wf-src
> +    - adi,wf-in-en
> +
> +patternProperties:
> +  "^phase@[0-3]$":
> +    type: object
> +    description: |
> +      Represents the external phases which are externally connected. Each phase
> +      has a current, voltage and power component
> +
> +    properties:
> +      reg:
> +        description: |
> +          The phase represented by a number
> +          0 - Phase A
> +          1 - unused
> +          2 - Phase B
> +          3 - unused
> +          4 - Phase C
> +        maxItems: 1
> +        minimum: 0
> +        maximum: 4
> +
> +    required:
> +      - reg
> +
> +examples:
> +  - |
> +    ade9078@0 {

Generic node name. Please pick something appropriate. Maybe "meter"?

> +	compatible = "adi,ade9078";

You have entirely broken indentation here. Use four spaces for DTS example.

> +	reg = <0>;
> +	spi-max-frequency = <7000000>;
> +
> +	#address-cells = <1>;
> +	#size-cells = <0>;
> +	reset-gpios = <&gpio 4 GPIO_ACTIVE_LOW>;
> +	interrupts = <2 IRQ_TYPE_EDGE_FALLING>, <3 IRQ_TYPE_EDGE_FALLING>;

You clearly did not test it... this won't work without headers.

> +	interrupt-names = "irq0", "irq1";
> +	interrupt-parent = <&gpio>;


Best regards,
Krzysztof
Rob Herring (Arm) Feb. 21, 2022, 2:36 a.m. UTC | #2
On Thu, 17 Feb 2022 15:51:39 +0200, chegbeli wrote:
> Added device tree bindings for the ADE9078
> 
> Signed-off-by: chegbeli <ciprian.hegbeli@analog.com>
> ---
>  .../bindings/iio/meter/adi,ade9078.yaml       | 153 ++++++++++++++++++
>  include/dt-bindings/iio/meter/adi,ade9078.h   |  21 +++
>  2 files changed, 174 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/iio/meter/adi,ade9078.yaml
>  create mode 100644 include/dt-bindings/iio/meter/adi,ade9078.h
> 

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:
./Documentation/devicetree/bindings/iio/meter/adi,ade9078.yaml:131:1: [error] syntax error: found character '\t' that cannot start any token (syntax)

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/iio/meter/adi,ade9078.yaml:  while scanning a block scalar
  in "<unicode string>", line 129, column 5
found a tab character where an indentation space is expected
  in "<unicode string>", line 131, column 1
make[1]: *** Deleting file 'Documentation/devicetree/bindings/iio/meter/adi,ade9078.example.dts'
Traceback (most recent call last):
  File "/usr/local/bin/dt-extract-example", line 46, in <module>
    binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
  File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/main.py", line 434, in load
    return constructor.get_single_data()
  File "/usr/local/lib/python3.8/dist-packages/ruamel/yaml/constructor.py", line 119, in get_single_data
    node = self.composer.get_single_node()
  File "_ruamel_yaml.pyx", line 706, in _ruamel_yaml.CParser.get_single_node
  File "_ruamel_yaml.pyx", line 724, in _ruamel_yaml.CParser._compose_document
  File "_ruamel_yaml.pyx", line 775, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 889, in _ruamel_yaml.CParser._compose_mapping_node
  File "_ruamel_yaml.pyx", line 773, in _ruamel_yaml.CParser._compose_node
  File "_ruamel_yaml.pyx", line 848, in _ruamel_yaml.CParser._compose_sequence_node
  File "_ruamel_yaml.pyx", line 904, in _ruamel_yaml.CParser._parse_next_event
ruamel.yaml.scanner.ScannerError: while scanning a block scalar
  in "<unicode string>", line 129, column 5
found a tab character where an indentation space is expected
  in "<unicode string>", line 131, column 1
make[1]: *** [Documentation/devicetree/bindings/Makefile:25: Documentation/devicetree/bindings/iio/meter/adi,ade9078.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/iio/meter/adi,ade9078.yaml: ignoring, error parsing file
make: *** [Makefile:1398: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

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

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/meter/adi,ade9078.yaml b/Documentation/devicetree/bindings/iio/meter/adi,ade9078.yaml
new file mode 100644
index 000000000000..e27d52e06e32
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/meter/adi,ade9078.yaml
@@ -0,0 +1,153 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2021 Analog Devices Inc.
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/bindings/iio/addac/adi,ade9078.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analog Devices ADE9078 High Performance, Polyphase Energy Metering driver
+
+mainterners:
+  -Ciprian Hegbeli <ciprian.hegbeli@analog.com>
+
+description: |
+  The ADE9078 1 is a highly accurate, fully integrated energy
+  metering device. Interfacing with both current transformer
+  (CT) and Rogowski coil sensors, the ADE9078 enables users to
+  develop a 3-phase metrology platform, which achieves high
+  performance for Class 1 up to Class 0.2 meters.
+
+properties:
+  compatible:
+    enum:
+      - adi,ade9078
+
+    reg:
+      maxItems: 1
+
+    '#address-cells':
+      const: 1
+
+    '#size-cells':
+      const: 0
+
+    spi-max-frequency:
+      maximum: 1000000
+
+    interrupts:
+      maxItems: 2
+
+    reset-gpios:
+      description: |
+        Must be the device tree identifier of the RESET pin. As the line is
+        active low, it should be marked GPIO_ACTIVE_LOW.
+      maxItems: 1
+
+    interrupt-names:
+      description: |
+        Names to be attributed to the interrupts of the device. Should be "irq0"
+        or "irq1"
+
+    adi,wf-cap-sel:
+      description: |
+        This bit selects whether the waveform buffer is filled with resampled
+        data or fixed data rate data
+        0 - WF_RESAMPLED_DATA
+        1 - WF_FIXED_DATA_RATE
+      maxItems: 1
+      minimum: 0
+      maximum: 1
+
+    adi,wf-mode:
+      description: |
+        Fixed data rate waveforms filling and trigger based modes.
+        0 - WFB_FULL_MODE (Stop when waveform buffer is full)
+        1 - WFB_EN_TRIG_MODE (Continuous fill—stop only on enabled trigger events)
+        2 - WFB_CENTER_EN_TRIG_MODE (Continuous filling—center capture around enabled trigger events)
+        3 - WFB_SVAE_EN_TRIG_MODE (Continuous fill—save event address of enabled trigger events)
+      maxItems: 1
+      minimum: 0
+      maximum: 3
+
+    adi,wf-src:
+      description: |
+        Waveform buffer source and DREADY, data ready update rate, selection.
+        0 - WFB_SRC_SINC4 (Sinc4 output, at 16 kSPS)
+        1 - Reserved
+        2 - WFB_SRC_SINC4_IIR_LPF (Sinc4 + IIR LPF output, at 4 kSPS)
+        3 - WFB_SRC_DSP (Current and voltage channel waveform samples,processed by the DSP
+            (xI_PCF, xV_PCF) at 4 kSPS)
+      maxItems: 1
+      minimum: 0
+      maximum: 3
+
+    adi,wf-in-en:
+      description: |
+        This setting determines whether the IN waveform samples are read out of
+        the waveform buffer through SPI.
+        0 - WFB_IN_DISABLE
+        1 - WFB_IN_EN
+      maxItems: 1
+      minimum: 0
+      maximum: 1
+
+  required:
+    - compatible
+    - reg
+    - reset-gpios
+    - interrupts
+    - interrupt-names
+    - adi,wf-cap-sel
+    - adi,wf-mode
+    - adi,wf-src
+    - adi,wf-in-en
+
+patternProperties:
+  "^phase@[0-3]$":
+    type: object
+    description: |
+      Represents the external phases which are externally connected. Each phase
+      has a current, voltage and power component
+
+    properties:
+      reg:
+        description: |
+          The phase represented by a number
+          0 - Phase A
+          1 - unused
+          2 - Phase B
+          3 - unused
+          4 - Phase C
+        maxItems: 1
+        minimum: 0
+        maximum: 4
+
+    required:
+      - reg
+
+examples:
+  - |
+    ade9078@0 {
+	compatible = "adi,ade9078";
+	reg = <0>;
+	spi-max-frequency = <7000000>;
+
+	#address-cells = <1>;
+	#size-cells = <0>;
+	reset-gpios = <&gpio 4 GPIO_ACTIVE_LOW>;
+	interrupts = <2 IRQ_TYPE_EDGE_FALLING>, <3 IRQ_TYPE_EDGE_FALLING>;
+	interrupt-names = "irq0", "irq1";
+	interrupt-parent = <&gpio>;
+
+	adi,wf-cap-sel = <WF_FIXED_DATA_RATE>;
+	adi,wf-mode = <WFB_FULL_MODE>;
+	adi,wf-src = <WFB_SRC_DSP>;
+	adi,wf-in-en = <WFB_IN_DISABLE>;
+
+	phase@0 {
+	    reg = <0>;
+	};
+	phase@1 {
+	    reg = <4>;
+	};
+    };
diff --git a/include/dt-bindings/iio/meter/adi,ade9078.h b/include/dt-bindings/iio/meter/adi,ade9078.h
new file mode 100644
index 000000000000..85004251a36b
--- /dev/null
+++ b/include/dt-bindings/iio/meter/adi,ade9078.h
@@ -0,0 +1,21 @@ 
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _DT_BINDINGS_ADI_ADE9078_H
+#define _DT_BINDINGS_ADI_ADE9078_H
+
+#define WF_RESAMPLED_DATA	0x0
+#define WF_FIXED_DATA_RATE	0x1
+
+#define WFB_FULL_MODE		0x0
+#define WFB_EN_TRIG_MODE	0x1
+#define WFB_CENTER_EN_TRIG_MODE	0x2
+#define WFB_SVAE_EN_TRIG_MODE	0x3
+
+#define WFB_SRC_SINC4		0x0
+#define WFB_SRC_SINC4_IIR_LPF	0x2
+#define WFB_SRC_DSP		0x3
+
+#define WFB_IN_DISABLE		0x0
+#define WFB_IN_EN		0x1
+
+#endif /* _DT_BINDINGS_ADI_ADE9078_H */