Re: [PATCH] usb: dwc3: exynos: remove usb_phy_generic support

[Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>]

Hi,

On Friday, August 29, 2014 11:33:04 AM Greg Kroah-Hartman wrote:
> On Fri, Aug 29, 2014 at 11:02:52AM +0200, Bartlomiej Zolnierkiewicz wrote:
> > On Thursday, August 28, 2014 12:29:52 PM Greg Kroah-Hartman wrote:
> > > On Thu, Aug 28, 2014 at 08:11:04PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > > > 
> > > > [ added Alan and Greg to cc: ]
> > > > 
> > > > Hi,
> > > > 
> > > > On Wednesday, August 27, 2014 11:42:25 PM Vivek Gautam wrote:
> > > > > Hi Baltlomiej,
> > > > > 
> > > > > 
> > > > > On Wed, Aug 27, 2014 at 1:22 PM, Bartlomiej Zolnierkiewicz
> > > > > <b.zolnierkie@samsung.com> wrote:
> > > > > > dwc3 driver is using the new Exynos5 SoC series USB DRD PHY driver
> > > > > > (PHY_EXYNOS5_USBDRD which selects GENERIC_PHY) as can be seen by
> > > > > > looking at the following commits:
> > > > > >
> > > > > >   7a4cf0fde054 ("ARM: dts: Update DWC3 usb controller to use new
> > > > > >   phy driver for exynos5250")
> > > > > >
> > > > > >   f070267b5fc1 ("ARM: dts: Enable support for DWC3 controller for
> > > > > >   exynos5420")
> > > > > >
> > > > > > Thus remove unused usb_phy_generic support from dwc3 Exynos glue
> > > > > > layer.
> > > > > >
> > > > > > [ The code that is being removed is harmful in the context of
> > > > > >   multi_v7_defconfig and enabling "EHCI support for Samsung
> > > > > >   S5P/EXYNOS SoC Series" (USB_EHCI_EXYNOS) + "OHCI support for
> > > > > >   Samsung S5P/EXYNOS SoC Series" (USB_OHCI_EXYNOS) because "EHCI
> > > > > >   support for OMAP3 and later chips" (USB_EHCI_HCD_OMAP) selects
> > > > > >   "NOP USB Transceiver Driver" (NOP_USB_XCEIV).  NOP USB driver
> > > > > >   attaches itself to usb_phy_generic platform devices created by
> > > > > >   dwc3 Exynos glue layer and later causes Exynos EHCI driver to
> > > > > >   fail probe and Exynos OHCI driver to hang on probe (as observed
> > > > > >   on Exynos5250 Arndale board). ]
> > > > > 
> > > > > The issue with EHCI and OHCI on exynos platforms is that until now
> > > > > they also request
> > > > > usb-phy and only later if they don't find one, they go ahead and get a
> > > > > 'phy' generic.
> > > > > 
> > > > > Fortunately we missed this issue with exynos_defconfig, and as you rightly
> > > > > mentioned with multi_v7_defconfig, the NOP_USB_XCEIV gets enabled and
> > > > > EHCI and OHCI exynos get no-op usb-phy, which actually blocks EHCI/OHCI from
> > > > > initializing the real PHYs.
> > > > > 
> > > > > This issue is resolved with patches:
> > > > > [PATCH v2 1/2] usb: host: ehci-exynos: Remove unnecessary usb-phy support
> > > > > [PATCH v2 2/2] usb: host: ohci-exynos: Remove unnecessary usb-phy support
> > > > > wherein we are completely removing the usb-phy support from ehci/ohci-exynos.
> > > > > So with these patches we should not see the issue mentioned by you.
> > > > 
> > > > Indeed, your patches fix the issue.
> > > > 
> > > > Greg, could these two patches ([1] & [2]) get merged quickly, pretty please
> > > > (they were already acked by Alan)?  They are not a mere cleanups because
> > > > they fix the actual problem with using multi_v7_defconfig which in turn has
> > > > been blocking Olof's defconfig update patch [3] for quite some time now.
> > > > Moreover these patches are limited to Exynos host drivers so they should be
> > > > pretty safe when it comes to potential regressions.
> > > > 
> > > > [1] http://www.spinics.net/lists/linux-usb/msg112294.html
> > > > [2] http://www.spinics.net/lists/linux-usb/msg112293.html
> > > > [3] http://www.spinics.net/lists/arm-kernel/msg349654.html
> > > 
> > > merged for 3.18-rc1, or do you "need" them for 3.17-final?
> > 
> > If it is not too much trouble please push them to 3.17-final.
> 
> They don't meet the "regression or bugfix" rule at all, so I can't do
> this, sorry.  I'll queue them up for 3.18.

These patches fix a real problem of boot hang when enabling Exynos USB
host drivers and using ARM multiplatform config so IMHO they fall into
bugfix category.

> > > I already reverted one patch for exynos for 3.17-final that is sitting
> > > in my tree to go to Linus soon as you all didn't seem to want it
> > > anymore, so I'm getting really confused here...
> > 
> > These two patches are a replacement for the one reverted and
> > they just remove the old code instead of keeping it as fallback.
> > This means that the reverted patch was not breaking anything and
> > these two new patches could have been also done as incremental
> > ones.  Sorry for the confusion.
> 
> As they came in too late for 3.17-rc1, they will have to wait for
> 3.18-rc1.

Okay..

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics