mfd: Fix usbhs_enable error handling

PATCH In review Axel Lin 4 participants 5 messages Mar 29, 2011 – Apr 11, 2011

Subsystems (2)

Multifunction devices (MFD) OMAP2+ support

lore

Share & tools

lore

Axel Lin

author

2011-03-29 10:08 UTC #0 lore raw

PATCH 1/1 mfd: Fix usbhs_enable error handling

In the case of missing platform_data we do not hold a spin_lock, thus we should not call spin_unlock_irqrestore in the error path.

Also simplify the error handling by separating the successful path from error path. I think this change improves readability.

Signed-off-by Axel Lin <[email protected]>

---
 drivers/mfd/omap-usb-host.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

drivers/mfd/omap-usb-host.c +3 -5

index cb01209..4157d76 100644
@@ -700,8 +700,7 @@ static int usbhs_enable(struct device *dev)
 	dev_dbg(dev, "starting TI HSUSB Controller\n");
 	if (!pdata) {
 		dev_dbg(dev, "missing platform_data\n");
-		ret =  -ENODEV;
-		goto end_enable;
+		return  -ENODEV;
 	}
 
 	spin_lock_irqsave(&omap->lock, flags);

@@ -915,7 +914,8 @@ static int usbhs_enable(struct device *dev)
 
 end_count:
 	omap->count++;
-	goto end_enable;
+	spin_unlock_irqrestore(&omap->lock, flags);
+	return 0;
 
 err_tll:
 	if (pdata->ehci_data->phy_reset) {

@@ -931,8 +931,6 @@ err_tll:
 	clk_disable(omap->usbhost_fs_fck);
 	clk_disable(omap->usbhost_hs_fck);
 	clk_disable(omap->usbhost_ick);
-
-end_enable:
 	spin_unlock_irqrestore(&omap->lock, flags);
 	return ret;
 }

Signature · 1 line

1.7.1

Keshava Munegowda

Acked-by on v1 1/1

2011-03-29 10:32 UTC #1 lore raw

Quoted 52 lines

-----Original Message-----
From: Axel Lin [mailto:[email protected]]
Sent: Tuesday, March 29, 2011 3:38 PM
To: [email protected]
Cc: Keshava Munegowda; Samuel Ortiz
Subject: [PATCH] mfd: Fix usbhs_enable error handling
In the case of missing platform_data we do not hold a spin_lock, thus we should not call spin_unlock_irqrestore in the error path.
Also simplify the error handling by separating the successful path from error path. I think this change improves readability.
Signed-off-by Axel Lin <[email protected]>
---
 drivers/mfd/omap-usb-host.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
drivers/mfd/omap-usb-host.c +3 -6
index cb01209..4157d76 100644
@@ -700,8 +700,7 @@ static int usbhs_enable(struct device *dev)
 	dev_dbg(dev, "starting TI HSUSB Controller\n");
 	if (!pdata) {
 		dev_dbg(dev, "missing platform_data\n");
-		ret =  -ENODEV;
-		goto end_enable;
+		return  -ENODEV;
 	}
 
 	spin_lock_irqsave(&omap->lock, flags);
@@ -915,7 +914,8 @@ static int usbhs_enable(struct device *dev)
 
 end_count:
 	omap->count++;
-	goto end_enable;
+	spin_unlock_irqrestore(&omap->lock, flags);
+	return 0;
 
 err_tll:
 	if (pdata->ehci_data->phy_reset) {
@@ -931,8 +931,6 @@ err_tll:
 	clk_disable(omap->usbhost_fs_fck);
 	clk_disable(omap->usbhost_hs_fck);
 	clk_disable(omap->usbhost_ick);
-
-end_enable:
 	spin_unlock_irqrestore(&omap->lock, flags);
 	return ret;
 }
--

Quoted 3 lines

1.7.1

Thanks Axel!

Acked-by Keshava Munegowda < [email protected]>

Keshava Munegowda

Acked-by on v1 1/1

2011-03-29 10:35 UTC #2 lore raw

In reply to Axel Lin

Quoted 52 lines

-----Original Message-----
From: Axel Lin [mailto:[email protected]]
Sent: Tuesday, March 29, 2011 3:38 PM
To: [email protected]
Cc: Keshava Munegowda; Samuel Ortiz
Subject: [PATCH] mfd: Fix usbhs_enable error handling
In the case of missing platform_data we do not hold a spin_lock, thus we should not call spin_unlock_irqrestore in the error path.
Also simplify the error handling by separating the successful path from error path. I think this change improves readability.
Signed-off-by Axel Lin <[email protected]>
---
 drivers/mfd/omap-usb-host.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
drivers/mfd/omap-usb-host.c +3 -6
index cb01209..4157d76 100644
@@ -700,8 +700,7 @@ static int usbhs_enable(struct device *dev)
 	dev_dbg(dev, "starting TI HSUSB Controller\n");
 	if (!pdata) {
 		dev_dbg(dev, "missing platform_data\n");
-		ret =  -ENODEV;
-		goto end_enable;
+		return  -ENODEV;
 	}
 
 	spin_lock_irqsave(&omap->lock, flags);
@@ -915,7 +914,8 @@ static int usbhs_enable(struct device *dev)
 
 end_count:
 	omap->count++;
-	goto end_enable;
+	spin_unlock_irqrestore(&omap->lock, flags);
+	return 0;
 
 err_tll:
 	if (pdata->ehci_data->phy_reset) {
@@ -931,8 +931,6 @@ err_tll:
 	clk_disable(omap->usbhost_fs_fck);
 	clk_disable(omap->usbhost_hs_fck);
 	clk_disable(omap->usbhost_ick);
-
-end_enable:
 	spin_unlock_irqrestore(&omap->lock, flags);
 	return ret;
 }
--

Quoted 1 line

1.7.1

Thanks Axel!

Acked-by Keshava Munegowda < [email protected]>

Quoted 1 line

Felipe Balbi

Acked-by

2011-03-29 11:06 UTC #3 lore raw

Quoted 55 lines · Keshava Munegowda

Quoted 52 lines
-----Original Message-----
From: Axel Lin [mailto:[email protected]]
Sent: Tuesday, March 29, 2011 3:38 PM
To: [email protected]
Cc: Keshava Munegowda; Samuel Ortiz
Subject: [PATCH] mfd: Fix usbhs_enable error handling
In the case of missing platform_data we do not hold a spin_lock, thus we should not call spin_unlock_irqrestore in the error path.
Also simplify the error handling by separating the successful path from error path. I think this change improves readability.
Signed-off-by Axel Lin <[email protected]>
---
 drivers/mfd/omap-usb-host.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)
drivers/mfd/omap-usb-host.c +3 -6
index cb01209..4157d76 100644
@@ -700,8 +700,7 @@ static int usbhs_enable(struct device *dev)
 	dev_dbg(dev, "starting TI HSUSB Controller\n");
 	if (!pdata) {
 		dev_dbg(dev, "missing platform_data\n");
-		ret =  -ENODEV;
-		goto end_enable;
+		return  -ENODEV;
 	}
 
 	spin_lock_irqsave(&omap->lock, flags);
@@ -915,7 +914,8 @@ static int usbhs_enable(struct device *dev)
 
 end_count:
 	omap->count++;
-	goto end_enable;
+	spin_unlock_irqrestore(&omap->lock, flags);
+	return 0;
 
 err_tll:
 	if (pdata->ehci_data->phy_reset) {
@@ -931,8 +931,6 @@ err_tll:
 	clk_disable(omap->usbhost_fs_fck);
 	clk_disable(omap->usbhost_hs_fck);
 	clk_disable(omap->usbhost_ick);
-
-end_enable:
 	spin_unlock_irqrestore(&omap->lock, flags);
 	return ret;
 }
--
Quoted 1 line
1.7.1
Thanks Axel!

Quoted 1 line

Acked-by Keshava Munegowda < [email protected]>

Samuel, Greg is out for the week, could you take this one through your tree instead of waiting for a week until we get this fix merged. If you need my Ack, here it goes:

Acked-by Felipe Balbi <[email protected]>

Signature · 1 line

balbi

Samuel Ortiz

2011-04-11 19:27 UTC #4 lore raw

In reply to Axel Lin

Hi Axel,

Quoted 5 lines · Axel Lin

In the case of missing platform_data we do not hold a spin_lock, thus we should not call spin_unlock_irqrestore in the error path.
Also simplify the error handling by separating the successful path from error path. I think this change improves readability.

Patch applied to my for-next branch, thanks.

Cheers,
Samuel.

Signature · 2 lines

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