Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] Missing `aclk_cbuf` and `hclk_cbuf` clocks in `rknn_core_1` node. - [High] Single power-domain definition for NPU cores causes probe failure (`-EEXIST`) and violates hardware constraints. - [High] The physical register address for `rknn_mmu_1` is incorrectly mapped to `0x2770a000` instead of `0x27712000`. --
--- Patch [8]: [RFC PATCH 8/9] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes --- commit 5396e10743f01d595779ccc7127139fe5f89e996 Author: Jiaxing Hu <[email protected]> arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes Add the RK3576 NPU: rknn_core_0/1 and rknn_mmu_0/1, with register addresses, GIC_SPI 247/248 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 [ ... ] > @@ -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>; > + 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 listing a single power domain here cause probe to fail? The rocket_core_init driver expects both RK3576_PD_NPU0 and RK3576_PD_NPU1 on rknn_core_0 to satisfy a hardware constraint where the CBUF->CMAC read path requires both. Because the DT only lists one power domain per core, the kernel platform bus auto-attaches it and sets dev->pm_domain. When the driver then calls devm_pm_domain_attach_list(), it will return -EEXIST, which the driver treats as a fatal error and aborts probe. Missing RK3576_PD_NPU1 on rknn_core_0 also breaks runtime PM, leading to AXI lockups. > + iommus = <&rknn_mmu_0>; > + 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] Will the driver fail to probe here due to missing CBUF clocks? The rocket_core_init function explicitly requests 6 clocks (aclk, hclk, npu, pclk, aclk_cbuf, hclk_cbuf) using devm_clk_bulk_get(). Since rknn_core_1 only defines 4 clocks, the API will fail with -ENOENT, making 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 this physical register address mapped correctly? It looks like the base address is misplaced by 0x8000 relative to where it belongs inside the rknn_core_1 memory footprint. The rknn_core_1 registers span 0x27710000-0x27715000, with a 1-page gap at 0x27712000 for the IOMMU (which is structurally symmetric to rknn_core_0 having its IOMMU at 0x27702000). When the IOMMU driver attempts to access 0x2770a000 instead of 0x27712000, it will likely cause a Synchronous External Abort or fail to translate memory for the NPU. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=8
