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.
>
> > 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.
>
> > Right now, users of the xhci-plat.c driver either use a 'usb_phy' only
> > and do not care about the attempt of generic PHY initialization within
> > the HCD core (as there is none); or they use a single 'phy' and the
> > code flow does not pass through the block setting
> > hcd->skip_phy_initialization anyway.
> >
> > While there is not users of both PHY types at the same time, drop this
> > limitation from the xhci-plat.c driver. Note that the tegra driver
> > probably has the same limitation and could definitely benefit from a
> > similar change.
> >
> > Cc: Johan Hovold <johan@xxxxxxxxxx>
> > Cc: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
> > Signed-off-by: Miquel Raynal <miquel.raynal@xxxxxxxxxxx>
> Acked-by: Martin Blumenstingl <martin.blumenstingl@xxxxxxxxxxxxxx>
>
> thank you very much for the patch and the detailed explanation!
>
>
> Martin
>
> [0] https://patchwork.kernel.org/patch/10841803/
Thanks,
Miquèl
