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

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

[Paul Kocialkowski]

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@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
 
 	return 0;
 }
-- 
2.20.1

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

[Jagan Teki]

On Thu, Mar 14, 2019 at 4:08 PM Paul Kocialkowski
<paul.kocialkowski@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@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?

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

[Paul Kocialkowski]

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@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@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.

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?

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

[Jagan Teki]

On Mon, Apr 15, 2019 at 1:52 PM Paul Kocialkowski
<paul.kocialkowski@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@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@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

Jagan.

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

[André Przywara]

On 17/04/2019 12:28, Jagan Teki wrote:
> On Mon, Apr 15, 2019 at 1:52 PM Paul Kocialkowski
> <paul.kocialkowski@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@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@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.

Cheers,
Andre.

[1] https://lists.denx.de/pipermail/u-boot/2019-May/369521.html

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

[Paul Kocialkowski]

Hi,

On Mon, 2019-05-27 at 00:50 +0100, 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@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@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@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.

I fully agree, and believe the rework should be a separate subsequent
series.

> 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.

It makes USB OTG work on my V3/V3s boards as well and there's a chance
it fixes A83t as well.

Cheers,

Paul

> Cheers,
> Andre.
> 
> [1] https://lists.denx.de/pipermail/u-boot/2019-May/369521.html
-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

[Paul Kocialkowski]

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@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@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@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?

Cheers and thanks in advance,

Paul

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

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

[Andre Przywara]

On Fri, 12 Jul 2019 11:44:38 +0200
Paul Kocialkowski <paul.kocialkowski@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@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@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@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@arm.com>
Tested-by: Andre Przywara <andre.przywara@arm.com>

Thanks,
Andre.

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

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

[Jagan Teki]

On Mon, May 27, 2019 at 5:21 AM André Przywara <andre.przywara@arm.com> wrote:
>
> On 17/04/2019 12:28, Jagan Teki wrote:
> > On Mon, Apr 15, 2019 at 1:52 PM Paul Kocialkowski
> > <paul.kocialkowski@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@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@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

Sorry, I missed this. yes rework can be a fancy but it's been
discussed months back so I was expected to have this meaningful rework
patch in-between so-that it would eventually reviewed and may plan for
this MW.

Having said that I'm not so interested to go with ifdef code in dm
driver( which we filter many thing before). better have a sample
rework patch which still can review know.

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

[Paul Kocialkowski]

Hi,

On Mon 15 Jul 19, 12:55, Jagan Teki wrote:
> On Mon, May 27, 2019 at 5:21 AM André Przywara <andre.przywara@arm.com> wrote:
> >
> > On 17/04/2019 12:28, Jagan Teki wrote:
> > > On Mon, Apr 15, 2019 at 1:52 PM Paul Kocialkowski
> > > <paul.kocialkowski@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@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@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
> 
> Sorry, I missed this. yes rework can be a fancy but it's been
> discussed months back so I was expected to have this meaningful rework
> patch in-between so-that it would eventually reviewed and may plan for
> this MW.

Oh, has anyone started work in this direction so far?

> Having said that I'm not so interested to go with ifdef code in dm
> driver

Do you have a better option than ifdef to suggest for now?

> ( which we filter many thing before). better have a sample
> rework patch which still can review know.

I'm afraid I don't follow what you mean with that.

Cheers,

Paul

-- 
Paul Kocialkowski, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

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

[Jagan Teki]

On Mon, Jul 15, 2019 at 1:01 PM Paul Kocialkowski
<paul.kocialkowski@bootlin.com> wrote:
>
> Hi,
>
> On Mon 15 Jul 19, 12:55, Jagan Teki wrote:
> > On Mon, May 27, 2019 at 5:21 AM André Przywara <andre.przywara@arm.com> wrote:
> > >
> > > On 17/04/2019 12:28, Jagan Teki wrote:
> > > > On Mon, Apr 15, 2019 at 1:52 PM Paul Kocialkowski
> > > > <paul.kocialkowski@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@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@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
> >
> > Sorry, I missed this. yes rework can be a fancy but it's been
> > discussed months back so I was expected to have this meaningful rework
> > patch in-between so-that it would eventually reviewed and may plan for
> > this MW.
>
> Oh, has anyone started work in this direction so far?

Not sure, I thought you might have working based on our early
communication on this thread.

>
> > Having said that I'm not so interested to go with ifdef code in dm
> > driver
>
> Do you have a better option than ifdef to suggest for now?

Yes, same like what I mentioned above.

Manage the route and passby dynamically by detecting id since we have
dr_mode verify the MUSB host or peripheral via usb_get_dr_mode.
drivers/usb/musb-new/ti-musb.c does this if I'm not wrong.

>
> > ( which we filter many thing before). better have a sample
> > rework patch which still can review know.
>
> I'm afraid I don't follow what you mean with that.

We have phy driver with lot of ifdef code, so then we created this dm
driver managed those via driver data like Linux does. So, adding ifdef
code again to this driver feel like going back ward.