Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops less error-prone

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Aug 5, 2009 at 10:05 PM, Rafael J. Wysocki<rjw@xxxxxxx> wrote:
> On Wednesday 05 August 2009, Rafael J. Wysocki wrote:
>> On Wednesday 05 August 2009, Albin Tonnerre wrote:
>> > On Tue, Aug 04, 2009 at 09:55:33PM -0700, Dmitry Torokhov wrote :
>> > > On Tue, Aug 04, 2009 at 11:36:12AM +0200, Albin Tonnerre wrote:
>> > > > In a number of cases, the .suspend, .freeze, .poweroff and .resume,
>> > > > .thaw, .restore functions are identical. However, they all need to be
>> > > > assigned to avoid regressionsm as the previous code called .suspend
>> > > > resp. .resume in all those cases. SIMPLE_PM_OPS allows to deal with
>> > > > this case.
>> >
>> >
>> > > I'd much rather have conversions done with a bit more analysis now that
>> > > our framework is more flexible and we can have specialized routines for
>> > > hibernation and suspend.
>> >
>> > I still think that even though they can, quite a number of drivers won't
>> > /need/ to have different functions for this, but maybe I'm mistaken.
>> >
>> > > Maybe we should try changing from run-time to build time warning so that
>> > > users are not overly concerned with it?
>> >
>> > I'm not sure that solves the problem. The fact is that even for developers, it's
>> > easy to overlook that assiging only the .suspend and .resume fields is probably
>> > a mistake.
>>
>> I agree, so I'm going to take the patch.
>>
>> I'll add a comment describing what the macro is for, though.
>
> Strictly speaking, I'm going to add the appended patch to the linux-next
> branch of the suspend-2.6 tree.
>
> Thanks,
> Rafael
>
> ---
> From: Albin Tonnerre <albin.tonnerre@xxxxxxxxxxxxxxxxxx>
> Subject: PM: Add convenience macro to make switching to dev_pm_ops less error-prone
>
> In a number of cases, the .suspend, .freeze, .poweroff and .resume,
> .thaw, .restore functions are identical. However, they all need to be
> assigned to avoid regressionsm as the previous code called .suspend
> resp. .resume in all those cases. SIMPLE_DEV_PM_OPS helps to deal
> with this case.
>
> [rjw: Changed the name of the macro and added the comment explaining its
>  purpose.]
>
> Signed-off-by: Albin Tonnerre <albin.tonnerre@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Rafael J. Wysocki <rjw@xxxxxxx>
> ---
>  include/linux/pm.h |   14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> Index: linux-2.6/include/linux/pm.h
> ===================================================================
> --- linux-2.6.orig/include/linux/pm.h
> +++ linux-2.6/include/linux/pm.h
> @@ -184,6 +184,20 @@ struct dev_pm_ops {
>        int (*restore_noirq)(struct device *dev);
>  };
>
> +/*
> + * Use this if you want to use the same suspend and resume callbacks for suspend
> + * to RAM and hibernation.
> + */
> +#define SIMPLE_DEV_PM_OPS(name, suspend_fn, resume_fn) \
> +struct dev_pm_ops name = { \
> +       .suspend = suspend_fn, \
> +       .resume = resume_fn, \
> +       .freeze = suspend_fn, \
> +       .thaw = resume_fn, \
> +       .poweroff = suspend_fn, \
> +       .resume = resume_fn, \

I think that second .resume was intended to be .restore instead.

regards
Philipp
_______________________________________________
linux-pm mailing list
linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linux-foundation.org/mailman/listinfo/linux-pm


[Index of Archives]     [Linux ACPI]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [CPU Freq]     [Kernel Newbies]     [Fedora Kernel]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux