Re: [PATCH v2 4/6] ARM: dts: rockchip: Add support for RV1106 and RV1103

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The DT defines distributed IOC blocks for each GPIO bank via `rockchip,grf` properties, which violates the upstream dt-binding schema and causes register corruption due to driver mismatch.
- [Medium] The device tree relies on the 'rockchip,rv1106-pinctrl' compatible string, which is entirely unsupported by the upstream pinctrl-rockchip driver and violates the dt-binding schema.
--

--- Patch [4]: [PATCH v2 4/6] ARM: dts: rockchip: Add support for RV1106 and RV1103 ---
commit e0d1bbbfe5826e8f7c17c9a0008a5c218538f589
Author: Simon Glass <sjg@chromium.org>

ARM: dts: rockchip: Add support for RV1106 and RV1103

Add the devicetree files for the Rockchip RV1106 and its RV1103 package
variant. The pinctrl data comes from the vendor kernel in the Luckfox Pico
SDK.

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

> diff --git a/arch/arm/boot/dts/rockchip/rv1106.dtsi b/arch/arm/boot/dts/rockchip/rv1106.dtsi
> new file mode 100644
> index 0000000000000..7b2e6142a103f
> --- /dev/null
> +++ b/arch/arm/boot/dts/rockchip/rv1106.dtsi
> @@ -0,0 +1,321 @@
[ ... ]
> +	pinctrl: pinctrl {
> +		compatible = "rockchip,rv1106-pinctrl";

[Severity: Medium]
Does this compatible string have accompanying dt-binding schema and driver
support? The 'rockchip,rv1106-pinctrl' string appears to be missing from the
upstream device tree schema and pinctrl driver, which could cause dtbs_check
validation failures and prevent the driver from probing.

> +		rockchip,grf = <&gpio1_ioc>;
> +		#address-cells = <1>;
> +		#size-cells = <1>;
> +		ranges;
> +
> +		gpio0: gpio@ff380000 {
> +			compatible = "rockchip,gpio-bank";
> +			reg = <0xff380000 0x100>;
> +			interrupts = <GIC_SPI 5 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cru PCLK_PMU_GPIO0>, <&cru DBCLK_PMU_GPIO0>;
> +			gpio-controller;
> +			#gpio-cells = <2>;
> +			gpio-ranges = <&pinctrl 0 0 32>;
> +			interrupt-controller;
> +			#interrupt-cells = <2>;
> +			rockchip,grf = <&gpio0_ioc>;

[Severity: High]
Will the upstream pinctrl-rockchip driver parse these bank-specific syscons
properly? The upstream driver typically only parses the parent pinctrl node's
rockchip,grf property.

Could this cause register corruption when configuring pins? Since the parent
pinctrl node points to gpio1_ioc, the driver might use gpio1_ioc for all
banks. This would silently overwrite gpio1_ioc registers when attempting to
configure pins for other banks like gpio0 or gpio2.

Also, does adding the rockchip,grf property to the gpio-bank child nodes
violate the rockchip,gpio-bank.yaml schema which forbids additional
properties?

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