[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Russ Dill]

From: Russ Dill <Russ.Dill@gmail.com>

'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
an issue where the ULPI PHYs were not held in reset while initializing
the EHCI controller. However, it also changes behavior in
omap-usb-host.c omap_usbhs_init by releasing reset while the
configuration in that function was done.

This change caused a regression on BB-xM where USB would not function
if 'usb start' had been run from u-boot before booting. A change was
made to release reset a little bit earlier which fixed the issue on
BB-xM and did not cause any regressions on 3430 sdp, the board for
which the fix was originally made.

This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
before adding hcd.', (3aa2ae74) caused a regression on OMAP5.

The original fix to hold the EHCI controller in reset during
initialization was correct, however it appears that changing
omap_usbhs_init to not hold the PHYs in reset during it's
configuration was incorrect. This patch first reverts both fixes, and
then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
reset as the original patch had done. It also is sure to incorporate
the _cansleep change that has been made in the meantime.

I've tested this on Beagleboard xM, I'd really like to get an ack from
the 3430 sdp and OMAP5 guys before getting this merged.

v3 - Brown paper bag its too early in the morning actually run
     git commit amend fix
v2 - Put cansleep gpiolib call outside of spinlock

Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
---
 drivers/mfd/omap-usb-host.c  |   48 +++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/host/ehci-omap.c |   18 +++++++---------
 2 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 7e96bb2..41088ec 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -25,6 +25,7 @@
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/spinlock.h>
+#include <linux/gpio.h>
 #include <plat/cpu.h>
 #include <plat/usb.h>
 #include <linux/pm_runtime.h>
@@ -500,8 +501,21 @@ static void omap_usbhs_init(struct device *dev)
 	dev_dbg(dev, "starting TI HSUSB Controller\n");
 
 	pm_runtime_get_sync(dev);
-	spin_lock_irqsave(&omap->lock, flags);
 
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[0],
+					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[1],
+					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+
+		/* Hold the PHY in RESET for enough time till DIR is high */
+		udelay(10);
+	}
+
+	spin_lock_irqsave(&omap->lock, flags);
 	omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
 	dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
 
@@ -581,9 +595,39 @@ static void omap_usbhs_init(struct device *dev)
 	}
 
 	spin_unlock_irqrestore(&omap->lock, flags);
+
+	if (pdata->ehci_data->phy_reset) {
+		/* Hold the PHY in RESET for enough time till
+		 * PHY is settled and ready
+		 */
+		udelay(10);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[0], 1);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[1], 1);
+	}
+
 	pm_runtime_put_sync(dev);
 }
 
+static void omap_usbhs_deinit(struct device *dev)
+{
+	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
+	struct usbhs_omap_platform_data	*pdata = &omap->platdata;
+
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[0]);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[1]);
+	}
+}
+
 
 /**
  * usbhs_omap_probe - initialize TI-based HCDs
@@ -767,6 +811,7 @@ static int __devinit usbhs_omap_probe(struct platform_device *pdev)
 	goto end_probe;
 
 err_alloc:
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 
 err_tll:
@@ -818,6 +863,7 @@ static int __devexit usbhs_omap_remove(struct platform_device *pdev)
 {
 	struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
 
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 	iounmap(omap->uhh_base);
 	clk_put(omap->init_60m_fclk);
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index a44294d..d0815b6 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -192,14 +192,13 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Hold PHYs in reset while initializing EHCI controller */
 	if (pdata->phy_reset) {
 		if (gpio_is_valid(pdata->reset_gpio_port[0]))
-			gpio_request_one(pdata->reset_gpio_port[0],
-					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[0], 0);
 
 		if (gpio_is_valid(pdata->reset_gpio_port[1]))
-			gpio_request_one(pdata->reset_gpio_port[1],
-					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 0);
 
 		/* Hold the PHY in RESET for enough time till DIR is high */
 		udelay(10);
@@ -241,6 +240,11 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 	omap_ehci->hcs_params = readl(&omap_ehci->caps->hcs_params);
 
 	ehci_reset(omap_ehci);
+	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+	if (ret) {
+		dev_err(dev, "failed to add hcd with err %d\n", ret);
+		goto err_add_hcd;
+	}
 
 	if (pdata->phy_reset) {
 		/* Hold the PHY in RESET for enough time till
@@ -255,12 +259,6 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 1);
 	}
 
-	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
-	if (ret) {
-		dev_err(dev, "failed to add hcd with err %d\n", ret);
-		goto err_add_hcd;
-	}
-
 	/* root ports should always stay powered */
 	ehci_port_power(omap_ehci, 1);
 
-- 
1.7.10.4

[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Russ Dill]

From: Russ Dill <Russ.Dill@gmail.com>

'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
an issue where the ULPI PHYs were not held in reset while initializing
the EHCI controller. However, it also changes behavior in
omap-usb-host.c omap_usbhs_init by releasing reset while the
configuration in that function was done.

This change caused a regression on BB-xM where USB would not function
if 'usb start' had been run from u-boot before booting. A change was
made to release reset a little bit earlier which fixed the issue on
BB-xM and did not cause any regressions on 3430 sdp, the board for
which the fix was originally made.

This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
before adding hcd.', (3aa2ae74) caused a regression on OMAP5.

The original fix to hold the EHCI controller in reset during
initialization was correct, however it appears that changing
omap_usbhs_init to not hold the PHYs in reset during it's
configuration was incorrect. This patch first reverts both fixes, and
then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
reset as the original patch had done. It also is sure to incorporate
the _cansleep change that has been made in the meantime.

I've tested this on Beagleboard xM, I'd really like to get an ack from
the 3430 sdp and OMAP5 guys before getting this merged.

v3 - Brown paper bag its too early in the morning actually run
     git commit amend fix
v2 - Put cansleep gpiolib call outside of spinlock

Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
---
 drivers/mfd/omap-usb-host.c  |   48 +++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/host/ehci-omap.c |   18 +++++++---------
 2 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 7e96bb2..41088ec 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -25,6 +25,7 @@
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/spinlock.h>
+#include <linux/gpio.h>
 #include <plat/cpu.h>
 #include <plat/usb.h>
 #include <linux/pm_runtime.h>
@@ -500,8 +501,21 @@ static void omap_usbhs_init(struct device *dev)
 	dev_dbg(dev, "starting TI HSUSB Controller\n");
 
 	pm_runtime_get_sync(dev);
-	spin_lock_irqsave(&omap->lock, flags);
 
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[0],
+					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[1],
+					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+
+		/* Hold the PHY in RESET for enough time till DIR is high */
+		udelay(10);
+	}
+
+	spin_lock_irqsave(&omap->lock, flags);
 	omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
 	dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
 
@@ -581,9 +595,39 @@ static void omap_usbhs_init(struct device *dev)
 	}
 
 	spin_unlock_irqrestore(&omap->lock, flags);
+
+	if (pdata->ehci_data->phy_reset) {
+		/* Hold the PHY in RESET for enough time till
+		 * PHY is settled and ready
+		 */
+		udelay(10);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[0], 1);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[1], 1);
+	}
+
 	pm_runtime_put_sync(dev);
 }
 
+static void omap_usbhs_deinit(struct device *dev)
+{
+	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
+	struct usbhs_omap_platform_data	*pdata = &omap->platdata;
+
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[0]);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[1]);
+	}
+}
+
 
 /**
  * usbhs_omap_probe - initialize TI-based HCDs
@@ -767,6 +811,7 @@ static int __devinit usbhs_omap_probe(struct platform_device *pdev)
 	goto end_probe;
 
 err_alloc:
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 
 err_tll:
@@ -818,6 +863,7 @@ static int __devexit usbhs_omap_remove(struct platform_device *pdev)
 {
 	struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
 
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 	iounmap(omap->uhh_base);
 	clk_put(omap->init_60m_fclk);
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index a44294d..d0815b6 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -192,14 +192,13 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Hold PHYs in reset while initializing EHCI controller */
 	if (pdata->phy_reset) {
 		if (gpio_is_valid(pdata->reset_gpio_port[0]))
-			gpio_request_one(pdata->reset_gpio_port[0],
-					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[0], 0);
 
 		if (gpio_is_valid(pdata->reset_gpio_port[1]))
-			gpio_request_one(pdata->reset_gpio_port[1],
-					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 0);
 
 		/* Hold the PHY in RESET for enough time till DIR is high */
 		udelay(10);
@@ -241,6 +240,11 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 	omap_ehci->hcs_params = readl(&omap_ehci->caps->hcs_params);
 
 	ehci_reset(omap_ehci);
+	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+	if (ret) {
+		dev_err(dev, "failed to add hcd with err %d\n", ret);
+		goto err_add_hcd;
+	}
 
 	if (pdata->phy_reset) {
 		/* Hold the PHY in RESET for enough time till
@@ -255,12 +259,6 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 1);
 	}
 
-	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
-	if (ret) {
-		dev_err(dev, "failed to add hcd with err %d\n", ret);
-		goto err_add_hcd;
-	}
-
 	/* root ports should always stay powered */
 	ehci_port_power(omap_ehci, 1);
 
-- 
1.7.10.4

RE: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Sarashetti, Mantesh]

'Acked-by: mantesh@ti.com'
'Tested-by: mantesh@ti.com'

Regards,
Mantesh
-----Original Message-----
From: Russ Dill [mailto:russ.dill@gmail.com] On Behalf Of Dill, Russ
Sent: Thursday, June 14, 2012 11:24 AM
To: linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org
Cc: Balbi, Felipe; Munegowda, Keshava; Partha Basak; Igor Grinberg; Samuel Ortiz; Sarashetti, Mantesh; Setty, Sapna; Russ Dill
Subject: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

From: Russ Dill <Russ.Dill@gmail.com>

'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
an issue where the ULPI PHYs were not held in reset while initializing
the EHCI controller. However, it also changes behavior in
omap-usb-host.c omap_usbhs_init by releasing reset while the
configuration in that function was done.

This change caused a regression on BB-xM where USB would not function
if 'usb start' had been run from u-boot before booting. A change was
made to release reset a little bit earlier which fixed the issue on
BB-xM and did not cause any regressions on 3430 sdp, the board for
which the fix was originally made.

This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
before adding hcd.', (3aa2ae74) caused a regression on OMAP5.

The original fix to hold the EHCI controller in reset during
initialization was correct, however it appears that changing
omap_usbhs_init to not hold the PHYs in reset during it's
configuration was incorrect. This patch first reverts both fixes, and
then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
reset as the original patch had done. It also is sure to incorporate
the _cansleep change that has been made in the meantime.

I've tested this on Beagleboard xM, I'd really like to get an ack from
the 3430 sdp and OMAP5 guys before getting this merged.

v3 - Brown paper bag its too early in the morning actually run
     git commit amend fix
v2 - Put cansleep gpiolib call outside of spinlock

Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
---
 drivers/mfd/omap-usb-host.c  |   48 +++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/host/ehci-omap.c |   18 +++++++---------
 2 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 7e96bb2..41088ec 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -25,6 +25,7 @@
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/spinlock.h>
+#include <linux/gpio.h>
 #include <plat/cpu.h>
 #include <plat/usb.h>
 #include <linux/pm_runtime.h>
@@ -500,8 +501,21 @@ static void omap_usbhs_init(struct device *dev)
 	dev_dbg(dev, "starting TI HSUSB Controller\n");
 
 	pm_runtime_get_sync(dev);
-	spin_lock_irqsave(&omap->lock, flags);
 
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[0],
+					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[1],
+					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+
+		/* Hold the PHY in RESET for enough time till DIR is high */
+		udelay(10);
+	}
+
+	spin_lock_irqsave(&omap->lock, flags);
 	omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
 	dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
 
@@ -581,9 +595,39 @@ static void omap_usbhs_init(struct device *dev)
 	}
 
 	spin_unlock_irqrestore(&omap->lock, flags);
+
+	if (pdata->ehci_data->phy_reset) {
+		/* Hold the PHY in RESET for enough time till
+		 * PHY is settled and ready
+		 */
+		udelay(10);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[0], 1);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[1], 1);
+	}
+
 	pm_runtime_put_sync(dev);
 }
 
+static void omap_usbhs_deinit(struct device *dev)
+{
+	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
+	struct usbhs_omap_platform_data	*pdata = &omap->platdata;
+
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[0]);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[1]);
+	}
+}
+
 
 /**
  * usbhs_omap_probe - initialize TI-based HCDs
@@ -767,6 +811,7 @@ static int __devinit usbhs_omap_probe(struct platform_device *pdev)
 	goto end_probe;
 
 err_alloc:
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 
 err_tll:
@@ -818,6 +863,7 @@ static int __devexit usbhs_omap_remove(struct platform_device *pdev)
 {
 	struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
 
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 	iounmap(omap->uhh_base);
 	clk_put(omap->init_60m_fclk);
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index a44294d..d0815b6 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -192,14 +192,13 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Hold PHYs in reset while initializing EHCI controller */
 	if (pdata->phy_reset) {
 		if (gpio_is_valid(pdata->reset_gpio_port[0]))
-			gpio_request_one(pdata->reset_gpio_port[0],
-					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[0], 0);
 
 		if (gpio_is_valid(pdata->reset_gpio_port[1]))
-			gpio_request_one(pdata->reset_gpio_port[1],
-					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 0);
 
 		/* Hold the PHY in RESET for enough time till DIR is high */
 		udelay(10);
@@ -241,6 +240,11 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 	omap_ehci->hcs_params = readl(&omap_ehci->caps->hcs_params);
 
 	ehci_reset(omap_ehci);
+	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+	if (ret) {
+		dev_err(dev, "failed to add hcd with err %d\n", ret);
+		goto err_add_hcd;
+	}
 
 	if (pdata->phy_reset) {
 		/* Hold the PHY in RESET for enough time till
@@ -255,12 +259,6 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 1);
 	}
 
-	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
-	if (ret) {
-		dev_err(dev, "failed to add hcd with err %d\n", ret);
-		goto err_add_hcd;
-	}
-
 	/* root ports should always stay powered */
 	ehci_port_power(omap_ehci, 1);
 
-- 
1.7.10.4

[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Sarashetti, Mantesh]

'Acked-by: mantesh at ti.com'
'Tested-by: mantesh at ti.com'

Regards,
Mantesh
-----Original Message-----
From: Russ Dill [mailto:russ.dill at gmail.com] On Behalf Of Dill, Russ
Sent: Thursday, June 14, 2012 11:24 AM
To: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
Cc: Balbi, Felipe; Munegowda, Keshava; Partha Basak; Igor Grinberg; Samuel Ortiz; Sarashetti, Mantesh; Setty, Sapna; Russ Dill
Subject: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

From: Russ Dill <Russ.Dill@gmail.com>

'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
an issue where the ULPI PHYs were not held in reset while initializing
the EHCI controller. However, it also changes behavior in
omap-usb-host.c omap_usbhs_init by releasing reset while the
configuration in that function was done.

This change caused a regression on BB-xM where USB would not function
if 'usb start' had been run from u-boot before booting. A change was
made to release reset a little bit earlier which fixed the issue on
BB-xM and did not cause any regressions on 3430 sdp, the board for
which the fix was originally made.

This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
before adding hcd.', (3aa2ae74) caused a regression on OMAP5.

The original fix to hold the EHCI controller in reset during
initialization was correct, however it appears that changing
omap_usbhs_init to not hold the PHYs in reset during it's
configuration was incorrect. This patch first reverts both fixes, and
then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
reset as the original patch had done. It also is sure to incorporate
the _cansleep change that has been made in the meantime.

I've tested this on Beagleboard xM, I'd really like to get an ack from
the 3430 sdp and OMAP5 guys before getting this merged.

v3 - Brown paper bag its too early in the morning actually run
     git commit amend fix
v2 - Put cansleep gpiolib call outside of spinlock

Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
---
 drivers/mfd/omap-usb-host.c  |   48 +++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/host/ehci-omap.c |   18 +++++++---------
 2 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 7e96bb2..41088ec 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -25,6 +25,7 @@
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/spinlock.h>
+#include <linux/gpio.h>
 #include <plat/cpu.h>
 #include <plat/usb.h>
 #include <linux/pm_runtime.h>
@@ -500,8 +501,21 @@ static void omap_usbhs_init(struct device *dev)
 	dev_dbg(dev, "starting TI HSUSB Controller\n");
 
 	pm_runtime_get_sync(dev);
-	spin_lock_irqsave(&omap->lock, flags);
 
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[0],
+					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[1],
+					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+
+		/* Hold the PHY in RESET for enough time till DIR is high */
+		udelay(10);
+	}
+
+	spin_lock_irqsave(&omap->lock, flags);
 	omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
 	dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
 
@@ -581,9 +595,39 @@ static void omap_usbhs_init(struct device *dev)
 	}
 
 	spin_unlock_irqrestore(&omap->lock, flags);
+
+	if (pdata->ehci_data->phy_reset) {
+		/* Hold the PHY in RESET for enough time till
+		 * PHY is settled and ready
+		 */
+		udelay(10);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[0], 1);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[1], 1);
+	}
+
 	pm_runtime_put_sync(dev);
 }
 
+static void omap_usbhs_deinit(struct device *dev)
+{
+	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
+	struct usbhs_omap_platform_data	*pdata = &omap->platdata;
+
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[0]);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[1]);
+	}
+}
+
 
 /**
  * usbhs_omap_probe - initialize TI-based HCDs
@@ -767,6 +811,7 @@ static int __devinit usbhs_omap_probe(struct platform_device *pdev)
 	goto end_probe;
 
 err_alloc:
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 
 err_tll:
@@ -818,6 +863,7 @@ static int __devexit usbhs_omap_remove(struct platform_device *pdev)
 {
 	struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
 
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 	iounmap(omap->uhh_base);
 	clk_put(omap->init_60m_fclk);
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index a44294d..d0815b6 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -192,14 +192,13 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Hold PHYs in reset while initializing EHCI controller */
 	if (pdata->phy_reset) {
 		if (gpio_is_valid(pdata->reset_gpio_port[0]))
-			gpio_request_one(pdata->reset_gpio_port[0],
-					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[0], 0);
 
 		if (gpio_is_valid(pdata->reset_gpio_port[1]))
-			gpio_request_one(pdata->reset_gpio_port[1],
-					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 0);
 
 		/* Hold the PHY in RESET for enough time till DIR is high */
 		udelay(10);
@@ -241,6 +240,11 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 	omap_ehci->hcs_params = readl(&omap_ehci->caps->hcs_params);
 
 	ehci_reset(omap_ehci);
+	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+	if (ret) {
+		dev_err(dev, "failed to add hcd with err %d\n", ret);
+		goto err_add_hcd;
+	}
 
 	if (pdata->phy_reset) {
 		/* Hold the PHY in RESET for enough time till
@@ -255,12 +259,6 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 1);
 	}
 
-	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
-	if (ret) {
-		dev_err(dev, "failed to add hcd with err %d\n", ret);
-		goto err_add_hcd;
-	}
-
 	/* root ports should always stay powered */
 	ehci_port_power(omap_ehci, 1);
 
-- 
1.7.10.4

Re: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Munegowda, Keshava]

On Fri, Jun 15, 2012 at 12:36 AM, Sarashetti, Mantesh <mantesh-l0cyMroinI0@public.gmane.org> wrote:
> 'Acked-by: mantesh-l0cyMroinI0@public.gmane.org'
> 'Tested-by: mantesh-l0cyMroinI0@public.gmane.org'
>
> Regards,
> Mantesh
> -----Original Message-----
> From: Russ Dill [mailto:russ.dill-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org] On Behalf Of Dill, Russ
> Sent: Thursday, June 14, 2012 11:24 AM
> To: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: Balbi, Felipe; Munegowda, Keshava; Partha Basak; Igor Grinberg; Samuel Ortiz; Sarashetti, Mantesh; Setty, Sapna; Russ Dill
> Subject: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.
>
> From: Russ Dill <Russ.Dill-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>
> 'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
> an issue where the ULPI PHYs were not held in reset while initializing
> the EHCI controller. However, it also changes behavior in
> omap-usb-host.c omap_usbhs_init by releasing reset while the
> configuration in that function was done.
>
> This change caused a regression on BB-xM where USB would not function
> if 'usb start' had been run from u-boot before booting. A change was
> made to release reset a little bit earlier which fixed the issue on
> BB-xM and did not cause any regressions on 3430 sdp, the board for
> which the fix was originally made.
>
> This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
> before adding hcd.', (3aa2ae74) caused a regression on OMAP5.
>
> The original fix to hold the EHCI controller in reset during
> initialization was correct, however it appears that changing
> omap_usbhs_init to not hold the PHYs in reset during it's
> configuration was incorrect. This patch first reverts both fixes, and
> then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
> reset as the original patch had done. It also is sure to incorporate
> the _cansleep change that has been made in the meantime.
>
> I've tested this on Beagleboard xM, I'd really like to get an ack from
> the 3430 sdp and OMAP5 guys before getting this merged.
>
> v3 - Brown paper bag its too early in the morning actually run
>     git commit amend fix
> v2 - Put cansleep gpiolib call outside of spinlock
>
> Signed-off-by: Russ Dill <Russ.Dill-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/mfd/omap-usb-host.c  |   48 +++++++++++++++++++++++++++++++++++++++++-
>  drivers/usb/host/ehci-omap.c |   18 +++++++---------
>  2 files changed, 55 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index 7e96bb2..41088ec 100644
> --- a/drivers/mfd/omap-usb-host.c
> +++ b/drivers/mfd/omap-usb-host.c
> @@ -25,6 +25,7 @@
>  #include <linux/clk.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/spinlock.h>
> +#include <linux/gpio.h>
>  #include <plat/cpu.h>
>  #include <plat/usb.h>
>  #include <linux/pm_runtime.h>
> @@ -500,8 +501,21 @@ static void omap_usbhs_init(struct device *dev)
>        dev_dbg(dev, "starting TI HSUSB Controller\n");
>
>        pm_runtime_get_sync(dev);
> -       spin_lock_irqsave(&omap->lock, flags);
>
> +       if (pdata->ehci_data->phy_reset) {
> +               if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
> +                       gpio_request_one(pdata->ehci_data->reset_gpio_port[0],
> +                                        GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
> +
> +               if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
> +                       gpio_request_one(pdata->ehci_data->reset_gpio_port[1],
> +                                        GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
> +
> +               /* Hold the PHY in RESET for enough time till DIR is high */
> +               udelay(10);
> +       }
> +
> +       spin_lock_irqsave(&omap->lock, flags);
>        omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
>        dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
>
> @@ -581,9 +595,39 @@ static void omap_usbhs_init(struct device *dev)
>        }
>
>        spin_unlock_irqrestore(&omap->lock, flags);
> +
> +       if (pdata->ehci_data->phy_reset) {
> +               /* Hold the PHY in RESET for enough time till
> +                * PHY is settled and ready
> +                */
> +               udelay(10);
> +
> +               if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
> +                       gpio_set_value_cansleep
> +                               (pdata->ehci_data->reset_gpio_port[0], 1);
> +
> +               if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
> +                       gpio_set_value_cansleep
> +                               (pdata->ehci_data->reset_gpio_port[1], 1);
> +       }
> +
>        pm_runtime_put_sync(dev);
>  }
>
> +static void omap_usbhs_deinit(struct device *dev)
> +{
> +       struct usbhs_hcd_omap           *omap = dev_get_drvdata(dev);
> +       struct usbhs_omap_platform_data *pdata = &omap->platdata;
> +
> +       if (pdata->ehci_data->phy_reset) {
> +               if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
> +                       gpio_free(pdata->ehci_data->reset_gpio_port[0]);
> +
> +               if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
> +                       gpio_free(pdata->ehci_data->reset_gpio_port[1]);
> +       }
> +}
> +
>
>  /**
>  * usbhs_omap_probe - initialize TI-based HCDs
> @@ -767,6 +811,7 @@ static int __devinit usbhs_omap_probe(struct platform_device *pdev)
>        goto end_probe;
>
>  err_alloc:
> +       omap_usbhs_deinit(&pdev->dev);
>        iounmap(omap->tll_base);
>
>  err_tll:
> @@ -818,6 +863,7 @@ static int __devexit usbhs_omap_remove(struct platform_device *pdev)
>  {
>        struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
>
> +       omap_usbhs_deinit(&pdev->dev);
>        iounmap(omap->tll_base);
>        iounmap(omap->uhh_base);
>        clk_put(omap->init_60m_fclk);
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index a44294d..d0815b6 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -192,14 +192,13 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
>                }
>        }
>
> +       /* Hold PHYs in reset while initializing EHCI controller */
>        if (pdata->phy_reset) {
>                if (gpio_is_valid(pdata->reset_gpio_port[0]))
> -                       gpio_request_one(pdata->reset_gpio_port[0],
> -                                        GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
> +                       gpio_set_value_cansleep(pdata->reset_gpio_port[0], 0);
>
>                if (gpio_is_valid(pdata->reset_gpio_port[1]))
> -                       gpio_request_one(pdata->reset_gpio_port[1],
> -                                        GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
> +                       gpio_set_value_cansleep(pdata->reset_gpio_port[1], 0);
>
>                /* Hold the PHY in RESET for enough time till DIR is high */
>                udelay(10);
> @@ -241,6 +240,11 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
>        omap_ehci->hcs_params = readl(&omap_ehci->caps->hcs_params);
>
>        ehci_reset(omap_ehci);
> +       ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
> +       if (ret) {
> +               dev_err(dev, "failed to add hcd with err %d\n", ret);
> +               goto err_add_hcd;
> +       }
>
>        if (pdata->phy_reset) {
>                /* Hold the PHY in RESET for enough time till
> @@ -255,12 +259,6 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
>                        gpio_set_value_cansleep(pdata->reset_gpio_port[1], 1);
>        }
>
> -       ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
> -       if (ret) {
> -               dev_err(dev, "failed to add hcd with err %d\n", ret);
> -               goto err_add_hcd;
> -       }
> -
>        /* root ports should always stay powered */
>        ehci_port_power(omap_ehci, 1);
>
> --
> 1.7.10.4
>

Hi Samuel

I have validated this patch v3 in omap3 beagle XM .

Acked-by: Keshava Munegowda <keshava_mgowda-l0cyMroinI0@public.gmane.org>
Tested-by: Keshava Munegowda <keshava_mgowda-l0cyMroinI0@public.gmane.org>

regards
keshava
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Munegowda, Keshava]

On Fri, Jun 15, 2012 at 12:36 AM, Sarashetti, Mantesh <mantesh@ti.com> wrote:
> 'Acked-by: mantesh at ti.com'
> 'Tested-by: mantesh at ti.com'
>
> Regards,
> Mantesh
> -----Original Message-----
> From: Russ Dill [mailto:russ.dill at gmail.com] On Behalf Of Dill, Russ
> Sent: Thursday, June 14, 2012 11:24 AM
> To: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
> Cc: Balbi, Felipe; Munegowda, Keshava; Partha Basak; Igor Grinberg; Samuel Ortiz; Sarashetti, Mantesh; Setty, Sapna; Russ Dill
> Subject: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.
>
> From: Russ Dill <Russ.Dill@gmail.com>
>
> 'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
> an issue where the ULPI PHYs were not held in reset while initializing
> the EHCI controller. However, it also changes behavior in
> omap-usb-host.c omap_usbhs_init by releasing reset while the
> configuration in that function was done.
>
> This change caused a regression on BB-xM where USB would not function
> if 'usb start' had been run from u-boot before booting. A change was
> made to release reset a little bit earlier which fixed the issue on
> BB-xM and did not cause any regressions on 3430 sdp, the board for
> which the fix was originally made.
>
> This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
> before adding hcd.', (3aa2ae74) caused a regression on OMAP5.
>
> The original fix to hold the EHCI controller in reset during
> initialization was correct, however it appears that changing
> omap_usbhs_init to not hold the PHYs in reset during it's
> configuration was incorrect. This patch first reverts both fixes, and
> then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
> reset as the original patch had done. It also is sure to incorporate
> the _cansleep change that has been made in the meantime.
>
> I've tested this on Beagleboard xM, I'd really like to get an ack from
> the 3430 sdp and OMAP5 guys before getting this merged.
>
> v3 - Brown paper bag its too early in the morning actually run
> ? ? git commit amend fix
> v2 - Put cansleep gpiolib call outside of spinlock
>
> Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
> ---
> ?drivers/mfd/omap-usb-host.c ?| ? 48 +++++++++++++++++++++++++++++++++++++++++-
> ?drivers/usb/host/ehci-omap.c | ? 18 +++++++---------
> ?2 files changed, 55 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index 7e96bb2..41088ec 100644
> --- a/drivers/mfd/omap-usb-host.c
> +++ b/drivers/mfd/omap-usb-host.c
> @@ -25,6 +25,7 @@
> ?#include <linux/clk.h>
> ?#include <linux/dma-mapping.h>
> ?#include <linux/spinlock.h>
> +#include <linux/gpio.h>
> ?#include <plat/cpu.h>
> ?#include <plat/usb.h>
> ?#include <linux/pm_runtime.h>
> @@ -500,8 +501,21 @@ static void omap_usbhs_init(struct device *dev)
> ? ? ? ?dev_dbg(dev, "starting TI HSUSB Controller\n");
>
> ? ? ? ?pm_runtime_get_sync(dev);
> - ? ? ? spin_lock_irqsave(&omap->lock, flags);
>
> + ? ? ? if (pdata->ehci_data->phy_reset) {
> + ? ? ? ? ? ? ? if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
> + ? ? ? ? ? ? ? ? ? ? ? gpio_request_one(pdata->ehci_data->reset_gpio_port[0],
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
> +
> + ? ? ? ? ? ? ? if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
> + ? ? ? ? ? ? ? ? ? ? ? gpio_request_one(pdata->ehci_data->reset_gpio_port[1],
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
> +
> + ? ? ? ? ? ? ? /* Hold the PHY in RESET for enough time till DIR is high */
> + ? ? ? ? ? ? ? udelay(10);
> + ? ? ? }
> +
> + ? ? ? spin_lock_irqsave(&omap->lock, flags);
> ? ? ? ?omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
> ? ? ? ?dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
>
> @@ -581,9 +595,39 @@ static void omap_usbhs_init(struct device *dev)
> ? ? ? ?}
>
> ? ? ? ?spin_unlock_irqrestore(&omap->lock, flags);
> +
> + ? ? ? if (pdata->ehci_data->phy_reset) {
> + ? ? ? ? ? ? ? /* Hold the PHY in RESET for enough time till
> + ? ? ? ? ? ? ? ?* PHY is settled and ready
> + ? ? ? ? ? ? ? ?*/
> + ? ? ? ? ? ? ? udelay(10);
> +
> + ? ? ? ? ? ? ? if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
> + ? ? ? ? ? ? ? ? ? ? ? gpio_set_value_cansleep
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (pdata->ehci_data->reset_gpio_port[0], 1);
> +
> + ? ? ? ? ? ? ? if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
> + ? ? ? ? ? ? ? ? ? ? ? gpio_set_value_cansleep
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (pdata->ehci_data->reset_gpio_port[1], 1);
> + ? ? ? }
> +
> ? ? ? ?pm_runtime_put_sync(dev);
> ?}
>
> +static void omap_usbhs_deinit(struct device *dev)
> +{
> + ? ? ? struct usbhs_hcd_omap ? ? ? ? ? *omap = dev_get_drvdata(dev);
> + ? ? ? struct usbhs_omap_platform_data *pdata = &omap->platdata;
> +
> + ? ? ? if (pdata->ehci_data->phy_reset) {
> + ? ? ? ? ? ? ? if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
> + ? ? ? ? ? ? ? ? ? ? ? gpio_free(pdata->ehci_data->reset_gpio_port[0]);
> +
> + ? ? ? ? ? ? ? if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
> + ? ? ? ? ? ? ? ? ? ? ? gpio_free(pdata->ehci_data->reset_gpio_port[1]);
> + ? ? ? }
> +}
> +
>
> ?/**
> ?* usbhs_omap_probe - initialize TI-based HCDs
> @@ -767,6 +811,7 @@ static int __devinit usbhs_omap_probe(struct platform_device *pdev)
> ? ? ? ?goto end_probe;
>
> ?err_alloc:
> + ? ? ? omap_usbhs_deinit(&pdev->dev);
> ? ? ? ?iounmap(omap->tll_base);
>
> ?err_tll:
> @@ -818,6 +863,7 @@ static int __devexit usbhs_omap_remove(struct platform_device *pdev)
> ?{
> ? ? ? ?struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
>
> + ? ? ? omap_usbhs_deinit(&pdev->dev);
> ? ? ? ?iounmap(omap->tll_base);
> ? ? ? ?iounmap(omap->uhh_base);
> ? ? ? ?clk_put(omap->init_60m_fclk);
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index a44294d..d0815b6 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -192,14 +192,13 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
> ? ? ? ? ? ? ? ?}
> ? ? ? ?}
>
> + ? ? ? /* Hold PHYs in reset while initializing EHCI controller */
> ? ? ? ?if (pdata->phy_reset) {
> ? ? ? ? ? ? ? ?if (gpio_is_valid(pdata->reset_gpio_port[0]))
> - ? ? ? ? ? ? ? ? ? ? ? gpio_request_one(pdata->reset_gpio_port[0],
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
> + ? ? ? ? ? ? ? ? ? ? ? gpio_set_value_cansleep(pdata->reset_gpio_port[0], 0);
>
> ? ? ? ? ? ? ? ?if (gpio_is_valid(pdata->reset_gpio_port[1]))
> - ? ? ? ? ? ? ? ? ? ? ? gpio_request_one(pdata->reset_gpio_port[1],
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
> + ? ? ? ? ? ? ? ? ? ? ? gpio_set_value_cansleep(pdata->reset_gpio_port[1], 0);
>
> ? ? ? ? ? ? ? ?/* Hold the PHY in RESET for enough time till DIR is high */
> ? ? ? ? ? ? ? ?udelay(10);
> @@ -241,6 +240,11 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
> ? ? ? ?omap_ehci->hcs_params = readl(&omap_ehci->caps->hcs_params);
>
> ? ? ? ?ehci_reset(omap_ehci);
> + ? ? ? ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
> + ? ? ? if (ret) {
> + ? ? ? ? ? ? ? dev_err(dev, "failed to add hcd with err %d\n", ret);
> + ? ? ? ? ? ? ? goto err_add_hcd;
> + ? ? ? }
>
> ? ? ? ?if (pdata->phy_reset) {
> ? ? ? ? ? ? ? ?/* Hold the PHY in RESET for enough time till
> @@ -255,12 +259,6 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
> ? ? ? ? ? ? ? ? ? ? ? ?gpio_set_value_cansleep(pdata->reset_gpio_port[1], 1);
> ? ? ? ?}
>
> - ? ? ? ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
> - ? ? ? if (ret) {
> - ? ? ? ? ? ? ? dev_err(dev, "failed to add hcd with err %d\n", ret);
> - ? ? ? ? ? ? ? goto err_add_hcd;
> - ? ? ? }
> -
> ? ? ? ?/* root ports should always stay powered */
> ? ? ? ?ehci_port_power(omap_ehci, 1);
>
> --
> 1.7.10.4
>

Hi Samuel

I have validated this patch v3 in omap3 beagle XM .

Acked-by: Keshava Munegowda <keshava_mgowda@ti.com>
Tested-by: Keshava Munegowda <keshava_mgowda@ti.com>

regards
keshava

RE: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Sunil Matange]

Thanks Keshava.

Regards
Sunil Matange


-----Original Message-----
From: Munegowda, Keshava [mailto:keshava_mgowda@ti.com]
Sent: Monday, June 18, 2012 1:59 PM
To: Samuel Ortiz
Cc: Dill, Russ; linux-omap@vger.kernel.org;
linux-arm-kernel@lists.infradead.org; Balbi, Felipe; Partha Basak; Igor
Grinberg; Setty, Sapna; Russ Dill; Mantesh Sarashetti; Sunil Matange;
Nishant Kamat; Linux USB Devel
Subject: Re: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix
issues.

On Fri, Jun 15, 2012 at 12:36 AM, Sarashetti, Mantesh <mantesh@ti.com>
wrote:
> 'Acked-by: mantesh@ti.com'
> 'Tested-by: mantesh@ti.com'
>
> Regards,
> Mantesh
> -----Original Message-----
> From: Russ Dill [mailto:russ.dill@gmail.com] On Behalf Of Dill, Russ
> Sent: Thursday, June 14, 2012 11:24 AM
> To: linux-omap@vger.kernel.org; linux-arm-kernel@lists.infradead.org
> Cc: Balbi, Felipe; Munegowda, Keshava; Partha Basak; Igor Grinberg;
Samuel Ortiz; Sarashetti, Mantesh; Setty, Sapna; Russ Dill
> Subject: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix
issues.
>
> From: Russ Dill <Russ.Dill@gmail.com>
>
> 'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
> an issue where the ULPI PHYs were not held in reset while initializing
> the EHCI controller. However, it also changes behavior in
> omap-usb-host.c omap_usbhs_init by releasing reset while the
> configuration in that function was done.
>
> This change caused a regression on BB-xM where USB would not function
> if 'usb start' had been run from u-boot before booting. A change was
> made to release reset a little bit earlier which fixed the issue on
> BB-xM and did not cause any regressions on 3430 sdp, the board for
> which the fix was originally made.
>
> This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
> before adding hcd.', (3aa2ae74) caused a regression on OMAP5.
>
> The original fix to hold the EHCI controller in reset during
> initialization was correct, however it appears that changing
> omap_usbhs_init to not hold the PHYs in reset during it's
> configuration was incorrect. This patch first reverts both fixes, and
> then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
> reset as the original patch had done. It also is sure to incorporate
> the _cansleep change that has been made in the meantime.
>
> I've tested this on Beagleboard xM, I'd really like to get an ack from
> the 3430 sdp and OMAP5 guys before getting this merged.
>
> v3 - Brown paper bag its too early in the morning actually run
>     git commit amend fix
> v2 - Put cansleep gpiolib call outside of spinlock
>
> Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
> ---
>  drivers/mfd/omap-usb-host.c  |   48
+++++++++++++++++++++++++++++++++++++++++-
>  drivers/usb/host/ehci-omap.c |   18 +++++++---------
>  2 files changed, 55 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index 7e96bb2..41088ec 100644
> --- a/drivers/mfd/omap-usb-host.c
> +++ b/drivers/mfd/omap-usb-host.c
> @@ -25,6 +25,7 @@
>  #include <linux/clk.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/spinlock.h>
> +#include <linux/gpio.h>
>  #include <plat/cpu.h>
>  #include <plat/usb.h>
>  #include <linux/pm_runtime.h>
> @@ -500,8 +501,21 @@ static void omap_usbhs_init(struct device *dev)
>        dev_dbg(dev, "starting TI HSUSB Controller\n");
>
>        pm_runtime_get_sync(dev);
> -       spin_lock_irqsave(&omap->lock, flags);
>
> +       if (pdata->ehci_data->phy_reset) {
> +               if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
> +
gpio_request_one(pdata->ehci_data->reset_gpio_port[0],
> +                                        GPIOF_OUT_INIT_LOW, "USB1 PHY
reset");
> +
> +               if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
> +
gpio_request_one(pdata->ehci_data->reset_gpio_port[1],
> +                                        GPIOF_OUT_INIT_LOW, "USB2 PHY
reset");
> +
> +               /* Hold the PHY in RESET for enough time till DIR is
high */
> +               udelay(10);
> +       }
> +
> +       spin_lock_irqsave(&omap->lock, flags);
>        omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
>        dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
>
> @@ -581,9 +595,39 @@ static void omap_usbhs_init(struct device *dev)
>        }
>
>        spin_unlock_irqrestore(&omap->lock, flags);
> +
> +       if (pdata->ehci_data->phy_reset) {
> +               /* Hold the PHY in RESET for enough time till
> +                * PHY is settled and ready
> +                */
> +               udelay(10);
> +
> +               if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
> +                       gpio_set_value_cansleep
> +                               (pdata->ehci_data->reset_gpio_port[0],
1);
> +
> +               if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
> +                       gpio_set_value_cansleep
> +                               (pdata->ehci_data->reset_gpio_port[1],
1);
> +       }
> +
>        pm_runtime_put_sync(dev);
>  }
>
> +static void omap_usbhs_deinit(struct device *dev)
> +{
> +       struct usbhs_hcd_omap           *omap = dev_get_drvdata(dev);
> +       struct usbhs_omap_platform_data *pdata = &omap->platdata;
> +
> +       if (pdata->ehci_data->phy_reset) {
> +               if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
> +                       gpio_free(pdata->ehci_data->reset_gpio_port[0]);
> +
> +               if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
> +                       gpio_free(pdata->ehci_data->reset_gpio_port[1]);
> +       }
> +}
> +
>
>  /**
>  * usbhs_omap_probe - initialize TI-based HCDs
> @@ -767,6 +811,7 @@ static int __devinit usbhs_omap_probe(struct
platform_device *pdev)
>        goto end_probe;
>
>  err_alloc:
> +       omap_usbhs_deinit(&pdev->dev);
>        iounmap(omap->tll_base);
>
>  err_tll:
> @@ -818,6 +863,7 @@ static int __devexit usbhs_omap_remove(struct
platform_device *pdev)
>  {
>        struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
>
> +       omap_usbhs_deinit(&pdev->dev);
>        iounmap(omap->tll_base);
>        iounmap(omap->uhh_base);
>        clk_put(omap->init_60m_fclk);
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index a44294d..d0815b6 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -192,14 +192,13 @@ static int ehci_hcd_omap_probe(struct
platform_device *pdev)
>                }
>        }
>
> +       /* Hold PHYs in reset while initializing EHCI controller */
>        if (pdata->phy_reset) {
>                if (gpio_is_valid(pdata->reset_gpio_port[0]))
> -                       gpio_request_one(pdata->reset_gpio_port[0],
> -                                        GPIOF_OUT_INIT_LOW, "USB1 PHY
reset");
> +
gpio_set_value_cansleep(pdata->reset_gpio_port[0], 0);
>
>                if (gpio_is_valid(pdata->reset_gpio_port[1]))
> -                       gpio_request_one(pdata->reset_gpio_port[1],
> -                                        GPIOF_OUT_INIT_LOW, "USB2 PHY
reset");
> +
gpio_set_value_cansleep(pdata->reset_gpio_port[1], 0);
>
>                /* Hold the PHY in RESET for enough time till DIR is high
*/
>                udelay(10);
> @@ -241,6 +240,11 @@ static int ehci_hcd_omap_probe(struct
platform_device *pdev)
>        omap_ehci->hcs_params = readl(&omap_ehci->caps->hcs_params);
>
>        ehci_reset(omap_ehci);
> +       ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
> +       if (ret) {
> +               dev_err(dev, "failed to add hcd with err %d\n", ret);
> +               goto err_add_hcd;
> +       }
>
>        if (pdata->phy_reset) {
>                /* Hold the PHY in RESET for enough time till
> @@ -255,12 +259,6 @@ static int ehci_hcd_omap_probe(struct
platform_device *pdev)
>
 gpio_set_value_cansleep(pdata->reset_gpio_port[1], 1);
>        }
>
> -       ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
> -       if (ret) {
> -               dev_err(dev, "failed to add hcd with err %d\n", ret);
> -               goto err_add_hcd;
> -       }
> -
>        /* root ports should always stay powered */
>        ehci_port_power(omap_ehci, 1);
>
> --
> 1.7.10.4
>

Hi Samuel

I have validated this patch v3 in omap3 beagle XM .

Acked-by: Keshava Munegowda <keshava_mgowda@ti.com>
Tested-by: Keshava Munegowda <keshava_mgowda@ti.com>

regards
keshava
--
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

[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Sunil Matange]

Thanks Keshava.

Regards
Sunil Matange


-----Original Message-----
From: Munegowda, Keshava [mailto:keshava_mgowda at ti.com]
Sent: Monday, June 18, 2012 1:59 PM
To: Samuel Ortiz
Cc: Dill, Russ; linux-omap at vger.kernel.org;
linux-arm-kernel at lists.infradead.org; Balbi, Felipe; Partha Basak; Igor
Grinberg; Setty, Sapna; Russ Dill; Mantesh Sarashetti; Sunil Matange;
Nishant Kamat; Linux USB Devel
Subject: Re: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix
issues.

On Fri, Jun 15, 2012 at 12:36 AM, Sarashetti, Mantesh <mantesh@ti.com>
wrote:
> 'Acked-by: mantesh at ti.com'
> 'Tested-by: mantesh at ti.com'
>
> Regards,
> Mantesh
> -----Original Message-----
> From: Russ Dill [mailto:russ.dill at gmail.com] On Behalf Of Dill, Russ
> Sent: Thursday, June 14, 2012 11:24 AM
> To: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org
> Cc: Balbi, Felipe; Munegowda, Keshava; Partha Basak; Igor Grinberg;
Samuel Ortiz; Sarashetti, Mantesh; Setty, Sapna; Russ Dill
> Subject: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix
issues.
>
> From: Russ Dill <Russ.Dill@gmail.com>
>
> 'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
> an issue where the ULPI PHYs were not held in reset while initializing
> the EHCI controller. However, it also changes behavior in
> omap-usb-host.c omap_usbhs_init by releasing reset while the
> configuration in that function was done.
>
> This change caused a regression on BB-xM where USB would not function
> if 'usb start' had been run from u-boot before booting. A change was
> made to release reset a little bit earlier which fixed the issue on
> BB-xM and did not cause any regressions on 3430 sdp, the board for
> which the fix was originally made.
>
> This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
> before adding hcd.', (3aa2ae74) caused a regression on OMAP5.
>
> The original fix to hold the EHCI controller in reset during
> initialization was correct, however it appears that changing
> omap_usbhs_init to not hold the PHYs in reset during it's
> configuration was incorrect. This patch first reverts both fixes, and
> then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
> reset as the original patch had done. It also is sure to incorporate
> the _cansleep change that has been made in the meantime.
>
> I've tested this on Beagleboard xM, I'd really like to get an ack from
> the 3430 sdp and OMAP5 guys before getting this merged.
>
> v3 - Brown paper bag its too early in the morning actually run
> ? ? git commit amend fix
> v2 - Put cansleep gpiolib call outside of spinlock
>
> Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
> ---
> ?drivers/mfd/omap-usb-host.c ?| ? 48
+++++++++++++++++++++++++++++++++++++++++-
> ?drivers/usb/host/ehci-omap.c | ? 18 +++++++---------
> ?2 files changed, 55 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index 7e96bb2..41088ec 100644
> --- a/drivers/mfd/omap-usb-host.c
> +++ b/drivers/mfd/omap-usb-host.c
> @@ -25,6 +25,7 @@
> ?#include <linux/clk.h>
> ?#include <linux/dma-mapping.h>
> ?#include <linux/spinlock.h>
> +#include <linux/gpio.h>
> ?#include <plat/cpu.h>
> ?#include <plat/usb.h>
> ?#include <linux/pm_runtime.h>
> @@ -500,8 +501,21 @@ static void omap_usbhs_init(struct device *dev)
> ? ? ? ?dev_dbg(dev, "starting TI HSUSB Controller\n");
>
> ? ? ? ?pm_runtime_get_sync(dev);
> - ? ? ? spin_lock_irqsave(&omap->lock, flags);
>
> + ? ? ? if (pdata->ehci_data->phy_reset) {
> + ? ? ? ? ? ? ? if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
> +
gpio_request_one(pdata->ehci_data->reset_gpio_port[0],
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?GPIOF_OUT_INIT_LOW, "USB1 PHY
reset");
> +
> + ? ? ? ? ? ? ? if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
> +
gpio_request_one(pdata->ehci_data->reset_gpio_port[1],
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?GPIOF_OUT_INIT_LOW, "USB2 PHY
reset");
> +
> + ? ? ? ? ? ? ? /* Hold the PHY in RESET for enough time till DIR is
high */
> + ? ? ? ? ? ? ? udelay(10);
> + ? ? ? }
> +
> + ? ? ? spin_lock_irqsave(&omap->lock, flags);
> ? ? ? ?omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
> ? ? ? ?dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
>
> @@ -581,9 +595,39 @@ static void omap_usbhs_init(struct device *dev)
> ? ? ? ?}
>
> ? ? ? ?spin_unlock_irqrestore(&omap->lock, flags);
> +
> + ? ? ? if (pdata->ehci_data->phy_reset) {
> + ? ? ? ? ? ? ? /* Hold the PHY in RESET for enough time till
> + ? ? ? ? ? ? ? ?* PHY is settled and ready
> + ? ? ? ? ? ? ? ?*/
> + ? ? ? ? ? ? ? udelay(10);
> +
> + ? ? ? ? ? ? ? if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
> + ? ? ? ? ? ? ? ? ? ? ? gpio_set_value_cansleep
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (pdata->ehci_data->reset_gpio_port[0],
1);
> +
> + ? ? ? ? ? ? ? if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
> + ? ? ? ? ? ? ? ? ? ? ? gpio_set_value_cansleep
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? (pdata->ehci_data->reset_gpio_port[1],
1);
> + ? ? ? }
> +
> ? ? ? ?pm_runtime_put_sync(dev);
> ?}
>
> +static void omap_usbhs_deinit(struct device *dev)
> +{
> + ? ? ? struct usbhs_hcd_omap ? ? ? ? ? *omap = dev_get_drvdata(dev);
> + ? ? ? struct usbhs_omap_platform_data *pdata = &omap->platdata;
> +
> + ? ? ? if (pdata->ehci_data->phy_reset) {
> + ? ? ? ? ? ? ? if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
> + ? ? ? ? ? ? ? ? ? ? ? gpio_free(pdata->ehci_data->reset_gpio_port[0]);
> +
> + ? ? ? ? ? ? ? if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
> + ? ? ? ? ? ? ? ? ? ? ? gpio_free(pdata->ehci_data->reset_gpio_port[1]);
> + ? ? ? }
> +}
> +
>
> ?/**
> ?* usbhs_omap_probe - initialize TI-based HCDs
> @@ -767,6 +811,7 @@ static int __devinit usbhs_omap_probe(struct
platform_device *pdev)
> ? ? ? ?goto end_probe;
>
> ?err_alloc:
> + ? ? ? omap_usbhs_deinit(&pdev->dev);
> ? ? ? ?iounmap(omap->tll_base);
>
> ?err_tll:
> @@ -818,6 +863,7 @@ static int __devexit usbhs_omap_remove(struct
platform_device *pdev)
> ?{
> ? ? ? ?struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
>
> + ? ? ? omap_usbhs_deinit(&pdev->dev);
> ? ? ? ?iounmap(omap->tll_base);
> ? ? ? ?iounmap(omap->uhh_base);
> ? ? ? ?clk_put(omap->init_60m_fclk);
> diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
> index a44294d..d0815b6 100644
> --- a/drivers/usb/host/ehci-omap.c
> +++ b/drivers/usb/host/ehci-omap.c
> @@ -192,14 +192,13 @@ static int ehci_hcd_omap_probe(struct
platform_device *pdev)
> ? ? ? ? ? ? ? ?}
> ? ? ? ?}
>
> + ? ? ? /* Hold PHYs in reset while initializing EHCI controller */
> ? ? ? ?if (pdata->phy_reset) {
> ? ? ? ? ? ? ? ?if (gpio_is_valid(pdata->reset_gpio_port[0]))
> - ? ? ? ? ? ? ? ? ? ? ? gpio_request_one(pdata->reset_gpio_port[0],
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?GPIOF_OUT_INIT_LOW, "USB1 PHY
reset");
> +
gpio_set_value_cansleep(pdata->reset_gpio_port[0], 0);
>
> ? ? ? ? ? ? ? ?if (gpio_is_valid(pdata->reset_gpio_port[1]))
> - ? ? ? ? ? ? ? ? ? ? ? gpio_request_one(pdata->reset_gpio_port[1],
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?GPIOF_OUT_INIT_LOW, "USB2 PHY
reset");
> +
gpio_set_value_cansleep(pdata->reset_gpio_port[1], 0);
>
> ? ? ? ? ? ? ? ?/* Hold the PHY in RESET for enough time till DIR is high
*/
> ? ? ? ? ? ? ? ?udelay(10);
> @@ -241,6 +240,11 @@ static int ehci_hcd_omap_probe(struct
platform_device *pdev)
> ? ? ? ?omap_ehci->hcs_params = readl(&omap_ehci->caps->hcs_params);
>
> ? ? ? ?ehci_reset(omap_ehci);
> + ? ? ? ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
> + ? ? ? if (ret) {
> + ? ? ? ? ? ? ? dev_err(dev, "failed to add hcd with err %d\n", ret);
> + ? ? ? ? ? ? ? goto err_add_hcd;
> + ? ? ? }
>
> ? ? ? ?if (pdata->phy_reset) {
> ? ? ? ? ? ? ? ?/* Hold the PHY in RESET for enough time till
> @@ -255,12 +259,6 @@ static int ehci_hcd_omap_probe(struct
platform_device *pdev)
>
?gpio_set_value_cansleep(pdata->reset_gpio_port[1], 1);
> ? ? ? ?}
>
> - ? ? ? ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
> - ? ? ? if (ret) {
> - ? ? ? ? ? ? ? dev_err(dev, "failed to add hcd with err %d\n", ret);
> - ? ? ? ? ? ? ? goto err_add_hcd;
> - ? ? ? }
> -
> ? ? ? ?/* root ports should always stay powered */
> ? ? ? ?ehci_port_power(omap_ehci, 1);
>
> --
> 1.7.10.4
>

Hi Samuel

I have validated this patch v3 in omap3 beagle XM .

Acked-by: Keshava Munegowda <keshava_mgowda@ti.com>
Tested-by: Keshava Munegowda <keshava_mgowda@ti.com>

regards
keshava

Re: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Munegowda, Keshava]

On Mon, Jul 2, 2012 at 4:52 PM, Samuel Ortiz <sameo@linux.intel.com> wrote:
> Hi Russ,
>
> On Thu, Jun 14, 2012 at 09:24:21AM -0700, Russ Dill wrote:
>> From: Russ Dill <Russ.Dill@gmail.com>
>>
>> 'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
>> an issue where the ULPI PHYs were not held in reset while initializing
>> the EHCI controller. However, it also changes behavior in
>> omap-usb-host.c omap_usbhs_init by releasing reset while the
>> configuration in that function was done.
>>
>> This change caused a regression on BB-xM where USB would not function
>> if 'usb start' had been run from u-boot before booting. A change was
>> made to release reset a little bit earlier which fixed the issue on
>> BB-xM and did not cause any regressions on 3430 sdp, the board for
>> which the fix was originally made.
>>
>> This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
>> before adding hcd.', (3aa2ae74) caused a regression on OMAP5.
>>
>> The original fix to hold the EHCI controller in reset during
>> initialization was correct, however it appears that changing
>> omap_usbhs_init to not hold the PHYs in reset during it's
>> configuration was incorrect. This patch first reverts both fixes, and
>> then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
>> reset as the original patch had done. It also is sure to incorporate
>> the _cansleep change that has been made in the meantime.
>>
>> I've tested this on Beagleboard xM, I'd really like to get an ack from
>> the 3430 sdp and OMAP5 guys before getting this merged.
>>
>> v3 - Brown paper bag its too early in the morning actually run
>>      git commit amend fix
>> v2 - Put cansleep gpiolib call outside of spinlock
>>
>> Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
>> ---
>>  drivers/mfd/omap-usb-host.c  |   48 +++++++++++++++++++++++++++++++++++++++++-
>>  drivers/usb/host/ehci-omap.c |   18 +++++++---------
>>  2 files changed, 55 insertions(+), 11 deletions(-)
> I applied this one to my for-linus branch, it will be part of my next 3.5 pull
> request to Linus.
>
> Cheers,
> Samuel.
>

Hi Samuel
       please drop this patch for now , since Alan stern has mentioned
improvements for this patch and we
need to wait for Alan's new patch on ehci-omap.c and then Rus dill
patch and my usbhs tll
patch series need to be rebased.


regards
keshava

[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Munegowda, Keshava]

On Mon, Jul 2, 2012 at 4:52 PM, Samuel Ortiz <sameo@linux.intel.com> wrote:
> Hi Russ,
>
> On Thu, Jun 14, 2012 at 09:24:21AM -0700, Russ Dill wrote:
>> From: Russ Dill <Russ.Dill@gmail.com>
>>
>> 'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
>> an issue where the ULPI PHYs were not held in reset while initializing
>> the EHCI controller. However, it also changes behavior in
>> omap-usb-host.c omap_usbhs_init by releasing reset while the
>> configuration in that function was done.
>>
>> This change caused a regression on BB-xM where USB would not function
>> if 'usb start' had been run from u-boot before booting. A change was
>> made to release reset a little bit earlier which fixed the issue on
>> BB-xM and did not cause any regressions on 3430 sdp, the board for
>> which the fix was originally made.
>>
>> This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
>> before adding hcd.', (3aa2ae74) caused a regression on OMAP5.
>>
>> The original fix to hold the EHCI controller in reset during
>> initialization was correct, however it appears that changing
>> omap_usbhs_init to not hold the PHYs in reset during it's
>> configuration was incorrect. This patch first reverts both fixes, and
>> then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
>> reset as the original patch had done. It also is sure to incorporate
>> the _cansleep change that has been made in the meantime.
>>
>> I've tested this on Beagleboard xM, I'd really like to get an ack from
>> the 3430 sdp and OMAP5 guys before getting this merged.
>>
>> v3 - Brown paper bag its too early in the morning actually run
>>      git commit amend fix
>> v2 - Put cansleep gpiolib call outside of spinlock
>>
>> Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
>> ---
>>  drivers/mfd/omap-usb-host.c  |   48 +++++++++++++++++++++++++++++++++++++++++-
>>  drivers/usb/host/ehci-omap.c |   18 +++++++---------
>>  2 files changed, 55 insertions(+), 11 deletions(-)
> I applied this one to my for-linus branch, it will be part of my next 3.5 pull
> request to Linus.
>
> Cheers,
> Samuel.
>

Hi Samuel
       please drop this patch for now , since Alan stern has mentioned
improvements for this patch and we
need to wait for Alan's new patch on ehci-omap.c and then Rus dill
patch and my usbhs tll
patch series need to be rebased.


regards
keshava

Re: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Samuel Ortiz]

Hi Russ,

On Thu, Jun 14, 2012 at 09:24:21AM -0700, Russ Dill wrote:
> From: Russ Dill <Russ.Dill@gmail.com>
> 
> 'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
> an issue where the ULPI PHYs were not held in reset while initializing
> the EHCI controller. However, it also changes behavior in
> omap-usb-host.c omap_usbhs_init by releasing reset while the
> configuration in that function was done.
> 
> This change caused a regression on BB-xM where USB would not function
> if 'usb start' had been run from u-boot before booting. A change was
> made to release reset a little bit earlier which fixed the issue on
> BB-xM and did not cause any regressions on 3430 sdp, the board for
> which the fix was originally made.
> 
> This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
> before adding hcd.', (3aa2ae74) caused a regression on OMAP5.
> 
> The original fix to hold the EHCI controller in reset during
> initialization was correct, however it appears that changing
> omap_usbhs_init to not hold the PHYs in reset during it's
> configuration was incorrect. This patch first reverts both fixes, and
> then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
> reset as the original patch had done. It also is sure to incorporate
> the _cansleep change that has been made in the meantime.
> 
> I've tested this on Beagleboard xM, I'd really like to get an ack from
> the 3430 sdp and OMAP5 guys before getting this merged.
> 
> v3 - Brown paper bag its too early in the morning actually run
>      git commit amend fix
> v2 - Put cansleep gpiolib call outside of spinlock
> 
> Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
> ---
>  drivers/mfd/omap-usb-host.c  |   48 +++++++++++++++++++++++++++++++++++++++++-
>  drivers/usb/host/ehci-omap.c |   18 +++++++---------
>  2 files changed, 55 insertions(+), 11 deletions(-)
I applied this one to my for-linus branch, it will be part of my next 3.5 pull
request to Linus.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Samuel Ortiz]

Hi Russ,

On Thu, Jun 14, 2012 at 09:24:21AM -0700, Russ Dill wrote:
> From: Russ Dill <Russ.Dill@gmail.com>
> 
> 'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
> an issue where the ULPI PHYs were not held in reset while initializing
> the EHCI controller. However, it also changes behavior in
> omap-usb-host.c omap_usbhs_init by releasing reset while the
> configuration in that function was done.
> 
> This change caused a regression on BB-xM where USB would not function
> if 'usb start' had been run from u-boot before booting. A change was
> made to release reset a little bit earlier which fixed the issue on
> BB-xM and did not cause any regressions on 3430 sdp, the board for
> which the fix was originally made.
> 
> This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
> before adding hcd.', (3aa2ae74) caused a regression on OMAP5.
> 
> The original fix to hold the EHCI controller in reset during
> initialization was correct, however it appears that changing
> omap_usbhs_init to not hold the PHYs in reset during it's
> configuration was incorrect. This patch first reverts both fixes, and
> then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
> reset as the original patch had done. It also is sure to incorporate
> the _cansleep change that has been made in the meantime.
> 
> I've tested this on Beagleboard xM, I'd really like to get an ack from
> the 3430 sdp and OMAP5 guys before getting this merged.
> 
> v3 - Brown paper bag its too early in the morning actually run
>      git commit amend fix
> v2 - Put cansleep gpiolib call outside of spinlock
> 
> Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
> ---
>  drivers/mfd/omap-usb-host.c  |   48 +++++++++++++++++++++++++++++++++++++++++-
>  drivers/usb/host/ehci-omap.c |   18 +++++++---------
>  2 files changed, 55 insertions(+), 11 deletions(-)
I applied this one to my for-linus branch, it will be part of my next 3.5 pull
request to Linus.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Alan Stern]

On Mon, 2 Jul 2012, Munegowda, Keshava wrote:

> Hi Samuel
>        please drop this patch for now , since Alan stern has mentioned
> improvements for this patch and we
> need to wait for Alan's new patch on ehci-omap.c and then Rus dill
> patch and my usbhs tll
> patch series need to be rebased.

No, that's okay.  I rebased my patch on top of Russ's; see

	http://marc.info/?l=linux-usb&m=134098478928668&w=2

so Russ's patch should be kept.  My patch needed other changes also, so
this wasn't a big deal.

The extra problems I mentioned in an earlier email will still need to
be fixed after both Russ's patch and my own are merged.

Alan Stern

[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Alan Stern]

On Mon, 2 Jul 2012, Munegowda, Keshava wrote:

> Hi Samuel
>        please drop this patch for now , since Alan stern has mentioned
> improvements for this patch and we
> need to wait for Alan's new patch on ehci-omap.c and then Rus dill
> patch and my usbhs tll
> patch series need to be rebased.

No, that's okay.  I rebased my patch on top of Russ's; see

	http://marc.info/?l=linux-usb&m=134098478928668&w=2

so Russ's patch should be kept.  My patch needed other changes also, so
this wasn't a big deal.

The extra problems I mentioned in an earlier email will still need to
be fixed after both Russ's patch and my own are merged.

Alan Stern

Re: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Munegowda, Keshava]

On Mon, Jul 2, 2012 at 7:35 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Mon, 2 Jul 2012, Munegowda, Keshava wrote:
>
>> Hi Samuel
>>        please drop this patch for now , since Alan stern has mentioned
>> improvements for this patch and we
>> need to wait for Alan's new patch on ehci-omap.c and then Rus dill
>> patch and my usbhs tll
>> patch series need to be rebased.
>
> No, that's okay.  I rebased my patch on top of Russ's; see
>
>         http://marc.info/?l=linux-usb&m=134098478928668&w=2
>
> so Russ's patch should be kept.  My patch needed other changes also, so
> this wasn't a big deal.
>
> The extra problems I mentioned in an earlier email will still need to
> be fixed after both Russ's patch and my own are merged.
>
> Alan Stern

Thanks Alan,
            Ok, then I will rebase my TLL driver patch series on top
of this patch.

regards
keshava

[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Munegowda, Keshava]

On Mon, Jul 2, 2012 at 7:35 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Mon, 2 Jul 2012, Munegowda, Keshava wrote:
>
>> Hi Samuel
>>        please drop this patch for now , since Alan stern has mentioned
>> improvements for this patch and we
>> need to wait for Alan's new patch on ehci-omap.c and then Rus dill
>> patch and my usbhs tll
>> patch series need to be rebased.
>
> No, that's okay.  I rebased my patch on top of Russ's; see
>
>         http://marc.info/?l=linux-usb&m=134098478928668&w=2
>
> so Russ's patch should be kept.  My patch needed other changes also, so
> this wasn't a big deal.
>
> The extra problems I mentioned in an earlier email will still need to
> be fixed after both Russ's patch and my own are merged.
>
> Alan Stern

Thanks Alan,
            Ok, then I will rebase my TLL driver patch series on top
of this patch.

regards
keshava

Re: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Munegowda, Keshava]

On Fri, Jun 29, 2012 at 9:03 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 27 Jun 2012, Russ Dill wrote:
>
>> From: Russ Dill <Russ.Dill@gmail.com>
>>
>> 'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
>> an issue where the ULPI PHYs were not held in reset while initializing
>> the EHCI controller. However, it also changes behavior in
>> omap-usb-host.c omap_usbhs_init by releasing reset while the
>> configuration in that function was done.
>>
>> This change caused a regression on BB-xM where USB would not function
>> if 'usb start' had been run from u-boot before booting. A change was
>> made to release reset a little bit earlier which fixed the issue on
>> BB-xM and did not cause any regressions on 3430 sdp, the board for
>> which the fix was originally made.
>>
>> This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
>> before adding hcd.', (3aa2ae74) caused a regression on OMAP5.
>>
>> The original fix to hold the EHCI controller in reset during
>> initialization was correct, however it appears that changing
>> omap_usbhs_init to not hold the PHYs in reset during it's
>> configuration was incorrect. This patch first reverts both fixes, and
>> then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
>> reset as the original patch had done. It also is sure to incorporate
>> the _cansleep change that has been made in the meantime.
>>
>> Tested on Beagleboard xM.
>
> Looking at the result of this patch, there seem to be a few mistakes
> remaining in the probe routine.
>
> If the regulator_get() call fails, the failure is logged but ignored.
> Is that really the right thing to do?
>
> The pm_runtime_get_sync() call occurs before the probe is finished.
> This means that ehci-hcd's resume routine will try to power-up the
> device before its data structures have been initialized.  That can't be
> right.
>
> The "get clocks" section occurs after the call to usb_add_hcd().  This
> means the controller will start running before the clock references are
> acquired -- so the clocks might still be turned off.  That can't be
> right either.
>
> If the clk_get(dev, "utmi_p1_gfclk") call fails, the error path never
> calls usb_remove_hcd().
>
> The "err_add_hcd" pathway never calls usb_put_hcd().
>
> I'm going to resubmit my most recent patch based the current
> ehci-omap.c, but you or someone else will still have to fix these
> problems.
>
> Alan Stern
>

Hi Rus Dill,
    once Alan post his changes on ehci-omap.c, please re-base this
patch on top of it.
so that, I will rebase UHH-TLL split series on top your patch.

regards
keshava

[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Munegowda, Keshava]

On Fri, Jun 29, 2012 at 9:03 PM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Wed, 27 Jun 2012, Russ Dill wrote:
>
>> From: Russ Dill <Russ.Dill@gmail.com>
>>
>> 'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
>> an issue where the ULPI PHYs were not held in reset while initializing
>> the EHCI controller. However, it also changes behavior in
>> omap-usb-host.c omap_usbhs_init by releasing reset while the
>> configuration in that function was done.
>>
>> This change caused a regression on BB-xM where USB would not function
>> if 'usb start' had been run from u-boot before booting. A change was
>> made to release reset a little bit earlier which fixed the issue on
>> BB-xM and did not cause any regressions on 3430 sdp, the board for
>> which the fix was originally made.
>>
>> This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
>> before adding hcd.', (3aa2ae74) caused a regression on OMAP5.
>>
>> The original fix to hold the EHCI controller in reset during
>> initialization was correct, however it appears that changing
>> omap_usbhs_init to not hold the PHYs in reset during it's
>> configuration was incorrect. This patch first reverts both fixes, and
>> then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
>> reset as the original patch had done. It also is sure to incorporate
>> the _cansleep change that has been made in the meantime.
>>
>> Tested on Beagleboard xM.
>
> Looking at the result of this patch, there seem to be a few mistakes
> remaining in the probe routine.
>
> If the regulator_get() call fails, the failure is logged but ignored.
> Is that really the right thing to do?
>
> The pm_runtime_get_sync() call occurs before the probe is finished.
> This means that ehci-hcd's resume routine will try to power-up the
> device before its data structures have been initialized.  That can't be
> right.
>
> The "get clocks" section occurs after the call to usb_add_hcd().  This
> means the controller will start running before the clock references are
> acquired -- so the clocks might still be turned off.  That can't be
> right either.
>
> If the clk_get(dev, "utmi_p1_gfclk") call fails, the error path never
> calls usb_remove_hcd().
>
> The "err_add_hcd" pathway never calls usb_put_hcd().
>
> I'm going to resubmit my most recent patch based the current
> ehci-omap.c, but you or someone else will still have to fix these
> problems.
>
> Alan Stern
>

Hi Rus Dill,
    once Alan post his changes on ehci-omap.c, please re-base this
patch on top of it.
so that, I will rebase UHH-TLL split series on top your patch.

regards
keshava

Re: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Alan Stern]

On Wed, 27 Jun 2012, Russ Dill wrote:

> From: Russ Dill <Russ.Dill@gmail.com>
> 
> 'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
> an issue where the ULPI PHYs were not held in reset while initializing
> the EHCI controller. However, it also changes behavior in
> omap-usb-host.c omap_usbhs_init by releasing reset while the
> configuration in that function was done.
> 
> This change caused a regression on BB-xM where USB would not function
> if 'usb start' had been run from u-boot before booting. A change was
> made to release reset a little bit earlier which fixed the issue on
> BB-xM and did not cause any regressions on 3430 sdp, the board for
> which the fix was originally made.
> 
> This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
> before adding hcd.', (3aa2ae74) caused a regression on OMAP5.
> 
> The original fix to hold the EHCI controller in reset during
> initialization was correct, however it appears that changing
> omap_usbhs_init to not hold the PHYs in reset during it's
> configuration was incorrect. This patch first reverts both fixes, and
> then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
> reset as the original patch had done. It also is sure to incorporate
> the _cansleep change that has been made in the meantime.
> 
> Tested on Beagleboard xM.

Looking at the result of this patch, there seem to be a few mistakes 
remaining in the probe routine.

If the regulator_get() call fails, the failure is logged but ignored.  
Is that really the right thing to do?

The pm_runtime_get_sync() call occurs before the probe is finished.  
This means that ehci-hcd's resume routine will try to power-up the 
device before its data structures have been initialized.  That can't be 
right.

The "get clocks" section occurs after the call to usb_add_hcd().  This
means the controller will start running before the clock references are
acquired -- so the clocks might still be turned off.  That can't be
right either.

If the clk_get(dev, "utmi_p1_gfclk") call fails, the error path never 
calls usb_remove_hcd().

The "err_add_hcd" pathway never calls usb_put_hcd().

I'm going to resubmit my most recent patch based the current 
ehci-omap.c, but you or someone else will still have to fix these 
problems.

Alan Stern

[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Alan Stern]

On Wed, 27 Jun 2012, Russ Dill wrote:

> From: Russ Dill <Russ.Dill@gmail.com>
> 
> 'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
> an issue where the ULPI PHYs were not held in reset while initializing
> the EHCI controller. However, it also changes behavior in
> omap-usb-host.c omap_usbhs_init by releasing reset while the
> configuration in that function was done.
> 
> This change caused a regression on BB-xM where USB would not function
> if 'usb start' had been run from u-boot before booting. A change was
> made to release reset a little bit earlier which fixed the issue on
> BB-xM and did not cause any regressions on 3430 sdp, the board for
> which the fix was originally made.
> 
> This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
> before adding hcd.', (3aa2ae74) caused a regression on OMAP5.
> 
> The original fix to hold the EHCI controller in reset during
> initialization was correct, however it appears that changing
> omap_usbhs_init to not hold the PHYs in reset during it's
> configuration was incorrect. This patch first reverts both fixes, and
> then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
> reset as the original patch had done. It also is sure to incorporate
> the _cansleep change that has been made in the meantime.
> 
> Tested on Beagleboard xM.

Looking at the result of this patch, there seem to be a few mistakes 
remaining in the probe routine.

If the regulator_get() call fails, the failure is logged but ignored.  
Is that really the right thing to do?

The pm_runtime_get_sync() call occurs before the probe is finished.  
This means that ehci-hcd's resume routine will try to power-up the 
device before its data structures have been initialized.  That can't be 
right.

The "get clocks" section occurs after the call to usb_add_hcd().  This
means the controller will start running before the clock references are
acquired -- so the clocks might still be turned off.  That can't be
right either.

If the clk_get(dev, "utmi_p1_gfclk") call fails, the error path never 
calls usb_remove_hcd().

The "err_add_hcd" pathway never calls usb_put_hcd().

I'm going to resubmit my most recent patch based the current 
ehci-omap.c, but you or someone else will still have to fix these 
problems.

Alan Stern

RE: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Sarashetti, Mantesh]

'Acked-by: mantesh-l0cyMroinI0@public.gmane.org'
'Tested-by: mantesh-l0cyMroinI0@public.gmane.org'

Regards,
Mantesh

-----Original Message-----
From: Russ Dill [mailto:russ.dill-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org] On Behalf Of Dill, Russ
Sent: Wednesday, June 27, 2012 9:00 PM
To: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org; linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Balbi, Felipe; Munegowda, Keshava; Partha Basak; Igor Grinberg; Samuel Ortiz; Sarashetti, Mantesh; Setty, Sapna; Russ Dill
Subject: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

From: Russ Dill <Russ.Dill-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>

'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
an issue where the ULPI PHYs were not held in reset while initializing
the EHCI controller. However, it also changes behavior in
omap-usb-host.c omap_usbhs_init by releasing reset while the
configuration in that function was done.

This change caused a regression on BB-xM where USB would not function
if 'usb start' had been run from u-boot before booting. A change was
made to release reset a little bit earlier which fixed the issue on
BB-xM and did not cause any regressions on 3430 sdp, the board for
which the fix was originally made.

This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
before adding hcd.', (3aa2ae74) caused a regression on OMAP5.

The original fix to hold the EHCI controller in reset during
initialization was correct, however it appears that changing
omap_usbhs_init to not hold the PHYs in reset during it's
configuration was incorrect. This patch first reverts both fixes, and
then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
reset as the original patch had done. It also is sure to incorporate
the _cansleep change that has been made in the meantime.

Tested on Beagleboard xM.

v3 - Don't double free gpio
v2 - Put cansleep gpiolib call outside of spinlock

Signed-off-by: Russ Dill <Russ.Dill-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/mfd/omap-usb-host.c  |   48 +++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/host/ehci-omap.c |   26 +++++++----------------
 2 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 7e96bb2..41088ec 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -25,6 +25,7 @@
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/spinlock.h>
+#include <linux/gpio.h>
 #include <plat/cpu.h>
 #include <plat/usb.h>
 #include <linux/pm_runtime.h>
@@ -500,8 +501,21 @@ static void omap_usbhs_init(struct device *dev)
 	dev_dbg(dev, "starting TI HSUSB Controller\n");
 
 	pm_runtime_get_sync(dev);
-	spin_lock_irqsave(&omap->lock, flags);
 
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[0],
+					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[1],
+					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+
+		/* Hold the PHY in RESET for enough time till DIR is high */
+		udelay(10);
+	}
+
+	spin_lock_irqsave(&omap->lock, flags);
 	omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
 	dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
 
@@ -581,9 +595,39 @@ static void omap_usbhs_init(struct device *dev)
 	}
 
 	spin_unlock_irqrestore(&omap->lock, flags);
+
+	if (pdata->ehci_data->phy_reset) {
+		/* Hold the PHY in RESET for enough time till
+		 * PHY is settled and ready
+		 */
+		udelay(10);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[0], 1);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[1], 1);
+	}
+
 	pm_runtime_put_sync(dev);
 }
 
+static void omap_usbhs_deinit(struct device *dev)
+{
+	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
+	struct usbhs_omap_platform_data	*pdata = &omap->platdata;
+
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[0]);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[1]);
+	}
+}
+
 
 /**
  * usbhs_omap_probe - initialize TI-based HCDs
@@ -767,6 +811,7 @@ static int __devinit usbhs_omap_probe(struct platform_device *pdev)
 	goto end_probe;
 
 err_alloc:
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 
 err_tll:
@@ -818,6 +863,7 @@ static int __devexit usbhs_omap_remove(struct platform_device *pdev)
 {
 	struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
 
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 	iounmap(omap->uhh_base);
 	clk_put(omap->init_60m_fclk);
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index a44294d..35f4788 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -192,14 +192,13 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Hold PHYs in reset while initializing EHCI controller */
 	if (pdata->phy_reset) {
 		if (gpio_is_valid(pdata->reset_gpio_port[0]))
-			gpio_request_one(pdata->reset_gpio_port[0],
-					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[0], 0);
 
 		if (gpio_is_valid(pdata->reset_gpio_port[1]))
-			gpio_request_one(pdata->reset_gpio_port[1],
-					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 0);
 
 		/* Hold the PHY in RESET for enough time till DIR is high */
 		udelay(10);
@@ -241,6 +240,11 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 	omap_ehci->hcs_params = readl(&omap_ehci->caps->hcs_params);
 
 	ehci_reset(omap_ehci);
+	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+	if (ret) {
+		dev_err(dev, "failed to add hcd with err %d\n", ret);
+		goto err_add_hcd;
+	}
 
 	if (pdata->phy_reset) {
 		/* Hold the PHY in RESET for enough time till
@@ -255,12 +259,6 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 1);
 	}
 
-	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
-	if (ret) {
-		dev_err(dev, "failed to add hcd with err %d\n", ret);
-		goto err_add_hcd;
-	}
-
 	/* root ports should always stay powered */
 	ehci_port_power(omap_ehci, 1);
 
@@ -288,7 +286,6 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev)
 {
 	struct device *dev				= &pdev->dev;
 	struct usb_hcd *hcd				= dev_get_drvdata(dev);
-	struct ehci_hcd_omap_platform_data *pdata	= dev->platform_data;
 
 	usb_remove_hcd(hcd);
 	disable_put_regulator(dev->platform_data);
@@ -297,13 +294,6 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev)
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
 
-	if (pdata->phy_reset) {
-		if (gpio_is_valid(pdata->reset_gpio_port[0]))
-			gpio_free(pdata->reset_gpio_port[0]);

[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Sarashetti, Mantesh]

'Acked-by: mantesh at ti.com'
'Tested-by: mantesh at ti.com'

Regards,
Mantesh

-----Original Message-----
From: Russ Dill [mailto:russ.dill at gmail.com] On Behalf Of Dill, Russ
Sent: Wednesday, June 27, 2012 9:00 PM
To: linux-omap at vger.kernel.org; linux-arm-kernel at lists.infradead.org; linux-usb at vger.kernel.org
Cc: Balbi, Felipe; Munegowda, Keshava; Partha Basak; Igor Grinberg; Samuel Ortiz; Sarashetti, Mantesh; Setty, Sapna; Russ Dill
Subject: [PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

From: Russ Dill <Russ.Dill@gmail.com>

'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
an issue where the ULPI PHYs were not held in reset while initializing
the EHCI controller. However, it also changes behavior in
omap-usb-host.c omap_usbhs_init by releasing reset while the
configuration in that function was done.

This change caused a regression on BB-xM where USB would not function
if 'usb start' had been run from u-boot before booting. A change was
made to release reset a little bit earlier which fixed the issue on
BB-xM and did not cause any regressions on 3430 sdp, the board for
which the fix was originally made.

This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
before adding hcd.', (3aa2ae74) caused a regression on OMAP5.

The original fix to hold the EHCI controller in reset during
initialization was correct, however it appears that changing
omap_usbhs_init to not hold the PHYs in reset during it's
configuration was incorrect. This patch first reverts both fixes, and
then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
reset as the original patch had done. It also is sure to incorporate
the _cansleep change that has been made in the meantime.

Tested on Beagleboard xM.

v3 - Don't double free gpio
v2 - Put cansleep gpiolib call outside of spinlock

Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
---
 drivers/mfd/omap-usb-host.c  |   48 +++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/host/ehci-omap.c |   26 +++++++----------------
 2 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 7e96bb2..41088ec 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -25,6 +25,7 @@
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/spinlock.h>
+#include <linux/gpio.h>
 #include <plat/cpu.h>
 #include <plat/usb.h>
 #include <linux/pm_runtime.h>
@@ -500,8 +501,21 @@ static void omap_usbhs_init(struct device *dev)
 	dev_dbg(dev, "starting TI HSUSB Controller\n");
 
 	pm_runtime_get_sync(dev);
-	spin_lock_irqsave(&omap->lock, flags);
 
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[0],
+					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[1],
+					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+
+		/* Hold the PHY in RESET for enough time till DIR is high */
+		udelay(10);
+	}
+
+	spin_lock_irqsave(&omap->lock, flags);
 	omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
 	dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
 
@@ -581,9 +595,39 @@ static void omap_usbhs_init(struct device *dev)
 	}
 
 	spin_unlock_irqrestore(&omap->lock, flags);
+
+	if (pdata->ehci_data->phy_reset) {
+		/* Hold the PHY in RESET for enough time till
+		 * PHY is settled and ready
+		 */
+		udelay(10);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[0], 1);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[1], 1);
+	}
+
 	pm_runtime_put_sync(dev);
 }
 
+static void omap_usbhs_deinit(struct device *dev)
+{
+	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
+	struct usbhs_omap_platform_data	*pdata = &omap->platdata;
+
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[0]);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[1]);
+	}
+}
+
 
 /**
  * usbhs_omap_probe - initialize TI-based HCDs
@@ -767,6 +811,7 @@ static int __devinit usbhs_omap_probe(struct platform_device *pdev)
 	goto end_probe;
 
 err_alloc:
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 
 err_tll:
@@ -818,6 +863,7 @@ static int __devexit usbhs_omap_remove(struct platform_device *pdev)
 {
 	struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
 
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 	iounmap(omap->uhh_base);
 	clk_put(omap->init_60m_fclk);
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index a44294d..35f4788 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -192,14 +192,13 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Hold PHYs in reset while initializing EHCI controller */
 	if (pdata->phy_reset) {
 		if (gpio_is_valid(pdata->reset_gpio_port[0]))
-			gpio_request_one(pdata->reset_gpio_port[0],
-					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[0], 0);
 
 		if (gpio_is_valid(pdata->reset_gpio_port[1]))
-			gpio_request_one(pdata->reset_gpio_port[1],
-					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 0);
 
 		/* Hold the PHY in RESET for enough time till DIR is high */
 		udelay(10);
@@ -241,6 +240,11 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 	omap_ehci->hcs_params = readl(&omap_ehci->caps->hcs_params);
 
 	ehci_reset(omap_ehci);
+	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+	if (ret) {
+		dev_err(dev, "failed to add hcd with err %d\n", ret);
+		goto err_add_hcd;
+	}
 
 	if (pdata->phy_reset) {
 		/* Hold the PHY in RESET for enough time till
@@ -255,12 +259,6 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 1);
 	}
 
-	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
-	if (ret) {
-		dev_err(dev, "failed to add hcd with err %d\n", ret);
-		goto err_add_hcd;
-	}
-
 	/* root ports should always stay powered */
 	ehci_port_power(omap_ehci, 1);
 
@@ -288,7 +286,6 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev)
 {
 	struct device *dev				= &pdev->dev;
 	struct usb_hcd *hcd				= dev_get_drvdata(dev);
-	struct ehci_hcd_omap_platform_data *pdata	= dev->platform_data;
 
 	usb_remove_hcd(hcd);
 	disable_put_regulator(dev->platform_data);
@@ -297,13 +294,6 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev)
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
 
-	if (pdata->phy_reset) {
-		if (gpio_is_valid(pdata->reset_gpio_port[0]))
-			gpio_free(pdata->reset_gpio_port[0]);
-
-		if (gpio_is_valid(pdata->reset_gpio_port[1]))
-			gpio_free(pdata->reset_gpio_port[1]);
-	}
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Russ Dill]

From: Russ Dill <Russ.Dill@gmail.com>

'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
an issue where the ULPI PHYs were not held in reset while initializing
the EHCI controller. However, it also changes behavior in
omap-usb-host.c omap_usbhs_init by releasing reset while the
configuration in that function was done.

This change caused a regression on BB-xM where USB would not function
if 'usb start' had been run from u-boot before booting. A change was
made to release reset a little bit earlier which fixed the issue on
BB-xM and did not cause any regressions on 3430 sdp, the board for
which the fix was originally made.

This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
before adding hcd.', (3aa2ae74) caused a regression on OMAP5.

The original fix to hold the EHCI controller in reset during
initialization was correct, however it appears that changing
omap_usbhs_init to not hold the PHYs in reset during it's
configuration was incorrect. This patch first reverts both fixes, and
then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
reset as the original patch had done. It also is sure to incorporate
the _cansleep change that has been made in the meantime.

Tested on Beagleboard xM.

v3 - Don't double free gpio
v2 - Put cansleep gpiolib call outside of spinlock

Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
---
 drivers/mfd/omap-usb-host.c  |   48 +++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/host/ehci-omap.c |   26 +++++++----------------
 2 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 7e96bb2..41088ec 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -25,6 +25,7 @@
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/spinlock.h>
+#include <linux/gpio.h>
 #include <plat/cpu.h>
 #include <plat/usb.h>
 #include <linux/pm_runtime.h>
@@ -500,8 +501,21 @@ static void omap_usbhs_init(struct device *dev)
 	dev_dbg(dev, "starting TI HSUSB Controller\n");
 
 	pm_runtime_get_sync(dev);
-	spin_lock_irqsave(&omap->lock, flags);
 
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[0],
+					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[1],
+					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+
+		/* Hold the PHY in RESET for enough time till DIR is high */
+		udelay(10);
+	}
+
+	spin_lock_irqsave(&omap->lock, flags);
 	omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
 	dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
 
@@ -581,9 +595,39 @@ static void omap_usbhs_init(struct device *dev)
 	}
 
 	spin_unlock_irqrestore(&omap->lock, flags);
+
+	if (pdata->ehci_data->phy_reset) {
+		/* Hold the PHY in RESET for enough time till
+		 * PHY is settled and ready
+		 */
+		udelay(10);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[0], 1);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[1], 1);
+	}
+
 	pm_runtime_put_sync(dev);
 }
 
+static void omap_usbhs_deinit(struct device *dev)
+{
+	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
+	struct usbhs_omap_platform_data	*pdata = &omap->platdata;
+
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[0]);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[1]);
+	}
+}
+
 
 /**
  * usbhs_omap_probe - initialize TI-based HCDs
@@ -767,6 +811,7 @@ static int __devinit usbhs_omap_probe(struct platform_device *pdev)
 	goto end_probe;
 
 err_alloc:
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 
 err_tll:
@@ -818,6 +863,7 @@ static int __devexit usbhs_omap_remove(struct platform_device *pdev)
 {
 	struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
 
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 	iounmap(omap->uhh_base);
 	clk_put(omap->init_60m_fclk);
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index a44294d..35f4788 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -192,14 +192,13 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Hold PHYs in reset while initializing EHCI controller */
 	if (pdata->phy_reset) {
 		if (gpio_is_valid(pdata->reset_gpio_port[0]))
-			gpio_request_one(pdata->reset_gpio_port[0],
-					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[0], 0);
 
 		if (gpio_is_valid(pdata->reset_gpio_port[1]))
-			gpio_request_one(pdata->reset_gpio_port[1],
-					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 0);
 
 		/* Hold the PHY in RESET for enough time till DIR is high */
 		udelay(10);
@@ -241,6 +240,11 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 	omap_ehci->hcs_params = readl(&omap_ehci->caps->hcs_params);
 
 	ehci_reset(omap_ehci);
+	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+	if (ret) {
+		dev_err(dev, "failed to add hcd with err %d\n", ret);
+		goto err_add_hcd;
+	}
 
 	if (pdata->phy_reset) {
 		/* Hold the PHY in RESET for enough time till
@@ -255,12 +259,6 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 1);
 	}
 
-	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
-	if (ret) {
-		dev_err(dev, "failed to add hcd with err %d\n", ret);
-		goto err_add_hcd;
-	}
-
 	/* root ports should always stay powered */
 	ehci_port_power(omap_ehci, 1);
 
@@ -288,7 +286,6 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev)
 {
 	struct device *dev				= &pdev->dev;
 	struct usb_hcd *hcd				= dev_get_drvdata(dev);
-	struct ehci_hcd_omap_platform_data *pdata	= dev->platform_data;
 
 	usb_remove_hcd(hcd);
 	disable_put_regulator(dev->platform_data);
@@ -297,13 +294,6 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev)
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
 
-	if (pdata->phy_reset) {
-		if (gpio_is_valid(pdata->reset_gpio_port[0]))
-			gpio_free(pdata->reset_gpio_port[0]);
-
-		if (gpio_is_valid(pdata->reset_gpio_port[1]))
-			gpio_free(pdata->reset_gpio_port[1]);
-	}
 	return 0;
 }
 
-- 
1.7.10.4

[PATCH v3] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.

[Russ Dill]

From: Russ Dill <Russ.Dill@gmail.com>

'ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset issue' (1fcb57d0) fixes
an issue where the ULPI PHYs were not held in reset while initializing
the EHCI controller. However, it also changes behavior in
omap-usb-host.c omap_usbhs_init by releasing reset while the
configuration in that function was done.

This change caused a regression on BB-xM where USB would not function
if 'usb start' had been run from u-boot before booting. A change was
made to release reset a little bit earlier which fixed the issue on
BB-xM and did not cause any regressions on 3430 sdp, the board for
which the fix was originally made.

This new fix, 'USB: EHCI: OMAP: Finish ehci omap phy reset cycle
before adding hcd.', (3aa2ae74) caused a regression on OMAP5.

The original fix to hold the EHCI controller in reset during
initialization was correct, however it appears that changing
omap_usbhs_init to not hold the PHYs in reset during it's
configuration was incorrect. This patch first reverts both fixes, and
then changes ehci_hcd_omap_probe in ehci-omap.c to hold the PHYs in
reset as the original patch had done. It also is sure to incorporate
the _cansleep change that has been made in the meantime.

Tested on Beagleboard xM.

v3 - Don't double free gpio
v2 - Put cansleep gpiolib call outside of spinlock

Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
---
 drivers/mfd/omap-usb-host.c  |   48 +++++++++++++++++++++++++++++++++++++++++-
 drivers/usb/host/ehci-omap.c |   26 +++++++----------------
 2 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 7e96bb2..41088ec 100644
--- a/drivers/mfd/omap-usb-host.c
+++ b/drivers/mfd/omap-usb-host.c
@@ -25,6 +25,7 @@
 #include <linux/clk.h>
 #include <linux/dma-mapping.h>
 #include <linux/spinlock.h>
+#include <linux/gpio.h>
 #include <plat/cpu.h>
 #include <plat/usb.h>
 #include <linux/pm_runtime.h>
@@ -500,8 +501,21 @@ static void omap_usbhs_init(struct device *dev)
 	dev_dbg(dev, "starting TI HSUSB Controller\n");
 
 	pm_runtime_get_sync(dev);
-	spin_lock_irqsave(&omap->lock, flags);
 
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[0],
+					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_request_one(pdata->ehci_data->reset_gpio_port[1],
+					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+
+		/* Hold the PHY in RESET for enough time till DIR is high */
+		udelay(10);
+	}
+
+	spin_lock_irqsave(&omap->lock, flags);
 	omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
 	dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
 
@@ -581,9 +595,39 @@ static void omap_usbhs_init(struct device *dev)
 	}
 
 	spin_unlock_irqrestore(&omap->lock, flags);
+
+	if (pdata->ehci_data->phy_reset) {
+		/* Hold the PHY in RESET for enough time till
+		 * PHY is settled and ready
+		 */
+		udelay(10);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[0], 1);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_set_value_cansleep
+				(pdata->ehci_data->reset_gpio_port[1], 1);
+	}
+
 	pm_runtime_put_sync(dev);
 }
 
+static void omap_usbhs_deinit(struct device *dev)
+{
+	struct usbhs_hcd_omap		*omap = dev_get_drvdata(dev);
+	struct usbhs_omap_platform_data	*pdata = &omap->platdata;
+
+	if (pdata->ehci_data->phy_reset) {
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[0]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[0]);
+
+		if (gpio_is_valid(pdata->ehci_data->reset_gpio_port[1]))
+			gpio_free(pdata->ehci_data->reset_gpio_port[1]);
+	}
+}
+
 
 /**
  * usbhs_omap_probe - initialize TI-based HCDs
@@ -767,6 +811,7 @@ static int __devinit usbhs_omap_probe(struct platform_device *pdev)
 	goto end_probe;
 
 err_alloc:
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 
 err_tll:
@@ -818,6 +863,7 @@ static int __devexit usbhs_omap_remove(struct platform_device *pdev)
 {
 	struct usbhs_hcd_omap *omap = platform_get_drvdata(pdev);
 
+	omap_usbhs_deinit(&pdev->dev);
 	iounmap(omap->tll_base);
 	iounmap(omap->uhh_base);
 	clk_put(omap->init_60m_fclk);
diff --git a/drivers/usb/host/ehci-omap.c b/drivers/usb/host/ehci-omap.c
index a44294d..35f4788 100644
--- a/drivers/usb/host/ehci-omap.c
+++ b/drivers/usb/host/ehci-omap.c
@@ -192,14 +192,13 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 		}
 	}
 
+	/* Hold PHYs in reset while initializing EHCI controller */
 	if (pdata->phy_reset) {
 		if (gpio_is_valid(pdata->reset_gpio_port[0]))
-			gpio_request_one(pdata->reset_gpio_port[0],
-					 GPIOF_OUT_INIT_LOW, "USB1 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[0], 0);
 
 		if (gpio_is_valid(pdata->reset_gpio_port[1]))
-			gpio_request_one(pdata->reset_gpio_port[1],
-					 GPIOF_OUT_INIT_LOW, "USB2 PHY reset");
+			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 0);
 
 		/* Hold the PHY in RESET for enough time till DIR is high */
 		udelay(10);
@@ -241,6 +240,11 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 	omap_ehci->hcs_params = readl(&omap_ehci->caps->hcs_params);
 
 	ehci_reset(omap_ehci);
+	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
+	if (ret) {
+		dev_err(dev, "failed to add hcd with err %d\n", ret);
+		goto err_add_hcd;
+	}
 
 	if (pdata->phy_reset) {
 		/* Hold the PHY in RESET for enough time till
@@ -255,12 +259,6 @@ static int ehci_hcd_omap_probe(struct platform_device *pdev)
 			gpio_set_value_cansleep(pdata->reset_gpio_port[1], 1);
 	}
 
-	ret = usb_add_hcd(hcd, irq, IRQF_SHARED);
-	if (ret) {
-		dev_err(dev, "failed to add hcd with err %d\n", ret);
-		goto err_add_hcd;
-	}
-
 	/* root ports should always stay powered */
 	ehci_port_power(omap_ehci, 1);
 
@@ -288,7 +286,6 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev)
 {
 	struct device *dev				= &pdev->dev;
 	struct usb_hcd *hcd				= dev_get_drvdata(dev);
-	struct ehci_hcd_omap_platform_data *pdata	= dev->platform_data;
 
 	usb_remove_hcd(hcd);
 	disable_put_regulator(dev->platform_data);
@@ -297,13 +294,6 @@ static int ehci_hcd_omap_remove(struct platform_device *pdev)
 	pm_runtime_put_sync(dev);
 	pm_runtime_disable(dev);
 
-	if (pdata->phy_reset) {
-		if (gpio_is_valid(pdata->reset_gpio_port[0]))
-			gpio_free(pdata->reset_gpio_port[0]);
-
-		if (gpio_is_valid(pdata->reset_gpio_port[1]))
-			gpio_free(pdata->reset_gpio_port[1]);
-	}
 	return 0;
 }
 
-- 
1.7.10.4