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

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

[Andre Przywara]

From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>

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 host support
for port 0.

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 host support for port 0 on platforms with PHY0 dual-route,
especially on boards like Pine64 (with only USB-A host ports) and
TV boxes without OTG ports.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
[Andre: tweak commit message, use IS_ENABLED()]
Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
Hi,

for H6 boards to work this requires a DT update (to get the <&usbphy 0>
links between HCI and PHY), which I will send later.
Tested on Pine H64, Pine64-LTS, OrangePi Zero, OrangePi PC 2, BananaPi M64,
BananaPi M1.

Cheers,
Andre

 drivers/phy/allwinner/phy-sun4i-usb.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
index 5723c980323..e6ceafc7648 100644
--- a/drivers/phy/allwinner/phy-sun4i-usb.c
+++ b/drivers/phy/allwinner/phy-sun4i-usb.c
@@ -313,9 +313,21 @@ static int sun4i_usb_phy_init(struct phy *phy)
 				    data->cfg->disc_thresh, PHY_DISCON_TH_LEN);
 	}
 
-	sun4i_usb_phy_passby(phy, true);
+	if (IS_ENABLED(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);
+	}
 
 	return 0;
 }
-- 
2.17.5

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

[Paul Kocialkowski]

[-- Attachment #1: Type: text/plain, Size: 3122 bytes --]

Hi Andre,

Le Wed 26 May 21, 01:57, Andre Przywara a écrit :
> From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> 
> 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 host support
> for port 0.
> 
> 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 host support for port 0 on platforms with PHY0 dual-route,
> especially on boards like Pine64 (with only USB-A host ports) and
> TV boxes without OTG ports.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> [Andre: tweak commit message, use IS_ENABLED()]
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> Hi,
> 
> for H6 boards to work this requires a DT update (to get the <&usbphy 0>
> links between HCI and PHY), which I will send later.
> Tested on Pine H64, Pine64-LTS, OrangePi Zero, OrangePi PC 2, BananaPi M64,
> BananaPi M1.

Thanks for resending this, I've also had to revive this patch lately to get
USB working on the V3 so I definitely second that it's still relevant!

Paul

> Cheers,
> Andre
> 
>  drivers/phy/allwinner/phy-sun4i-usb.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> index 5723c980323..e6ceafc7648 100644
> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> @@ -313,9 +313,21 @@ static int sun4i_usb_phy_init(struct phy *phy)
>  				    data->cfg->disc_thresh, PHY_DISCON_TH_LEN);
>  	}
>  
> -	sun4i_usb_phy_passby(phy, true);
> +	if (IS_ENABLED(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);
> +	}
>  
>  	return 0;
>  }
> -- 
> 2.17.5
> 

-- 
Developer of free digital technology and hardware support.

Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Andre Przywara]

On Wed, 26 May 2021 12:48:27 +0200
Paul Kocialkowski <contact@paulk.fr> wrote:

Hi Paul,

> Le Wed 26 May 21, 01:57, Andre Przywara a écrit :
> > From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > 
> > 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 host support
> > for port 0.
> > 
> > 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 host support for port 0 on platforms with PHY0 dual-route,
> > especially on boards like Pine64 (with only USB-A host ports) and
> > TV boxes without OTG ports.
> > 
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > [Andre: tweak commit message, use IS_ENABLED()]
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> > Hi,
> > 
> > for H6 boards to work this requires a DT update (to get the <&usbphy 0>
> > links between HCI and PHY), which I will send later.
> > Tested on Pine H64, Pine64-LTS, OrangePi Zero, OrangePi PC 2, BananaPi M64,
> > BananaPi M1.  
> 
> Thanks for resending this, I've also had to revive this patch lately to get
> USB working on the V3 so I definitely second that it's still relevant!

Great! I had this under observation for quite a while, but was somewhat
puzzled because your original commit message mentioned that OTG was
broken, which worked fine for me. Maybe this was fixed meanwhile?

But what was still broken for me is host port 0, which disables one of
the two USB-A ports on the Pine64 boards (Pine64+, Pine64-LTS,
Pine-H64), also on those TV boxes. This makes connecting a keyboard and
an USB stick at the same time complicated.
But even with this patch alone it doesn't work, because the DTs are
(were) missing the phys property for ehci0/ohci0 (which I meanwhile
fixed in Linux, for exactly that reason). We synced the A64 DTs already
back into U-Boot, and I will send the H3/H5/H6 parts today.

So can you confirm that this now works for you, ideally with both OTG
and host ports on the same device? I tried this with enforcing OTG
on the Pine64, but would love to see reports from others.
Please respond with a Tested-by: tag then!

Thanks!
Andre

P.S. I kept your bootlin address as the author (that's what we do at
work in those cases: keep attribution to the employer sponsoring the
work at the time), please let me know if I should change this. I can add
your current address in some other tag, to have a working contact.

> 
> Paul
> 
> > Cheers,
> > Andre
> > 
> >  drivers/phy/allwinner/phy-sun4i-usb.c | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> > index 5723c980323..e6ceafc7648 100644
> > --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> > +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> > @@ -313,9 +313,21 @@ static int sun4i_usb_phy_init(struct phy *phy)
> >  				    data->cfg->disc_thresh, PHY_DISCON_TH_LEN);
> >  	}
> >  
> > -	sun4i_usb_phy_passby(phy, true);
> > +	if (IS_ENABLED(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);
> > +	}
> >  
> >  	return 0;
> >  }
> > -- 
> > 2.17.5
> >   
> 

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

[Jagan Teki]

On Wed, May 26, 2021 at 6:27 AM Andre Przywara <andre.przywara@arm.com> wrote:
>
> From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
>
> 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 host support
> for port 0.
>
> 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 host support for port 0 on platforms with PHY0 dual-route,
> especially on boards like Pine64 (with only USB-A host ports) and
> TV boxes without OTG ports.
>
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> [Andre: tweak commit message, use IS_ENABLED()]
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> Hi,
>
> for H6 boards to work this requires a DT update (to get the <&usbphy 0>
> links between HCI and PHY), which I will send later.
> Tested on Pine H64, Pine64-LTS, OrangePi Zero, OrangePi PC 2, BananaPi M64,
> BananaPi M1.
>
> Cheers,
> Andre
>
>  drivers/phy/allwinner/phy-sun4i-usb.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> index 5723c980323..e6ceafc7648 100644
> --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> @@ -313,9 +313,21 @@ static int sun4i_usb_phy_init(struct phy *phy)
>                                     data->cfg->disc_thresh, PHY_DISCON_TH_LEN);
>         }
>
> -       sun4i_usb_phy_passby(phy, true);
> +       if (IS_ENABLED(CONFIG_USB_MUSB_SUNXI)) {

I believe i did comment this before to use driver_data flag as this is
full dm driver instead of macro style.

Jagan.

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

[Andre Przywara]

On Wed, 26 May 2021 22:37:14 +0530
Jagan Teki <jagan@amarulasolutions.com> wrote:

Hi Jagan,

thanks for having a look!

> On Wed, May 26, 2021 at 6:27 AM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> >
> > 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 host support
> > for port 0.
> >
> > 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 host support for port 0 on platforms with PHY0 dual-route,
> > especially on boards like Pine64 (with only USB-A host ports) and
> > TV boxes without OTG ports.
> >
> > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > [Andre: tweak commit message, use IS_ENABLED()]
> > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > ---
> > Hi,
> >
> > for H6 boards to work this requires a DT update (to get the <&usbphy 0>
> > links between HCI and PHY), which I will send later.
> > Tested on Pine H64, Pine64-LTS, OrangePi Zero, OrangePi PC 2, BananaPi M64,
> > BananaPi M1.
> >
> > Cheers,
> > Andre
> >
> >  drivers/phy/allwinner/phy-sun4i-usb.c | 16 ++++++++++++++--
> >  1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> > index 5723c980323..e6ceafc7648 100644
> > --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> > +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> > @@ -313,9 +313,21 @@ static int sun4i_usb_phy_init(struct phy *phy)
> >                                     data->cfg->disc_thresh, PHY_DISCON_TH_LEN);
> >         }
> >
> > -       sun4i_usb_phy_passby(phy, true);
> > +       if (IS_ENABLED(CONFIG_USB_MUSB_SUNXI)) {  
> 
> I believe i did comment this before to use driver_data flag as this is
> full dm driver instead of macro style.

Which driver_data field would that be? This is not about a particular
SoC's PHY, this is about whether we use peripheral or host mode for
controller 0. As Paul mentioned in the commit message above:

"... Since host/device mode detection is not available from the PHY
code and because U-Boot does not support changing the mode dynamically
anyway, ...."

So a possible alternative would be to look up the dr_mode property in
the DT node. BUT: this property lives in the musb DT node, not in this
node the PHY driver knows about. Happy to take a patch that makes the
connection and looks that up. But I am not sure that covers all cases.

Meanwhile a equivalent and MUCH simpler solution is to use the Kconfig
symbol for the MUSB driver: as Paul correctly mentioned, this is a
static decision: only one of them can be effectively active in a build,
and inclusion of the MUSB driver wins over the host controller. So
using this symbol as a switch seems to be the best solution to me.

Cheers,
Andre

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

[Jagan Teki]

On Wed, May 26, 2021 at 11:02 PM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Wed, 26 May 2021 22:37:14 +0530
> Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Hi Jagan,
>
> thanks for having a look!
>
> > On Wed, May 26, 2021 at 6:27 AM Andre Przywara <andre.przywara@arm.com> wrote:
> > >
> > > From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > >
> > > 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 host support
> > > for port 0.
> > >
> > > 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 host support for port 0 on platforms with PHY0 dual-route,
> > > especially on boards like Pine64 (with only USB-A host ports) and
> > > TV boxes without OTG ports.
> > >
> > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > [Andre: tweak commit message, use IS_ENABLED()]
> > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > ---
> > > Hi,
> > >
> > > for H6 boards to work this requires a DT update (to get the <&usbphy 0>
> > > links between HCI and PHY), which I will send later.
> > > Tested on Pine H64, Pine64-LTS, OrangePi Zero, OrangePi PC 2, BananaPi M64,
> > > BananaPi M1.
> > >
> > > Cheers,
> > > Andre
> > >
> > >  drivers/phy/allwinner/phy-sun4i-usb.c | 16 ++++++++++++++--
> > >  1 file changed, 14 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> > > index 5723c980323..e6ceafc7648 100644
> > > --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> > > +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> > > @@ -313,9 +313,21 @@ static int sun4i_usb_phy_init(struct phy *phy)
> > >                                     data->cfg->disc_thresh, PHY_DISCON_TH_LEN);
> > >         }
> > >
> > > -       sun4i_usb_phy_passby(phy, true);
> > > +       if (IS_ENABLED(CONFIG_USB_MUSB_SUNXI)) {
> >
> > I believe i did comment this before to use driver_data flag as this is
> > full dm driver instead of macro style.
>
> Which driver_data field would that be? This is not about a particular
> SoC's PHY, this is about whether we use peripheral or host mode for
> controller 0. As Paul mentioned in the commit message above:
>
> "... Since host/device mode detection is not available from the PHY
> code and because U-Boot does not support changing the mode dynamically
> anyway, ...."

Yeah., I missed it. Thanks for pointing it out.


>
> So a possible alternative would be to look up the dr_mode property in
> the DT node. BUT: this property lives in the musb DT node, not in this
> node the PHY driver knows about. Happy to take a patch that makes the
> connection and looks that up. But I am not sure that covers all cases.
>
> Meanwhile a equivalent and MUCH simpler solution is to use the Kconfig
> symbol for the MUSB driver: as Paul correctly mentioned, this is a
> static decision: only one of them can be effectively active in a build,
> and inclusion of the MUSB driver wins over the host controller. So
> using this symbol as a switch seems to be the best solution to me.

Handling dr_mode can be possible in U-Boot, I did tried but not
completed as patch.
drivers/usb/musb-new/ti-musb.c has base code for ti musb chips.

May be supporting that would handle this case.

Jagan.

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

[Andre Przywara]

On Wed, 26 May 2021 23:51:41 +0530
Jagan Teki <jagan@amarulasolutions.com> wrote:

Hi,

> On Wed, May 26, 2021 at 11:02 PM Andre Przywara <andre.przywara@arm.com> wrote:
> >
> > On Wed, 26 May 2021 22:37:14 +0530
> > Jagan Teki <jagan@amarulasolutions.com> wrote:
> >
> > Hi Jagan,
> >
> > thanks for having a look!
> >  
> > > On Wed, May 26, 2021 at 6:27 AM Andre Przywara <andre.przywara@arm.com> wrote:  
> > > >
> > > > From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > >
> > > > 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 host support
> > > > for port 0.
> > > >
> > > > 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 host support for port 0 on platforms with PHY0 dual-route,
> > > > especially on boards like Pine64 (with only USB-A host ports) and
> > > > TV boxes without OTG ports.
> > > >
> > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > [Andre: tweak commit message, use IS_ENABLED()]
> > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > > ---
> > > > Hi,
> > > >
> > > > for H6 boards to work this requires a DT update (to get the <&usbphy 0>
> > > > links between HCI and PHY), which I will send later.
> > > > Tested on Pine H64, Pine64-LTS, OrangePi Zero, OrangePi PC 2, BananaPi M64,
> > > > BananaPi M1.
> > > >
> > > > Cheers,
> > > > Andre
> > > >
> > > >  drivers/phy/allwinner/phy-sun4i-usb.c | 16 ++++++++++++++--
> > > >  1 file changed, 14 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> > > > index 5723c980323..e6ceafc7648 100644
> > > > --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> > > > +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> > > > @@ -313,9 +313,21 @@ static int sun4i_usb_phy_init(struct phy *phy)
> > > >                                     data->cfg->disc_thresh, PHY_DISCON_TH_LEN);
> > > >         }
> > > >
> > > > -       sun4i_usb_phy_passby(phy, true);
> > > > +       if (IS_ENABLED(CONFIG_USB_MUSB_SUNXI)) {  
> > >
> > > I believe i did comment this before to use driver_data flag as this is
> > > full dm driver instead of macro style.  
> >
> > Which driver_data field would that be? This is not about a particular
> > SoC's PHY, this is about whether we use peripheral or host mode for
> > controller 0. As Paul mentioned in the commit message above:
> >
> > "... Since host/device mode detection is not available from the PHY
> > code and because U-Boot does not support changing the mode dynamically
> > anyway, ...."  
> 
> Yeah., I missed it. Thanks for pointing it out.
> 
> 
> >
> > So a possible alternative would be to look up the dr_mode property in
> > the DT node. BUT: this property lives in the musb DT node, not in this
> > node the PHY driver knows about. Happy to take a patch that makes the
> > connection and looks that up. But I am not sure that covers all cases.
> >
> > Meanwhile a equivalent and MUCH simpler solution is to use the Kconfig
> > symbol for the MUSB driver: as Paul correctly mentioned, this is a
> > static decision: only one of them can be effectively active in a build,
> > and inclusion of the MUSB driver wins over the host controller. So
> > using this symbol as a switch seems to be the best solution to me.  
> 
> Handling dr_mode can be possible in U-Boot, I did tried but not
> completed as patch.
> drivers/usb/musb-new/ti-musb.c has base code for ti musb chips.

Sure, there are certainly ways to do that. As I said: patches welcome!

But given that this patch here is around for 2 years now and fixes a
real problem - without any downsides, as far as I can tell - I would
rather take this first.
And while it sounds indeed cleaner to look at dr_mode, there is more to
it for it to be really useful: we probably need some form of dynamic
switching between peripheral and host mode, either by code (user calls
fastboot vs. user calls "usb reset"), or by sampling the ID pin.
And even if not, how would we deal with the case when dr_mode says
peripheral, but the MUSB driver is not compiled in?
That's why looking at CONFIG_USB_MUSB_SUNXI is actually a quite clever
and easy solution, at least for now.

Cheers,
Andre

> May be supporting that would handle this case.
> 
> Jagan.

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

[Jagan Teki]

On Thu, May 27, 2021 at 12:38 AM Andre Przywara <andre.przywara@arm.com> wrote:
>
> On Wed, 26 May 2021 23:51:41 +0530
> Jagan Teki <jagan@amarulasolutions.com> wrote:
>
> Hi,
>
> > On Wed, May 26, 2021 at 11:02 PM Andre Przywara <andre.przywara@arm.com> wrote:
> > >
> > > On Wed, 26 May 2021 22:37:14 +0530
> > > Jagan Teki <jagan@amarulasolutions.com> wrote:
> > >
> > > Hi Jagan,
> > >
> > > thanks for having a look!
> > >
> > > > On Wed, May 26, 2021 at 6:27 AM Andre Przywara <andre.przywara@arm.com> wrote:
> > > > >
> > > > > From: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > >
> > > > > 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 host support
> > > > > for port 0.
> > > > >
> > > > > 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 host support for port 0 on platforms with PHY0 dual-route,
> > > > > especially on boards like Pine64 (with only USB-A host ports) and
> > > > > TV boxes without OTG ports.
> > > > >
> > > > > Signed-off-by: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> > > > > [Andre: tweak commit message, use IS_ENABLED()]
> > > > > Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> > > > > ---
> > > > > Hi,
> > > > >
> > > > > for H6 boards to work this requires a DT update (to get the <&usbphy 0>
> > > > > links between HCI and PHY), which I will send later.
> > > > > Tested on Pine H64, Pine64-LTS, OrangePi Zero, OrangePi PC 2, BananaPi M64,
> > > > > BananaPi M1.
> > > > >
> > > > > Cheers,
> > > > > Andre
> > > > >
> > > > >  drivers/phy/allwinner/phy-sun4i-usb.c | 16 ++++++++++++++--
> > > > >  1 file changed, 14 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/phy/allwinner/phy-sun4i-usb.c b/drivers/phy/allwinner/phy-sun4i-usb.c
> > > > > index 5723c980323..e6ceafc7648 100644
> > > > > --- a/drivers/phy/allwinner/phy-sun4i-usb.c
> > > > > +++ b/drivers/phy/allwinner/phy-sun4i-usb.c
> > > > > @@ -313,9 +313,21 @@ static int sun4i_usb_phy_init(struct phy *phy)
> > > > >                                     data->cfg->disc_thresh, PHY_DISCON_TH_LEN);
> > > > >         }
> > > > >
> > > > > -       sun4i_usb_phy_passby(phy, true);
> > > > > +       if (IS_ENABLED(CONFIG_USB_MUSB_SUNXI)) {
> > > >
> > > > I believe i did comment this before to use driver_data flag as this is
> > > > full dm driver instead of macro style.
> > >
> > > Which driver_data field would that be? This is not about a particular
> > > SoC's PHY, this is about whether we use peripheral or host mode for
> > > controller 0. As Paul mentioned in the commit message above:
> > >
> > > "... Since host/device mode detection is not available from the PHY
> > > code and because U-Boot does not support changing the mode dynamically
> > > anyway, ...."
> >
> > Yeah., I missed it. Thanks for pointing it out.
> >
> >
> > >
> > > So a possible alternative would be to look up the dr_mode property in
> > > the DT node. BUT: this property lives in the musb DT node, not in this
> > > node the PHY driver knows about. Happy to take a patch that makes the
> > > connection and looks that up. But I am not sure that covers all cases.
> > >
> > > Meanwhile a equivalent and MUCH simpler solution is to use the Kconfig
> > > symbol for the MUSB driver: as Paul correctly mentioned, this is a
> > > static decision: only one of them can be effectively active in a build,
> > > and inclusion of the MUSB driver wins over the host controller. So
> > > using this symbol as a switch seems to be the best solution to me.
> >
> > Handling dr_mode can be possible in U-Boot, I did tried but not
> > completed as patch.
> > drivers/usb/musb-new/ti-musb.c has base code for ti musb chips.
>
> Sure, there are certainly ways to do that. As I said: patches welcome!
>
> But given that this patch here is around for 2 years now and fixes a
> real problem - without any downsides, as far as I can tell - I would
> rather take this first.
> And while it sounds indeed cleaner to look at dr_mode, there is more to
> it for it to be really useful: we probably need some form of dynamic
> switching between peripheral and host mode, either by code (user calls
> fastboot vs. user calls "usb reset"), or by sampling the ID pin.
> And even if not, how would we deal with the case when dr_mode says
> peripheral, but the MUSB driver is not compiled in?
> That's why looking at CONFIG_USB_MUSB_SUNXI is actually a quite clever
> and easy solution, at least for now.

Agreed!