Re: [PATCH 26/27] arm64: dts: rockchip: Add RK3572 SoC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] arm64: dts: rockchip: duplicate pdmm1-clk*-idle node names in rk3572-pinctrl
- [Medium] arm64: dts: rockchip: missing pixel clock pin in rk3572 rgb3x8_pins_m1
--

commit 187a63c766133c9d7856a97e9575a5f3703cf119
Author: Cunhao Lu <1579567540@qq.com>

arm64: dts: rockchip: Add RK3572 SoC

This commit adds the base device tree files for the Rockchip rk3572,
including the operating points, pinctrl definitions, and peripheral nodes.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3572-pinctrl.dtsi b/arch/arm64/boot/dts/rockchip/rk3572-pinctrl.dtsi
> new file mode 100644
> index 0000000000000..8440feea419b9
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3572-pinctrl.dtsi
> @@ -0,0 +1,3935 @@

[ ... ]

> +	pdm {
> +		/omit-if-no-ref/
> +		pdmm0_clk0_idle: pdmm0-clk0-idle {
> +			rockchip,pins =
> +				<0 RK_PC4 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +
> +		/omit-if-no-ref/
> +		pdmm0_clk1_idle: pdmm0-clk1-idle {
> +			rockchip,pins =
> +				<0 RK_PC3 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +
> +		/omit-if-no-ref/
> +		pdmm1_clk0_idle: pdmm1-clk0-idle {
> +			rockchip,pins =
> +				<2 RK_PB2 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +
> +		/omit-if-no-ref/
> +		pdmm1_clk1_idle: pdmm1-clk1-idle {
> +			rockchip,pins =
> +				<2 RK_PB4 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +
> +		/omit-if-no-ref/
> +		pdmm2_clk0_idle: pdmm1-clk0-idle {
> +			rockchip,pins =
> +				<1 RK_PC1 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +
> +		/omit-if-no-ref/
> +		pdmm2_clk1_idle: pdmm1-clk1-idle {
> +			rockchip,pins =
> +				<1 RK_PD5 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +
> +		/omit-if-no-ref/
> +		pdmm3_clk0_idle: pdmm1-clk0-idle {
> +			rockchip,pins =
> +				<2 RK_PC5 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +
> +		/omit-if-no-ref/
> +		pdmm3_clk1_idle: pdmm1-clk1-idle {
> +			rockchip,pins =
> +				<2 RK_PC7 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +
> +		/omit-if-no-ref/
> +		pdmm4_clk0_idle: pdmm1-clk0-idle {
> +			rockchip,pins =
> +				<3 RK_PB1 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +
> +		/omit-if-no-ref/
> +		pdmm4_clk1_idle: pdmm1-clk1-idle {
> +			rockchip,pins =
> +				<3 RK_PA7 RK_FUNC_GPIO &pcfg_pull_none>;
> +		};
> +	};

[Severity: Medium]
Should the node names for pdmm2_clk0_idle, pdmm3_clk0_idle, and
pdmm4_clk0_idle under the pdm node in rk3572-pinctrl.dtsi be
pdmm2-clk0-idle, pdmm3-clk0-idle, and pdmm4-clk0-idle (and likewise for the
clk1-idle nodes), rather than repeating pdmm1-clk0-idle and pdmm1-clk1-idle?

Because these sibling nodes share the same node names under the pdm parent,
dtc merges them into a single pdmm1-clk0-idle node and a single
pdmm1-clk1-idle node, overwriting rockchip,pins with the values from
pdmm4_clk0_idle and pdmm4_clk1_idle when any of the pdmm1, pdmm2, or pdmm3
idle clock pinctrl phandles are referenced.

[ ... ]

> +	vo {

[ ... ]

> +		/omit-if-no-ref/
> +		rgb3x8_pins_m0: rgb3x8-pins-m0 {
> +			rockchip,pins =
> +				<3 RK_PD7 3 &pcfg_pull_none>,
> +				<3 RK_PD0 3 &pcfg_pull_none>,
> +				<3 RK_PC7 3 &pcfg_pull_none>,
> +				<3 RK_PC6 3 &pcfg_pull_none>,
> +				<3 RK_PC5 3 &pcfg_pull_none>,
> +				<3 RK_PC4 3 &pcfg_pull_none>,
> +				<3 RK_PC1 3 &pcfg_pull_none>,
> +				<3 RK_PC0 3 &pcfg_pull_none>,
> +				<3 RK_PB7 3 &pcfg_pull_none>,
> +				<3 RK_PD4 3 &pcfg_pull_none>,
> +				<3 RK_PD5 3 &pcfg_pull_none>,
> +				<3 RK_PD6 3 &pcfg_pull_none>;
> +		};
> +
> +		/omit-if-no-ref/
> +		rgb3x8_pins_m1: rgb3x8-pins-m1 {
> +			rockchip,pins =
> +				<3 RK_PB6 3 &pcfg_pull_none>,
> +				<3 RK_PB5 3 &pcfg_pull_none>,
> +				<3 RK_PB4 3 &pcfg_pull_none>,
> +				<3 RK_PB0 3 &pcfg_pull_none>,
> +				<3 RK_PA7 3 &pcfg_pull_none>,
> +				<3 RK_PA6 3 &pcfg_pull_none>,
> +				<3 RK_PA5 3 &pcfg_pull_none>,
> +				<3 RK_PA4 3 &pcfg_pull_none>,
> +				<3 RK_PD4 3 &pcfg_pull_none>,
> +				<3 RK_PD5 3 &pcfg_pull_none>,
> +				<3 RK_PD6 3 &pcfg_pull_none>;
> +		};

[Severity: Medium]
Is rgb3x8_pins_m1 under the vo node in rk3572-pinctrl.dtsi missing the
<3 RK_PD7 3 &pcfg_pull_none> pixel clock pin?

rgb3x8_pins_m0 and the other display pin groups (vo_lcdc_pins, bt1120_pins,
bt656_pins, rgb565_pins, rgb666_pins, and rgb888_pins) all include this pin
alongside their data and sync pins, whereas rgb3x8_pins_m1 only lists the 8
data pins and 3 sync pins.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-add-board-ok3572-c-v1-0-0b0fd85a31ee@qq.com?part=26