[PATCH v7 4/5] arm64: dts: rockchip: add core dtsi for RK3562 SoC

[Chukun Pan]

Hi,

> <snip>
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3562.dtsi
> @@ -0,0 +1,1187 @@
> <snip>
> +#include <dt-bindings/phy/phy.h>
> +#include <dt-bindings/power/rockchip,rk3562-power.h>
> +#include <dt-bindings/pinctrl/rockchip.h>

Does this need to be sorted alphabetically?

> <snip>
> +		idle-states {
> +			entry-method = "psci";

It would be better to leave a blank line here.

> +			CPU_SLEEP: cpu-sleep {

> <snip>
> +		pwm0: pwm at ff230000 {
> +			compatible = "rockchip,rk3562-pwm", "rockchip,rk3328-pwm";
> +			reg = <0x0 0xff230000 0x0 0x10>;
> +			#pwm-cells = <3>;
> +			pinctrl-names = "active";

It should be `pinctrl-names = "default";` for pwm, see also [1]

> +			pinctrl-0 = <&pwm0m0_pins>;
> +			clocks = <&cru CLK_PMU1_PWM0>, <&cru PCLK_PMU1_PWM0>;
> +			clock-names = "pwm", "pclk";
> +			status = "disabled";
> +		};

The pinctrl should be placed below the clock.

> <snip>
> +			power: power-controller {
> +				compatible = "rockchip,rk3562-power-controller";
> +				#power-domain-cells = <1>;
> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				status = "okay";
> +

`status = "okay"` is not needed here.
Also remove extra blank lines.

> +
> +				power-domain at RK3562_PD_GPU {
> <snip>
> +				power-domain at RK3562_PD_VI {
> +					reg = <RK3562_PD_VI>;
> +					#power-domain-cells = <1>;
> +					#address-cells = <1>;
> +					#size-cells = <0>;
> +					pm_qos = <&qos_isp>,
> +						 <&qos_vicap>;
> +
> +					power-domain at RK3562_PD_VEPU {
> +						reg = <RK3562_PD_VEPU>;
> +						pm_qos = <&qos_vepu>;
> +					#power-domain-cells = <0>;

This line is missing a tab, resulting in an indentation error.

> +					};
> +				};
> <snip>

> +		pcie2x1: pcie at ff500000 {
> +			compatible = "rockchip,rk3562-pcie", "rockchip,rk3568-pcie";
> +			#address-cells = <3>;
> +			#size-cells = <2>;

#address-cells/#size-cells should be placed above `status = "disabled";`
I think other nodes also need to change this. (Some for #pwm-cells)

> +			interrupt-names = "sys", "pmc", "msg", "legacy", "err", "msi";

I noticed that the bsp 5.10 kernel said that pcie only has 8 MSI vectors,
[2][3] but in the bsp 6.1 kernel it changed to 32 MSI vectors [4].

The rockchip documentation also says there are only 8 MSI vectors:

[5] Page37 8.8 "RK3528/RK3562/RK3576可分配的MSI或者MSI-X总数是8个"
Translate into English: "The total number of MSI or MSI-X that
can be allocated by RK3528/RK3562/RK3576 is 8"

We noticed this when supporting rk3528, so which one is correct?

> +			phys = <&combphy_pu PHY_TYPE_PCIE>;

s/combphy_pu/combphy

> <snip>
+		combphy_pu: phy at ff750000 {

Please change it to `combphy` like rk3568.

Heiko I know you just merged this, it's a bit offensive but I think
it would be better to drop these patches and fix them further.
Or kever would you like to send a fix patch? Although most of
them are typo issues, it will take a few patches to fix it.

[1] https://lore.kernel.org/lkml/20250310140916.14384-2-ziyao@disroot.org/
[2] https://github.com/rockchip-linux/kernel/commit/4f0c9ccc79c373aa97084b3b1ab0651ca4248227
[3] https://github.com/rockchip-linux/kernel/commit/afb85c759cfadc4051c42a9703860071a9877f2e
[4] https://github.com/coolpi-george/coolpi-kernel/commit/522b94122ec797760dcd466851250cbdfafff50f
[5] https://github.com/ArmSoM/rk3506-rkr4.2-sdk/blob/main/docs/cn/Common/PCIe/Rockchip_Developer_Guide_PCIe_CN.pdf

Thanks,
Chukun

--
2.25.1