| 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: <74125120-5d62-41b6-9c3a-904f1565fc95@rock-chips.com>
Date: Thu, 25 Dec 2025 14:44:41 +0800
From: Ye Zhang <ye.zhang@...k-chips.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
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
在 2025/12/16 23:52, Krzysztof Kozlowski 写道:
> 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.
I understand your point now. In v4, I will remove rockchip,rmio. yaml
and drop the separate RMIO child node entirely.
Instead of creating a fake device node, I will use a phandle property
within the main pinctrl node to reference the GRF/PMU syscon that
contains the RMIO registers. For example:
rockchip,rmio-grf = <&rmio_grf>;
The fixed parameters (like offset and pin count) will be moved into the
driver code.
>> +
>> "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.
In v4, I will remove the generic compatible string rockchip,rmio and
rely solely on the specific compatible. Since the separate RMIO node is
being removed (as per the point above), this compatible issue will
naturally be resolved as the logic moves into the main driver.
>
>> +
>> + 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
The description will be corrected to"Phandles to the GRF/PMU syscons
containing the RMIO registers. These are required to access the RMIO
controller registers"
>> +
>> + 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.
In v4, I will remove rockchip,rmio. yaml
>> +
>> + 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.
In v4, I will remove rockchip,rmio. yaml
>> + 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?
The standard pinmux binding is not suitable here because RMIO acts as a
secondary layer of muxing, distinct from the primary IOMUX.
The primary IOMUX (handled by rockchip,pins) routes the pin to the RMIO
block, while the RMIO configuration determines the specific function
within that block.
In v4, I plan to define rockchip,rmio as a supplemental vendor-specific
property within the pin group, formatted as <rmio_id rmio_pin_id
rmio_function>.
For example:
&pinctrl {
rm_io24 {
/omit-if-no-ref/
rm_io24_uart1_tx: rm-io24-uart1-tx {
rockchip,pins =
<1 RK_PB1 16 &pcfg_pull_none>;
rockchip,rmio =
<0 24 RMIO_UART1_TX>;
};
};
>> +
>> + 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?
In v4, I will remove rockchip,rmio. yaml
>> + 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.
These macros are intended to be used in the Device Tree sources to
improve readability, avoiding “magic numbers” for the RMIO function IDs.
For example, instead of writing <0 24 1>, the DTS uses <0 24
RMIO_UART1_TX>, which is much clearer.
in v4, I will provide the relevant dts
> Best regards,
> Krzysztof
>
> _______________________________________________
> Linux-rockchip mailing list
> Linux-rockchip@...ts.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-rockchip
>
Powered by blists - more mailing lists