Re: [PATCH v3 3/3] riscv: dts: spacemit: add usb3.0 support for K1

[Ze Huang <huangze@whut.edu.cn>]

On Mon, May 19, 2025 at 11:37:30AM +0200, Krzysztof Kozlowski wrote:
> On Sun, May 18, 2025 at 03:19:21AM GMT, Ze Huang wrote:
> > diff --git a/arch/riscv/boot/dts/spacemit/k1.dtsi b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > index 61f5ca250ded0da7b91cd4bbd55a5574a89c6ab0..164244fdb49f5d50a8abadb7b7e478cccc828087 100644
> > --- a/arch/riscv/boot/dts/spacemit/k1.dtsi
> > +++ b/arch/riscv/boot/dts/spacemit/k1.dtsi
> > @@ -4,6 +4,8 @@
> >   */
> >  
> >  #include <dt-bindings/clock/spacemit,k1-syscon.h>
> > +#include <dt-bindings/gpio/gpio.h>
> > +#include <dt-bindings/phy/phy.h>
> >  
> >  /dts-v1/;
> >  / {
> > @@ -346,6 +348,15 @@ soc {
> >  		dma-noncoherent;
> >  		ranges;
> >  
> > +		mbus0: dram-controller@0 {
> 
> Missing compatible.
> 
> > +			reg = <0x0 0x00000000 0x0 0x80000000>;
> > +			reg-names = "dram";
> 
> Where are the bindings for this?
> 
> > +			#address-cells = <2>;
> > +			#size-cells = <2>;
> 
> Why are these needed?
> 

Will drop them

> > +			dma-ranges = <0x0 0x00000000 0x0 0x00000000 0x0 0x80000000>;
> > +			#interconnect-cells = <0>;
> 
> No, you cannot just add any properties to any custom node. You need ABI
> for all this.
> 

Thanks for correcting, will create a binding for it.

> > +		};
> > +
> >  		syscon_rcpu: system-controller@c0880000 {
> >  			compatible = "spacemit,k1-syscon-rcpu";
> >  			reg = <0x0 0xc0880000 0x0 0x2048>;
> > @@ -358,6 +369,64 @@ syscon_rcpu2: system-controller@c0888000 {
> >  			#reset-cells = <1>;
> >  		};
> >  
> > +		usb_dwc3: usb@c0a00000 {
> > +			compatible = "spacemit,k1-dwc3";
> > +			reg = <0x0 0xc0a00000 0x0 0x10000>;
> > +			clocks = <&syscon_apmu CLK_USB30>;
> > +			clock-names = "usbdrd30";
> > +			resets = <&syscon_apmu RESET_USB3_0>;
> > +			interrupt-parent = <&plic>;
> > +			interrupts = <125>;
> > +			interconnects = <&mbus0>;
> > +			interconnect-names = "dma-mem";
> > +			phys = <&usbphy2>, <&combphy PHY_TYPE_USB3>;
> > +			phy-names = "usb2-phy", "usb3-phy";
> > +			dr_mode = "host";
> 
> This does not look like property of the soc.
> 

Will move `dr_mode` to board level

> > +			phy_type = "utmi";
> > +			snps,hsphy_interface = "utmi";
> > +			snps,dis_enblslpm_quirk;
> > +			snps,dis-u2-freeclk-exists-quirk;
> > +			snps,dis-del-phy-power-chg-quirk;
> > +			snps,dis_u2_susphy_quirk;
> > +			snps,dis_u3_susphy_quirk;
> > +			snps,dis_rxdet_inp3_quirk;
> > +			status = "disabled";
> > +		};
> > +
> > +		usbphy0: phy@c0940000 {
> > +			compatible = "spacemit,k1-usb2-phy";
> > +			reg = <0x0 0xc0940000 0x0 0x200>;
> > +			clocks = <&syscon_apmu CLK_USB_AXI>;
> > +			#phy-cells = <0>;
> > +			status = "disabled";
> 
> What is missing here? Why is this node disabled?
> 

There're three USB controllers on K1 [1]:
- A USB2.0 OTG
- A USB2.0 Host Only
- A USB3.0 with a USB2.0 DRD interface

usbphy0 is for USB2.0 OTG, and ushphy1 is for USB2.0 Host Only.
They're not supported yet.

Link: https://developer.spacemit.com/documentation?token=AjHDwrW78igAAEkiHracBI9HnTb#part5 [1]

> Best regards,
> Krzysztof
> 
> 
> 
>