* Liam Breck [off-list ref] [170203 16:25]:

On Fri, Feb 3, 2017 at 1:17 PM, Tony Lindgren [off-list ref] wrote:

quoted

 static int bq24190_remove(struct i2c_client *client)
 {
        struct bq24190_dev_info *bdi = i2c_get_clientdata(client);
+       int error;

-       pm_runtime_get_sync(bdi->dev);
-       bq24190_register_reset(bdi);
-       pm_runtime_put_sync(bdi->dev);
+       error = pm_runtime_get_sync(bdi->dev);
+       if (error < 0) {
+               dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", error);
+               pm_runtime_put_noidle(bdi->dev);
+       }

+       bq24190_register_reset(bdi);
        bq24190_sysfs_remove_group(bdi);
        power_supply_unregister(bdi->battery);
        power_supply_unregister(bdi->charger);
+       pm_runtime_put_sync(bdi->dev);
+       pm_runtime_dont_use_autosuspend(bdi->dev);
        pm_runtime_disable(bdi->dev);

I think you addressed this, but should the above be

if (!error)
  pm_runtime_put_sync(bdi->dev);

Hmm yeah.. But we need to check for if (error >= 0), also in the other
places see below.

Regards,

Tony

8< --------------------

diff --git a/drivers/power/supply/bq24190_charger.c b/drivers/power/supply/bq24190_charger.c
--- a/drivers/power/supply/bq24190_charger.c
+++ b/drivers/power/supply/bq24190_charger.c

@@ -1487,7 +1487,8 @@ static int bq24190_remove(struct i2c_client *client)
 	bq24190_sysfs_remove_group(bdi);
 	power_supply_unregister(bdi->battery);
 	power_supply_unregister(bdi->charger);
-	pm_runtime_put_sync(bdi->dev);
+	if (error >= 0)
+		pm_runtime_put_sync(bdi->dev);
 	pm_runtime_dont_use_autosuspend(bdi->dev);
 	pm_runtime_disable(bdi->dev);
 

@@ -1541,7 +1542,7 @@ static int bq24190_pm_suspend(struct device *dev)
 
 	bq24190_register_reset(bdi);
 
-	if (!error) {
+	if (error >= 0) {
 		pm_runtime_mark_last_busy(bdi->dev);
 		pm_runtime_put_autosuspend(bdi->dev);
 	}

@@ -1568,7 +1569,7 @@ static int bq24190_pm_resume(struct device *dev)
 	bq24190_set_mode_host(bdi);
 	bq24190_read(bdi, BQ24190_REG_SS, &bdi->ss_reg);
 
-	if (!error) {
+	if (error >= 0) {
 		pm_runtime_mark_last_busy(bdi->dev);
 		pm_runtime_put_autosuspend(bdi->dev);
 	}

-- 
2.11.0