linux-kernel - Re: [PATCH] spi: st: Switch from CONFIG_PM_SLEEP guards to pm_sleep

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMuHMdW9jbV344NDdq41ypqdmeh_eqdrTvnAn8PZcfqHc1OESA@mail.gmail.com>
Date: Wed, 30 Jul 2025 20:32:03 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Raphaël Gallais-Pou <rgallaispou@...il.com>
Cc: Mark Brown <broonie@...nel.org>, linux-spi@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] spi: st: Switch from CONFIG_PM_SLEEP guards to pm_sleep_ptr()

Hi Raphaël,

On Wed, 30 Jul 2025 at 17:38, Raphaël Gallais-Pou <rgallaispou@...il.com> wrote:
> Le 28/07/2025 à 09:35, Geert Uytterhoeven a écrit :
> > On Tue, 10 Jun 2025 at 16:59, Raphael Gallais-Pou <rgallaispou@...il.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@...il.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@...ux-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