Re: [PATCH v3 1/5] riscv: dts: spacemit: k3: move USB3 phy to board level

[Aurelien Jarno <aurelien@aurel32.net>]

On 2026-08-22 20:33, Inochi Amaoto wrote:
> On Sat, Aug 22, 2026 at 11:37:44AM +0200, Aurelien Jarno wrote:
> > On 2026-08-22 10:17, Inochi Amaoto wrote:
> > > On Wed, Aug 19, 2026 at 06:15:42PM +0200, Aurelien Jarno wrote:
> > > > On the SpacemiT K3 SoC, the combo phy provides 8 lines that can
> > > > either be used for PCIe or USB3, depending on its configuration
> > > > (spacemit,apmu). This configuration is defined at the board level, and
> > > > assigning a phy line to a USB3 controller while it is configured as PCIe
> > > > results in an "Invalid PHY mode" error.
> > > > 
> > > > Move the USB3 phy at the board level, the as it is done for the
> > > > submitted PCIe controller [1]. Keep the USB2 phy at the SoC level as
> > > > there is one phy per USB controller.
> > > > 
> > > > [1] https://lore.kernel.org/20260727094726.890179-3-inochiama@gmail.com/
> > > > 
> > > > Fixes: 46380e4b1534c1626cecd7f4d0abb5e0cce09af6 ("riscv: dts: spacemit: k3: add USB controller and USB phy support")
> > > > Cc: Inochi Amaoto <inochiama@gmail.com>
> > > > Signed-off-by: Aurelien Jarno <aurelien@aurel32.net>
> > > > ---
> > > >  arch/riscv/boot/dts/spacemit/k3-pico-itx.dts | 3 +++
> > > >  arch/riscv/boot/dts/spacemit/k3.dtsi         | 5 ++---
> > > >  2 files changed, 5 insertions(+), 3 deletions(-)
> > > > 
> > > > diff --git a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
> > > > index b084a35aa00e..171bd2f40aa6 100644
> > > > --- a/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
> > > > +++ b/arch/riscv/boot/dts/spacemit/k3-pico-itx.dts
> > > > @@ -328,5 +328,8 @@ &usb3d_u2phy {
> > > >  };
> > > >  
> > > >  &usb3d {
> > > > +	phys = <&usb3d_u2phy>,
> > > > +	       <&combophy 4 PHY_TYPE_USB3>;
> > > > +	phy-names = "usb2-phy", "usb3-phy";
> > > >  	status = "okay";
> > > >  };
> > > > diff --git a/arch/riscv/boot/dts/spacemit/k3.dtsi b/arch/riscv/boot/dts/spacemit/k3.dtsi
> > > > index c3f2dce0969c..3e24054d2084 100644
> > > > --- a/arch/riscv/boot/dts/spacemit/k3.dtsi
> > > > +++ b/arch/riscv/boot/dts/spacemit/k3.dtsi
> > > > @@ -452,9 +452,8 @@ usb3d: usb@81a00000 {
> > > >  				 <&syscon_apmu RESET_APMU_USB3_D_VCC>,
> > > >  				 <&syscon_apmu RESET_APMU_USB3_D_PHY>;
> > > >  			reset-names = "ahb", "vcc", "phy";
> > > 
> > > > -			phys = <&usb3d_u2phy>,
> > > > -			       <&combophy 4 PHY_TYPE_USB3>;
> > > > -			phy-names = "usb2-phy", "usb3-phy";
> > > > +			phys = <&usb3d_u2phy>;
> > > > +			phy-names = "usb2-phy";
> > > 
> > > I suggest removing this completely as we can not determine the
> > > phy we used. I think it is suitable for setting this in board
> > > file only
> > 
> > Yes, moving all the phy definition to the board level is an option. I 
> > decided to keep the USB2 phy at the SoC level, because contrary to the 
> > USB3 phys, there is a one to one mapping between the controller and the 
> > phy. Do you think it is a wrong assumption?
> > 
> 
> By default, I prefer to make the forgotten thing as an error in
> validation for any flexible field. If you leave the default value,
> the forgotten field will lead to a downgrade hardware (or maybe
> broken hardware). So I prefer to remove this rather than give a
> default value.

Ok, thanks for the feedback, i'll fix that in the v4.

Regards
Aurelien

-- 
Aurelien Jarno                          GPG: 4096R/1DDD8C9B
aurelien@aurel32.net                     http://aurel32.net