On Tue, Jul 12, 2022 at 06:02:22PM +0530, Sumit Garg wrote:
> On Mon, 11 Jul 2022 at 21:16, Daniel Thompson
> <daniel.thompson@xxxxxxxxxx> wrote:
> >
> > On Mon, Jul 11, 2022 at 02:00:38PM +0530, Sumit Garg wrote:
> > > Currently the DT for QCS404 SoC has setup for 2 USB2 PHYs with one each
> > > assigned to USB3 controller and USB2 controller. This assignment is
> > > incorrect which only works by luck: as when each USB HCI comes up it
> > > configures the *other* controllers PHY which is enough to make them
> > > happy. If, for any reason, we were to disable one of the controllers then
> > > both would stop working.
> > >
> > > This was a difficult inconsistency to be caught which was found while
> > > trying to enable USB support in u-boot. So with all the required drivers
> > > ported to u-boot, I couldn't get the same USB storage device enumerated
> > > in u-boot which was being enumerated fine by the kernel.
> > >
> > > The root cause of the problem came out to be that I wasn't enabling USB2
> > > PHY: "usb2_phy_prim" in u-boot. Then I realised that via simply disabling
> > > the same USB2 PHY currently assigned to USB2 host controller in the
> > > kernel disabled enumeration for USB3 host controller as well.
> > >
> > > So fix this inconsistency by correctly assigning USB2 PHYs.
> > >
> > > Fixes: 9375e7d719b3 ("arm64: dts: qcom: qcs404: Add USB devices and PHYs")
> > > Signed-off-by: Sumit Garg <sumit.garg@xxxxxxxxxx>
> >
> > I've not got one of these board (nor any documentation for them) but the
> > description and change look OK. Thus FWIW:
> >
> > Reviewed-by: Daniel Thompson <daniel.thompson@xxxxxxxxxx>
> >
>
> Thanks Daniel for the review.
No worries.
> BTW, I did confirmed that this fix is correct with respect to
> documentation (SA2150P LINUX USB TECHNICAL OVERVIEW) as well:
>
> 2.1 USB memory addresses
> ■ USB3.0 core address starts with 0x7580000. USB3.0 is connected to:
> □ SS PHY with start address as 0x78000
> □ HS PHY with start address as 0x7a000.
> ■ USB2.0 core address starts with 0x78c0000; it is connected only to
> HS PHY with the start address as 0x7c000.
I didn't mean to imply the patch was in any way deficient (the patch
description showed your experimental method pretty clearly). I just
wanted to be clear that I hadn't double checked anything outside of the
patch itself!
Daniel.
