[RFC PATCH v2 0/8] accel/rocket: RK3576 NPU (RKNN) enablement

[RFC PATCH v2 0/8] accel/rocket: RK3576 NPU (RKNN) enablement

[Jiaxing Hu]

This adds RK3576 NPU support to accel/rocket.  RFC: the NPU brings up and
runs, but full inference is not correct yet, and I would rather get
feedback now than sit on it.

Tested on a Radxa ROCK 4D.  The NPU probes, powers on, brings up its
IOMMUs, and runs jobs to completion (rocket + Mesa Teflon).  Getting there
took a few fixes outside the driver too: a power-domain settle delay and
reset cycling (else a cold power-on throws an async SError), the full
clock set on the MMU nodes, clearing a stale firmware page fault, and both
NPU power domains on the core.

Not solved: only the first operation in a power session gives the right
answer.  Every later one engages and DMAs its input, but the compute
output never lands, so the readback is the zero-point.  I chased it
through the whole software surface (registers, IOMMU/TLB, cache,
power/clock/reset, dispatch) and it reproduces even with the driver
environment matched to the vendor's on the same kernel, so it looks to be
below software, in the fixed-function block.  Traces available.

Two asks: review of the bring-up and the shared pmdomain/iommu changes;
and if anyone knows the RK3576 NPU internals, whether that
per-power-session arm can be driven from software at all.

The series is split by subsystem (binding, pmdomain, iommu, driver, DT).
The board DT keeps vdd_npu_s0 always-on for now; happy to switch to
proper runtime control if preferred.

Changes in v2 (thanks to Chaoyi Chen and the Sashiko review bot):
  - dt-bindings: keep sram-supply required for rk3588 via allOf/if-then
    instead of dropping it from the shared required list
  - pmdomain: release the resets in rockchip_pm_remove_one_domain()
  - iommu: drop the redundant "err == 0" check in the clock probe
  - iommu: replace the two "skip orphaned-fault banks" patches with one
    that clears the stale firmware page fault (CMD_PAGE_FAULT_DONE) before
    enabling stall, so no bank needs special-casing -- Chaoyi asked
    whether dealing with the fault avoids the skip, and it does
  - accel/rocket: gate every RK3576-specific difference (extra clocks,
    single reset, multi power-domain attach, polled completion) behind
    of_device_id match data so the RK3588 path is byte-for-byte unchanged;
    drop the unused DPU register mapping (the driver did no pre-arm)
  - series is now 8 patches (was 9)

I used Claude Opus 4.8 to help trim this series out of my debugging tree
and generate the diffs.

Jiaxing Hu (8):
  dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core
  pmdomain/rockchip: add optional per-domain power-on settle delay
  pmdomain/rockchip: cycle optional power-domain resets on power-on
  iommu/rockchip: take all DT clocks
  iommu/rockchip: clear stale page faults before enabling stall
  accel/rocket: add RK3576 NPU (RKNN) support
  arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes
  arm64: dts: rockchip: rk3576-rock-4d: enable NPU

 .../npu/rockchip,rk3588-rknn-core.yaml        |  12 +-
 .../boot/dts/rockchip/rk3576-rock-4d.dts      |  16 +++
 arch/arm64/boot/dts/rockchip/rk3576.dtsi      |  78 ++++++++++-
 drivers/accel/rocket/rocket_core.c            |  39 +++++-
 drivers/accel/rocket/rocket_core.h            |  22 ++-
 drivers/accel/rocket/rocket_device.c          |   4 +
 drivers/accel/rocket/rocket_drv.c             |  22 ++-
 drivers/accel/rocket/rocket_job.c             | 127 ++++++++++++++++--
 drivers/iommu/rockchip-iommu.c                |  41 +++---
 drivers/pmdomain/rockchip/pm-domains.c        |  69 ++++++----
 10 files changed, 371 insertions(+), 59 deletions(-)

-- 
2.43.0

[RFC PATCH v2 1/8] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core

[Jiaxing Hu]

Document the RK3576 NPU core: add rockchip,rk3576-rknn-core to the
compatible enum.  The RK3576 has no NPU SRAM supply, so make sram-supply
required only for rockchip,rk3588-rknn-core via an allOf/if-then instead
of dropping it from the shared required list (which would have weakened
validation for RK3588).

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 .../bindings/npu/rockchip,rk3588-rknn-core.yaml      | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
index caca2a490..985cde6b2 100644
--- a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
+++ b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
@@ -21,6 +21,7 @@ properties:
 
   compatible:
     enum:
+      - rockchip,rk3576-rknn-core
       - rockchip,rk3588-rknn-core
 
   reg:
@@ -75,7 +76,16 @@ required:
   - resets
   - reset-names
   - npu-supply
-  - sram-supply
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: rockchip,rk3588-rknn-core
+    then:
+      required:
+        - sram-supply
 
 additionalProperties: false
 
-- 
2.43.0

Re: [RFC PATCH v2 1/8] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The schema allows `sram-supply` to be specified for RK3576 even though the hardware does not support it.
--

--- Patch [1]: [RFC PATCH v2 1/8] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core ---
commit af7a16605153304ec36efac86040bd1175195ecd
Author: Jiaxing Hu <gahing@gahingwoo.com>

dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core

This commit documents the RK3576 NPU core by adding it to the compatible
enum. It also moves sram-supply from the globally required list to an
if-then block specifically for RK3588, as the RK3576 does not use it.

> diff --git a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
> index caca2a4903cd1..985cde6b2f523 100644
> --- a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
> +++ b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml

[ ... ]

> @@ -75,7 +76,16 @@ required:
>    - resets
>    - reset-names
>    - npu-supply
> -  - sram-supply
> +
> +allOf:
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: rockchip,rk3588-rknn-core
> +    then:
> +      required:
> +        - sram-supply

[Severity: Low]
Does this change inadvertently allow sram-supply to be specified for RK3576?

Because sram-supply is defined in the top-level properties block, removing
it from the global required list makes it implicitly optional for RK3576,
rather than forbidding it.

Device tree nodes for RK3576 could mistakenly include sram-supply without
triggering a validation error during dt_binding_check.

Would it be appropriate to add an else block setting sram-supply: false to
fully restrict it?

>  additionalProperties: false

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

[RFC PATCH v2 2/8] pmdomain/rockchip: add optional per-domain power-on settle delay

[Jiaxing Hu]

The RK3576 NPU domains need a short settle time after the idle
request is released before the QoS registers behind the domain are
written: restoring QoS immediately after de-idle can raise an SError
on the register access while the NoC is still settling (observed on
ROCK 4D as a panic inside rockchip_pd_power() during NPU runtime-PM
cycling).

The vendor kernel handles this with a per-domain delay_us field
applied between the de-idle request and the QoS restore, carrying
15us for the RK3576 NPUTOP domain.  Mirror that: add delay_us to
rockchip_domain_info, honor it in rockchip_pd_power() right before
rockchip_pmu_restore_qos(), and set 15us for NPUTOP, NPU0 and NPU1
(the two core domains power-cycle constantly under runtime PM and
showed the same symptom during bring-up).  All other domains keep 0,
so nothing changes for them.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/pmdomain/rockchip/pm-domains.c | 51 ++++++++++++++------------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 490bbb1d1..1787f62ba 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -59,6 +59,7 @@ struct rockchip_domain_info {
 	u32 pwr_offset;
 	u32 mem_offset;
 	u32 req_offset;
+	u32 delay_us;
 };
 
 struct rockchip_pmu_info {
@@ -185,7 +186,7 @@ struct rockchip_pmu {
 	.need_regulator = regulator,			\
 }
 
-#define DOMAIN_M_O_R_G(_name, p_offset, pwr, status, m_offset, m_status, r_status, r_offset, req, idle, ack, g_mask, wakeup)	\
+#define DOMAIN_M_O_R_G(_name, p_offset, pwr, status, m_offset, m_status, r_status, r_offset, req, idle, ack, g_mask, delay, wakeup)	\
 {							\
 	.name = _name,					\
 	.pwr_offset = p_offset,				\
@@ -200,6 +201,7 @@ struct rockchip_pmu {
 	.req_mask = (req),				\
 	.idle_mask = (idle),				\
 	.clk_ungate_mask = (g_mask),			\
+	.delay_us = (delay),				\
 	.ack_mask = (ack),				\
 	.active_wakeup = wakeup,			\
 }
@@ -244,8 +246,8 @@ struct rockchip_pmu {
 #define DOMAIN_RK3568(name, pwr, req, wakeup)		\
 	DOMAIN_M(name, pwr, pwr, req, req, req, wakeup)
 
-#define DOMAIN_RK3576(name, p_offset, pwr, status, r_status, r_offset, req, idle, g_mask, wakeup)	\
-	DOMAIN_M_O_R_G(name, p_offset, pwr, status, 0, r_status, r_status, r_offset, req, idle, idle, g_mask, wakeup)
+#define DOMAIN_RK3576(name, p_offset, pwr, status, r_status, r_offset, req, idle, g_mask, delay, wakeup)	\
+	DOMAIN_M_O_R_G(name, p_offset, pwr, status, 0, r_status, r_status, r_offset, req, idle, idle, g_mask, delay, wakeup)
 
 /*
  * Dynamic Memory Controller may need to coordinate with us -- see
@@ -612,7 +614,6 @@ static int rockchip_do_pmu_set_power_domain(struct rockchip_pm_domain *pd,
 			return ret;
 	}
 
-
 	ret = readx_poll_timeout_atomic(rockchip_pmu_domain_is_on, pd, is_on,
 					is_on == on, 0, 10000);
 	if (ret) {
@@ -667,6 +668,9 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
 		if (ret < 0)
 			goto out;
 
+		if (pd->info->delay_us)
+			udelay(pd->info->delay_us);
+
 		rockchip_pmu_restore_qos(pd);
 	}
 
@@ -1286,25 +1290,26 @@ static const struct rockchip_domain_info rk3568_pm_domains[] = {
 };
 
 static const struct rockchip_domain_info rk3576_pm_domains[] = {
-	[RK3576_PD_NPU]		= DOMAIN_RK3576("npu",    0x0, BIT(0),  BIT(0), 0,       0x0, 0,       0,       0,       false),
-	[RK3576_PD_NVM]		= DOMAIN_RK3576("nvm",    0x0, BIT(6),  0,      BIT(6),  0x4, BIT(2),  BIT(18), BIT(2),  false),
-	[RK3576_PD_SDGMAC]	= DOMAIN_RK3576("sdgmac", 0x0, BIT(7),  0,      BIT(7),  0x4, BIT(1),  BIT(17), 0x6,     false),
-	[RK3576_PD_AUDIO]	= DOMAIN_RK3576("audio",  0x0, BIT(8),  0,      BIT(8),  0x4, BIT(0),  BIT(16), BIT(0),  false),
-	[RK3576_PD_PHP]		= DOMAIN_RK3576("php",    0x0, BIT(9),  0,      BIT(9),  0x0, BIT(15), BIT(15), BIT(15), false),
-	[RK3576_PD_SUBPHP]	= DOMAIN_RK3576("subphp", 0x0, BIT(10), 0,      BIT(10), 0x0, 0,       0,       0,       false),
-	[RK3576_PD_VOP]		= DOMAIN_RK3576("vop",    0x0, BIT(11), 0,      BIT(11), 0x0, 0x6000,  0x6000,  0x6000,  false),
-	[RK3576_PD_VO1]		= DOMAIN_RK3576("vo1",    0x0, BIT(14), 0,      BIT(14), 0x0, BIT(12), BIT(12), 0x7000,  false),
-	[RK3576_PD_VO0]		= DOMAIN_RK3576("vo0",    0x0, BIT(15), 0,      BIT(15), 0x0, BIT(11), BIT(11), 0x6800,  false),
-	[RK3576_PD_USB]		= DOMAIN_RK3576("usb",    0x4, BIT(0),  0,      BIT(16), 0x0, BIT(10), BIT(10), 0x6400,  true),
-	[RK3576_PD_VI]		= DOMAIN_RK3576("vi",     0x4, BIT(1),  0,      BIT(17), 0x0, BIT(9),  BIT(9),  BIT(9),  false),
-	[RK3576_PD_VEPU0]	= DOMAIN_RK3576("vepu0",  0x4, BIT(2),  0,      BIT(18), 0x0, BIT(7),  BIT(7),  0x280,   false),
-	[RK3576_PD_VEPU1]	= DOMAIN_RK3576("vepu1",  0x4, BIT(3),  0,      BIT(19), 0x0, BIT(8),  BIT(8),  BIT(8),  false),
-	[RK3576_PD_VDEC]	= DOMAIN_RK3576("vdec",   0x4, BIT(4),  0,      BIT(20), 0x0, BIT(6),  BIT(6),  BIT(6),  false),
-	[RK3576_PD_VPU]		= DOMAIN_RK3576("vpu",    0x4, BIT(5),  0,      BIT(21), 0x0, BIT(5),  BIT(5),  BIT(5),  false),
-	[RK3576_PD_NPUTOP]	= DOMAIN_RK3576("nputop", 0x4, BIT(6),  0,      BIT(22), 0x0, 0x18,    0x18,    0x18,    false),
-	[RK3576_PD_NPU0]	= DOMAIN_RK3576("npu0",   0x4, BIT(7),  0,      BIT(23), 0x0, BIT(1),  BIT(1),  0x1a,    false),
-	[RK3576_PD_NPU1]	= DOMAIN_RK3576("npu1",   0x4, BIT(8),  0,      BIT(24), 0x0, BIT(2),  BIT(2),  0x1c,    false),
-	[RK3576_PD_GPU]		= DOMAIN_RK3576("gpu",    0x4, BIT(9),  0,      BIT(25), 0x0, BIT(0),  BIT(0),  BIT(0),  false),
+	/*                                               name    p_offset pwr      status  r_status r_offset req      idle     g_mask   delay wakeup */
+	[RK3576_PD_NPU]		= DOMAIN_RK3576("npu",    0x0, BIT(0),  BIT(0), 0,       0x0, 0,       0,       0,       0,    false),
+	[RK3576_PD_NVM]		= DOMAIN_RK3576("nvm",    0x0, BIT(6),  0,      BIT(6),  0x4, BIT(2),  BIT(18), BIT(2),  0,    false),
+	[RK3576_PD_SDGMAC]	= DOMAIN_RK3576("sdgmac", 0x0, BIT(7),  0,      BIT(7),  0x4, BIT(1),  BIT(17), 0x6,     0,    false),
+	[RK3576_PD_AUDIO]	= DOMAIN_RK3576("audio",  0x0, BIT(8),  0,      BIT(8),  0x4, BIT(0),  BIT(16), BIT(0),  0,    false),
+	[RK3576_PD_PHP]		= DOMAIN_RK3576("php",    0x0, BIT(9),  0,      BIT(9),  0x0, BIT(15), BIT(15), BIT(15), 0,    false),
+	[RK3576_PD_SUBPHP]	= DOMAIN_RK3576("subphp", 0x0, BIT(10), 0,      BIT(10), 0x0, 0,       0,       0,       0,    false),
+	[RK3576_PD_VOP]		= DOMAIN_RK3576("vop",    0x0, BIT(11), 0,      BIT(11), 0x0, 0x6000,  0x6000,  0x6000,  0,    false),
+	[RK3576_PD_VO1]		= DOMAIN_RK3576("vo1",    0x0, BIT(14), 0,      BIT(14), 0x0, BIT(12), BIT(12), 0x7000,  0,    false),
+	[RK3576_PD_VO0]		= DOMAIN_RK3576("vo0",    0x0, BIT(15), 0,      BIT(15), 0x0, BIT(11), BIT(11), 0x6800,  0,    false),
+	[RK3576_PD_USB]		= DOMAIN_RK3576("usb",    0x4, BIT(0),  0,      BIT(16), 0x0, BIT(10), BIT(10), 0x6400,  0,    true),
+	[RK3576_PD_VI]		= DOMAIN_RK3576("vi",     0x4, BIT(1),  0,      BIT(17), 0x0, BIT(9),  BIT(9),  BIT(9),  0,    false),
+	[RK3576_PD_VEPU0]	= DOMAIN_RK3576("vepu0",  0x4, BIT(2),  0,      BIT(18), 0x0, BIT(7),  BIT(7),  0x280,   0,    false),
+	[RK3576_PD_VEPU1]	= DOMAIN_RK3576("vepu1",  0x4, BIT(3),  0,      BIT(19), 0x0, BIT(8),  BIT(8),  BIT(8),  0,    false),
+	[RK3576_PD_VDEC]	= DOMAIN_RK3576("vdec",   0x4, BIT(4),  0,      BIT(20), 0x0, BIT(6),  BIT(6),  BIT(6),  0,    false),
+	[RK3576_PD_VPU]		= DOMAIN_RK3576("vpu",    0x4, BIT(5),  0,      BIT(21), 0x0, BIT(5),  BIT(5),  BIT(5),  0,    false),
+	[RK3576_PD_NPUTOP]	= DOMAIN_RK3576("nputop", 0x4, BIT(6),  0,      BIT(22), 0x0, 0x18,    0x18,    0x18,    15,   false),
+	[RK3576_PD_NPU0]	= DOMAIN_RK3576("npu0",   0x4, BIT(7),  0,      BIT(23), 0x0, BIT(1),  BIT(1),  0x1a,    15,   false),
+	[RK3576_PD_NPU1]	= DOMAIN_RK3576("npu1",   0x4, BIT(8),  0,      BIT(24), 0x0, BIT(2),  BIT(2),  0x1c,    15,   false),
+	[RK3576_PD_GPU]		= DOMAIN_RK3576("gpu",    0x4, BIT(9),  0,      BIT(25), 0x0, BIT(0),  BIT(0),  BIT(0),  0,    false),
 };
 
 static const struct rockchip_domain_info rk3588_pm_domains[] = {
-- 
2.43.0

Re: [RFC PATCH v2 2/8] pmdomain/rockchip: add optional per-domain power-on settle delay

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing state rollbacks in the error paths of rockchip_pd_power() can leave the hardware in an inconsistent or unusable state, leading to a kernel panic.
--

--- Patch [2]: [RFC PATCH v2 2/8] pmdomain/rockchip: add optional per-domain power-on settle delay ---
commit 5d8a3f8a8260643f6533a94cae0393fe9867b0cc
Author: Jiaxing Hu <gahing@gahingwoo.com>

pmdomain/rockchip: add optional per-domain power-on settle delay

This commit adds a per-domain settle delay for the RK3576 NPU domains.
It introduces a delay_us field to rockchip_domain_info and applies the
delay in rockchip_pd_power() before restoring QoS settings.

> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 490bbb1d1d8e8..1787f62ba3a81 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c

[ ... ]

> @@ -667,6 +668,9 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
>  		if (ret < 0)
>  			goto out;

[Severity: High]
This is a pre-existing issue, but there appears to be a problem with the
error path in rockchip_pd_power() when a domain transition fails.

When the genpd framework attempts to power off a domain and
rockchip_pd_power() is called with power_on set to false, the driver
successfully idles the bus interconnect:

        if (!power_on) {
                rockchip_pmu_save_qos(pd);

                /* if powering down, idle request to NIU first */
                ret = rockchip_pmu_set_idle_request(pd, true);
                if (ret < 0)
                        goto out;
        }

If the subsequent call to rockchip_do_pmu_set_power_domain(pd, false) fails
due to a hardware timeout, the function jumps to the out label without
reverting the idle request.

The genpd framework seems to gracefully handle the error by keeping the
domain status as GENPD_STATE_ON. However, when a consumer device later
resumes and attempts to access its MMIO registers, the interconnect remains
physically idled.

Can this cause the bus to hang and raise an SError, leading to a kernel
panic? Would it be appropriate to revert the idle request in the error path
before returning?

>  
> +		if (pd->info->delay_us)
> +			udelay(pd->info->delay_us);
> +
>  		rockchip_pmu_restore_qos(pd);
>  	}
>

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

[RFC PATCH v2 3/8] pmdomain/rockchip: cycle optional power-domain resets on power-on

[Jiaxing Hu]

Some power domains contain hardware bus interfaces that require an
explicit assert/deassert edge on their reset line after the domain is
powered on.  The CRU preserves reset state across power cycles (stays
deasserted), so the hardware never sees the edge it needs unless the
driver pulses it explicitly.

Add an optional resets property to rockchip_pm_domain.  When present,
the resets are asserted and deasserted (with a 10 us hold) inside
rockchip_pd_power() after the idle bits are cleared and the delay_us
settling time has elapsed, but before any device in the domain resumes.
The resets are released in rockchip_pm_remove_one_domain().

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/pmdomain/rockchip/pm-domains.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index 1787f62ba..8ce0cc3de 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -18,6 +18,7 @@
 #include <linux/of_address.h>
 #include <linux/of_clk.h>
 #include <linux/clk.h>
+#include <linux/reset.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/mfd/syscon.h>
@@ -102,6 +103,7 @@ struct rockchip_pm_domain {
 	struct clk_bulk_data *clks;
 	struct device_node *node;
 	struct regulator *supply;
+	struct reset_control *resets;
 };
 
 struct rockchip_pmu {
@@ -671,6 +673,12 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
 		if (pd->info->delay_us)
 			udelay(pd->info->delay_us);
 
+		if (pd->resets) {
+			reset_control_assert(pd->resets);
+			udelay(10);
+			reset_control_deassert(pd->resets);
+		}
+
 		rockchip_pmu_restore_qos(pd);
 	}
 
@@ -840,6 +848,14 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 	if (error)
 		goto err_put_clocks;
 
+	pd->resets = of_reset_control_array_get_optional_exclusive(node);
+	if (IS_ERR(pd->resets)) {
+		error = PTR_ERR(pd->resets);
+		dev_err(pmu->dev, "%pOFn: failed to get resets: %d\n", node, error);
+		pd->resets = NULL;
+		goto err_unprepare_clocks;
+	}
+
 	pd->num_qos = of_count_phandle_with_args(node, "pm_qos",
 						 NULL);
 
@@ -910,6 +926,7 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
 	clk_bulk_unprepare(pd->num_clks, pd->clks);
 err_put_clocks:
 	clk_bulk_put(pd->num_clks, pd->clks);
+	reset_control_put(pd->resets);
 	return error;
 }
 
@@ -928,6 +945,7 @@ static void rockchip_pm_remove_one_domain(struct rockchip_pm_domain *pd)
 
 	clk_bulk_unprepare(pd->num_clks, pd->clks);
 	clk_bulk_put(pd->num_clks, pd->clks);
+	reset_control_put(pd->resets);
 
 	/* protect the zeroing of pm->num_clks */
 	mutex_lock(&pd->pmu->mutex);
-- 
2.43.0

Re: [RFC PATCH v2 3/8] pmdomain/rockchip: cycle optional power-domain resets on power-on

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Probe error path fails to remove parent power domains (due to subdomain links), leaving freed memory linked in the global genpd list, leading to a Use-After-Free.
--

--- Patch [3]: [RFC PATCH v2 3/8] pmdomain/rockchip: cycle optional power-domain resets on power-on ---
commit 6b47e7abcdceaeb541eb946e08c3911e47f8d51e
Author: Jiaxing Hu <gahing@gahingwoo.com>

pmdomain/rockchip: cycle optional power-domain resets on power-on

This commit adds support for an optional resets property to the rockchip
power domain driver. Resets are now explicitly asserted and deasserted
during the power-on sequence to ensure the hardware interfaces observe
the necessary reset edge.

> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index 1787f62ba3a81..8ce0cc3de0bfb 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c

[ ... ]

> @@ -928,6 +945,7 @@ static void rockchip_pm_remove_one_domain(struct rockchip_pm_domain *pd)
>  
>  	clk_bulk_unprepare(pd->num_clks, pd->clks);
>  	clk_bulk_put(pd->num_clks, pd->clks);
> +	reset_control_put(pd->resets);

[Severity: Critical]
This is a pre-existing issue, but does this error path leave freed memory
linked in the global genpd list?

When a probe failure triggers cleanup via rockchip_pm_domain_cleanup(), it
iterates over all domains and calls rockchip_pm_remove_one_domain().

Since subdomain links are not explicitly removed first, pm_genpd_remove()
fails with -EBUSY for parent domains and leaves them linked in the global
gpd_list:

rockchip_pm_remove_one_domain()
    ret = pm_genpd_remove(&pd->genpd);
    if (ret < 0)
        dev_err(...) // -EBUSY is caught here but ignored

Because devm will subsequently free the rockchip_pm_domain structure which
embeds the genpd structure, could this lead to a use-after-free later when
the genpd core iterates gpd_list, for instance in genpd_power_off_unused()?

>  
>  	/* protect the zeroing of pm->num_clks */
>  	mutex_lock(&pd->pmu->mutex);

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

[RFC PATCH v2 4/8] iommu/rockchip: take all DT clocks

[Jiaxing Hu]

rk_iommu only enabled a fixed {aclk,iface} pair.  On the RK3576 NPU the
MMU sits behind the CBUF/DSU gates, so writes to DTE_ADDR are silently
dropped until those clocks run too (reads work, writes need more clocks).
Use devm_clk_bulk_get_all() to take every clock the devicetree provides
instead of a fixed pair, so a platform can list the full set it needs.
Clocks stay optional for the older clock-less devicetrees.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/iommu/rockchip-iommu.c | 28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 0013cf196..62cd6b022 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -93,11 +93,6 @@ struct rk_iommu_domain {
 	struct iommu_domain domain;
 };
 
-/* list of clocks required by IOMMU */
-static const char * const rk_iommu_clocks[] = {
-	"aclk", "iface",
-};
-
 struct rk_iommu_ops {
 	phys_addr_t (*pt_address)(u32 dte);
 	u32 (*mk_dtentries)(dma_addr_t pt_dma);
@@ -1246,25 +1241,20 @@ static int rk_iommu_probe(struct platform_device *pdev)
 	iommu->reset_disabled = device_property_read_bool(dev,
 					"rockchip,disable-mmu-reset");
 
-	iommu->num_clocks = ARRAY_SIZE(rk_iommu_clocks);
-	iommu->clocks = devm_kcalloc(iommu->dev, iommu->num_clocks,
-				     sizeof(*iommu->clocks), GFP_KERNEL);
-	if (!iommu->clocks)
-		return -ENOMEM;
-
-	for (i = 0; i < iommu->num_clocks; ++i)
-		iommu->clocks[i].id = rk_iommu_clocks[i];
-
 	/*
-	 * iommu clocks should be present for all new devices and devicetrees
-	 * but there are older devicetrees without clocks out in the wild.
-	 * So clocks as optional for the time being.
+	 * Take every clock the devicetree provides.  Most IOMMU instances
+	 * need exactly "aclk" + "iface", but e.g. the RK3576 NPU IOMMUs sit
+	 * behind additional gates (CBUF/DSU) whose clocks must be running
+	 * for register writes to land.  Clocks stay optional because there
+	 * are older devicetrees without clocks out in the wild.
 	 */
-	err = devm_clk_bulk_get(iommu->dev, iommu->num_clocks, iommu->clocks);
+	err = devm_clk_bulk_get_all(iommu->dev, &iommu->clocks);
 	if (err == -ENOENT)
 		iommu->num_clocks = 0;
-	else if (err)
+	else if (err < 0)
 		return err;
+	else
+		iommu->num_clocks = err;
 
 	err = clk_bulk_prepare(iommu->num_clocks, iommu->clocks);
 	if (err)
-- 
2.43.0

Re: [RFC PATCH v2 4/8] iommu/rockchip: take all DT clocks

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Resource mapping loop incorrectly indexes `iommu->bases` array, leading to gaps and invalid pointer dereferences
--

--- Patch [4]: [RFC PATCH v2 4/8] iommu/rockchip: take all DT clocks ---
commit 77bcbb4eca287b3632f08a8528235d32ff400f72
Author: Jiaxing Hu <gahing@gahingwoo.com>

iommu/rockchip: take all DT clocks
    
rk_iommu only enabled a fixed {aclk,iface} pair.  On the RK3576 NPU the
MMU sits behind the CBUF/DSU gates, so writes to DTE_ADDR are silently
dropped until those clocks run too (reads work, writes need more clocks).
Use devm_clk_bulk_get_all() to take every clock the devicetree provides
instead of a fixed pair, so a platform can list the full set it needs.
Clocks stay optional for the older clock-less devicetrees.

> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 87ae036d64145..2ac65c0c5c0b4 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c

[ ... ]

> @@ -1254,25 +1249,20 @@ static int rk_iommu_probe(struct platform_device *pdev)

[Severity: High]
This is a pre-existing issue, but does the resource mapping loop in
rk_iommu_probe() incorrectly index the iommu->bases array?

If multiple memory resources are provided but one fails to map (for example,
if num_res = 2 and mapping for index 0 fails), iommu->bases[0] holds an
ERR_PTR while iommu->bases[1] holds the valid base. However, iommu->num_mmu
becomes 1.

Since subsequent driver loops iterate from 0 to num_mmu - 1 and
unconditionally dereference iommu->bases, this would lead to a crash on
iommu->bases[0].

Also, if num_res == 0, devm_kcalloc() returns ZERO_SIZE_PTR and reading
iommu->bases[0] on the error path would panic:

	for (i = 0; i < num_res; i++) {
		res = platform_get_resource(pdev, IORESOURCE_MEM, i);
		if (!res)
			continue;
		iommu->bases[i] = devm_ioremap_resource(&pdev->dev, res);
		if (IS_ERR(iommu->bases[i]))
			continue;
		iommu->num_mmu++;
	}
	if (iommu->num_mmu == 0)
		return PTR_ERR(iommu->bases[0]);

Could we assign to iommu->bases[iommu->num_mmu] instead of iommu->bases[i]
to prevent gaps in the array, and handle the num_res == 0 error path
differently?

> 	iommu->reset_disabled = device_property_read_bool(dev,
> 					"rockchip,disable-mmu-reset");
>  
> -	iommu->num_clocks = ARRAY_SIZE(rk_iommu_clocks);
> -	iommu->clocks = devm_kcalloc(iommu->dev, iommu->num_clocks,
> -				     sizeof(*iommu->clocks), GFP_KERNEL);
> -	if (!iommu->clocks)
> -		return -ENOMEM;
> -
> -	for (i = 0; i < iommu->num_clocks; ++i)
> -		iommu->clocks[i].id = rk_iommu_clocks[i];
> -
> 	/*
> -	 * iommu clocks should be present for all new devices and devicetrees
> -	 * but there are older devicetrees without clocks out in the wild.
> -	 * So clocks as optional for the time being.
> +	 * Take every clock the devicetree provides.  Most IOMMU instances
> +	 * need exactly "aclk" + "iface", but e.g. the RK3576 NPU IOMMUs sit
> +	 * behind additional gates (CBUF/DSU) whose clocks must be running
> +	 * for register writes to land.  Clocks stay optional because there
> +	 * are older devicetrees without clocks out in the wild.
> 	 */
> -	err = devm_clk_bulk_get(iommu->dev, iommu->num_clocks, iommu->clocks);
> +	err = devm_clk_bulk_get_all(iommu->dev, &iommu->clocks);
> 	if (err == -ENOENT)
> 		iommu->num_clocks = 0;
> -	else if (err)
> +	else if (err < 0)
> 		return err;
> +	else
> +		iommu->num_clocks = err;
>  
> 	err = clk_bulk_prepare(iommu->num_clocks, iommu->clocks);
> 	if (err)

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

[RFC PATCH v2 5/8] iommu/rockchip: clear stale page faults before enabling stall

[Jiaxing Hu]

Boot firmware can leave an IOMMU bank in PAGE_FAULT_ACTIVE before the
driver has configured paging:

  PAGE_FAULT_ACTIVE=1  STALL_ACTIVE=0  IDLE=1

Such a bank ignores CMD_ENABLE_STALL and never reaches STALL_ACTIVE, so
rk_iommu_enable_stall()'s readx_poll_timeout() spins until it times out
(seen on the RK3576 NPU, whose MMUs share this poll across banks).

Rather than special-casing these banks in the stall path, acknowledge the
stale fault with CMD_PAGE_FAULT_DONE before enabling stall, so every bank
starts from a clean state and the normal stall sequence applies to all of
them.  Banks without a pending fault are untouched.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/iommu/rockchip-iommu.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
index 62cd6b022..68bd55433 100644
--- a/drivers/iommu/rockchip-iommu.c
+++ b/drivers/iommu/rockchip-iommu.c
@@ -415,6 +415,19 @@ static int rk_iommu_enable_stall(struct rk_iommu *iommu)
 	if (!rk_iommu_is_paging_enabled(iommu))
 		return 0;
 
+	/*
+	 * Boot firmware can leave a bank in PAGE_FAULT_ACTIVE with no handler
+	 * (PAGE_FAULT_ACTIVE & !STALL_ACTIVE & IDLE).  Such a bank ignores
+	 * CMD_ENABLE_STALL and never reaches STALL_ACTIVE, timing out the poll
+	 * below.  Acknowledge any stale fault first so every bank starts clean.
+	 */
+	for (i = 0; i < iommu->num_mmu; i++) {
+		if (rk_iommu_read(iommu->bases[i], RK_MMU_STATUS) &
+		    RK_MMU_STATUS_PAGE_FAULT_ACTIVE)
+			writel(RK_MMU_CMD_PAGE_FAULT_DONE,
+			       iommu->bases[i] + RK_MMU_COMMAND);
+	}
+
 	rk_iommu_command(iommu, RK_MMU_CMD_ENABLE_STALL);
 
 	ret = readx_poll_timeout(rk_iommu_is_stall_active, iommu, val,
-- 
2.43.0

Re: [RFC PATCH v2 5/8] iommu/rockchip: clear stale page faults before enabling stall

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Clearing the stale page fault state without clearing the associated interrupt status leads to a spurious IOMMU interrupt upon unmasking if the MMU reset is disabled.
--

--- Patch [5]: [RFC PATCH v2 5/8] iommu/rockchip: clear stale page faults before enabling stall ---
commit 6445c2bf287495e424096d491a8245a58e126724
Author: Jiaxing Hu <gahing@gahingwoo.com>

iommu/rockchip: clear stale page faults before enabling stall

Boot firmware can leave an IOMMU bank in PAGE_FAULT_ACTIVE before the
driver has configured paging. This commit acknowledges these stale faults
with CMD_PAGE_FAULT_DONE before enabling stall so all banks start clean
and avoid timing out.

> diff --git a/drivers/iommu/rockchip-iommu.c b/drivers/iommu/rockchip-iommu.c
> index 2ac65c0c5c0b4..e477fe79ca4a3 100644
> --- a/drivers/iommu/rockchip-iommu.c
> +++ b/drivers/iommu/rockchip-iommu.c
> @@ -417,6 +417,19 @@ static int rk_iommu_enable_stall(struct rk_iommu *iommu)
>  	if (!rk_iommu_is_paging_enabled(iommu))
>  		return 0;
>  
> +	/*
> +	 * Boot firmware can leave a bank in PAGE_FAULT_ACTIVE with no handler
> +	 * (PAGE_FAULT_ACTIVE & !STALL_ACTIVE & IDLE).  Such a bank ignores
> +	 * CMD_ENABLE_STALL and never reaches STALL_ACTIVE, timing out the poll
> +	 * below.  Acknowledge any stale fault first so every bank starts clean.
> +	 */
> +	for (i = 0; i < iommu->num_mmu; i++) {
> +		if (rk_iommu_read(iommu->bases[i], RK_MMU_STATUS) &
> +		    RK_MMU_STATUS_PAGE_FAULT_ACTIVE)
> +			writel(RK_MMU_CMD_PAGE_FAULT_DONE,
> +			       iommu->bases[i] + RK_MMU_COMMAND);

[Severity: High]
Does clearing the page fault state here also require clearing the associated 
interrupt status in RK_MMU_INT_CLEAR?

If a device uses rockchip,disable-mmu-reset, the hardware reset during
rk_iommu_enable() is skipped. When interrupts are subsequently unmasked
by writing to RK_MMU_INT_MASK, an uncleared raw interrupt bit would
immediately trigger a spurious interrupt.

Could this spurious interrupt lead to report_iommu_fault() being called
prematurely, potentially panicking the system or failing device attachment?

> +	}
> +
>  	rk_iommu_command(iommu, RK_MMU_CMD_ENABLE_STALL);

[ ... ]

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

[RFC PATCH v2 6/8] accel/rocket: add RK3576 NPU (RKNN) support

[Jiaxing Hu]

Add RK3576 support to the rocket DRM accelerator driver (used with the
Mesa Teflon TFLite delegate).  Per-SoC differences are selected by new
of_device_id match data (struct rocket_soc_data) so the RK3588 path stays
unchanged:

  - match rockchip,rk3576-rknn-core; iterate its nodes at probe
  - RK3576 takes six clocks (adds the CBUF domain ACLK/HCLK_RKNN_CBUF):
    the CNA fills the CBUF and CORE reads from it, so the compute path
    stalls without them.  RK3588 keeps its four clocks.
  - RK3576 requests one reset (srst_a); its BIU reset (srst_h) is driven
    from the power domain.  RK3588 keeps both.
  - RK3576 spans two power domains (PD_NPU0 + PD_NPU1) and attaches the
    list explicitly; RK3588 is single-domain and keeps the driver-core
    auto-attach.
  - RK3576 has no maskable completion interrupt (PC_DONE is read-only in
    INTERRUPT_MASK), so it polls PC_DONE via an hrtimer; RK3588 keeps the
    DPU completion IRQ.
  - guard rocket_job_timedout() MMIO behind pm_runtime_active()

Tested on a Radxa ROCK 4D: the NPU probes, powers on, brings up its
IOMMUs and runs submitted jobs to completion.  Full multi-layer inference
is not yet correct on this SoC (only the first operation per power session
produces valid output); see the cover letter.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/accel/rocket/rocket_core.c   |  39 +++++++-
 drivers/accel/rocket/rocket_core.h   |  22 ++++-
 drivers/accel/rocket/rocket_device.c |   4 +
 drivers/accel/rocket/rocket_drv.c    |  22 ++++-
 drivers/accel/rocket/rocket_job.c    | 127 +++++++++++++++++++++++++--
 5 files changed, 200 insertions(+), 14 deletions(-)

diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
index b3b2fa9ba..140e37969 100644
--- a/drivers/accel/rocket/rocket_core.c
+++ b/drivers/accel/rocket/rocket_core.c
@@ -8,6 +8,7 @@
 #include <linux/err.h>
 #include <linux/iommu.h>
 #include <linux/platform_device.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_runtime.h>
 #include <linux/reset.h>
 
@@ -21,14 +22,31 @@ int rocket_core_init(struct rocket_core *core)
 	u32 version;
 	int err = 0;
 
+	/*
+	 * RK3576 moves the BIU (srst_h) reset to its power domain, so it only
+	 * requests srst_a here (soc->num_resets == 1); RK3588 keeps both.
+	 */
 	core->resets[0].id = "srst_a";
 	core->resets[1].id = "srst_h";
-	err = devm_reset_control_bulk_get_exclusive(&pdev->dev, ARRAY_SIZE(core->resets),
+	err = devm_reset_control_bulk_get_exclusive(&pdev->dev, core->soc->num_resets,
 						    core->resets);
 	if (err)
 		return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
 
-	err = devm_clk_bulk_get(dev, ARRAY_SIZE(core->clks), core->clks);
+	core->clks[0].id = "aclk";
+	core->clks[1].id = "hclk";
+	core->clks[2].id = "npu";
+	core->clks[3].id = "pclk";
+	/*
+	 * RK3576 (soc->num_clks == 6): the CBUF (convolution buffer) has its own
+	 * clock domain. The CNA fills the CBUF and CORE reads from it; without
+	 * these the compute path stalls after loading one slice (RDMA, which
+	 * bypasses the CBUF, still runs). The vendor keeps all NPU clocks on
+	 * whenever powered.
+	 */
+	core->clks[4].id = "aclk_cbuf";
+	core->clks[5].id = "hclk_cbuf";
+	err = devm_clk_bulk_get(dev, core->soc->num_clks, core->clks);
 	if (err)
 		return dev_err_probe(dev, err, "failed to get clocks for core %d\n", core->index);
 
@@ -65,6 +83,23 @@ int rocket_core_init(struct rocket_core *core)
 		return err;
 	}
 
+	/*
+	 * RK3576: the NPU spans TWO power domains (PD_NPU0 + PD_NPU1). The vendor
+	 * powers BOTH from its single NPU node even when computing on one core --
+	 * the CBUF->CMAC read path only works fully with NPU1 powered. A device
+	 * with more than one power-domain is skipped by the driver-core single-PD
+	 * auto-attach, so attach the list explicitly.  RK3588 has a single domain
+	 * and keeps the driver-core auto-attach (soc->multi_power_domain == false).
+	 */
+	if (core->soc->multi_power_domain) {
+		struct dev_pm_domain_list *pd_list;
+
+		err = devm_pm_domain_attach_list(dev, NULL, &pd_list);
+		if (err < 0)
+			return dev_err_probe(dev, err,
+					     "failed to attach NPU power domains\n");
+	}
+
 	pm_runtime_use_autosuspend(dev);
 
 	/*
diff --git a/drivers/accel/rocket/rocket_core.h b/drivers/accel/rocket/rocket_core.h
index f6d738285..2ab389c4b 100644
--- a/drivers/accel/rocket/rocket_core.h
+++ b/drivers/accel/rocket/rocket_core.h
@@ -6,6 +6,7 @@
 
 #include <drm/gpu_scheduler.h>
 #include <linux/clk.h>
+#include <linux/hrtimer.h>
 #include <linux/io.h>
 #include <linux/mutex_types.h>
 #include <linux/reset.h>
@@ -27,16 +28,30 @@
 #define rocket_core_writel(core, reg, value) \
 	writel(value, (core)->core_iomem + (REG_CORE_##reg) - REG_CORE_S_STATUS)
 
+/*
+ * Per-SoC differences, selected by the of_device_id match data.  The RK3588
+ * path (all flags/counts at their base values) must stay byte-for-byte the
+ * original behaviour; RK3576 opts in to the extra clocks, the multi-domain
+ * attach and the polled completion.
+ */
+struct rocket_soc_data {
+	unsigned int num_clks;		/* clk_bulk count: 4 base, 6 with CBUF */
+	unsigned int num_resets;	/* reset_bulk count: 2 base, 1 on RK3576 */
+	bool multi_power_domain;	/* device spans more than one PM domain */
+	bool poll_completion;		/* PC_DONE not routable to the GIC; poll it */
+};
+
 struct rocket_core {
 	struct device *dev;
 	struct rocket_device *rdev;
+	const struct rocket_soc_data *soc;
 	unsigned int index;
 
 	int irq;
 	void __iomem *pc_iomem;
 	void __iomem *cna_iomem;
 	void __iomem *core_iomem;
-	struct clk_bulk_data clks[4];
+	struct clk_bulk_data clks[6];
 	struct reset_control_bulk_data resets[2];
 
 	struct iommu_group *iommu_group;
@@ -52,6 +67,11 @@ struct rocket_core {
 		atomic_t pending;
 	} reset;
 
+	/* RK3576 has no completion IRQ; poll for PC_DONE via hrtimer. */
+	struct hrtimer poll_timer;
+	struct work_struct poll_work;
+	atomic_t poll_active;
+
 	struct drm_gpu_scheduler sched;
 	u64 fence_context;
 	u64 emit_seqno;
diff --git a/drivers/accel/rocket/rocket_device.c b/drivers/accel/rocket/rocket_device.c
index 46e6ee1e7..bfb00f967 100644
--- a/drivers/accel/rocket/rocket_device.c
+++ b/drivers/accel/rocket/rocket_device.c
@@ -31,6 +31,10 @@ struct rocket_device *rocket_device_init(struct platform_device *pdev,
 		if (of_device_is_available(core_node))
 			num_cores++;
 
+	for_each_compatible_node(core_node, NULL, "rockchip,rk3576-rknn-core")
+		if (of_device_is_available(core_node))
+			num_cores++;
+
 	rdev->cores = devm_kcalloc(dev, num_cores, sizeof(*rdev->cores), GFP_KERNEL);
 	if (!rdev->cores)
 		return ERR_PTR(-ENOMEM);
diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
index 8bbbce594..7f7dfa374 100644
--- a/drivers/accel/rocket/rocket_drv.c
+++ b/drivers/accel/rocket/rocket_drv.c
@@ -176,6 +176,7 @@ static int rocket_probe(struct platform_device *pdev)
 
 	rdev->cores[core].rdev = rdev;
 	rdev->cores[core].dev = &pdev->dev;
+	rdev->cores[core].soc = of_device_get_match_data(&pdev->dev);
 	rdev->cores[core].index = core;
 
 	rdev->num_cores++;
@@ -213,8 +214,23 @@ static void rocket_remove(struct platform_device *pdev)
 	}
 }
 
+static const struct rocket_soc_data rk3588_soc_data = {
+	.num_clks = 4,
+	.num_resets = 2,
+	.multi_power_domain = false,
+	.poll_completion = false,
+};
+
+static const struct rocket_soc_data rk3576_soc_data = {
+	.num_clks = 6,
+	.num_resets = 1,
+	.multi_power_domain = true,
+	.poll_completion = true,
+};
+
 static const struct of_device_id dt_match[] = {
-	{ .compatible = "rockchip,rk3588-rknn-core" },
+	{ .compatible = "rockchip,rk3588-rknn-core", .data = &rk3588_soc_data },
+	{ .compatible = "rockchip,rk3576-rknn-core", .data = &rk3576_soc_data },
 	{}
 };
 MODULE_DEVICE_TABLE(of, dt_match);
@@ -240,7 +256,7 @@ static int rocket_device_runtime_resume(struct device *dev)
 	if (core < 0)
 		return -ENODEV;
 
-	err = clk_bulk_prepare_enable(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
+	err = clk_bulk_prepare_enable(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
 	if (err) {
 		dev_err(dev, "failed to enable (%d) clocks for core %d\n", err, core);
 		return err;
@@ -260,7 +276,7 @@ static int rocket_device_runtime_suspend(struct device *dev)
 	if (!rocket_job_is_idle(&rdev->cores[core]))
 		return -EBUSY;
 
-	clk_bulk_disable_unprepare(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
+	clk_bulk_disable_unprepare(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
 
 	return 0;
 }
diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
index 2f1861f96..ce3f7c92e 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -7,6 +7,7 @@
 #include <drm/drm_file.h>
 #include <drm/drm_gem.h>
 #include <drm/rocket_accel.h>
+#include <linux/hrtimer.h>
 #include <linux/interrupt.h>
 #include <linux/iommu.h>
 #include <linux/platform_device.h>
@@ -20,6 +21,16 @@
 
 #define JOB_TIMEOUT_MS 500
 
+/*
+ * RK3576: INTERRUPT_MASK bits 28-29 are read-only (hardware rejects the write),
+ * so the PC_DONE completion signal cannot be routed to the GIC via the normal
+ * interrupt-mask path.  We poll OPERATION_ENABLE every RK3576_POLL_INTERVAL_NS
+ * instead of waiting for a completion IRQ.
+ */
+#define PC_INTERRUPT_MASK_RK3576_PC_DONE_0  0x10000000u
+#define PC_INTERRUPT_MASK_RK3576_PC_DONE_1  0x20000000u
+#define RK3576_POLL_INTERVAL_NS  1000000LL  /* 1 ms */
+
 static struct rocket_job *
 to_rocket_job(struct drm_sched_job *sched_job)
 {
@@ -137,8 +148,24 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
 	rocket_pc_writel(core, REGISTER_AMOUNTS,
 			 PC_REGISTER_AMOUNTS_PC_DATA_AMOUNT((task->regcmd_count + 1) / 2 - 1));
 
-	rocket_pc_writel(core, INTERRUPT_MASK, PC_INTERRUPT_MASK_DPU_0 | PC_INTERRUPT_MASK_DPU_1);
-	rocket_pc_writel(core, INTERRUPT_CLEAR, PC_INTERRUPT_CLEAR_DPU_0 | PC_INTERRUPT_CLEAR_DPU_1);
+	if (core->soc->poll_completion) {
+		/*
+		 * RK3576: PC_DONE (bits 28-29) is read-only in INTERRUPT_MASK, so
+		 * it cannot be routed to the GIC; enable the DMA-error interrupts
+		 * and poll PC_DONE via the hrtimer started below.
+		 */
+		rocket_pc_writel(core, INTERRUPT_MASK,
+				 PC_INTERRUPT_MASK_DMA_READ_ERROR |
+				 PC_INTERRUPT_MASK_DMA_WRITE_ERROR);
+		rocket_pc_writel(core, INTERRUPT_CLEAR,
+				 PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
+				 PC_INTERRUPT_MASK_RK3576_PC_DONE_1);
+	} else {
+		rocket_pc_writel(core, INTERRUPT_MASK,
+				 PC_INTERRUPT_MASK_DPU_0 | PC_INTERRUPT_MASK_DPU_1);
+		rocket_pc_writel(core, INTERRUPT_CLEAR,
+				 PC_INTERRUPT_CLEAR_DPU_0 | PC_INTERRUPT_CLEAR_DPU_1);
+	}
 
 	rocket_pc_writel(core, TASK_CON, PC_TASK_CON_RESERVED_0(1) |
 					 PC_TASK_CON_TASK_COUNT_CLEAR(1) |
@@ -149,7 +176,14 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
 
 	rocket_pc_writel(core, OPERATION_ENABLE, PC_OPERATION_ENABLE_OP_EN(1));
 
-	dev_dbg(core->dev, "Submitted regcmd at 0x%llx to core %d", task->regcmd, core->index);
+	if (core->soc->poll_completion) {
+		atomic_set(&core->poll_active, 1);
+		hrtimer_start(&core->poll_timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS),
+			      HRTIMER_MODE_REL);
+	} else {
+		dev_dbg(core->dev, "Submitted regcmd at 0x%llx to core %d",
+			task->regcmd, core->index);
+	}
 }
 
 static int rocket_acquire_object_fences(struct drm_gem_object **bos,
@@ -326,12 +360,55 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
 	return fence;
 }
 
+static void rocket_job_handle_irq(struct rocket_core *core);
+
+static enum hrtimer_restart rocket_poll_timer_fn(struct hrtimer *timer)
+{
+	struct rocket_core *core = container_of(timer, struct rocket_core, poll_timer);
+
+	if (!atomic_read(&core->poll_active))
+		return HRTIMER_NORESTART;
+
+	/*
+	 * On RK3576, OPERATION_ENABLE is not cleared by hardware on completion;
+	 * check INTERRUPT_RAW_STATUS bits 28-29 (PC_DONE_0/1) instead.
+	 */
+	if (rocket_pc_readl(core, OPERATION_ENABLE) == 0 ||
+	    (rocket_pc_readl(core, INTERRUPT_RAW_STATUS) &
+	     (PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
+	      PC_INTERRUPT_MASK_RK3576_PC_DONE_1))) {
+		atomic_set(&core->poll_active, 0);
+		schedule_work(&core->poll_work);
+		return HRTIMER_NORESTART;
+	}
+
+	hrtimer_forward_now(timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS));
+	return HRTIMER_RESTART;
+}
+
+static void rocket_poll_work_fn(struct work_struct *work)
+{
+	struct rocket_core *core = container_of(work, struct rocket_core, poll_work);
+
+	rocket_job_handle_irq(core);
+}
+
 static void rocket_job_handle_irq(struct rocket_core *core)
 {
+	u32 clear = 0x1ffff;
+
+	if (core->soc->poll_completion) {
+		/* Stop the completion poll -- we're handling it now. */
+		atomic_set(&core->poll_active, 0);
+		hrtimer_cancel(&core->poll_timer);
+		clear |= PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
+			 PC_INTERRUPT_MASK_RK3576_PC_DONE_1;
+	}
+
 	pm_runtime_mark_last_busy(core->dev);
 
 	rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
-	rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
+	rocket_pc_writel(core, INTERRUPT_CLEAR, clear);
 
 	scoped_guard(mutex, &core->job_lock)
 		if (core->in_flight_job) {
@@ -353,6 +430,12 @@ rocket_reset(struct rocket_core *core, struct drm_sched_job *bad)
 	if (!atomic_read(&core->reset.pending))
 		return;
 
+	if (core->soc->poll_completion) {
+		atomic_set(&core->poll_active, 0);
+		hrtimer_cancel(&core->poll_timer);
+		cancel_work_sync(&core->poll_work);
+	}
+
 	drm_sched_stop(&core->sched, bad);
 
 	/*
@@ -385,7 +468,14 @@ static enum drm_gpu_sched_stat rocket_job_timedout(struct drm_sched_job *sched_j
 	struct rocket_device *rdev = job->rdev;
 	struct rocket_core *core = sched_to_core(rdev, sched_job->sched);
 
-	dev_err(core->dev, "NPU job timed out");
+	if (pm_runtime_active(core->dev))
+		dev_err(core->dev,
+			"NPU job timed out: RAW_STATUS=0x%08x MASK=0x%08x OP_EN=0x%08x\n",
+			rocket_pc_readl(core, INTERRUPT_RAW_STATUS),
+			rocket_pc_readl(core, INTERRUPT_MASK),
+			rocket_pc_readl(core, OPERATION_ENABLE));
+	else
+		dev_err(core->dev, "NPU job timed out (device not active)\n");
 
 	atomic_set(&core->reset.pending, 1);
 	rocket_reset(core, sched_job);
@@ -424,9 +514,22 @@ static irqreturn_t rocket_job_irq_handler(int irq, void *data)
 	WARN_ON(raw_status & PC_INTERRUPT_RAW_STATUS_DMA_READ_ERROR);
 	WARN_ON(raw_status & PC_INTERRUPT_RAW_STATUS_DMA_WRITE_ERROR);
 
-	if (!(raw_status & PC_INTERRUPT_RAW_STATUS_DPU_0 ||
-	      raw_status & PC_INTERRUPT_RAW_STATUS_DPU_1))
-		return IRQ_NONE;
+	if (core->soc->poll_completion) {
+		/*
+		 * RK3576: completion is polled (rocket_poll_timer_fn); only the
+		 * DMA-error bits (0-13) can raise this shared IRQ.
+		 */
+		u32 active = raw_status & 0x3fff;
+
+		if (!active)
+			return IRQ_NONE;
+
+		rocket_pc_writel(core, INTERRUPT_CLEAR, active);
+	} else {
+		if (!(raw_status & PC_INTERRUPT_RAW_STATUS_DPU_0 ||
+		      raw_status & PC_INTERRUPT_RAW_STATUS_DPU_1))
+			return IRQ_NONE;
+	}
 
 	rocket_pc_writel(core, INTERRUPT_MASK, 0x0);
 
@@ -445,6 +548,10 @@ int rocket_job_init(struct rocket_core *core)
 	int ret;
 
 	INIT_WORK(&core->reset.work, rocket_reset_work);
+	INIT_WORK(&core->poll_work, rocket_poll_work_fn);
+	hrtimer_setup(&core->poll_timer, rocket_poll_timer_fn, CLOCK_MONOTONIC,
+		      HRTIMER_MODE_REL);
+	atomic_set(&core->poll_active, 0);
 	spin_lock_init(&core->fence_lock);
 	mutex_init(&core->job_lock);
 
@@ -486,6 +593,10 @@ int rocket_job_init(struct rocket_core *core)
 
 void rocket_job_fini(struct rocket_core *core)
 {
+	atomic_set(&core->poll_active, 0);
+	hrtimer_cancel(&core->poll_timer);
+	cancel_work_sync(&core->poll_work);
+
 	drm_sched_fini(&core->sched);
 
 	cancel_work_sync(&core->reset.work);
-- 
2.43.0

Re: [RFC PATCH v2 6/8] accel/rocket: add RK3576 NPU (RKNN) support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:

New issues:
- [High] Resource leak on `devm_pm_domain_attach_list` failure.
- [High] Teardown order race in `rocket_job_fini` allows the hrtimer to be re-armed after cancellation.
- [High] Race condition and hardware state corruption on RK3576 DMA errors due to double-processing and unsynchronized MMIO writes.

Pre-existing issues:
- [High] Unconditional MMIO access in shared IRQ handler causes bus faults while device is suspended.
- [High] Leaked IOMMU group reference on job completion.
- [High] The runtime suspend callback powers down the device without synchronizing the shared IRQ.
- [High] Calling `drm_sched_fini` on a partially initialized scheduler causes a kernel panic.
--

--- Patch [6]: [RFC PATCH v2 6/8] accel/rocket: add RK3576 NPU (RKNN) support ---
commit 3624dfd3291e5171660b2a505645f90fa60606b8
Author: Jiaxing Hu <gahing@gahingwoo.com>

accel/rocket: add RK3576 NPU (RKNN) support

This commit adds RK3576 support to the rocket DRM accelerator driver. It
introduces new clocks, power domains, and a polled completion mechanism
since the RK3576 has no maskable completion interrupt.

> diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
> index b3b2fa9ba645a..140e37969a7b1 100644
> --- a/drivers/accel/rocket/rocket_core.c
> +++ b/drivers/accel/rocket/rocket_core.c
> @@ -65,6 +83,23 @@ int rocket_core_init(struct rocket_core *core)
>  		return err;
>  	}
>  
> +	/*
> +	 * RK3576: the NPU spans TWO power domains (PD_NPU0 + PD_NPU1).
> [ ... ]
> +	if (core->soc->multi_power_domain) {
> +		struct dev_pm_domain_list *pd_list;
> +
> +		err = devm_pm_domain_attach_list(dev, NULL, &pd_list);
> +		if (err < 0)
> +			return dev_err_probe(dev, err,
> +					     "failed to attach NPU power domains\n");
> +	}
> +

[Severity: High]
Does this early return leak resources?

In rocket_core_init(), rocket_job_init() is called earlier, which allocates
non-devm resources such as an ordered workqueue and a DRM scheduler.

If devm_pm_domain_attach_list() fails, the function returns directly with
dev_err_probe() and skips the necessary rocket_job_fini() and
iommu_group_put() cleanup.

> diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
> --- a/drivers/accel/rocket/rocket_drv.c
> +++ b/drivers/accel/rocket/rocket_drv.c
> @@ -260,7 +276,7 @@ static int rocket_device_runtime_suspend(struct device *dev)
>  	if (!rocket_job_is_idle(&rdev->cores[core]))
>  		return -EBUSY;
>  
> -	clk_bulk_disable_unprepare(ARRAY_SIZE(rdev->cores[core].clks), rdev->cores[core].clks);
> +	clk_bulk_disable_unprepare(rdev->cores[core].soc->num_clks, rdev->cores[core].clks);
>  

[Severity: High]
This is a pre-existing issue, but does this power down the device without
synchronizing the shared IRQ?

Since the driver uses IRQF_SHARED, another device sharing the IRQ line could
trigger the interrupt handler just as the clocks are being disabled.

Can this race condition cause the IRQ handler to execute on powered-down
hardware, potentially leading to a kernel panic? Should we call
synchronize_irq() before turning off the clocks?

> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
> @@ -326,12 +360,55 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
> [ ... ]
> +static void rocket_job_handle_irq(struct rocket_core *core)
> +{
> +	u32 clear = 0x1ffff;
> +
> +	if (core->soc->poll_completion) {
> +		/* Stop the completion poll -- we're handling it now. */
> +		atomic_set(&core->poll_active, 0);
> +		hrtimer_cancel(&core->poll_timer);
> +		clear |= PC_INTERRUPT_MASK_RK3576_PC_DONE_0 |
> +			 PC_INTERRUPT_MASK_RK3576_PC_DONE_1;
> +	}
> +
>  	pm_runtime_mark_last_busy(core->dev);
>  
>  	rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
> -	rocket_pc_writel(core, INTERRUPT_CLEAR, 0x1ffff);
> +	rocket_pc_writel(core, INTERRUPT_CLEAR, clear);
>  

[Severity: High]
Can a DMA error on RK3576 trigger a race condition that corrupts hardware
state?

On RK3576, a DMA error drops OPERATION_ENABLE to 0 and raises a DMA error
interrupt. This simultaneously triggers the threaded IRQ handler and the
poll timer workqueue, which will both see OPERATION_ENABLE == 0.

Both threads will concurrently invoke rocket_job_handle_irq(). Because this
function clears OPERATION_ENABLE outside the job_lock, one thread can write
0 to OPERATION_ENABLE right after the other thread has already submitted the
next job and written 1.

Does this permanently halt the hardware by leaving the newly submitted job
disabled?

>  	scoped_guard(mutex, &core->job_lock)
>  		if (core->in_flight_job) {
> [ ... ]
>  			iommu_detach_group(NULL, iommu_group_get(core->dev));

[Severity: High]
This is a pre-existing issue, but does this leak the IOMMU group reference?

The call to iommu_group_get() increments the reference count, but
iommu_detach_group() does not consume or release it.

Since job completion is a hot path, will this quickly overflow the refcount
or leak memory? Should the driver just use the already cached
core->iommu_group instead?

> @@ -424,9 +514,22 @@ static irqreturn_t rocket_job_irq_handler(int irq, void *data)
>  {
>  	struct rocket_core *core = data;
>  	u32 raw_status = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);

[Severity: High]
This is a pre-existing issue, but can this unconditional MMIO read cause a
bus fault if the device is suspended?

Because the driver uses IRQF_SHARED, another device could trigger
rocket_job_irq_handler() while the NPU is runtime suspended and powered down.
Reading from a powered-down device typically returns garbage data or causes a
synchronous external abort.

Should we use pm_runtime_get_if_active() to verify the device is powered
before touching MMIO registers?

> @@ -486,6 +593,10 @@ int rocket_job_init(struct rocket_core *core)
> [ ... ]
>  err_sched:
>  	drm_sched_fini(&core->sched);

[Severity: High]
This is a pre-existing issue, but can calling drm_sched_fini() here cause a
kernel panic?

If drm_sched_init() fails internally (e.g., due to memory allocation failure),
the waitqueues and lists inside the scheduler structure are left uninitialized.
When we jump to the err_sched label, drm_sched_fini() will unconditionally
try to wake up these uninitialized waitqueues.

> @@ -486,6 +593,10 @@ int rocket_job_init(struct rocket_core *core)
>  
>  void rocket_job_fini(struct rocket_core *core)
>  {
> +	atomic_set(&core->poll_active, 0);
> +	hrtimer_cancel(&core->poll_timer);
> +	cancel_work_sync(&core->poll_work);
> +
>  	drm_sched_fini(&core->sched);
>  

[Severity: High]
Is there a teardown order race that allows the hrtimer to be re-armed after
cancellation?

The poll timer and workqueue are cancelled before stopping the DRM scheduler.
If a concurrent scheduler thread pulls a pending job and calls
rocket_job_hw_submit(), it will restart poll_timer.

Can this lead to a use-after-free when the timer fires on freed memory after
teardown completes? Should we stop the scheduler before cancelling the timer?

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

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

[Jiaxing Hu]

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.

Run the full RKNN clock set (CLK_RKNN_DSU0, ACLK_RKNN0/1, HCLK_RKNN_ROOT,
ACLK/HCLK_RKNN_CBUF) during the NPU0/NPU1 power transitions and on the
MMU nodes; without the functional and CBUF clocks the block powers on but
its registers (including the IOMMU banks) read back dead. Drive the RKNN
BIU reset (SRST_A_RKNN0/1_BIU) from the power domain so it fires before
the IOMMU resumes.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 arch/arm64/boot/dts/rockchip/rk3576.dtsi | 78 +++++++++++++++++++++++-
 1 file changed, 76 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
index e12a2a0cf..5033f7628 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>;
 							pm_qos = <&qos_npu_m0>;
 							#power-domain-cells = <0>;
 						};
 						power-domain@RK3576_PD_NPU1 {
 							reg = <RK3576_PD_NPU1>;
 							clocks = <&cru HCLK_RKNN_ROOT>,
-								 <&cru ACLK_RKNN1>;
+								 <&cru ACLK_RKNN1>,
+								 <&cru CLK_RKNN_DSU0>,
+								 <&cru ACLK_RKNN_CBUF>,
+								 <&cru HCLK_RKNN_CBUF>;
+							resets = <&cru SRST_A_RKNN1_BIU>;
 							pm_qos = <&qos_npu_m1>;
 							#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>;
+			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>;
+			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>;
+			#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";
+			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>;
+			interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cru ACLK_RKNN1>, <&cru HCLK_RKNN_ROOT>,
+				 <&cru CLK_RKNN_DSU0>, <&cru ACLK_RKNN_CBUF>,
+				 <&cru HCLK_RKNN_CBUF>;
+			#iommu-cells = <0>;
+			power-domains = <&power RK3576_PD_NPU1>;
+			status = "disabled";
+		};
+
 		gmac0: ethernet@2a220000 {
 			compatible = "rockchip,rk3576-gmac", "snps,dwmac-4.20a";
 			reg = <0x0 0x2a220000 0x0 0x10000>;
-- 
2.43.0

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

[sashiko-bot]

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

[RFC PATCH v2 8/8] arm64: dts: rockchip: rk3576-rock-4d: enable NPU

[Jiaxing Hu]

Enable rknn_core_0 and rknn_mmu_0 on the Radxa ROCK 4D and supply the
NPU rail (vdd_npu_s0) via npu-supply.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
index 272af1012..09d2fc98b 100644
--- a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
+++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
@@ -442,6 +442,7 @@ regulator-state-mem {
 			};
 
 			vdd_npu_s0: dcdc-reg2 {
+				regulator-always-on;
 				regulator-boot-on;
 				regulator-enable-ramp-delay = <400>;
 				regulator-min-microvolt = <550000>;
@@ -869,3 +870,18 @@ vp0_out_hdmi: endpoint@ROCKCHIP_VOP2_EP_HDMI0 {
 		remote-endpoint = <&hdmi_in_vp0>;
 	};
 };
+
+&rknn_core_0 {
+	npu-supply = <&vdd_npu_s0>;
+	/*
+	 * Power BOTH NPU domains (NPU0 + NPU1) like the vendor's single NPU node,
+	 * even though rocket computes only on core 0: the CBUF->CMAC read path is
+	 * only fully powered with NPU1 up. rocket attaches the multi-PD list.
+	 */
+	power-domains = <&power RK3576_PD_NPU0>, <&power RK3576_PD_NPU1>;
+	status = "okay";
+};
+
+&rknn_mmu_0 {
+	status = "okay";
+};
-- 
2.43.0

Re: [RFC PATCH v2 8/8] arm64: dts: rockchip: rk3576-rock-4d: enable NPU

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] An SoC-level power domain dependency is improperly implemented as a board-level override in `rk3576-rock-4d.dts`.
- [Medium] The device tree assigns two power domains to `rknn_core_0`, which violates the DT binding schema's `maxItems: 1` constraint.
- [Medium] The NPU regulator is forced permanently on via `regulator-always-on`, circumventing proper power management, and the `npu-supply` property is functionally dead code.
--

--- Patch [8]: [RFC PATCH v2 8/8] arm64: dts: rockchip: rk3576-rock-4d: enable NPU ---
commit 1ea2a06c9849a71ed6c8698699f42f0e551d0d72
Author: Jiaxing Hu <gahing@gahingwoo.com>

arm64: dts: rockchip: rk3576-rock-4d: enable NPU
    
Enable rknn_core_0 and rknn_mmu_0 on the Radxa ROCK 4D and supply the
NPU rail (vdd_npu_s0) via npu-supply.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> index 272af1012ab03..09d2fc98bb4c1 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> +++ b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> @@ -442,6 +442,7 @@ regulator-state-mem {
>  			};
>  
>  			vdd_npu_s0: dcdc-reg2 {
> +				regulator-always-on;

[Severity: Medium]
Does this unconditionally force the regulator on, completely defeating runtime
power management and causing a permanent power leak when the NPU is idle?

>  				regulator-boot-on;
>  				regulator-enable-ramp-delay = <400>;
>  				regulator-min-microvolt = <550000>;

[ ... ]

> +
> +&rknn_core_0 {
> +	npu-supply = <&vdd_npu_s0>;

[Severity: Medium]
The commit message mentions supplying the NPU rail via npu-supply, but is this
property functionally dead code here?

The rocket driver does not appear to make any regulator API calls, and there
is no OPP table present to trigger generic devfreq parsing.

> +	/*
> +	 * Power BOTH NPU domains (NPU0 + NPU1) like the vendor's single NPU node,
> +	 * even though rocket computes only on core 0: the CBUF->CMAC read path is
> +	 * only fully powered with NPU1 up. rocket attaches the multi-PD list.
> +	 */
> +	power-domains = <&power RK3576_PD_NPU0>, <&power RK3576_PD_NPU1>;

[Severity: Medium]
Will this assignment of two power domains fail dtbs_check validation?

Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
explicitly enforces maxItems: 1 for power-domains.

[Severity: High]
Should this power domain dependency be placed in the SoC's .dtsi file instead
of a board-specific override?

Since the CBUF->CMAC read path hardware dependency is a property of the RK3576
silicon rather than board wiring, keeping it in the board DTS means any other
RK3576 board enabling rknn_core_0 via the base rk3576.dtsi might silently lack
this dependency, leading to hardware hangs or corrupted reads.

> +	status = "okay";
> +};
> +
> +&rknn_mmu_0 {
> +	status = "okay";
> +};

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