| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <5154ca76-0f23-4b52-8e6d-07005c52ac6d@kernel.org>
Date: Tue, 16 Dec 2025 16:52:08 +0100
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Ye Zhang <ye.zhang@...k-chips.com>,
Linus Walleij <linus.walleij@...aro.org>, Heiko Stuebner <heiko@...ech.de>
Cc: Bartosz Golaszewski <brgl@...ev.pl>, Rob Herring <robh@...nel.org>,
Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
<conor+dt@...nel.org>, linux-gpio@...r.kernel.org,
devicetree@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-rockchip@...ts.infradead.org, linux-kernel@...r.kernel.org,
tao.huang@...k-chips.com
Subject: Re: [PATCH v3 6/7] dt-bindings: pinctrl: rockchip: Add RMIO
controller binding
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@...k-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@...ech.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
Powered by blists - more mailing lists