Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1236282
| From | Ulf Hansson <ulf.hansson@linaro.org> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v7 1/2] PM / sleep: Go direct_complete if driver has no callbacks |
| Date | 2015-09-30 15:30 +0200 |
| Message-ID | <qepCi-6f9-7@gated-at.bofh.it> (permalink) |
| References | <qeml4-1mP-1@gated-at.bofh.it> <qeml6-1mP-17@gated-at.bofh.it> <qenTQ-3J6-27@gated-at.bofh.it> <qeoPU-54G-7@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
[...]
>> > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> > index 16550c63d611..3cae1aa1885b 100644
>> > --- a/drivers/base/power/domain.c
>> > +++ b/drivers/base/power/domain.c
>> > @@ -20,6 +20,8 @@
>> > #include <linux/suspend.h>
>> > #include <linux/export.h>
>> >
>> > +#include "power.h"
>> > +
>> > #define GENPD_RETRY_MAX_MS 250 /* Approximate */
>> >
>> > #define GENPD_DEV_CALLBACK(genpd, type, callback, dev) \
>> > @@ -1305,6 +1307,7 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
>> >
>> > list_add_tail(&gpd_data->base.list_node, &genpd->dev_list);
>> >
>> > + device_check_pm_callbacks(dev);
>>
>> Don't do this while the genpd mutex is held as there is no need to.
>> Please move it outside the lock and only do it for non-error case.
>>
>> > out:
>> > mutex_unlock(&genpd->lock);
>> >
>> > @@ -1369,6 +1372,8 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
>> >
>> > genpd_free_dev_data(dev, gpd_data);
>> >
>> > + device_check_pm_callbacks(dev);
>> > +
>> > return 0;
>>
>> I can't tell whether this is an interesting feature to use for devices
>> that gets attached to the ACPI PM domain. Although, you currently
>> doesn't deal with that case, and too me I think this looks a bit
>> weird/unsymmetrical.
>
> Good point.
>
> It needs to be done in every situation where a PM domain is or can be used.
>
> I guess we might require all PM domains to be attached to devices after
> a successful probe at the latest (no PM domains should be attached/detached
> after probe succeeds IOW), in which case it should be sufficient to do the
> device_check_pm_callbacks() thing each time after probe successds.
>
> Thoughts?
Ohh, I just realize that my comment was wrong. Sorry for the noise.
Why genpd needs special treatment is to take care off the
"non-probing" case. In other words when devices gets added to genpd
directly by using the pm_genpd_add_device() API. Perhaps that deserves
a comment though.
>
>> > out:
>> > diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
>> > index 1710c26ba097..5af45d20b677 100644
>> > --- a/drivers/base/power/main.c
>> > +++ b/drivers/base/power/main.c
>> > @@ -131,6 +131,8 @@ void device_pm_add(struct device *dev)
>> > dev_name(dev->parent));
>> > list_add_tail(&dev->power.entry, &dpm_list);
>> > mutex_unlock(&dpm_list_mtx);
>> > +
>> > + device_check_pm_callbacks(dev);
>> > }
>> >
>> > /**
>> > @@ -1569,6 +1571,11 @@ static int device_prepare(struct device *dev, pm_message_t state)
>> >
>> > dev->power.wakeup_path = device_may_wakeup(dev);
>> >
>> > + if (dev->power.no_pm_callbacks) {
>> > + ret = 1; /* Let device go direct_complete */
>> > + goto unlock;
>> > + }
>> > +
>> > if (dev->pm_domain) {
>> > info = "preparing power domain ";
>> > callback = dev->pm_domain->ops.prepare;
>> > @@ -1591,6 +1598,7 @@ static int device_prepare(struct device *dev, pm_message_t state)
>> > if (callback)
>> > ret = callback(dev);
>> >
>> > +unlock:
>> > device_unlock(dev);
>> >
>> > if (ret < 0) {
>> > diff --git a/drivers/base/power/power.h b/drivers/base/power/power.h
>> > index 998fa6b23084..f665a7850016 100644
>> > --- a/drivers/base/power/power.h
>> > +++ b/drivers/base/power/power.h
>> > @@ -29,6 +29,8 @@ struct wake_irq {
>> > extern void dev_pm_arm_wake_irq(struct wake_irq *wirq);
>> > extern void dev_pm_disarm_wake_irq(struct wake_irq *wirq);
>> >
>> > +extern void device_check_pm_callbacks(struct device *dev);
>> > +
>> > #ifdef CONFIG_PM_SLEEP
>> >
>> > extern int device_wakeup_attach_irq(struct device *dev,
>> > @@ -102,6 +104,10 @@ static inline void dev_pm_disarm_wake_irq(struct wake_irq *wirq)
>> > {
>> > }
>> >
>> > +static inline void device_check_pm_callbacks(struct device *dev)
>> > +{
>> > +}
>> > +
>> > #endif
>> >
>> > #ifdef CONFIG_PM_SLEEP
>> > diff --git a/include/linux/pm.h b/include/linux/pm.h
>> > index 35d599e7250d..e334b9b8cd46 100644
>> > --- a/include/linux/pm.h
>> > +++ b/include/linux/pm.h
>> > @@ -566,6 +566,7 @@ struct dev_pm_info {
>> > bool ignore_children:1;
>> > bool early_init:1; /* Owned by the PM core */
>> > bool direct_complete:1; /* Owned by the PM core */
>> > + bool no_pm_callbacks:1; /* Owned by the PM core */
>>
>> This flag is only being used while CONFIG_PM is set. I suggest we move
>> within that ifdef.
>
> I guess you mean CONFIG_PM_SLEEP?
Yes!
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PATCH v7 0/2] Allow USB devices to remain runtime-suspended when sleeping Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-09-30 12:00 +0200
[PATCH v7 1/2] PM / sleep: Go direct_complete if driver has no callbacks Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-09-30 12:00 +0200
Re: [PATCH v7 1/2] PM / sleep: Go direct_complete if driver has no callbacks Ulf Hansson <ulf.hansson@linaro.org> - 2015-09-30 13:40 +0200
Re: [PATCH v7 1/2] PM / sleep: Go direct_complete if driver has no callbacks "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-09-30 14:40 +0200
Re: [PATCH v7 1/2] PM / sleep: Go direct_complete if driver has no callbacks Ulf Hansson <ulf.hansson@linaro.org> - 2015-09-30 15:30 +0200
Re: [PATCH v7 1/2] PM / sleep: Go direct_complete if driver has no callbacks Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-09-30 16:10 +0200
Re: [PATCH v7 1/2] PM / sleep: Go direct_complete if driver has no callbacks "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-09-30 22:50 +0200
Re: [PATCH v7 1/2] PM / sleep: Go direct_complete if driver has no callbacks Ulf Hansson <ulf.hansson@linaro.org> - 2015-10-01 11:10 +0200
Re: [PATCH v7 1/2] PM / sleep: Go direct_complete if driver has no callbacks "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2015-10-02 00:00 +0200
[PATCH v7 2/2] USB / PM: Allow USB devices to remain runtime-suspended when sleeping Tomeu Vizoso <tomeu.vizoso@collabora.com> - 2015-09-30 12:00 +0200
csiph-web