[PATCH] sh: exports for delay.h

[PATCH] sh: exports for delay.h

[Al Viro]

	__delay() is either exported or exists as a static inline
on all architectures - except sh.

	Add the missing export of __delay(), move the exports of
the rest of that bunch from sh_ksyms32.c to the place where all
of them are defined (i.e. arch/sh/lib/delay.c).

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
diff --git a/arch/sh/kernel/sh_ksyms_32.c b/arch/sh/kernel/sh_ksyms_32.c
index 5858936cb431..0b9b28144efe 100644
--- a/arch/sh/kernel/sh_ksyms_32.c
+++ b/arch/sh/kernel/sh_ksyms_32.c
@@ -2,7 +2,6 @@
 #include <linux/module.h>
 #include <linux/string.h>
 #include <linux/uaccess.h>
-#include <linux/delay.h>
 #include <linux/mm.h>
 #include <asm/checksum.h>
 #include <asm/sections.h>
@@ -12,9 +11,6 @@ EXPORT_SYMBOL(memcpy);
 EXPORT_SYMBOL(memset);
 EXPORT_SYMBOL(memmove);
 EXPORT_SYMBOL(__copy_user);
-EXPORT_SYMBOL(__udelay);
-EXPORT_SYMBOL(__ndelay);
-EXPORT_SYMBOL(__const_udelay);
 EXPORT_SYMBOL(strlen);
 EXPORT_SYMBOL(csum_partial);
 EXPORT_SYMBOL(csum_partial_copy_generic);
diff --git a/arch/sh/lib/delay.c b/arch/sh/lib/delay.c
index dad8e6a54906..63cd32550b0c 100644
--- a/arch/sh/lib/delay.c
+++ b/arch/sh/lib/delay.c
@@ -7,6 +7,7 @@
 
 #include <linux/sched.h>
 #include <linux/delay.h>
+#include <linux/export.h>
 
 void __delay(unsigned long loops)
 {
@@ -29,6 +30,7 @@ void __delay(unsigned long loops)
 		: "0" (loops)
 		: "t");
 }
+EXPORT_SYMBOL(__delay);
 
 inline void __const_udelay(unsigned long xloops)
 {
@@ -41,14 +43,16 @@ inline void __const_udelay(unsigned long xloops)
 		: "macl", "mach");
 	__delay(++xloops);
 }
+EXPORT_SYMBOL(__const_udelay);
 
 void __udelay(unsigned long usecs)
 {
 	__const_udelay(usecs * 0x000010c6);  /* 2**32 / 1000000 */
 }
+EXPORT_SYMBOL(__udelay);
 
 void __ndelay(unsigned long nsecs)
 {
 	__const_udelay(nsecs * 0x00000005);
 }
-
+EXPORT_SYMBOL(__ndelay);

Re: [PATCH] sh: exports for delay.h

[John Paul Adrian Glaubitz]

Hi Al,

On Sun, 2024-12-22 at 22:22 +0000, Al Viro wrote:
> 	__delay() is either exported or exists as a static inline
> on all architectures - except sh.
> 
> 	Add the missing export of __delay(), move the exports of
> the rest of that bunch from sh_ksyms32.c to the place where all
> of them are defined (i.e. arch/sh/lib/delay.c).
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
> diff --git a/arch/sh/kernel/sh_ksyms_32.c b/arch/sh/kernel/sh_ksyms_32.c
> index 5858936cb431..0b9b28144efe 100644
> --- a/arch/sh/kernel/sh_ksyms_32.c
> +++ b/arch/sh/kernel/sh_ksyms_32.c
> @@ -2,7 +2,6 @@
>  #include <linux/module.h>
>  #include <linux/string.h>
>  #include <linux/uaccess.h>
> -#include <linux/delay.h>
>  #include <linux/mm.h>
>  #include <asm/checksum.h>
>  #include <asm/sections.h>
> @@ -12,9 +11,6 @@ EXPORT_SYMBOL(memcpy);
>  EXPORT_SYMBOL(memset);
>  EXPORT_SYMBOL(memmove);
>  EXPORT_SYMBOL(__copy_user);
> -EXPORT_SYMBOL(__udelay);
> -EXPORT_SYMBOL(__ndelay);
> -EXPORT_SYMBOL(__const_udelay);
>  EXPORT_SYMBOL(strlen);
>  EXPORT_SYMBOL(csum_partial);
>  EXPORT_SYMBOL(csum_partial_copy_generic);
> diff --git a/arch/sh/lib/delay.c b/arch/sh/lib/delay.c
> index dad8e6a54906..63cd32550b0c 100644
> --- a/arch/sh/lib/delay.c
> +++ b/arch/sh/lib/delay.c
> @@ -7,6 +7,7 @@
>  
>  #include <linux/sched.h>
>  #include <linux/delay.h>
> +#include <linux/export.h>
>  
>  void __delay(unsigned long loops)
>  {
> @@ -29,6 +30,7 @@ void __delay(unsigned long loops)
>  		: "0" (loops)
>  		: "t");
>  }
> +EXPORT_SYMBOL(__delay);
>  
>  inline void __const_udelay(unsigned long xloops)
>  {
> @@ -41,14 +43,16 @@ inline void __const_udelay(unsigned long xloops)
>  		: "macl", "mach");
>  	__delay(++xloops);
>  }
> +EXPORT_SYMBOL(__const_udelay);
>  
>  void __udelay(unsigned long usecs)
>  {
>  	__const_udelay(usecs * 0x000010c6);  /* 2**32 / 1000000 */
>  }
> +EXPORT_SYMBOL(__udelay);
>  
>  void __ndelay(unsigned long nsecs)
>  {
>  	__const_udelay(nsecs * 0x00000005);
>  }
> -
> +EXPORT_SYMBOL(__ndelay);

Thanks, this looks good and I'll pick it up for sh-linux 6.14, although
I'm probably going to make the subject a little more descriptive ;-).

Reviewed-by: John Paul Adrian Glaubitz <glaubitz@physik.fu-berlin.de>

Adrian

-- 
 .''`.  John Paul Adrian Glaubitz
: :' :  Debian Developer
`. `'   Physicist
  `-    GPG: 62FF 8A75 84E0 2956 9546  0006 7426 3B37 F5B5 F913

Re: [PATCH] sh: exports for delay.h

[Geert Uytterhoeven]

Hi Al,

On Sun, Dec 22, 2024 at 11:23 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>         __delay() is either exported or exists as a static inline
> on all architectures - except sh.
>
>         Add the missing export of __delay(), move the exports of
> the rest of that bunch from sh_ksyms32.c to the place where all
> of them are defined (i.e. arch/sh/lib/delay.c).
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Thanks for your patch!

> --- a/arch/sh/lib/delay.c
> +++ b/arch/sh/lib/delay.c
> @@ -7,6 +7,7 @@
>
>  #include <linux/sched.h>
>  #include <linux/delay.h>
> +#include <linux/export.h>
>
>  void __delay(unsigned long loops)
>  {
> @@ -29,6 +30,7 @@ void __delay(unsigned long loops)
>                 : "0" (loops)
>                 : "t");
>  }
> +EXPORT_SYMBOL(__delay);

Please do not export __delay, as it is an internal implementation detail.
Drivers should not call __delay() directly, as it has non-standardized
semantics, or may not even exist.

Unfortunately this comes up once in a while, cfr. commit
7619f957dc8cb8b2 ("Revert "sh: add missing EXPORT_SYMBOL() for
__delay"").

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] sh: exports for delay.h

[Al Viro]

On Fri, Dec 27, 2024 at 04:58:58PM +0100, Geert Uytterhoeven wrote:

> Please do not export __delay, as it is an internal implementation detail.
> Drivers should not call __delay() directly, as it has non-standardized
> semantics, or may not even exist.

As the matter of fact, it *does* exist and is either exported or a static
inline with everything used by it exported - on every architecture except sh.

Re: [PATCH] sh: exports for delay.h

[Geert Uytterhoeven]

Hi Al,

On Fri, Dec 27, 2024 at 5:22 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Fri, Dec 27, 2024 at 04:58:58PM +0100, Geert Uytterhoeven wrote:
> > Please do not export __delay, as it is an internal implementation detail.
> > Drivers should not call __delay() directly, as it has non-standardized
> > semantics, or may not even exist.
>
> As the matter of fact, it *does* exist and is either exported or a static
> inline with everything used by it exported - on every architecture except sh.

OK, it does exist, because init/calibrate.c needs it. But that should
be the sole user outside architecture-specific code.

Unlike some other architectures, SH does not emit calls to __delay()
from various (static inline) *delay() functions.  Hence there is no
need to export it to modules. Heck, the only reason it cannot be made
static is because init/calibrate.c needs it.

Unfortunately there are still a few drivers that call __delay()
directly (usually with hardcoded constants, how portable, and
cpufreq-unfriendly), which causes people to try once in a while to
"fix the build" by re-adding the export on SH :-(

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] sh: exports for delay.h

[Randy Dunlap]

On 12/27/24 10:20 AM, Geert Uytterhoeven wrote:
> Hi Al,
> 
> On Fri, Dec 27, 2024 at 5:22 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>> On Fri, Dec 27, 2024 at 04:58:58PM +0100, Geert Uytterhoeven wrote:
>>> Please do not export __delay, as it is an internal implementation detail.
>>> Drivers should not call __delay() directly, as it has non-standardized
>>> semantics, or may not even exist.
>>
>> As the matter of fact, it *does* exist and is either exported or a static
>> inline with everything used by it exported - on every architecture except sh.
> 
> OK, it does exist, because init/calibrate.c needs it. But that should
> be the sole user outside architecture-specific code.
> 
> Unlike some other architectures, SH does not emit calls to __delay()
> from various (static inline) *delay() functions.  Hence there is no
> need to export it to modules. Heck, the only reason it cannot be made
> static is because init/calibrate.c needs it.
> 
> Unfortunately there are still a few drivers that call __delay()
> directly (usually with hardcoded constants, how portable, and
> cpufreq-unfriendly), which causes people to try once in a while to
> "fix the build" by re-adding the export on SH :-(

Yeah, I did that in 2021 because of:

ERROR: modpost: "__delay" [drivers/net/mdio/mdio-cavium.ko] undefined!

mdio-cavium still uses __delay() so I guess it still breaks an SH build.

-- 
~Randy