power: reset: at91: mark at91_wakeup_status non-__init

Message ID 20231212214658.213510-1-arnd@kernel.org
State New
Headers
Series power: reset: at91: mark at91_wakeup_status non-__init |

Commit Message

Arnd Bergmann Dec. 12, 2023, 9:46 p.m. UTC
  From: Arnd Bergmann <arnd@arndb.de>

Two copies of the at91_wakeup_status() function are called by the
respective probe() callbacks and are marked __init, but the probe
functions are no longer annotated that way. This works with gcc because
the functions always get inlined, but clang keeps them separate, which
can lead to executing freed memory:

WARNING: modpost: vmlinux: section mismatch in reference: at91_poweroff_probe+0x80 (section: .text) -> at91_wakeup_status (section: .init.text)
WARNING: modpost: vmlinux: section mismatch in reference: at91_shdwc_probe+0xcc (section: .text) -> at91_wakeup_status (section: .init.text)

Drop the incorrect annotation on these.

Fixes: 099806de68b7 ("power: reset: at91-poweroff: Stop using module_platform_driver_probe()")
Fixes: dde74a5de817 ("power: reset: at91-sama5d2_shdwc: Stop using module_platform_driver_probe()")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/power/reset/at91-poweroff.c      | 2 +-
 drivers/power/reset/at91-sama5d2_shdwc.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
  

Comments

Nathan Chancellor Dec. 12, 2023, 9:50 p.m. UTC | #1
On Tue, Dec 12, 2023 at 10:46:49PM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> Two copies of the at91_wakeup_status() function are called by the
> respective probe() callbacks and are marked __init, but the probe
> functions are no longer annotated that way. This works with gcc because
> the functions always get inlined, but clang keeps them separate, which
> can lead to executing freed memory:
> 
> WARNING: modpost: vmlinux: section mismatch in reference: at91_poweroff_probe+0x80 (section: .text) -> at91_wakeup_status (section: .init.text)
> WARNING: modpost: vmlinux: section mismatch in reference: at91_shdwc_probe+0xcc (section: .text) -> at91_wakeup_status (section: .init.text)
> 
> Drop the incorrect annotation on these.
> 
> Fixes: 099806de68b7 ("power: reset: at91-poweroff: Stop using module_platform_driver_probe()")
> Fixes: dde74a5de817 ("power: reset: at91-sama5d2_shdwc: Stop using module_platform_driver_probe()")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks, I sent the same change three weeks ago at this point:

https://lore.kernel.org/20231120-fix-at91-modpost-warnings-v1-1-813671933863@kernel.org/

Your commit message is a little better than mine and I don't really care
which one goes in but it would be good if this could get cleared up
soon...

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  drivers/power/reset/at91-poweroff.c      | 2 +-
>  drivers/power/reset/at91-sama5d2_shdwc.c | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/power/reset/at91-poweroff.c b/drivers/power/reset/at91-poweroff.c
> index 126e774e210c..93eece027865 100644
> --- a/drivers/power/reset/at91-poweroff.c
> +++ b/drivers/power/reset/at91-poweroff.c
> @@ -57,7 +57,7 @@ static struct shdwc {
>  	void __iomem *mpddrc_base;
>  } at91_shdwc;
>  
> -static void __init at91_wakeup_status(struct platform_device *pdev)
> +static void at91_wakeup_status(struct platform_device *pdev)
>  {
>  	const char *reason;
>  	u32 reg = readl(at91_shdwc.shdwc_base + AT91_SHDW_SR);
> diff --git a/drivers/power/reset/at91-sama5d2_shdwc.c b/drivers/power/reset/at91-sama5d2_shdwc.c
> index af95c7b39cb3..959ce0dbe91d 100644
> --- a/drivers/power/reset/at91-sama5d2_shdwc.c
> +++ b/drivers/power/reset/at91-sama5d2_shdwc.c
> @@ -107,7 +107,7 @@ static const unsigned long long sdwc_dbc_period[] = {
>  	0, 3, 32, 512, 4096, 32768,
>  };
>  
> -static void __init at91_wakeup_status(struct platform_device *pdev)
> +static void at91_wakeup_status(struct platform_device *pdev)
>  {
>  	struct shdwc *shdw = platform_get_drvdata(pdev);
>  	const struct reg_config *rcfg = shdw->rcfg;
> -- 
> 2.39.2
> 
>
  
Uwe Kleine-König Dec. 13, 2023, 7:25 a.m. UTC | #2
Hello,

[dropped Justin Stitt from Cc, their email address bounced for me in the
past]

On Tue, Dec 12, 2023 at 02:50:02PM -0700, Nathan Chancellor wrote:
> On Tue, Dec 12, 2023 at 10:46:49PM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > Two copies of the at91_wakeup_status() function are called by the
> > respective probe() callbacks and are marked __init, but the probe
> > functions are no longer annotated that way. This works with gcc because
> > the functions always get inlined, but clang keeps them separate, which
> > can lead to executing freed memory:
> > 
> > WARNING: modpost: vmlinux: section mismatch in reference: at91_poweroff_probe+0x80 (section: .text) -> at91_wakeup_status (section: .init.text)
> > WARNING: modpost: vmlinux: section mismatch in reference: at91_shdwc_probe+0xcc (section: .text) -> at91_wakeup_status (section: .init.text)
> > 
> > Drop the incorrect annotation on these.
> > 
> > Fixes: 099806de68b7 ("power: reset: at91-poweroff: Stop using module_platform_driver_probe()")
> > Fixes: dde74a5de817 ("power: reset: at91-sama5d2_shdwc: Stop using module_platform_driver_probe()")
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> 
> Thanks, I sent the same change three weeks ago at this point:
> 
> https://lore.kernel.org/20231120-fix-at91-modpost-warnings-v1-1-813671933863@kernel.org/
> 
> Your commit message is a little better than mine and I don't really care
> which one goes in but it would be good if this could get cleared up
> soon...
> 
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>

I don't care either. Given the change is identical (Nathan's submission
even has "index 126e774e210c..93eece027865 100644" which exactly matches
Arnd's patch), I'll forward my Reviewed-by: tag to here:

Reviewed-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

Thanks
Uwe
  
Nicolas Ferre Dec. 13, 2023, 1:01 p.m. UTC | #3
On 13/12/2023 at 08:25, Uwe Kleine-König wrote:
> On Tue, Dec 12, 2023 at 02:50:02PM -0700, Nathan Chancellor wrote:
>> On Tue, Dec 12, 2023 at 10:46:49PM +0100, Arnd Bergmann wrote:
>>> From: Arnd Bergmann<arnd@arndb.de>
>>>
>>> Two copies of the at91_wakeup_status() function are called by the
>>> respective probe() callbacks and are marked __init, but the probe
>>> functions are no longer annotated that way. This works with gcc because
>>> the functions always get inlined, but clang keeps them separate, which
>>> can lead to executing freed memory:
>>>
>>> WARNING: modpost: vmlinux: section mismatch in reference: at91_poweroff_probe+0x80 (section: .text) -> at91_wakeup_status (section: .init.text)
>>> WARNING: modpost: vmlinux: section mismatch in reference: at91_shdwc_probe+0xcc (section: .text) -> at91_wakeup_status (section: .init.text)
>>>
>>> Drop the incorrect annotation on these.
>>>
>>> Fixes: 099806de68b7 ("power: reset: at91-poweroff: Stop using module_platform_driver_probe()")
>>> Fixes: dde74a5de817 ("power: reset: at91-sama5d2_shdwc: Stop using module_platform_driver_probe()")
>>> Signed-off-by: Arnd Bergmann<arnd@arndb.de>
>> Thanks, I sent the same change three weeks ago at this point:
>>
>> https://lore.kernel.org/20231120-fix-at91-modpost-warnings-v1-1-813671933863@kernel.org/
>>
>> Your commit message is a little better than mine and I don't really care
>> which one goes in but it would be good if this could get cleared up
>> soon...
>>
>> Reviewed-by: Nathan Chancellor<nathan@kernel.org>
> I don't care either. Given the change is identical (Nathan's submission
> even has "index 126e774e210c..93eece027865 100644" which exactly matches
> Arnd's patch), I'll forward my Reviewed-by: tag to here:
> 
> Reviewed-by: Uwe Kleine-König<u.kleine-koenig@pengutronix.de>

Likewise:
Acked-by: Nicolas Ferre <nicolas.ferre@microchip.com>

Use whichever is preferred.
Regards,
   Nicolas
  

Patch

diff --git a/drivers/power/reset/at91-poweroff.c b/drivers/power/reset/at91-poweroff.c
index 126e774e210c..93eece027865 100644
--- a/drivers/power/reset/at91-poweroff.c
+++ b/drivers/power/reset/at91-poweroff.c
@@ -57,7 +57,7 @@  static struct shdwc {
 	void __iomem *mpddrc_base;
 } at91_shdwc;
 
-static void __init at91_wakeup_status(struct platform_device *pdev)
+static void at91_wakeup_status(struct platform_device *pdev)
 {
 	const char *reason;
 	u32 reg = readl(at91_shdwc.shdwc_base + AT91_SHDW_SR);
diff --git a/drivers/power/reset/at91-sama5d2_shdwc.c b/drivers/power/reset/at91-sama5d2_shdwc.c
index af95c7b39cb3..959ce0dbe91d 100644
--- a/drivers/power/reset/at91-sama5d2_shdwc.c
+++ b/drivers/power/reset/at91-sama5d2_shdwc.c
@@ -107,7 +107,7 @@  static const unsigned long long sdwc_dbc_period[] = {
 	0, 3, 32, 512, 4096, 32768,
 };
 
-static void __init at91_wakeup_status(struct platform_device *pdev)
+static void at91_wakeup_status(struct platform_device *pdev)
 {
 	struct shdwc *shdw = platform_get_drvdata(pdev);
 	const struct reg_config *rcfg = shdw->rcfg;