Re: [PATCH v3 6/7] dt-bindings: pinctrl: rockchip: Add RMIO controller binding

在 2025/12/27 2:07, Linus Walleij 写道:

Hi Ye,

thanks for your patch!

On Tue, Dec 16, 2025 at 3:50 PM Ye Zhang [off-list ref] wrote:

quoted

diff --git a/Documentation/devicetree/bindings/pinctrl/rockchip,rmio.yaml b/Documentation/devicetree/bindings/pinctrl/rockchip,rmio.yaml
+  rockchip,rmio-grf:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      The phandle of the syscon node (GRF or PMU) containing the RMIO registers.
+      This property is required if the RMIO registers are located in a different
+      syscon than the parent pinctrl node.
+
+  rockchip,offset:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      The offset of the RMIO configuration registers within the GRF.

Can't this just be a cell in the phandle?

In my upcoming v4, it will be moved into the driver code.

quoted

+  rockchip,pins-num:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      The number of physical pins supported by this RMIO instance.
+      Used for boundary checking and driver initialization.

Isn't this implicit from the compatible? Why is this different
between two device trees using the same compatible pin
controller? I don't get it, I think this should be a constant in the
code based on the compatible instead.

In my upcoming v4, it will be moved into the driver code.

quoted

+patternProperties:
+  "^[a-z0-9-]+$":
+    type: object
+    description:
+      Function node grouping multiple groups.
+
+    patternProperties:
+      "^[a-z0-9-]+$":
+        type: object
+        description:
+          Group node containing the pinmux configuration.
+
+        properties:
+          rockchip,rmio:
+            $ref: /schemas/types.yaml#/definitions/uint32-matrix
+            description:
+              A list of pin-function pairs. The format is <pin_id function_id>.
+            minItems: 1
+            items:
+              items:
+                - description: RMIO Pin ID (0 to pins-num - 1)
+                  minimum: 0
+                  maximum: 31
+                - description: Function ID
+                  minimum: 0
+                  maximum: 98

Please avoid these custom properties and just use the standard
"pinmux" property. I don't want any more opaque custom bindings
for functions and groups.

Reference Documentation/devicetree/bindings/pinctrl/pinmux-node.yaml
and use pinmux from there.

You can use some shifting and defines to shoehorn your config
into a single u32 and parse that in your driver; i.e. instead of
rockchip,rmio = <1, 2>;
use
pinmux = <1 << 8 | 2 << 0>;
these shifter numerals can come from defines.
In the driver shift & mask out the components you want.

e.g.;

quoted

+            rmio-uart {
+                rmio_pin27_uart1_tx: rmio-pin27-uart1-tx {
+                    rockchip,rmio = <27 RMIO_UART1_TX>;

pinmux = <27 << 8 | RMIO_UART1_TX>;

In v4, I will remove rockchip,rmio.yaml

I understand your preference for standard bindings.  However, there is a 
specific constraint here: the RMIO acts as a secondary layer of muxing, 
sitting behind the primary IOMUX controller.

The existing Rockchip pinctrl binding uses the vendor-specific 
rockchip,pins property for the primary IOMUX configuration.  If I were 
to use the standard pinmux property for RMIO, the node would contain 
mixed bindings like this:

node {
     /* Primary IOMUX (existing binding) */
     rockchip,pins = <1 RK_PB1 16 &pcfg_pull_none>;
     /* Secondary RMIO  */
     pinmux = <(RMIO_ID << 16) | (RMIO_PIN << 8) | RMIO_FUNC>;
};

Since this node describes a single hardware pin configuration that 
requires two separate hardware settings (Primary Mux + Secondary RMIO), 
I thought keeping the secondary config as a vendor-specific property 
(rockchip,rmio) alongside rockchip,pins would be more consistent and 
less confusing than mixing legacy custom bindings with standard pinmux.

In v4, I have removed the separate RMIO child node entirely.  The RMIO 
configuration is now integrated into the main pinctrl group as a 
supplemental property:

node {

rockchip,pins = <1 RK_PB1 7 &pcfg_pull_none>

/* rmio_id pin_id func_id */
rockchip,rmio-pins = <0 24 68>;
};

quoted

+++ b/include/dt-bindings/pinctrl/rockchip,rk3506-rmio.h

These number dumps are not appreciated inside the bindings
despite quite a few found their way in there.

Use something like
arch/*/dts/rockchip/rk3506-rmio-pins.dtsi
and include that into your device trees instead.

In my upcoming v4, rockchip,rk3506-rmio.h will be removed.