[PATCH] Switch atmel_serial to dev_pm_ops

[PATCH] Switch atmel_serial to dev_pm_ops

[Albin Tonnerre]

Signed-off-by: Albin Tonnerre <albin.tonnerre@free-electrons.com>
---
 drivers/serial/atmel_serial.c |   27 +++++++++++++++------------
 1 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/serial/atmel_serial.c b/drivers/serial/atmel_serial.c
index 607d43a..3546ead 100644
--- a/drivers/serial/atmel_serial.c
+++ b/drivers/serial/atmel_serial.c
@@ -38,6 +38,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/atmel_pdc.h>
 #include <linux/atmel_serial.h>
+#include <linux/pm.h>
 
 #include <asm/io.h>
 
@@ -1488,10 +1489,9 @@ static bool atmel_serial_clk_will_stop(void)
 #endif
 }
 
-static int atmel_serial_suspend(struct platform_device *pdev,
-				pm_message_t state)
+static int atmel_serial_suspend(struct device *dev)
 {
-	struct uart_port *port = platform_get_drvdata(pdev);
+	struct uart_port *port = dev_get_drvdata(dev);
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 
 	if (atmel_is_console_port(port) && console_suspend_enabled) {
@@ -1501,28 +1501,32 @@ static int atmel_serial_suspend(struct platform_device *pdev,
 	}
 
 	/* we can not wake up if we're running on slow clock */
-	atmel_port->may_wakeup = device_may_wakeup(&pdev->dev);
+	atmel_port->may_wakeup = device_may_wakeup(dev);
 	if (atmel_serial_clk_will_stop())
-		device_set_wakeup_enable(&pdev->dev, 0);
+		device_set_wakeup_enable(dev, 0);
 
 	uart_suspend_port(&atmel_uart, port);
 
 	return 0;
 }
 
-static int atmel_serial_resume(struct platform_device *pdev)
+static int atmel_serial_resume(struct device *dev)
 {
-	struct uart_port *port = platform_get_drvdata(pdev);
+	struct uart_port *port = dev_get_drvdata(dev);
 	struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
 
 	uart_resume_port(&atmel_uart, port);
-	device_set_wakeup_enable(&pdev->dev, atmel_port->may_wakeup);
+	device_set_wakeup_enable(dev, atmel_port->may_wakeup);
 
 	return 0;
 }
+
+static struct dev_pm_ops atmel_serial_pm_ops = {
+	.suspend = atmel_serial_suspend,
+	.resume = atmel_serial_resume,
+};
 #else
-#define atmel_serial_suspend NULL
-#define atmel_serial_resume NULL
+static struct dev_pm_ops atmel_serial_pm_ops;
 #endif
 
 static int __devinit atmel_serial_probe(struct platform_device *pdev)
@@ -1603,11 +1607,10 @@ static int __devexit atmel_serial_remove(struct platform_device *pdev)
 static struct platform_driver atmel_serial_driver = {
 	.probe		= atmel_serial_probe,
 	.remove		= __devexit_p(atmel_serial_remove),
-	.suspend	= atmel_serial_suspend,
-	.resume		= atmel_serial_resume,
 	.driver		= {
 		.name	= "atmel_usart",
 		.owner	= THIS_MODULE,
+		.pm = &atmel_serial_pm_ops,
 	},
 };
 
-- 
1.6.3.3

Re: [PATCH] Switch atmel_serial to dev_pm_ops

[Haavard Skinnemoen]

Albin Tonnerre wrote:
> +static struct dev_pm_ops atmel_serial_pm_ops = {
> +	.suspend = atmel_serial_suspend,
> +	.resume = atmel_serial_resume,
> +};
>  #else
> -#define atmel_serial_suspend NULL
> -#define atmel_serial_resume NULL
> +static struct dev_pm_ops atmel_serial_pm_ops;

Why not

#define atmel_serial_pm_ops	NULL

>  #endif
>  
>  static int __devinit atmel_serial_probe(struct platform_device *pdev)
> @@ -1603,11 +1607,10 @@ static int __devexit atmel_serial_remove(struct platform_device *pdev)
>  static struct platform_driver atmel_serial_driver = {
>  	.probe		= atmel_serial_probe,
>  	.remove		= __devexit_p(atmel_serial_remove),
> -	.suspend	= atmel_serial_suspend,
> -	.resume		= atmel_serial_resume,
>  	.driver		= {
>  		.name	= "atmel_usart",
>  		.owner	= THIS_MODULE,
> +		.pm = &atmel_serial_pm_ops,

Please use a TAB instead of space to make everything nice and
consistent.

Other than that,

Acked-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>

Re: [PATCH] Switch atmel_serial to dev_pm_ops

[Albin Tonnerre]

On Mon, Aug 03, 2009 at 05:09:47PM +0200, Haavard Skinnemoen wrote :
> Albin Tonnerre wrote:
> > +static struct dev_pm_ops atmel_serial_pm_ops = {
> > +	.suspend = atmel_serial_suspend,
> > +	.resume = atmel_serial_resume,
> > +};
> >  #else
> > -#define atmel_serial_suspend NULL
> > -#define atmel_serial_resume NULL
> > +static struct dev_pm_ops atmel_serial_pm_ops;

> Why not

> #define atmel_serial_pm_ops	NULL

Because the .pm field is set to the address of atmel_serial_pm_ops, and iirc
taking the address of NULL is not allowed.

> >  #endif

> >  static int __devinit atmel_serial_probe(struct platform_device *pdev)
> > @@ -1603,11 +1607,10 @@ static int __devexit atmel_serial_remove(struct platform_device *pdev)
> >  static struct platform_driver atmel_serial_driver = {
> >  	.probe		= atmel_serial_probe,
> >  	.remove		= __devexit_p(atmel_serial_remove),
> > -	.suspend	= atmel_serial_suspend,
> > -	.resume		= atmel_serial_resume,
> >  	.driver		= {
> >  		.name	= "atmel_usart",
> >  		.owner	= THIS_MODULE,
> > +		.pm = &atmel_serial_pm_ops,

> Please use a TAB instead of space to make everything nice and
> consistent.

That's weird, because my editor does show a TAB there, and applying the patch
with git am results in a TAB.

Regards,
-- 
Albin Tonnerre, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

Re: [PATCH] Switch atmel_serial to dev_pm_ops

[Frans Pop]

Haavard Skinnemoen wrote:
> Other than that,
> Acked-by: Haavard Skinnemoen <haavard.skinnemoen@atmel.com>

Just to make sure...

Does this driver need to support hibernation (suspend to disk)? If it 
does, the patch is incomplete; see http://lkml.org/lkml/2009/7/25/118.

Cheers,
FJP

Re: [PATCH] Switch atmel_serial to dev_pm_ops

[Haavard Skinnemoen]

Albin Tonnerre wrote:
> On Mon, Aug 03, 2009 at 05:09:47PM +0200, Haavard Skinnemoen wrote :
> > Why not
> 
> > #define atmel_serial_pm_ops	NULL
> 
> Because the .pm field is set to the address of atmel_serial_pm_ops, and iirc
> taking the address of NULL is not allowed.

Good point.

> > >  		.name	= "atmel_usart",
> > >  		.owner	= THIS_MODULE,
> > > +		.pm = &atmel_serial_pm_ops,
> 
> > Please use a TAB instead of space to make everything nice and
> > consistent.
> 
> That's weird, because my editor does show a TAB there, and applying the patch
> with git am results in a TAB.

Hmm, it doesn't show a TAB when I apply it here...

Oh well, it's not a big deal either way.

Haavard

Re: [PATCH] Switch atmel_serial to dev_pm_ops

[Frans Pop]

Albin Tonnerre wrote:
>> Why not
> 
>> #define atmel_serial_pm_ops   NULL
> 
> Because the .pm field is set to the address of atmel_serial_pm_ops, and
> iirc taking the address of NULL is not allowed.

The following construction is possible however (first used by Magnus Damm 
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,
      }
...

Cheers,
FJP