Re: [PATCH v7 10/10] arm64: dts: rockchip: enable NPU on ROCK 5B

["Heiko Stübner" <heiko@sntech.de>]

Am Freitag, 6. Juni 2025, 11:20:32 Mitteleuropäische Sommerzeit schrieb Nicolas Frattaroli:
> Hi Tomeu,
> 
> On Friday, 6 June 2025 08:28:30 Central European Summer Time Tomeu Vizoso wrote:
> > From: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > 
> > The NPU on the ROCK5B uses the same regulator for both the sram-supply
> > and the npu's supply. Add this regulator, and enable all the NPU bits.
> > Also add the regulator as a domain-supply to the pd_npu power domain.
> > 
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > Signed-off-by: Tomeu Vizoso <tomeu@tomeuvizoso.net>
> > ---
> >  arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts | 56 +++++++++++++++++++++++++
> >  1 file changed, 56 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> > index d22068475c5dc6cb885f878f3f527a66edf1ba70..49500f7cbcb14af4919a6c1997e9e53a01d84973 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dts
> > @@ -316,6 +316,28 @@ regulator-state-mem {
> >  	};
> >  };
> >  
> > +&i2c1 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&i2c1m2_xfer>;
> > +	status = "okay";
> > +
> > +	vdd_npu_s0: regulator@42 {
> > +		compatible = "rockchip,rk8602";
> > +		reg = <0x42>;
> > +		fcs,suspend-voltage-selector = <1>;
> > +		regulator-name = "vdd_npu_s0";
> > +		regulator-boot-on;
> > +		regulator-min-microvolt = <550000>;
> > +		regulator-max-microvolt = <950000>;
> > +		regulator-ramp-delay = <2300>;
> > +		vin-supply = <&vcc5v0_sys>;
> > +
> > +		regulator-state-mem {
> > +			regulator-off-in-suspend;
> > +		};
> > +	};
> > +};
> > +
> >  &i2c6 {
> >  	status = "okay";
> >  
> > @@ -440,6 +462,10 @@ &pd_gpu {
> >  	domain-supply = <&vdd_gpu_s0>;
> >  };
> >  
> > +&pd_npu {
> > +	domain-supply = <&vdd_npu_s0>;
> > +};
> > +
> >  &pinctrl {
> >  	hdmirx {
> >  		hdmirx_hpd: hdmirx-5v-detection {
> > @@ -500,6 +526,36 @@ &pwm1 {
> >  	status = "okay";
> >  };
> >  
> > +&rknn_core_top {
> > +	npu-supply = <&vdd_npu_s0>;
> > +	sram-supply = <&vdd_npu_s0>;
> > +	status = "okay";
> > +};
> > +
> > +&rknn_core_1 {
> > +	npu-supply = <&vdd_npu_s0>;
> > +	sram-supply = <&vdd_npu_s0>;
> > +	status = "okay";
> > +};
> > +
> > +&rknn_core_2 {
> > +	npu-supply = <&vdd_npu_s0>;
> > +	sram-supply = <&vdd_npu_s0>;
> > +	status = "okay";
> > +};
> > +
> > +&rknn_mmu_top {
> > +	status = "okay";
> > +};
> > +
> > +&rknn_mmu_1 {
> > +	status = "okay";
> > +};
> > +
> > +&rknn_mmu_2 {
> > +	status = "okay";
> > +};
> > +
> >  &saradc {
> >  	vref-supply = <&avcc_1v8_s0>;
> >  	status = "okay";
> > 
> > 
> 
> Feel free to replace this patch with the following, if your series is
> based on linux-next or v6.16. It moves the enablement into the new
> shared ROCK 5B/5B+ dtsi, and I've added a regulator-enable-ramp-delay
> while I was at it because I've run into hard-to-reproduce problems
> relating to it before that Heiko quickly identified and fixed in his
> recent series[1] for basically all already present regulators. Remains
> to be seen if the final patch lands in that form but this should make
> it easier for people to try out as it means a bad luck roll for the
> day won't make them run into as many weird issues.
> 
> [1]: https://lore.kernel.org/all/20250605185001.377055-1-heiko@sntech.de/

Reading that just now reminds me to point to 
https://lore.kernel.org/lkml/20250606190418.478633-1-heiko@sntech.de/

As Chen Yu pointed out in the reply to v2, this is more a property of the
regulator IC itself, so likely should go into the driver.

So with a bit of luck after 6.16-rc1 all the fan53555 clones should use
somewhat hardware-accurate enable-times.


> From ff1c370a158f4340aa5dfa4ed5034e815e5371be Mon Sep 17 00:00:00 2001
> From: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> Date: Tue, 3 Jun 2025 17:03:10 +0200
> Subject: [PATCH] arm64: dts: rockchip: enable NPU on ROCK 5B/+
> 
> The NPU on the ROCK5B uses the same regulator for both the sram-supply
> and the npu's supply. Add this regulator, and enable all the NPU bits.
> Also add the regulator as a domain-supply to the pd_npu power domain.
> 
> The 5B+'s regulator setup is identical to the 5B in this regard, so it
> goes in the shared .dtsi.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  .../boot/dts/rockchip/rk3588-rock-5b.dtsi     | 57 +++++++++++++++++++
>  1 file changed, 57 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
> index 51e83f0ed809..5a20cc2555fb 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-rock-5b.dtsi
> @@ -332,6 +332,29 @@ regulator-state-mem {
>  	};
>  };
>  
> +&i2c1 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&i2c1m2_xfer>;
> +	status = "okay";
> +
> +	vdd_npu_s0: regulator@42 {
> +		compatible = "rockchip,rk8602";
> +		reg = <0x42>;
> +		fcs,suspend-voltage-selector = <1>;
> +		regulator-name = "vdd_npu_s0";
> +		regulator-boot-on;
> +		regulator-enable-ramp-delay = <500>;
> +		regulator-min-microvolt = <550000>;
> +		regulator-max-microvolt = <950000>;
> +		regulator-ramp-delay = <2300>;
> +		vin-supply = <&vcc5v0_sys>;
> +
> +		regulator-state-mem {
> +			regulator-off-in-suspend;
> +		};
> +	};
> +};
> +
>  &i2c3 {

I think your patch here has some other dependency still?
Because in the rk3588-rock-5b.dtsi there is no i2c3 yet
In torvalds' branch from _just now_ ;-) .


Heiko

>  	status = "okay";
>  };
> @@ -521,6 +544,10 @@ &pd_gpu {
>  	domain-supply = <&vdd_gpu_s0>;
>  };
>  
> +&pd_npu {
> +	domain-supply = <&vdd_npu_s0>;
> +};
> +
>  &pinctrl {
>  	hdmirx {
>  		hdmirx_hpd: hdmirx-5v-detection {
> @@ -585,6 +612,36 @@ &pwm1 {
>  	status = "okay";
>  };
>  
> +&rknn_core_top {
> +	npu-supply = <&vdd_npu_s0>;
> +	sram-supply = <&vdd_npu_s0>;
> +	status = "okay";
> +};
> +
> +&rknn_core_1 {
> +	npu-supply = <&vdd_npu_s0>;
> +	sram-supply = <&vdd_npu_s0>;
> +	status = "okay";
> +};
> +
> +&rknn_core_2 {
> +	npu-supply = <&vdd_npu_s0>;
> +	sram-supply = <&vdd_npu_s0>;
> +	status = "okay";
> +};
> +
> +&rknn_mmu_top {
> +	status = "okay";
> +};
> +
> +&rknn_mmu_1 {
> +	status = "okay";
> +};
> +
> +&rknn_mmu_2 {
> +	status = "okay";
> +};
> +
>  &saradc {
>  	vref-supply = <&avcc_1v8_s0>;
>  	status = "okay";
>