usb: host: xhci-plat: Prevent an abnormally restrictive PHY init skipping

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hi Martin,

Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote on Sat,
30 Mar 2019 13:53:29 +0100:

> Hi Miquel,
> 
> On Fri, Mar 29, 2019 at 4:16 PM Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >
> > Hi Martin,
> >
> > Martin Blumenstingl <martin.blumenstingl@googlemail.com> wrote on Tue,
> > 26 Mar 2019 18:29:25 +0100:
> >  
> > > Hello Miquel,
> > >
> > > On Tue, Mar 26, 2019 at 9:39 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote:  
> > > >
> > > > In the past, USB PHY handling has been moved in the HCD core. Some
> > > > host controller drivers needing more control of the PHYs, they have
> > > > been granted the freedom to handle themselves the PHY states and to
> > > > prevent the HCD core to do so in commit 4e88d4c08301 ("usb: add a flag
> > > > to skip PHY initialization to struct usb_hcd"). With this change, any
> > > > USB host controller could set the hcd->skip_phy_initialization flag so
> > > > that the HCD core would just skip the PHY initialization sequence.  
> > > nit-pick: strictly speaking host controller drivers were able to skip
> > > the core's PHY initialization sequence by setting hcd->phy or
> > > hcd->usb_phy.  
> >
> > Indeed!
> >  
> > > My commit just made it easier to understand (at least
> > > for me) what's going on  
> >
> > Actually it also had the effect to merge the two conditions
> > (having set either hcd->phy or hcd->usb_phy) in one bit of information
> > which, IMHO, had an impact thereafter.  
> excellent catch - I haven't noticed that before
> 
> > >  
> > > > However, in the USB subsystem, there are currently two entirely
> > > > different forms of PHY: one is called 'usb_phy' and is
> > > > USB-subsystem-wide, while there is also the generic and kernel-wide
> > > > 'phy' from the (recent) generic PHY framework.
> > > >
> > > > When the commit above was introduced, both type of PHYs where handled
> > > > by the HCD core.
> > > >
> > > > Later, commit bc40f5341741 ("USB: core: hcd: drop support for legacy
> > > > phys") removed the support for the former type of PHYs in the HCD
> > > > core. These 'usb_phy' are still present though, but managed from the
> > > > controller drivers only. Hence, setting the
> > > > hcd->skip_phy_initialization flag just because a 'usb_phy' is
> > > > initialized by a controller driver is a non-sense.
> > > >
> > > > For instance on Armada CP110, a 'usb_phy' is there to enable the power
> > > > supply to the USB host, while there is also a COMPHY block providing
> > > > SERDES lanes configuration that is referenced as a PHY from the common
> > > > PHY framework.  
> > > arch/arm64/boot/dts/marvell/armada-8040-clearfog-gt-8k.dts (using this
> > > as an example, because it's what I found first) could be changed to
> > > use the phy-supply property of the (recent) generic PHY framework.
> > > This is documented here:
> > > Documentation/devicetree/bindings/phy/phy-bindings.txt
> > >
> > > as far as I understand both, generic PHY's phy-supply and
> > > usb-nop-xceiv's vcc-supply are *always* enabling the supply when the
> > > PHY is enabled.
> > > for an OTG capable USB controller this may not be correct, because
> > > VBUS should only be provided in "host" mode, but not in "peripheral"
> > > mode.
> > > dwc2 has a special vbus-supply property for this (documentation is
> > > currently not reflecting this, but I sent a patch to fix it: [0])
> > >
> > > I'm aware that this has nothing to do with your patch, I just wanted
> > > to let you know in case you didn't know about it yet (so you can judge
> > > for yourself whether another change somewhere is appropriate)  
> >
> > Actually this is really interesting! Thanks for sharing Rob's answer to
> > your thread. I think this patch still has a meaning but in the mean
> > time I will convert the usb-phy property to the common PHY framework
> > using (as Rob told you) a connector and a phy-supply attached to it.  
> that seems like a good plan. the current patch should still be
> applied, shouldn't it?

Yes indeed.

> also can you please CC me on the connector patches (whenever they are
> ready, I want to see whether we have to update the Amlogic platforms
> as well)

Sure, I'll Cc: you.

Regards,
Miquèl