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

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

[Bartlomiej Zolnierkiewicz]

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(-)

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);
 

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

[Jingoo Han]

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);
> 
> 

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

[Vivek Gautam]

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



-- 
Best Regards
Vivek Gautam
Samsung R&D Institute, Bangalore
India

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

[Felipe Balbi]

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

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.

-- 
balbi

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

[Bartlomiej Zolnierkiewicz]

[ 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

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

[Greg Kroah-Hartman]

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

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

[Bartlomiej Zolnierkiewicz]

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

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

[Greg Kroah-Hartman]

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

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

[Bartlomiej Zolnierkiewicz]

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