* [PATCH] ARM: omap2: fix omap5_realtime_timer_init definition
@ 2020-05-29 20:16 Arnd Bergmann
2020-05-29 20:44 ` Tony Lindgren
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2020-05-29 20:16 UTC (permalink / raw)
To: soc, Tony Lindgren, Arnd Bergmann
Cc: Olof Johansson, Stefan Agner, linux-arm-kernel, linux-omap, linux-kernel
There is one more regression introduced by the last build fix:
arch/arm/mach-omap2/timer.c:170:6: error: attribute declaration must precede definition [-Werror,-Wignored-attributes]
void __init omap5_realtime_timer_init(void)
^
arch/arm/mach-omap2/common.h:118:20: note: previous definition is here
static inline void omap5_realtime_timer_init(void)
^
arch/arm/mach-omap2/timer.c:170:13: error: redefinition of 'omap5_realtime_timer_init'
void __init omap5_realtime_timer_init(void)
^
arch/arm/mach-omap2/common.h:118:20: note: previous definition is here
static inline void omap5_realtime_timer_init(void)
Address this by removing the now obsolete #ifdefs in that file and
just building the entire file based on the flag that controls the
omap5_realtime_timer_init function declaration.
Fixes: d86ad463d670 ("ARM: OMAP2+: Fix regression for using local timer on non-SMP SoCs")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
If this looks ok, I'd apply it directly on top again for the merge window.
---
arch/arm/mach-omap2/Makefile | 6 +++---
arch/arm/mach-omap2/timer.c | 10 ----------
2 files changed, 3 insertions(+), 13 deletions(-)
diff --git a/arch/arm/mach-omap2/Makefile b/arch/arm/mach-omap2/Makefile
index 40898b1fd7da..732e614c56b2 100644
--- a/arch/arm/mach-omap2/Makefile
+++ b/arch/arm/mach-omap2/Makefile
@@ -46,9 +46,9 @@ obj-$(CONFIG_SOC_OMAP5) += $(omap-4-5-common) $(smp-y) sleep44xx.o
obj-$(CONFIG_SOC_AM43XX) += $(omap-4-5-common)
obj-$(CONFIG_SOC_DRA7XX) += $(omap-4-5-common) $(smp-y) sleep44xx.o
-omap5-dra7-common = timer.o
-obj-$(CONFIG_SOC_OMAP5) += $(omap5-dra7-common)
-obj-$(CONFIG_SOC_DRA7XX) += $(omap5-dra7-common)
+omap5-dra7-common-$(CONFIG_SOC_HAS_REALTIME_COUNTER) = timer.o
+obj-$(CONFIG_SOC_OMAP5) += $(omap5-dra7-common-y)
+obj-$(CONFIG_SOC_DRA7XX) += $(omap5-dra7-common-y)
# Functions loaded to SRAM
obj-$(CONFIG_SOC_OMAP2420) += sram242x.o
diff --git a/arch/arm/mach-omap2/timer.c b/arch/arm/mach-omap2/timer.c
index c1737e737a94..620ba69c8f11 100644
--- a/arch/arm/mach-omap2/timer.c
+++ b/arch/arm/mach-omap2/timer.c
@@ -39,8 +39,6 @@
#define INCREMENTER_DENUMERATOR_RELOAD_OFFSET 0x14
#define NUMERATOR_DENUMERATOR_MASK 0xfffff000
-#ifdef CONFIG_SOC_HAS_REALTIME_COUNTER
-
static unsigned long arch_timer_freq;
void set_cntfreq(void)
@@ -159,14 +157,6 @@ static void __init realtime_counter_init(void)
iounmap(base);
}
-#else
-
-static inline void realtime_counter_init(void)
-{
-}
-
-#endif /* CONFIG_SOC_HAS_REALTIME_COUNTER */
-
void __init omap5_realtime_timer_init(void)
{
omap_clk_init();
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] ARM: omap2: fix omap5_realtime_timer_init definition
2020-05-29 20:16 [PATCH] ARM: omap2: fix omap5_realtime_timer_init definition Arnd Bergmann
@ 2020-05-29 20:44 ` Tony Lindgren
2020-05-29 21:07 ` Arnd Bergmann
0 siblings, 1 reply; 6+ messages in thread
From: Tony Lindgren @ 2020-05-29 20:44 UTC (permalink / raw)
To: Arnd Bergmann
Cc: soc, Olof Johansson, Stefan Agner, linux-arm-kernel, linux-omap,
linux-kernel
* Arnd Bergmann <arnd@arndb.de> [200529 20:18]:
> There is one more regression introduced by the last build fix:
Argh.. I did run make randconfig for like 10 builds
after the last fix.
> arch/arm/mach-omap2/timer.c:170:6: error: attribute declaration must precede definition [-Werror,-Wignored-attributes]
> void __init omap5_realtime_timer_init(void)
> ^
> arch/arm/mach-omap2/common.h:118:20: note: previous definition is here
> static inline void omap5_realtime_timer_init(void)
> ^
> arch/arm/mach-omap2/timer.c:170:13: error: redefinition of 'omap5_realtime_timer_init'
> void __init omap5_realtime_timer_init(void)
> ^
> arch/arm/mach-omap2/common.h:118:20: note: previous definition is here
> static inline void omap5_realtime_timer_init(void)
>
> Address this by removing the now obsolete #ifdefs in that file and
> just building the entire file based on the flag that controls the
> omap5_realtime_timer_init function declaration.
I think this will introduce other randconfig build failures
as SOC_HAS_REALTIME_COUNTER is bool in Kconfig.
We still need to call omap5_realtime_timer_init() even if
SOC_HAS_REALTIME_COUNTER is not set.
Regards,
Tony
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] ARM: omap2: fix omap5_realtime_timer_init definition
2020-05-29 20:44 ` Tony Lindgren
@ 2020-05-29 21:07 ` Arnd Bergmann
2020-05-29 21:14 ` Tony Lindgren
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2020-05-29 21:07 UTC (permalink / raw)
To: Tony Lindgren
Cc: SoC Team, Olof Johansson, Stefan Agner, Linux ARM, linux-omap,
linux-kernel
On Fri, May 29, 2020 at 10:44 PM Tony Lindgren <tony@atomide.com> wrote:
>
> * Arnd Bergmann <arnd@arndb.de> [200529 20:18]:
> > There is one more regression introduced by the last build fix:
>
> Argh.. I did run make randconfig for like 10 builds
> after the last fix.
>
> > Address this by removing the now obsolete #ifdefs in that file and
> > just building the entire file based on the flag that controls the
> > omap5_realtime_timer_init function declaration.
>
> I think this will introduce other randconfig build failures
> as SOC_HAS_REALTIME_COUNTER is bool in Kconfig.
I did a few hundred randconfig builds with the patch and have
not yet seen any further problems.
> We still need to call omap5_realtime_timer_init() even if
> SOC_HAS_REALTIME_COUNTER is not set.
This is what's in the header file:
#ifdef CONFIG_SOC_HAS_REALTIME_COUNTER
extern void omap5_realtime_timer_init(void);
#else
static inline void omap5_realtime_timer_init(void)
{
}
#endif
In fact, the inline stub is what that caused the regression,
so I think it's ok with my patch.
Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ARM: omap2: fix omap5_realtime_timer_init definition
2020-05-29 21:07 ` Arnd Bergmann
@ 2020-05-29 21:14 ` Tony Lindgren
2020-05-29 21:39 ` Arnd Bergmann
0 siblings, 1 reply; 6+ messages in thread
From: Tony Lindgren @ 2020-05-29 21:14 UTC (permalink / raw)
To: Arnd Bergmann
Cc: SoC Team, Olof Johansson, Stefan Agner, Linux ARM, linux-omap,
linux-kernel
* Arnd Bergmann <arnd@arndb.de> [200529 21:09]:
> On Fri, May 29, 2020 at 10:44 PM Tony Lindgren <tony@atomide.com> wrote:
> >
> > * Arnd Bergmann <arnd@arndb.de> [200529 20:18]:
> > > There is one more regression introduced by the last build fix:
> >
> > Argh.. I did run make randconfig for like 10 builds
> > after the last fix.
> >
> > > Address this by removing the now obsolete #ifdefs in that file and
> > > just building the entire file based on the flag that controls the
> > > omap5_realtime_timer_init function declaration.
> >
> > I think this will introduce other randconfig build failures
> > as SOC_HAS_REALTIME_COUNTER is bool in Kconfig.
>
> I did a few hundred randconfig builds with the patch and have
> not yet seen any further problems.
Ah right, it works for randconfig builds now but won't boot :)
> > We still need to call omap5_realtime_timer_init() even if
> > SOC_HAS_REALTIME_COUNTER is not set.
>
> This is what's in the header file:
>
> #ifdef CONFIG_SOC_HAS_REALTIME_COUNTER
> extern void omap5_realtime_timer_init(void);
> #else
> static inline void omap5_realtime_timer_init(void)
> {
> }
> #endif
>
> In fact, the inline stub is what that caused the regression,
> so I think it's ok with my patch.
To me it seems not having SOC_HAS_REALTIME_COUNTER will
cause omap5_realtime_timer_init() not get called?
That initializes clocks and calls timer_probe(). So this
will result in non-booting system AFAIK, the header
file stub should no rely CONFIG_SOC_HAS_REALTIME_COUNTER
also, but rather ! CONFIG_SOC_OMAP5 || CONFIG_SOC_DRA7XX.
Also the Makefile change at least seems wrong, that
can't rely on CONFIG_SOC_HAS_REALTIME_COUNTER.
Regards,
Tony
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ARM: omap2: fix omap5_realtime_timer_init definition
2020-05-29 21:14 ` Tony Lindgren
@ 2020-05-29 21:39 ` Arnd Bergmann
2020-05-29 21:46 ` Tony Lindgren
0 siblings, 1 reply; 6+ messages in thread
From: Arnd Bergmann @ 2020-05-29 21:39 UTC (permalink / raw)
To: Tony Lindgren
Cc: SoC Team, Olof Johansson, Stefan Agner, Linux ARM, linux-omap,
linux-kernel
On Fri, May 29, 2020 at 11:14 PM Tony Lindgren <tony@atomide.com> wrote:
> * Arnd Bergmann <arnd@arndb.de> [200529 21:09]:
> >
> > #ifdef CONFIG_SOC_HAS_REALTIME_COUNTER
> > extern void omap5_realtime_timer_init(void);
> > #else
> > static inline void omap5_realtime_timer_init(void)
> > {
> > }
> > #endif
> >
> > In fact, the inline stub is what that caused the regression,
> > so I think it's ok with my patch.
>
> To me it seems not having SOC_HAS_REALTIME_COUNTER will
> cause omap5_realtime_timer_init() not get called?
Correct, this looked to me like it was the intention of that
symbol. Unfortunately there is no help text but it is user
selectable:
config SOC_HAS_REALTIME_COUNTER
bool "Real time free running counter"
depends on SOC_OMAP5 || SOC_DRA7XX
default y
> That initializes clocks and calls timer_probe(). So this
> will result in non-booting system AFAIK, the header
> file stub should no rely CONFIG_SOC_HAS_REALTIME_COUNTER
> also, but rather ! CONFIG_SOC_OMAP5 || CONFIG_SOC_DRA7XX.
>
> Also the Makefile change at least seems wrong, that
> can't rely on CONFIG_SOC_HAS_REALTIME_COUNTER.
How about just removing the prompt on
CONFIG_SOC_HAS_REALTIME_COUNTER but keeping the
rest of my patch? That way it's just always enabled when
there is a chip that needs it enabled in the kernel config.
The only other usage of the symbol is
#ifdef CONFIG_SOC_HAS_REALTIME_COUNTER
void set_cntfreq(void);
#else
static inline void set_cntfreq(void)
{
}
#endif
Alternatively, we could just remove the Kconfig symbol
altogether and rely on (SOC_OMAP5 || SOC_DRA7XX)
everywhere, but that seems a little more fragile in case
there is going to be another chip that needs it.
Arnd
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] ARM: omap2: fix omap5_realtime_timer_init definition
2020-05-29 21:39 ` Arnd Bergmann
@ 2020-05-29 21:46 ` Tony Lindgren
0 siblings, 0 replies; 6+ messages in thread
From: Tony Lindgren @ 2020-05-29 21:46 UTC (permalink / raw)
To: Arnd Bergmann
Cc: SoC Team, Olof Johansson, Stefan Agner, Linux ARM, linux-omap,
linux-kernel, santosh.shilimkar
* Arnd Bergmann <arnd@arndb.de> [200529 21:41]:
> On Fri, May 29, 2020 at 11:14 PM Tony Lindgren <tony@atomide.com> wrote:
> > * Arnd Bergmann <arnd@arndb.de> [200529 21:09]:
> > >
> > > #ifdef CONFIG_SOC_HAS_REALTIME_COUNTER
> > > extern void omap5_realtime_timer_init(void);
> > > #else
> > > static inline void omap5_realtime_timer_init(void)
> > > {
> > > }
> > > #endif
> > >
> > > In fact, the inline stub is what that caused the regression,
> > > so I think it's ok with my patch.
> >
> > To me it seems not having SOC_HAS_REALTIME_COUNTER will
> > cause omap5_realtime_timer_init() not get called?
>
> Correct, this looked to me like it was the intention of that
> symbol. Unfortunately there is no help text but it is user
> selectable:
>
> config SOC_HAS_REALTIME_COUNTER
> bool "Real time free running counter"
> depends on SOC_OMAP5 || SOC_DRA7XX
> default y
Maybe this is a legacy Kconfig option since Santosh got
the cpuidle coupled to switch things to using the always on
timers for idle modes years ago already.
> > That initializes clocks and calls timer_probe(). So this
> > will result in non-booting system AFAIK, the header
> > file stub should no rely CONFIG_SOC_HAS_REALTIME_COUNTER
> > also, but rather ! CONFIG_SOC_OMAP5 || CONFIG_SOC_DRA7XX.
> >
> > Also the Makefile change at least seems wrong, that
> > can't rely on CONFIG_SOC_HAS_REALTIME_COUNTER.
>
> How about just removing the prompt on
> CONFIG_SOC_HAS_REALTIME_COUNTER but keeping the
> rest of my patch? That way it's just always enabled when
> there is a chip that needs it enabled in the kernel config.
>
> The only other usage of the symbol is
>
> #ifdef CONFIG_SOC_HAS_REALTIME_COUNTER
> void set_cntfreq(void);
> #else
> static inline void set_cntfreq(void)
> {
> }
> #endif
Yeah it's already default y, so I'd say let's just get
rid of the option.
> Alternatively, we could just remove the Kconfig symbol
> altogether and rely on (SOC_OMAP5 || SOC_DRA7XX)
> everywhere, but that seems a little more fragile in case
> there is going to be another chip that needs it.
Sounds like we can just remove CONFIG_SOC_HAS_REALTIME_COUNTER
and rely on (SOC_OMAP5 || SOC_DRA7XX).
Regards,
Tony
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-29 21:47 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-29 20:16 [PATCH] ARM: omap2: fix omap5_realtime_timer_init definition Arnd Bergmann
2020-05-29 20:44 ` Tony Lindgren
2020-05-29 21:07 ` Arnd Bergmann
2020-05-29 21:14 ` Tony Lindgren
2020-05-29 21:39 ` Arnd Bergmann
2020-05-29 21:46 ` Tony Lindgren
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).