| Message ID | 20241222222259.GF1977892@ZenIV (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | sh: exports for delay.h | expand |
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
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
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.
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
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.
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);
__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> ---