linux-kernel - Re: [PATCH] PM: Add convenience macro to make switching to dev_pm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <74d0deb30908051317o61b31117t35aaf8b9f02710d7@mail.gmail.com>
Date:	Wed, 5 Aug 2009 22:17:47 +0200
From:	pHilipp Zabel <philipp.zabel@...il.com>
To:	"Rafael J. Wysocki" <rjw@...k.pl>
Cc:	Albin Tonnerre <albin.tonnerre@...e-electrons.com>,
	Dmitry Torokhov <dmitry.torokhov@...il.com>,
	Frans Pop <elendil@...net.nl>, linux-kernel@...r.kernel.org,
	Linux PM List <linux-pm@...ts.linux-foundation.org>,
	Daniel Mack <daniel@...aq.de>
Subject: Re: [PATCH] PM: Add convenience macro to make switching to dev_pm_ops 
	less error-prone

On Wed, Aug 5, 2009 at 10:05 PM, Rafael J. Wysocki<rjw@...k.pl> 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@...e-electrons.com>
> 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@...e-electrons.com>
> Signed-off-by: Rafael J. Wysocki <rjw@...k.pl>
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/