Re[2]: [PATCH 2/4] tty: max310x: Use dev_pm_ops

[Alexander Shiyan]

> On 03/11/2013 07:41 PM, Alexander Shiyan wrote:
> >> On 03/11/2013 07:10 PM, Alexander Shiyan wrote:
> >>> Hello.
> >>>
> >>>> Use dev_pm_ops instead of the deprecated legacy suspend/resume for the
> >>>> max310x driver.
> >>>>
> >>>> Cc: Alexander Shiyan <shc_work@xxxxxxx>
> >>>> Signed-off-by: Lars-Peter Clausen <lars@xxxxxxxxxx>
> >>>> ---
> >>>>  drivers/tty/serial/max310x.c | 24 ++++++++++++++++--------
> >>>>  1 file changed, 16 insertions(+), 8 deletions(-)
> >>>>
> >>>> diff --git a/drivers/tty/serial/max310x.c b/drivers/tty/serial/max310x.c
> >>>> index 0c2422c..8941e64 100644
> >>>> --- a/drivers/tty/serial/max310x.c
> >>>> +++ b/drivers/tty/serial/max310x.c
> >>>> @@ -881,12 +881,14 @@ static struct uart_ops max310x_ops = {
> >>>>  	.verify_port	= max310x_verify_port,
> >>>>  };
> >>>>  
> >>>> -static int max310x_suspend(struct spi_device *spi, pm_message_t state)
> >>>> +#ifdef CONFIG_PM_SLEEP
> >>>> +
> >>>> +static int max310x_suspend(struct device *dev)
> >>>>  {
> >>>>  	int ret;
> >>>> -	struct max310x_port *s = dev_get_drvdata(&spi->dev);
> >>>> +	struct max310x_port *s = dev_get_drvdata(dev);
> >>>>  
> >>>> -	dev_dbg(&spi->dev, "Suspend\n");
> >>>> +	dev_dbg(dev, "Suspend\n");
> >>>>  
> >>>>  	ret = uart_suspend_port(&s->uart, &s->port);
> >>>>  
> >>>> @@ -905,11 +907,11 @@ static int max310x_suspend(struct spi_device *spi, pm_message_t state)
> >>>>  	return ret;
> >>>>  }
> >>>>  
> >>>> -static int max310x_resume(struct spi_device *spi)
> >>>> +static int max310x_resume(struct device *dev)
> >>>>  {
> >>>> -	struct max310x_port *s = dev_get_drvdata(&spi->dev);
> >>>> +	struct max310x_port *s = dev_get_drvdata(dev);
> >>>>  
> >>>> -	dev_dbg(&spi->dev, "Resume\n");
> >>>> +	dev_dbg(dev, "Resume\n");
> >>>>  
> >>>>  	if (s->pdata->suspend)
> >>>>  		s->pdata->suspend(0);
> >>>> @@ -928,6 +930,13 @@ static int max310x_resume(struct spi_device *spi)
> >>>>  	return uart_resume_port(&s->uart, &s->port);
> >>>>  }
> >>>>  
> >>>> +static SIMPLE_DEV_PM_OPS(max310x_pm_ops, max310x_suspend, max310x_resume);
> >>>> +#define MAX310X_PM_OPS (&max310x_pm_ops)
> >>>> +
> >>>> +#else
> >>>> +#define MAX310X_PM_OPS NULL
> >>>> +#endif
> >>>> +
> >>>>  #ifdef CONFIG_GPIOLIB
> >>>>  static int max310x_gpio_get(struct gpio_chip *chip, unsigned offset)
> >>>>  {
> >>>> @@ -1242,11 +1251,10 @@ static struct spi_driver max310x_driver = {
> >>>>  	.driver = {
> >>>>  		.name	= "max310x",
> >>>>  		.owner	= THIS_MODULE,
> >>>> +		.pm	= MAX310X_PM_OPS,
> >>>
> >>> Check for CONFIG_PM_SLEEP not necessary at all.
> >>> <linux/pm.h> will do all for us.
> >>
> >> No it wont, you'll end up with a dev_pm_ops struct full of zeros and two
> > I.e. NULL, it is OK.
> 
> But what's the point of keeping it around?

This allows you to keep checking the code at compile time,
as well as macro IS_ENABLED() inside the code.
#ifdef does not allow this.

> 
> > 
> >> warnings from your compiler about unused functions.
> > I think attribute "__maybe_unused" can help here.
> 
> Or a #ifdef
> 
> > 
> >>>>  	},
> >>>>  	.probe		= max310x_probe,
> >>>>  	.remove		= max310x_remove,
> >>>> -	.suspend	= max310x_suspend,
> >>>> -	.resume		= max310x_resume,
> >>>>  	.id_table	= max310x_id_table,
> >>>>  };
> >>>>  module_spi_driver(max310x_driver);
> >>>> -- 
> >>>> 1.8.0

---
èº{.nÇ+‰·Ÿ®‰­†+%ŠËlzwm…ébëæìr¸›zX§»®w¥Š{ayºÊÚë,j­¢f£¢·hš‹àz¹®w¥¢¸¢·¦j:+v‰¨ŠwèjØm¶Ÿÿ¾«‘êçzZ+ƒùšŽŠÝj"ú!¶iO•æ¬z·švØ^¶m§ÿðÃnÆàþY&—