| Message ID | 2141307.zYQLjP6V6U@amdc1032 (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wednesday, August 27, 2014 4:53 PM, Bartlomiej Zolnierkiewicz 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). ] I also agree with this patch, because dwc3 IPs of all Exynos SoCs do not use "NOP USB Transceiver Driver". So, "usb_phy_generic" can be removed from Exynos dwc3 driver. Is there any reason to support 'usb_phy_generic' for Exynos dwc3? If so, please let me know. Thank you. Best regards, Jingoo Han > > Cc: Olof Johansson <olof@lixom.net> > Cc: Kukjin Kim <kgene.kim@samsung.com> > Cc: Vivek Gautam <gautam.vivek@samsung.com> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > --- > drivers/usb/dwc3/dwc3-exynos.c | 68 ----------------------------------------- > 1 file changed, 1 insertion(+), 67 deletions(-) > > Index: b/drivers/usb/dwc3/dwc3-exynos.c > =================================================================== > --- a/drivers/usb/dwc3/dwc3-exynos.c 2014-08-25 14:57:04.991781925 +0200 > +++ b/drivers/usb/dwc3/dwc3-exynos.c 2014-08-27 09:16:38.312617727 +0200 > @@ -23,15 +23,12 @@ > #include <linux/platform_data/dwc3-exynos.h> > #include <linux/dma-mapping.h> > #include <linux/clk.h> > -#include <linux/usb/otg.h> > -#include <linux/usb/usb_phy_generic.h> > +#include <linux/pm_runtime.h> > #include <linux/of.h> > #include <linux/of_platform.h> > #include <linux/regulator/consumer.h> > > struct dwc3_exynos { > - struct platform_device *usb2_phy; > - struct platform_device *usb3_phy; > struct device *dev; > > struct clk *clk; > @@ -39,61 +36,6 @@ struct dwc3_exynos { > struct regulator *vdd10; > }; > > -static int dwc3_exynos_register_phys(struct dwc3_exynos *exynos) > -{ > - struct usb_phy_generic_platform_data pdata; > - struct platform_device *pdev; > - int ret; > - > - memset(&pdata, 0x00, sizeof(pdata)); > - > - pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO); > - if (!pdev) > - return -ENOMEM; > - > - exynos->usb2_phy = pdev; > - pdata.type = USB_PHY_TYPE_USB2; > - pdata.gpio_reset = -1; > - > - ret = platform_device_add_data(exynos->usb2_phy, &pdata, sizeof(pdata)); > - if (ret) > - goto err1; > - > - pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO); > - if (!pdev) { > - ret = -ENOMEM; > - goto err1; > - } > - > - exynos->usb3_phy = pdev; > - pdata.type = USB_PHY_TYPE_USB3; > - > - ret = platform_device_add_data(exynos->usb3_phy, &pdata, sizeof(pdata)); > - if (ret) > - goto err2; > - > - ret = platform_device_add(exynos->usb2_phy); > - if (ret) > - goto err2; > - > - ret = platform_device_add(exynos->usb3_phy); > - if (ret) > - goto err3; > - > - return 0; > - > -err3: > - platform_device_del(exynos->usb2_phy); > - > -err2: > - platform_device_put(exynos->usb3_phy); > - > -err1: > - platform_device_put(exynos->usb2_phy); > - > - return ret; > -} > - > static int dwc3_exynos_remove_child(struct device *dev, void *unused) > { > struct platform_device *pdev = to_platform_device(dev); > @@ -127,12 +69,6 @@ static int dwc3_exynos_probe(struct plat > > platform_set_drvdata(pdev, exynos); > > - ret = dwc3_exynos_register_phys(exynos); > - if (ret) { > - dev_err(dev, "couldn't register PHYs\n"); > - return ret; > - } > - > clk = devm_clk_get(dev, "usbdrd30"); > if (IS_ERR(clk)) { > dev_err(dev, "couldn't get clock\n"); > @@ -194,8 +130,6 @@ static int dwc3_exynos_remove(struct pla > struct dwc3_exynos *exynos = platform_get_drvdata(pdev); > > device_for_each_child(&pdev->dev, NULL, dwc3_exynos_remove_child); > - platform_device_unregister(exynos->usb2_phy); > - platform_device_unregister(exynos->usb3_phy); > > clk_disable_unprepare(exynos->clk); > > -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. Now for the DWC3 part, the usb-phy part was added to use register two separate no-op-xceivers of USB2_PHY type and USB3_PHY type, so that platforms with no separate PHY can still be able to use dwc3. > > Cc: Olof Johansson <olof@lixom.net> > Cc: Kukjin Kim <kgene.kim@samsung.com> > Cc: Vivek Gautam <gautam.vivek@samsung.com> > Acked-by: Kyungmin Park <kyungmin.park@samsung.com> > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > --- > drivers/usb/dwc3/dwc3-exynos.c | 68 ----------------------------------------- > 1 file changed, 1 insertion(+), 67 deletions(-) > > Index: b/drivers/usb/dwc3/dwc3-exynos.c > =================================================================== > --- a/drivers/usb/dwc3/dwc3-exynos.c 2014-08-25 14:57:04.991781925 +0200 > +++ b/drivers/usb/dwc3/dwc3-exynos.c 2014-08-27 09:16:38.312617727 +0200 > @@ -23,15 +23,12 @@ > #include <linux/platform_data/dwc3-exynos.h> > #include <linux/dma-mapping.h> > #include <linux/clk.h> > -#include <linux/usb/otg.h> > -#include <linux/usb/usb_phy_generic.h> > +#include <linux/pm_runtime.h> > #include <linux/of.h> > #include <linux/of_platform.h> > #include <linux/regulator/consumer.h> > > struct dwc3_exynos { > - struct platform_device *usb2_phy; > - struct platform_device *usb3_phy; > struct device *dev; > > struct clk *clk; > @@ -39,61 +36,6 @@ struct dwc3_exynos { > struct regulator *vdd10; > }; > > -static int dwc3_exynos_register_phys(struct dwc3_exynos *exynos) > -{ > - struct usb_phy_generic_platform_data pdata; > - struct platform_device *pdev; > - int ret; > - > - memset(&pdata, 0x00, sizeof(pdata)); > - > - pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO); > - if (!pdev) > - return -ENOMEM; > - > - exynos->usb2_phy = pdev; > - pdata.type = USB_PHY_TYPE_USB2; > - pdata.gpio_reset = -1; > - > - ret = platform_device_add_data(exynos->usb2_phy, &pdata, sizeof(pdata)); > - if (ret) > - goto err1; > - > - pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO); > - if (!pdev) { > - ret = -ENOMEM; > - goto err1; > - } > - > - exynos->usb3_phy = pdev; > - pdata.type = USB_PHY_TYPE_USB3; > - > - ret = platform_device_add_data(exynos->usb3_phy, &pdata, sizeof(pdata)); > - if (ret) > - goto err2; > - > - ret = platform_device_add(exynos->usb2_phy); > - if (ret) > - goto err2; > - > - ret = platform_device_add(exynos->usb3_phy); > - if (ret) > - goto err3; > - > - return 0; > - > -err3: > - platform_device_del(exynos->usb2_phy); > - > -err2: > - platform_device_put(exynos->usb3_phy); > - > -err1: > - platform_device_put(exynos->usb2_phy); > - > - return ret; > -} > - > static int dwc3_exynos_remove_child(struct device *dev, void *unused) > { > struct platform_device *pdev = to_platform_device(dev); > @@ -127,12 +69,6 @@ static int dwc3_exynos_probe(struct plat > > platform_set_drvdata(pdev, exynos); > > - ret = dwc3_exynos_register_phys(exynos); > - if (ret) { > - dev_err(dev, "couldn't register PHYs\n"); > - return ret; > - } > - > clk = devm_clk_get(dev, "usbdrd30"); > if (IS_ERR(clk)) { > dev_err(dev, "couldn't get clock\n"); > @@ -194,8 +130,6 @@ static int dwc3_exynos_remove(struct pla > struct dwc3_exynos *exynos = platform_get_drvdata(pdev); > > device_for_each_child(&pdev->dev, NULL, dwc3_exynos_remove_child); > - platform_device_unregister(exynos->usb2_phy); > - platform_device_unregister(exynos->usb3_phy); > > clk_disable_unprepare(exynos->clk); > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Wed, Aug 27, 2014 at 11:42:25PM +0530, Vivek Gautam wrote: > 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. right, when I added PHY support to dwc3 I expected people to remote NOP transceivers from their glue layers after they had proper PHY drivers available. > 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. > > Now for the DWC3 part, the usb-phy part was added to use register two > separate no-op-xceivers of USB2_PHY type and USB3_PHY type, > so that platforms with no separate PHY can still be able to use dwc3. correct.
[ 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 > Now for the DWC3 part, the usb-phy part was added to use register two > separate no-op-xceivers of USB2_PHY type and USB3_PHY type, > so that platforms with no separate PHY can still be able to use dwc3. Is such support actually useful for Exynos platforms (it resides in dwc3-exynos.c)? I think that all DWC3 Exynos users need corresponding USB PHY support, no? Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics > > Cc: Olof Johansson <olof@lixom.net> > > Cc: Kukjin Kim <kgene.kim@samsung.com> > > Cc: Vivek Gautam <gautam.vivek@samsung.com> > > Acked-by: Kyungmin Park <kyungmin.park@samsung.com> > > Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> > > --- > > drivers/usb/dwc3/dwc3-exynos.c | 68 ----------------------------------------- > > 1 file changed, 1 insertion(+), 67 deletions(-) > > > > Index: b/drivers/usb/dwc3/dwc3-exynos.c > > =================================================================== > > --- a/drivers/usb/dwc3/dwc3-exynos.c 2014-08-25 14:57:04.991781925 +0200 > > +++ b/drivers/usb/dwc3/dwc3-exynos.c 2014-08-27 09:16:38.312617727 +0200 > > @@ -23,15 +23,12 @@ > > #include <linux/platform_data/dwc3-exynos.h> > > #include <linux/dma-mapping.h> > > #include <linux/clk.h> > > -#include <linux/usb/otg.h> > > -#include <linux/usb/usb_phy_generic.h> > > +#include <linux/pm_runtime.h> > > #include <linux/of.h> > > #include <linux/of_platform.h> > > #include <linux/regulator/consumer.h> > > > > struct dwc3_exynos { > > - struct platform_device *usb2_phy; > > - struct platform_device *usb3_phy; > > struct device *dev; > > > > struct clk *clk; > > @@ -39,61 +36,6 @@ struct dwc3_exynos { > > struct regulator *vdd10; > > }; > > > > -static int dwc3_exynos_register_phys(struct dwc3_exynos *exynos) > > -{ > > - struct usb_phy_generic_platform_data pdata; > > - struct platform_device *pdev; > > - int ret; > > - > > - memset(&pdata, 0x00, sizeof(pdata)); > > - > > - pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO); > > - if (!pdev) > > - return -ENOMEM; > > - > > - exynos->usb2_phy = pdev; > > - pdata.type = USB_PHY_TYPE_USB2; > > - pdata.gpio_reset = -1; > > - > > - ret = platform_device_add_data(exynos->usb2_phy, &pdata, sizeof(pdata)); > > - if (ret) > > - goto err1; > > - > > - pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO); > > - if (!pdev) { > > - ret = -ENOMEM; > > - goto err1; > > - } > > - > > - exynos->usb3_phy = pdev; > > - pdata.type = USB_PHY_TYPE_USB3; > > - > > - ret = platform_device_add_data(exynos->usb3_phy, &pdata, sizeof(pdata)); > > - if (ret) > > - goto err2; > > - > > - ret = platform_device_add(exynos->usb2_phy); > > - if (ret) > > - goto err2; > > - > > - ret = platform_device_add(exynos->usb3_phy); > > - if (ret) > > - goto err3; > > - > > - return 0; > > - > > -err3: > > - platform_device_del(exynos->usb2_phy); > > - > > -err2: > > - platform_device_put(exynos->usb3_phy); > > - > > -err1: > > - platform_device_put(exynos->usb2_phy); > > - > > - return ret; > > -} > > - > > static int dwc3_exynos_remove_child(struct device *dev, void *unused) > > { > > struct platform_device *pdev = to_platform_device(dev); > > @@ -127,12 +69,6 @@ static int dwc3_exynos_probe(struct plat > > > > platform_set_drvdata(pdev, exynos); > > > > - ret = dwc3_exynos_register_phys(exynos); > > - if (ret) { > > - dev_err(dev, "couldn't register PHYs\n"); > > - return ret; > > - } > > - > > clk = devm_clk_get(dev, "usbdrd30"); > > if (IS_ERR(clk)) { > > dev_err(dev, "couldn't get clock\n"); > > @@ -194,8 +130,6 @@ static int dwc3_exynos_remove(struct pla > > struct dwc3_exynos *exynos = platform_get_drvdata(pdev); > > > > device_for_each_child(&pdev->dev, NULL, dwc3_exynos_remove_child); > > - platform_device_unregister(exynos->usb2_phy); > > - platform_device_unregister(exynos->usb3_phy); > > > > clk_disable_unprepare(exynos->clk); > > > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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? 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... thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. > 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. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. > > 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. thanks, greg k-h -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: b/drivers/usb/dwc3/dwc3-exynos.c =================================================================== --- a/drivers/usb/dwc3/dwc3-exynos.c 2014-08-25 14:57:04.991781925 +0200 +++ b/drivers/usb/dwc3/dwc3-exynos.c 2014-08-27 09:16:38.312617727 +0200 @@ -23,15 +23,12 @@ #include <linux/platform_data/dwc3-exynos.h> #include <linux/dma-mapping.h> #include <linux/clk.h> -#include <linux/usb/otg.h> -#include <linux/usb/usb_phy_generic.h> +#include <linux/pm_runtime.h> #include <linux/of.h> #include <linux/of_platform.h> #include <linux/regulator/consumer.h> struct dwc3_exynos { - struct platform_device *usb2_phy; - struct platform_device *usb3_phy; struct device *dev; struct clk *clk; @@ -39,61 +36,6 @@ struct dwc3_exynos { struct regulator *vdd10; }; -static int dwc3_exynos_register_phys(struct dwc3_exynos *exynos) -{ - struct usb_phy_generic_platform_data pdata; - struct platform_device *pdev; - int ret; - - memset(&pdata, 0x00, sizeof(pdata)); - - pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO); - if (!pdev) - return -ENOMEM; - - exynos->usb2_phy = pdev; - pdata.type = USB_PHY_TYPE_USB2; - pdata.gpio_reset = -1; - - ret = platform_device_add_data(exynos->usb2_phy, &pdata, sizeof(pdata)); - if (ret) - goto err1; - - pdev = platform_device_alloc("usb_phy_generic", PLATFORM_DEVID_AUTO); - if (!pdev) { - ret = -ENOMEM; - goto err1; - } - - exynos->usb3_phy = pdev; - pdata.type = USB_PHY_TYPE_USB3; - - ret = platform_device_add_data(exynos->usb3_phy, &pdata, sizeof(pdata)); - if (ret) - goto err2; - - ret = platform_device_add(exynos->usb2_phy); - if (ret) - goto err2; - - ret = platform_device_add(exynos->usb3_phy); - if (ret) - goto err3; - - return 0; - -err3: - platform_device_del(exynos->usb2_phy); - -err2: - platform_device_put(exynos->usb3_phy); - -err1: - platform_device_put(exynos->usb2_phy); - - return ret; -} - static int dwc3_exynos_remove_child(struct device *dev, void *unused) { struct platform_device *pdev = to_platform_device(dev); @@ -127,12 +69,6 @@ static int dwc3_exynos_probe(struct plat platform_set_drvdata(pdev, exynos); - ret = dwc3_exynos_register_phys(exynos); - if (ret) { - dev_err(dev, "couldn't register PHYs\n"); - return ret; - } - clk = devm_clk_get(dev, "usbdrd30"); if (IS_ERR(clk)) { dev_err(dev, "couldn't get clock\n"); @@ -194,8 +130,6 @@ static int dwc3_exynos_remove(struct pla struct dwc3_exynos *exynos = platform_get_drvdata(pdev); device_for_each_child(&pdev->dev, NULL, dwc3_exynos_remove_child); - platform_device_unregister(exynos->usb2_phy); - platform_device_unregister(exynos->usb3_phy); clk_disable_unprepare(exynos->clk);
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). ] Cc: Olof Johansson <olof@lixom.net> Cc: Kukjin Kim <kgene.kim@samsung.com> Cc: Vivek Gautam <gautam.vivek@samsung.com> Acked-by: Kyungmin Park <kyungmin.park@samsung.com> Signed-off-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com> --- drivers/usb/dwc3/dwc3-exynos.c | 68 ----------------------------------------- 1 file changed, 1 insertion(+), 67 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html