Re: [PATCH v2] PM: Introduce DEFINE_PM_GENERIC_FUNC macro to reduce code duplication

[Dhruva Gole <d-gole@ti.com>]

On Oct 14, 2025 at 12:52:03 +0530, Kaushlendra Kumar wrote:
> Add DEFINE_PM_GENERIC_FUNC macro to completely eliminate repetitive
> code patterns in power management generic operations. This macro
> generates the entire function definition including signature,
> implementation, and symbol export for each pm_generic_* function.
> 
> This reduces code duplication significantly while maintaining the
> same functionality and improving code maintainability.
> 
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---
> Changes in v2:
> - Include function signature and symbol export in macro as suggested
> ---
>  drivers/base/power/generic_ops.c | 158 +++++--------------------------
>  1 file changed, 26 insertions(+), 132 deletions(-)
> 
> diff --git a/drivers/base/power/generic_ops.c b/drivers/base/power/generic_ops.c
> index 6502720bb564..0afea5d8f8ef 100644
> --- a/drivers/base/power/generic_ops.c
> +++ b/drivers/base/power/generic_ops.c
> @@ -8,6 +8,14 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/export.h>
>  
> +#define DEFINE_PM_GENERIC_FUNC(func_name, op_name) \
> +int pm_generic_##func_name(struct device *dev) \
> +{ \
> +	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; \
> +	return pm && pm->op_name ? pm->op_name(dev) : 0; \
> +} \
> +EXPORT_SYMBOL_GPL(pm_generic_##func_name)
> +

NAK.

Honestly, I am not okay with this level of optimization just to reduce
lines of code. It was much more verbose and clear earlier and I don't
really understand the full benefit of this patch.

If we look at this from the eyes of a newcomer to the kernel, who does
not follow the PM lists/ is coming from older kernels and tries to jump
to the definitions of pm_generic_runtime_suspend or whatever - it's
going to break for them. I use ctags to jump around kernel definitions
extensively and after this patch I will have to jump through hoops to
find what pm_generic_XYZ is doing and where it even is defined. grep
will break too.

Unless there's real proof that this greatly improves code readability or
performance that I can't think of right now.

Perhaps Greg/ Rafael can comment on whether this is truly justified
from a maintainence perspective, but as a PM susbsys user it will make
things just complicated for consumers of these ops to dig into IMO.

>  #ifdef CONFIG_PM
>  /**
>   * pm_generic_runtime_suspend - Generic runtime suspend callback for subsystems.
> @@ -17,16 +25,7 @@
>   * ->runtime_suspend(), execute it and return its error code.  Otherwise,
>   * return 0.
>   */
> -int pm_generic_runtime_suspend(struct device *dev)
> -{
> -	const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL;
> -	int ret;
> -
> -	ret = pm && pm->runtime_suspend ? pm->runtime_suspend(dev) : 0;
> -
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(pm_generic_runtime_suspend);
> +DEFINE_PM_GENERIC_FUNC(runtime_suspend, runtime_suspend);
[...]

-- 
Best regards,
Dhruva Gole
Texas Instruments Incorporated