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

[Krzysztof Kozlowski <krzk@kernel.org>]

On 16/12/2025 12:20, Ye Zhang wrote:
> 1. Add header file with constants for RMIO function IDs for the Rockchip
> RK3506 SoC.
> 2. Add device tree binding for the RMIO (Rockchip Matrix I/O) controller
> which is a sub-device of the main pinctrl on some Rockchip SoCs.
> 
> Signed-off-by: Ye Zhang <ye.zhang@rock-chips.com>
> ---
>  .../bindings/pinctrl/rockchip,pinctrl.yaml    |   9 ++
>  .../bindings/pinctrl/rockchip,rmio.yaml       | 106 +++++++++++++++++
>  .../pinctrl/rockchip,rk3506-rmio.h            | 109 ++++++++++++++++++
>  3 files changed, 224 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/pinctrl/rockchip,rmio.yaml
>  create mode 100644 include/dt-bindings/pinctrl/rockchip,rk3506-rmio.h
> 
> diff --git a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> index 97960245676d..9a27eaf7942b 100644
> --- a/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> +++ b/Documentation/devicetree/bindings/pinctrl/rockchip,pinctrl.yaml
> @@ -82,6 +82,15 @@ required:
>    - rockchip,grf
>  
>  patternProperties:
> +  "rmio[0-9]*$":
> +    type: object
> +
> +    $ref: "/schemas/pinctrl/rockchip,rmio.yaml#"
> +
> +    description:
> +      The RMIO (Rockchip Matrix I/O) controller node which functions as a
> +      sub-device of the main pinctrl to handle flexible function routing.

No. Your child has no resources, so it's not proper hardware
representation. Don't use Linux driver model in the bindings.

That's a NAK. Don't send the same AGAIN without addressing comments like
you did here.

> +
>    "gpio@[0-9a-f]+$":
>      type: object
>  
> diff --git a/Documentation/devicetree/bindings/pinctrl/rockchip,rmio.yaml b/Documentation/devicetree/bindings/pinctrl/rockchip,rmio.yaml
> new file mode 100644
> index 000000000000..af0b34512fb9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pinctrl/rockchip,rmio.yaml
> @@ -0,0 +1,106 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/pinctrl/rockchip,rmio.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: RMIO (Rockchip Matrix I/O) Controller
> +
> +maintainers:
> +  - Heiko Stuebner <heiko@sntech.de>
> +
> +description: |
> +  The RMIO controller provides a flexible routing matrix that allows mapping
> +  various internal peripheral functions (UART, SPI, PWM, etc.) to specific
> +  physical pins. This block is typically a sub-block of the GRF
> +  (General Register Files) or PMU (Power Management Unit).
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - rockchip,rk3506-rmio
> +      - const: rockchip,rmio

I don't see how pinctrl deserves generic compatible. I already commented
on this.


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

Why "if"? How this can be flexible?

Anyway, you did not address my previous comment at all.

NAK

> +
> +  rockchip,offset:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      The offset of the RMIO configuration registers within the GRF.

No, this belongs to the phandle.

Look how this is already described and do not come with other style.

> +
> +  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.
> +
> +patternProperties:
> +  "^[a-z0-9-]+$":

No, use a prefix or suffix. See other pinctrl bindings.

> +    type: object
> +    description:
> +      Function node grouping multiple groups.
> +
> +    patternProperties:
> +      "^[a-z0-9-]+$":

Same ocmment

> +        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
> +
> +        required:
> +          - rockchip,rmio

Why aren't you using standard pinctrl bindings?

> +
> +        additionalProperties: false
> +
> +    additionalProperties: false
> +
> +required:
> +  - compatible
> +  - rockchip,rmio-grf
> +  - rockchip,offset
> +  - rockchip,pins-num
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/pinctrl/rockchip,rk3506-rmio.h>
> +
> +    pinctrl: pinctrl {

What's this?

> +        rmio: rmio {
> +            compatible = "rockchip,rk3506-rmio", "rockchip,rmio";
> +            rockchip,rmio-grf = <&grf_pmu>;
> +            rockchip,offset = <0x80>;
> +            rockchip,pins-num = <32>;
> +
> +            rmio-uart {
> +                rmio_pin27_uart1_tx: rmio-pin27-uart1-tx {
> +                    rockchip,rmio = <27 RMIO_UART1_TX>;
> +                };
> +
> +                rmio_pin28_uart1_rx: rmio-pin28-uart1-rx {
> +                    rockchip,rmio = <28 RMIO_UART1_RX>;
> +                };
> +            };
> +        };
> +    };
> diff --git a/include/dt-bindings/pinctrl/rockchip,rk3506-rmio.h b/include/dt-bindings/pinctrl/rockchip,rk3506-rmio.h
> new file mode 100644
> index 000000000000..b129e9a8c287
> --- /dev/null
> +++ b/include/dt-bindings/pinctrl/rockchip,rk3506-rmio.h
> @@ -0,0 +1,109 @@
> +/* SPDX-License-Identifier: (GPL-2.0 OR MIT) */
> +/*
> + * Copyright (c) 2025 Rockchip Electronics Co., Ltd.
> + */
> +
> +#ifndef __DT_BINDINGS_PINCTRL_ROCKCHIP_RK3506_RMIO_H
> +#define __DT_BINDINGS_PINCTRL_ROCKCHIP_RK3506_RMIO_H
> +
> +/* RMIO function definition */
> +#define RMIO_UART1_TX			1
> +#define RMIO_UART1_RX			2
> +#define RMIO_UART2_TX			3
> +#define RMIO_UART2_RX			4
> +#define RMIO_UART3_TX			5
> +#define RMIO_UART3_RX			6
> +#define RMIO_UART3_CTSN			7
> +#define RMIO_UART3_RTSN			8
> +#define RMIO_UART4_TX			9
> +#define RMIO_UART4_RX			10
> +#define RMIO_UART4_CTSN			11
> +#define RMIO_UART4_RTSN			12
> +#define RMIO_MIPITE			13
> +#define RMIO_CLK_32K			14
> +#define RMIO_I2C0_SCL			15
> +#define RMIO_I2C0_SDA			16


I do not see how this is a binding. Please point me to the patch using
this in the driver.

Best regards,
Krzysztof