pda_power: ac_draw used before set

PATCH In review Paul Parsons 2 participants 4 messages Sep 19, 2012 – Sep 21, 2012

Share & tools

Paul Parsons

author

2012-09-19 17:36 UTC #0 lore raw

PATCH 1/1 pda_power: ac_draw used before set

When I reboot my iPAQ hx4700 in its cradle, the battery will not begin to charge even though the AC supply is connected. Charging will start only after the PDA power driver is tickled by some other power event, such as reseating the iPAQ in its cradle or connecting the USB cable.

The problem lies in pda_power_probe(), where ac_draw is used by the call to update_charger() before being set by the call to regulator_get().

Moving the regulator_get() call to before the update_charger() call fixes the problem.

Signed-off-by Paul Parsons <[email protected]>

Cc Philipp Zabel <[email protected]>

---
 drivers/power/pda_power.c |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

drivers/power/pda_power.c +9 -7

index 7312f26..d3be834d 100644
@@ -281,6 +281,14 @@ static int pda_power_probe(struct platform_device *pdev)
 			goto init_failed;
 	}
 
+	ac_draw = regulator_get(dev, "ac_draw");
+	if (IS_ERR(ac_draw)) {
+		dev_dbg(dev, "couldn't get ac_draw regulator\n");
+		ret = PTR_ERR(ac_draw);
+		ac_draw = NULL;
+		goto ac_draw_failed;
+	}
+
 	update_status();
 	update_charger();

@@ -309,13 +317,6 @@ static int pda_power_probe(struct platform_device *pdev)
 		pda_psy_usb.num_supplicants = pdata->num_supplicants;
 	}
 
-	ac_draw = regulator_get(dev, "ac_draw");
-	if (IS_ERR(ac_draw)) {
-		dev_dbg(dev, "couldn't get ac_draw regulator\n");
-		ac_draw = NULL;
-		ret = PTR_ERR(ac_draw);
-	}
-
 #ifdef CONFIG_USB_OTG_UTILS
 	transceiver = usb_get_phy(USB_PHY_TYPE_USB2);
 	if (!IS_ERR_OR_NULL(transceiver)) {

@@ -415,6 +416,7 @@ ac_supply_failed:
 		regulator_put(ac_draw);
 		ac_draw = NULL;
 	}
+ac_draw_failed:
 	if (pdata->exit)
 		pdata->exit(dev);
 init_failed:

Signature · 1 line

1.7.8.6

Anton Vorontsov

2012-09-20 21:35 UTC #1 lore raw

Quoted 8 lines · Paul Parsons

When I reboot my iPAQ hx4700 in its cradle, the battery will not begin to charge even though the AC supply is connected. Charging will start only after the PDA power driver is tickled by some other power event, such as reseating the iPAQ in its cradle or connecting the USB cable.
The problem lies in pda_power_probe(), where ac_draw is used by the call to update_charger() before being set by the call to regulator_get().

Quoted 6 lines

Moving the regulator_get() call to before the update_charger() call fixes the problem.
Signed-off-by Paul Parsons <[email protected]>
Cc Philipp Zabel <[email protected]>
---

Great to see that someone is still using hx4700. :-)

Thanks for the patch, it is merged now!

Cheers,
Anton.

p.s.
The patch was mangled (word wrapped), but I fixed it up. Just fyi...

Paul Parsons

author

2012-09-21 00:22 UTC #2 lore raw

Hello Anton,

Quoted 12 lines · Anton Vorontsov

Quoted 11 lines · Paul Parsons
When I reboot my iPAQ hx4700 in its cradle, the battery will not begin to charge even though the AC supply is connected. Charging will start only after the PDA power driver is tickled by some other power event, such as reseating the iPAQ in its cradle or connecting the USB cable.
The problem lies in pda_power_probe(), where ac_draw is used by the call to update_charger() before being set by the call to regulator_get().
Moving the regulator_get() call to before the update_charger() call fixes the problem.

Quoted 7 lines

Quoted 3 lines
Signed-off-by Paul Parsons <[email protected]>
Cc Philipp Zabel <[email protected]>
---
Great to see that someone is still using hx4700. :-)
Thanks for the patch, it is merged now!

I am having second thoughts about the goto ac_draw_failed I added. This will cause pda_power_probe() to fail if regulator_get() fails. However the way ac_draw is used - always after a check for NULL - suggests that a failed call to regulator_get() was not fatal.

I am happy to submit another patch to remove the goto and label.

Regards,
Paul

Anton Vorontsov

2012-09-21 00:31 UTC #3 lore raw

Quoted 17 lines · Paul Parsons

Quoted 17 lines
Quoted 8 lines
When I reboot my iPAQ hx4700 in its cradle, the battery will not begin to charge even though the AC supply is connected. Charging will start only after the PDA power driver is tickled by some other power event, such as reseating the iPAQ in its cradle or connecting the USB cable.
The problem lies in pda_power_probe(), where ac_draw is used by the call to update_charger() before being set by the call to regulator_get().
Quoted 6 lines
Moving the regulator_get() call to before the update_charger() call fixes the problem.
Signed-off-by Paul Parsons <[email protected]>
Cc Philipp Zabel <[email protected]>
---
Great to see that someone is still using hx4700. :-)

Quoted 8 lines

Quoted 1 line
Thanks for the patch, it is merged now!
I am having second thoughts about the goto ac_draw_failed I added. This will cause pda_power_probe() to fail if regulator_get() fails. However the way ac_draw is used - always after a check for NULL - suggests that a failed call to regulator_get() was not fatal.
I am happy to submit another patch to remove the goto and label.

Sure thing, just send another patch on top.

Thanks!
Anton.