diff mbox series

[v4,1/3] dt-bindings: uwb: Device tree information for Nxp SR1XX SOCs

Message ID 20220527184351.3829543-2-manjunatha.venkatesh@nxp.com
State New
Headers show
Series [v4,1/3] dt-bindings: uwb: Device tree information for Nxp SR1XX SOCs | expand

Commit Message

Manjunatha Venkatesh May 27, 2022, 6:43 p.m. UTC 
Ultra-wideband (UWB) is a short-range wireless communication protocol.

NXP has SR1XX family of UWB Subsystems (UWBS) devices. SR1XX SOCs
are FiRa Compliant. SR1XX SOCs are flash less devices and they need
Firmware Download on every device boot. More details on the SR1XX Family
can be found at https://www.nxp.com/products/:UWB-TRIMENSION

The sr1xx driver work the SR1XX Family of UWBS, and uses UWB Controller
Interface (UCI).  The corresponding details are available in the FiRa
Consortium Website (https://www.firaconsortium.org/).

Message-ID: <20220504171337.3416983-1-manjunatha.venkatesh@nxp.com>
Signed-off-by: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
---
 .../bindings/uwb/nxp,uwb-sr1xx.yaml           | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/uwb/nxp,uwb-sr1xx.yaml

Comments

Krzysztof Kozlowski May 29, 2022, 11:37 a.m. UTC | #1 
On 27/05/2022 20:43, Manjunatha Venkatesh wrote:
> Ultra-wideband (UWB) is a short-range wireless communication protocol.
> 
> NXP has SR1XX family of UWB Subsystems (UWBS) devices. SR1XX SOCs
> are FiRa Compliant. SR1XX SOCs are flash less devices and they need
> Firmware Download on every device boot. More details on the SR1XX Family
> can be found at https://www.nxp.com/products/:UWB-TRIMENSION
> 
> The sr1xx driver work the SR1XX Family of UWBS, and uses UWB Controller
> Interface (UCI).  The corresponding details are available in the FiRa
> Consortium Website (https://www.firaconsortium.org/).
> 
> Message-ID: <20220504171337.3416983-1-manjunatha.venkatesh@nxp.com>

This is a confusing tag... Why do you need it and what does it mean in
Linux kernel process?

> Signed-off-by: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> ---
>  .../bindings/uwb/nxp,uwb-sr1xx.yaml           | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/uwb/nxp,uwb-sr1xx.yaml
> 
> diff --git a/Documentation/devicetree/bindings/uwb/nxp,uwb-sr1xx.yaml b/Documentation/devicetree/bindings/uwb/nxp,uwb-sr1xx.yaml
> new file mode 100644
> index 000000000000..226fec908968
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/uwb/nxp,uwb-sr1xx.yaml
> @@ -0,0 +1,67 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/uwb/nxp,uwb-sr1xx.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Ultra Wide Band(UWB)driver support for NXP SR1XX SOCs family

No references to driver. Please describe here the title of the hardware.

> +
> +maintainers:
> +  - Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> +
> +description: The sr1xx driver work for the NXP SR1XX Family of Ultra Wide
> +    Band Subsystem(UWBS), and uses UWB Controller Interface(UCI).
> +    The corresponding details are available in the FiRa Consortium Website
> +    (https://www.firaconsortium.org/).More details on the SR1XX Family can be
> +    found at https://www.nxp.com/products/:UWB-TRIMENSION

The same. Additionally use proper white-spaces. Before every '('. After
every '.'.

> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - nxp,sr1xx
> +
> +      - const: nxp,sr1xx

This is wrong and does not make any sense. You also did not test the
bindings.

Please test them before submitting. No need to waste reviewers time for
basic automation tasks.

Also do not include any wildcards (1xx) but specific model name.

> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    maxItems: 1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - clock-names
> +  - clocks
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    spi2: spi@ffd68000 {

No need for alias.

> +		compatible = "arm,pl022", "arm,primecell";
> +		reg = <0x0 0xffd68000 0x0 0x1000>;
> +		#address-cells = <1>;
> +		#size-cells = <0>;
> +		interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
> +		clocks = <&crg_ctrl HI3660_CLK_GATE_SPI2>;
> +		clock-names = "apb_pclk";
> +		pinctrl-names = "default";
> +		pinctrl-0 = <&spi2_pmx_func &spi2_cfg_func>;
> +		num-cs = <1>;
> +		cs-gpios = <&gpio27 2 0>;
> +		status = "ok";

No point of enabling this.

> +	sr1xx@0 {

The indentation and look of it is unacceptable...

> +	 compatible = "nxp,sr1xx";
> +	 reg = <0>;
> +	 nxp,sr1xx-irq = <&gpio26 1 0>;
> +	 nxp,sr1xx-ce = <&gpio2 5 0>;
> +	 nxp,sr1xx-ri = <&gpio24 1 0>;
> +	 spi-max-frequency = <20000000>;
> +	};
> +    };
> +
> +...


Best regards,
Krzysztof
Rob Herring (Arm) May 29, 2022, 1:27 p.m. UTC | #2 
On Sat, 28 May 2022 00:13:49 +0530, Manjunatha Venkatesh wrote:
> Ultra-wideband (UWB) is a short-range wireless communication protocol.
> 
> NXP has SR1XX family of UWB Subsystems (UWBS) devices. SR1XX SOCs
> are FiRa Compliant. SR1XX SOCs are flash less devices and they need
> Firmware Download on every device boot. More details on the SR1XX Family
> can be found at https://www.nxp.com/products/:UWB-TRIMENSION
> 
> The sr1xx driver work the SR1XX Family of UWBS, and uses UWB Controller
> Interface (UCI).  The corresponding details are available in the FiRa
> Consortium Website (https://www.firaconsortium.org/).
> 
> Message-ID: <20220504171337.3416983-1-manjunatha.venkatesh@nxp.com>
> Signed-off-by: Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
> ---
>  .../bindings/uwb/nxp,uwb-sr1xx.yaml           | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/uwb/nxp,uwb-sr1xx.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:
./Documentation/devicetree/bindings/uwb/nxp,uwb-sr1xx.yaml:45:1: [error] syntax error: found character '\t' that cannot start any token (syntax)

dtschema/dtc warnings/errors:
make[1]: *** Deleting file 'Documentation/devicetree/bindings/uwb/nxp,uwb-sr1xx.example.dts'
Traceback (most recent call last):
  File "/usr/local/bin/dt-extract-example", line 52, in <module>
    binding = yaml.load(open(args.yamlfile, encoding='utf-8').read())
  File "/usr/local/lib/python3.10/dist-packages/ruamel/yaml/main.py", line 434, in load
    return constructor.get_single_data()
  File "/usr/local/lib/python3.10/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 43, column 5
found a tab character where an indentation space is expected
  in "<unicode string>", line 45, column 1
make[1]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/uwb/nxp,uwb-sr1xx.example.dts] Error 1
make[1]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/uwb/nxp,uwb-sr1xx.yaml:  while scanning a block scalar
  in "<unicode string>", line 43, column 5
found a tab character where an indentation space is expected
  in "<unicode string>", line 45, column 1
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/uwb/nxp,uwb-sr1xx.yaml: ignoring, error parsing file
make: *** [Makefile:1401: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

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

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/uwb/nxp,uwb-sr1xx.yaml b/Documentation/devicetree/bindings/uwb/nxp,uwb-sr1xx.yaml
new file mode 100644
index 000000000000..226fec908968
--- /dev/null
+++ b/Documentation/devicetree/bindings/uwb/nxp,uwb-sr1xx.yaml
@@ -0,0 +1,67 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/uwb/nxp,uwb-sr1xx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ultra Wide Band(UWB)driver support for NXP SR1XX SOCs family
+
+maintainers:
+  - Manjunatha Venkatesh <manjunatha.venkatesh@nxp.com>
+
+description: The sr1xx driver work for the NXP SR1XX Family of Ultra Wide
+    Band Subsystem(UWBS), and uses UWB Controller Interface(UCI).
+    The corresponding details are available in the FiRa Consortium Website
+    (https://www.firaconsortium.org/).More details on the SR1XX Family can be
+    found at https://www.nxp.com/products/:UWB-TRIMENSION
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - nxp,sr1xx
+
+      - const: nxp,sr1xx
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clock-names
+  - clocks
+
+additionalProperties: false
+
+examples:
+  - |
+    spi2: spi@ffd68000 {
+		compatible = "arm,pl022", "arm,primecell";
+		reg = <0x0 0xffd68000 0x0 0x1000>;
+		#address-cells = <1>;
+		#size-cells = <0>;
+		interrupts = <GIC_SPI 116 IRQ_TYPE_LEVEL_HIGH>;
+		clocks = <&crg_ctrl HI3660_CLK_GATE_SPI2>;
+		clock-names = "apb_pclk";
+		pinctrl-names = "default";
+		pinctrl-0 = <&spi2_pmx_func &spi2_cfg_func>;
+		num-cs = <1>;
+		cs-gpios = <&gpio27 2 0>;
+		status = "ok";
+	sr1xx@0 {
+	 compatible = "nxp,sr1xx";
+	 reg = <0>;
+	 nxp,sr1xx-irq = <&gpio26 1 0>;
+	 nxp,sr1xx-ce = <&gpio2 5 0>;
+	 nxp,sr1xx-ri = <&gpio24 1 0>;
+	 spi-max-frequency = <20000000>;
+	};
+    };
+
+...