As the potential failure of the wm8350_register_irq(),
it should be better to check it and return error if fails.
Also, use 'free_' in order to avoid same code.
Fixes: 14431aa0c5a4 ("power_supply: Add support for WM8350 PMU")
Signed-off-by: Jiasheng Jiang <[email protected]>
---
drivers/power/supply/wm8350_power.c | 96 ++++++++++++++++++++++++-----
1 file changed, 82 insertions(+), 14 deletions(-)
diff --git a/drivers/power/supply/wm8350_power.c b/drivers/power/supply/wm8350_power.c
index e05cee457471..e1d39e96e656 100644
--- a/drivers/power/supply/wm8350_power.c
+++ b/drivers/power/supply/wm8350_power.c
@@ -408,44 +408,112 @@ static const struct power_supply_desc wm8350_usb_desc = {
* Initialisation
*********************************************************************/
-static void wm8350_init_charger(struct wm8350 *wm8350)
+static int wm8350_init_charger(struct wm8350 *wm8350)
{
+ int ret;
+
/* register our interest in charger events */
- wm8350_register_irq(wm8350, WM8350_IRQ_CHG_BAT_HOT,
+ ret = wm8350_register_irq(wm8350, WM8350_IRQ_CHG_BAT_HOT,
wm8350_charger_handler, 0, "Battery hot", wm8350);
- wm8350_register_irq(wm8350, WM8350_IRQ_CHG_BAT_COLD,
+ if (ret)
+ goto err;
+
+ ret = wm8350_register_irq(wm8350, WM8350_IRQ_CHG_BAT_COLD,
wm8350_charger_handler, 0, "Battery cold", wm8350);
- wm8350_register_irq(wm8350, WM8350_IRQ_CHG_BAT_FAIL,
+ if (ret)
+ goto free_CHG_BAT_HOT;
+
+ ret = wm8350_register_irq(wm8350, WM8350_IRQ_CHG_BAT_FAIL,
wm8350_charger_handler, 0, "Battery fail", wm8350);
- wm8350_register_irq(wm8350, WM8350_IRQ_CHG_TO,
+ if (ret)
+ goto free_CHG_BAT_COLD;
+
+ ret = wm8350_register_irq(wm8350, WM8350_IRQ_CHG_TO,
wm8350_charger_handler, 0,
"Charger timeout", wm8350);
- wm8350_register_irq(wm8350, WM8350_IRQ_CHG_END,
+ if (ret)
+ goto free_CHG_BAT_FAIL;
+
+ ret = wm8350_register_irq(wm8350, WM8350_IRQ_CHG_END,
wm8350_charger_handler, 0,
"Charge end", wm8350);
- wm8350_register_irq(wm8350, WM8350_IRQ_CHG_START,
+ if (ret)
+ goto free_CHG_TO;
+
+ ret = wm8350_register_irq(wm8350, WM8350_IRQ_CHG_START,
wm8350_charger_handler, 0,
"Charge start", wm8350);
- wm8350_register_irq(wm8350, WM8350_IRQ_CHG_FAST_RDY,
+ if (ret)
+ goto free_CHG_END;
+
+ ret = wm8350_register_irq(wm8350, WM8350_IRQ_CHG_FAST_RDY,
wm8350_charger_handler, 0,
"Fast charge ready", wm8350);
- wm8350_register_irq(wm8350, WM8350_IRQ_CHG_VBATT_LT_3P9,
+ if (ret)
+ goto free_CHG_START;
+
+ ret = wm8350_register_irq(wm8350, WM8350_IRQ_CHG_VBATT_LT_3P9,
wm8350_charger_handler, 0,
"Battery <3.9V", wm8350);
- wm8350_register_irq(wm8350, WM8350_IRQ_CHG_VBATT_LT_3P1,
+ if (ret)
+ goto free_CHG_FAST_RDY;
+
+ ret = wm8350_register_irq(wm8350, WM8350_IRQ_CHG_VBATT_LT_3P1,
wm8350_charger_handler, 0,
"Battery <3.1V", wm8350);
- wm8350_register_irq(wm8350, WM8350_IRQ_CHG_VBATT_LT_2P85,
+ if (ret)
+ goto free_CHG_VBATT_LT_3P9;
+
+ ret = wm8350_register_irq(wm8350, WM8350_IRQ_CHG_VBATT_LT_2P85,
wm8350_charger_handler, 0,
"Battery <2.85V", wm8350);
+ if (ret)
+ goto free_CHG_VBATT_LT_3P1;
/* and supply change events */
- wm8350_register_irq(wm8350, WM8350_IRQ_EXT_USB_FB,
+ ret = wm8350_register_irq(wm8350, WM8350_IRQ_EXT_USB_FB,
wm8350_charger_handler, 0, "USB", wm8350);
- wm8350_register_irq(wm8350, WM8350_IRQ_EXT_WALL_FB,
+ if (ret)
+ goto free_CHG_VBATT_LT_2P85;
+
+ ret = wm8350_register_irq(wm8350, WM8350_IRQ_EXT_WALL_FB,
wm8350_charger_handler, 0, "Wall", wm8350);
- wm8350_register_irq(wm8350, WM8350_IRQ_EXT_BAT_FB,
+ if (ret)
+ goto free_EXT_USB_FB;
+
+ ret = wm8350_register_irq(wm8350, WM8350_IRQ_EXT_BAT_FB,
wm8350_charger_handler, 0, "Battery", wm8350);
+ if (ret)
+ goto free_EXT_WALL_FB;
+
+ return 0;
+
+free_EXT_WALL_FB:
+ wm8350_free_irq(wm8350, WM8350_IRQ_EXT_WALL_FB, wm8350);
+free_EXT_USB_FB:
+ wm8350_free_irq(wm8350, WM8350_IRQ_EXT_USB_FB, wm8350);
+free_CHG_VBATT_LT_2P85:
+ wm8350_free_irq(wm8350, WM8350_IRQ_CHG_VBATT_LT_2P85, wm8350);
+free_CHG_VBATT_LT_3P1:
+ wm8350_free_irq(wm8350, WM8350_IRQ_CHG_VBATT_LT_3P1, wm8350);
+free_CHG_VBATT_LT_3P9:
+ wm8350_free_irq(wm8350, WM8350_IRQ_CHG_VBATT_LT_3P9, wm8350);
+free_CHG_FAST_RDY:
+ wm8350_free_irq(wm8350, WM8350_IRQ_CHG_FAST_RDY, wm8350);
+free_CHG_START:
+ wm8350_free_irq(wm8350, WM8350_IRQ_CHG_START, wm8350);
+free_CHG_END:
+ wm8350_free_irq(wm8350, WM8350_IRQ_CHG_END, wm8350);
+free_CHG_TO:
+ wm8350_free_irq(wm8350, WM8350_IRQ_CHG_TO, wm8350);
+free_CHG_BAT_FAIL:
+ wm8350_free_irq(wm8350, WM8350_IRQ_CHG_BAT_FAIL, wm8350);
+free_CHG_BAT_COLD:
+ wm8350_free_irq(wm8350, WM8350_IRQ_CHG_BAT_COLD, wm8350);
+free_CHG_BAT_HOT:
+ wm8350_free_irq(wm8350, WM8350_IRQ_CHG_BAT_HOT, wm8350);
+err:
+ return ret;
}
static void free_charger_irq(struct wm8350 *wm8350)
--
2.25.1
On Thu, Mar 03, 2022 at 05:33:39PM +0800, Jiasheng Jiang wrote:
> As the potential failure of the wm8350_register_irq(),
> it should be better to check it and return error if fails.
> Also, use 'free_' in order to avoid same code.
>
> Fixes: 14431aa0c5a4 ("power_supply: Add support for WM8350 PMU")
> Signed-off-by: Jiasheng Jiang <[email protected]>
> ---
> + ret = wm8350_register_irq(wm8350, WM8350_IRQ_CHG_BAT_COLD,
> wm8350_charger_handler, 0, "Battery cold", wm8350);
> - wm8350_register_irq(wm8350, WM8350_IRQ_CHG_BAT_FAIL,
> + if (ret)
> + goto free_CHG_BAT_HOT;
Probably be nicer to use non-caps for the labels, otherwise looks
good to me.
Thanks,
Charles
On Thu, Mar 03, 2022 at 09:41:03PM +0800, Charles Keepax wrote:
>> + if (ret)
>> + goto free_CHG_BAT_HOT;
>
> Probably be nicer to use non-caps for the labels, otherwise looks
> good to me.
Thanks, I have submitted a v2 to fix them.
Jiang