Align fsl,pins with dtschema

[Alexander Stein]

Am Dienstag, 26. Juli 2022, 11:35:52 CEST schrieb Krzysztof Kozlowski:
> On 23/07/2022 14:15, Peng Fan wrote:
> > All,
> > 
> > I am thinking whether we need to modify the fsl,pins as below. Because
> > fsl,pins use uint32-matrix, so just wanna know we should do this update.
> > 
> > Anyway there is NO dtbs check warning. If we update fsl,pins, there are
> > bunches of dts files need update. Do you think we need to update or not?
> > 
> >          pinctrl_ov5640: ov5640grp {
> > 
> > -               fsl,pins = <
> > -                       MX8MM_IOMUXC_GPIO1_IO07_GPIO1_IO7              
> > 0x19 -                       MX8MM_IOMUXC_GPIO1_IO06_GPIO1_IO6           
> >    0x19 -                      
> > MX8MM_IOMUXC_GPIO1_IO14_CCMSRCGPCMIX_CLKO1      0x59 -               >;
> > +               fsl,pins =
> > +                       <MX8MM_IOMUXC_GPIO1_IO07_GPIO1_IO7
> > 0x19>,
> > +                       <MX8MM_IOMUXC_GPIO1_IO06_GPIO1_IO6
> > 0x19>,
> > +                       <MX8MM_IOMUXC_GPIO1_IO14_CCMSRCGPCMIX_CLKO1
> > 0x59>;
> > 
> >          };
> 
> I believe yes, but as Rob replied, the dtschema has trouble matching the
> different fsl,pins, if I understood correctly.

I remember getting a lot of warnings, most probably using old dt-schema before 
[1] and [2].
[1] only mentions problems detecting matrix "where neither dimension is 
fixed".
Having that said, adding a small uint32-matrix example to dt-schema tests 
raises errors when used incorrectly. Take the following diff:
---8<---
diff --git a/test/device.dts b/test/device.dts
index 47aa31c..a0988c6 100644
--- a/test/device.dts
+++ b/test/device.dts
@@ -51,5 +51,6 @@
                vendor,int-array-prop = <5>, <6>, <7>, <8>;
                vendor,int-array-size-only-prop = <2>, <3>, <4>;
                vendor,int64-array-prop = /bits/ 64 <0x10000000 0x1>;
+               vendor,uint32-matrix-prop = <1 2 3>, <4 5 6>, <7>;
        };
 };
\ No newline at end of file
diff --git a/test/schemas/good-example.yaml b/test/schemas/good-example.yaml
index b8493a9..dc72046 100644
--- a/test/schemas/good-example.yaml
+++ b/test/schemas/good-example.yaml
@@ -177,6 +177,17 @@ properties:
           - description: the 2nd cell data
     description: Vendor specific array of phandles property
 
+  vendor,uint32-matrix-prop:
+    $ref: /schemas/types.yaml#/definitions/uint32-matrix
+    description: Vendor specific 32-bit integer matrix property
+    items:
+      items:
+        - description: |
+            1st item
+        - description: |
+            2nd item
+        - description: |
+            3rd item
 
 required:
   - compatible
---8<---

test-dt-validate.py does detect this misuse:
> Failed validating 'minItems' in schema['properties']['vendor,uint32-matrix-
prop']['items']:
>     {'maxItems': 3, 'minItems': 3, 'type': 'array'}
> 
> On instance['vendor,uint32-matrix-prop'][2]:
>     [7]
This seems pretty good to me.

But apparently this is not true for fsl,pins in board .dts. Given the 
following diff, converting to an actual matrix and adding a excessive number 
(Note the 5 at the end), this raises no error. So I suspect this is treated as 
an array rather than a matrix.

---8<---
--- a/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
+++ b/arch/arm64/boot/dts/freescale/imx8mp-evk.dts
@@ -505,10 +505,9 @@ MX8MP_IOMUXC_SD2_RESET_B__GPIO2_IO19       0x40
        };
 
        pinctrl_uart2: uart2grp {
-               fsl,pins = <
-                       MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX    0x140
-                       MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX    0x140
-               >;
+               fsl,pins =
+                       <MX8MP_IOMUXC_UART2_RXD__UART2_DCE_RX   0x140>,
+                       <MX8MP_IOMUXC_UART2_TXD__UART2_DCE_TX   0x140 5>;
        };
 
        pinctrl_usb1_vbus: usb1grp {
---8<---

This is how I tested:
$ git describe
v5.19-rc8
$ dt-validate --version
2022.8.dev24+g59f2e31
$ make dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/pinctrl/
fsl,imx8mp-pinctrl.yaml
make[1]: Entering directory '/mnt/ssd/repo/linux/build_arm64'
  DTC     arch/arm64/boot/dts/freescale/imx8mp-evk.dtb
  CHECK   arch/arm64/boot/dts/freescale/imx8mp-evk.dtb
make[1]: Leaving directory '/mnt/ssd/repo/linux/build_arm64'

Best regards,
Alexander

[1] https://lore.kernel.org/all/20220310160513.1708182-3-robh@kernel.org/
[2] https://github.com/devicetree-org/dt-schema/commit/
5646cd473017d702844fda382c7b88994e080ee8