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. My commit just made it easier to understand (at least
for me) what's going on
> 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)
> 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/
