Messages in this thread | | | Date | Wed, 3 Sep 2014 10:35:35 +0530 | Subject | Re: [RFC PATCH] pm: prevent suspend until power supply events are processed | From | Viresh Kumar <> |
| |
On Wed, Sep 3, 2014 at 10:09 AM, Viresh Kumar <viresh.kumar@linaro.org> wrote: > Don't have Zoran's new email address, but probably other might have > answers to my queries.
Got Zoran's email id finally :)
> I have just started with the power-supply framework a day or two back > and so my understanding might not be good enough :) > > On Sat, Aug 3, 2013 at 2:08 AM, Zoran Markovic > <zoran.markovic@linaro.org> wrote: >> 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) { > > Can this be false here? We have reached here as the work was > scheduled after setting it to true.. > > Maybe a WARN_ON(psy->changed) is more sensible here ? > >> + 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); > > I got a bit confused here. Does the above comment say this: > > The supplies dependent on 'psy' may change states and that *may* > change the state of 'psy' again? And so psy->changed is set to true > again?
| |