[PATCH 05/16] gpio: pl061: Use modern PM macros

Jisheng Zhang posted 16 patches 1 month, 2 weeks ago
[PATCH 05/16] gpio: pl061: Use modern PM macros
Posted by Jisheng Zhang 1 month, 2 weeks ago
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 pl061_context_save_regs structure is always embedded into struct
pl061 to simplify code, so this brings a tiny 8 bytes memory overhead
for !CONFIG_PM_SLEP.

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/gpio/gpio-pl061.c | 17 ++---------------
 1 file changed, 2 insertions(+), 15 deletions(-)

diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
index 02e4ffcf5a6f..919cf86fd590 100644
--- a/drivers/gpio/gpio-pl061.c
+++ b/drivers/gpio/gpio-pl061.c
@@ -37,7 +37,6 @@
 
 #define PL061_GPIO_NR	8
 
-#ifdef CONFIG_PM
 struct pl061_context_save_regs {
 	u8 gpio_data;
 	u8 gpio_dir;
@@ -46,7 +45,6 @@ struct pl061_context_save_regs {
 	u8 gpio_iev;
 	u8 gpio_ie;
 };
-#endif
 
 struct pl061 {
 	raw_spinlock_t		lock;
@@ -55,9 +53,7 @@ struct pl061 {
 	struct gpio_chip	gc;
 	int			parent_irq;
 
-#ifdef CONFIG_PM
 	struct pl061_context_save_regs csave_regs;
-#endif
 };
 
 static int pl061_get_direction(struct gpio_chip *gc, unsigned offset)
@@ -367,7 +363,6 @@ static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
 	return 0;
 }
 
-#ifdef CONFIG_PM
 static int pl061_suspend(struct device *dev)
 {
 	struct pl061 *pl061 = dev_get_drvdata(dev);
@@ -411,13 +406,7 @@ static int pl061_resume(struct device *dev)
 	return 0;
 }
 
-static const struct dev_pm_ops pl061_dev_pm_ops = {
-	.suspend = pl061_suspend,
-	.resume = pl061_resume,
-	.freeze = pl061_suspend,
-	.restore = pl061_resume,
-};
-#endif
+static DEFINE_SIMPLE_DEV_PM_OPS(pl061_dev_pm_ops, pl061_suspend, pl061_resume);
 
 static const struct amba_id pl061_ids[] = {
 	{
@@ -431,9 +420,7 @@ MODULE_DEVICE_TABLE(amba, pl061_ids);
 static struct amba_driver pl061_gpio_driver = {
 	.drv = {
 		.name	= "pl061_gpio",
-#ifdef CONFIG_PM
-		.pm	= &pl061_dev_pm_ops,
-#endif
+		.pm	= pm_sleep_ptr(&pl061_dev_pm_ops),
 	},
 	.id_table	= pl061_ids,
 	.probe		= pl061_probe,
-- 
2.50.1
Re: [PATCH 05/16] gpio: pl061: Use modern PM macros
Posted by Linus Walleij 1 month, 1 week ago
On Wed, Aug 20, 2025 at 5:58 PM 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 pl061_context_save_regs structure is always embedded into struct
> pl061 to simplify code, so this brings a tiny 8 bytes memory overhead
> for !CONFIG_PM_SLEP.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>

Hm true, 8 bytes is not a big deal.

Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij
Re: [PATCH 05/16] gpio: pl061: Use modern PM macros
Posted by Andy Shevchenko 1 month, 1 week ago
On Thu, Aug 21, 2025 at 2:52 PM Linus Walleij <linus.walleij@linaro.org> wrote:
> On Wed, Aug 20, 2025 at 5:58 PM Jisheng Zhang <jszhang@kernel.org> wrote:

...

> > The pl061_context_save_regs structure is always embedded into struct
> > pl061 to simplify code, so this brings a tiny 8 bytes memory overhead
> > for !CONFIG_PM_SLEP.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
>
> Hm true, 8 bytes is not a big deal.

It's better to double check. This data is per GPIO chip, the Q is how
many chips can be present on one system. Then the calculus will be N *
8 bytes.

Knowing gpio-dwapb (and having even a HW with it) the commit message
(there) seems like a slight manipulation as it doesn't clarify this
detail.

-- 
With Best Regards,
Andy Shevchenko