Re: [PATCH 3/6] dt-bindings: pinctrl: Add fsl,ls1012a-pinctrl yaml file

On Tue, 27 Aug 2024, Krzysztof Kozlowski wrote:

On Tue, Aug 27, 2024 at 12:10:44PM +1000, David Leonard wrote:

quoted

Add a binding schema and examples for the LS1012A's pinctrl function.

Signed-off-by: David Leonard <redacted>
---

It does not look like you tested the bindings, at least after quick
look. Please run  (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.

I've since updated tools (dt-valdate 2024.5 and yamllint 1.33.0) and
rebased onto v6.11-rc1.

quoted

--- /dev/null
+++ b/Documentation/devicetree/bindings/pinctrl/fsl,ls1012a-pinctrl.yaml

@@ -0,0 +1,83 @@

quoted

+description: >

Drop >

quoted

+  Bindings for LS1012A pinmux control.

Drop "Bindings for" and explain the hardware.

Changed to

  description:
    The LS1012A pinmux controller can override the RCW and
    alter some pin groups' functions in a limited way.

quoted

+
+properties:
+  compatible:
+    const: fsl,ls1012a-pinctrl
+
+  reg:
+    description: Specifies the base address of the PMUXCR0 register.
+    maxItems: 2

Instead list and describe the items.

Changed to

    reg:
      items:
        - description: Physical base address of the PMUXCR0 register.
        - description: Size of the PMUXCR0 register (4).

Is this what you meant?

quoted

+
+  big-endian:
+    description: If present, the PMUXCR0 register is implemented in big-endian.

Why is this here? Either it is or it is not?

You're right. Changed to

    big-endian: true

(This also lead to some code simplification)

quoted

+    type: boolean
+
+  dcfg-regmap:

Missing vendor prefix.

Will use "fsl,"

quoted

+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      The phandle of the syscon node for the DCFG registers.

Instead explain what it is needed it for, how is it used.

Changed to

    fsl,dcfg-regmap:
      $ref: /schemas/types.yaml#/definitions/phandle
      description:
        The phandle of the syscon node for the DCFG registers
        providing the RCW's pin configuration that is in effect when
        the pinmux controller is disabled.

quoted

+
+patternProperties:
+  '^pinctrl-':

Rather -pins$ or ^pins-

Changed to ^pins-  See example at the end.

quoted

+allOf:
+  - $ref: pinctrl.yaml#

Thies goes after required.

quoted

+
+required:
+  - compatible
+  - reg

Relocated

quoted

+additionalProperties: false
+
+examples:
+  - |
+    pinctrl: pinctrl@1570430 {
+        compatible = "fsl,ls1012a-pinctrl";
+        reg = <0x0 0x1570430 0x0 0x4>;
+        big-endian;
+        dcfg-regmap = <&dcfg>;
+        pinctrl_qspi_1: pinctrl-qspi-1 {
+            groups = "qspi_1_grp";
+            function = "spi";
+        };
+        pinctrl_qspi_2: pinctrl-qspi-2 {
+            groups = "qspi_1_grp", "qspi_2_grp";
+            function = "spi";
+        };
+        pinctrl_qspi_4: pinctrl-qspi-4 {
+            groups = "qspi_1_grp", "qspi_2_grp", "qspi_3_grp";
+            function = "spi";
+        };
+    };
+  - |
+    qspi: quadspi@1550000 {

Drop, useless and not related.

Dropped the qspi example. Examples are now

   examples:
     - |
       pinctrl: pinctrl@1570430 {
 	  compatible = "fsl,ls1012a-pinctrl";
 	  reg = <0x0 0x1570430 0x0 0x4>;
 	  big-endian;
 	  fsl,dcfg-regmap = <&dcfg>;
 	  pinctrl_qspi_1: pins-qspi-1 { /* buswidth 1 */
 	      groups = "qspi_1_grp";
 	      function = "spi";
 	  };
 	  pinctrl_qspi_2: pins-qspi-2 { /* buswidth 2 */
 	      groups = "qspi_1_grp", "qspi_2_grp";
 	      function = "spi";
 	  };
 	  pinctrl_qspi_4: pins-qspi-4 { /* buswidth 4 */
 	      groups = "qspi_1_grp", "qspi_2_grp", "qspi_3_grp";
 	      function = "spi";
 	  };
       };


Thanks,

David