1. Patchew
  2. linux
  3. View series

[PATCH] spi: st: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()

Raphael Gallais-Pou posted 1 patch 4 months ago 
drivers/spi/spi-st-ssc4.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
[PATCH] spi: st: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()
Posted by Raphael Gallais-Pou 4 months ago 
Letting the compiler remove these functions when the kernel is built
without CONFIG_PM_SLEEP support is simpler and less error prone than the
use of #ifdef based kernel configuration guards.

Signed-off-by: Raphael Gallais-Pou <rgallaispou@gmail.com>
---
 drivers/spi/spi-st-ssc4.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/drivers/spi/spi-st-ssc4.c b/drivers/spi/spi-st-ssc4.c
index 4cff976ab16fbdf3708ab870176a04f2628b501b..49ab4c515156fbabe0761028a5cb4770b61ca508 100644
--- a/drivers/spi/spi-st-ssc4.c
+++ b/drivers/spi/spi-st-ssc4.c
@@ -378,8 +378,7 @@ static void spi_st_remove(struct platform_device *pdev)
 	pinctrl_pm_select_sleep_state(&pdev->dev);
 }
 
-#ifdef CONFIG_PM
-static int spi_st_runtime_suspend(struct device *dev)
+static int __maybe_unused spi_st_runtime_suspend(struct device *dev)
 {
 	struct spi_controller *host = dev_get_drvdata(dev);
 	struct spi_st *spi_st = spi_controller_get_devdata(host);
@@ -392,7 +391,7 @@ static int spi_st_runtime_suspend(struct device *dev)
 	return 0;
 }
 
-static int spi_st_runtime_resume(struct device *dev)
+static int __maybe_unused spi_st_runtime_resume(struct device *dev)
 {
 	struct spi_controller *host = dev_get_drvdata(dev);
 	struct spi_st *spi_st = spi_controller_get_devdata(host);
@@ -403,10 +402,8 @@ static int spi_st_runtime_resume(struct device *dev)
 
 	return ret;
 }
-#endif
 
-#ifdef CONFIG_PM_SLEEP
-static int spi_st_suspend(struct device *dev)
+static int __maybe_unused spi_st_suspend(struct device *dev)
 {
 	struct spi_controller *host = dev_get_drvdata(dev);
 	int ret;
@@ -418,7 +415,7 @@ static int spi_st_suspend(struct device *dev)
 	return pm_runtime_force_suspend(dev);
 }
 
-static int spi_st_resume(struct device *dev)
+static int __maybe_unused spi_st_resume(struct device *dev)
 {
 	struct spi_controller *host = dev_get_drvdata(dev);
 	int ret;
@@ -429,7 +426,6 @@ static int spi_st_resume(struct device *dev)
 
 	return pm_runtime_force_resume(dev);
 }
-#endif
 
 static const struct dev_pm_ops spi_st_pm = {
 	SET_SYSTEM_SLEEP_PM_OPS(spi_st_suspend, spi_st_resume)
@@ -445,7 +441,7 @@ MODULE_DEVICE_TABLE(of, stm_spi_match);
 static struct platform_driver spi_st_driver = {
 	.driver = {
 		.name = "spi-st",
-		.pm = &spi_st_pm,
+		.pm = pm_sleep_ptr(&spi_st_pm),
 		.of_match_table = of_match_ptr(stm_spi_match),
 	},
 	.probe = spi_st_probe,

---
base-commit: 475c850a7fdd0915b856173186d5922899d65686
change-id: 20250609-update_pm_macro-b6def2446904

Best regards,
-- 
Raphael Gallais-Pou <rgallaispou@gmail.com>
Re: [PATCH] spi: st: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()
Posted by Geert Uytterhoeven 2 months, 2 weeks ago 
Hi Raphael,

On Tue, 10 Jun 2025 at 16:59, Raphael Gallais-Pou <rgallaispou@gmail.com> wrote:
> Letting the compiler remove these functions when the kernel is built
> without CONFIG_PM_SLEEP support is simpler and less error prone than the
> use of #ifdef based kernel configuration guards.
>
> Signed-off-by: Raphael Gallais-Pou <rgallaispou@gmail.com>

Thanks for your patch, which is now commit 7d61715c58a39edc ("spi:
rspi: Convert to DEFINE_SIMPLE_DEV_PM_OPS()") in spi/for-next.

> --- a/drivers/spi/spi-st-ssc4.c
> +++ b/drivers/spi/spi-st-ssc4.c
> @@ -378,8 +378,7 @@ static void spi_st_remove(struct platform_device *pdev)
>         pinctrl_pm_select_sleep_state(&pdev->dev);
>  }
>
> -#ifdef CONFIG_PM
> -static int spi_st_runtime_suspend(struct device *dev)
> +static int __maybe_unused spi_st_runtime_suspend(struct device *dev)

The __maybe_unused can be removed, too...

> @@ -429,7 +426,6 @@ static int spi_st_resume(struct device *dev)
>
>         return pm_runtime_force_resume(dev);
>  }
> -#endif
>
>  static const struct dev_pm_ops spi_st_pm = {
>         SET_SYSTEM_SLEEP_PM_OPS(spi_st_suspend, spi_st_resume)

... if you would update these, too:

    -    SET_SYSTEM_SLEEP_PM_OPS(spi_st_suspend, spi_st_resume)
    -    SET_RUNTIME_PM_OPS(spi_st_runtime_suspend, spi_st_runtime_resume, NULL)
    +    SYSTEM_SLEEP_PM_OPS(spi_st_suspend, spi_st_resume)
    +    RUNTIME_PM_OPS(spi_st_runtime_suspend, spi_st_runtime_resume, NULL)

> @@ -445,7 +441,7 @@ MODULE_DEVICE_TABLE(of, stm_spi_match);
>  static struct platform_driver spi_st_driver = {
>         .driver = {
>                 .name = "spi-st",
> -               .pm = &spi_st_pm,
> +               .pm = pm_sleep_ptr(&spi_st_pm),

This should use pm_ptr() instead, as spi_st_pm defines not only system
sleep ops, but also Runtime PM ops.

>                 .of_match_table = of_match_ptr(stm_spi_match),
>         },
>         .probe = spi_st_probe,

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH] spi: st: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()
Posted by Raphaël Gallais-Pou 2 months, 1 week ago 

Le 28/07/2025 à 09:35, Geert Uytterhoeven a écrit :
> Hi Raphael,
> 
> On Tue, 10 Jun 2025 at 16:59, Raphael Gallais-Pou <rgallaispou@gmail.com> wrote:
>> Letting the compiler remove these functions when the kernel is built
>> without CONFIG_PM_SLEEP support is simpler and less error prone than the
>> use of #ifdef based kernel configuration guards.
>>
>> Signed-off-by: Raphael Gallais-Pou <rgallaispou@gmail.com>
> 
> Thanks for your patch, which is now commit 7d61715c58a39edc ("spi:
> rspi: Convert to DEFINE_SIMPLE_DEV_PM_OPS()") in spi/for-next.

Hi Geert,

Did you mean commit 6f8584a4826f ("spi: st: Switch from CONFIG_PM_SLEEP 
guards to pm_sleep_ptr()", 2025-06-09) ?

7d61715c58a39edc ("spi: rspi: Convert to DEFINE_SIMPLE_DEV_PM_OPS()")
points to another reference, authored by you. :)>
>> --- a/drivers/spi/spi-st-ssc4.c
>> +++ b/drivers/spi/spi-st-ssc4.c
>> @@ -378,8 +378,7 @@ static void spi_st_remove(struct platform_device *pdev)
>>          pinctrl_pm_select_sleep_state(&pdev->dev);
>>   }
>>
>> -#ifdef CONFIG_PM
>> -static int spi_st_runtime_suspend(struct device *dev)
>> +static int __maybe_unused spi_st_runtime_suspend(struct device *dev)
> 
> The __maybe_unused can be removed, too...
> 
>> @@ -429,7 +426,6 @@ static int spi_st_resume(struct device *dev)
>>
>>          return pm_runtime_force_resume(dev);
>>   }
>> -#endif
>>
>>   static const struct dev_pm_ops spi_st_pm = {
>>          SET_SYSTEM_SLEEP_PM_OPS(spi_st_suspend, spi_st_resume)
> 
> ... if you would update these, too:
> 
>      -    SET_SYSTEM_SLEEP_PM_OPS(spi_st_suspend, spi_st_resume)
>      -    SET_RUNTIME_PM_OPS(spi_st_runtime_suspend, spi_st_runtime_resume, NULL)
>      +    SYSTEM_SLEEP_PM_OPS(spi_st_suspend, spi_st_resume)
>      +    RUNTIME_PM_OPS(spi_st_runtime_suspend, spi_st_runtime_resume, NULL)
> 
>> @@ -445,7 +441,7 @@ MODULE_DEVICE_TABLE(of, stm_spi_match);
>>   static struct platform_driver spi_st_driver = {
>>          .driver = {
>>                  .name = "spi-st",
>> -               .pm = &spi_st_pm,
>> +               .pm = pm_sleep_ptr(&spi_st_pm),
> 
> This should use pm_ptr() instead, as spi_st_pm defines not only system
> sleep ops, but also Runtime PM ops.


Anyway, that is indeed a good catch.  I actually got lost between 
CONFIG_PM and CONFIG_PM_SLEEP, which is now clarified.

I made a fix, but will send it after my summer break.

Thanks,

Best regards,
Raphaël>
>>                  .of_match_table = of_match_ptr(stm_spi_match),
>>          },
>>          .probe = spi_st_probe,
> 
> Gr{oetje,eeting}s,
> 
>                          Geert
>
Re: [PATCH] spi: st: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()
Posted by Geert Uytterhoeven 2 months, 1 week ago 
Hi Raphaël,

On Wed, 30 Jul 2025 at 17:38, Raphaël Gallais-Pou <rgallaispou@gmail.com> wrote:
> Le 28/07/2025 à 09:35, Geert Uytterhoeven a écrit :
> > On Tue, 10 Jun 2025 at 16:59, Raphael Gallais-Pou <rgallaispou@gmail.com> wrote:
> >> Letting the compiler remove these functions when the kernel is built
> >> without CONFIG_PM_SLEEP support is simpler and less error prone than the
> >> use of #ifdef based kernel configuration guards.
> >>
> >> Signed-off-by: Raphael Gallais-Pou <rgallaispou@gmail.com>
> >
> > Thanks for your patch, which is now commit 7d61715c58a39edc ("spi:
> > rspi: Convert to DEFINE_SIMPLE_DEV_PM_OPS()") in spi/for-next.
>
> Did you mean commit 6f8584a4826f ("spi: st: Switch from CONFIG_PM_SLEEP
> guards to pm_sleep_ptr()", 2025-06-09) ?

Oops, you are right. Sorry for the confusion.

> 7d61715c58a39edc ("spi: rspi: Convert to DEFINE_SIMPLE_DEV_PM_OPS()")
> points to another reference, authored by you. :)>
> >> --- a/drivers/spi/spi-st-ssc4.c
> >> +++ b/drivers/spi/spi-st-ssc4.c
> >> @@ -378,8 +378,7 @@ static void spi_st_remove(struct platform_device *pdev)
> >>          pinctrl_pm_select_sleep_state(&pdev->dev);
> >>   }
> >>
> >> -#ifdef CONFIG_PM
> >> -static int spi_st_runtime_suspend(struct device *dev)
> >> +static int __maybe_unused spi_st_runtime_suspend(struct device *dev)
> >
> > The __maybe_unused can be removed, too...
> >
> >> @@ -429,7 +426,6 @@ static int spi_st_resume(struct device *dev)
> >>
> >>          return pm_runtime_force_resume(dev);
> >>   }
> >> -#endif
> >>
> >>   static const struct dev_pm_ops spi_st_pm = {
> >>          SET_SYSTEM_SLEEP_PM_OPS(spi_st_suspend, spi_st_resume)
> >
> > ... if you would update these, too:
> >
> >      -    SET_SYSTEM_SLEEP_PM_OPS(spi_st_suspend, spi_st_resume)
> >      -    SET_RUNTIME_PM_OPS(spi_st_runtime_suspend, spi_st_runtime_resume, NULL)
> >      +    SYSTEM_SLEEP_PM_OPS(spi_st_suspend, spi_st_resume)
> >      +    RUNTIME_PM_OPS(spi_st_runtime_suspend, spi_st_runtime_resume, NULL)
> >
> >> @@ -445,7 +441,7 @@ MODULE_DEVICE_TABLE(of, stm_spi_match);
> >>   static struct platform_driver spi_st_driver = {
> >>          .driver = {
> >>                  .name = "spi-st",
> >> -               .pm = &spi_st_pm,
> >> +               .pm = pm_sleep_ptr(&spi_st_pm),
> >
> > This should use pm_ptr() instead, as spi_st_pm defines not only system
> > sleep ops, but also Runtime PM ops.
>
>
> Anyway, that is indeed a good catch.  I actually got lost between
> CONFIG_PM and CONFIG_PM_SLEEP, which is now clarified.
>
> I made a fix, but will send it after my summer break.

OK, looking forward to it!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
Re: [PATCH] spi: st: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()
Posted by Mark Brown 2 months, 3 weeks ago 
On Mon, 09 Jun 2025 23:21:09 +0200, Raphael Gallais-Pou wrote:
> Letting the compiler remove these functions when the kernel is built
> without CONFIG_PM_SLEEP support is simpler and less error prone than the
> use of #ifdef based kernel configuration guards.
> 
> 

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-next

Thanks!

[1/1] spi: st: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()
      commit: 6f8584a4826f01a55d3d0c4bbad5961f1de52fc9

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark
Re: [PATCH] spi: st: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()
Posted by Raphaël Gallais-Pou 2 months, 4 weeks ago 

Le 09/06/2025 à 23:21, Raphael Gallais-Pou a écrit :
> Letting the compiler remove these functions when the kernel is built
> without CONFIG_PM_SLEEP support is simpler and less error prone than the
> use of #ifdef based kernel configuration guards.
>
> Signed-off-by: Raphael Gallais-Pou <rgallaispou@gmail.com>

Hi Mark,

Gentle ping ! :)

Thanks,
Raphaël
> ---
>   drivers/spi/spi-st-ssc4.c | 14 +++++---------
>   1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/spi/spi-st-ssc4.c b/drivers/spi/spi-st-ssc4.c
> index 4cff976ab16fbdf3708ab870176a04f2628b501b..49ab4c515156fbabe0761028a5cb4770b61ca508 100644
> --- a/drivers/spi/spi-st-ssc4.c
> +++ b/drivers/spi/spi-st-ssc4.c
> @@ -378,8 +378,7 @@ static void spi_st_remove(struct platform_device *pdev)
>   	pinctrl_pm_select_sleep_state(&pdev->dev);
>   }
>   
> -#ifdef CONFIG_PM
> -static int spi_st_runtime_suspend(struct device *dev)
> +static int __maybe_unused spi_st_runtime_suspend(struct device *dev)
>   {
>   	struct spi_controller *host = dev_get_drvdata(dev);
>   	struct spi_st *spi_st = spi_controller_get_devdata(host);
> @@ -392,7 +391,7 @@ static int spi_st_runtime_suspend(struct device *dev)
>   	return 0;
>   }
>   
> -static int spi_st_runtime_resume(struct device *dev)
> +static int __maybe_unused spi_st_runtime_resume(struct device *dev)
>   {
>   	struct spi_controller *host = dev_get_drvdata(dev);
>   	struct spi_st *spi_st = spi_controller_get_devdata(host);
> @@ -403,10 +402,8 @@ static int spi_st_runtime_resume(struct device *dev)
>   
>   	return ret;
>   }
> -#endif
>   
> -#ifdef CONFIG_PM_SLEEP
> -static int spi_st_suspend(struct device *dev)
> +static int __maybe_unused spi_st_suspend(struct device *dev)
>   {
>   	struct spi_controller *host = dev_get_drvdata(dev);
>   	int ret;
> @@ -418,7 +415,7 @@ static int spi_st_suspend(struct device *dev)
>   	return pm_runtime_force_suspend(dev);
>   }
>   
> -static int spi_st_resume(struct device *dev)
> +static int __maybe_unused spi_st_resume(struct device *dev)
>   {
>   	struct spi_controller *host = dev_get_drvdata(dev);
>   	int ret;
> @@ -429,7 +426,6 @@ static int spi_st_resume(struct device *dev)
>   
>   	return pm_runtime_force_resume(dev);
>   }
> -#endif
>   
>   static const struct dev_pm_ops spi_st_pm = {
>   	SET_SYSTEM_SLEEP_PM_OPS(spi_st_suspend, spi_st_resume)
> @@ -445,7 +441,7 @@ MODULE_DEVICE_TABLE(of, stm_spi_match);
>   static struct platform_driver spi_st_driver = {
>   	.driver = {
>   		.name = "spi-st",
> -		.pm = &spi_st_pm,
> +		.pm = pm_sleep_ptr(&spi_st_pm),
>   		.of_match_table = of_match_ptr(stm_spi_match),
>   	},
>   	.probe = spi_st_probe,
>
> ---
> base-commit: 475c850a7fdd0915b856173186d5922899d65686
> change-id: 20250609-update_pm_macro-b6def2446904
>
> Best regards,
Re: [PATCH] spi: st: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()
Posted by Mark Brown 2 months, 4 weeks ago 
On Sun, Jul 13, 2025 at 03:56:48PM +0200, Raphaël Gallais-Pou wrote:
> Le 09/06/2025 à 23:21, Raphael Gallais-Pou a écrit :
> > Letting the compiler remove these functions when the kernel is built
> > without CONFIG_PM_SLEEP support is simpler and less error prone than the
> > use of #ifdef based kernel configuration guards.
> > 
> > Signed-off-by: Raphael Gallais-Pou <rgallaispou@gmail.com>
> 
> Hi Mark,
> 
> Gentle ping ! :)

Please don't send content free pings and please allow a reasonable time
for review.  People get busy, go on holiday, attend conferences and so 
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review.  If there have been
review comments then people may be waiting for those to be addressed.

Sending content free pings adds to the mail volume (if they are seen at
all) which is often the problem and since they can't be reviewed
directly if something has gone wrong you'll have to resend the patches
anyway, so sending again is generally a better approach though there are
some other maintainers who like them - if in doubt look at how patches
for the subsystem are normally handled.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.