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] [thread-next>] [day] [month] [year] [list]

Message-ID: <b3395812-6000-4e23-9b25-dbbc97547acd@gmail.com>
Date: Wed, 30 Jul 2025 17:38:05 +0200
From: Raphaël Gallais-Pou <rgallaispou@...il.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
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()



Le 28/07/2025 à 09:35, Geert Uytterhoeven a écrit :
> Hi Raphael,
> 
> 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.

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
>