2013-08-02 20:38:55

by Zoran Markovic

[permalink] [raw]
Subject: [RFC PATCH] pm: prevent suspend until power supply events are processed

This patch, originally authored by Arve Hjonnevag and Todd Poynor,
prevents the system from entering suspend mode until the power
supply plug, unplug, or any other change of state event is fully
processed. This guarantees that the screen lights up and displays
the battery charging state. The implementation uses the power
supply wakeup_source object.

Cc: Anton Vorontsov <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Arve Hjonnevag <[email protected]>
Cc: Todd Poynor <[email protected]>
Cc: John Stultz <[email protected]>
Signed-off-by: Zoran Markovic <[email protected]>
---
drivers/power/power_supply_core.c | 37 +++++++++++++++++++++++++++++++------
include/linux/power_supply.h | 2 ++
2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
index 3b2d5df..e68d598 100644
--- a/drivers/power/power_supply_core.c
+++ b/drivers/power/power_supply_core.c
@@ -67,23 +67,41 @@ static int __power_supply_changed_work(struct device *dev, void *data)

static void power_supply_changed_work(struct work_struct *work)
{
+ unsigned long flags;
struct power_supply *psy = container_of(work, struct power_supply,
changed_work);

dev_dbg(psy->dev, "%s\n", __func__);

- class_for_each_device(power_supply_class, NULL, psy,
- __power_supply_changed_work);
-
- power_supply_update_leds(psy);
-
- kobject_uevent(&psy->dev->kobj, KOBJ_CHANGE);
+ spin_lock_irqsave(&psy->changed_lock, flags);
+ if (psy->changed) {
+ psy->changed = false;
+ spin_unlock_irqrestore(&psy->changed_lock, flags);
+ class_for_each_device(power_supply_class, NULL, psy,
+ __power_supply_changed_work);
+ power_supply_update_leds(psy);
+ kobject_uevent(&psy->dev->kobj, KOBJ_CHANGE);
+ spin_lock_irqsave(&psy->changed_lock, flags);
+ }
+ /* dependent power supplies (e.g. battery) may have changed
+ * state as a result of this event, so poll again and hold
+ * the wakeup_source until all events are processed.
+ */
+ if (!psy->changed)
+ pm_relax(psy->dev);
+ spin_unlock_irqrestore(&psy->changed_lock, flags);
}

void power_supply_changed(struct power_supply *psy)
{
+ unsigned long flags;
+
dev_dbg(psy->dev, "%s\n", __func__);

+ spin_lock_irqsave(&psy->changed_lock, flags);
+ psy->changed = true;
+ pm_stay_awake(psy->dev);
+ spin_unlock_irqrestore(&psy->changed_lock, flags);
schedule_work(&psy->changed_work);
}
EXPORT_SYMBOL_GPL(power_supply_changed);
@@ -500,6 +518,11 @@ int power_supply_register(struct device *parent, struct power_supply *psy)
goto check_supplies_failed;
}

+ spin_lock_init(&psy->changed_lock);
+ rc = device_init_wakeup(dev, true);
+ if (rc)
+ goto wakeup_init_failed;
+
rc = kobject_set_name(&dev->kobj, "%s", psy->name);
if (rc)
goto kobject_set_name_failed;
@@ -529,6 +552,7 @@ create_triggers_failed:
register_cooler_failed:
psy_unregister_thermal(psy);
register_thermal_failed:
+wakeup_init_failed:
device_del(dev);
kobject_set_name_failed:
device_add_failed:
@@ -546,6 +570,7 @@ void power_supply_unregister(struct power_supply *psy)
power_supply_remove_triggers(psy);
psy_unregister_cooler(psy);
psy_unregister_thermal(psy);
+ device_init_wakeup(psy->dev, false);
device_unregister(psy->dev);
}
EXPORT_SYMBOL_GPL(power_supply_unregister);
diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
index 804b906..253d412 100644
--- a/include/linux/power_supply.h
+++ b/include/linux/power_supply.h
@@ -194,6 +194,8 @@ struct power_supply {
/* private */
struct device *dev;
struct work_struct changed_work;
+ spinlock_t changed_lock;
+ bool changed;
#ifdef CONFIG_THERMAL
struct thermal_zone_device *tzd;
struct thermal_cooling_device *tcd;
--
1.7.9.5


2013-08-22 17:10:44

by Zoran Markovic

[permalink] [raw]
Subject: Re: [RFC PATCH] pm: prevent suspend until power supply events are processed

Any opinions on this patch?
Regards, Zoran

On 2 August 2013 13:38, Zoran Markovic <[email protected]> wrote:
> This patch, originally authored by Arve Hjonnevag and Todd Poynor,
> prevents the system from entering suspend mode until the power
> supply plug, unplug, or any other change of state event is fully
> processed. This guarantees that the screen lights up and displays
> the battery charging state. The implementation uses the power
> supply wakeup_source object.
>
> Cc: Anton Vorontsov <[email protected]>
> Cc: David Woodhouse <[email protected]>
> Cc: Arve Hjonnevag <[email protected]>
> Cc: Todd Poynor <[email protected]>
> Cc: John Stultz <[email protected]>
> Signed-off-by: Zoran Markovic <[email protected]>
> ---
> drivers/power/power_supply_core.c | 37 +++++++++++++++++++++++++++++++------
> include/linux/power_supply.h | 2 ++
> 2 files changed, 33 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/power/power_supply_core.c b/drivers/power/power_supply_core.c
> index 3b2d5df..e68d598 100644
> --- a/drivers/power/power_supply_core.c
> +++ b/drivers/power/power_supply_core.c
> @@ -67,23 +67,41 @@ static int __power_supply_changed_work(struct device *dev, void *data)
>
> static void power_supply_changed_work(struct work_struct *work)
> {
> + unsigned long flags;
> struct power_supply *psy = container_of(work, struct power_supply,
> changed_work);
>
> dev_dbg(psy->dev, "%s\n", __func__);
>
> - class_for_each_device(power_supply_class, NULL, psy,
> - __power_supply_changed_work);
> -
> - power_supply_update_leds(psy);
> -
> - kobject_uevent(&psy->dev->kobj, KOBJ_CHANGE);
> + spin_lock_irqsave(&psy->changed_lock, flags);
> + if (psy->changed) {
> + psy->changed = false;
> + spin_unlock_irqrestore(&psy->changed_lock, flags);
> + class_for_each_device(power_supply_class, NULL, psy,
> + __power_supply_changed_work);
> + power_supply_update_leds(psy);
> + kobject_uevent(&psy->dev->kobj, KOBJ_CHANGE);
> + spin_lock_irqsave(&psy->changed_lock, flags);
> + }
> + /* dependent power supplies (e.g. battery) may have changed
> + * state as a result of this event, so poll again and hold
> + * the wakeup_source until all events are processed.
> + */
> + if (!psy->changed)
> + pm_relax(psy->dev);
> + spin_unlock_irqrestore(&psy->changed_lock, flags);
> }
>
> void power_supply_changed(struct power_supply *psy)
> {
> + unsigned long flags;
> +
> dev_dbg(psy->dev, "%s\n", __func__);
>
> + spin_lock_irqsave(&psy->changed_lock, flags);
> + psy->changed = true;
> + pm_stay_awake(psy->dev);
> + spin_unlock_irqrestore(&psy->changed_lock, flags);
> schedule_work(&psy->changed_work);
> }
> EXPORT_SYMBOL_GPL(power_supply_changed);
> @@ -500,6 +518,11 @@ int power_supply_register(struct device *parent, struct power_supply *psy)
> goto check_supplies_failed;
> }
>
> + spin_lock_init(&psy->changed_lock);
> + rc = device_init_wakeup(dev, true);
> + if (rc)
> + goto wakeup_init_failed;
> +
> rc = kobject_set_name(&dev->kobj, "%s", psy->name);
> if (rc)
> goto kobject_set_name_failed;
> @@ -529,6 +552,7 @@ create_triggers_failed:
> register_cooler_failed:
> psy_unregister_thermal(psy);
> register_thermal_failed:
> +wakeup_init_failed:
> device_del(dev);
> kobject_set_name_failed:
> device_add_failed:
> @@ -546,6 +570,7 @@ void power_supply_unregister(struct power_supply *psy)
> power_supply_remove_triggers(psy);
> psy_unregister_cooler(psy);
> psy_unregister_thermal(psy);
> + device_init_wakeup(psy->dev, false);
> device_unregister(psy->dev);
> }
> EXPORT_SYMBOL_GPL(power_supply_unregister);
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 804b906..253d412 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -194,6 +194,8 @@ struct power_supply {
> /* private */
> struct device *dev;
> struct work_struct changed_work;
> + spinlock_t changed_lock;
> + bool changed;
> #ifdef CONFIG_THERMAL
> struct thermal_zone_device *tzd;
> struct thermal_cooling_device *tcd;
> --
> 1.7.9.5
>

2013-08-28 01:48:11

by Anton Vorontsov

[permalink] [raw]
Subject: Re: [RFC PATCH] pm: prevent suspend until power supply events are processed

On Fri, Aug 02, 2013 at 01:38:02PM -0700, Zoran Markovic wrote:
> This patch, originally authored by Arve Hjonnevag and Todd Poynor,
> prevents the system from entering suspend mode until the power
> supply plug, unplug, or any other change of state event is fully
> processed. This guarantees that the screen lights up and displays
> the battery charging state. The implementation uses the power
> supply wakeup_source object.
>
> Cc: Anton Vorontsov <[email protected]>
> Cc: David Woodhouse <[email protected]>
> Cc: Arve Hjonnevag <[email protected]>
> Cc: Todd Poynor <[email protected]>
> Cc: John Stultz <[email protected]>
> Signed-off-by: Zoran Markovic <[email protected]>
> ---
...
> + kobject_uevent(&psy->dev->kobj, KOBJ_CHANGE);
> + spin_lock_irqsave(&psy->changed_lock, flags);
> + }
> + /* dependent power supplies (e.g. battery) may have changed

Multi-line comments style issue...

> + * state as a result of this event, so poll again and hold
> + * the wakeup_source until all events are processed.
> + */
> + if (!psy->changed)
> + pm_relax(psy->dev);
...
> diff --git a/include/linux/power_supply.h b/include/linux/power_supply.h
> index 804b906..253d412 100644
> --- a/include/linux/power_supply.h
> +++ b/include/linux/power_supply.h
> @@ -194,6 +194,8 @@ struct power_supply {
> /* private */
> struct device *dev;
> struct work_struct changed_work;
> + spinlock_t changed_lock;

#include <linux/spinlock.h> is needed.

I fixed it up and applied the patch, thanks a lot!

Anton