Good Morning Frans,
On Mon, Jul 20, 2009 at 10:00 PM, Frans Pop<elendil@xxxxxxxxx> wrote:
>> Signed-off-by: Manuel Lauss <manuel.lauss@xxxxxxxxx>
>> ---
>> Run-tested on Au1200.
>>
>> drivers/mmc/host/au1xmmc.c | 23 +++++++++++------------
>> 1 files changed, 11 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/mmc/host/au1xmmc.c b/drivers/mmc/host/au1xmmc.c
>> index d3f5561..70509c9 100644
>> --- a/drivers/mmc/host/au1xmmc.c
>> +++ b/drivers/mmc/host/au1xmmc.c
>> @@ -1131,13 +1131,12 @@ static int __devexit au1xmmc_remove(struct
>> platform_device *pdev) return 0;
>> }
>>
>> -#ifdef CONFIG_PM
>
> Won't the removal of this test cause a build failure if CONFIG_PM is not
> set? If the removal of the test is safe, this should IMHO at least be
> explained in the commit message.
No, it builds just fine without CONFIG_PM; it was there to shave off a
few bytes from the kernel image. But not everyone tests this driver
with CONFIG_PM=y, because apparently noone really needed PM on
this platform (Alchemy), and a full build of most of the boards using this
driver fails with PM enabled.
This way the PM methods at least get a compile-test in the non-pm case.
>> -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 +1149,27 @@ 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);
>> }
>> -#else
>> -#define au1xmmc_suspend NULL
>> -#define au1xmmc_resume NULL
>
> ... drop the 3 lines above (as you did) ...
>
>> -#endif
>
> ... move this #endif to after the new struct below ...
>
>> +
>> +static struct dev_pm_ops au1xmmc_pmops = {
>> + .resume = au1xmmc_resume,
>> + .suspend = au1xmmc_suspend,
>> +};
>>
>> 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,
I like what Magnus Damm did for some of the SuperH drivers:
#ifdef CONFIG_PM
[...]
#define DRIVER_PM_OPS (&driver_pm_ops)
#else
#define DRIVER_PM_OPS NULL
#endif
...
.driver = {
...
.pm = DRIVER_PM_OPS,
}
...
I'd like to keep the pm stuff enabled at all times since it doesn't hurt
in the non-pm case and if kernel size becomes a problem I can add the
#defines back.
What do you think?
Thank you!
Manuel Lauss
