Devicetree
 help / color / mirror / Atom feed
* [RFC PATCH v4 0/6] accel/rocket: RK3576 NPU (RKNN) enablement
@ 2026-08-03  9:41 Jiaxing Hu
  2026-08-03  9:41 ` [RFC PATCH v4 1/6] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core Jiaxing Hu
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Jiaxing Hu @ 2026-08-03  9:41 UTC (permalink / raw)
  To: tomeu, heiko, robh, krzk+dt, conor+dt, ulfh, p.zabel, ogabbay
  Cc: royalnet026, alchark, chaoyi.chen, krzk, will, dri-devel,
	linux-rockchip, linux-pm, devicetree, linux-arm-kernel,
	linux-kernel, Jiaxing Hu

This is a fixes only revision. Nothing here changes what the NPU does; it
is six bugs found in v3, five of them by the Sashiko review bot and each
one checked against the vendor DT, the vendor driver or the hardware
before being believed.

Tested on a Radxa ROCK 4D, on next-20260730.

Changes in v4
-------------

  * rk3576.dtsi: rknn_core_1 was at the wrong address. The vendor node is
    reg = <0x27700000 0x8000>, <0x27708000 0x8000> and its driver takes
    base[i] straight from those, so core 1 lives at 0x27708000, not
    0x27710000. rknn_mmu_1 at 0x2770a000 was consistent with the vendor
    layout all along.

  * rk3576.dtsi: both cores carried five reg entries including dpu and
    dpu_rdma, which the binding does not allow and the driver does not
    map. Cut to the three the binding defines. I had validated the
    binding itself in v3 but never ran dtbs_check against it.

  * rk3576.dtsi: rknn_core_1 was missing the CBUF clocks, so it could
    never have probed on RK3576, where the driver asks for six by name.

  * rk3576.dtsi: each core now lists both NPU power domains. With one
    domain the driver core auto-attaches it and
    devm_pm_domain_attach_list() then returns -EEXIST, so the driver
    could only ever have worked on a board that overrode this, which is
    exactly what rk3576-rock-4d.dts was doing. The board override is
    dropped. The IOMMUs keep a single domain each, since they rely on
    that same auto-attach.

  * accel/rocket: rocket_job_fini() did not cancel the completion poll
    timer or its work, so unbind could leave them running against freed
    memory.

  * accel/rocket: a poll work already queued when the interrupt lands
    could finalise the next job as well. It now carries the sequence
    number of the job it was started for.

Verified on hardware: the NPU probes with the two domain list, a known
byte exact convolution stays byte exact six times over, and an
unbind/rebind cycle rebinds cleanly and runs again with no warning.

Igor Paunovic gave a Tested-by on the v3 driver patch. I have not
carried it over, since patch 4 changed after it. Igor, the changes are
in the poll_completion path and in rocket_job_fini, so RK3588 never
reaches either, but it is your tag to give.

What is still wrong
-------------------

Unchanged from v3, and the search has narrowed rather than moved.

A single convolution is byte exact, and re-running that same one is byte
exact every time. What fails is running a different one after it: the
second configuration computes nothing and writes out a zero point
surface, while the one already resident keeps working. Going back to it
is byte exact again.

Tomeu suggested this looked like the ping-pong register bank never
switching, which fits, and the readback agrees that the pointer is stuck:
we write S_POINTER bit 0 as 0 and it reads back 1, on every job, for the
rest of the session. But the driver cannot move it. Flipping bit 0 per
submit, in the direct writes and in all four regcmd entries, changes
neither the readback nor the result. Selecting a bank the way
rk3576_state_init does, with the PP bits cleared, stops the units arming
at all. Pulsing POINTER_PP_CLEAR, with or without EXECUTER_PP_CLEAR,
moves nothing. The vendor does not switch banks per submit either; it
writes 0xe exactly as we do, and only does the 0, 1, 0x1e dance once per
reset. Adding that sequence verbatim, at the same point the vendor calls
it, changes nothing.

A read snapshot of every block the driver can reach, pc, cna, core, dpu
and rdma, 20 KB in total, taken at the same point in a job that computed
and one that did not, differs in exactly one word, and that word is
OPERATION_ENABLE. At completion the register state carries no trace of
which job worked.

So it is not the register writes (both drivers enumerated), not the
regcmd payload (vendor bytes replayed through rocket still fail), not the
register state at completion, not the ping-pong controls, and not
clocks, genpd, IOMMU, cache or resets. The vendor computes different
configurations correctly on this silicon and this kernel, so a difference
exists and it is somewhere none of that reaches. Suggestions very
welcome.

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

Jiaxing Hu (6):
  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
  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        |  15 +-
 .../boot/dts/rockchip/rk3576-rock-4d.dts      |  10 +
 arch/arm64/boot/dts/rockchip/rk3576.dtsi      |  76 ++++-
 drivers/accel/rocket/rocket_core.c            |  53 ++-
 drivers/accel/rocket/rocket_core.h            |  21 +-
 drivers/accel/rocket/rocket_device.c          |   4 +
 drivers/accel/rocket/rocket_drv.c             |  25 +-
 drivers/accel/rocket/rocket_job.c             | 306 ++++++++++++++++++
 drivers/pmdomain/rockchip/pm-domains.c        |  71 ++--
 9 files changed, 550 insertions(+), 31 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC PATCH v4 1/6] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core
  2026-08-03  9:41 [RFC PATCH v4 0/6] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
@ 2026-08-03  9:41 ` Jiaxing Hu
  2026-08-03  9:41 ` [RFC PATCH v4 2/6] pmdomain/rockchip: add optional per-domain power-on settle delay Jiaxing Hu
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Jiaxing Hu @ 2026-08-03  9:41 UTC (permalink / raw)
  To: tomeu, heiko, robh, krzk+dt, conor+dt, ulfh, p.zabel, ogabbay
  Cc: royalnet026, alchark, chaoyi.chen, krzk, will, dri-devel,
	linux-rockchip, linux-pm, devicetree, linux-arm-kernel,
	linux-kernel, Jiaxing Hu

The RK3576 NPU has two cores of the same RKNN block the RK3588 binding
already describes, but no NPU SRAM supply, so sram-supply is required
for rockchip,rk3588-rknn-core only and rejected otherwise.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 .../bindings/npu/rockchip,rk3588-rknn-core.yaml   | 15 ++++++++++++++-
 1 file changed, 14 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..0a7baac24 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,19 @@ required:
   - resets
   - reset-names
   - npu-supply
-  - sram-supply
+
+allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: rockchip,rk3588-rknn-core
+    then:
+      required:
+        - sram-supply
+    else:
+      properties:
+        sram-supply: false
 
 additionalProperties: false
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH v4 2/6] pmdomain/rockchip: add optional per-domain power-on settle delay
  2026-08-03  9:41 [RFC PATCH v4 0/6] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
  2026-08-03  9:41 ` [RFC PATCH v4 1/6] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core Jiaxing Hu
@ 2026-08-03  9:41 ` Jiaxing Hu
  2026-08-03  9:41 ` [RFC PATCH v4 3/6] pmdomain/rockchip: cycle optional power-domain resets on power-on Jiaxing Hu
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Jiaxing Hu @ 2026-08-03  9:41 UTC (permalink / raw)
  To: tomeu, heiko, robh, krzk+dt, conor+dt, ulfh, p.zabel, ogabbay
  Cc: royalnet026, alchark, chaoyi.chen, krzk, will, dri-devel,
	linux-rockchip, linux-pm, devicetree, linux-arm-kernel,
	linux-kernel, Jiaxing Hu

The RK3576 NPU domains need a short settle time after the idle request
is released before the QoS registers behind the domain answer. Without
it rockchip_pmu_restore_qos() reads back zeroes, and the NPU throws an
async SError on the first cold power-on.

Give rockchip_domain_info an optional delay_us and wait for it between
releasing idle and restoring QoS. Rename DOMAIN_M_O_R_G to
DOMAIN_M_O_R_G_W, since the suffixes name the fields the macro sets and
this one now also carries a wakeup delay; RK3576 is its only user, so
the old spelling is not kept around.

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

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index ba66ae719..e1857f878 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/delay.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/mfd/syscon.h>
@@ -59,6 +60,7 @@ struct rockchip_domain_info {
 	u32 pwr_offset;
 	u32 mem_offset;
 	u32 req_offset;
+	u32 delay_us;
 };
 
 struct rockchip_pmu_info {
@@ -185,7 +187,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_W(_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 +202,7 @@ struct rockchip_pmu {
 	.req_mask = (req),				\
 	.idle_mask = (idle),				\
 	.clk_ungate_mask = (g_mask),			\
+	.delay_us = (delay),				\
 	.ack_mask = (ack),				\
 	.active_wakeup = wakeup,			\
 }
@@ -258,8 +261,8 @@ struct rockchip_pmu {
 #define DOMAIN_RK3568(name, pwr, req, wakeup, regulator)		\
 	DOMAIN_M_R(name, pwr, pwr, req, req, req, wakeup, regulator)
 
-#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_W(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
@@ -681,6 +684,10 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
 		if (ret < 0)
 			goto out;
 
+		/* Some domains need to settle before the QoS registers answer. */
+		if (pd->info->delay_us)
+			udelay(pd->info->delay_us);
+
 		rockchip_pmu_restore_qos(pd);
 	}
 
@@ -1300,25 +1307,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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH v4 3/6] pmdomain/rockchip: cycle optional power-domain resets on power-on
  2026-08-03  9:41 [RFC PATCH v4 0/6] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
  2026-08-03  9:41 ` [RFC PATCH v4 1/6] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core Jiaxing Hu
  2026-08-03  9:41 ` [RFC PATCH v4 2/6] pmdomain/rockchip: add optional per-domain power-on settle delay Jiaxing Hu
@ 2026-08-03  9:41 ` Jiaxing Hu
  2026-08-03 10:00   ` sashiko-bot
  2026-08-03  9:41 ` [RFC PATCH v4 4/6] accel/rocket: add RK3576 NPU (RKNN) support Jiaxing Hu
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Jiaxing Hu @ 2026-08-03  9:41 UTC (permalink / raw)
  To: tomeu, heiko, robh, krzk+dt, conor+dt, ulfh, p.zabel, ogabbay
  Cc: royalnet026, alchark, chaoyi.chen, krzk, will, dri-devel,
	linux-rockchip, linux-pm, devicetree, linux-arm-kernel,
	linux-kernel, Jiaxing Hu

Some Rockchip domains come out of power-on with their bus interface in
an undefined state. On the RK3576 NPU this shows up as a hang on the
first access after a cold power-on; the vendor kernel cycles the
domain's resets at this point and that clears it.

Take the domain node's resets if it has any, and pulse them between
releasing idle and restoring QoS. The resets are optional, so domains
that do not declare any behave exactly as before.

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

diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
index e1857f878..82051f600 100644
--- a/drivers/pmdomain/rockchip/pm-domains.c
+++ b/drivers/pmdomain/rockchip/pm-domains.c
@@ -19,6 +19,7 @@
 #include <linux/of_clk.h>
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/reset.h>
 #include <linux/regmap.h>
 #include <linux/regulator/consumer.h>
 #include <linux/mfd/syscon.h>
@@ -103,6 +104,7 @@ struct rockchip_pm_domain {
 	struct clk_bulk_data *clks;
 	struct device_node *node;
 	struct regulator *supply;
+	struct reset_control *resets;
 };
 
 struct rockchip_pmu {
@@ -688,6 +690,13 @@ static int rockchip_pd_power(struct rockchip_pm_domain *pd, bool power_on)
 		if (pd->info->delay_us)
 			udelay(pd->info->delay_us);
 
+		/* Optional: some domains need their resets cycled after power-on. */
+		if (pd->resets) {
+			reset_control_assert(pd->resets);
+			udelay(10);
+			reset_control_deassert(pd->resets);
+		}
+
 		rockchip_pmu_restore_qos(pd);
 	}
 
@@ -857,6 +866,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);
 
@@ -927,6 +944,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;
 }
 
@@ -945,6 +963,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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH v4 4/6] accel/rocket: add RK3576 NPU (RKNN) support
  2026-08-03  9:41 [RFC PATCH v4 0/6] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
                   ` (2 preceding siblings ...)
  2026-08-03  9:41 ` [RFC PATCH v4 3/6] pmdomain/rockchip: cycle optional power-domain resets on power-on Jiaxing Hu
@ 2026-08-03  9:41 ` Jiaxing Hu
  2026-08-03  9:56   ` sashiko-bot
  2026-08-03 15:44   ` Igor Paunovic
  2026-08-03  9:41 ` [RFC PATCH v4 5/6] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes Jiaxing Hu
  2026-08-03  9:41 ` [RFC PATCH v4 6/6] arm64: dts: rockchip: rk3576-rock-4d: enable NPU Jiaxing Hu
  5 siblings, 2 replies; 13+ messages in thread
From: Jiaxing Hu @ 2026-08-03  9:41 UTC (permalink / raw)
  To: tomeu, heiko, robh, krzk+dt, conor+dt, ulfh, p.zabel, ogabbay
  Cc: royalnet026, alchark, chaoyi.chen, krzk, will, dri-devel,
	linux-rockchip, linux-pm, devicetree, linux-arm-kernel,
	linux-kernel, Jiaxing Hu

The RK3576 carries the same RKNN block as the RK3588, with two cores
instead of three and a few platform differences:

 - the CBUF (convolution buffer) has its own clock domain, so the core
   needs six clocks rather than four;
 - the BIU reset moved into the power domain, leaving one reset here;
 - the NPU spans two power domains, and a device with more than one is
   skipped by the driver-core single-domain auto-attach, so the list has
   to be attached explicitly;
 - the DPU completion interrupt is armed exactly as on RK3588 but never
   reaches the GIC. The completion is visible in INTERRUPT_RAW_STATUS,
   so sample that from an hrtimer rather than wait for an interrupt that
   does not come. The interrupt stays armed, so if it ever does arrive
   the normal handler finalises the job first.

Select all of that from of_device_id match data so the RK3588 path keeps
its existing counts and behaviour unchanged.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 drivers/accel/rocket/rocket_core.c   |  53 ++++-
 drivers/accel/rocket/rocket_core.h   |  21 +-
 drivers/accel/rocket/rocket_device.c |   4 +
 drivers/accel/rocket/rocket_drv.c    |  25 ++-
 drivers/accel/rocket/rocket_job.c    | 306 +++++++++++++++++++++++++++
 5 files changed, 403 insertions(+), 6 deletions(-)

diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
index b3b2fa9ba..5738fccdb 100644
--- a/drivers/accel/rocket/rocket_core.c
+++ b/drivers/accel/rocket/rocket_core.c
@@ -8,12 +8,40 @@
 #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>
 
 #include "rocket_core.h"
 #include "rocket_job.h"
 
+/*
+ * The vendor's rk3576_state_init, which rocket has no equivalent of. It runs at
+ * probe and after every reset, and it is the only place either driver selects a
+ * ping-pong bank: S_POINTER 0, write DATA_SIZE1, S_POINTER 1, write DATA_SIZE1
+ * again, then arm with 0x1e. So the vendor initialises BOTH banks once per
+ * reset and then leaves the pointer alone, exactly like we do per submit.
+ *
+ * We never initialise bank 1 at all, which fits what the board shows: the first
+ * configuration computes and a second, different one does not.
+ */
+int rocket_state_init = 1;
+module_param_named(state_init, rocket_state_init, int, 0644);
+MODULE_PARM_DESC(state_init, "Run the vendor's state_init on power-up (default 1)");
+
+void rocket_core_state_init(struct rocket_core *core)
+{
+	if (!rocket_state_init)
+		return;
+
+	rocket_pc_writel(core, BASE_ADDRESS, 0x1);
+	rocket_cna_writel(core, S_POINTER, 0);
+	rocket_cna_writel(core, DATA_SIZE1, 0x80000000);
+	rocket_cna_writel(core, S_POINTER, 1);
+	rocket_cna_writel(core, DATA_SIZE1, 0x80000000);
+	rocket_cna_writel(core, S_POINTER, 0x1e);
+}
+
 int rocket_core_init(struct rocket_core *core)
 {
 	struct device *dev = core->dev;
@@ -21,14 +49,22 @@ int rocket_core_init(struct rocket_core *core)
 	u32 version;
 	int err = 0;
 
+	/* RK3576 moves the BIU reset into its power domain and takes only srst_a. */
 	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 clocks the CBUF separately; the compute path stalls without these. */
+	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 +101,19 @@ int rocket_core_init(struct rocket_core *core)
 		return err;
 	}
 
+	/*
+	 * RK3576 spans two power domains, and a multi-domain device is skipped
+	 * by the driver-core single-domain auto-attach, so attach the list here.
+	 */
+	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..e4cc11336 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,25 @@
 #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. */
+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;		/* completion IRQ never reaches the GIC */
+};
+
 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 +62,14 @@ struct rocket_core {
 		atomic_t pending;
 	} reset;
 
+	struct hrtimer poll_timer;
+	struct work_struct poll_work;
+	atomic_t poll_active;
+	unsigned int poll_ticks;
+	unsigned int poll_seq;
+	unsigned int poll_work_seq;
+	unsigned int sptr_bank;
+
 	struct drm_gpu_scheduler sched;
 	u64 fence_context;
 	u64 emit_seqno;
@@ -60,5 +78,6 @@ struct rocket_core {
 int rocket_core_init(struct rocket_core *core);
 void rocket_core_fini(struct rocket_core *core);
 void rocket_core_reset(struct rocket_core *core);
+void rocket_core_state_init(struct rocket_core *core);
 
 #endif
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..95599e791 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,12 +256,15 @@ 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;
 	}
 
+	/* Vendor runs its state_init once per power-up; we never did. */
+	rocket_core_state_init(&rdev->cores[core]);
+
 	return 0;
 }
 
@@ -260,7 +279,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 bb77b6bf0..c21310ef7 100644
--- a/drivers/accel/rocket/rocket_job.c
+++ b/drivers/accel/rocket/rocket_job.c
@@ -7,6 +7,8 @@
 #include <drm/drm_file.h>
 #include <drm/drm_gem.h>
 #include <drm/rocket_accel.h>
+#include <linux/dma-map-ops.h>
+#include <linux/hrtimer.h>
 #include <linux/interrupt.h>
 #include <linux/overflow.h>
 #include <linux/iommu.h>
@@ -21,6 +23,216 @@
 
 #define JOB_TIMEOUT_MS 500
 
+/*
+ * RK3576 arms the same DPU completion as RK3588, but the interrupt never
+ * reaches the GIC. The completion itself is visible in INTERRUPT_RAW_STATUS,
+ * so sample that instead. The tick cap bounds jobs that never raise it at all,
+ * which is the same open problem as the wrong inference results.
+ */
+/*
+ * EXPERIMENT (not for upstream), 2026-08-01, following Tomeu's suggestion that
+ * the block is stuck on ping-pong bank 0.
+ *
+ * S_POINTER bit 0 (CNA_S_POINTER_POINTER) selects which register bank the
+ * writes that follow land in. The vendor's rk3576_state_init programs both
+ * banks by writing S_POINTER=0 then S_POINTER=1 around the same register.
+ * rocket writes 0xe (PP_EN|EXECUTER_PP_EN|PP_MODE, bit 0 clear) both directly
+ * in hw_submit and, via mesa, four times inside every regcmd, so the pointer
+ * never leaves bank 0.
+ *
+ * That matches what the board does: re-running one configuration is byte exact
+ * forever, and loading a different one computes nothing until a reset. If the
+ * block only reloads its configuration when the bank flips, nothing after the
+ * first load is ever picked up.
+ *
+ * sptr_alt=1 flips bit 0 per submit, in the direct writes and in the regcmd
+ * (which would otherwise overwrite them during replay).
+ */
+static int rocket_sptr_alt;
+module_param_named(sptr_alt, rocket_sptr_alt, int, 0644);
+MODULE_PARM_DESC(sptr_alt, "S_POINTER bank: 0=off, 1=flip bit 0, 2=vendor style bare bank select");
+
+/*
+ * Readback said the POINTER field is not ours to write: with PP_MODE set we
+ * write bit 0 as 0 and it reads back 1, flipping it changes nothing, and
+ * clearing the PP bits to write a bare bank stops the units arming at all
+ * (EXECUTER never latches, and even the model that normally works fails).
+ * So the pointer is stuck at 1 and the driver cannot steer it directly.
+ *
+ * The one thing the vendor does that we never do is pulse POINTER_PP_CLEAR.
+ * Its rk3576_state_init ends with 0x1e, which is our 0xe plus bit 4, and we
+ * only ever write that once at power on. If the pointer is released by the
+ * clear rather than by a write to bit 0, that would fit the shape of this
+ * bug exactly: the first configuration lands and nothing after it does.
+ *
+ * pp_clear=1 pulses it on CNA and CORE before each submit.
+ * pp_clear=2 also sets EXECUTER_PP_CLEAR (bit 5).
+ */
+/*
+ * Read snapshot of the register blocks the driver maps, taken at the same point
+ * in two jobs and diffed. Everything done so far has been a writel audit, which
+ * by construction cannot see a bit the hardware sets and the driver never
+ * writes. That is the class INTERRUPT_MASK bit 31 turned out to belong to.
+ *
+ * pc, cna and core come through the driver's own mappings. DPU and RDMA are not
+ * mapped by rocket, so they get their own ioremap: the vendor DT covers the
+ * whole core as one 32 KB range and its driver reads 0x4000/0x4004/0x4008/0x4018
+ * and 0x5000/0x5004/0x5008, so both blocks are decoded and safe.
+ *
+ * What is NOT safe is 0x27702000. Mainline splits it out as rknn_mmu_0 and
+ * rk_iommu owns it; a first attempt swept the range as one contiguous block,
+ * mapped over it and wedged the board with RCU stalls. Skip it.
+ */
+static int rocket_snap;
+module_param_named(snap, rocket_snap, int, 0644);
+MODULE_PARM_DESC(snap, "Snapshot pc/cna/core per job and diff job 0 against later ones");
+
+#define ROCKET_SNAP_BLK   0x1000
+#define ROCKET_SNAP_NBLK  5
+#define ROCKET_SNAP_WORDS (ROCKET_SNAP_NBLK * ROCKET_SNAP_BLK / 4)
+#define ROCKET_SNAP_DPU_PHYS  0x27704000UL	/* dpu + rdma, 2 blocks */
+
+static u32 *rocket_snap_buf[2];
+static unsigned int rocket_snap_n;
+static void __iomem *rocket_snap_dpu;
+
+static void rocket_snap_take(struct rocket_core *core)
+{
+	void __iomem *blk[ROCKET_SNAP_NBLK];
+	unsigned int slot, b, i, w = 0, diffs = 0;
+	u32 *cur;
+
+	if (!rocket_snap)
+		return;
+
+	if (!rocket_snap_buf[0]) {
+		rocket_snap_buf[0] = kmalloc_array(ROCKET_SNAP_WORDS, 4, GFP_KERNEL);
+		rocket_snap_buf[1] = kmalloc_array(ROCKET_SNAP_WORDS, 4, GFP_KERNEL);
+		if (!rocket_snap_buf[0] || !rocket_snap_buf[1])
+			return;
+	}
+
+	if (!rocket_snap_dpu)
+		rocket_snap_dpu = ioremap(ROCKET_SNAP_DPU_PHYS, 2 * ROCKET_SNAP_BLK);
+	if (!rocket_snap_dpu)
+		return;
+
+	blk[0] = core->pc_iomem;
+	blk[1] = core->cna_iomem;
+	blk[2] = core->core_iomem;
+	blk[3] = rocket_snap_dpu;
+	blk[4] = rocket_snap_dpu + ROCKET_SNAP_BLK;
+
+	slot = rocket_snap_n ? 1 : 0;
+	cur = rocket_snap_buf[slot];
+	for (b = 0; b < ROCKET_SNAP_NBLK; b++)
+		for (i = 0; i < ROCKET_SNAP_BLK; i += 4)
+			cur[w++] = readl(blk[b] + i);
+
+	if (rocket_snap_n) {
+		static const char * const name[ROCKET_SNAP_NBLK] = {
+			"pc", "cna", "core", "dpu", "rdma"
+		};
+		static const u32 base[ROCKET_SNAP_NBLK] = {
+			0x0000, 0x1000, 0x3000, 0x4000, 0x5000
+		};
+
+		for (i = 0; i < ROCKET_SNAP_WORDS; i++) {
+			if (rocket_snap_buf[0][i] == cur[i])
+				continue;
+			if (++diffs > 48)
+				break;
+			b = i / (ROCKET_SNAP_BLK / 4);
+			dev_info(core->dev, "snap diff %s+0x%04x: job0=%08x job%u=%08x\n",
+				 name[b],
+				 base[b] + (i % (ROCKET_SNAP_BLK / 4)) * 4,
+				 rocket_snap_buf[0][i], rocket_snap_n, cur[i]);
+		}
+		dev_info(core->dev, "snap: job%u differs from job0 in %u words%s\n",
+			 rocket_snap_n, diffs, diffs > 48 ? " (truncated)" : "");
+	} else {
+		dev_info(core->dev, "snap: job0 baseline captured\n");
+	}
+	rocket_snap_n++;
+}
+
+static int rocket_pp_clear;
+module_param_named(pp_clear, rocket_pp_clear, int, 0644);
+MODULE_PARM_DESC(pp_clear, "Pulse POINTER_PP_CLEAR before each submit (0=off, 1=pointer, 2=pointer+executer)");
+
+/*
+ * Round 1 of the ping-pong experiment flipped bit 0 per submit and changed
+ * nothing at all: the second configuration still computed nothing and the
+ * repeated same-configuration case still worked. That only rules out the
+ * simplest reading though. rocket and mesa both write 0xe, which has
+ * POINTER_PP_EN, EXECUTER_PP_EN and POINTER_PP_MODE set, while the vendor's
+ * rk3576_state_init selects a bank with a BARE 0 or 1, all those bits clear.
+ * If PP_MODE means the hardware owns the pointer, our bit 0 was a don't care
+ * and the null result says nothing.
+ *
+ * So before trying anything else, read the register back and see whether the
+ * field is even live: what it holds after our write, and whether it moves on
+ * its own across a job.
+ */
+static int rocket_sptr_dbg;
+module_param_named(sptr_dbg, rocket_sptr_dbg, int, 0644);
+MODULE_PARM_DESC(sptr_dbg, "Log S_POINTER of all four units at submit and at completion");
+
+static void rocket_sptr_dump(struct rocket_core *core, const char *tag)
+{
+	if (!rocket_sptr_dbg)
+		return;
+
+	/*
+	 * Only CNA and CORE are mapped (reg-names are pc, cna, core), and their
+	 * offsets are relative to each block's own base, which is what the
+	 * rocket_*_readl macros do. Reading them off pc_iomem faults.
+	 */
+	dev_info(core->dev, "sptr %s: cna=%08x core=%08x\n", tag,
+		 rocket_cna_readl(core, S_POINTER),
+		 rocket_core_readl(core, S_POINTER));
+}
+
+static void rocket_sptr_patch_regcmd(struct rocket_core *core, struct rocket_job *job,
+				     struct rocket_task *task, u32 bank)
+{
+	phys_addr_t phys = iommu_iova_to_phys(job->domain->domain, task->regcmd);
+	unsigned int i, patched = 0;
+	struct scatterlist sg;
+	struct page *page;
+	u64 *rv;
+
+	if (!phys || !pfn_valid(PFN_DOWN(phys)))
+		return;
+
+	rv = phys_to_virt(phys);
+	for (i = 0; i < task->regcmd_count; i++) {
+		u16 reg = rv[i] & 0xffff;
+		u32 val;
+
+		if (reg != 0x1004 && reg != 0x3004 && reg != 0x4004 && reg != 0x5004)
+			continue;
+
+		val = (u32)((rv[i] >> 16) & 0xffffffffu);
+		if (rocket_sptr_alt == 2)
+			val = bank;		/* bare, like rk3576_state_init */
+		else
+			val = (val & ~1u) | bank;
+		rv[i] = (rv[i] & ~(0xffffffffULL << 16)) | ((u64)val << 16);
+		patched++;
+	}
+
+	page = pfn_to_page(PFN_DOWN(phys));
+	sg_init_table(&sg, 1);
+	sg_set_page(&sg, page, PAGE_SIZE, 0);
+	dma_sync_sg_for_device(core->dev, &sg, 1, DMA_TO_DEVICE);
+
+	dev_info(core->dev, "sptr_alt: bank=%u patched %u S_POINTER entries\n", bank, patched);
+}
+
+#define RK3576_POLL_INTERVAL_NS	1000000LL	/* 1 ms */
+#define RK3576_POLL_MAX_TICKS	8
+
 static struct rocket_job *
 to_rocket_job(struct drm_sched_job *sched_job)
 {
@@ -111,6 +323,7 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
 {
 	struct rocket_task *task;
 	unsigned int extra_bit;
+	u32 bank = 0;
 
 	/* Don't queue the job if a reset is in progress */
 	if (atomic_read(&core->reset.pending))
@@ -125,6 +338,12 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
 
 	 /* From rknpu, in the TRM this bit is marked as reserved */
 	extra_bit = 0x10000000 * core->index;
+
+	if (rocket_sptr_alt) {
+		bank = core->sptr_bank & 1;
+		core->sptr_bank++;
+		extra_bit |= bank;
+	}
 	rocket_cna_writel(core, S_POINTER, CNA_S_POINTER_POINTER_PP_EN(1) |
 					   CNA_S_POINTER_EXECUTER_PP_EN(1) |
 					   CNA_S_POINTER_POINTER_PP_MODE(1) |
@@ -135,6 +354,9 @@ static void rocket_job_hw_submit(struct rocket_core *core, struct rocket_job *jo
 					    CORE_S_POINTER_POINTER_PP_MODE(1) |
 					    extra_bit);
 
+	if (rocket_sptr_alt)
+		rocket_sptr_patch_regcmd(core, job, task, bank);
+
 	rocket_pc_writel(core, BASE_ADDRESS, task->regcmd);
 	rocket_pc_writel(core, REGISTER_AMOUNTS,
 			 PC_REGISTER_AMOUNTS_PC_DATA_AMOUNT((task->regcmd_count + 1) / 2 - 1));
@@ -151,6 +373,33 @@ 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));
 
+	if (core->soc->poll_completion) {
+		core->poll_ticks = 0;
+		core->poll_seq++;
+		atomic_set(&core->poll_active, 1);
+		hrtimer_start(&core->poll_timer, ns_to_ktime(RK3576_POLL_INTERVAL_NS),
+			      HRTIMER_MODE_REL);
+	}
+
+	if (rocket_pp_clear) {
+		u32 base = CNA_S_POINTER_POINTER_PP_EN(1) |
+			   CNA_S_POINTER_EXECUTER_PP_EN(1) |
+			   CNA_S_POINTER_POINTER_PP_MODE(1) | extra_bit;
+		u32 clr = base | CNA_S_POINTER_POINTER_PP_CLEAR(1);
+
+		if (rocket_pp_clear == 2)
+			clr |= CNA_S_POINTER_EXECUTER_PP_CLEAR(1);
+
+		rocket_sptr_dump(core, "pre-clear ");
+		rocket_cna_writel(core, S_POINTER, clr);
+		rocket_core_writel(core, S_POINTER, clr);
+		rocket_sptr_dump(core, "post-clear");
+		rocket_cna_writel(core, S_POINTER, base);
+		rocket_core_writel(core, S_POINTER, base);
+	}
+
+	rocket_sptr_dump(core, "at-kick   ");
+
 	dev_dbg(core->dev, "Submitted regcmd at 0x%llx to core %d", task->regcmd, core->index);
 }
 
@@ -341,8 +590,55 @@ static struct dma_fence *rocket_job_run(struct drm_sched_job *sched_job)
 	return ERR_PTR(ret);
 }
 
+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);
+	u32 raw;
+
+	if (!atomic_read(&core->poll_active))
+		return HRTIMER_NORESTART;
+
+	core->poll_work_seq = core->poll_seq;
+
+	raw = rocket_pc_readl(core, INTERRUPT_RAW_STATUS);
+	if ((raw & (PC_INTERRUPT_RAW_STATUS_DPU_0 | PC_INTERRUPT_RAW_STATUS_DPU_1)) ||
+	    ++core->poll_ticks >= RK3576_POLL_MAX_TICKS) {
+		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);
+
+	/*
+	 * The interrupt can land while this work is already queued, finalise the
+	 * job and let the next one start. Without this the stale work would then
+	 * finalise that new job as well.
+	 */
+	if (core->poll_work_seq != core->poll_seq)
+		return;
+
+	rocket_job_handle_irq(core);
+}
+
 static void rocket_job_handle_irq(struct rocket_core *core)
 {
+	if (core->soc->poll_completion) {
+		atomic_set(&core->poll_active, 0);
+		hrtimer_cancel(&core->poll_timer);
+	}
+
+	rocket_sptr_dump(core, "at-done   ");
+	rocket_snap_take(core);
+
 	pm_runtime_mark_last_busy(core->dev);
 
 	rocket_pc_writel(core, OPERATION_ENABLE, 0x0);
@@ -460,6 +756,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);
 
@@ -503,6 +803,12 @@ void rocket_job_fini(struct rocket_core *core)
 {
 	drm_sched_fini(&core->sched);
 
+	if (core->soc->poll_completion) {
+		atomic_set(&core->poll_active, 0);
+		hrtimer_cancel(&core->poll_timer);
+		cancel_work_sync(&core->poll_work);
+	}
+
 	cancel_work_sync(&core->reset.work);
 	destroy_workqueue(core->reset.wq);
 }
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH v4 5/6] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes
  2026-08-03  9:41 [RFC PATCH v4 0/6] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
                   ` (3 preceding siblings ...)
  2026-08-03  9:41 ` [RFC PATCH v4 4/6] accel/rocket: add RK3576 NPU (RKNN) support Jiaxing Hu
@ 2026-08-03  9:41 ` Jiaxing Hu
  2026-08-03  9:56   ` sashiko-bot
  2026-08-03 16:05   ` Igor Paunovic
  2026-08-03  9:41 ` [RFC PATCH v4 6/6] arm64: dts: rockchip: rk3576-rock-4d: enable NPU Jiaxing Hu
  5 siblings, 2 replies; 13+ messages in thread
From: Jiaxing Hu @ 2026-08-03  9:41 UTC (permalink / raw)
  To: tomeu, heiko, robh, krzk+dt, conor+dt, ulfh, p.zabel, ogabbay
  Cc: royalnet026, alchark, chaoyi.chen, krzk, will, dri-devel,
	linux-rockchip, linux-pm, devicetree, linux-arm-kernel,
	linux-kernel, Jiaxing Hu

Add the two RKNN cores and their IOMMUs, plus the NPU power-domain
resets the pmdomain driver now cycles on power-on. Both cores are
disabled by default; boards enable what they wire up.

Each core lists both NPU power domains, its own first. The compute path
needs NPU1 powered even when only core 0 runs, and a node with a single
domain would be auto-attached by the driver core before the driver can
attach the list itself. The IOMMUs keep one domain each, since they rely
on that same auto-attach.

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

diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
index b0c0d3c8b..6baed2579 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>;
 						};
@@ -1832,6 +1840,70 @@ 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>;
+			reg-names = "pc", "cna", "core";
+			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>, <&power RK3576_PD_NPU1>;
+			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@27708000 {
+			compatible = "rockchip,rk3576-rknn-core";
+			reg = <0x0 0x27708000 0x0 0x1000>,
+			      <0x0 0x27709000 0x0 0x1000>,
+			      <0x0 0x2770b000 0x0 0x1000>;
+			reg-names = "pc", "cna", "core";
+			interrupts = <GIC_SPI 248 IRQ_TYPE_LEVEL_HIGH>;
+			clocks = <&cru ACLK_RKNN1>, <&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_RKNN1>;
+			reset-names = "srst_a";
+			power-domains = <&power RK3576_PD_NPU1>, <&power RK3576_PD_NPU0>;
+			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


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC PATCH v4 6/6] arm64: dts: rockchip: rk3576-rock-4d: enable NPU
  2026-08-03  9:41 [RFC PATCH v4 0/6] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
                   ` (4 preceding siblings ...)
  2026-08-03  9:41 ` [RFC PATCH v4 5/6] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes Jiaxing Hu
@ 2026-08-03  9:41 ` Jiaxing Hu
  2026-08-03 10:00   ` sashiko-bot
  5 siblings, 1 reply; 13+ messages in thread
From: Jiaxing Hu @ 2026-08-03  9:41 UTC (permalink / raw)
  To: tomeu, heiko, robh, krzk+dt, conor+dt, ulfh, p.zabel, ogabbay
  Cc: royalnet026, alchark, chaoyi.chen, krzk, will, dri-devel,
	linux-rockchip, linux-pm, devicetree, linux-arm-kernel,
	linux-kernel, Jiaxing Hu

Enable rknn_core_0 and its IOMMU on the Radxa ROCK 4D and supply the
core from vdd_npu_s0.

Signed-off-by: Jiaxing Hu <gahing@gahingwoo.com>
---
 arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts | 10 ++++++++++
 1 file changed, 10 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..965e0906b 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,12 @@ vp0_out_hdmi: endpoint@ROCKCHIP_VOP2_EP_HDMI0 {
 		remote-endpoint = <&hdmi_in_vp0>;
 	};
 };
+
+&rknn_core_0 {
+	npu-supply = <&vdd_npu_s0>;
+	status = "okay";
+};
+
+&rknn_mmu_0 {
+	status = "okay";
+};
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v4 5/6] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes
  2026-08-03  9:41 ` [RFC PATCH v4 5/6] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes Jiaxing Hu
@ 2026-08-03  9:56   ` sashiko-bot
  2026-08-03 16:05   ` Igor Paunovic
  1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-03  9:56 UTC (permalink / raw)
  To: Jiaxing Hu; +Cc: conor+dt, dri-devel, robh, devicetree

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] The newly added NPU core nodes violate the `rockchip,rk3588-rknn-core` binding schema across multiple properties.
- [Low] The newly added IOMMU nodes violate the `rockchip,iommu` binding schema by declaring excessive clocks and missing required properties.
- [Low] Adding a `resets` property to the NPU power domains violates the generic Rockchip power controller schema.
--

--- Patch [5]: [RFC PATCH v4 5/6] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes ---
commit aae8c26737d67b11f41b924b5322997c922df364
Author: Jiaxing Hu <gahing@gahingwoo.com>

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

Add the two RKNN cores and their IOMMUs, plus the NPU power-domain
resets the pmdomain driver now cycles on power-on. Both cores are
disabled by default; boards enable what they wire up.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3576.dtsi b/arch/arm64/boot/dts/rockchip/rk3576.dtsi
> --- 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: Low]
Does adding the resets property here violate the generic Rockchip power
controller schema?

The rockchip,power-controller.yaml binding restricts properties for pd-nodes
and it doesn't appear to list resets as an allowable property, which could
cause a dtbs_check failure.

>  							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>;

[Severity: Low]
This shares the same resets property schema issue mentioned for RK3576_PD_NPU0
above.

[ ... ]

> +		rknn_core_0: npu@27700000 {
> +			compatible = "rockchip,rk3576-rknn-core";
> +			reg = <0x0 0x27700000 0x0 0x1000>,
> +			      <0x0 0x27701000 0x0 0x1000>,
> +			      <0x0 0x27703000 0x0 0x1000>;
> +			reg-names = "pc", "cna", "core";
> +			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>, <&power RK3576_PD_NPU1>;

[Severity: Low]
Do the power-domains, clocks, and missing npu-supply properties violate the
rockchip,rk3588-rknn-core binding schema? 

The current schema seems to require exactly 1 power-domain, up to 4 clocks,
and explicitly requires an npu-supply.

> +			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: Low]
Does this node violate the rockchip,iommu binding schema by declaring 5 clocks
without a clock-names mapping? 

The rockchip,iommu.yaml schema restricts clocks to exactly 2 items and
requires the clock-names property to be defined.

[ ... ]

> +		rknn_core_1: npu@27708000 {
> +			compatible = "rockchip,rk3576-rknn-core";
[ ... ]
> +			power-domains = <&power RK3576_PD_NPU1>, <&power RK3576_PD_NPU0>;

[Severity: Low]
This shares the same rockchip,rk3588-rknn-core schema violation mentioned for
rknn_core_0.

> +			iommus = <&rknn_mmu_1>;
> +			status = "disabled";
> +		};
> +
> +		rknn_mmu_1: iommu@2770a000 {
> +			compatible = "rockchip,rk3576-iommu", "rockchip,rk3568-iommu";
[ ... ]
> +			clocks = <&cru ACLK_RKNN1>, <&cru HCLK_RKNN_ROOT>,
> +				 <&cru CLK_RKNN_DSU0>, <&cru ACLK_RKNN_CBUF>,
> +				 <&cru HCLK_RKNN_CBUF>;

[Severity: Low]
This shares the same rockchip,iommu schema violation mentioned for rknn_mmu_0
regarding extra clocks and missing clock-names.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v4 4/6] accel/rocket: add RK3576 NPU (RKNN) support
  2026-08-03  9:41 ` [RFC PATCH v4 4/6] accel/rocket: add RK3576 NPU (RKNN) support Jiaxing Hu
@ 2026-08-03  9:56   ` sashiko-bot
  2026-08-03 15:44   ` Igor Paunovic
  1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-03  9:56 UTC (permalink / raw)
  To: Jiaxing Hu; +Cc: devicetree, robh, conor+dt, dri-devel

Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [Critical] Arbitrary physical memory corruption in experimental debug function rocket_sptr_patch_regcmd.
- [High] TOCTOU race in rocket_poll_work_fn leads to job corruption and premature completion.
- [High] Partial allocation failure causes NULL pointer dereference in rocket_snap_take.
- [High] Data race and memory leak on global variables in rocket_snap_take.
- [High] Hardware initialization is skipped after a job timeout reset, causing subsequent jobs to fail.
- [High] Teardown ordering bug: DRM scheduler is destroyed before canceling polling timer and work.
- [High] Resource leak on devm_pm_domain_attach_list error path.
- [Low] Logic error: rocket_core_reset hardcodes ARRAY_SIZE instead of utilizing the SoC-specific reset count.
--

--- Patch [4]: [RFC PATCH v4 4/6] accel/rocket: add RK3576 NPU (RKNN) support ---
commit 312454d18760cf447cabd81f08e885f92b5a4313
Author: Jiaxing Hu <gahing@gahingwoo.com>

accel/rocket: add RK3576 NPU (RKNN) support

This commit introduces support for the RK3576 NPU by abstracting platform
differences such as clock configurations, reset mechanisms, and power
domains. It also implements an hrtimer-based polling mechanism for job
completion since the RK3576 DPU completion interrupt does not reach the GIC.

> diff --git a/drivers/accel/rocket/rocket_core.c b/drivers/accel/rocket/rocket_core.c
> index b3b2fa9ba645a..5738fccdbd133 100644
> --- a/drivers/accel/rocket/rocket_core.c
> +++ b/drivers/accel/rocket/rocket_core.c
[ ... ]
> @@ -21,14 +49,22 @@ int rocket_core_init(struct rocket_core *core)
>  	/* RK3576 moves the BIU reset into its power domain and takes only srst_a. */
>  	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);

[Severity: Low]
Since we now fetch the resets using core->soc->num_resets, do we also need
to update rocket_core_reset() to use this count?

If rocket_core_reset() still uses ARRAY_SIZE(core->resets), it will try to
assert and deassert uninitialized elements on RK3576 where num_resets is 1:

rocket_core_reset() {
    reset_control_bulk_assert(ARRAY_SIZE(core->resets), core->resets);
    ...
}

>  	if (err)
>  		return dev_err_probe(dev, err, "failed to get resets for core %d\n", core->index);
[ ... ]
> @@ -65,6 +101,19 @@ int rocket_core_init(struct rocket_core *core)
>  		return err;
>  	}
>  
> +	/*
> +	 * RK3576 spans two power domains, and a multi-domain device is skipped
> +	 * by the driver-core single-domain auto-attach, so attach the list here.
> +	 */
> +	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 error path leak the resources allocated by rocket_job_init()?

Earlier in this function, rocket_job_init() successfully sets up non-devm
resources like the ordered workqueue (core->reset.wq), initializes the DRM
scheduler, and acquires a reference to core->iommu_group.

If devm_pm_domain_attach_list() fails, we return immediately. Do we need a
cleanup path here that calls rocket_job_fini() and releases the IOMMU
group?

> +	}
> +
>  	pm_runtime_use_autosuspend(dev);
[ ... ]
> diff --git a/drivers/accel/rocket/rocket_drv.c b/drivers/accel/rocket/rocket_drv.c
> index 8bbbce594883e..95599e791e26f 100644
> --- a/drivers/accel/rocket/rocket_drv.c
> +++ b/drivers/accel/rocket/rocket_drv.c
[ ... ]
> @@ -240,12 +256,15 @@ 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;
>  	}
>  
> +	/* Vendor runs its state_init once per power-up; we never did. */
> +	rocket_core_state_init(&rdev->cores[core]);

[Severity: High]
Does the timeout reset path also need to call this state initialization?

Looking at rocket_reset() which handles job timeouts, it calls
rocket_core_reset() to wipe the hardware state and then restarts the
scheduler.

Because the device does not go through a full suspend/resume cycle before the
next job executes, rocket_device_runtime_resume() is not triggered. Will
subsequent jobs fail because the hardware is left uninitialized?

> +
>  	return 0;
>  }
[ ... ]
> diff --git a/drivers/accel/rocket/rocket_job.c b/drivers/accel/rocket/rocket_job.c
> index bb77b6bf0f231..c21310ef78c7f 100644
> --- a/drivers/accel/rocket/rocket_job.c
> +++ b/drivers/accel/rocket/rocket_job.c
[ ... ]
> +static void rocket_snap_take(struct rocket_core *core)
> +{
> +	void __iomem *blk[ROCKET_SNAP_NBLK];
> +	unsigned int slot, b, i, w = 0, diffs = 0;
> +	u32 *cur;
> +
> +	if (!rocket_snap)
> +		return;
> +
> +	if (!rocket_snap_buf[0]) {

[Severity: High]
Are these global static variables safe from concurrent access?

RK3576 and RK3588 have multiple independent NPU cores. If two cores finish
jobs simultaneously and enter rocket_snap_take(), both might see
!rocket_snap_buf[0] as true. Could this result in double allocations and
leaked memory, as well as corrupted snapshot data from concurrent writes?

> +		rocket_snap_buf[0] = kmalloc_array(ROCKET_SNAP_WORDS, 4, GFP_KERNEL);
> +		rocket_snap_buf[1] = kmalloc_array(ROCKET_SNAP_WORDS, 4, GFP_KERNEL);
> +		if (!rocket_snap_buf[0] || !rocket_snap_buf[1])
> +			return;

[Severity: High]
Can this error path lead to a NULL pointer dereference on subsequent calls?

If rocket_snap_buf[0] allocates successfully but rocket_snap_buf[1] fails,
we return without freeing rocket_snap_buf[0].

The next time this function runs, !rocket_snap_buf[0] will be false, so it
skips the allocation block entirely. Later in the function, it will try to
use rocket_snap_buf[1], which is still NULL.

> +	}
> +
[ ... ]
> +static void rocket_sptr_patch_regcmd(struct rocket_core *core, struct rocket_job *job,
> +				     struct rocket_task *task, u32 bank)
> +{
> +	phys_addr_t phys = iommu_iova_to_phys(job->domain->domain, task->regcmd);
> +	unsigned int i, patched = 0;
> +	struct scatterlist sg;
> +	struct page *page;
> +	u64 *rv;
> +
> +	if (!phys || !pfn_valid(PFN_DOWN(phys)))
> +		return;
> +
> +	rv = phys_to_virt(phys);
> +	for (i = 0; i < task->regcmd_count; i++) {

[Severity: Critical]
Does this sequence allow userspace to overwrite arbitrary kernel physical
memory?

iommu_iova_to_phys() gives us a physical address, but phys_to_virt() is only
valid for direct-mapped memory, and the physical pages backing the IOVA are
not guaranteed to be contiguous.

Since task->regcmd_count is user-controlled, can an attacker provide a large
count that causes the loop to scan past the page boundary and overwrite memory
outside the intended buffer?

> +		u16 reg = rv[i] & 0xffff;
[ ... ]
> +static void rocket_poll_work_fn(struct work_struct *work)
> +{
> +	struct rocket_core *core = container_of(work, struct rocket_core, poll_work);
> +
> +	/*
> +	 * The interrupt can land while this work is already queued, finalise the
> +	 * job and let the next one start. Without this the stale work would then
> +	 * finalise that new job as well.
> +	 */
> +	if (core->poll_work_seq != core->poll_seq)
> +		return;
> +
> +	rocket_job_handle_irq(core);

[Severity: High]
Is this lockless sequence check vulnerable to a race condition?

If rocket_poll_work_fn() evaluates the condition to false but is preempted
immediately after, a physical IRQ could fire. That IRQ would complete the
current job and submit a new one (incrementing poll_seq).

When rocket_poll_work_fn() resumes, it will unconditionally execute
rocket_job_handle_irq(). Could this abruptly kill the newly submitted job
and prematurely signal its completion fence?

> +}
[ ... ]
> @@ -503,6 +803,12 @@ void rocket_job_fini(struct rocket_core *core)
>  {
>  	drm_sched_fini(&core->sched);
>  
> +	if (core->soc->poll_completion) {
> +		atomic_set(&core->poll_active, 0);
> +		hrtimer_cancel(&core->poll_timer);
> +		cancel_work_sync(&core->poll_work);
> +	}

[Severity: High]
Should the polling timer and work be stopped before destroying the scheduler?

drm_sched_fini() halts the scheduler, but any already-queued poll_work can
still execute immediately after. If it does, rocket_job_handle_irq() might
submit the next task to the hardware.

As teardown continues, clocks are disabled and memory is unmapped. If the NPU
is actively executing a newly started job when this happens, could it cause
bus hangs or IOMMU faults?

> +
>  	cancel_work_sync(&core->reset.work);
>  	destroy_workqueue(core->reset.wq);
>  }

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v4 3/6] pmdomain/rockchip: cycle optional power-domain resets on power-on
  2026-08-03  9:41 ` [RFC PATCH v4 3/6] pmdomain/rockchip: cycle optional power-domain resets on power-on Jiaxing Hu
@ 2026-08-03 10:00   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-03 10:00 UTC (permalink / raw)
  To: Jiaxing Hu; +Cc: devicetree, conor+dt, dri-devel, robh

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

Pre-existing issues:
- [High] A pre-existing resource leak occurs in `rockchip_pm_add_one_domain()` when `of_clk_get()` fails, leading to permanent leaks of previously acquired clock references and potential unbounded resource exhaustion upon probe deferral.
--

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

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

Some Rockchip domains come out of power-on with their bus interface in
an undefined state. This patch cycles the domain's resets to clear it.
It takes the domain node's resets if it has any, and pulses them between
releasing idle and restoring QoS.

> diff --git a/drivers/pmdomain/rockchip/pm-domains.c b/drivers/pmdomain/rockchip/pm-domains.c
> index e1857f8786864..82051f60090f3 100644
> --- a/drivers/pmdomain/rockchip/pm-domains.c
> +++ b/drivers/pmdomain/rockchip/pm-domains.c

[ ... ]

> @@ -857,6 +866,14 @@ static int rockchip_pm_add_one_domain(struct rockchip_pmu *pmu,
>  	if (error)
>  		goto err_put_clocks;

[Severity: High]
This is a pre-existing issue, but looking at the clock acquisition loop just
above this in rockchip_pm_add_one_domain():

	for (i = 0; i < pd->num_clks; i++) {
		pd->clks[i].clk = of_clk_get(node, i);
		if (IS_ERR(pd->clks[i].clk)) {
			error = PTR_ERR(pd->clks[i].clk);
			dev_err(pmu->dev,
				"%pOFn: failed to get clk at index %d: %d\n",
				node, i, error);
			return error;
		}
	}

If a clock at index > 0 fails to be acquired (for example, returning
-EPROBE_DEFER), does the direct return bypass the err_put_clocks cleanup
path? It appears this might permanently leak the previously acquired clock
references, potentially leading to unbounded resource exhaustion upon probe
deferral retries. Should this code clean up the successfully acquired clocks
instead of returning directly?

> +	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);

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v4 6/6] arm64: dts: rockchip: rk3576-rock-4d: enable NPU
  2026-08-03  9:41 ` [RFC PATCH v4 6/6] arm64: dts: rockchip: rk3576-rock-4d: enable NPU Jiaxing Hu
@ 2026-08-03 10:00   ` sashiko-bot
  0 siblings, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-08-03 10:00 UTC (permalink / raw)
  To: Jiaxing Hu; +Cc: dri-devel, devicetree, conor+dt, robh

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The `vdd_npu_s0` regulator is forcibly kept `regulator-always-on`, defeating runtime dynamic power management for the NPU.
- [Medium] The patch only enables the first NPU core (`rknn_core_0`), leaving the second symmetric NPU core (`rknn_core_1`) disabled.
--

--- Patch [6]: [RFC PATCH v4 6/6] arm64: dts: rockchip: rk3576-rock-4d: enable NPU ---
commit 2ef1c8164923c68f245bae8c6ee995f5fd70d4d3
Author: Jiaxing Hu <gahing@gahingwoo.com>

arm64: dts: rockchip: rk3576-rock-4d: enable NPU

Enable rknn_core_0 and its IOMMU on the Radxa ROCK 4D and supply the
core from vdd_npu_s0.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts b/arch/arm64/boot/dts/rockchip/rk3576-rock-4d.dts
> index 272af1012ab03..965e0906bd07e 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 keeping this regulator always-on defeat runtime dynamic power management
for the NPU?

The regulator will remain permanently powered, leading to unnecessary power
drain when the NPU is idle. Could we remove regulator-always-on and instead
assign this as the domain-supply for the NPU power domain so the generic
power domain framework can dynamically toggle the voltage?

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

[ ... ]

> @@ -869,3 +870,12 @@ vp0_out_hdmi: endpoint@ROCKCHIP_VOP2_EP_HDMI0 {
>  		remote-endpoint = <&hdmi_in_vp0>;
>  	};
>  };
> +
> +&rknn_core_0 {
> +	npu-supply = <&vdd_npu_s0>;
> +	status = "okay";
> +};
> +
> +&rknn_mmu_0 {
> +	status = "okay";
> +};

[Severity: Medium]
Is there a reason to only enable the first NPU core (rknn_core_0) and leave
the second symmetric core (rknn_core_1) disabled?

Since both cores share the same voltage supply and power domains, leaving
rknn_core_1 and rknn_mmu_1 disabled limits the board to half of its
physical AI compute capacity.

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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v4 4/6] accel/rocket: add RK3576 NPU (RKNN) support
  2026-08-03  9:41 ` [RFC PATCH v4 4/6] accel/rocket: add RK3576 NPU (RKNN) support Jiaxing Hu
  2026-08-03  9:56   ` sashiko-bot
@ 2026-08-03 15:44   ` Igor Paunovic
  1 sibling, 0 replies; 13+ messages in thread
From: Igor Paunovic @ 2026-08-03 15:44 UTC (permalink / raw)
  To: Jiaxing Hu
  Cc: tomeu, heiko, robh, krzk+dt, conor+dt, ulfh, p.zabel, ogabbay,
	alchark, chaoyi.chen, krzk, will, dri-devel, linux-rockchip,
	linux-pm, devicetree, linux-arm-kernel, linux-kernel

Hi Jiaxing,

Thanks for the re-spin, and thanks for asking about the tag rather than
carrying it over. I went to re-test on RK3588 and stopped at the diff,
because patch 4/6 is not the fixes-only patch the cover describes: it
carries your debugging tree with it.

Diffing v3 4/6 against v4 4/6, rocket_job.c goes from +55 to +306 lines,
and 202 of those added lines are the ping-pong experiment. It brings in
five module parameters that were not in v3:

  state_init  (rocket_core.c)  - default 1, i.e. ON
  sptr_alt    (rocket_job.c)
  snap        (rocket_job.c)
  pp_clear    (rocket_job.c)
  sptr_dbg    (rocket_job.c)

plus rocket_snap_take() with its ioremap() of a hardcoded
ROCKET_SNAP_DPU_PHYS and two kmalloc_array() buffers that are never
freed, and rocket_sptr_patch_regcmd() doing phys_to_virt() on the
IOMMU-mapped regcmd and a hand-rolled dma_sync_sg_for_device(). The
comment above the block says it itself:

  /*
   * EXPERIMENT (not for upstream), 2026-08-01, following Tomeu's
   * suggestion that the block is stuck on ping-pong bank 0.

So I think the trim simply did not catch this hunk.

The part that matters for the tag you asked me about:

  rocket_core_state_init() is called unconditionally from
  rocket_device_runtime_resume(), not behind core->soc->poll_completion
  or any RK3576 check, and rocket_state_init defaults to 1.

So on RK3588 every runtime resume now writes PC BASE_ADDRESS = 0x1 and
the CNA S_POINTER 0 / DATA_SIZE1 / S_POINTER 1 / DATA_SIZE1 / S_POINTER
0x1e sequence, i.e. the RK3576 vendor init replayed on RK3588 silicon.
That is a third change to my board that the cover does not list, and it
is the one I would have to characterise before I could put my name on
anything. "RK3588 never reaches either" is true of the two fixes you
name, but not of this patch as it stands.

The two fixes themselves look right to me, and both are correctly gated:

  * rocket_job_fini() cancelling poll_timer and poll_work under
    soc->poll_completion - RK3588 has poll_completion = false, so it
    cannot regress here.

  * poll_work_seq / poll_seq in rocket_poll_work_fn() - same gate, and
    the stale-work window it closes is real.

So: send a v5 with 4/6 trimmed back to the fixes, and I will re-run the
RK3588 bench and give you the Tested-by on that. It is a mechanical
respin, no new debugging needed from your side.

Two smaller things while I am in there, for whenever the experiment code
does come out anyway:

  * rocket_state_init is not static, so it lands in the global namespace.
  * The two snap buffers and the ioremap are never released, so with
    snap=1 that leaks per module load.

On the real problem: I do not have an RK3576 to poke at, but the shape
you describe - one configuration byte exact forever, a second one
computing nothing, the pointer reading back 1 no matter what we write,
and the 20 KB snapshot differing only in OPERATION_ENABLE - reads to me
less like a register we are failing to write and more like something the
block never re-fetches. The regcmd is DMA'd, so the interesting question
might be whether the second configuration's regcmd is actually being
read by the block at all on the second submit, rather than whether the
bank flipped. If you can get at it, an IOMMU fault trace or a read-side
counter over the regcmd buffer across the two submits would separate
"fetched and ignored" from "never fetched". If it is never fetched, the
bank is a red herring and the ping-pong work has been ruling out the
wrong half of the path.

Happy to run anything you want tried on RK3588 as a control.

Thanks,
Igor

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC PATCH v4 5/6] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes
  2026-08-03  9:41 ` [RFC PATCH v4 5/6] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes Jiaxing Hu
  2026-08-03  9:56   ` sashiko-bot
@ 2026-08-03 16:05   ` Igor Paunovic
  1 sibling, 0 replies; 13+ messages in thread
From: Igor Paunovic @ 2026-08-03 16:05 UTC (permalink / raw)
  To: Jiaxing Hu
  Cc: tomeu, heiko, robh, krzk+dt, conor+dt, ulfh, p.zabel, ogabbay,
	alchark, chaoyi.chen, krzk, will, dri-devel, linux-rockchip,
	linux-pm, devicetree, linux-arm-kernel, linux-kernel

Hi Jiaxing,

Following on from my note on 4/6, since a v5 is coming anyway: rknn_core_0
as written here does not validate against the binding 1/6 installs. 1/6
adds the compatible and the conditional sram-supply, so every other
constraint in rockchip,rk3588-rknn-core.yaml still describes RK3588 only.

I put your node through dt-validate (dtschema 2026.6) against the binding
with 1/6 applied, macros resolved to plain numbers and everything else
left as you wrote it:

  npu@27700000 (rockchip,rk3576-rknn-core): clocks: [...6 entries...] is too long
  npu@27700000 (rockchip,rk3576-rknn-core): clock-names: ['aclk', 'hclk',
      'npu', 'pclk', 'aclk_cbuf', 'hclk_cbuf'] is too long
  npu@27700000 (rockchip,rk3576-rknn-core): power-domains: [[2, 7], [2, 8]] is too long
  npu@27700000 (rockchip,rk3576-rknn-core): resets: [[1, 20]] is too short
  npu@27700000 (rockchip,rk3576-rknn-core): reset-names: ['srst_a'] is too short

The last two are the ones easy to miss: the binding writes only
"resets: maxItems: 2", and dtschema fills in minItems from maxItems, so a
single reset is a hard failure rather than a permitted subset.

The v4 changelog says the reg entries were cut to the three the binding
defines after running dtbs_check, so I suspect that run predates the CBUF
clocks and the second power domain going in.

The fix is the same allOf shape you already used for sram-supply. This is
the diff I tested, on top of 1/6:

--- a/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
+++ b/Documentation/devicetree/bindings/npu/rockchip,rk3588-rknn-core.yaml
@@
   clocks:
-    maxItems: 4
+    minItems: 4
+    maxItems: 6

   clock-names:
+    minItems: 4
     items:
       - const: aclk
       - const: hclk
       - const: npu
       - const: pclk
+      - const: aclk_cbuf
+      - const: hclk_cbuf
@@
   power-domains:
-    maxItems: 1
+    minItems: 1
+    maxItems: 2

   resets:
+    minItems: 1
     maxItems: 2

   reset-names:
+    minItems: 1
     items:
       - const: srst_a
       - const: srst_h
@@ allOf:
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: rockchip,rk3576-rknn-core
+    then:
+      properties:
+        clocks:
+          minItems: 6
+        clock-names:
+          minItems: 6
+        power-domains:
+          minItems: 2
+        resets:
+          minItems: 1
+          maxItems: 1
+        reset-names:
+          maxItems: 1
+    else:
+      properties:
+        clocks:
+          maxItems: 4
+        clock-names:
+          maxItems: 4
+        power-domains:
+          maxItems: 1
+        resets:
+          minItems: 2
+        reset-names:
+          minItems: 2

With that, your node validates clean, and the RK3588 example in the
binding still passes, so nothing loosens for the existing SoC - the else
branch pins it back to exactly what it has today. Take it as a starting
point rather than a finished patch; the DT maintainers may well want the
per-SoC clock list spelled out differently.

Separately, and this one spans 3/6 and 5/6: the resets you add to the
power-domain@ nodes have no binding at all. Same test, against
Documentation/devicetree/bindings/power/rockchip,power-controller.yaml:

  power-controller (rockchip,rk3576-power-controller): power-domain@7:
      Unevaluated properties are not allowed ('resets' was unexpected)

$defs/pd-node there defines reg, clocks, domain-supply, pm_qos and
#power-domain-cells, and every nesting level is unevaluatedProperties:
false. So 3/6 teaches the driver to read a property the schema does not
admit; that patch needs a binding change of its own, before or alongside
it.

For what it is worth I did check the DOMAIN_M_O_R_G to DOMAIN_M_O_R_G_W
rename in 2/6 against mainline, and you are right that DOMAIN_RK3576 is
its only user, so nothing else moves.

None of this touches RK3588, so it does not change what I said about the
tag on 4/6 - just worth folding into the same v5 rather than finding it
in a v6.

Thanks,
Igor

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-08-03 16:05 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-03  9:41 [RFC PATCH v4 0/6] accel/rocket: RK3576 NPU (RKNN) enablement Jiaxing Hu
2026-08-03  9:41 ` [RFC PATCH v4 1/6] dt-bindings: npu: rockchip: add rockchip,rk3576-rknn-core Jiaxing Hu
2026-08-03  9:41 ` [RFC PATCH v4 2/6] pmdomain/rockchip: add optional per-domain power-on settle delay Jiaxing Hu
2026-08-03  9:41 ` [RFC PATCH v4 3/6] pmdomain/rockchip: cycle optional power-domain resets on power-on Jiaxing Hu
2026-08-03 10:00   ` sashiko-bot
2026-08-03  9:41 ` [RFC PATCH v4 4/6] accel/rocket: add RK3576 NPU (RKNN) support Jiaxing Hu
2026-08-03  9:56   ` sashiko-bot
2026-08-03 15:44   ` Igor Paunovic
2026-08-03  9:41 ` [RFC PATCH v4 5/6] arm64: dts: rockchip: rk3576: add NPU (RKNN) nodes Jiaxing Hu
2026-08-03  9:56   ` sashiko-bot
2026-08-03 16:05   ` Igor Paunovic
2026-08-03  9:41 ` [RFC PATCH v4 6/6] arm64: dts: rockchip: rk3576-rock-4d: enable NPU Jiaxing Hu
2026-08-03 10:00   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox