| Message ID | 1372919202-26755-1-git-send-email-ruchika@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 07/04/2013 01:26 AM, Ruchika Kharwar wrote: > DRA7XX has several USB OTG subsystems. USB_OTG_SS1 includes a USB1 and USB2 > phy. USB_OTG_SS2 includes only a USB2 phy. > This patch allows the dwc3 probe to continue if a usb3_phy is not found. The need for this will go away as soon as "of_get_maximum_speed" is introduced. The speed can then be used to determine if a USB3 phy is required or not for dra7xx as well. > > This patch currently works for DRA7XX and submitted in the interim a nicer > method emerges. > > Signed-off-by: Ruchika Kharwar <ruchika@ti.com> > --- > drivers/usb/dwc3/core.c | 38 ++++++++++++++++++++------------------ > drivers/usb/dwc3/gadget.c | 11 ++++++++--- > 2 files changed, 28 insertions(+), 21 deletions(-) > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 358375e..feea92d 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -100,7 +100,10 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc) > dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); > > usb_phy_init(dwc->usb2_phy); > - usb_phy_init(dwc->usb3_phy); > + > + if (!IS_ERR(dwc->usb3_phy)) > + usb_phy_init(dwc->usb3_phy); > + > mdelay(100); > > /* Clear USB3 PHY reset */ > @@ -360,7 +363,9 @@ err0: > static void dwc3_core_exit(struct dwc3 *dwc) > { > usb_phy_shutdown(dwc->usb2_phy); > - usb_phy_shutdown(dwc->usb3_phy); > + > + if (!IS_ERR(dwc->usb3_phy)) > + usb_phy_shutdown(dwc->usb3_phy); > } > > #define DWC3_ALIGN_MASK (16 - 1) > @@ -450,22 +455,13 @@ static int dwc3_probe(struct platform_device *pdev) > } > > if (IS_ERR(dwc->usb3_phy)) { > - ret = PTR_ERR(dwc->usb3_phy); > - > - /* > - * if -ENXIO is returned, it means PHY layer wasn't > - * enabled, so it makes no sense to return -EPROBE_DEFER > - * in that case, since no PHY driver will ever probe. > - */ > - if (ret == -ENXIO) > - return ret; > - > - dev_err(dev, "no usb3 phy configured\n"); > - return -EPROBE_DEFER; > + dev_dbg(dev, "no usb3 phy configured, possibly absent\n"); > } > > usb_phy_set_suspend(dwc->usb2_phy, 0); > - usb_phy_set_suspend(dwc->usb3_phy, 0); > + > + if (!IS_ERR(dwc->usb3_phy)) > + usb_phy_set_suspend(dwc->usb3_phy, 0); > > spin_lock_init(&dwc->lock); > platform_set_drvdata(pdev, dwc); > @@ -604,7 +600,9 @@ static int dwc3_remove(struct platform_device *pdev) > struct dwc3 *dwc = platform_get_drvdata(pdev); > > usb_phy_set_suspend(dwc->usb2_phy, 1); > - usb_phy_set_suspend(dwc->usb3_phy, 1); > + > + if (!IS_ERR(dwc->usb3_phy)) > + usb_phy_set_suspend(dwc->usb3_phy, 1); > > pm_runtime_put(&pdev->dev); > pm_runtime_disable(&pdev->dev); > @@ -700,7 +698,9 @@ static int dwc3_suspend(struct device *dev) > dwc->gctl = dwc3_readl(dwc->regs, DWC3_GCTL); > spin_unlock_irqrestore(&dwc->lock, flags); > > - usb_phy_shutdown(dwc->usb3_phy); > + if (!IS_ERR(dwc->usb3_phy)) > + usb_phy_shutdown(dwc->usb3_phy); > + > usb_phy_shutdown(dwc->usb2_phy); > > return 0; > @@ -711,7 +711,9 @@ static int dwc3_resume(struct device *dev) > struct dwc3 *dwc = dev_get_drvdata(dev); > unsigned long flags; > > - usb_phy_init(dwc->usb3_phy); > + if (!IS_ERR(dwc->usb3_phy)) > + usb_phy_init(dwc->usb3_phy); > + > usb_phy_init(dwc->usb2_phy); > msleep(100); > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c > index b5e5b35..7ca3745 100644 > --- a/drivers/usb/dwc3/gadget.c > +++ b/drivers/usb/dwc3/gadget.c > @@ -2176,7 +2176,9 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc) > if (dwc->revision < DWC3_REVISION_194A) { > /* Resume PHYs */ > dwc3_gadget_usb2_phy_suspend(dwc, false); > - dwc3_gadget_usb3_phy_suspend(dwc, false); > + > + if (!IS_ERR(dwc->usb3_phy)) > + dwc3_gadget_usb3_phy_suspend(dwc, false); > } > > if (dwc->gadget.speed != USB_SPEED_UNKNOWN) > @@ -2231,7 +2233,8 @@ static void dwc3_gadget_phy_suspend(struct dwc3 *dwc, u8 speed) > case USB_SPEED_HIGH: > case USB_SPEED_FULL: > case USB_SPEED_LOW: > - dwc3_gadget_usb3_phy_suspend(dwc, true); > + if (!IS_ERR(dwc->usb3_phy)) > + dwc3_gadget_usb3_phy_suspend(dwc, true); > break; > } > } > @@ -2649,7 +2652,9 @@ int dwc3_gadget_init(struct dwc3 *dwc) > /* Enable USB2 LPM and automatic phy suspend only on recent versions */ > if (dwc->revision >= DWC3_REVISION_194A) { > dwc3_gadget_usb2_phy_suspend(dwc, false); > - dwc3_gadget_usb3_phy_suspend(dwc, false); > + > + if (!IS_ERR(dwc->usb3_phy)) > + dwc3_gadget_usb3_phy_suspend(dwc, false); > } > > ret = usb_add_gadget_udc(dwc->dev, &dwc->gadget); -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 05, 2013 at 09:35:05AM -0500, Ruchika Kharwar wrote: > > On 07/04/2013 01:26 AM, Ruchika Kharwar wrote: > >DRA7XX has several USB OTG subsystems. USB_OTG_SS1 includes a USB1 and USB2 > >phy. USB_OTG_SS2 includes only a USB2 phy. > >This patch allows the dwc3 probe to continue if a usb3_phy is not found. > The need for this will go away as soon as "of_get_maximum_speed" is > introduced. The speed can then be used to determine if a USB3 phy is > required or not for dra7xx as well. there is one problem with this approach (even my approach which I had cooked before). I was testing OMAP5 today (finally got most pieces in mainline, so I could test something with little out of tree), and as it turns out, the USB3PHY DPLL is used to generate the PIPE3 clock and that clock is used to power some important part of the IP. From what I could grasp out of OMAP5 ES2.0 TRM and Synopsys databook, I believe OMAP5 decided to tie ram_clk to pipe3_clk (the PIP3 bus clock) and if we let USB3 PHY be optional on OMAP5, we might end up with broken DTS files which cause device not to work. That aside, what caught my attention is how the problem exposes itself: We end up in a situation where DWC3_DEPCMD_SETEPCONFIG never completes although the previous DWC3_DEPCMD_DEPSTARTCFG completes just fine. This happens early on, when we're trying to enable physical endpoint 0, then the driver bails out. I'm assuming that DWC3_DEPCMD_DEPSTARTCFG is very easy to process, maybe it just puts the internal FSM into a post-idle state which simply waits for the endpoint configuration to come through the registers while, in turn, DWC3_DEPCMD_DEPSTARTCFG is far more complex to process and needs other parts of the IP to be fully powered and clocked in order to complete. If my speculation is correct thus far, it also means that those blocks are somehow powered by pipe3_clk (or a derivation of it). It would be nice if SNPS could shed some light here, but I'll continue digging the docs, for now the feature of optional USB3PHY will be pending in a separate branch while I try to get all information related to this problem. The decision came after figuring out that if we don't usb_phy_init(dwc->usb3_phy), OMAP5 would never, ever work because the USB3 PHY DPLL is locked in ->init() callback of that driver in ordre to conserve power. cheers
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index 358375e..feea92d 100644 --- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -100,7 +100,10 @@ static void dwc3_core_soft_reset(struct dwc3 *dwc) dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg); usb_phy_init(dwc->usb2_phy); - usb_phy_init(dwc->usb3_phy); + + if (!IS_ERR(dwc->usb3_phy)) + usb_phy_init(dwc->usb3_phy); + mdelay(100); /* Clear USB3 PHY reset */ @@ -360,7 +363,9 @@ err0: static void dwc3_core_exit(struct dwc3 *dwc) { usb_phy_shutdown(dwc->usb2_phy); - usb_phy_shutdown(dwc->usb3_phy); + + if (!IS_ERR(dwc->usb3_phy)) + usb_phy_shutdown(dwc->usb3_phy); } #define DWC3_ALIGN_MASK (16 - 1) @@ -450,22 +455,13 @@ static int dwc3_probe(struct platform_device *pdev) } if (IS_ERR(dwc->usb3_phy)) { - ret = PTR_ERR(dwc->usb3_phy); - - /* - * if -ENXIO is returned, it means PHY layer wasn't - * enabled, so it makes no sense to return -EPROBE_DEFER - * in that case, since no PHY driver will ever probe. - */ - if (ret == -ENXIO) - return ret; - - dev_err(dev, "no usb3 phy configured\n"); - return -EPROBE_DEFER; + dev_dbg(dev, "no usb3 phy configured, possibly absent\n"); } usb_phy_set_suspend(dwc->usb2_phy, 0); - usb_phy_set_suspend(dwc->usb3_phy, 0); + + if (!IS_ERR(dwc->usb3_phy)) + usb_phy_set_suspend(dwc->usb3_phy, 0); spin_lock_init(&dwc->lock); platform_set_drvdata(pdev, dwc); @@ -604,7 +600,9 @@ static int dwc3_remove(struct platform_device *pdev) struct dwc3 *dwc = platform_get_drvdata(pdev); usb_phy_set_suspend(dwc->usb2_phy, 1); - usb_phy_set_suspend(dwc->usb3_phy, 1); + + if (!IS_ERR(dwc->usb3_phy)) + usb_phy_set_suspend(dwc->usb3_phy, 1); pm_runtime_put(&pdev->dev); pm_runtime_disable(&pdev->dev); @@ -700,7 +698,9 @@ static int dwc3_suspend(struct device *dev) dwc->gctl = dwc3_readl(dwc->regs, DWC3_GCTL); spin_unlock_irqrestore(&dwc->lock, flags); - usb_phy_shutdown(dwc->usb3_phy); + if (!IS_ERR(dwc->usb3_phy)) + usb_phy_shutdown(dwc->usb3_phy); + usb_phy_shutdown(dwc->usb2_phy); return 0; @@ -711,7 +711,9 @@ static int dwc3_resume(struct device *dev) struct dwc3 *dwc = dev_get_drvdata(dev); unsigned long flags; - usb_phy_init(dwc->usb3_phy); + if (!IS_ERR(dwc->usb3_phy)) + usb_phy_init(dwc->usb3_phy); + usb_phy_init(dwc->usb2_phy); msleep(100); diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c index b5e5b35..7ca3745 100644 --- a/drivers/usb/dwc3/gadget.c +++ b/drivers/usb/dwc3/gadget.c @@ -2176,7 +2176,9 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc) if (dwc->revision < DWC3_REVISION_194A) { /* Resume PHYs */ dwc3_gadget_usb2_phy_suspend(dwc, false); - dwc3_gadget_usb3_phy_suspend(dwc, false); + + if (!IS_ERR(dwc->usb3_phy)) + dwc3_gadget_usb3_phy_suspend(dwc, false); } if (dwc->gadget.speed != USB_SPEED_UNKNOWN) @@ -2231,7 +2233,8 @@ static void dwc3_gadget_phy_suspend(struct dwc3 *dwc, u8 speed) case USB_SPEED_HIGH: case USB_SPEED_FULL: case USB_SPEED_LOW: - dwc3_gadget_usb3_phy_suspend(dwc, true); + if (!IS_ERR(dwc->usb3_phy)) + dwc3_gadget_usb3_phy_suspend(dwc, true); break; } } @@ -2649,7 +2652,9 @@ int dwc3_gadget_init(struct dwc3 *dwc) /* Enable USB2 LPM and automatic phy suspend only on recent versions */ if (dwc->revision >= DWC3_REVISION_194A) { dwc3_gadget_usb2_phy_suspend(dwc, false); - dwc3_gadget_usb3_phy_suspend(dwc, false); + + if (!IS_ERR(dwc->usb3_phy)) + dwc3_gadget_usb3_phy_suspend(dwc, false); } ret = usb_add_gadget_udc(dwc->dev, &dwc->gadget);
DRA7XX has several USB OTG subsystems. USB_OTG_SS1 includes a USB1 and USB2 phy. USB_OTG_SS2 includes only a USB2 phy. This patch allows the dwc3 probe to continue if a usb3_phy is not found. This patch currently works for DRA7XX and submitted in the interim a nicer method emerges. Signed-off-by: Ruchika Kharwar <ruchika@ti.com> --- drivers/usb/dwc3/core.c | 38 ++++++++++++++++++++------------------ drivers/usb/dwc3/gadget.c | 11 ++++++++--- 2 files changed, 28 insertions(+), 21 deletions(-)