Hi Miquel,
On Fri, Mar 29, 2019 at 4:16 PM Miquel Raynal <miquel.raynal@xxxxxxxxxxx> wrote:
>
> Hi Martin,
>
> Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx> 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@xxxxxxxxxxx> 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?
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)
Regards
Martin
