linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 02/22] lp8727_charger: cleanup _probe() and _remove()
@ 2012-08-31  9:23 Kim, Milo
  0 siblings, 0 replies; only message in thread
From: Kim, Milo @ 2012-08-31  9:23 UTC (permalink / raw)
  To: Anton Vorontsov; +Cc: David Woodhouse, linux-kernel

 If the lp8727_register_psy() gets failed, registered interrupt handler
 should be freed, but this is not complete solution.
 It has still problem.
 Assume that the IRQ occurs while unregistering power supply devices.
 Then the ISR will access to freed IRQ.

 From Anton's opinion, it can be resolved if re-ordering the call sequence.

 Register power supplies first, then create interrupt handler.
 Then no need to free the IRQ in _probe().
 Additionally goto statements can be removed because those can be replaced with
 return statements.

 The _remove() should be changed the sequence - in reverse order of _probe()

Signed-off-by: Milo(Woogyom) Kim <milo.kim@ti.com>
---
 drivers/power/lp8727_charger.c |   20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/drivers/power/lp8727_charger.c b/drivers/power/lp8727_charger.c
index 0d3cb1d..b2df114 100644
--- a/drivers/power/lp8727_charger.c
+++ b/drivers/power/lp8727_charger.c
@@ -442,35 +442,33 @@ static int lp8727_probe(struct i2c_client *cl, const struct i2c_device_id *id)
 	ret = lp8727_init_device(pchg);
 	if (ret) {
 		dev_err(pchg->dev, "i2c communication err: %d", ret);
-		goto error;
+		return ret;
 	}
 
-	ret = lp8727_intr_config(pchg);
+	ret = lp8727_register_psy(pchg);
 	if (ret) {
-		dev_err(pchg->dev, "irq handler err: %d", ret);
-		goto error;
+		dev_err(pchg->dev, "power supplies register err: %d", ret);
+		return ret;
 	}
 
-	ret = lp8727_register_psy(pchg);
+	ret = lp8727_intr_config(pchg);
 	if (ret) {
-		dev_err(pchg->dev, "power supplies register err: %d", ret);
-		goto error;
+		dev_err(pchg->dev, "irq handler err: %d", ret);
+		lp8727_unregister_psy(pchg);
+		return ret;
 	}
 
 	return 0;
-
-error:
-	return ret;
 }
 
 static int __devexit lp8727_remove(struct i2c_client *cl)
 {
 	struct lp8727_chg *pchg = i2c_get_clientdata(cl);
 
-	lp8727_unregister_psy(pchg);
 	free_irq(pchg->client->irq, pchg);
 	flush_workqueue(pchg->irqthread);
 	destroy_workqueue(pchg->irqthread);
+	lp8727_unregister_psy(pchg);
 	return 0;
 }
 
-- 
1.7.9.5


Best Regards,
Milo



^ permalink raw reply related	[flat|nested] only message in thread

only message in thread, other threads:[~2012-08-31  9:23 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-31  9:23 [PATCH 02/22] lp8727_charger: cleanup _probe() and _remove() Kim, Milo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).