Re: [PATCH v9 2/2] arm64: dts: rockchip: Add ALIENTEK QuarkPi-CA2

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] arm64: dts: rockchip: missing rknn_mmu_0/1/2 enablement breaks NPU open()
- [High] arm64: dts: rockchip: missing domain-supply on pd_gpu and pd_npu
--

commit e4632e8324e154e7c3c642a2039af079523bc6b6
Author: Bob Hui <bg9oxa@163.com>

arm64: dts: rockchip: Add ALIENTEK QuarkPi-CA2

This commit adds the device tree for the ALIENTEK QuarkPi-CA2, an RK3588S
based board, enabling its onboard storage, regulators, display outputs,
networking, USB, audio, GPU, and NPU peripherals.

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-quarkpi-ca2.dts b/arch/arm64/boot/dts/rockchip/rk3588s-quarkpi-ca2.dts
> new file mode 100644
> index 0000000000000..68ee2c0ad9d80
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588s-quarkpi-ca2.dts

[ ... ]

> +&gpu {
> +	mali-supply = <&vdd_gpu_s0>;
> +	status = "okay";
> +};

[Severity: High]
Should &pd_gpu and &pd_npu also specify domain-supply = <&vdd_gpu_s0> and
domain-supply = <&vdd_npu_s0>?

In drivers/pmdomain/rockchip/pm-domains.c, rk3588_pm_domains marks both
RK3588_PD_GPU (&pd_gpu) and RK3588_PD_NPU (&pd_npu) with
need_regulator = true, and rockchip_pm_add_one_domain() powers those domains
off during PMU probe. When a consumer attaches or resumes via
genpd_power_on() -> rockchip_pd_power_on() -> rockchip_pd_regulator_enable(),
the PMU driver looks up domain-supply on the power-domain node:

drivers/pmdomain/rockchip/pm-domains.c:rockchip_pd_regulator_enable() {
	if (!pd->info->need_regulator)
		return 0;

	if (IS_ERR_OR_NULL(pd->supply)) {
		pd->supply = devm_of_regulator_get(pmu->dev, pd->node, "domain");

		if (IS_ERR(pd->supply))
			return PTR_ERR(pd->supply);
	}

	return regulator_enable(pd->supply);
}

In arch/arm64/boot/dts/rockchip/rk3588s-quarkpi-ca2.dts, vdd_gpu_s0 is only
regulator-boot-on (without regulator-always-on) and both vdd_gpu_s0 and
vdd_npu_s0 configure regulator-off-in-suspend. Without domain-supply on
&pd_gpu and &pd_npu, the regulator core falls back to a dummy regulator with
a warning and rockchip_pd_power() attempts the PMU power-on handshake before
vdd_gpu_s0 or vdd_npu_s0 has been enabled (such as when panthor probes as a
module after unused boot-on regulators are disabled, or across system suspend
and resume).

> +
> +&rknn_core_0 {
> +	npu-supply = <&vdd_npu_s0>;
> +	sram-supply = <&vdd_npu_mem_s0>;
> +	status = "okay";
> +};
> +
> +&rknn_core_1 {
> +	npu-supply = <&vdd_npu_s0>;
> +	sram-supply = <&vdd_npu_mem_s0>;
> +	status = "okay";
> +};
> +
> +&rknn_core_2 {
> +	npu-supply = <&vdd_npu_s0>;
> +	sram-supply = <&vdd_npu_mem_s0>;
> +	status = "okay";
> +};

[Severity: High]
Will opening the NPU accel device fail with -ENODEV unless &rknn_mmu_0,
&rknn_mmu_1, and &rknn_mmu_2 are also enabled?

In arch/arm64/boot/dts/rockchip/rk3588-base.dtsi, rknn_core_0, rknn_core_1,
and rknn_core_2 reference iommus = <&rknn_mmu_0>, <&rknn_mmu_1>, and
<&rknn_mmu_2>, which default to status = "disabled":

	rknn_mmu_0: iommu@fdab9000 {
		...
		power-domains = <&power RK3588_PD_NPUTOP>;
		status = "disabled";
	};

Because arch/arm64/boot/dts/rockchip/rk3588s-quarkpi-ca2.dts enables the
three rknn_core_* nodes without setting status = "okay" on &rknn_mmu_0,
&rknn_mmu_1, and &rknn_mmu_2, of_iommu_xlate() in drivers/iommu/of_iommu.c
returns -ENODEV:

	if (!of_device_is_available(iommu_spec->np))
		return -ENODEV;

This leaves the NPU core devices without an attached IOMMU. When userspace
opens the NPU device, rocket_open() in drivers/accel/rocket/rocket_drv.c
calls rocket_iommu_domain_create() -> iommu_paging_domain_alloc() ->
__iommu_paging_domain_alloc_flags():

drivers/accel/rocket/rocket_drv.c:rocket_open() {
	...
	rocket_priv->domain = rocket_iommu_domain_create(rdev->cores[0].dev);
	if (IS_ERR(rocket_priv->domain)) {
		ret = PTR_ERR(rocket_priv->domain);
		goto err_free;
	}
	...
}

which checks !dev_has_iommu(dev) and fails unconditionally with -ENODEV.

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261021-quarkpi-ca2-v9-0-b75006b22bc5@163.com?part=2