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

[Ye Zhang]

在 2025/12/27 2:07, Linus Walleij 写道:
> Hi Ye,
>
> thanks for your patch!
>
> On Tue, Dec 16, 2025 at 3:50 PM Ye Zhang <ye.zhang@xxxxxxxxxxxxxx> wrote:
>
>
> > 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.
> > +  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.
> > +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.;
>
> > +            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>;
};

> > +++ 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.