From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paul Kocialkowski Date: Mon, 15 Jul 2019 09:31:51 +0200 Subject: [U-Boot] [linux-sunxi] [PATCH] phy: sun4i-usb: Fix PHY0 routing and passby configuration for MUSB In-Reply-To: References: <20190314103845.7985-1-paul.kocialkowski@bootlin.com> <0415574a5d8efd727f0f8b114ae585f7e3b4c8c9.camel@bootlin.com> <0bbe9848-1747-5ac2-395d-8b7a4aafa195@arm.com> Message-ID: <20190715073151.GA21374@aptenodytes> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de Hi, On Mon 15 Jul 19, 12:55, Jagan Teki wrote: > On Mon, May 27, 2019 at 5:21 AM André Przywara wrote: > > > > On 17/04/2019 12:28, Jagan Teki wrote: > > > On Mon, Apr 15, 2019 at 1:52 PM Paul Kocialkowski > > > 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 > > >>> 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 > > >>>> --- > > >>>> 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