diff mbox series

ARM: omap2: drop broken broadcast timer hack

Message ID 20200528091923.2951100-1-arnd@arndb.de (mailing list archive)
State New, archived
Headers show
Series ARM: omap2: drop broken broadcast timer hack | expand

Commit Message

Arnd Bergmann May 28, 2020, 9:19 a.m. UTC
The OMAP4 timer code had a special hack for using the broadcast timer
without SMP. Since the dmtimer is now gone, this also needs to be dropped
to avoid a link failure for non-SMP AM43xx configurations:

kernel/time/tick-broadcast.o: in function `tick_device_uses_broadcast':
tick-broadcast.c:(.text+0x130): undefined reference to `tick_broadcast'

Fixes: 2ee04b88547a ("ARM: OMAP2+: Drop old timer code for dmtimer and 32k counter")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/arm/mach-omap2/Kconfig | 1 -
 arch/arm/mach-omap2/timer.c | 6 ------
 2 files changed, 7 deletions(-)

Comments

Tony Lindgren May 28, 2020, 1:46 p.m. UTC | #1
* Arnd Bergmann <arnd@arndb.de> [200528 09:20]:
> The OMAP4 timer code had a special hack for using the broadcast timer
> without SMP. Since the dmtimer is now gone, this also needs to be dropped
> to avoid a link failure for non-SMP AM43xx configurations:
> 
> kernel/time/tick-broadcast.o: in function `tick_device_uses_broadcast':
> tick-broadcast.c:(.text+0x130): undefined reference to `tick_broadcast'

Hmm this sounds like a regression though. Isn't this needed for using
the ARM local timers on non-SMP SoC, so a separate timer from dmtimer?

I've probably removed something accidentally to cause this.

Regards,

Tony


> Fixes: 2ee04b88547a ("ARM: OMAP2+: Drop old timer code for dmtimer and 32k counter")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  arch/arm/mach-omap2/Kconfig | 1 -
>  arch/arm/mach-omap2/timer.c | 6 ------
>  2 files changed, 7 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
> index ca74473f01df..ec4450a5c296 100644
> --- a/arch/arm/mach-omap2/Kconfig
> +++ b/arch/arm/mach-omap2/Kconfig
> @@ -67,7 +67,6 @@ config SOC_AM43XX
>  	select ARM_GIC
>  	select MACH_OMAP_GENERIC
>  	select HAVE_ARM_SCU
> -	select GENERIC_CLOCKEVENTS_BROADCAST
>  	select HAVE_ARM_TWD
>  	select ARM_ERRATA_754322
>  	select ARM_ERRATA_775420
> diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
> index 2d4ea386fc38..786336ee27ef 100644
> --- a/arch/arm/mach-omap2/timer.c
> +++ b/arch/arm/mach-omap2/timer.c
> @@ -46,12 +46,6 @@ void set_cntfreq(void)
>  	omap_smc1(OMAP5_DRA7_MON_SET_CNTFRQ_INDEX, arch_timer_freq);
>  }
>  
> -#if !defined(CONFIG_SMP) && defined(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST)
> -void tick_broadcast(const struct cpumask *mask)
> -{
> -}
> -#endif
> -
>  #if defined(CONFIG_SOC_OMAP5) || defined(CONFIG_SOC_DRA7XX)
>  
>  /*
> -- 
> 2.26.2
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Tony Lindgren May 28, 2020, 1:50 p.m. UTC | #2
* Tony Lindgren <tony@atomide.com> [200528 13:47]:
> * Arnd Bergmann <arnd@arndb.de> [200528 09:20]:
> > The OMAP4 timer code had a special hack for using the broadcast timer
> > without SMP. Since the dmtimer is now gone, this also needs to be dropped
> > to avoid a link failure for non-SMP AM43xx configurations:
> > 
> > kernel/time/tick-broadcast.o: in function `tick_device_uses_broadcast':
> > tick-broadcast.c:(.text+0x130): undefined reference to `tick_broadcast'
> 
> Hmm this sounds like a regression though. Isn't this needed for using
> the ARM local timers on non-SMP SoC, so a separate timer from dmtimer?
> 
> I've probably removed something accidentally to cause this.

Sounds like arch/arm/mach-omap2/Makefile change needs to be removed
to always still build in timer.o. And probably timer.c needs back
the ifdef for CONFIG_SOC_HAS_REALTIME_COUNTER.

I'll take a look today.

Regards,

Tony
Tony Lindgren May 28, 2020, 3:57 p.m. UTC | #3
* Tony Lindgren <tony@atomide.com> [200528 13:51]:
> * Tony Lindgren <tony@atomide.com> [200528 13:47]:
> > * Arnd Bergmann <arnd@arndb.de> [200528 09:20]:
> > > The OMAP4 timer code had a special hack for using the broadcast timer
> > > without SMP. Since the dmtimer is now gone, this also needs to be dropped
> > > to avoid a link failure for non-SMP AM43xx configurations:
> > > 
> > > kernel/time/tick-broadcast.o: in function `tick_device_uses_broadcast':
> > > tick-broadcast.c:(.text+0x130): undefined reference to `tick_broadcast'
> > 
> > Hmm this sounds like a regression though. Isn't this needed for using
> > the ARM local timers on non-SMP SoC, so a separate timer from dmtimer?
> > 
> > I've probably removed something accidentally to cause this.
> 
> Sounds like arch/arm/mach-omap2/Makefile change needs to be removed
> to always still build in timer.o. And probably timer.c needs back
> the ifdef for CONFIG_SOC_HAS_REALTIME_COUNTER.
> 
> I'll take a look today.

I've sent a patch along those lines as:

[PATCH] ARM: OMAP2+: Fix regression for using local timer on non-SMP SoCs

A link for the patch at [0] below.

Regards,

Tony

[0] https://lore.kernel.org/linux-omap/20200528155453.8585-1-tony@atomide.com/T/#u
Santosh Shilimkar May 28, 2020, 4:18 p.m. UTC | #4
On 5/28/20 8:57 AM, Tony Lindgren wrote:
> * Tony Lindgren <tony@atomide.com> [200528 13:51]:
>> * Tony Lindgren <tony@atomide.com> [200528 13:47]:
>>> * Arnd Bergmann <arnd@arndb.de> [200528 09:20]:
>>>> The OMAP4 timer code had a special hack for using the broadcast timer
>>>> without SMP. Since the dmtimer is now gone, this also needs to be dropped
>>>> to avoid a link failure for non-SMP AM43xx configurations:
>>>>
>>>> kernel/time/tick-broadcast.o: in function `tick_device_uses_broadcast':
>>>> tick-broadcast.c:(.text+0x130): undefined reference to `tick_broadcast'
>>>
>>> Hmm this sounds like a regression though. Isn't this needed for using
>>> the ARM local timers on non-SMP SoC, so a separate timer from dmtimer?
>>>
>>> I've probably removed something accidentally to cause this.
>>
>> Sounds like arch/arm/mach-omap2/Makefile change needs to be removed
>> to always still build in timer.o. And probably timer.c needs back
>> the ifdef for CONFIG_SOC_HAS_REALTIME_COUNTER.
>>
>> I'll take a look today.
> 
> I've sent a patch along those lines as:
> 
> [PATCH] ARM: OMAP2+: Fix regression for using local timer on non-SMP SoCs
> 
> A link for the patch at [0] below.
> 
CPU local timers not being in always ON power domain use to be the
reason on early version of the SOCs but later SOC moved the CPU local
timer also in always on domain. Probably AM43xx does loose local timer
on CPU PD in low power so yes broadcast would be needed with dmtimer
help.

> 
> [0] https://lore.kernel.org/linux-omap/20200528155453.8585-1-tony@atomide.com/T/#u
> 
This should restore it.

Regards,
Santosh
Arnd Bergmann May 28, 2020, 8:33 p.m. UTC | #5
On Thu, May 28, 2020 at 6:18 PM <santosh.shilimkar@oracle.com> wrote:
> On 5/28/20 8:57 AM, Tony Lindgren wrote:
> > * Tony Lindgren <tony@atomide.com> [200528 13:51]:
> >> * Tony Lindgren <tony@atomide.com> [200528 13:47]:
> >>> * Arnd Bergmann <arnd@arndb.de> [200528 09:20]:
> >>>> The OMAP4 timer code had a special hack for using the broadcast timer
> >>>> without SMP. Since the dmtimer is now gone, this also needs to be dropped
> >>>> to avoid a link failure for non-SMP AM43xx configurations:
> >>>>
> >>>> kernel/time/tick-broadcast.o: in function `tick_device_uses_broadcast':
> >>>> tick-broadcast.c:(.text+0x130): undefined reference to `tick_broadcast'
> >>>
> >>> Hmm this sounds like a regression though. Isn't this needed for using
> >>> the ARM local timers on non-SMP SoC, so a separate timer from dmtimer?
> >>>
> >>> I've probably removed something accidentally to cause this.
> >>
> >> Sounds like arch/arm/mach-omap2/Makefile change needs to be removed
> >> to always still build in timer.o. And probably timer.c needs back
> >> the ifdef for CONFIG_SOC_HAS_REALTIME_COUNTER.
> >>
> >> I'll take a look today.
> >
> > I've sent a patch along those lines as:
> >
> > [PATCH] ARM: OMAP2+: Fix regression for using local timer on non-SMP SoCs
> >
> > A link for the patch at [0] below.
> >
> CPU local timers not being in always ON power domain use to be the
> reason on early version of the SOCs but later SOC moved the CPU local
> timer also in always on domain. Probably AM43xx does loose local timer
> on CPU PD in low power so yes broadcast would be needed with dmtimer
> help.
>
> >
> > [0] https://lore.kernel.org/linux-omap/20200528155453.8585-1-tony@atomide.com/T/#u
> >
> This should restore it.

Should I apply the fix directly to the arm/soc branch that has the
other changes then?

     Arnd
Tony Lindgren May 28, 2020, 8:38 p.m. UTC | #6
* Arnd Bergmann <arnd@arndb.de> [200528 20:35]:
> On Thu, May 28, 2020 at 6:18 PM <santosh.shilimkar@oracle.com> wrote:
> > On 5/28/20 8:57 AM, Tony Lindgren wrote:
> > > * Tony Lindgren <tony@atomide.com> [200528 13:51]:
> > >> * Tony Lindgren <tony@atomide.com> [200528 13:47]:
> > >>> * Arnd Bergmann <arnd@arndb.de> [200528 09:20]:
> > >>>> The OMAP4 timer code had a special hack for using the broadcast timer
> > >>>> without SMP. Since the dmtimer is now gone, this also needs to be dropped
> > >>>> to avoid a link failure for non-SMP AM43xx configurations:
> > >>>>
> > >>>> kernel/time/tick-broadcast.o: in function `tick_device_uses_broadcast':
> > >>>> tick-broadcast.c:(.text+0x130): undefined reference to `tick_broadcast'
> > >>>
> > >>> Hmm this sounds like a regression though. Isn't this needed for using
> > >>> the ARM local timers on non-SMP SoC, so a separate timer from dmtimer?
> > >>>
> > >>> I've probably removed something accidentally to cause this.
> > >>
> > >> Sounds like arch/arm/mach-omap2/Makefile change needs to be removed
> > >> to always still build in timer.o. And probably timer.c needs back
> > >> the ifdef for CONFIG_SOC_HAS_REALTIME_COUNTER.
> > >>
> > >> I'll take a look today.
> > >
> > > I've sent a patch along those lines as:
> > >
> > > [PATCH] ARM: OMAP2+: Fix regression for using local timer on non-SMP SoCs
> > >
> > > A link for the patch at [0] below.
> > >
> > CPU local timers not being in always ON power domain use to be the
> > reason on early version of the SOCs but later SOC moved the CPU local
> > timer also in always on domain. Probably AM43xx does loose local timer
> > on CPU PD in low power so yes broadcast would be needed with dmtimer
> > help.
> >
> > >
> > > [0] https://lore.kernel.org/linux-omap/20200528155453.8585-1-tony@atomide.com/T/#u
> > >
> > This should restore it.
> 
> Should I apply the fix directly to the arm/soc branch that has the
> other changes then?

Sure please do, that saves a single-fix pull request.

Regards,

Tony
diff mbox series

Patch

diff --git a/arch/arm/mach-omap2/Kconfig b/arch/arm/mach-omap2/Kconfig
index ca74473f01df..ec4450a5c296 100644
--- a/arch/arm/mach-omap2/Kconfig
+++ b/arch/arm/mach-omap2/Kconfig
@@ -67,7 +67,6 @@  config SOC_AM43XX
 	select ARM_GIC
 	select MACH_OMAP_GENERIC
 	select HAVE_ARM_SCU
-	select GENERIC_CLOCKEVENTS_BROADCAST
 	select HAVE_ARM_TWD
 	select ARM_ERRATA_754322
 	select ARM_ERRATA_775420
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index 2d4ea386fc38..786336ee27ef 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -46,12 +46,6 @@  void set_cntfreq(void)
 	omap_smc1(OMAP5_DRA7_MON_SET_CNTFRQ_INDEX, arch_timer_freq);
 }
 
-#if !defined(CONFIG_SMP) && defined(CONFIG_GENERIC_CLOCKEVENTS_BROADCAST)
-void tick_broadcast(const struct cpumask *mask)
-{
-}
-#endif
-
 #if defined(CONFIG_SOC_OMAP5) || defined(CONFIG_SOC_DRA7XX)
 
 /*