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

[sashiko-bot@kernel.org]

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