Hi Rob,

On Tue, 2021-12-14 at 10:38 -0600, Rob Herring wrote:

On Sun, Dec 12, 2021 at 04:40:13AM +0800, Tinghan Shen wrote:

quoted

The mt8195 pinctrl driver requires a 'pins' subnode to wrap pin
properties.

Why?

It's because the dtbs_check reports following errors,

/proj/mtk15399/cros/src/third_party/kernel/v5.10/arch/arm64/boot/dts/me
diatek/mt8195-evb.dt.yaml: pinctrl@10005000: i2c0-pins: 'pinmux' is a
required property
        From schema:
/proj/mtk15399/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/pinctrl/pinctrl-mt8195.yaml
/proj/mtk15399/cros/src/third_party/kernel/v5.10/arch/arm64/boot/dts/me
diatek/mt8195-evb.dt.yaml: pinctrl@10005000: i2c0-pins: 'pins_bus' does
not match any of the regexes: 'pinctrl-[0-9]+'
        From schema:
/proj/mtk15399/cros/src/third_party/kernel/v5.10/Documentation/devicetr
ee/bindings/pinctrl/pinctrl-mt8195.yaml

So, I've to add 'pins' to fix this failure.

quoted

Signed-off-by: Tinghan Shen <redacted>
---
 .../bindings/pinctrl/pinctrl-mt8195.yaml      | 307 +++++++++-----
----
 1 file changed, 160 insertions(+), 147 deletions(-)

diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-

mt8195.yaml b/Documentation/devicetree/bindings/pinctrl/pinctrl-
mt8195.yaml
index 5e2bb2bf3a55..2035bf164740 100644

--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8195.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-mt8195.yaml

@@ -59,133 +59,143 @@ properties:
 patternProperties:
   '-pins$':
     type: object
-    description: |
-      A pinctrl node should contain at least one subnodes

representing the
-      pinctrl groups available on the machine. Each subnode will
list the
-      pins it needs, and how they should be configured, with
regard to muxer
-      configuration, pullups, drive strength, input enable/disable
and
-      input schmitt.
-      An example of using macro:
-      pincontroller {
-        /* GPIO0 set as multifunction GPIO0 */
-        gpio_pin {
-          pinmux = <PINMUX_GPIO0__FUNC_GPIO0>;
-        };
-        /* GPIO8 set as multifunction SDA0 */
-        i2c0_pin {
-          pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
-        };
-      };
-    $ref: "pinmux-node.yaml"
-
-    properties:
-      pinmux:
-        description: |
-          Integer array, represents gpio pin number and mux
setting.
-          Supported pin number and mux varies for different SoCs,
and are defined
-          as macros in dt-bindings/pinctrl/<soc>-pinfunc.h
directly.
-
-      drive-strength:
-        enum: [2, 4, 6, 8, 10, 12, 14, 16]
-
-      bias-pull-down:
-        description: |
-          For pull down type is normal, it don't need add RSEL &
R1R0 define
-          and resistance value.
-          For pull down type is PUPD/R0/R1 type, it can add R1R0
define to
-          set different resistance. It can support
"MTK_PUPD_SET_R1R0_00" &
-          "MTK_PUPD_SET_R1R0_01" & "MTK_PUPD_SET_R1R0_10" &
"MTK_PUPD_SET_R1R0_11"
-          define in mt8195.
-          For pull down type is RSEL, it can add RSEL define &
resistance value(ohm)
-          to set different resistance by identifying property
"mediatek,rsel_resistance_in_si_unit".
-          It can support "MTK_PULL_SET_RSEL_000" &
"MTK_PULL_SET_RSEL_001"
-          & "MTK_PULL_SET_RSEL_010" & "MTK_PULL_SET_RSEL_011" &
"MTK_PULL_SET_RSEL_100"
-          & "MTK_PULL_SET_RSEL_101" & "MTK_PULL_SET_RSEL_110" &
"MTK_PULL_SET_RSEL_111"
-          define in mt8195. It can also support resistance
value(ohm) "75000" & "5000" in mt8195.
-          oneOf:
-            - enum: [100, 101, 102, 103]
-            - description: mt8195 pull down PUPD/R0/R1 type define
value.
-            - enum: [200, 201, 202, 203, 204, 205, 206, 207]
-            - description: mt8195 pull down RSEL type define
value.
-            - enum: [75000, 5000]
-            - description: mt8195 pull down RSEL type si unit
value(ohm).
-
-          An example of using RSEL define:
-          pincontroller {
-            i2c0_pin {
-              pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
-              bias-pull-down = <MTK_PULL_SET_RSEL_001>;
-            };
-          };
-          An example of using si unit resistance value(ohm):
-          &pio {
-            mediatek,rsel_resistance_in_si_unit;
-          }
-          pincontroller {
-            i2c0_pin {
-              pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
-              bias-pull-down = <75000>;
-            };
-          };
-
-      bias-pull-up:
+    additionalProperties: false
+    patternProperties:
+      'pins':

Can we constrain this a bit more.

After discussed with colleagues working on pinctrl, 
'^pins' is fine for them. is this acceptable?

quoted

+        type: object
+        additionalProperties: false
         description: |
-          For pull up type is normal, it don't need add RSEL &
R1R0 define
-          and resistance value.
-          For pull up type is PUPD/R0/R1 type, it can add R1R0
define to
-          set different resistance. It can support
"MTK_PUPD_SET_R1R0_00" &
-          "MTK_PUPD_SET_R1R0_01" & "MTK_PUPD_SET_R1R0_10" &
"MTK_PUPD_SET_R1R0_11"
-          define in mt8195.
-          For pull up type is RSEL, it can add RSEL define &
resistance value(ohm)
-          to set different resistance by identifying property
"mediatek,rsel_resistance_in_si_unit".
-          It can support "MTK_PULL_SET_RSEL_000" &
"MTK_PULL_SET_RSEL_001"
-          & "MTK_PULL_SET_RSEL_010" & "MTK_PULL_SET_RSEL_011" &
"MTK_PULL_SET_RSEL_100"
-          & "MTK_PULL_SET_RSEL_101" & "MTK_PULL_SET_RSEL_110" &
"MTK_PULL_SET_RSEL_111"
-          define in mt8195. It can also support resistance
value(ohm)
-          "1000" & "1500" & "2000" & "3000" & "4000" & "5000" &
"10000" & "75000" in mt8195.
-          oneOf:
-            - enum: [100, 101, 102, 103]
-            - description: mt8195 pull up PUPD/R0/R1 type define
value.
-            - enum: [200, 201, 202, 203, 204, 205, 206, 207]
-            - description: mt8195 pull up RSEL type define value.
-            - enum: [1000, 1500, 2000, 3000, 4000, 5000, 10000,
75000]
-            - description: mt8195 pull up RSEL type si unit
value(ohm).
-          An example of using RSEL define:
+          A pinctrl node should contain at least one subnodes
representing the
+          pinctrl groups available on the machine. Each subnode
will list the
+          pins it needs, and how they should be configured, with
regard to muxer
+          configuration, pullups, drive strength, input
enable/disable and
+          input schmitt.
+          An example of using macro:
           pincontroller {
-            i2c0_pin {
-              pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
-              bias-pull-up = <MTK_PULL_SET_RSEL_001>;
+            /* GPIO0 set as multifunction GPIO0 */
+            gpio_pin {
+              pinmux = <PINMUX_GPIO0__FUNC_GPIO0>;
             };
-          };
-          An example of using si unit resistance value(ohm):
-          &pio {
-            mediatek,rsel_resistance_in_si_unit;
-          }
-          pincontroller {
+            /* GPIO8 set as multifunction SDA0 */
             i2c0_pin {
               pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
-              bias-pull-up = <1000>;
             };
           };
-
-      bias-disable: true
-
-      output-high: true
-
-      output-low: true
-
-      input-enable: true
-
-      input-disable: true
-
-      input-schmitt-enable: true
-
-      input-schmitt-disable: true
-
-    required:
-      - pinmux
-
-    additionalProperties: false
+        $ref: "pinmux-node.yaml"
+
+        properties:
+          pinmux:
+            description: |
+              Integer array, represents gpio pin number and mux
setting.
+              Supported pin number and mux varies for different
SoCs, and are
+              defined as macros in dt-bindings/pinctrl/<soc>-
pinfunc.h
+              directly.
+
+          drive-strength:
+            enum: [2, 4, 6, 8, 10, 12, 14, 16]
+
+          bias-pull-down:
+            description: |
+              For pull down type is normal, it don't need add RSEL
& R1R0 define
+              and resistance value.
+              For pull down type is PUPD/R0/R1 type, it can add
R1R0 define to
+              set different resistance. It can support
"MTK_PUPD_SET_R1R0_00" &
+              "MTK_PUPD_SET_R1R0_01" & "MTK_PUPD_SET_R1R0_10" &
+              "MTK_PUPD_SET_R1R0_11" define in mt8195.
+              For pull down type is RSEL, it can add RSEL define &
resistance
+              value(ohm) to set different resistance by
identifying property
+              "mediatek,rsel_resistance_in_si_unit".
+              It can support "MTK_PULL_SET_RSEL_000" &
"MTK_PULL_SET_RSEL_001"
+              & "MTK_PULL_SET_RSEL_010" & "MTK_PULL_SET_RSEL_011"
+              & "MTK_PULL_SET_RSEL_100" & "MTK_PULL_SET_RSEL_101"
+              & "MTK_PULL_SET_RSEL_110" & "MTK_PULL_SET_RSEL_111"
+              define in mt8195. It can also support resistance
value(ohm)
+              "75000" & "5000" in mt8195.
+              oneOf:
+                - enum: [100, 101, 102, 103]
+                - description: mt8195 pull down PUPD/R0/R1 type
define value.
+                - enum: [200, 201, 202, 203, 204, 205, 206, 207]
+                - description: mt8195 pull down RSEL type define
value.
+                - enum: [75000, 5000]
+                - description: mt8195 pull down RSEL type si unit
value(ohm).
+
+              An example of using RSEL define:
+              pincontroller {
+                i2c0_pin {
+                  pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
+                  bias-pull-down = <MTK_PULL_SET_RSEL_001>;

Isn't this now out of date?

quoted

+                };
+              };
+              An example of using si unit resistance value(ohm):
+              &pio {
+                mediatek,rsel_resistance_in_si_unit;
+              }
+              pincontroller {
+                i2c0_pin {
+                  pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
+                  bias-pull-down = <75000>;
+                };
+              };
+
+          bias-pull-up:
+            description: |
+              For pull up type is normal, it don't need add RSEL &
R1R0 define
+              and resistance value.
+              For pull up type is PUPD/R0/R1 type, it can add R1R0
define to
+              set different resistance. It can support
"MTK_PUPD_SET_R1R0_00" &
+              "MTK_PUPD_SET_R1R0_01" & "MTK_PUPD_SET_R1R0_10" &
+              "MTK_PUPD_SET_R1R0_11" define in mt8195.
+              For pull up type is RSEL, it can add RSEL define &
resistance
+              value(ohm) to set different resistance by
identifying property
+              "mediatek,rsel_resistance_in_si_unit".
+              It can support "MTK_PULL_SET_RSEL_000" &
"MTK_PULL_SET_RSEL_001"
+              & "MTK_PULL_SET_RSEL_010" & "MTK_PULL_SET_RSEL_011"
+              & "MTK_PULL_SET_RSEL_100" & "MTK_PULL_SET_RSEL_101"
+              & "MTK_PULL_SET_RSEL_110" & "MTK_PULL_SET_RSEL_111"
+              define in mt8195. It can also support resistance
value(ohm)
+              "1000" & "1500" & "2000" & "3000" & "4000" & "5000"
& "10000" &
+              "75000" in mt8195.
+              oneOf:
+                - enum: [100, 101, 102, 103]
+                - description: mt8195 pull up PUPD/R0/R1 type
define value.
+                - enum: [200, 201, 202, 203, 204, 205, 206, 207]
+                - description: mt8195 pull up RSEL type define
value.
+                - enum: [1000, 1500, 2000, 3000, 4000, 5000,
10000, 75000]
+                - description: mt8195 pull up RSEL type si unit
value(ohm).
+              An example of using RSEL define:
+              pincontroller {
+                i2c0_pin {
+                  pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
+                  bias-pull-up = <MTK_PULL_SET_RSEL_001>;
+                };
+              };
+              An example of using si unit resistance value(ohm):
+              &pio {
+                mediatek,rsel_resistance_in_si_unit;
+              }
+              pincontroller {
+                i2c0_pin {
+                  pinmux = <PINMUX_GPIO8__FUNC_SDA0>;
+                  bias-pull-up = <1000>;
+                };
+              };
+
+          bias-disable: true
+
+          output-high: true
+
+          output-low: true
+
+          input-enable: true
+
+          input-disable: true
+
+          input-schmitt-enable: true
+
+          input-schmitt-disable: true
+
+        required:
+          - pinmux
 
 required:
   - compatible

@@ -201,30 +211,33 @@ additionalProperties: false
 
 examples:
   - |
-            #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
-            #include <dt-bindings/interrupt-controller/arm-gic.h>
-            pio: pinctrl@10005000 {
-                    compatible = "mediatek,mt8195-pinctrl";
-                    reg = <0x10005000 0x1000>,
-                          <0x11d10000 0x1000>,
-                          <0x11d30000 0x1000>,
-                          <0x11d40000 0x1000>,
-                          <0x11e20000 0x1000>,
-                          <0x11eb0000 0x1000>,
-                          <0x11f40000 0x1000>,
-                          <0x1000b000 0x1000>;
-                    reg-names = "iocfg0", "iocfg_bm", "iocfg_bl",
-                          "iocfg_br", "iocfg_lm", "iocfg_rb",
-                          "iocfg_tl", "eint";
-                    gpio-controller;
-                    #gpio-cells = <2>;
-                    gpio-ranges = <&pio 0 0 144>;
-                    interrupt-controller;
-                    interrupts = <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH

0>;
-                    #interrupt-cells = <2>;
-
-                    pio-pins {
-                      pinmux = <PINMUX_GPIO0__FUNC_GPIO0>;
-                      output-low;
-                    };
-            };
+    #include <dt-bindings/pinctrl/mt8195-pinfunc.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #
+    pio: pinctrl@10005000 {
+      compatible = "mediatek,mt8195-pinctrl";
+      reg = <0x10005000 0x1000>,
+            <0x11d10000 0x1000>,
+            <0x11d30000 0x1000>,
+            <0x11d40000 0x1000>,
+            <0x11e20000 0x1000>,
+            <0x11eb0000 0x1000>,
+            <0x11f40000 0x1000>,
+            <0x1000b000 0x1000>;
+      reg-names = "iocfg0", "iocfg_bm", "iocfg_bl",
+                  "iocfg_br", "iocfg_lm", "iocfg_rb",
+                  "iocfg_tl", "eint";
+      gpio-controller;
+      #gpio-cells = <2>;
+      gpio-ranges = <&pio 0 0 144>;
+      interrupt-controller;
+      interrupts = <GIC_SPI 225 IRQ_TYPE_LEVEL_HIGH 0>;
+      #interrupt-cells = <2>;
+
+      pio-pins {
+        pins_bus {

Don't use '_' in node names.

quoted

+          pinmux = <PINMUX_GPIO0__FUNC_GPIO0>;
+          output-low;
+        };
+      };
+    };
-- 
2.18.0

I'll update the examples and node names at next version.
Thank you!

Best regards,
TingHan