Thread (17 messages) 17 messages, 2 authors, 2017-02-07

Re: [PATCH 2/2] power: bq24190_charger: Use PM runtime autosuspend

From: Liam Breck <hidden>
Date: 2017-01-31 00:14:16
Also in: linux-omap

Hi, these emails are missing v2 in subject line.


On Mon, Jan 30, 2017 at 4:02 PM, Tony Lindgren [off-list ref] wrote:
quoted hunk ↗ jump to hunk
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 <redacted>
Acked-by: Mark Greer <mgreer@animalcreek.com>
Acked-by: Liam Breck <redacted>
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
...
@@ -1364,12 +1394,16 @@ static int bq24190_probe(struct i2c_client *client,
        }

        pm_runtime_enable(dev);
-       pm_runtime_resume(dev);
+       pm_runtime_set_autosuspend_delay(dev, 600);
+       pm_runtime_use_autosuspend(dev);
+       ret = pm_runtime_get_sync(dev);
+       if (ret < 0)
+               goto out1;
Call get_sync() before set_autosuspend() ?
quoted hunk ↗ jump to hunk
        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);
Change order of above two if my prev comment is right.
quoted hunk ↗ jump to hunk
        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_dont_use_autosuspend(bdi->dev);
+       pm_runtime_put_sync(bdi->dev);
Alternatively, switch order of above two.
        pm_runtime_disable(bdi->dev);

        if (bdi->gpio_int)
Apologies if I'm nitpicking!

~.~
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help