[v2] misc/nvram: only declare mutex where it's used

[PATCH v2] misc/nvram: only declare mutex where it's used

Posted by Simon Richter 1 month, 1 week ago

The ioctls using this are platform specific compatibility code. On other
platforms, this generates a warning for an unused static variable.

Fixes: 20e07af71f34d ("powerpc: Adopt nvram module for PPC64")
Signed-off-by: Simon Richter <Simon.Richter@hogyros.de>
---
 drivers/char/nvram.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/char/nvram.c b/drivers/char/nvram.c
index 9eff426a9286..34bbeba073fe 100644
--- a/drivers/char/nvram.c
+++ b/drivers/char/nvram.c
@@ -53,7 +53,10 @@
 #include <asm/nvram.h>
 #endif
 
+#if defined(CONFIG_PPC32) || defined(CONFIG_X86) || defined(CONFIG_M68K)
 static DEFINE_MUTEX(nvram_mutex);
+#endif
+
 static DEFINE_SPINLOCK(nvram_state_lock);
 static int nvram_open_cnt;	/* #times opened */
 static int nvram_open_mode;	/* special open modes */
-- 
2.47.3

Re: [PATCH v2] misc/nvram: only declare mutex where it's used

Posted by Greg Kroah-Hartman 1 month, 1 week ago

On Sun, May 03, 2026 at 04:47:08PM +0900, Simon Richter wrote:
> The ioctls using this are platform specific compatibility code. On other
> platforms, this generates a warning for an unused static variable.
> 
> Fixes: 20e07af71f34d ("powerpc: Adopt nvram module for PPC64")
> Signed-off-by: Simon Richter <Simon.Richter@hogyros.de>
> ---
>  drivers/char/nvram.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/char/nvram.c b/drivers/char/nvram.c
> index 9eff426a9286..34bbeba073fe 100644
> --- a/drivers/char/nvram.c
> +++ b/drivers/char/nvram.c
> @@ -53,7 +53,10 @@
>  #include <asm/nvram.h>
>  #endif
>  
> +#if defined(CONFIG_PPC32) || defined(CONFIG_X86) || defined(CONFIG_M68K)
>  static DEFINE_MUTEX(nvram_mutex);
> +#endif
> +
>  static DEFINE_SPINLOCK(nvram_state_lock);
>  static int nvram_open_cnt;	/* #times opened */
>  static int nvram_open_mode;	/* special open modes */
> -- 
> 2.47.3
> 

Why has no one ever noticed this since 2019?  What changed recently to
cause this warning to show up?

thanks,

greg k-h

Re: [PATCH v2] misc/nvram: only declare mutex where it's used

Posted by Simon Richter 1 month, 1 week ago

Hi,

On 5/3/26 17:19, Greg Kroah-Hartman wrote:

> Why has no one ever noticed this since 2019?  What changed recently to
> cause this warning to show up?

  - CONFIG_NVRAM depends on X86 || HAVE_ARCH_NVRAM_OPS
  - HAVE_ARCH_NVRAM_OPS is set only on PPC and M68K

so nvram.c is compiled only with CONFIG_X86 || CONFIG_PPC || CONFIG_M68K

  - ppc32 and m68k actually use the mutex

so it shows up only on PPC64 if you build with CONFIG_NVRAM, and is only 
an error if you also build with CONFIG_WERROR as well.

The warning on ppc64 in introduced in 20e07af71f34d, so 95ac14b8a3281 
does not need a fix as the mutex is actually used in all configurations 
where the driver can be enabled.

It's also debatable if the Kconfig is optimal -- HAVE_ARCH_NVRAM_OPS is 
defined in arch/ and used only in this one place, so either x86 should 
set it as well, or it's removed and CONFIG_NVRAM gets an explicit 
architecture list.

    Simon

Re: [PATCH v2] misc/nvram: only declare mutex where it's used

Posted by Arnd Bergmann 1 month, 1 week ago

On Sun, May 3, 2026, at 09:47, Simon Richter wrote:
> The ioctls using this are platform specific compatibility code. On other
> platforms, this generates a warning for an unused static variable.
>
> Fixes: 20e07af71f34d ("powerpc: Adopt nvram module for PPC64")
> Signed-off-by: Simon Richter <Simon.Richter@hogyros.de>
> ---
>  drivers/char/nvram.c | 3 +++
>  1 file changed, 3 insertions(+)

I think the better fix is "[PATCH v5] char/nvram: Remove redundant
nvram_mutex", see

https://lore.kernel.org/all/20260428061540.73668-1-venkat88@linux.ibm.com/

      Arnd