[U-Boot] [linux-sunxi] [PATCH] phy: sun4i-usb: Fix PHY0 routing and passby configuration for MUSB

Fri Jul 12 10:44:32 UTC 2019

On Fri, 12 Jul 2019 11:44:38 +0200
Paul Kocialkowski <paul.kocialkowski at bootlin.com> wrote:

Hi,

> On Mon 27 May 19, 00:50, André Przywara wrote:
> > On 17/04/2019 12:28, Jagan Teki wrote:  
> > > On Mon, Apr 15, 2019 at 1:52 PM Paul Kocialkowski
> > > <paul.kocialkowski at bootlin.com> wrote:  
> > 
> > Hi,
> >   
> > >> Le vendredi 12 avril 2019 à 14:49 +0530, Jagan Teki a écrit :  
> > >>> On Thu, Mar 14, 2019 at 4:08 PM Paul Kocialkowski
> > >>> <paul.kocialkowski at bootlin.com> wrote:  
> > >>>> Recent Allwinner platforms (starting with the H3) only use the MUSB
> > >>>> controller for peripheral mode and use HCI for host mode. As a result,
> > >>>> extra steps need to be taken to properly route USB signals to one or
> > >>>> the other. More precisely, the following is required:
> > >>>> * Routing the pins to either HCI/MUSB (controlled by PHY);
> > >>>> * Enabling USB PHY passby in HCI mode (controlled by PMU).
> > >>>>
> > >>>> The current code will enable passby for each PHY and reroute PHY0 to
> > >>>> MUSB, which is inconsistent and results in broken USB peripheral support.
> > >>>>
> > >>>> Passby on PHY0 must only be enabled when we want to use HCI. Since
> > >>>> host/device mode detection is not available from the PHY code and
> > >>>> because U-Boot does not support changing the mode dynamically anyway,
> > >>>> we can just mux the controller to MUSB if it is enabled and mux it to
> > >>>> HCI otherwise.
> > >>>>
> > >>>> This fixes USB peripheral support for platforms with PHY0 dual-route,
> > >>>> especially H3/H5 and V3s.
> > >>>>
> > >>>> Signed-off-by: Paul Kocialkowski <paul.kocialkowski at bootlin.com>
> > >>>> ---
> > >>>>  drivers/phy/allwinner/phy-sun4i-usb.c | 14 +++++++++++++-
> > >>>>  1 file changed, 13 insertions(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> > >>>> index f206fa3f5d48..4f1c7e519d71 100644
> > >>>> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> > >>>> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> > >>>> @@ -302,9 +302,21 @@ static int sun4i_usb_phy_init(struct phy *phy)
> > >>>>                                     data->cfg->disc_thresh, PHY_DISCON_TH_LEN);
> > >>>>         }
> > >>>>
> > >>>> +#ifdef CONFIG_USB_MUSB_SUNXI
> > >>>> +       /* Needed for HCI and conflicts with MUSB, keep PHY0 on MUSB */
> > >>>> +       if (usb_phy->id != 0)
> > >>>> +               sun4i_usb_phy_passby(phy, true);
> > >>>> +
> > >>>> +       /* Route PHY0 to MUSB to allow USB gadget */
> > >>>> +       if (data->cfg->phy0_dual_route)
> > >>>> +               sun4i_usb_phy0_reroute(data, true);
> > >>>> +#else
> > >>>>         sun4i_usb_phy_passby(phy, true);
> > >>>>
> > >>>> -       sun4i_usb_phy0_reroute(data, true);
> > >>>> +       /* Route PHY0 to HCI to allow USB host */
> > >>>> +       if (data->cfg->phy0_dual_route)
> > >>>> +               sun4i_usb_phy0_reroute(data, false);
> > >>>> +#endif  
> > >>>
> > >>> I think we can manage this route and passby dynamically by detecting
> > >>> id since we have dr_mode verify the MUSB host or peripheral via
> > >>> usb_get_dr_mode, any chance to try that way?  
> > >>
> > >> Oh, I didn't know that U-Boot has support for usb_get_dr_mode these
> > >> days. Thanks!
> > >>
> > >> So far, the sunxi port has been using Kconfig to distinguish between
> > >> host/device (unless I'm mistaken?) so I feel like this should be a
> > >> separate follow-up patch to convert the sunxi MUSB glue + PHY to
> > >> detecting dr_mode using usb_get_dr_mode. This feels like a significant
> > >> rework, too.  
> > > 
> > > Yes.
> > >   
> > >>
> > >> Also, how should we handle the OTG case? I'm not sure we can support
> > >> having both musb host and gadget built-in at this point. But that would
> > >> certainly be welcome as part of the rework, too.
> > >>
> > >> What do you think?  
> > > 
> > > You mean handling dr_mode at runtime.
> > > 
> > > If yes, It is bit new where we can register the musb as UCLASS_MISC
> > > wrapper and decide to bind driver for host and peripheral by decoding
> > > dr_mode. and indeed host should register with UCLASS_USB and
> > > peripheral with UCLASS_USB_GADGET_GENERIC.
> > > 
> > > I tried this wrapper before, not placed in-between because of other
> > > work but TI musb has similar code to manage this
> > > drivers/usb/musb-new/ti-musb.c  
> > 
> > Before we go wild with any fancy rework, can we possibly take this patch
> > as it? As I realised, this is basically a better version of the patch I
> > sent two weeks ago [1]. I tried Paul's patch back then, but was missing
> > the phys property in the DT, which I addressed in patch 2/2.
> > 
> > So I would appreciate if we can take this patch, as it solves a real
> > problem (upper USB port not working) on many Pine64 boards (given the
> > small DT change is in place). And on those boards the OTG functionality
> > is not really feasible anyway, as VBUS is either permanently enabled or
> > at least tied to the other host port's supply, so we can't turn it off
> > for peripheral mode.  
> 
> Looks like nothing has moved forward on this (quite significant) fixup.
> 
> Could we please merge it as soon as time allows?

Yes, +1.
Jagan, can you please make sure to push this in this merge window?

FWIW:
Reviewed-by: Andre Przywara <andre.przywara at arm.com>
Tested-by: Andre Przywara <andre.przywara at arm.com>

Thanks,
Andre.

> 
> Cheers and thanks in advance,
> 
> Paul
> 
> > Cheers,
> > Andre.
> > 
> > [1] https://lists.denx.de/pipermail/u-boot/2019-May/369521.html  
> 