2013-03-11 17:43:10

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 1/4] tty: max3100: Use dev_pm_ops

Use dev_pm_ops instead of the deprecated legacy suspend/resume for the
max3100 driver.

Signed-off-by: Lars-Peter Clausen <[email protected]>
---
drivers/tty/serial/max3100.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/tty/serial/max3100.c b/drivers/tty/serial/max3100.c
index 32517d4..57da9bb 100644
--- a/drivers/tty/serial/max3100.c
+++ b/drivers/tty/serial/max3100.c
@@ -849,11 +849,11 @@ static int max3100_remove(struct spi_device *spi)
return 0;
}

-#ifdef CONFIG_PM
+#ifdef CONFIG_PM_SLEEP

-static int max3100_suspend(struct spi_device *spi, pm_message_t state)
+static int max3100_suspend(struct device *dev)
{
- struct max3100_port *s = dev_get_drvdata(&spi->dev);
+ struct max3100_port *s = dev_get_drvdata(dev);

dev_dbg(&s->spi->dev, "%s\n", __func__);

@@ -874,9 +874,9 @@ static int max3100_suspend(struct spi_device *spi, pm_message_t state)
return 0;
}

-static int max3100_resume(struct spi_device *spi)
+static int max3100_resume(struct device *dev)
{
- struct max3100_port *s = dev_get_drvdata(&spi->dev);
+ struct max3100_port *s = dev_get_drvdata(dev);

dev_dbg(&s->spi->dev, "%s\n", __func__);

@@ -894,21 +894,21 @@ static int max3100_resume(struct spi_device *spi)
return 0;
}

+static SIMPLE_DEV_PM_OPS(max3100_pm_ops, max3100_suspend, max3100_resume);
+#define MAX3100_PM_OPS (&max3100_pm_ops)
+
#else
-#define max3100_suspend NULL
-#define max3100_resume NULL
+#define MAX3100_PM_OPS NULL
#endif

static struct spi_driver max3100_driver = {
.driver = {
.name = "max3100",
.owner = THIS_MODULE,
+ .pm = MAX3100_PM_OPS,
},
-
.probe = max3100_probe,
.remove = max3100_remove,
- .suspend = max3100_suspend,
- .resume = max3100_resume,
};

module_spi_driver(max3100_driver);
--
1.8.0


2013-03-11 17:43:11

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 2/4] tty: max310x: Use dev_pm_ops

Use dev_pm_ops instead of the deprecated legacy suspend/resume for the
max310x driver.

Cc: Alexander Shiyan <[email protected]>
Signed-off-by: Lars-Peter Clausen <[email protected]>
---
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,
},
.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

2013-03-11 17:43:16

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 4/4] tty: ifx6x60: Remove unused suspend/resume callbacks

The ifx6x60 driver implements both legacy suspend/resume callbacks and
dev_pm_ops. The SPI core is going to ignore legacy suspend/resume
callbacks if a driver implements dev_pm_ops. Since the legacy suspend/resume
callbacks are empty in this case it is safe to just remove them.

Cc: Bi Chao <[email protected]>
Cc: Chen Jun <[email protected]>
Signed-off-by: Lars-Peter Clausen <[email protected]>
---
drivers/tty/serial/ifx6x60.c | 26 --------------------------
1 file changed, 26 deletions(-)

diff --git a/drivers/tty/serial/ifx6x60.c b/drivers/tty/serial/ifx6x60.c
index 68d7ce9..ae63c14 100644
--- a/drivers/tty/serial/ifx6x60.c
+++ b/drivers/tty/serial/ifx6x60.c
@@ -1325,30 +1325,6 @@ static void ifx_spi_spi_shutdown(struct spi_device *spi)
*/

/**
- * ifx_spi_spi_suspend - suspend SPI on system suspend
- * @dev: device being suspended
- *
- * Suspend the SPI side. No action needed on Intel MID platforms, may
- * need extending for other systems.
- */
-static int ifx_spi_spi_suspend(struct spi_device *spi, pm_message_t msg)
-{
- return 0;
-}
-
-/**
- * ifx_spi_spi_resume - resume SPI side on system resume
- * @dev: device being suspended
- *
- * Suspend the SPI side. No action needed on Intel MID platforms, may
- * need extending for other systems.
- */
-static int ifx_spi_spi_resume(struct spi_device *spi)
-{
- return 0;
-}
-
-/**
* ifx_spi_pm_suspend - suspend modem on system suspend
* @dev: device being suspended
*
@@ -1437,8 +1413,6 @@ static struct spi_driver ifx_spi_driver = {
.probe = ifx_spi_spi_probe,
.shutdown = ifx_spi_spi_shutdown,
.remove = ifx_spi_spi_remove,
- .suspend = ifx_spi_spi_suspend,
- .resume = ifx_spi_spi_resume,
.id_table = ifx_id_table
};

--
1.8.0

2013-03-11 17:44:00

by Lars-Peter Clausen

[permalink] [raw]
Subject: [PATCH 3/4] tty: mrst_max3110: Use dev_pm_ops

Use dev_pm_ops instead of the deprecated legacy suspend/resume for the
mrst_max3110 driver.

Cc: Alan Cox <[email protected]>
Signed-off-by: Lars-Peter Clausen <[email protected]>
---
drivers/tty/serial/mrst_max3110.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/tty/serial/mrst_max3110.c b/drivers/tty/serial/mrst_max3110.c
index f641c23..9b6ef20 100644
--- a/drivers/tty/serial/mrst_max3110.c
+++ b/drivers/tty/serial/mrst_max3110.c
@@ -743,9 +743,10 @@ static struct uart_driver serial_m3110_reg = {
.cons = &serial_m3110_console,
};

-#ifdef CONFIG_PM
-static int serial_m3110_suspend(struct spi_device *spi, pm_message_t state)
+#ifdef CONFIG_PM_SLEEP
+static int serial_m3110_suspend(struct device *dev)
{
+ struct spi_device *spi = to_spi_device(dev);
struct uart_max3110 *max = spi_get_drvdata(spi);

disable_irq(max->irq);
@@ -754,8 +755,9 @@ static int serial_m3110_suspend(struct spi_device *spi, pm_message_t state)
return 0;
}

-static int serial_m3110_resume(struct spi_device *spi)
+static int serial_m3110_resume(struct device *dev)
{
+ struct spi_device *spi = to_spi_device(dev);
struct uart_max3110 *max = spi_get_drvdata(spi);

max3110_out(max, max->cur_conf);
@@ -763,9 +765,13 @@ static int serial_m3110_resume(struct spi_device *spi)
enable_irq(max->irq);
return 0;
}
+
+static SIMPLE_DEV_PM_OPS(serial_m3110_pm_ops, serial_m3110_suspend,
+ serial_m3110_resume);
+#define SERIAL_M3110_PM_OPS (&serial_m3110_pm_ops)
+
#else
-#define serial_m3110_suspend NULL
-#define serial_m3110_resume NULL
+#define SERIAL_M3110_PM_OPS NULL
#endif

static int serial_m3110_probe(struct spi_device *spi)
@@ -872,11 +878,10 @@ static struct spi_driver uart_max3110_driver = {
.driver = {
.name = "spi_max3111",
.owner = THIS_MODULE,
+ .pm = SERIAL_M3110_PM_OPS,
},
.probe = serial_m3110_probe,
.remove = serial_m3110_remove,
- .suspend = serial_m3110_suspend,
- .resume = serial_m3110_resume,
};

static int __init serial_m3110_init(void)
--
1.8.0

2013-03-11 18:10:37

by Alexander Shiyan

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

Hello.

> Use dev_pm_ops instead of the deprecated legacy suspend/resume for the
> max310x driver.
>
> Cc: Alexander Shiyan <[email protected]>
> Signed-off-by: Lars-Peter Clausen <[email protected]>
> ---
> 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.

> },
> .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?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-03-11 18:32:49

by Lars-Peter Clausen

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

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 <[email protected]>
>> Signed-off-by: Lars-Peter Clausen <[email protected]>
>> ---
>> 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
warnings from your compiler about unused functions.

>
>> },
>> .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
>
> ---

2013-03-11 18:41:47

by Alexander Shiyan

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

> 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 <[email protected]>
> >> Signed-off-by: Lars-Peter Clausen <[email protected]>
> >> ---
> >> 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.

> warnings from your compiler about unused functions.
I think attribute "__maybe_unused" can help here.

> >> },
> >> .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?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-03-11 18:47:40

by Lars-Peter Clausen

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

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 <[email protected]>
>>>> Signed-off-by: Lars-Peter Clausen <[email protected]>
>>>> ---
>>>> 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?

>
>> 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
>>>
>>> ---
>
> ---

2013-03-11 18:54:51

by Alexander Shiyan

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

> 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 <[email protected]>
> >>>> Signed-off-by: Lars-Peter Clausen <[email protected]>
> >>>> ---
> >>>> 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?+???????+%?????ݶ??w??{.n?+????{??G?????{ay?ʇڙ?,j??f???h?????????z_??(?階?ݢj"???m??????G????????????&???~???iO???z??v?^?m???? ????????I?

2013-03-11 19:07:16

by Lars-Peter Clausen

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

On 03/11/2013 07:54 PM, Alexander Shiyan wrote:
>> 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 <[email protected]>
>>>>>> Signed-off-by: Lars-Peter Clausen <[email protected]>
>>>>>> ---
>>>>>> 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.

Hm, ok that actually makes sense. But one of the reasons why you'd want to
disable suspend support is because you don't need it and want to save some
memory. Quite often the dev_pm_ops struct is about the same order of size than
the actually suspend/resume code. So even though you disable suspend support
the memory is still used.

>
>>
>>>
>>>> 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
>
> ---