[PATCH] sh: add missing EXPORT_SYMBOL() for __delay

[PATCH] sh: add missing EXPORT_SYMBOL() for __delay

[morimoto]

From: morimoto <kuninori.morimoto.gx@renesas.com>

__delay() is used from kernel module.
We need EXPORT_SYMBOL(), otherwise we will get compile error.

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

Signed-off-by: morimoto <kuninori.morimoto.gx@renesas.com>
Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/sh/lib/delay.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/sh/lib/delay.c b/arch/sh/lib/delay.c
index dad8e6a..540e670 100644
--- a/arch/sh/lib/delay.c
+++ b/arch/sh/lib/delay.c
@@ -29,6 +29,7 @@ void __delay(unsigned long loops)
 		: "0" (loops)
 		: "t");
 }
+EXPORT_SYMBOL(__delay);
 
 inline void __const_udelay(unsigned long xloops)
 {
-- 
2.7.4

Re: [PATCH] sh: add missing EXPORT_SYMBOL() for __delay

[Kuninori Morimoto]

Hi

> From: morimoto <kuninori.morimoto.gx@renesas.com>
> 
> __delay() is used from kernel module.
> We need EXPORT_SYMBOL(), otherwise we will get compile error.
> 
> ERROR: "__delay" [drivers/net/phy/mdio-cavium.ko] undefined!
> 
> Signed-off-by: morimoto <kuninori.morimoto.gx@renesas.com>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---

I'm so sorry.
The Signed is strange.
I will re-post patch.
Please drop it.

Thank you for your help !!
Best regards
---
Kuninori Morimoto

[PATCH] sh: add missing EXPORT_SYMBOL() for __delay

[Kuninori Morimoto]

From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

__delay() is used from kernel module.
We need EXPORT_SYMBOL(), otherwise we will get compile error.

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

Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
---
 arch/sh/lib/delay.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/sh/lib/delay.c b/arch/sh/lib/delay.c
index dad8e6a..540e670 100644
--- a/arch/sh/lib/delay.c
+++ b/arch/sh/lib/delay.c
@@ -29,6 +29,7 @@ void __delay(unsigned long loops)
 		: "0" (loops)
 		: "t");
 }
+EXPORT_SYMBOL(__delay);
 
 inline void __const_udelay(unsigned long xloops)
 {
-- 
2.7.4

Re: [PATCH] sh: add missing EXPORT_SYMBOL() for __delay

[Geert Uytterhoeven]

Hi Morimoto-san,

On Thu, Dec 12, 2019 at 3:38 AM Kuninori Morimoto
<kuninori.morimoto.gx@renesas.com> wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>
> __delay() is used from kernel module.
> We need EXPORT_SYMBOL(), otherwise we will get compile error.
>
> ERROR: "__delay" [drivers/net/phy/mdio-cavium.ko] undefined!
>
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

Thanks for your patch!

> --- a/arch/sh/lib/delay.c
> +++ b/arch/sh/lib/delay.c
> @@ -29,6 +29,7 @@ void __delay(unsigned long loops)
>                 : "0" (loops)
>                 : "t");
>  }
> +EXPORT_SYMBOL(__delay);
>
>  inline void __const_udelay(unsigned long xloops)
>  {

I believe the correct fix is make drivers/net/phy/mdio-cavium.c use one
of [nmu]delay() instead.

Cfr.
https://lore.kernel.org/lkml/CAMuHMdUERaoHLNKi03zCuYi7NevgBFjXrV=pt0Yy=HOeRiL25Q@mail.gmail.com/

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: add missing EXPORT_SYMBOL() for __delay

[Kuninori Morimoto]

Hi Geert

Thank you for your feedback

> > --- a/arch/sh/lib/delay.c
> > +++ b/arch/sh/lib/delay.c
> > @@ -29,6 +29,7 @@ void __delay(unsigned long loops)
> >                 : "0" (loops)
> >                 : "t");
> >  }
> > +EXPORT_SYMBOL(__delay);
> >
> >  inline void __const_udelay(unsigned long xloops)
> >  {
> 
> I believe the correct fix is make drivers/net/phy/mdio-cavium.c use one
> of [nmu]delay() instead.
> 
> Cfr.
> https://lore.kernel.org/lkml/CAMuHMdUERaoHLNKi03zCuYi7NevgBFjXrV=pt0Yy=HOeRiL25Q@mail.gmail.com/

OK, make sense.
I did it because some architecture is using EXPORT_SYMBOL() to __delay.

Thank you for your help !!
Best regards
---
Kuninori Morimoto

Re: [PATCH] sh: add missing EXPORT_SYMBOL() for __delay

[Yoshinori Sato]

On Thu, 12 Dec 2019 11:38:43 +0900,
Kuninori Morimoto wrote:
> 
> 
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> __delay() is used from kernel module.
> We need EXPORT_SYMBOL(), otherwise we will get compile error.
> 
> ERROR: "__delay" [drivers/net/phy/mdio-cavium.ko] undefined!
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> ---
>  arch/sh/lib/delay.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/sh/lib/delay.c b/arch/sh/lib/delay.c
> index dad8e6a..540e670 100644
> --- a/arch/sh/lib/delay.c
> +++ b/arch/sh/lib/delay.c
> @@ -29,6 +29,7 @@ void __delay(unsigned long loops)
>  		: "0" (loops)
>  		: "t");
>  }
> +EXPORT_SYMBOL(__delay);
>  
>  inline void __const_udelay(unsigned long xloops)
>  {
> -- 
> 2.7.4
> 

Applied sh-next.
Thanks.

-- 
Yosinori Sato

Re: [PATCH] sh: add missing EXPORT_SYMBOL() for __delay

[Guenter Roeck]

On Thu, Dec 12, 2019 at 11:38:43AM +0900, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> __delay() is used from kernel module.
> We need EXPORT_SYMBOL(), otherwise we will get compile error.
> 
> ERROR: "__delay" [drivers/net/phy/mdio-cavium.ko] undefined!
> 
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>

I must admit that this patch completely baffles me. __delay was
already exported, only elsewhere in the file. With this patch
in place, it is exported twice, and all sh builds in -next fail
with

In file included from include/linux/linkage.h:7,
                 from arch/sh/include/asm/bug.h:5,
                 from include/linux/bug.h:5,
                 from include/linux/thread_info.h:12,
                 from include/asm-generic/current.h:5,
                 from ./arch/sh/include/generated/asm/current.h:1,
                 from include/linux/sched.h:12,
                 from arch/sh/lib/delay.c:8:
include/linux/export.h:67:36: error: redefinition of '__ksymtab___delay'

Guenter

> ---
>  arch/sh/lib/delay.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/sh/lib/delay.c b/arch/sh/lib/delay.c
> index dad8e6a..540e670 100644
> --- a/arch/sh/lib/delay.c
> +++ b/arch/sh/lib/delay.c
> @@ -29,6 +29,7 @@ void __delay(unsigned long loops)
>  		: "0" (loops)
>  		: "t");
>  }
> +EXPORT_SYMBOL(__delay);
>  
>  inline void __const_udelay(unsigned long xloops)
>  {

Re: [PATCH] sh: add missing EXPORT_SYMBOL() for __delay

[Rich Felker]

On Tue, Jul 21, 2020 at 07:38:40PM -0700, Guenter Roeck wrote:
> On Thu, Dec 12, 2019 at 11:38:43AM +0900, Kuninori Morimoto wrote:
> > From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > 
> > __delay() is used from kernel module.
> > We need EXPORT_SYMBOL(), otherwise we will get compile error.
> > 
> > ERROR: "__delay" [drivers/net/phy/mdio-cavium.ko] undefined!
> > 
> > Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> 
> I must admit that this patch completely baffles me. __delay was
> already exported, only elsewhere in the file. With this patch
> in place, it is exported twice, and all sh builds in -next fail
> with
> 
> In file included from include/linux/linkage.h:7,
>                  from arch/sh/include/asm/bug.h:5,
>                  from include/linux/bug.h:5,
>                  from include/linux/thread_info.h:12,
>                  from include/asm-generic/current.h:5,
>                  from ./arch/sh/include/generated/asm/current.h:1,
>                  from include/linux/sched.h:12,
>                  from arch/sh/lib/delay.c:8:
> include/linux/export.h:67:36: error: redefinition of '__ksymtab___delay'
> 
> Guenter
> 
> > ---
> >  arch/sh/lib/delay.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/arch/sh/lib/delay.c b/arch/sh/lib/delay.c
> > index dad8e6a..540e670 100644
> > --- a/arch/sh/lib/delay.c
> > +++ b/arch/sh/lib/delay.c
> > @@ -29,6 +29,7 @@ void __delay(unsigned long loops)
> >  		: "0" (loops)
> >  		: "t");
> >  }
> > +EXPORT_SYMBOL(__delay);
> >  
> >  inline void __const_udelay(unsigned long xloops)
> >  {

I presently have a revert of this commit in queue for next. If it's
sufficiently breaking (and especially if there are other regressions
that need to be fixed, see the pmd_free thing) I could try to get it
in for 5.8 still but that's getting a bit late.

Rich

Re: [PATCH] sh: add missing EXPORT_SYMBOL() for __delay

[Guenter Roeck]

Rich,

On 7/22/20 3:52 PM, Rich Felker wrote:
> On Tue, Jul 21, 2020 at 07:38:40PM -0700, Guenter Roeck wrote:
>> On Thu, Dec 12, 2019 at 11:38:43AM +0900, Kuninori Morimoto wrote:
>>> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>>>
>>> __delay() is used from kernel module.
>>> We need EXPORT_SYMBOL(), otherwise we will get compile error.
>>>
>>> ERROR: "__delay" [drivers/net/phy/mdio-cavium.ko] undefined!
>>>
>>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
>>
>> I must admit that this patch completely baffles me. __delay was
>> already exported, only elsewhere in the file. With this patch
>> in place, it is exported twice, and all sh builds in -next fail
>> with
>>
>> In file included from include/linux/linkage.h:7,
>>                  from arch/sh/include/asm/bug.h:5,
>>                  from include/linux/bug.h:5,
>>                  from include/linux/thread_info.h:12,
>>                  from include/asm-generic/current.h:5,
>>                  from ./arch/sh/include/generated/asm/current.h:1,
>>                  from include/linux/sched.h:12,
>>                  from arch/sh/lib/delay.c:8:
>> include/linux/export.h:67:36: error: redefinition of '__ksymtab___delay'
>>
>> Guenter
>>
>>> ---
>>>  arch/sh/lib/delay.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/arch/sh/lib/delay.c b/arch/sh/lib/delay.c
>>> index dad8e6a..540e670 100644
>>> --- a/arch/sh/lib/delay.c
>>> +++ b/arch/sh/lib/delay.c
>>> @@ -29,6 +29,7 @@ void __delay(unsigned long loops)
>>>  		: "0" (loops)
>>>  		: "t");
>>>  }
>>> +EXPORT_SYMBOL(__delay);
>>>  
>>>  inline void __const_udelay(unsigned long xloops)
>>>  {
> 
> I presently have a revert of this commit in queue for next. If it's
> sufficiently breaking (and especially if there are other regressions
> that need to be fixed, see the pmd_free thing) I could try to get it
> in for 5.8 still but that's getting a bit late.
> 

The patch in mainline is ok. It appears that it has been applied
again in -next.

"git log --oneline v5.7.. arch/sh/lib/delay.c" on top of next-20200721
reports:

ee0e4f15dfd4 (origin/akpm) sh: add missing EXPORT_SYMBOL() for __delay
d1f56f318d23 sh: add missing EXPORT_SYMBOL() for __delay

Maybe it just needs to be dropped from the akpm tree in -next ?

Thanks,
Guenter

Re: [PATCH] sh: add missing EXPORT_SYMBOL() for __delay

[Rich Felker]

On Wed, Jul 22, 2020 at 04:52:56PM -0700, Guenter Roeck wrote:
> Rich,
> 
> On 7/22/20 3:52 PM, Rich Felker wrote:
> > On Tue, Jul 21, 2020 at 07:38:40PM -0700, Guenter Roeck wrote:
> >> On Thu, Dec 12, 2019 at 11:38:43AM +0900, Kuninori Morimoto wrote:
> >>> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> >>>
> >>> __delay() is used from kernel module.
> >>> We need EXPORT_SYMBOL(), otherwise we will get compile error.
> >>>
> >>> ERROR: "__delay" [drivers/net/phy/mdio-cavium.ko] undefined!
> >>>
> >>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> >>
> >> I must admit that this patch completely baffles me. __delay was
> >> already exported, only elsewhere in the file. With this patch
> >> in place, it is exported twice, and all sh builds in -next fail
> >> with
> >>
> >> In file included from include/linux/linkage.h:7,
> >>                  from arch/sh/include/asm/bug.h:5,
> >>                  from include/linux/bug.h:5,
> >>                  from include/linux/thread_info.h:12,
> >>                  from include/asm-generic/current.h:5,
> >>                  from ./arch/sh/include/generated/asm/current.h:1,
> >>                  from include/linux/sched.h:12,
> >>                  from arch/sh/lib/delay.c:8:
> >> include/linux/export.h:67:36: error: redefinition of '__ksymtab___delay'
> >>
> >> Guenter
> >>
> >>> ---
> >>>  arch/sh/lib/delay.c | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/arch/sh/lib/delay.c b/arch/sh/lib/delay.c
> >>> index dad8e6a..540e670 100644
> >>> --- a/arch/sh/lib/delay.c
> >>> +++ b/arch/sh/lib/delay.c
> >>> @@ -29,6 +29,7 @@ void __delay(unsigned long loops)
> >>>  		: "0" (loops)
> >>>  		: "t");
> >>>  }
> >>> +EXPORT_SYMBOL(__delay);
> >>>  
> >>>  inline void __const_udelay(unsigned long xloops)
> >>>  {
> > 
> > I presently have a revert of this commit in queue for next. If it's
> > sufficiently breaking (and especially if there are other regressions
> > that need to be fixed, see the pmd_free thing) I could try to get it
> > in for 5.8 still but that's getting a bit late.
> > 
> 
> The patch in mainline is ok. It appears that it has been applied
> again in -next.
> 
> "git log --oneline v5.7.. arch/sh/lib/delay.c" on top of next-20200721
> reports:
> 
> ee0e4f15dfd4 (origin/akpm) sh: add missing EXPORT_SYMBOL() for __delay
> d1f56f318d23 sh: add missing EXPORT_SYMBOL() for __delay
> 
> Maybe it just needs to be dropped from the akpm tree in -next ?

Could it be a bad merge or patch applied twice or something? I don't
see how the symbol is being exported twice. The argument to revert the
patch still seems correct.

Looking at current linux-next, ba722ca780 from akpm is re-adding the
patch after it gets reverted. Andrew, could you drop your copy of this
patch? It's already upstream as of 5.8-rc1 and now just needs to be
reverted upstream.

Rich

Re: [PATCH] sh: add missing EXPORT_SYMBOL() for __delay

[Andrew Morton]

On Fri, 24 Jul 2020 15:44:23 -0400 Rich Felker <dalias@libc.org> wrote:

> Andrew, could you drop your copy of this
> patch? 

I have done so, thanks.

Re: [PATCH] sh: add missing EXPORT_SYMBOL() for __delay

[Geert Uytterhoeven]

On Thu, Jul 23, 2020 at 1:53 AM Guenter Roeck <linux@roeck-us.net> wrote:
> On 7/22/20 3:52 PM, Rich Felker wrote:
> > On Tue, Jul 21, 2020 at 07:38:40PM -0700, Guenter Roeck wrote:
> >> On Thu, Dec 12, 2019 at 11:38:43AM +0900, Kuninori Morimoto wrote:
> >>> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> >>>
> >>> __delay() is used from kernel module.
> >>> We need EXPORT_SYMBOL(), otherwise we will get compile error.
> >>>
> >>> ERROR: "__delay" [drivers/net/phy/mdio-cavium.ko] undefined!
> >>>
> >>> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> >>
> >> I must admit that this patch completely baffles me. __delay was
> >> already exported, only elsewhere in the file. With this patch
> >> in place, it is exported twice, and all sh builds in -next fail
> >> with
> >>
> >> In file included from include/linux/linkage.h:7,
> >>                  from arch/sh/include/asm/bug.h:5,
> >>                  from include/linux/bug.h:5,
> >>                  from include/linux/thread_info.h:12,
> >>                  from include/asm-generic/current.h:5,
> >>                  from ./arch/sh/include/generated/asm/current.h:1,
> >>                  from include/linux/sched.h:12,
> >>                  from arch/sh/lib/delay.c:8:
> >> include/linux/export.h:67:36: error: redefinition of '__ksymtab___delay'
> >>
> >> Guenter
> >>
> >>> ---
> >>>  arch/sh/lib/delay.c | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>>
> >>> diff --git a/arch/sh/lib/delay.c b/arch/sh/lib/delay.c
> >>> index dad8e6a..540e670 100644
> >>> --- a/arch/sh/lib/delay.c
> >>> +++ b/arch/sh/lib/delay.c
> >>> @@ -29,6 +29,7 @@ void __delay(unsigned long loops)
> >>>             : "0" (loops)
> >>>             : "t");
> >>>  }
> >>> +EXPORT_SYMBOL(__delay);
> >>>
> >>>  inline void __const_udelay(unsigned long xloops)
> >>>  {
> >
> > I presently have a revert of this commit in queue for next. If it's
> > sufficiently breaking (and especially if there are other regressions
> > that need to be fixed, see the pmd_free thing) I could try to get it
> > in for 5.8 still but that's getting a bit late.
> >
>
> The patch in mainline is ok. It appears that it has been applied
> again in -next.
>
> "git log --oneline v5.7.. arch/sh/lib/delay.c" on top of next-20200721
> reports:
>
> ee0e4f15dfd4 (origin/akpm) sh: add missing EXPORT_SYMBOL() for __delay
> d1f56f318d23 sh: add missing EXPORT_SYMBOL() for __delay
>
> Maybe it just needs to be dropped from the akpm tree in -next ?

IMHO all of them should be dropped/reverted.
__delay is an internal implementation detail, not to be used by drivers.

See also include/asm-generic/delay.h:

    /* Undefined functions to get compile-time errors */
    ...
    extern void __delay(unsigned long loops);

Cfr. '[PATCH] Revert "sh: add missing EXPORT_SYMBOL() for __delay"'
https://lore.kernel.org/lkml/20200608080636.27862-1-geert+renesas@glider.be/

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