Re: [PATCH 04/12] mtd: physmap: Remove #ifdef guards for PM related functions

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] mtd: physmap: misuse of pm_ptr() on shutdown hook physmap_flash_shutdown()
--

commit bb8ff3575985a70d1ab3eb3ce9de88e80be2c1c0
Author: Paul Cercueil <paul.cercueil@bootlin.com>

mtd: physmap: Remove #ifdef guards for PM related functions

This commit removes the CONFIG_PM #ifdef guard around
physmap_flash_shutdown() and wraps its assignment in physmap_flash_driver
with pm_ptr().

>     mtd: physmap: Remove #ifdef guards for PM related functions
>     
>     Use the new 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: Paul Cercueil <paul.cercueil@bootlin.com>
> 
> diff --git a/drivers/mtd/maps/physmap-core.c b/drivers/mtd/maps/physmap-core.c
> index dcda7685fc99b..afda90fc23eef 100644
> --- a/drivers/mtd/maps/physmap-core.c
> +++ b/drivers/mtd/maps/physmap-core.c
> @@ -600,7 +600,6 @@ static int physmap_flash_probe(struct platform_device *dev)
>  	return err;
>  }
>  
> -#ifdef CONFIG_PM
>  static void physmap_flash_shutdown(struct platform_device *dev)
>  {
>  	struct physmap_flash_info *info = platform_get_drvdata(dev);
> @@ -610,14 +609,11 @@ static void physmap_flash_shutdown(struct platform_device *dev)
>  		if (mtd_suspend(info->mtds[i]) == 0)
>  			mtd_resume(info->mtds[i]);
>  }
> -#else
> -#define physmap_flash_shutdown NULL
> -#endif
>  
>  static struct platform_driver physmap_flash_driver = {
>  	.probe		= physmap_flash_probe,
>  	.remove		= physmap_flash_remove,
> -	.shutdown	= physmap_flash_shutdown,
> +	.shutdown	= pm_ptr(physmap_flash_shutdown),

[Severity: Low]
The commit message describes using PM macros for suspend and resume
functions, and this wraps physmap_flash_shutdown() in pm_ptr(). However,
physmap_flash_shutdown() is the platform_driver .shutdown hook rather than a
dev_pm_ops callback.

The original #ifdef CONFIG_PM guard appears to be a leftover from commit
17c2dae3aaff9b, which placed physmap_flash_suspend(),
physmap_flash_resume(), and physmap_flash_shutdown() inside a single
#ifdef CONFIG_PM block before the driver's suspend and resume callbacks were
later removed in favor of mtd_class PM ops.

Since device_shutdown() -> platform_shutdown() invokes the .shutdown hook
during system reboot, halt, poweroff, or kexec regardless of CONFIG_PM (to
return flash chips to read-array mode), and both mtd_suspend() and
mtd_resume() in include/linux/mtd/mtd.h are defined unconditionally, does
physmap_flash_shutdown() need to depend on CONFIG_PM at all? Could
.shutdown be assigned unconditionally without pm_ptr(), with the commit
message updated to match?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261105171158.399247-1-paul.cercueil@bootlin.com?part=4

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/