diff mbox series

[v1,2/3] dt-bindings: ufs: Document Rockchip UFS host controller

Message ID 1722928800-137042-3-git-send-email-shawn.lin@rock-chips.com
State New
Headers show
Series Init support for RK3576 UFS controller | expand

Commit Message

Shawn Lin Aug. 6, 2024, 7:19 a.m. UTC
Document Rockchip UFS host controller for RK3576 SoC.

Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
---

 .../devicetree/bindings/ufs/rockchip,ufs.yaml      | 78 ++++++++++++++++++++++
 1 file changed, 78 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/ufs/rockchip,ufs.yaml

Comments

Krzysztof Kozlowski Aug. 6, 2024, 8:46 a.m. UTC | #1
On 06/08/2024 09:19, Shawn Lin wrote:
> Document Rockchip UFS host controller for RK3576 SoC.
> 

<form letter>
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.
</form letter>

Limited review follows.


> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
>  .../devicetree/bindings/ufs/rockchip,ufs.yaml      | 78 ++++++++++++++++++++++
>  1 file changed, 78 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ufs/rockchip,ufs.yaml
> 
> diff --git a/Documentation/devicetree/bindings/ufs/rockchip,ufs.yaml b/Documentation/devicetree/bindings/ufs/rockchip,ufs.yaml
> new file mode 100644
> index 0000000..e2e492c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ufs/rockchip,ufs.yaml

Filename as compatible.

> @@ -0,0 +1,78 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/ufs/rockchip,ufs.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip UFS Host Controller
> +
> +maintainers:
> +  - Shawn Lin <shawn.lin@rock-chips.com>
> +
> +allOf:
> +  - $ref: ufs-common.yaml
> +
> +properties:
> +  compatible:
> +    const: rockchip,rk3576-ufs
> +
> +  reg:
> +    maxItems: 5
> +
> +  reg-names:
> +    items:
> +     - const: hci
> +     - const: mphy
> +     - const: hci_grf
> +     - const: mphy_grf
> +     - const: hci_apb
> +
> +  clocks:
> +    maxItems: 4
> +
> +  clock-names:
> +    items:
> +      - const: core
> +      - const: pclk
> +      - const: pclk_mphy
> +      - const: ref_out
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    maxItems: 4

List the items instead

> +
> +required:
> +  - compatible
> +  - reg

reg-names are not required? Test your DTS (where is it btw?) without
reg-names then.

> +  - clocks
> +  - clock-names
> +  - power-domains
> +  - resets
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/rockchip,rk3576-cru.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/power/rk3576-power.h>
> +
> +    ufs: ufs@2a2d0000 {
> +            compatible = "rockchip,rk3576-ufs";
> +            reg = <0x0 0x2a2d0000 0 0x10000>,

Fix indentation. See writing schema.

Use 4 spaces for example indentation
.
> +	          <0x0 0x2b040000 0 0x10000>,
> +		  <0x0 0x2601f000 0 0x1000>,
> +		  <0x0 0x2603c000 0 0x1000>,
> +		  <0x0 0x2a2e0000 0 0x10000>;
> +            reg-names = "hci", "mphy", "hci_grf", "mphy_grf", "hci_apb";
> +            clocks = <&cru ACLK_UFS_SYS>, <&cru PCLK_USB_ROOT>, <&cru PCLK_MPHY>,
> +                     <&cru CLK_REF_UFS_CLKOUT>;
> +            clock-names = "core", "pclk", "pclk_mphy", "ref_out";
> +            interrupts = <GIC_SPI 361 IRQ_TYPE_LEVEL_HIGH>;
> +            power-domains = <&power RK3576_PD_USB>;
> +            resets = <&cru SRST_A_UFS_BIU>, <&cru SRST_A_UFS_SYS>, <&cru SRST_A_UFS>,
> +	             <&cru SRST_P_UFS_GRF>;
> +            reset-names = "biu", "sys", "ufs", "grf";

Crap. This was never tested!

NAK

Best regards,
Krzysztof
Rob Herring Aug. 6, 2024, 10:18 a.m. UTC | #2
On Tue, 06 Aug 2024 15:19:59 +0800, Shawn Lin wrote:
> Document Rockchip UFS host controller for RK3576 SoC.
> 
> Signed-off-by: Shawn Lin <shawn.lin@rock-chips.com>
> ---
> 
>  .../devicetree/bindings/ufs/rockchip,ufs.yaml      | 78 ++++++++++++++++++++++
>  1 file changed, 78 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/ufs/rockchip,ufs.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/ufs/rockchip,ufs.yaml:24:6: [warning] wrong indentation: expected 6 but found 5 (indentation)
./Documentation/devicetree/bindings/ufs/rockchip,ufs.yaml:65:1: [error] syntax error: found character '\t' that cannot start any token (syntax)

dtschema/dtc warnings/errors:
make[2]: *** Deleting file 'Documentation/devicetree/bindings/ufs/rockchip,ufs.example.dts'
Documentation/devicetree/bindings/ufs/rockchip,ufs.yaml:65:1: found a tab character where an indentation space is expected
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/ufs/rockchip,ufs.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/ufs/rockchip,ufs.yaml:65:1: found a tab character where an indentation space is expected
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/ufs/rockchip,ufs.yaml: ignoring, error parsing file
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/1722928800-137042-3-git-send-email-shawn.lin@rock-chips.com

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

Patch

diff --git a/Documentation/devicetree/bindings/ufs/rockchip,ufs.yaml b/Documentation/devicetree/bindings/ufs/rockchip,ufs.yaml
new file mode 100644
index 0000000..e2e492c
--- /dev/null
+++ b/Documentation/devicetree/bindings/ufs/rockchip,ufs.yaml
@@ -0,0 +1,78 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/ufs/rockchip,ufs.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip UFS Host Controller
+
+maintainers:
+  - Shawn Lin <shawn.lin@rock-chips.com>
+
+allOf:
+  - $ref: ufs-common.yaml
+
+properties:
+  compatible:
+    const: rockchip,rk3576-ufs
+
+  reg:
+    maxItems: 5
+
+  reg-names:
+    items:
+     - const: hci
+     - const: mphy
+     - const: hci_grf
+     - const: mphy_grf
+     - const: hci_apb
+
+  clocks:
+    maxItems: 4
+
+  clock-names:
+    items:
+      - const: core
+      - const: pclk
+      - const: pclk_mphy
+      - const: ref_out
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 4
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/rockchip,rk3576-cru.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/power/rk3576-power.h>
+
+    ufs: ufs@2a2d0000 {
+            compatible = "rockchip,rk3576-ufs";
+            reg = <0x0 0x2a2d0000 0 0x10000>,
+	          <0x0 0x2b040000 0 0x10000>,
+		  <0x0 0x2601f000 0 0x1000>,
+		  <0x0 0x2603c000 0 0x1000>,
+		  <0x0 0x2a2e0000 0 0x10000>;
+            reg-names = "hci", "mphy", "hci_grf", "mphy_grf", "hci_apb";
+            clocks = <&cru ACLK_UFS_SYS>, <&cru PCLK_USB_ROOT>, <&cru PCLK_MPHY>,
+                     <&cru CLK_REF_UFS_CLKOUT>;
+            clock-names = "core", "pclk", "pclk_mphy", "ref_out";
+            interrupts = <GIC_SPI 361 IRQ_TYPE_LEVEL_HIGH>;
+            power-domains = <&power RK3576_PD_USB>;
+            resets = <&cru SRST_A_UFS_BIU>, <&cru SRST_A_UFS_SYS>, <&cru SRST_A_UFS>,
+	             <&cru SRST_P_UFS_GRF>;
+            reset-names = "biu", "sys", "ufs", "grf";
+    };