Cc: Frans Pop <[email protected]>
Signed-off-by: Manuel Lauss <[email protected]>
---
V1->V2: don't remove CONFIG_PM
drivers/mmc/host/au1xmmc.c | 24 +++++++++++++++---------
1 files changed, 15 insertions(+), 9 deletions(-)
diff --git a/drivers/mmc/host/au1xmmc.c b/drivers/mmc/host/au1xmmc.c
index d3f5561..2d4e20f 100644
--- a/drivers/mmc/host/au1xmmc.c
+++ b/drivers/mmc/host/au1xmmc.c
@@ -1132,12 +1132,12 @@ static int __devexit au1xmmc_remove(struct platform_device *pdev)
}
#ifdef CONFIG_PM
-static int au1xmmc_suspend(struct platform_device *pdev, pm_message_t state)
+static int au1xmmc_suspend(struct device *dev)
{
- struct au1xmmc_host *host = platform_get_drvdata(pdev);
+ struct au1xmmc_host *host = dev_get_drvdata(dev);
int ret;
- ret = mmc_suspend_host(host->mmc, state);
+ ret = mmc_suspend_host(host->mmc, PMSG_SUSPEND);
if (ret)
return ret;
@@ -1150,27 +1150,33 @@ static int au1xmmc_suspend(struct platform_device *pdev, pm_message_t state)
return 0;
}
-static int au1xmmc_resume(struct platform_device *pdev)
+static int au1xmmc_resume(struct device *dev)
{
- struct au1xmmc_host *host = platform_get_drvdata(pdev);
+ struct au1xmmc_host *host = dev_get_drvdata(dev);
au1xmmc_reset_controller(host);
return mmc_resume_host(host->mmc);
}
+
+static struct dev_pm_ops au1xmmc_pmops = {
+ .resume = au1xmmc_resume,
+ .suspend = au1xmmc_suspend,
+};
+
+#define AU1XMMC_PMOPS &au1xmmc_pmops
+
#else
-#define au1xmmc_suspend NULL
-#define au1xmmc_resume NULL
+#define AU1XMMC_PMOPS NULL
#endif
static struct platform_driver au1xmmc_driver = {
.probe = au1xmmc_probe,
.remove = au1xmmc_remove,
- .suspend = au1xmmc_suspend,
- .resume = au1xmmc_resume,
.driver = {
.name = DRIVER_NAME,
.owner = THIS_MODULE,
+ .pm = AU1XMMC_PMOPS,
},
};
--
1.6.3.3
Hi Manuel,
On Wed, Jul 22, 2009 at 05:18:39PM +0200, Manuel Lauss wrote:
> Cc: Frans Pop <[email protected]>
> Signed-off-by: Manuel Lauss <[email protected]>
>
> +
> +static struct dev_pm_ops au1xmmc_pmops = {
> + .resume = au1xmmc_resume,
> + .suspend = au1xmmc_suspend,
> +};
> +
Was suspend to disk tested? It requires freeze()/thaw().
--
Dmitry
On Saturday 25 July 2009, Dmitry Torokhov wrote:
> On Wed, Jul 22, 2009 at 05:18:39PM +0200, Manuel Lauss wrote:
> > Cc: Frans Pop <[email protected]>
> > Signed-off-by: Manuel Lauss <[email protected]>
> >
> > +
> > +static struct dev_pm_ops au1xmmc_pmops = {
> > + .resume = au1xmmc_resume,
> > + .suspend = au1xmmc_suspend,
> > +};
> > +
>
> Was suspend to disk tested? It requires freeze()/thaw().
Is that a regression introduced by this patch then? If so, many more of
the recent dev_pm_ops conversion patches would need to be revisited.
On Sat, Jul 25, 2009 at 7:44 PM, Dmitry
Torokhov<[email protected]> wrote:
> Hi Manuel,
>
> On Wed, Jul 22, 2009 at 05:18:39PM +0200, Manuel Lauss wrote:
>> Cc: Frans Pop <[email protected]>
>> Signed-off-by: Manuel Lauss <[email protected]>
>>
>> +
>> +static struct dev_pm_ops au1xmmc_pmops = {
>> + ? ? .resume ? ? ? ? = au1xmmc_resume,
>> + ? ? .suspend ? ? ? ?= au1xmmc_suspend,
>> +};
>> +
>
> Was suspend to disk tested? It requires freeze()/thaw().
No, only suspend-to-ram, but that's good to know.
Currently for me only STR is important (and testable),
I'll have to check wheter STD works on MIPS with my test
hardware.
Thank you!
Manuel Lauss
On Sat, Jul 25, 2009 at 08:18:58PM +0200, Frans Pop wrote:
> On Saturday 25 July 2009, Dmitry Torokhov wrote:
> > On Wed, Jul 22, 2009 at 05:18:39PM +0200, Manuel Lauss wrote:
> > > Cc: Frans Pop <[email protected]>
> > > Signed-off-by: Manuel Lauss <[email protected]>
> > >
> > > +
> > > +static struct dev_pm_ops au1xmmc_pmops = {
> > > + .resume = au1xmmc_resume,
> > > + .suspend = au1xmmc_suspend,
> > > +};
> > > +
> >
> > Was suspend to disk tested? It requires freeze()/thaw().
>
> Is that a regression introduced by this patch then? If so, many more of
> the recent dev_pm_ops conversion patches would need to be revisited.
Yes, as far as I understand they would. Let's ask Rafael to confirm...
--
Dmitry
On Saturday 25 July 2009, Dmitry Torokhov wrote:
> On Sat, Jul 25, 2009 at 08:18:58PM +0200, Frans Pop wrote:
> > On Saturday 25 July 2009, Dmitry Torokhov wrote:
> > > On Wed, Jul 22, 2009 at 05:18:39PM +0200, Manuel Lauss wrote:
> > > > Cc: Frans Pop <[email protected]>
> > > > Signed-off-by: Manuel Lauss <[email protected]>
> > > >
> > > > +
> > > > +static struct dev_pm_ops au1xmmc_pmops = {
> > > > + .resume = au1xmmc_resume,
> > > > + .suspend = au1xmmc_suspend,
> > > > +};
> > > > +
> > >
> > > Was suspend to disk tested? It requires freeze()/thaw().
> >
> > Is that a regression introduced by this patch then? If so, many more of
> > the recent dev_pm_ops conversion patches would need to be revisited.
>
> Yes, as far as I understand they would. Let's ask Rafael to confirm...
Yes, they would. In general, you'd probably want to do something like this:
static struct dev_pm_ops au1xmmc_pmops = {
.resume = au1xmmc_resume,
.suspend = au1xmmc_suspend,
.freeze = au1xmmc_resume,
.thaw = au1xmmc_suspend,
.restore = au1xmmc_resume,
.poweroff = au1xmmc_suspend,
};
but in this particular case it's probably better to define separate callbacks
for .freeze() and .thaw() at least.
During hibernation we call .freeze() and .thaw() before and after creating
the image, respectively, and then .poweroff() is called right after the image
has been saved. During resume .freeze() is called after the image has been
loaded and before the control goes to the image kernel, which then calls
.restore().
HTH
I see I forgot about that myself. I'll fix up the floppy and hp-wmi patches.
Best,
Rafael
On Saturday 25 July 2009, Rafael J. Wysocki wrote:
> > > > Was suspend to disk tested? It requires freeze()/thaw().
> > >
> > > Is that a regression introduced by this patch then? If so, many
> > > more of the recent dev_pm_ops conversion patches would need to be
> > > revisited.
>
> Yes, they would. ?In general, you'd probably want to do something like
> this:
>
> static struct dev_pm_ops au1xmmc_pmops = {
> ????????.resume?????????= au1xmmc_resume,
> ????????.suspend????????????????= au1xmmc_suspend,
> ????????.freeze?????????= au1xmmc_resume,
> ????????.thaw???????????= au1xmmc_suspend,
> ????????.restore????????????????= au1xmmc_resume,
> ????????.poweroff???????= au1xmmc_suspend,
> };
>
> but in this particular case it's probably better to define separate
> callbacks for .freeze() and .thaw() at least.
>
> During hibernation we call .freeze() and .thaw() before and after
> creating the image, respectively, and then .poweroff() is called right
> after the image has been saved. ?During resume .freeze() is called
> after the image has been loaded and before the control goes to the
> image kernel, which then calls .restore().
Yes, I see that in drivers/base/platform.c (legacy) .suspend resp. .resume
also got called for those cases?
Ouch :-(
I've added others who've submitted dev_pm_ops patches in CC.
> I'll fix up the floppy and hp-wmi patches.
Note that those are already in mainline, as is pcspkr.
Thanks,
FJP
On Saturday 25 July 2009, you wrote:
> > I'll fix up the floppy and hp-wmi patches.
>
> Note that those are already in mainline, as is pcspkr.
Here's an overview of commits (that I could find) already in mainline that
look to have gotten it wrong:
6daa79b3 drivers/serial/sh-sci.c (Paul Mundt)
2dbc8a23 drivers/video/hitfb.c (Paul Mundt)
35db715b drivers/input/misc/pcspkr.c (/me)
2702403c drivers/platform/x86/hp-wmi.c (/me)
142a735b drivers/block/floppy.c (/me)
On Saturday 25 July 2009, Frans Pop wrote:
> On Saturday 25 July 2009, you wrote:
> > > I'll fix up the floppy and hp-wmi patches.
> >
> > Note that those are already in mainline, as is pcspkr.
>
> Here's an overview of commits (that I could find) already in mainline that
> look to have gotten it wrong:
>
> 6daa79b3 drivers/serial/sh-sci.c (Paul Mundt)
> 2dbc8a23 drivers/video/hitfb.c (Paul Mundt)
> 35db715b drivers/input/misc/pcspkr.c (/me)
> 2702403c drivers/platform/x86/hp-wmi.c (/me)
> 142a735b drivers/block/floppy.c (/me)
Are you sure they're all in mainline?
The last change in mainline floppy.c happened on Jun 30 and I have the last
two (fixed up already) in my linux-next branch.
Best,
Rafael
On Saturday 25 July 2009, Rafael J. Wysocki wrote:
> On Saturday 25 July 2009, Frans Pop wrote:
> > On Saturday 25 July 2009, you wrote:
> > > > I'll fix up the floppy and hp-wmi patches.
> > >
> > > Note that those are already in mainline, as is pcspkr.
> >
> > Here's an overview of commits (that I could find) already in mainline
> > that look to have gotten it wrong:
> >
> > 6daa79b3 drivers/serial/sh-sci.c (Paul Mundt)
> > 2dbc8a23 drivers/video/hitfb.c (Paul Mundt)
> > 35db715b drivers/input/misc/pcspkr.c (/me)
> > 2702403c drivers/platform/x86/hp-wmi.c (/me)
> > 142a735b drivers/block/floppy.c (/me)
>
> Are you sure they're all in mainline?
Ugh, you're right. Only the first three are.
For the last two I was confused by the mails from akm that he'd dropped
them from his queue. I'd not realized that was because you took them.
On Jul 25, 2009, at 1:21 PM, Frans Pop <[email protected]> wrote:
> On Saturday 25 July 2009, Rafael J. Wysocki wrote:
>>>>> Was suspend to disk tested? It requires freeze()/thaw().
>>>>
>>>> Is that a regression introduced by this patch then? If so, many
>>>> more of the recent dev_pm_ops conversion patches would need to be
>>>> revisited.
>>
>> Yes, they would. In general, you'd probably want to do something
>> like
>> this:
>>
>> static struct dev_pm_ops au1xmmc_pmops = {
>> .resume = au1xmmc_resume,
>> .suspend = au1xmmc_suspend,
>> .freeze = au1xmmc_resume,
>> .thaw = au1xmmc_suspend,
>> .restore = au1xmmc_resume,
>> .poweroff = au1xmmc_suspend,
>> };
>>
>> but in this particular case it's probably better to define separate
>> callbacks for .freeze() and .thaw() at least.
>>
>> During hibernation we call .freeze() and .thaw() before and after
>> creating the image, respectively, and then .poweroff() is called
>> right
>> after the image has been saved. During resume .freeze() is called
>> after the image has been loaded and before the control goes to the
>> image kernel, which then calls .restore().
>
> Yes, I see that in drivers/base/platform.c (legacy) .suspend
> resp. .resume
> also got called for those cases?
> Ouch :-(
>
> I've added others who've submitted dev_pm_ops patches in CC.
>
>> I'll fix up the floppy and hp-wmi patches.
>
> Note that those are already in mainline, as is pcspkr.
>
Pcspkr should be fine as is since we just want to shut it off and with
s2d it will happen automatically when we power down.
--
Dmitry
On Sat, Jul 25, 2009 at 09:39:30PM +0200, Rafael J. Wysocki wrote:
> Yes, they would. In general, you'd probably want to do something like this:
> static struct dev_pm_ops au1xmmc_pmops = {
> .resume = au1xmmc_resume,
> .suspend = au1xmmc_suspend,
> .freeze = au1xmmc_resume,
> .thaw = au1xmmc_suspend,
I'd have expected freeze and thaw to be the other way around here?
On Sunday 26 July 2009, Mark Brown wrote:
> On Sat, Jul 25, 2009 at 09:39:30PM +0200, Rafael J. Wysocki wrote:
>
> > Yes, they would. In general, you'd probably want to do something like this:
>
> > static struct dev_pm_ops au1xmmc_pmops = {
> > .resume = au1xmmc_resume,
> > .suspend = au1xmmc_suspend,
> > .freeze = au1xmmc_resume,
> > .thaw = au1xmmc_suspend,
>
> I'd have expected freeze and thaw to be the other way around here?
Sure, sorry.
.suspend() corresponds to .freeze() and .poweroff(), while .resume()
corresponds to .thaw() and .restore().
Best,
Rafael