| Message ID | 20170131003632.GB7403@atomide.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Mon, Jan 30, 2017 at 4:36 PM, Tony Lindgren <tony@atomide.com> wrote: > From: Tony Lindgren <tony@atomide.com> > Date: Mon, 30 Jan 2017 07:10:31 -0800 > Subject: [PATCH] power: bq24190_charger: Use PM runtime autosuspend > > We can get quite a few interrupts when the battery is trickle charging. > Let's enable PM runtime autosuspend to avoid constantly toggling device > driver PM runtime state. > > Let's use a 600 ms timeout as that's how long the USB chager detection > might take. > > Cc: Liam Breck <kernel@networkimprov.net> > Acked-by: Mark Greer <mgreer@animalcreek.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > drivers/power/supply/bq24190_charger.c | 153 ++++++++++++++++++++++++--------- > 1 file changed, 111 insertions(+), 42 deletions(-) > > 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 > @@ -420,6 +420,7 @@ static ssize_t bq24190_sysfs_show(struct device *dev, > struct power_supply *psy = dev_get_drvdata(dev); > struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy); > struct bq24190_sysfs_field_info *info; > + ssize_t count; > int ret; > u8 v; > > @@ -427,11 +428,20 @@ static ssize_t bq24190_sysfs_show(struct device *dev, > if (!info) > return -EINVAL; > > + ret = pm_runtime_get_sync(bdi->dev); > + if (ret < 0) > + return ret; Let's not stop here if get_sync fails. > ret = bq24190_read_mask(bdi, info->reg, info->mask, info->shift, &v); > if (ret) > - return ret; > + count = ret; > + else > + count = scnprintf(buf, PAGE_SIZE, "%hhx\n", v); > > - return scnprintf(buf, PAGE_SIZE, "%hhx\n", v); > + pm_runtime_mark_last_busy(bdi->dev); > + pm_runtime_put_autosuspend(bdi->dev); > + > + return count; > } > > static ssize_t bq24190_sysfs_store(struct device *dev, > @@ -451,9 +461,16 @@ static ssize_t bq24190_sysfs_store(struct device *dev, > if (ret < 0) > return ret; > > + ret = pm_runtime_get_sync(bdi->dev); > + if (ret < 0) > + return ret; Nor here 2. > ret = bq24190_write_mask(bdi, info->reg, info->mask, info->shift, v); > if (ret) > - return ret; > + count = ret; > + > + pm_runtime_mark_last_busy(bdi->dev); > + pm_runtime_put_autosuspend(bdi->dev); > > return count; > } > @@ -795,7 +812,9 @@ static int bq24190_charger_get_property(struct power_supply *psy, > > dev_dbg(bdi->dev, "prop: %d\n", psp); > > - pm_runtime_get_sync(bdi->dev); > + ret = pm_runtime_get_sync(bdi->dev); > + if (ret < 0) > + return ret; Nor here 3. > switch (psp) { > case POWER_SUPPLY_PROP_CHARGE_TYPE: > @@ -835,7 +854,9 @@ static int bq24190_charger_get_property(struct power_supply *psy, > ret = -ENODATA; > } > > - pm_runtime_put_sync(bdi->dev); > + pm_runtime_mark_last_busy(bdi->dev); > + pm_runtime_put_autosuspend(bdi->dev); > + > return ret; > } > > @@ -848,7 +869,9 @@ static int bq24190_charger_set_property(struct power_supply *psy, > > dev_dbg(bdi->dev, "prop: %d\n", psp); > > - pm_runtime_get_sync(bdi->dev); > + ret = pm_runtime_get_sync(bdi->dev); > + if (ret < 0) > + return ret; Nor here 4. > switch (psp) { > case POWER_SUPPLY_PROP_CHARGE_TYPE: > @@ -864,7 +887,9 @@ static int bq24190_charger_set_property(struct power_supply *psy, > ret = -EINVAL; > } > > - pm_runtime_put_sync(bdi->dev); > + pm_runtime_mark_last_busy(bdi->dev); > + pm_runtime_put_autosuspend(bdi->dev); > + > return ret; > } > > @@ -1065,7 +1090,9 @@ static int bq24190_battery_get_property(struct power_supply *psy, > > dev_dbg(bdi->dev, "prop: %d\n", psp); > > - pm_runtime_get_sync(bdi->dev); > + ret = pm_runtime_get_sync(bdi->dev); > + if (ret < 0) > + return ret; Nor here 5. > switch (psp) { > case POWER_SUPPLY_PROP_STATUS: > @@ -1093,7 +1120,9 @@ static int bq24190_battery_get_property(struct power_supply *psy, > ret = -ENODATA; > } > > - pm_runtime_put_sync(bdi->dev); > + pm_runtime_mark_last_busy(bdi->dev); > + pm_runtime_put_autosuspend(bdi->dev); > + > return ret; > } > > @@ -1106,7 +1135,9 @@ static int bq24190_battery_set_property(struct power_supply *psy, > > dev_dbg(bdi->dev, "prop: %d\n", psp); > > - pm_runtime_get_sync(bdi->dev); > + ret = pm_runtime_get_sync(bdi->dev); > + if (ret < 0) > + return ret; Nor here 6. > switch (psp) { > case POWER_SUPPLY_PROP_ONLINE: > @@ -1119,7 +1150,9 @@ static int bq24190_battery_set_property(struct power_supply *psy, > ret = -EINVAL; > } > > - pm_runtime_put_sync(bdi->dev); > + pm_runtime_mark_last_busy(bdi->dev); > + pm_runtime_put_autosuspend(bdi->dev); > + > return ret; > } > > @@ -1234,11 +1267,15 @@ static void bq24190_check_status(struct bq24190_dev_info *bdi) > static irqreturn_t bq24190_irq_handler_thread(int irq, void *data) > { > struct bq24190_dev_info *bdi = data; > + int ret; > > bdi->irq_event = true; > - pm_runtime_get_sync(bdi->dev); > + ret = pm_runtime_get_sync(bdi->dev); > + if (ret < 0) > + return ret; Nor here 7. > bq24190_check_status(bdi); > - pm_runtime_put_sync(bdi->dev); > + pm_runtime_mark_last_busy(bdi->dev); > + pm_runtime_put_autosuspend(bdi->dev); > bdi->irq_event = false; > > return IRQ_HANDLED; > @@ -1249,33 +1286,26 @@ static int bq24190_hw_init(struct bq24190_dev_info *bdi) > u8 v; > int ret; > > - pm_runtime_get_sync(bdi->dev); > - > /* First check that the device really is what its supposed to be */ > ret = bq24190_read_mask(bdi, BQ24190_REG_VPRS, > BQ24190_REG_VPRS_PN_MASK, > BQ24190_REG_VPRS_PN_SHIFT, > &v); > if (ret < 0) > - goto out; > + return ret; > > - if (v != bdi->model) { > - ret = -ENODEV; > - goto out; > - } > + if (v != bdi->model) > + return -ENODEV; > > ret = bq24190_register_reset(bdi); > if (ret < 0) > - goto out; > + return ret; > > ret = bq24190_set_mode_host(bdi); > if (ret < 0) > - goto out; > + return ret; > > - ret = bq24190_read(bdi, BQ24190_REG_SS, &bdi->ss_reg); > -out: > - pm_runtime_put_sync(bdi->dev); > - return ret; > + return bq24190_read(bdi, BQ24190_REG_SS, &bdi->ss_reg); > } > > #ifdef CONFIG_OF > @@ -1364,12 +1394,16 @@ static int bq24190_probe(struct i2c_client *client, > } > > pm_runtime_enable(dev); > - pm_runtime_resume(dev); > + pm_runtime_use_autosuspend(dev); > + pm_runtime_set_autosuspend_delay(dev, 600); > + ret = pm_runtime_get_sync(dev); > + if (ret < 0) > + goto out1; Nor here 8. > ret = bq24190_hw_init(bdi); > if (ret < 0) { > dev_err(dev, "Hardware init failed\n"); > - goto out1; > + goto out2; > } > > charger_cfg.drv_data = bdi; > @@ -1380,7 +1414,7 @@ static int bq24190_probe(struct i2c_client *client, > if (IS_ERR(bdi->charger)) { > dev_err(dev, "Can't register charger\n"); > ret = PTR_ERR(bdi->charger); > - goto out1; > + goto out2; > } > > battery_cfg.drv_data = bdi; > @@ -1389,13 +1423,13 @@ static int bq24190_probe(struct i2c_client *client, > if (IS_ERR(bdi->battery)) { > dev_err(dev, "Can't register battery\n"); > ret = PTR_ERR(bdi->battery); > - goto out2; > + goto out3; > } > > ret = bq24190_sysfs_create_group(bdi); > if (ret) { > dev_err(dev, "Can't create sysfs entries\n"); > - goto out3; > + goto out4; > } > > bdi->initialized = true; > @@ -1406,21 +1440,30 @@ static int bq24190_probe(struct i2c_client *client, > "bq24190-charger", bdi); > if (ret < 0) { > dev_err(dev, "Can't set up irq handler\n"); > - goto out4; > + goto out5; > } > > + enable_irq_wake(bdi->irq); > + > + pm_runtime_mark_last_busy(dev); > + pm_runtime_put_autosuspend(dev); > + > return 0; > > -out4: > +out5: > bq24190_sysfs_remove_group(bdi); > > -out3: > +out4: > power_supply_unregister(bdi->battery); > > -out2: > +out3: > power_supply_unregister(bdi->charger); > > +out2: > + pm_runtime_put_sync(dev); > + > out1: > + pm_runtime_dont_use_autosuspend(dev); > pm_runtime_disable(dev); > if (bdi->gpio_int) > gpio_free(bdi->gpio_int); > @@ -1430,14 +1473,20 @@ static int bq24190_probe(struct i2c_client *client, > 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); > > if (bdi->gpio_int) > @@ -1480,10 +1529,20 @@ static int bq24190_pm_suspend(struct device *dev) > { > struct i2c_client *client = to_i2c_client(dev); > struct bq24190_dev_info *bdi = i2c_get_clientdata(client); > + int error; > + > + 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); > + } > > - pm_runtime_get_sync(bdi->dev); > bq24190_register_reset(bdi); > - pm_runtime_put_sync(bdi->dev); > + > + if (!error) { > + pm_runtime_mark_last_busy(bdi->dev); > + pm_runtime_put_autosuspend(bdi->dev); > + } > > return 0; > } > @@ -1492,15 +1551,25 @@ static int bq24190_pm_resume(struct device *dev) > { > struct i2c_client *client = to_i2c_client(dev); > struct bq24190_dev_info *bdi = i2c_get_clientdata(client); > + int error; > > bdi->f_reg = 0; > bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state */ > > - pm_runtime_get_sync(bdi->dev); > + error = pm_runtime_get_sync(bdi->dev); > + if (error < 0) { > + pm_runtime_put_noidle(bdi->dev); > + dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", error); > + } > + > bq24190_register_reset(bdi); > bq24190_set_mode_host(bdi); > bq24190_read(bdi, BQ24190_REG_SS, &bdi->ss_reg); > - pm_runtime_put_sync(bdi->dev); > + > + if (!error) { > + pm_runtime_mark_last_busy(bdi->dev); > + pm_runtime_put_autosuspend(bdi->dev); > + } We need helper funcs... static inline int bq24190_pmrt_get(struct device* dev) { int err = pm_runtime_get_sync(dev); if (err < 0) { dev_warn(dev, "pm_runtime_get failed: %i\n", err); pm_runtime_put_noidle(dev); } return err; } static inline void bq24190_pmrt_put(struct device* dev, int err) { if (!err) { pm_runtime_mark_last_busy(dev); pm_runtime_put_autosuspend(dev); } } Apologies, should have raised this on last version! -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
* Liam Breck <liam@networkimprov.net> [170202 17:44]: > > @@ -427,11 +428,20 @@ static ssize_t bq24190_sysfs_show(struct device *dev, > > if (!info) > > return -EINVAL; > > > > + ret = pm_runtime_get_sync(bdi->dev); > > + if (ret < 0) > > + return ret; > > Let's not stop here if get_sync fails. Hmm care to describe why we would want to ignore errors on pm_runtime_get()? I don't think that's a good idea.. And for sysfs_show() output? If these calls fail it's a sign of a serious problem somewhere and it should be fixed instead of ignoring the errors and attempting to continue. Ignoring errors with these calls can lead into problems that are hard to debug. > > @@ -1364,12 +1394,16 @@ static int bq24190_probe(struct i2c_client *client, > > } > > > > pm_runtime_enable(dev); > > - pm_runtime_resume(dev); > > + pm_runtime_use_autosuspend(dev); > > + pm_runtime_set_autosuspend_delay(dev, 600); > > + ret = pm_runtime_get_sync(dev); > > + if (ret < 0) > > + goto out1; > > Nor here 8. Eek, let's not try to continue the probe if pm_runtime_get_fails. Let's just assume it's either implemented and must be working, or a nop operation that will not do anything. Depending on the implementation, this could affect something in the hardware like pin multiplexing, parent device clocking.. On the module exit path it might make sense as we know the device was working to get there so it might be possible to do something :) > We need helper funcs... > > static inline int bq24190_pmrt_get(struct device* dev) { > int err = pm_runtime_get_sync(dev); > if (err < 0) { > dev_warn(dev, "pm_runtime_get failed: %i\n", err); > pm_runtime_put_noidle(dev); > } > return err; > } > > static inline void bq24190_pmrt_put(struct device* dev, int err) { > if (!err) { > pm_runtime_mark_last_busy(dev); > pm_runtime_put_autosuspend(dev); > } > } If you want to implement something like that it should probably be done in a Linux generic way. There's still quite a bit boilerplate code needed for the autosuspend usage. Regards, Tony -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Jan 30, 2017 at 4:36 PM, Tony Lindgren <tony@atomide.com> wrote: > From: Tony Lindgren <tony@atomide.com> > Date: Mon, 30 Jan 2017 07:10:31 -0800 > Subject: [PATCH] power: bq24190_charger: Use PM runtime autosuspend > > We can get quite a few interrupts when the battery is trickle charging. > Let's enable PM runtime autosuspend to avoid constantly toggling device > driver PM runtime state. > > Let's use a 600 ms timeout as that's how long the USB chager detection > might take. > > Cc: Liam Breck <kernel@networkimprov.net> > Acked-by: Mark Greer <mgreer@animalcreek.com> > Signed-off-by: Tony Lindgren <tony@atomide.com> > --- > drivers/power/supply/bq24190_charger.c | 153 ++++++++++++++++++++++++--------- > 1 file changed, 111 insertions(+), 42 deletions(-) > > 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 > ... > @@ -1234,11 +1267,15 @@ static void bq24190_check_status(struct bq24190_dev_info *bdi) > static irqreturn_t bq24190_irq_handler_thread(int irq, void *data) > { > struct bq24190_dev_info *bdi = data; > + int ret; > > bdi->irq_event = true; > - pm_runtime_get_sync(bdi->dev); > + ret = pm_runtime_get_sync(bdi->dev); > + if (ret < 0) > + return ret; > bq24190_check_status(bdi); > - pm_runtime_put_sync(bdi->dev); > + pm_runtime_mark_last_busy(bdi->dev); > + pm_runtime_put_autosuspend(bdi->dev); > bdi->irq_event = false; > > return IRQ_HANDLED; OK get_sync() failure should return error to userspace callers, and abort probe(), makes sense. But should the interrupt handler (above) emit a warning and continue as in resume() and suspend()? If not is ret the right value to return in lieu of irq_handled? -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 @@ -420,6 +420,7 @@ static ssize_t bq24190_sysfs_show(struct device *dev, struct power_supply *psy = dev_get_drvdata(dev); struct bq24190_dev_info *bdi = power_supply_get_drvdata(psy); struct bq24190_sysfs_field_info *info; + ssize_t count; int ret; u8 v; @@ -427,11 +428,20 @@ static ssize_t bq24190_sysfs_show(struct device *dev, if (!info) return -EINVAL; + ret = pm_runtime_get_sync(bdi->dev); + if (ret < 0) + return ret; + ret = bq24190_read_mask(bdi, info->reg, info->mask, info->shift, &v); if (ret) - return ret; + count = ret; + else + count = scnprintf(buf, PAGE_SIZE, "%hhx\n", v); - return scnprintf(buf, PAGE_SIZE, "%hhx\n", v); + pm_runtime_mark_last_busy(bdi->dev); + pm_runtime_put_autosuspend(bdi->dev); + + return count; } static ssize_t bq24190_sysfs_store(struct device *dev, @@ -451,9 +461,16 @@ static ssize_t bq24190_sysfs_store(struct device *dev, if (ret < 0) return ret; + ret = pm_runtime_get_sync(bdi->dev); + if (ret < 0) + return ret; + ret = bq24190_write_mask(bdi, info->reg, info->mask, info->shift, v); if (ret) - return ret; + count = ret; + + pm_runtime_mark_last_busy(bdi->dev); + pm_runtime_put_autosuspend(bdi->dev); return count; } @@ -795,7 +812,9 @@ static int bq24190_charger_get_property(struct power_supply *psy, dev_dbg(bdi->dev, "prop: %d\n", psp); - pm_runtime_get_sync(bdi->dev); + ret = pm_runtime_get_sync(bdi->dev); + if (ret < 0) + return ret; switch (psp) { case POWER_SUPPLY_PROP_CHARGE_TYPE: @@ -835,7 +854,9 @@ static int bq24190_charger_get_property(struct power_supply *psy, ret = -ENODATA; } - pm_runtime_put_sync(bdi->dev); + pm_runtime_mark_last_busy(bdi->dev); + pm_runtime_put_autosuspend(bdi->dev); + return ret; } @@ -848,7 +869,9 @@ static int bq24190_charger_set_property(struct power_supply *psy, dev_dbg(bdi->dev, "prop: %d\n", psp); - pm_runtime_get_sync(bdi->dev); + ret = pm_runtime_get_sync(bdi->dev); + if (ret < 0) + return ret; switch (psp) { case POWER_SUPPLY_PROP_CHARGE_TYPE: @@ -864,7 +887,9 @@ static int bq24190_charger_set_property(struct power_supply *psy, ret = -EINVAL; } - pm_runtime_put_sync(bdi->dev); + pm_runtime_mark_last_busy(bdi->dev); + pm_runtime_put_autosuspend(bdi->dev); + return ret; } @@ -1065,7 +1090,9 @@ static int bq24190_battery_get_property(struct power_supply *psy, dev_dbg(bdi->dev, "prop: %d\n", psp); - pm_runtime_get_sync(bdi->dev); + ret = pm_runtime_get_sync(bdi->dev); + if (ret < 0) + return ret; switch (psp) { case POWER_SUPPLY_PROP_STATUS: @@ -1093,7 +1120,9 @@ static int bq24190_battery_get_property(struct power_supply *psy, ret = -ENODATA; } - pm_runtime_put_sync(bdi->dev); + pm_runtime_mark_last_busy(bdi->dev); + pm_runtime_put_autosuspend(bdi->dev); + return ret; } @@ -1106,7 +1135,9 @@ static int bq24190_battery_set_property(struct power_supply *psy, dev_dbg(bdi->dev, "prop: %d\n", psp); - pm_runtime_get_sync(bdi->dev); + ret = pm_runtime_get_sync(bdi->dev); + if (ret < 0) + return ret; switch (psp) { case POWER_SUPPLY_PROP_ONLINE: @@ -1119,7 +1150,9 @@ static int bq24190_battery_set_property(struct power_supply *psy, ret = -EINVAL; } - pm_runtime_put_sync(bdi->dev); + pm_runtime_mark_last_busy(bdi->dev); + pm_runtime_put_autosuspend(bdi->dev); + return ret; } @@ -1234,11 +1267,15 @@ static void bq24190_check_status(struct bq24190_dev_info *bdi) static irqreturn_t bq24190_irq_handler_thread(int irq, void *data) { struct bq24190_dev_info *bdi = data; + int ret; bdi->irq_event = true; - pm_runtime_get_sync(bdi->dev); + ret = pm_runtime_get_sync(bdi->dev); + if (ret < 0) + return ret; bq24190_check_status(bdi); - pm_runtime_put_sync(bdi->dev); + pm_runtime_mark_last_busy(bdi->dev); + pm_runtime_put_autosuspend(bdi->dev); bdi->irq_event = false; return IRQ_HANDLED; @@ -1249,33 +1286,26 @@ static int bq24190_hw_init(struct bq24190_dev_info *bdi) u8 v; int ret; - pm_runtime_get_sync(bdi->dev); - /* First check that the device really is what its supposed to be */ ret = bq24190_read_mask(bdi, BQ24190_REG_VPRS, BQ24190_REG_VPRS_PN_MASK, BQ24190_REG_VPRS_PN_SHIFT, &v); if (ret < 0) - goto out; + return ret; - if (v != bdi->model) { - ret = -ENODEV; - goto out; - } + if (v != bdi->model) + return -ENODEV; ret = bq24190_register_reset(bdi); if (ret < 0) - goto out; + return ret; ret = bq24190_set_mode_host(bdi); if (ret < 0) - goto out; + return ret; - ret = bq24190_read(bdi, BQ24190_REG_SS, &bdi->ss_reg); -out: - pm_runtime_put_sync(bdi->dev); - return ret; + return bq24190_read(bdi, BQ24190_REG_SS, &bdi->ss_reg); } #ifdef CONFIG_OF @@ -1364,12 +1394,16 @@ static int bq24190_probe(struct i2c_client *client, } pm_runtime_enable(dev); - pm_runtime_resume(dev); + pm_runtime_use_autosuspend(dev); + pm_runtime_set_autosuspend_delay(dev, 600); + ret = pm_runtime_get_sync(dev); + if (ret < 0) + goto out1; ret = bq24190_hw_init(bdi); if (ret < 0) { dev_err(dev, "Hardware init failed\n"); - goto out1; + goto out2; } charger_cfg.drv_data = bdi; @@ -1380,7 +1414,7 @@ static int bq24190_probe(struct i2c_client *client, if (IS_ERR(bdi->charger)) { dev_err(dev, "Can't register charger\n"); ret = PTR_ERR(bdi->charger); - goto out1; + goto out2; } battery_cfg.drv_data = bdi; @@ -1389,13 +1423,13 @@ static int bq24190_probe(struct i2c_client *client, if (IS_ERR(bdi->battery)) { dev_err(dev, "Can't register battery\n"); ret = PTR_ERR(bdi->battery); - goto out2; + goto out3; } ret = bq24190_sysfs_create_group(bdi); if (ret) { dev_err(dev, "Can't create sysfs entries\n"); - goto out3; + goto out4; } bdi->initialized = true; @@ -1406,21 +1440,30 @@ static int bq24190_probe(struct i2c_client *client, "bq24190-charger", bdi); if (ret < 0) { dev_err(dev, "Can't set up irq handler\n"); - goto out4; + goto out5; } + enable_irq_wake(bdi->irq); + + pm_runtime_mark_last_busy(dev); + pm_runtime_put_autosuspend(dev); + return 0; -out4: +out5: bq24190_sysfs_remove_group(bdi); -out3: +out4: power_supply_unregister(bdi->battery); -out2: +out3: power_supply_unregister(bdi->charger); +out2: + pm_runtime_put_sync(dev); + out1: + pm_runtime_dont_use_autosuspend(dev); pm_runtime_disable(dev); if (bdi->gpio_int) gpio_free(bdi->gpio_int); @@ -1430,14 +1473,20 @@ static int bq24190_probe(struct i2c_client *client, 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); if (bdi->gpio_int) @@ -1480,10 +1529,20 @@ static int bq24190_pm_suspend(struct device *dev) { struct i2c_client *client = to_i2c_client(dev); struct bq24190_dev_info *bdi = i2c_get_clientdata(client); + int error; + + 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); + } - pm_runtime_get_sync(bdi->dev); bq24190_register_reset(bdi); - pm_runtime_put_sync(bdi->dev); + + if (!error) { + pm_runtime_mark_last_busy(bdi->dev); + pm_runtime_put_autosuspend(bdi->dev); + } return 0; } @@ -1492,15 +1551,25 @@ static int bq24190_pm_resume(struct device *dev) { struct i2c_client *client = to_i2c_client(dev); struct bq24190_dev_info *bdi = i2c_get_clientdata(client); + int error; bdi->f_reg = 0; bdi->ss_reg = BQ24190_REG_SS_VBUS_STAT_MASK; /* impossible state */ - pm_runtime_get_sync(bdi->dev); + error = pm_runtime_get_sync(bdi->dev); + if (error < 0) { + pm_runtime_put_noidle(bdi->dev); + dev_warn(bdi->dev, "pm_runtime_get failed: %i\n", error); + } + bq24190_register_reset(bdi); bq24190_set_mode_host(bdi); bq24190_read(bdi, BQ24190_REG_SS, &bdi->ss_reg); - pm_runtime_put_sync(bdi->dev); + + if (!error) { + pm_runtime_mark_last_busy(bdi->dev); + pm_runtime_put_autosuspend(bdi->dev); + } /* Things may have changed while suspended so alert upper layer */ power_supply_changed(bdi->charger);