sh: exports for delay.h

PATCH In review Al Viro 4 participants 6 messages Dec 22, 2024 – Dec 27, 2024

Subsystems (2)

SuperH Generic include/asm header files

lore

Share & tools

lore

Al Viro

author

2024-12-22 22:23 UTC #0 lore raw

PATCH 1/1 sh: exports for delay.h

__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 <[email protected]>

---

arch/sh/kernel/sh_ksyms_32.c +0 -4

index 5858936cb431..0b9b28144efe 100644
@@ -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);

arch/sh/lib/delay.c +5 -1

index dad8e6a54906..63cd32550b0c 100644
@@ -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);

John Paul Adrian Glaubitz

maintainer Reviewed-by on v1 1/1

2024-12-22 22:37 UTC #1 lore raw

Hi Al,

Quoted 59 lines · 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 <[email protected]>

---

arch/sh/kernel/sh_ksyms_32.c +0 -4

index 5858936cb431..0b9b28144efe 100644
@@ -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);

arch/sh/lib/delay.c +3 -0

index dad8e6a54906..63cd32550b0c 100644
@@ -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)
 {

arch/sh/lib/delay.c inline void __const_udelay(unsigned long xloops) · line 58

Quoted 10 lines

arch/sh/lib/delay.c +2 -1

 	__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 <[email protected]>

Adrian

Signature · 4 lines

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

Geert Uytterhoeven

2024-12-27 15:59 UTC #2 lore raw

In reply to Al Viro

Hi Al,

Quoted 8 lines · 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 <[email protected]>

Thanks for your patch!

Quoted 11 lines

arch/sh/lib/delay.c +1 -0

@@ -7,6 +7,7 @@
 
 #include <linux/sched.h>
 #include <linux/delay.h>
+#include <linux/export.h>
 
 void __delay(unsigned long loops)
 {

arch/sh/lib/delay.c void __delay(unsigned long loops) · line 33

Quoted 4 lines

arch/sh/lib/delay.c +1 -0

@@ -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

Signature · 5 lines

Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

Al Viro

author

2024-12-27 16:22 UTC #3 lore raw

Quoted 3 lines · Geert Uytterhoeven

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.

Geert Uytterhoeven

2024-12-27 18:20 UTC #4 lore raw

Hi Al,

Quoted 7 lines · Al Viro

Quoted 3 lines · Geert Uytterhoeven
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

Signature · 5 lines

Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

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

Randy Dunlap

2024-12-27 18:32 UTC #5 lore raw

Quoted 19 lines · Geert Uytterhoeven

Hi Al,
Quoted 7 lines · Al Viro
Quoted 3 lines · Geert Uytterhoeven
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.

Quoted 4 lines

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.

Signature · 1 line

~Randy