Use the modern PM macros for the suspend and resume functions to be
automatically dropped by the compiler when CONFIG_PM or
CONFIG_PM_SLEEP are disabled, without having to use #ifdef guards.
This has the advantage of always compiling these functions in,
independently of any Kconfig option. Thanks to that, bugs and other
regressions are subsequently easier to catch.
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/gpio/gpio-dwapb.c | 18 ++++--------------
1 file changed, 4 insertions(+), 14 deletions(-)
diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index b42ff46d292b..4986c465c9a8 100644
--- a/drivers/gpio/gpio-dwapb.c
+++ b/drivers/gpio/gpio-dwapb.c
@@ -79,7 +79,6 @@ struct dwapb_platform_data {
unsigned int nports;
};
-#ifdef CONFIG_PM_SLEEP
/* Store GPIO context across system-wide suspend/resume transitions */
struct dwapb_context {
u32 data;
@@ -92,7 +91,6 @@ struct dwapb_context {
u32 int_deb;
u32 wake_en;
};
-#endif
struct dwapb_gpio_port_irqchip {
unsigned int nr_irqs;
@@ -103,9 +101,7 @@ struct dwapb_gpio_port {
struct gpio_generic_chip chip;
struct dwapb_gpio_port_irqchip *pirq;
struct dwapb_gpio *gpio;
-#ifdef CONFIG_PM_SLEEP
struct dwapb_context *ctx;
-#endif
unsigned int idx;
};
@@ -363,7 +359,6 @@ static int dwapb_irq_set_type(struct irq_data *d, u32 type)
return 0;
}
-#ifdef CONFIG_PM_SLEEP
static int dwapb_irq_set_wake(struct irq_data *d, unsigned int enable)
{
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
@@ -378,9 +373,6 @@ static int dwapb_irq_set_wake(struct irq_data *d, unsigned int enable)
return 0;
}
-#else
-#define dwapb_irq_set_wake NULL
-#endif
static const struct irq_chip dwapb_irq_chip = {
.name = DWAPB_DRIVER_NAME,
@@ -390,7 +382,7 @@ static const struct irq_chip dwapb_irq_chip = {
.irq_set_type = dwapb_irq_set_type,
.irq_enable = dwapb_irq_enable,
.irq_disable = dwapb_irq_disable,
- .irq_set_wake = dwapb_irq_set_wake,
+ .irq_set_wake = pm_sleep_ptr(dwapb_irq_set_wake),
.flags = IRQCHIP_IMMUTABLE,
GPIOCHIP_IRQ_RESOURCE_HELPERS,
};
@@ -759,7 +751,6 @@ static int dwapb_gpio_probe(struct platform_device *pdev)
return 0;
}
-#ifdef CONFIG_PM_SLEEP
static int dwapb_gpio_suspend(struct device *dev)
{
struct dwapb_gpio *gpio = dev_get_drvdata(dev);
@@ -844,15 +835,14 @@ static int dwapb_gpio_resume(struct device *dev)
return 0;
}
-#endif
-static SIMPLE_DEV_PM_OPS(dwapb_gpio_pm_ops, dwapb_gpio_suspend,
- dwapb_gpio_resume);
+static DEFINE_SIMPLE_DEV_PM_OPS(dwapb_gpio_pm_ops,
+ dwapb_gpio_suspend, dwapb_gpio_resume);
static struct platform_driver dwapb_gpio_driver = {
.driver = {
.name = DWAPB_DRIVER_NAME,
- .pm = &dwapb_gpio_pm_ops,
+ .pm = pm_sleep_ptr(&dwapb_gpio_pm_ops),
.of_match_table = dwapb_of_match,
.acpi_match_table = dwapb_acpi_match,
},
--
2.51.0On Wed, Nov 19, 2025 at 10:43:13PM +0800, Jisheng Zhang wrote:
> Use the modern PM macros for the suspend and resume functions to be
> automatically dropped by the compiler when CONFIG_PM or
> CONFIG_PM_SLEEP are disabled, without having to use #ifdef guards.
>
> This has the advantage of always compiling these functions in,
> independently of any Kconfig option. Thanks to that, bugs and other
> regressions are subsequently easier to catch.
...
> -#ifdef CONFIG_PM_SLEEP
> /* Store GPIO context across system-wide suspend/resume transitions */
> struct dwapb_context {
> u32 data;
> u32 int_deb;
> u32 wake_en;
> };
> -#endif
This ifdeffery is to protect the type definition? It may be removed for sure.
...
> struct dwapb_gpio_port_irqchip {
> unsigned int nr_irqs;
> struct gpio_generic_chip chip;
> struct dwapb_gpio_port_irqchip *pirq;
> struct dwapb_gpio *gpio;
> -#ifdef CONFIG_PM_SLEEP
> struct dwapb_context *ctx;
> -#endif
But why this? For the PM_SLEEP=n cases it will give an unrequested overhead.
> unsigned int idx;
> };
Otherwise LGTM.
--
With Best Regards,
Andy Shevchenko
On Wed, Nov 19, 2025 at 05:42:59PM +0200, Andy Shevchenko wrote:
> On Wed, Nov 19, 2025 at 10:43:13PM +0800, Jisheng Zhang wrote:
> > Use the modern PM macros for the suspend and resume functions to be
> > automatically dropped by the compiler when CONFIG_PM or
> > CONFIG_PM_SLEEP are disabled, without having to use #ifdef guards.
> >
> > This has the advantage of always compiling these functions in,
> > independently of any Kconfig option. Thanks to that, bugs and other
> > regressions are subsequently easier to catch.
>
> ...
>
> > -#ifdef CONFIG_PM_SLEEP
> > /* Store GPIO context across system-wide suspend/resume transitions */
> > struct dwapb_context {
> > u32 data;
>
> > u32 int_deb;
> > u32 wake_en;
> > };
> > -#endif
>
> This ifdeffery is to protect the type definition? It may be removed for sure.
>
> ...
>
> > struct dwapb_gpio_port_irqchip {
> > unsigned int nr_irqs;
>
> > struct gpio_generic_chip chip;
> > struct dwapb_gpio_port_irqchip *pirq;
> > struct dwapb_gpio *gpio;
> > -#ifdef CONFIG_PM_SLEEP
> > struct dwapb_context *ctx;
> > -#endif
>
> But why this? For the PM_SLEEP=n cases it will give an unrequested overhead.
the pm_ptr() and pm_sleep_ptr() can optimize out the PM related
functions, but those functions are still compiled, so if we keep the
#ifdef, there will be build errors.
>
> > unsigned int idx;
> > };
>
> Otherwise LGTM.
>
> --
> With Best Regards,
> Andy Shevchenko
>
>
On Wed, Nov 19, 2025 at 6:09 PM Jisheng Zhang <jszhang@kernel.org> wrote: > On Wed, Nov 19, 2025 at 05:42:59PM +0200, Andy Shevchenko wrote: > > On Wed, Nov 19, 2025 at 10:43:13PM +0800, Jisheng Zhang wrote: ... > > > -#ifdef CONFIG_PM_SLEEP > > > struct dwapb_context *ctx; > > > -#endif > > > > But why this? For the PM_SLEEP=n cases it will give an unrequested overhead. > > the pm_ptr() and pm_sleep_ptr() can optimize out the PM related > functions, but those functions are still compiled, so if we keep the > #ifdef, there will be build errors. Is this an expectation or you can share the error, please? -- With Best Regards, Andy Shevchenko
On Wed, Nov 19, 2025 at 06:11:53PM +0200, Andy Shevchenko wrote: > On Wed, Nov 19, 2025 at 6:09 PM Jisheng Zhang <jszhang@kernel.org> wrote: > > On Wed, Nov 19, 2025 at 05:42:59PM +0200, Andy Shevchenko wrote: > > > On Wed, Nov 19, 2025 at 10:43:13PM +0800, Jisheng Zhang wrote: > > ... > > > > > -#ifdef CONFIG_PM_SLEEP > > > > struct dwapb_context *ctx; > > > > -#endif > > > > > > But why this? For the PM_SLEEP=n cases it will give an unrequested overhead. > > > > the pm_ptr() and pm_sleep_ptr() can optimize out the PM related > > functions, but those functions are still compiled, so if we keep the > > #ifdef, there will be build errors. > > Is this an expectation or you can share the error, please? drivers/gpio/gpio-dwapb.c: In function ‘dwapb_irq_set_wake’: drivers/gpio/gpio-dwapb.c:368:51: error: ‘struct dwapb_gpio_port’ h as no member named ‘ctx’ 368 | struct dwapb_context *ctx = gpio->ports[0].ctx;
On Thu, Nov 20, 2025 at 12:04:58AM +0800, Jisheng Zhang wrote: > On Wed, Nov 19, 2025 at 06:11:53PM +0200, Andy Shevchenko wrote: > > On Wed, Nov 19, 2025 at 6:09 PM Jisheng Zhang <jszhang@kernel.org> wrote: > > > On Wed, Nov 19, 2025 at 05:42:59PM +0200, Andy Shevchenko wrote: > > > > On Wed, Nov 19, 2025 at 10:43:13PM +0800, Jisheng Zhang wrote: ... > > > > > -#ifdef CONFIG_PM_SLEEP > > > > > struct dwapb_context *ctx; > > > > > -#endif > > > > > > > > But why this? For the PM_SLEEP=n cases it will give an unrequested overhead. > > > > > > the pm_ptr() and pm_sleep_ptr() can optimize out the PM related > > > functions, but those functions are still compiled, so if we keep the > > > #ifdef, there will be build errors. > > > > Is this an expectation or you can share the error, please? > > drivers/gpio/gpio-dwapb.c: In function ‘dwapb_irq_set_wake’: > drivers/gpio/gpio-dwapb.c:368:51: error: ‘struct dwapb_gpio_port’ h > as no member named ‘ctx’ > 368 | struct dwapb_context *ctx = gpio->ports[0].ctx; Thank you! I just answered in the other email that I will tag v4 after fixing typos and dropping unrelated changes. -- With Best Regards, Andy Shevchenko
© 2016 - 2025 Red Hat, Inc.