From: Hans de Goede <hdegoede@redhat.com>
Date: Mon, 28 Nov 2022 09:28:49 +0000 (+0100)
Subject: power: supply: bq25890: Fix usb-notifier probe and remove races
X-Git-Tag: v6.6.7~3872^2~1
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=7e6fb67808ab5ceba73a6f45d0942e1e25ac56a7;p=platform%2Fkernel%2Flinux-starfive.git

power: supply: bq25890: Fix usb-notifier probe and remove races

There are 2 races surrounding the usb-notifier:

1. The notifier, and thus usb_work, may run before the bq->charger
   power_supply class device is registered. But usb_work may call
   power_supply_changed() which relies on the psy device being registered.

2. usb_work may be pending/running at remove() time, so it needs to be
   cancelled on remove after unregistering the usb-notifier.

Fix 1. by moving usb-notifier registration to after the power_supply
registration.

Fix 2. by adding a cancel_work_sync() call directly after the usb-notifier
unregistration.

Reviewed-by: Marek Vasut <marex@denx.de>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
---

diff --git a/drivers/power/supply/bq25890_charger.c b/drivers/power/supply/bq25890_charger.c
index 866c475..2d731ea 100644
--- a/drivers/power/supply/bq25890_charger.c
+++ b/drivers/power/supply/bq25890_charger.c
@@ -1391,40 +1391,34 @@ static int bq25890_probe(struct i2c_client *client)
 	if (ret)
 		return ret;
 
-	if (!IS_ERR_OR_NULL(bq->usb_phy)) {
-		INIT_WORK(&bq->usb_work, bq25890_usb_work);
-		bq->usb_nb.notifier_call = bq25890_usb_notifier;
-		usb_register_notifier(bq->usb_phy, &bq->usb_nb);
-	}
-
 	ret = bq25890_power_supply_init(bq);
-	if (ret < 0) {
-		dev_err(dev, "Failed to register power supply\n");
-		goto err_unregister_usb_notifier;
-	}
+	if (ret < 0)
+		return dev_err_probe(dev, ret, "registering power supply\n");
 
 	ret = devm_request_threaded_irq(dev, client->irq, NULL,
 					bq25890_irq_handler_thread,
 					IRQF_TRIGGER_FALLING | IRQF_ONESHOT,
 					BQ25890_IRQ_PIN, bq);
 	if (ret)
-		goto err_unregister_usb_notifier;
-
-	return 0;
+		return ret;
 
-err_unregister_usb_notifier:
-	if (!IS_ERR_OR_NULL(bq->usb_phy))
-		usb_unregister_notifier(bq->usb_phy, &bq->usb_nb);
+	if (!IS_ERR_OR_NULL(bq->usb_phy)) {
+		INIT_WORK(&bq->usb_work, bq25890_usb_work);
+		bq->usb_nb.notifier_call = bq25890_usb_notifier;
+		usb_register_notifier(bq->usb_phy, &bq->usb_nb);
+	}
 
-	return ret;
+	return 0;
 }
 
 static void bq25890_remove(struct i2c_client *client)
 {
 	struct bq25890_device *bq = i2c_get_clientdata(client);
 
-	if (!IS_ERR_OR_NULL(bq->usb_phy))
+	if (!IS_ERR_OR_NULL(bq->usb_phy)) {
 		usb_unregister_notifier(bq->usb_phy, &bq->usb_nb);
+		cancel_work_sync(&bq->usb_work);
+	}
 
 	if (!bq->skip_reset) {
 		/* reset all registers to default values */