Re: [RFC PATCH v2 7/8] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] `rknn_core_1` is missing the `aclk_cbuf` and `hclk_cbuf` clocks and their corresponding `clock-names`, which will cause the driver probe to fail unconditionally.
- [High] The NPU core nodes specify only a single power domain each, which causes the driver's explicit power domain attachment to fail with `-EEXIST`.
- [High] The register address for `rknn_mmu_1` (`0x2770a000`) is highly likely a typo for `0x27712000`.
- [Medium] The new RK3576 NPU, IOMMU, and power domain nodes violate multiple device tree binding schemas because the schemas were not properly updated for the RK3576 variant properties.
--

--- Patch [7]: [RFC PATCH v2 7/8] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes ---
commit fcbf6a103f04119fa8ae492b4bcf746be4890ddf
Author: Jiaxing Hu <gahing@gahingwoo.com>

arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes

This commit adds the RK3576 NPU core and MMU nodes, configuring their
register addresses, interrupts, clocks, resets, and power domains.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> index e12a2a0cfb891..5033f762840ae 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3576.dtsi

[ ... ]

> @@ -1070,14 +1070,22 @@ power-domain@RK3576_PD_NPUTOP {
>  						power-domain@RK3576_PD_NPU0 {
>  							reg = <RK3576_PD_NPU0>;
>  							clocks = <&cru HCLK_RKNN_ROOT>,
> -								 <&cru ACLK_RKNN0>;
> +								 <&cru ACLK_RKNN0>,
> +								 <&cru CLK_RKNN_DSU0>,
> +								 <&cru ACLK_RKNN_CBUF>,
> +								 <&cru HCLK_RKNN_CBUF>;
> +							resets = <&cru SRST_A_RKNN0_BIU>;

[Severity: Medium]
Will this fail device tree schema validation?

The resets property appears to be completely undocumented in
rockchip,power-controller.yaml's pd-node.

>  							pm_qos = <&qos_npu_m0>;
>  							#power-domain-cells = <0>;
>  						};

[ ... ]

> @@ -1804,6 +1812,72 @@ qos_npu_m1ro: qos@27f22100 {
>  			reg = <0x0 0x27f22100 0x0 0x20>;
>  		};
>  
> +		rknn_core_0: npu@27700000 {
> +			compatible = "rockchip,rk3576-rknn-core";
> +			reg = <0x0 0x27700000 0x0 0x1000>,
> +			      <0x0 0x27701000 0x0 0x1000>,
> +			      <0x0 0x27703000 0x0 0x1000>,
> +			      <0x0 0x27704000 0x0 0x1000>,
> +			      <0x0 0x27705000 0x0 0x1000>;
> +			reg-names = "pc", "cna", "core", "dpu", "dpu_rdma";
> +			interrupts = <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cru ACLK_RKNN0>, <&cru HCLK_RKNN_ROOT>,
> +				 <&cru CLK_RKNN_DSU0>, <&cru PCLK_NPUTOP_ROOT>,
> +				 <&cru ACLK_RKNN_CBUF>, <&cru HCLK_RKNN_CBUF>;

[Severity: Medium]
Does this node violate the rockchip,rk3588-rknn-core.yaml schema?

The schema strictly limits reg to a maximum of 3 entries and clocks to a
maximum of 4, but there are 5 and 6 here respectively.

> +			clock-names = "aclk", "hclk", "npu", "pclk",
> +				      "aclk_cbuf", "hclk_cbuf";
> +			resets = <&cru SRST_A_RKNN0>;
> +			reset-names = "srst_a";
> +			power-domains = <&power RK3576_PD_NPU0>;

[Severity: High]
Does specifying a single power domain here cause the driver probe to fail?

Because rk3576.dtsi lists only a single power domain for the core node, the
Linux driver core (via genpd_dev_pm_attach) automatically attaches it during
early probe and sets dev->pm_domain.

When the driver's probe runs and calls devm_pm_domain_attach_list(), won't
it see that dev->pm_domain is already set and abort with -EEXIST, rendering
both NPU cores unusable?

> +			iommus = <&rknn_mmu_0>;
> +			status = "disabled";
> +		};
> +
> +		rknn_mmu_0: iommu@27702000 {
> +			compatible = "rockchip,rk3576-iommu", "rockchip,rk3568-iommu";
> +			reg = <0x0 0x27702000 0x0 0x100>,
> +			      <0x0 0x27702100 0x0 0x100>;
> +			interrupts = <GIC_SPI 247 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cru ACLK_RKNN0>, <&cru HCLK_RKNN_ROOT>,
> +				 <&cru CLK_RKNN_DSU0>, <&cru ACLK_RKNN_CBUF>,
> +				 <&cru HCLK_RKNN_CBUF>;

[Severity: Medium]
Will this fail device tree schema validation for rockchip,iommu.yaml?

The schema limits clocks to 2 and requires clock-names, while 5 clocks are
specified here without clock-names.

> +			#iommu-cells = <0>;
> +			power-domains = <&power RK3576_PD_NPU0>;
> +			status = "disabled";
> +		};
> +
> +		rknn_core_1: npu@27710000 {
> +			compatible = "rockchip,rk3576-rknn-core";
> +			reg = <0x0 0x27710000 0x0 0x1000>,
> +			      <0x0 0x27711000 0x0 0x1000>,
> +			      <0x0 0x27713000 0x0 0x1000>,
> +			      <0x0 0x27714000 0x0 0x1000>,
> +			      <0x0 0x27715000 0x0 0x1000>;
> +			reg-names = "pc", "cna", "core", "dpu", "dpu_rdma";
> +			interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
> +			clocks = <&cru ACLK_RKNN1>, <&cru HCLK_RKNN_ROOT>,
> +				 <&cru CLK_RKNN_DSU0>, <&cru PCLK_NPUTOP_ROOT>;
> +			clock-names = "aclk", "hclk", "npu", "pclk";

[Severity: High]
Are the aclk_cbuf and hclk_cbuf clocks missing from this list?

The rocket driver initializes with .num_clks = 6 for the RK3576 variant and
requests all 6 clocks by name using devm_clk_bulk_get().

Since rknn_core_1 only specifies 4 clocks, won't the bulk clock acquisition
fail with -ENOENT during device probe, rendering the second NPU core unusable?

> +			resets = <&cru SRST_A_RKNN1>;
> +			reset-names = "srst_a";
> +			power-domains = <&power RK3576_PD_NPU1>;
> +			iommus = <&rknn_mmu_1>;
> +			status = "disabled";
> +		};
> +
> +		rknn_mmu_1: iommu@2770a000 {
> +			compatible = "rockchip,rk3576-iommu", "rockchip,rk3568-iommu";
> +			reg = <0x0 0x2770a000 0x0 0x100>,
> +			      <0x0 0x2770a100 0x0 0x100>;

[Severity: High]
Is the register address 0x2770a000 a typo for 0x27712000?

Looking at rknn_core_0, it spans 0x27700000 with a gap at 0x27702000 for
its MMU. rknn_core_1 spans 0x27710000 and has an identical gap at
0x27712000.

Placing the MMU at 0x2770a000 could lead to mapping unassigned I/O space
instead of the target IOMMU control registers, causing a synchronous external
abort when accessed.

> +			interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718031146.3368811-1-gahing@gahingwoo.com?part=7