Messages in this thread Patch in this message | | | From | Dmitry Torokhov <> | Subject | Re: [PATCH 1/1] acer-wmi: switch driver to dev_pm_ops | Date | Sun, 26 Jul 2009 14:33:00 -0700 |
| |
On Jul 26, 2009, at 1:28 PM, Arnaud Faucher <arnaud.faucher@gmail.com> wrote:
> On dim, 2009-07-26 at 19:35 +0100, Carlos Corbacho wrote: >> On Sunday 26 July 2009 19:08:09 Dmitry Torokhov wrote: >>> On Sun, Jul 26, 2009 at 03:23:29PM +0100, Carlos Corbacho wrote: >>>> [Removing linux-mips from CC - I don't know why they'd be >>>> interested in >>>> an x86 only platform driver...] >>>> >>>> On Sunday 26 July 2009 14:53:33 Arnaud Faucher wrote: >>>>> Gets rid of the following warning: >>>>> Platform driver 'acer-wmi' needs updating - please use dev_pm_ops >>>>> >>>>> Take 2, thanks to Dmitry, Rafael and Frans for pointing out PM >>>>> issue on >>>>> hibernation when using dev_pm_ops blindly. >>>>> >>>>> This patch was tested against suspendand hibernation (Acer mail >>>>> led >>>>> status). >>>>> >>>>> Signed-off-by: Arnaud Faucher <arnaud.faucher@gmail.com> >>>>> --- >>>>> drivers/platform/x86/acer-wmi.c | 17 ++++++++++++----- >>>>> 1 files changed, 12 insertions(+), 5 deletions(-) >>>>> >>>>> diff --git a/drivers/platform/x86/acer-wmi.c >>>>> b/drivers/platform/x86/acer-wmi.c >>>>> index be2fd6f..29374bc 100644 >>>>> --- a/drivers/platform/x86/acer-wmi.c >>>>> +++ b/drivers/platform/x86/acer-wmi.c >>>>> @@ -1152,8 +1152,7 @@ static int acer_platform_remove(struct >>>>> platform_device *device) >>>>> return 0; >>>>> } >>>>> >>>>> -static int acer_platform_suspend(struct platform_device *dev, >>>>> -pm_message_t state) >>>>> +static int acer_platform_suspend(struct device *dev) >>>>> { >>>>> u32 value; >>>>> struct acer_data *data = &interface->data; >>>>> @@ -1174,7 +1173,7 @@ pm_message_t state) >>>>> return 0; >>>>> } >>>>> >>>>> -static int acer_platform_resume(struct platform_device *device) >>>>> +static int acer_platform_resume(struct device *dev) >>>>> { >>>>> struct acer_data *data = &interface->data; >>>>> >>>>> @@ -1190,15 +1189,23 @@ static int acer_platform_resume(struct >>>>> platform_device *device) >>>>> return 0; >>>>> } >>>>> >>>>> +static struct dev_pm_ops acer_platform_pm_ops = { >>>>> + .suspend = acer_platform_suspend, >>>>> + .resume = acer_platform_resume, >>>> >>>> Are these necessary? For suspend-to-RAM, I've never needed these. >>>> The old >>>> callbacks here were just for suspend-to-disk. >>> >>> That is not correct. Old suspend and resume callbacks were called >>> for >>> both S2R and S2D. Whether it is actually needed for S2R I don't >>> know but >>> looking at the code they should not hurt. >> >> I'm aware they were called for S2RAM as well, but that was just a >> limitation >> of the old calls - as I say, they're not needed for it (at least on >> my >> hardware anyway). >> > > I was looking for similar functionality. > >>>>> + .freeze = acer_platform_suspend, >>>>> + .thaw = acer_platform_resume, >>>> >>>> If we only need these callbacks for freeze & thaw, they should be >>>> rebamed. >>>> >>>>> + .poweroff = acer_platform_suspend, >>>>> + .restore = acer_platform_resume, >>>> >>>> What do poweroff and restore mean in this context. Do my comments >>>> above >>>> apply again (i.e. are the callbacks necessary here)? >>> >>> I don't think poweroff handler is needed. > > After testing many combinations, I observed that I had to use that > much > callbacks. For example, when omitting to wire .poweroff/.restore, > with .freeze/.thaw linked to suspend()/resume(), the state (of the > mail > led) is not restored correctly after S2D.
Have you tried with just 3 - freeze, thaw and restore?
>
-- Dmitry
| |