Hi Marek, On Sun, Dec 3, 2023 at 10:37 PM Marek Vasut <ma...@denx.de> wrote: > > On 12/3/23 22:42, Shantur Rathore wrote: > > Hi Marek, > > > > On Sun, Dec 3, 2023 at 8:42 PM Marek Vasut <ma...@denx.de> wrote: > >> > >> On 11/24/23 01:37, Shantur Rathore wrote: > >>> Hi Marek, > >> > >> Hi, > >> > >> sorry for the late reply. > >> > >>>>>>> In my case RockPro64, the power to usb ports onboard is controlled by > >>>>>>> a regulator. > >>>>>>> This regulator is enabled as part of init as here > >>>>>>> https://github.com/u-boot/u-boot/blob/master/arch/arm/dts/rk3399-rockpro64.dtsi#L177 > >>>>>>> > >>>>>>> On init, the usb devices connected to the port are powered up, in my > >>>>>>> case AM8180 (a RTL9210 based ) NVMe to USB enclosure with UAS. > >>>>>>> But the usb controller is only enabled at 'usb start' and by this time > >>>>>>> the device is in some state that it doesn't get detected. > >>>>>>> > >>>>>>> One way to make sure the devices connected to the ports are in an > >>>>>>> initialising state is by issuing a port reset before scanning the > >>>>>>> port. > >>>>>> > >>>>>> Why not remove this regulator-always-on and let the PHY driver turn the > >>>>>> VBUS ON only when it is needed ? > >>>>>> > >>>>>> Wouldn't that solve the problem too AND remove unnecessarily enabled > >>>>>> regulator ? > >>>>>> > >>>>>> [...] > >>>>> > >>>>> Removing the regulator-always-on does make it work but there are few > >>>>> issues > >>>>> > >>>>> 1. regulator is used to control power to multiple ports ( USB3.0 and > >>>>> USB2.0 in RockPro64 ), > >>>>> so this could lead to all ports turning off if a phy resets / turns off > >>>>> power > >>>> > >>>> I vaguely recall seeing some refcounting patch for regulator subsystem, > >>>> would that help ? > >>> > >>> I don't think this would be a problem for u-boot, but linux maybe. > >> > >> How so ? > > > > Actually, I am wrong. I wasn't sure if there is refcounting for the > > regulator subsystem > > in linux. just found it has so this is null and void. > > > >> > >>>>> 2. Even with regulator-always-on and without this reset port patch, > >>>>> linux was always > >>>>> able to detect the drive on the port, so there is something missing in > >>>>> u-boot. > >>>>> 3. Looking at usb hub code in Linux, I found that for USB3.0 it tries > >>>>> to reset the port before > >>>>> scanning. The comment says > >>>>> > >>>>> /* Is a USB 3.0 port in the Inactive or Compliance Mode state? > >>>>> * Port warm reset is required to recover > >>>>> */ > >>>>> > >>>>> i. > >>>>> https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#L5736 > >>>>> ii. > >>>>> https://github.com/torvalds/linux/blob/master/drivers/usb/core/hub.c#L2836 > >>>>> > >>>>> I believe this isn't implemented in u-boot and hence explicit reset of > >>>>> a port fixes the issue. > >>>> > >>>> I feel this is separate issue and what needs to be fixed first is the > >>>> hard-wired VBus control. > >>> > >>> I beg to differ on this, couple of reasons for this > >>> > >>> 1. The same drive works fine without being reset on the USB2.0 ports - > >>> this > >>> confirms that the issue is related to USB3.0 only. > >> > >> This is a separate issue from the hard-wired VBus control. I agree the > >> issue does exist, I would simply like to avoid conflating the hard-wired > >> VBus control with device reset. > >> > >>> 2. Linux is able to detect the same drive on USB3.0 when u-boot fails - > >>> this > >>> confirms issue doesn't lie with regulator-always-on > >> > >> See above > >> > >>> 3. The links I pasted earlier mentions that in USB3.0 the ports need reset > >>> and this is done before the port is scanned. Adding the similar reset in > >>> u-boot > >>> fixes all with the same regulator-always-on. AFAIK u-boot doesn't handle > >>> this special USB3.0 case and maybe this is why I was finding a few posts > >>> around the drive not being detected in the USB3.0 port in u-boot but > >>> works in > >>> a USB2.0 port. > >> > >> I am not disputing the need for USB 3.0 device reset, I believe there > >> are two issues here -- one is the hard-wired VBus regulator -- the other > >> is this USB 3.0 reset. They should both be fixed. > > > > Sure, agreed 100%. > > Do we need to fix both at the same time? > > Both patches seem to be independent. > > Separate patch for the regulator please .
Thanks, I am working on the regulator patch for rk3399-rockpro64-u-boot but there seems to be a bug in the rk3399-rockpro64.dtsi from linux where the typec phy is configure with wrong regulator and if I do the patch as below --- a/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi +++ b/arch/arm/dts/rk3399-rockpro64-u-boot.dtsi @@ -22,6 +22,23 @@ }; }; +&vcc5v0_host { + /delete-property/ regulator-always-on; +}; + +&vcc5v0_typec { + /delete-property/ regulator-always-on; +}; + +&vcc5v0_usb { + /delete-property/ regulator-always-on; + /delete-property/ regulator-boot-on; +}; The typec port won't work until I correct the issue with +&u2phy1_host { + phy-supply = <&vcc5v0_typec>; +}; + What would be acceptable here? 1. Leave regulator for typec as it was 2. disable regulator-always-on and fix phy here. Kind regards, Shantur