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.

The dwapb_context structure is always embedded into struct
dwapb_gpio_port to simplify code. Sure this brings a tiny 36 bytes
data overhead for !CONFIG_PM_SLEP. After greping the arm/arm64/riscv
dts dir, the max port number is 6, the berlin2q soc families, so this
means current we have wasted 216 bytes memory which is trivial
compared to the system memory.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/gpio/gpio-dwapb.c | 32 ++++++++------------------------
 1 file changed, 8 insertions(+), 24 deletions(-)

diff --git a/drivers/gpio/gpio-dwapb.c b/drivers/gpio/gpio-dwapb.c
index b42ff46d292b..a431bea959ed 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
+	struct dwapb_context	ctx;
 	unsigned int		idx;
 };
 
@@ -363,12 +359,11 @@ 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);
 	struct dwapb_gpio *gpio = to_dwapb_gpio(gc);
-	struct dwapb_context *ctx = gpio->ports[0].ctx;
+	struct dwapb_context *ctx = &gpio->ports[0].ctx;
 	irq_hw_number_t bit = irqd_to_hwirq(d);
 
 	if (enable)
@@ -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,
 };
@@ -515,12 +507,6 @@ static int dwapb_gpio_add_port(struct dwapb_gpio *gpio,
 	port->gpio = gpio;
 	port->idx = pp->idx;
 
-#ifdef CONFIG_PM_SLEEP
-	port->ctx = devm_kzalloc(gpio->dev, sizeof(*port->ctx), GFP_KERNEL);
-	if (!port->ctx)
-		return -ENOMEM;
-#endif
-
 	dat = gpio->regs + GPIO_EXT_PORTA + pp->idx * GPIO_EXT_PORT_STRIDE;
 	set = gpio->regs + GPIO_SWPORTA_DR + pp->idx * GPIO_SWPORT_DR_STRIDE;
 	dirout = gpio->regs + GPIO_SWPORTA_DDR + pp->idx * GPIO_SWPORT_DDR_STRIDE;
@@ -759,7 +745,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);
@@ -770,7 +755,7 @@ static int dwapb_gpio_suspend(struct device *dev)
 		for (i = 0; i < gpio->nr_ports; i++) {
 			unsigned int offset;
 			unsigned int idx = gpio->ports[i].idx;
-			struct dwapb_context *ctx = gpio->ports[i].ctx;
+			struct dwapb_context *ctx = &gpio->ports[i].ctx;
 
 			offset = GPIO_SWPORTA_DDR + idx * GPIO_SWPORT_DDR_STRIDE;
 			ctx->dir = dwapb_read(gpio, offset);
@@ -818,7 +803,7 @@ static int dwapb_gpio_resume(struct device *dev)
 	for (i = 0; i < gpio->nr_ports; i++) {
 		unsigned int offset;
 		unsigned int idx = gpio->ports[i].idx;
-		struct dwapb_context *ctx = gpio->ports[i].ctx;
+		struct dwapb_context *ctx = &gpio->ports[i].ctx;
 
 		offset = GPIO_SWPORTA_DR + idx * GPIO_SWPORT_DR_STRIDE;
 		dwapb_write(gpio, offset, ctx->data);
@@ -844,15 +829,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.0

On Tue, Nov 18, 2025 at 2:50 AM Jisheng Zhang <jszhang@kernel.org> 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.
>
> The dwapb_context structure is always embedded into struct
> dwapb_gpio_port to simplify code. Sure this brings a tiny 36 bytes
> data overhead for !CONFIG_PM_SLEP. After greping the arm/arm64/riscv

SLEEP
grepping

> dts dir, the max port number is 6, the berlin2q soc families, so this
> means current we have wasted 216 bytes memory which is trivial

currently

> compared to the system memory.

I still think the embedding is not related to this change and should
be justified in a separate patch. W/o that part the rest looks fine.

-- 
With Best Regards,
Andy Shevchenko

On Tue, Nov 18, 2025 at 12:15:35PM +0200, Andy Shevchenko wrote:
> On Tue, Nov 18, 2025 at 2:50 AM Jisheng Zhang <jszhang@kernel.org> 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.
> >
> > The dwapb_context structure is always embedded into struct
> > dwapb_gpio_port to simplify code. Sure this brings a tiny 36 bytes
> > data overhead for !CONFIG_PM_SLEP. After greping the arm/arm64/riscv
> 
> SLEEP
> grepping
> 
> > dts dir, the max port number is 6, the berlin2q soc families, so this
> > means current we have wasted 216 bytes memory which is trivial
> 
> currently
> 
> > compared to the system memory.
> 
> I still think the embedding is not related to this change and should
> be justified in a separate patch. W/o that part the rest looks fine.

I got your mind now: it looks like you prefer a seperate patch for the
embedding. Let me explain why I have the embedding within this patch:
the pm_ptr() or pm_sleep_ptr() just optimizes out the PM functions, but the
PM funtions are still compiled, so w/o the embedding, it's impossible
to clean up the code with the modern PM macros.

On Wed, Nov 19, 2025 at 08:42:05PM +0800, Jisheng Zhang wrote:
> On Tue, Nov 18, 2025 at 12:15:35PM +0200, Andy Shevchenko wrote:
> > On Tue, Nov 18, 2025 at 2:50 AM Jisheng Zhang <jszhang@kernel.org> 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.
> > >
> > > The dwapb_context structure is always embedded into struct
> > > dwapb_gpio_port to simplify code. Sure this brings a tiny 36 bytes
> > > data overhead for !CONFIG_PM_SLEP. After greping the arm/arm64/riscv
> > 
> > SLEEP
> > grepping
> > 
> > > dts dir, the max port number is 6, the berlin2q soc families, so this
> > > means current we have wasted 216 bytes memory which is trivial
> > 
> > currently
> > 
> > > compared to the system memory.
> > 
> > I still think the embedding is not related to this change and should
> > be justified in a separate patch. W/o that part the rest looks fine.
> 
> I got your mind now: it looks like you prefer a seperate patch for the
> embedding. Let me explain why I have the embedding within this patch:
> the pm_ptr() or pm_sleep_ptr() just optimizes out the PM functions, but the
> PM funtions are still compiled, so w/o the embedding, it's impossible
> to clean up the code with the modern PM macros.

For dwapb, I can still acchieve the clean up w/ only embedding the
pointer. But I'm not sure whether embedding the struture deserve a seperate
patch.

BTW: as Michael mentioned during v1 review, the driver allocates the
struct with kzalloc and stores a pointer to it, so considering the
pointer itself and the kmalloc overhead/alignment etc, current gpio-dwapb
have an overhead in the same order of magnitude when PM=y

On Wed, Nov 19, 2025 at 3:17 PM Jisheng Zhang <jszhang@kernel.org> wrote:
> On Wed, Nov 19, 2025 at 08:42:05PM +0800, Jisheng Zhang wrote:
> > On Tue, Nov 18, 2025 at 12:15:35PM +0200, Andy Shevchenko wrote:
> > > On Tue, Nov 18, 2025 at 2:50 AM Jisheng Zhang <jszhang@kernel.org> 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.
> > > >
> > > > The dwapb_context structure is always embedded into struct
> > > > dwapb_gpio_port to simplify code. Sure this brings a tiny 36 bytes
> > > > data overhead for !CONFIG_PM_SLEP. After greping the arm/arm64/riscv
> > >
> > > SLEEP
> > > grepping
> > >
> > > > dts dir, the max port number is 6, the berlin2q soc families, so this
> > > > means current we have wasted 216 bytes memory which is trivial
> > >
> > > currently
> > >
> > > > compared to the system memory.
> > >
> > > I still think the embedding is not related to this change and should
> > > be justified in a separate patch. W/o that part the rest looks fine.
> >
> > I got your mind now: it looks like you prefer a seperate patch for the
> > embedding. Let me explain why I have the embedding within this patch:
> > the pm_ptr() or pm_sleep_ptr() just optimizes out the PM functions, but the
> > PM funtions are still compiled, so w/o the embedding, it's impossible
> > to clean up the code with the modern PM macros.

It's possible, but it will require some other refactoring most likely.

...

> For dwapb, I can still acchieve the clean up w/ only embedding the
> pointer. But I'm not sure whether embedding the struture deserve a seperate
> patch.

> BTW: as Michael mentioned during v1 review, the driver allocates the
> struct with kzalloc and stores a pointer to it, so considering the
> pointer itself and the kmalloc overhead/alignment etc, current gpio-dwapb
> have an overhead in the same order of magnitude when PM=y

This is a good point. Was it mentioned in the commit message?

...

Okay, I am not going to tag this patch, I leave it for Bart to decide,
but I don't like the idea of blowing the run-time memory footprint
just because we may need it in CONFIG_PM=y case.

-- 
With Best Regards,
Andy Shevchenko