Re: [PATCH 04/12] pinctrl: rockchip: Add RV1106 pinctrl support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `rv1106_pin_banks` array defines `gpio0` and `gpio2` with 32 pins, which exceeds actual hardware capacity and causes overlapping/aliased register accesses.
--

commit fbf0d23655e7fe240a1a8b92b59254646172a530
Author: Simon Glass <sjg@chromium.org>

pinctrl: rockchip: Add RV1106 pinctrl support

This commit adds pinctrl support for the Rockchip RV1106 SoC, ported from
the vendor kernel. It defines the pin controller banks and offsets for
drive, pull, and schmitt registers, and adds matching compatible strings.

Link: https://github.com/LuckfoxTECH/luckfox-pico

> diff --git a/drivers/pinctrl/pinctrl-rockchip.c b/drivers/pinctrl/pinctrl-rockchip.c
> index 7e0fcd45fd265..f9cbcb955853c 100644
> --- a/drivers/pinctrl/pinctrl-rockchip.c
> +++ b/drivers/pinctrl/pinctrl-rockchip.c

[ ... ]

> @@ -4623,6 +4787,48 @@ static struct rockchip_pin_ctrl rv1103b_pin_ctrl __maybe_unused = {
>  	.schmitt_calc_reg	= rv1103b_calc_schmitt_reg_and_bit,
>  };
>  
> +static struct rockchip_pin_bank rv1106_pin_banks[] = {
> +	PIN_BANK_IOMUX_FLAGS(0, 32, "gpio0",
> +			     IOMUX_WIDTH_4BIT | IOMUX_SOURCE_PMU,
> +			     IOMUX_WIDTH_4BIT | IOMUX_SOURCE_PMU,
> +			     IOMUX_WIDTH_4BIT | IOMUX_SOURCE_PMU,
> +			     IOMUX_WIDTH_4BIT | IOMUX_SOURCE_PMU),

[Severity: High]
Does setting nr_pins to 32 for gpio0 cause register overlap?

If gpio0 has 32 pins, configuring drive strength for pins >= 20 would require
16 registers (offsets 0x10 to 0x4F, since there are 2 pins per register).

This range directly overlaps with the bank's PULL (0x38) and SMT (0x40)
offsets. If a device tree requests these higher pins, would it silently
overwrite the pull and schmitt configuration registers?

> +	PIN_BANK_IOMUX_FLAGS_OFFSET(1, 32, "gpio1",
> +				    IOMUX_WIDTH_4BIT,
> +				    IOMUX_WIDTH_4BIT,
> +				    IOMUX_WIDTH_4BIT,
> +				    IOMUX_WIDTH_4BIT,
> +				    0, 0x08, 0x10, 0x18),
> +	PIN_BANK_IOMUX_FLAGS_OFFSET(2, 32, "gpio2",
> +				    IOMUX_WIDTH_4BIT,
> +				    IOMUX_WIDTH_4BIT,
> +				    IOMUX_WIDTH_4BIT,
> +				    IOMUX_WIDTH_4BIT,
> +				    0x10020, 0x10028, 0, 0),

[Severity: High]
Could this IOMUX offset configuration cause unintended register aliasing?

The gpio2 bank is defined with 32 pins, but offsets for pins 16-31 are
explicitly set to 0.

If a device tree configures multiplexing for these upper pins, wouldn't it
read an offset of 0x0 and write to regmap_base offset 0, which is the IOMUX
register for gpio1? Could we reduce the pin count for this bank to match
the actual hardware capacity and prevent this?

> +	PIN_BANK_IOMUX_FLAGS_OFFSET(3, 32, "gpio3",
> +				    IOMUX_WIDTH_4BIT,
> +				    IOMUX_WIDTH_4BIT,

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260706195818.3906949-1-sjg@chromium.org?part=4