All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.
@ 2012-06-14 11:31 ` Russ Dill
  0 siblings, 0 replies; 6+ messages in thread
From: Russ Dill @ 2012-06-14 11:31 UTC (permalink / raw)
  To: linux-omap, linux-arm-kernel
  Cc: Felipe Balbi, Keshava Munegowda, Partha Basak, Igor Grinberg,
	Samuel Ortiz, mantesh, s-sapna1, Russ Dill, Russ Dill

'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.

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

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 7e96bb2..10984e2 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>
@@ -502,6 +503,19 @@ static void omap_usbhs_init(struct device *dev)
 	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);
+	}
+
 	omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
 	dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
 
@@ -580,10 +594,39 @@ static void omap_usbhs_init(struct device *dev)
 			usbhs_omap_tll_init(dev, OMAP_TLL_CHANNEL_COUNT);
 	}
 
+	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);
+	}
+
 	spin_unlock_irqrestore(&omap->lock, flags);
 	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 +810,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 +862,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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.
@ 2012-06-14 11:31 ` Russ Dill
  0 siblings, 0 replies; 6+ messages in thread
From: Russ Dill @ 2012-06-14 11:31 UTC (permalink / raw)
  To: linux-arm-kernel

'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.

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

diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
index 7e96bb2..10984e2 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>
@@ -502,6 +503,19 @@ static void omap_usbhs_init(struct device *dev)
 	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);
+	}
+
 	omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
 	dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
 
@@ -580,10 +594,39 @@ static void omap_usbhs_init(struct device *dev)
 			usbhs_omap_tll_init(dev, OMAP_TLL_CHANNEL_COUNT);
 	}
 
+	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);
+	}
+
 	spin_unlock_irqrestore(&omap->lock, flags);
 	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 +810,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 +862,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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.
  2012-06-14 11:31 ` Russ Dill
@ 2012-06-14 12:59   ` Igor Grinberg
  -1 siblings, 0 replies; 6+ messages in thread
From: Igor Grinberg @ 2012-06-14 12:59 UTC (permalink / raw)
  To: Russ Dill
  Cc: linux-omap, linux-arm-kernel, Felipe Balbi, Keshava Munegowda,
	Partha Basak, Samuel Ortiz, mantesh, s-sapna1, Russ Dill

Hi Russ,

On 06/14/12 14:31, Russ Dill wrote:
> '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.
> 
> Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
> ---
>  drivers/mfd/omap-usb-host.c  |   45 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/host/ehci-omap.c |   18 ++++++++---------
>  2 files changed, 53 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index 7e96bb2..10984e2 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>
> @@ -502,6 +503,19 @@ static void omap_usbhs_init(struct device *dev)
>  	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);
> +	}

Hmmm.... I think this hunk should come before the spin_lock_irqsave() call,
as it might sleep with I2C GPIO extenders...

> +
>  	omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
>  	dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
>  
> @@ -580,10 +594,39 @@ static void omap_usbhs_init(struct device *dev)
>  			usbhs_omap_tll_init(dev, OMAP_TLL_CHANNEL_COUNT);
>  	}
>  
> +	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);
> +	}

Same here, after the the spin_unlock_irqrestore() call?

> +
>  	spin_unlock_irqrestore(&omap->lock, flags);
>  	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 +810,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 +862,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);
>  

-- 
Regards,
Igor.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.
@ 2012-06-14 12:59   ` Igor Grinberg
  0 siblings, 0 replies; 6+ messages in thread
From: Igor Grinberg @ 2012-06-14 12:59 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Russ,

On 06/14/12 14:31, Russ Dill wrote:
> '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.
> 
> Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
> ---
>  drivers/mfd/omap-usb-host.c  |   45 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/usb/host/ehci-omap.c |   18 ++++++++---------
>  2 files changed, 53 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
> index 7e96bb2..10984e2 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>
> @@ -502,6 +503,19 @@ static void omap_usbhs_init(struct device *dev)
>  	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);
> +	}

Hmmm.... I think this hunk should come before the spin_lock_irqsave() call,
as it might sleep with I2C GPIO extenders...

> +
>  	omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
>  	dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
>  
> @@ -580,10 +594,39 @@ static void omap_usbhs_init(struct device *dev)
>  			usbhs_omap_tll_init(dev, OMAP_TLL_CHANNEL_COUNT);
>  	}
>  
> +	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);
> +	}

Same here, after the the spin_unlock_irqrestore() call?

> +
>  	spin_unlock_irqrestore(&omap->lock, flags);
>  	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 +810,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 +862,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);
>  

-- 
Regards,
Igor.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.
  2012-06-14 12:59   ` Igor Grinberg
@ 2012-06-14 16:09     ` Russ Dill
  -1 siblings, 0 replies; 6+ messages in thread
From: Russ Dill @ 2012-06-14 16:09 UTC (permalink / raw)
  To: Igor Grinberg
  Cc: linux-omap, linux-arm-kernel, Felipe Balbi, Keshava Munegowda,
	Partha Basak, Samuel Ortiz, mantesh, s-sapna1

On Thu, Jun 14, 2012 at 5:59 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Hi Russ,
>
> On 06/14/12 14:31, Russ Dill wrote:
>> '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.
>>
>> Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
>> ---
>>  drivers/mfd/omap-usb-host.c  |   45 ++++++++++++++++++++++++++++++++++++++++++
>>  drivers/usb/host/ehci-omap.c |   18 ++++++++---------
>>  2 files changed, 53 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
>> index 7e96bb2..10984e2 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>
>> @@ -502,6 +503,19 @@ static void omap_usbhs_init(struct device *dev)
>>       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);
>> +     }
>
> Hmmm.... I think this hunk should come before the spin_lock_irqsave() call,
> as it might sleep with I2C GPIO extenders...
>
>> +
>>       omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
>>       dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
>>
>> @@ -580,10 +594,39 @@ static void omap_usbhs_init(struct device *dev)
>>                       usbhs_omap_tll_init(dev, OMAP_TLL_CHANNEL_COUNT);
>>       }
>>
>> +     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);
>> +     }
>
> Same here, after the the spin_unlock_irqrestore() call?

That's embarrassing, thanks, I'll resend.

>> +
>>       spin_unlock_irqrestore(&omap->lock, flags);
>>       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 +810,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 +862,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);
>>
>
> --
> Regards,
> Igor.
--
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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues.
@ 2012-06-14 16:09     ` Russ Dill
  0 siblings, 0 replies; 6+ messages in thread
From: Russ Dill @ 2012-06-14 16:09 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jun 14, 2012 at 5:59 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> Hi Russ,
>
> On 06/14/12 14:31, Russ Dill wrote:
>> '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.
>>
>> Signed-off-by: Russ Dill <Russ.Dill@gmail.com>
>> ---
>> ?drivers/mfd/omap-usb-host.c ?| ? 45 ++++++++++++++++++++++++++++++++++++++++++
>> ?drivers/usb/host/ehci-omap.c | ? 18 ++++++++---------
>> ?2 files changed, 53 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/mfd/omap-usb-host.c b/drivers/mfd/omap-usb-host.c
>> index 7e96bb2..10984e2 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>
>> @@ -502,6 +503,19 @@ static void omap_usbhs_init(struct device *dev)
>> ? ? ? 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);
>> + ? ? }
>
> Hmmm.... I think this hunk should come before the spin_lock_irqsave() call,
> as it might sleep with I2C GPIO extenders...
>
>> +
>> ? ? ? omap->usbhs_rev = usbhs_read(omap->uhh_base, OMAP_UHH_REVISION);
>> ? ? ? dev_dbg(dev, "OMAP UHH_REVISION 0x%x\n", omap->usbhs_rev);
>>
>> @@ -580,10 +594,39 @@ static void omap_usbhs_init(struct device *dev)
>> ? ? ? ? ? ? ? ? ? ? ? usbhs_omap_tll_init(dev, OMAP_TLL_CHANNEL_COUNT);
>> ? ? ? }
>>
>> + ? ? 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);
>> + ? ? }
>
> Same here, after the the spin_unlock_irqrestore() call?

That's embarrassing, thanks, I'll resend.

>> +
>> ? ? ? spin_unlock_irqrestore(&omap->lock, flags);
>> ? ? ? 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 +810,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 +862,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);
>>
>
> --
> Regards,
> Igor.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-06-14 16:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-06-14 11:31 [PATCH] ARM: OMAP3: USB: Fix the EHCI ULPI PHY reset fix issues Russ Dill
2012-06-14 11:31 ` Russ Dill
2012-06-14 12:59 ` Igor Grinberg
2012-06-14 12:59   ` Igor Grinberg
2012-06-14 16:09   ` Russ Dill
2012-06-14 16:09     ` Russ Dill

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.