Re: [PATCH v3 11/12] arm64: dts: rockchip: rk3588s-nanopi-r6: refactor to support M6 boards

[Heiko Stuebner <heiko@sntech.de>]

Hi Joachim,

Am Donnerstag, 16. Juli 2026, 11:24:20 Mitteleuropäische Sommerzeit schrieb Joachim Eastwood:
> On Mon, 13 Jul 2026 at 22:41, <sashiko-bot@kernel.org> wrote:
> >
> > Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> > - [Medium] Unintentional restoration of `regulator-boot-on` for `vdd_npu_s0` during refactoring.
> > - [Low] Dangling phandle reference `&vcc5v0_usb_otg0` in the common `.dtsi` file.
> > --
> >
> > --- Patch [11]: [PATCH v3 11/12] arm64: dts: rockchip: rk3588s-nanopi-r6: refactor to support M6 boards ---
> > commit 056adff75ec08989b96f09adc0efff6c3a5d662f
> > Author: Joachim Eastwood <joachim.eastwood@gmail.com>
> >
> > arm64: dts: rockchip: rk3588s-nanopi-r6: refactor to support M6 boards
> >
> > FriendlyElec NanoPi6 series consists of four distinct boards; R6S, R6C,
> > M6 and M6V2. Refactor FriendlyElec NanoPi R6 base dtsi in preparation
> > for adding NanoPi M6 support.
> >
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588s-nanopi.dtsi b/arch/arm64/boot/dts/rockchip/rk3588s-nanopi.dtsi
> > > new file mode 100644
> > > index 0000000000000..a8e532ad8b7a8
> > > --- /dev/null
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3588s-nanopi.dtsi
> >
> > [ ... ]
> >
> > > +&i2c2 {
> > > +     status = "okay";
> > > +
> > > +     vdd_npu_s0: regulator@42 {
> > > +             compatible = "rockchip,rk8602";
> > > +             reg = <0x42>;
> > > +             fcs,suspend-voltage-selector = <1>;
> > > +             regulator-name = "vdd_npu_s0";
> > > +             regulator-min-microvolt = <550000>;
> > > +             regulator-max-microvolt = <950000>;
> > > +             regulator-ramp-delay = <2300>;
> > > +             regulator-boot-on;
> >
> > [Severity: Medium]
> > Is the addition of regulator-boot-on here intentional?
> 
> Seems like I screwed up the refactoring of the code, and Sashiko caught me.
> I will fix it and send out a new patch set later.
> 
> > This appears to accidentally revert a prior fix that removed the boot-on
> > property for vdd_npu_s0 to save power during early boot.
> >
> > > +             vin-supply = <&vcc5v0_sys>;
> > > +
> > > +             regulator-state-mem {
> > > +                     regulator-off-in-suspend;
> > > +             };
> > > +     };
> > > +};
> >
> > [ ... ]
> >
> > > +&u2phy0_otg {
> > > +     phy-supply = <&vcc5v0_usb_otg0>;
> >
> > [Severity: Low]
> > Does this create a dangling phandle reference in the common dtsi file?
> >
> > Since the vcc5v0_usb_otg0 regulator was left in the board-specific
> > rk3588s-nanopi-r6.dtsi file, it looks like compiling a new board (like the M6)
> > that includes this common dtsi will fail to build if it doesn't also define a
> > regulator with this exact label.
> 
> I don't think this is really an issue. Both nanopi-r6.dtsi and nanopi-m6.dtsi
> will have those regulators defined.
> 
> Unless someone tells me this is bad practice I will keep this as it is.

I think this is ok. Like as you said, the regulator will be defined by
boards - just with a different gpio I guess.
And nobody expects to build the core dtsi and if another new board
materializes that does not have the otg port (and thus regulator), we'll
need to move more stuff anyway :-)


Heiko