* [PATCH 2/2] gpio: mpc8xxx: switch to using DEFINE_RUNTIME_DEV_PM_OPS()
2024-09-03 15:45 [PATCH 1/2] gpio: mpc8xxx: order headers alphabetically Bartosz Golaszewski
@ 2024-09-03 15:45 ` Bartosz Golaszewski
2024-09-03 16:23 ` Andy Shevchenko
2024-09-03 16:20 ` [PATCH 1/2] gpio: mpc8xxx: order headers alphabetically Andy Shevchenko
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Bartosz Golaszewski @ 2024-09-03 15:45 UTC (permalink / raw)
To: Linus Walleij, Andy Shevchenko, Martyn Welch
Cc: linux-gpio, linux-kernel, Bartosz Golaszewski
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
Use the preferred API for assigning system sleep pm callbacks in drivers.
Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
---
drivers/gpio/gpio-mpc8xxx.c | 24 +++++++++++-------------
1 file changed, 11 insertions(+), 13 deletions(-)
diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-mpc8xxx.c
index e084e08f54387..edb228d9af277 100644
--- a/drivers/gpio/gpio-mpc8xxx.c
+++ b/drivers/gpio/gpio-mpc8xxx.c
@@ -17,6 +17,7 @@
#include <linux/mod_devicetable.h>
#include <linux/of.h>
#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
#include <linux/property.h>
#include <linux/slab.h>
#include <linux/spinlock.h>
@@ -431,30 +432,28 @@ static void mpc8xxx_remove(struct platform_device *pdev)
}
}
-#ifdef CONFIG_PM
-static int mpc8xxx_suspend(struct platform_device *pdev, pm_message_t state)
+static int mpc8xxx_suspend(struct device *dev)
{
- struct mpc8xxx_gpio_chip *mpc8xxx_gc = platform_get_drvdata(pdev);
+ struct mpc8xxx_gpio_chip *mpc8xxx_gc = dev_get_drvdata(dev);
- if (mpc8xxx_gc->irqn && device_may_wakeup(&pdev->dev))
+ if (mpc8xxx_gc->irqn && device_may_wakeup(dev))
enable_irq_wake(mpc8xxx_gc->irqn);
return 0;
}
-static int mpc8xxx_resume(struct platform_device *pdev)
+static int mpc8xxx_resume(struct device *dev)
{
- struct mpc8xxx_gpio_chip *mpc8xxx_gc = platform_get_drvdata(pdev);
+ struct mpc8xxx_gpio_chip *mpc8xxx_gc = dev_get_drvdata(dev);
- if (mpc8xxx_gc->irqn && device_may_wakeup(&pdev->dev))
+ if (mpc8xxx_gc->irqn && device_may_wakeup(dev))
disable_irq_wake(mpc8xxx_gc->irqn);
return 0;
}
-#else
-#define mpc8xxx_suspend NULL
-#define mpc8xxx_resume NULL
-#endif
+
+static DEFINE_RUNTIME_DEV_PM_OPS(mpc8xx_pm_ops, mpc8xxx_suspend,
+ mpc8xxx_resume, NULL);
#ifdef CONFIG_ACPI
static const struct acpi_device_id gpio_acpi_ids[] = {
@@ -467,12 +466,11 @@ MODULE_DEVICE_TABLE(acpi, gpio_acpi_ids);
static struct platform_driver mpc8xxx_plat_driver = {
.probe = mpc8xxx_probe,
.remove_new = mpc8xxx_remove,
- .suspend = mpc8xxx_suspend,
- .resume = mpc8xxx_resume,
.driver = {
.name = "gpio-mpc8xxx",
.of_match_table = mpc8xxx_gpio_ids,
.acpi_match_table = ACPI_PTR(gpio_acpi_ids),
+ .pm = pm_sleep_ptr(&mpc8xx_pm_ops),
},
};
--
2.43.0
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH 2/2] gpio: mpc8xxx: switch to using DEFINE_RUNTIME_DEV_PM_OPS()
2024-09-03 15:45 ` [PATCH 2/2] gpio: mpc8xxx: switch to using DEFINE_RUNTIME_DEV_PM_OPS() Bartosz Golaszewski
@ 2024-09-03 16:23 ` Andy Shevchenko
2024-09-03 16:25 ` Andy Shevchenko
0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2024-09-03 16:23 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Martyn Welch, linux-gpio, linux-kernel,
Bartosz Golaszewski
On Tue, Sep 03, 2024 at 05:45:33PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Use the preferred API for assigning system sleep pm callbacks in drivers.
>
> Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Hmm... Maybe I should pay more attention when answering emails.
Please, use my @linux.intel.com address for Linux kernel contributions.
...
> #include <linux/mod_devicetable.h>
> #include <linux/of.h>
> #include <linux/platform_device.h>
> +#include <linux/pm_runtime.h>
You need pm.h as macros defined there.
> #include <linux/property.h>
> #include <linux/slab.h>
> #include <linux/spinlock.h>
...
> +static DEFINE_RUNTIME_DEV_PM_OPS(mpc8xx_pm_ops, mpc8xxx_suspend,
> + mpc8xxx_resume, NULL);
I would split logically, i.e.
static DEFINE_RUNTIME_DEV_PM_OPS(mpc8xx_pm_ops,
mpc8xxx_suspend, mpc8xxx_resume, NULL);
OR
static DEFINE_RUNTIME_DEV_PM_OPS(mpc8xx_pm_ops,
mpc8xxx_suspend, mpc8xxx_resume,
NULL);
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] gpio: mpc8xxx: switch to using DEFINE_RUNTIME_DEV_PM_OPS()
2024-09-03 16:23 ` Andy Shevchenko
@ 2024-09-03 16:25 ` Andy Shevchenko
2024-09-03 18:36 ` Bartosz Golaszewski
0 siblings, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2024-09-03 16:25 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Martyn Welch, linux-gpio, linux-kernel,
Bartosz Golaszewski
On Tue, Sep 03, 2024 at 07:23:31PM +0300, Andy Shevchenko wrote:
> On Tue, Sep 03, 2024 at 05:45:33PM +0200, Bartosz Golaszewski wrote:
...
> > #include <linux/mod_devicetable.h>
> > #include <linux/of.h>
> > #include <linux/platform_device.h>
> > +#include <linux/pm_runtime.h>
>
> You need pm.h as macros defined there.
...or both...
> > #include <linux/property.h>
> > #include <linux/slab.h>
> > #include <linux/spinlock.h>
...
> > +static DEFINE_RUNTIME_DEV_PM_OPS(mpc8xx_pm_ops, mpc8xxx_suspend,
> > + mpc8xxx_resume, NULL);
This one comes from pm_runtime.h, but pm*_ptr() ones from pm.h.
And it seems you wanted pm_ptr().
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] gpio: mpc8xxx: switch to using DEFINE_RUNTIME_DEV_PM_OPS()
2024-09-03 16:25 ` Andy Shevchenko
@ 2024-09-03 18:36 ` Bartosz Golaszewski
2024-09-04 11:40 ` Martyn Welch
0 siblings, 1 reply; 9+ messages in thread
From: Bartosz Golaszewski @ 2024-09-03 18:36 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Bartosz Golaszewski, Linus Walleij, Martyn Welch, linux-gpio,
linux-kernel
On Tue, 3 Sept 2024 at 18:25, Andy Shevchenko
<andriy.shevchenko@intel.com> wrote:
>
> On Tue, Sep 03, 2024 at 07:23:31PM +0300, Andy Shevchenko wrote:
> > On Tue, Sep 03, 2024 at 05:45:33PM +0200, Bartosz Golaszewski wrote:
>
> ...
>
> > > #include <linux/mod_devicetable.h>
> > > #include <linux/of.h>
> > > #include <linux/platform_device.h>
> > > +#include <linux/pm_runtime.h>
> >
> > You need pm.h as macros defined there.
>
> ...or both...
>
pm_runtime.h implies pm.h.
> > > #include <linux/property.h>
> > > #include <linux/slab.h>
> > > #include <linux/spinlock.h>
>
> ...
>
> > > +static DEFINE_RUNTIME_DEV_PM_OPS(mpc8xx_pm_ops, mpc8xxx_suspend,
> > > + mpc8xxx_resume, NULL);
>
> This one comes from pm_runtime.h, but pm*_ptr() ones from pm.h.
>
> And it seems you wanted pm_ptr().
>
Yeah, I'm not sure really. The suspend and resume callbacks for
platform devices are not documented but it looks like they're only
used for system sleep. Martyn: which one do we actually need?
Bart
> --
> With Best Regards,
> Andy Shevchenko
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] gpio: mpc8xxx: switch to using DEFINE_RUNTIME_DEV_PM_OPS()
2024-09-03 18:36 ` Bartosz Golaszewski
@ 2024-09-04 11:40 ` Martyn Welch
0 siblings, 0 replies; 9+ messages in thread
From: Martyn Welch @ 2024-09-04 11:40 UTC (permalink / raw)
To: Bartosz Golaszewski, Andy Shevchenko
Cc: Bartosz Golaszewski, Linus Walleij, linux-gpio, linux-kernel
On Tue, 2024-09-03 at 20:36 +0200, Bartosz Golaszewski wrote:
> On Tue, 3 Sept 2024 at 18:25, Andy Shevchenko
> <andriy.shevchenko@intel.com> wrote:
> >
> > On Tue, Sep 03, 2024 at 07:23:31PM +0300, Andy Shevchenko wrote:
> > > On Tue, Sep 03, 2024 at 05:45:33PM +0200, Bartosz Golaszewski
> > > wrote:
> >
> > ...
> >
> > > > #include <linux/mod_devicetable.h>
> > > > #include <linux/of.h>
> > > > #include <linux/platform_device.h>
> > > > +#include <linux/pm_runtime.h>
> > >
> > > You need pm.h as macros defined there.
> >
> > ...or both...
> >
>
> pm_runtime.h implies pm.h.
>
> > > > #include <linux/property.h>
> > > > #include <linux/slab.h>
> > > > #include <linux/spinlock.h>
> >
> > ...
> >
> > > > +static DEFINE_RUNTIME_DEV_PM_OPS(mpc8xx_pm_ops,
> > > > mpc8xxx_suspend,
> > > > + mpc8xxx_resume, NULL);
> >
> > This one comes from pm_runtime.h, but pm*_ptr() ones from pm.h.
> >
> > And it seems you wanted pm_ptr().
> >
>
> Yeah, I'm not sure really. The suspend and resume callbacks for
> platform devices are not documented but it looks like they're only
> used for system sleep. Martyn: which one do we actually need?
>
Looking at commit 9d8619190031 (PM: runtime: Add
DEFINE_RUNTIME_DEV_PM_OPS() macro), the use of
DEFINE_RUNTIME_DEV_PM_OPS() sets up PM operations for all situations,
not just sleeping, so I think we need pm_ptr() here.
I'd explicitly added the functionality for suspension due to sleep,
though if a GPIO line is being used as some form of external interrupt,
there may well be cases where it would be beneficial to be able to
receive it in other low power states?
Martyn
> Bart
>
> > --
> > With Best Regards,
> > Andy Shevchenko
> >
> >
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] gpio: mpc8xxx: order headers alphabetically
2024-09-03 15:45 [PATCH 1/2] gpio: mpc8xxx: order headers alphabetically Bartosz Golaszewski
2024-09-03 15:45 ` [PATCH 2/2] gpio: mpc8xxx: switch to using DEFINE_RUNTIME_DEV_PM_OPS() Bartosz Golaszewski
@ 2024-09-03 16:20 ` Andy Shevchenko
2024-09-03 17:23 ` Martyn Welch
2024-09-04 7:23 ` Bartosz Golaszewski
3 siblings, 0 replies; 9+ messages in thread
From: Andy Shevchenko @ 2024-09-03 16:20 UTC (permalink / raw)
To: Bartosz Golaszewski
Cc: Linus Walleij, Martyn Welch, linux-gpio, linux-kernel,
Bartosz Golaszewski
On Tue, Sep 03, 2024 at 05:45:32PM +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Cleanup the includes by putting them in alphabetical order.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] gpio: mpc8xxx: order headers alphabetically
2024-09-03 15:45 [PATCH 1/2] gpio: mpc8xxx: order headers alphabetically Bartosz Golaszewski
2024-09-03 15:45 ` [PATCH 2/2] gpio: mpc8xxx: switch to using DEFINE_RUNTIME_DEV_PM_OPS() Bartosz Golaszewski
2024-09-03 16:20 ` [PATCH 1/2] gpio: mpc8xxx: order headers alphabetically Andy Shevchenko
@ 2024-09-03 17:23 ` Martyn Welch
2024-09-04 7:23 ` Bartosz Golaszewski
3 siblings, 0 replies; 9+ messages in thread
From: Martyn Welch @ 2024-09-03 17:23 UTC (permalink / raw)
To: Bartosz Golaszewski, Linus Walleij, Andy Shevchenko
Cc: linux-gpio, linux-kernel, Bartosz Golaszewski
On Tue, 2024-09-03 at 17:45 +0200, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
>
> Cleanup the includes by putting them in alphabetical order.
>
> Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
> ---
> drivers/gpio/gpio-mpc8xxx.c | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpio/gpio-mpc8xxx.c b/drivers/gpio/gpio-
> mpc8xxx.c
> index ab30c911c9d50..e084e08f54387 100644
> --- a/drivers/gpio/gpio-mpc8xxx.c
> +++ b/drivers/gpio/gpio-mpc8xxx.c
> @@ -7,19 +7,19 @@
> */
>
> #include <linux/acpi.h>
> -#include <linux/kernel.h>
> -#include <linux/init.h>
> -#include <linux/platform_device.h>
> -#include <linux/spinlock.h>
> -#include <linux/io.h>
> -#include <linux/of.h>
> -#include <linux/property.h>
> -#include <linux/mod_devicetable.h>
> -#include <linux/slab.h>
> -#include <linux/irq.h>
> -#include <linux/gpio/driver.h>
> #include <linux/bitops.h>
> +#include <linux/gpio/driver.h>
> +#include <linux/init.h>
> #include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/irq.h>
> +#include <linux/kernel.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/property.h>
> +#include <linux/slab.h>
> +#include <linux/spinlock.h>
>
> #define MPC8XXX_GPIO_PINS 32
>
Reviewed-by: Martyn Welch <martyn.welch@collabora.com>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/2] gpio: mpc8xxx: order headers alphabetically
2024-09-03 15:45 [PATCH 1/2] gpio: mpc8xxx: order headers alphabetically Bartosz Golaszewski
` (2 preceding siblings ...)
2024-09-03 17:23 ` Martyn Welch
@ 2024-09-04 7:23 ` Bartosz Golaszewski
3 siblings, 0 replies; 9+ messages in thread
From: Bartosz Golaszewski @ 2024-09-04 7:23 UTC (permalink / raw)
To: Linus Walleij, Andy Shevchenko, Martyn Welch, Bartosz Golaszewski
Cc: Bartosz Golaszewski, linux-gpio, linux-kernel
From: Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
On Tue, 03 Sep 2024 17:45:32 +0200, Bartosz Golaszewski wrote:
> Cleanup the includes by putting them in alphabetical order.
>
>
Applied, thanks!
[1/2] gpio: mpc8xxx: order headers alphabetically
commit: ccaf84694ce7e7438706185c726310be51954fd3
Best regards,
--
Bartosz Golaszewski <bartosz.golaszewski@linaro.org>
^ permalink raw reply [flat|nested] 9+ messages in thread