| Message ID | 20120930232253.GD30637@obsidianresearch.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi, On Sun, Sep 30, 2012 at 4:22 PM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > Move the CACHE_FEROCEON_L2 test to kirkwood_l2_init, since linking > fails on the reference to feroceon_l2_init. > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > --- > arch/arm/mach-kirkwood/board-dt.c | 2 -- > arch/arm/mach-kirkwood/common.c | 4 ++-- > 2 files changed, 2 insertions(+), 4 deletions(-) > > diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c > index e4eb450..d00223e 100644 > --- a/arch/arm/mach-kirkwood/board-dt.c > +++ b/arch/arm/mach-kirkwood/board-dt.c > @@ -50,9 +50,7 @@ static void __init kirkwood_dt_init(void) > > kirkwood_setup_cpu_mbus(); > > -#ifdef CONFIG_CACHE_FEROCEON_L2 > kirkwood_l2_init(); > -#endif > > /* Setup root of clk tree */ > kirkwood_clk_init(); > diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c > index 1201191..4177304 100644 > --- a/arch/arm/mach-kirkwood/common.c > +++ b/arch/arm/mach-kirkwood/common.c > @@ -645,6 +645,7 @@ char * __init kirkwood_id(void) > > void __init kirkwood_l2_init(void) > { > +#ifdef CONFIG_CACHE_FEROCEON_L2 > #ifdef CONFIG_CACHE_FEROCEON_L2_WRITETHROUGH Aren't these added ifdefs completely redundant? L2_WRITETHROUGH is dependent on the outer options anyway. -Olof
On Sun, Sep 30, 2012 at 07:44:10PM -0700, Olof Johansson wrote: > > void __init kirkwood_l2_init(void) > > { > > +#ifdef CONFIG_CACHE_FEROCEON_L2 > > #ifdef CONFIG_CACHE_FEROCEON_L2_WRITETHROUGH > > Aren't these added ifdefs completely redundant? L2_WRITETHROUGH is > dependent on the outer options anyway. No, the complete new function is: void __init kirkwood_l2_init(void) { #ifdef CONFIG_CACHE_FEROCEON_L2 #ifdef CONFIG_CACHE_FEROCEON_L2_WRITETHROUGH writel(readl(L2_CONFIG_REG) | L2_WRITETHROUGH, L2_CONFIG_REG); feroceon_l2_init(1); #else writel(readl(L2_CONFIG_REG) & ~L2_WRITETHROUGH, L2_CONFIG_REG); feroceon_l2_init(0); #endif #endif } So you hit the #else clause and link fails on feroceon_l2_init because of: obj-$(CONFIG_CACHE_FEROCEON_L2) += cache-feroceon-l2.o Jason
On Sun, Sep 30, 2012 at 07:44:10PM -0700, Olof Johansson wrote: > Hi, > > On Sun, Sep 30, 2012 at 4:22 PM, Jason Gunthorpe > <jgunthorpe@obsidianresearch.com> wrote: > > Move the CACHE_FEROCEON_L2 test to kirkwood_l2_init, since linking > > fails on the reference to feroceon_l2_init. > > > > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > > --- > > arch/arm/mach-kirkwood/board-dt.c | 2 -- > > arch/arm/mach-kirkwood/common.c | 4 ++-- > > 2 files changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c > > index e4eb450..d00223e 100644 > > --- a/arch/arm/mach-kirkwood/board-dt.c > > +++ b/arch/arm/mach-kirkwood/board-dt.c > > @@ -50,9 +50,7 @@ static void __init kirkwood_dt_init(void) > > > > kirkwood_setup_cpu_mbus(); > > > > -#ifdef CONFIG_CACHE_FEROCEON_L2 > > kirkwood_l2_init(); > > -#endif > > > > /* Setup root of clk tree */ > > kirkwood_clk_init(); > > diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c > > index 1201191..4177304 100644 > > --- a/arch/arm/mach-kirkwood/common.c > > +++ b/arch/arm/mach-kirkwood/common.c > > @@ -645,6 +645,7 @@ char * __init kirkwood_id(void) > > > > void __init kirkwood_l2_init(void) > > { > > +#ifdef CONFIG_CACHE_FEROCEON_L2 > > #ifdef CONFIG_CACHE_FEROCEON_L2_WRITETHROUGH > > Aren't these added ifdefs completely redundant? L2_WRITETHROUGH is > dependent on the outer options anyway. I thought so as well, so I went and looked at the code. There is a #else between the original #ifdef/#endif pair. So, feroceon_l2_init() gets called in both cases. hth, Jason.
On Sun, Sep 30, 2012 at 9:43 PM, Jason Gunthorpe <jgunthorpe@obsidianresearch.com> wrote: > On Sun, Sep 30, 2012 at 07:44:10PM -0700, Olof Johansson wrote: > >> > void __init kirkwood_l2_init(void) >> > { >> > +#ifdef CONFIG_CACHE_FEROCEON_L2 >> > #ifdef CONFIG_CACHE_FEROCEON_L2_WRITETHROUGH >> >> Aren't these added ifdefs completely redundant? L2_WRITETHROUGH is >> dependent on the outer options anyway. > > No, the complete new function is: > > void __init kirkwood_l2_init(void) > { > #ifdef CONFIG_CACHE_FEROCEON_L2 > #ifdef CONFIG_CACHE_FEROCEON_L2_WRITETHROUGH > writel(readl(L2_CONFIG_REG) | L2_WRITETHROUGH, L2_CONFIG_REG); > feroceon_l2_init(1); > #else > writel(readl(L2_CONFIG_REG) & ~L2_WRITETHROUGH, L2_CONFIG_REG); > feroceon_l2_init(0); > #endif > #endif > } Ah. diff took out the #else in the middle and I didn't look at the file. Obviously correct as-is. In some other places we choose to put the ifdef outside the function and provide a static inline stub in the header file instead. Either way works fine though. -Olof
On Mon, Oct 01, 2012 at 07:31:58AM -0700, Olof Johansson wrote: > Ah. diff took out the #else in the middle and I didn't look at the > file. Obviously correct as-is. > > In some other places we choose to put the ifdef outside the function > and provide a static inline stub in the header file instead. Either > way works fine though. I don't actually expect anyone will run CONFIG_ARCH_KIRKWOOD_DT without CACHE_FEROCEON_L2 - I had thought about just forcing kconfig to include CACHE_FEROCEON_L2, but figured there was some reason they were separate.. Is there anything further I should do to get this patch upstream? Thanks, Jason
On Tue, Oct 02, 2012 at 11:50:02AM -0600, Jason Gunthorpe wrote: > On Mon, Oct 01, 2012 at 07:31:58AM -0700, Olof Johansson wrote: > > > Ah. diff took out the #else in the middle and I didn't look at the > > file. Obviously correct as-is. > > > > In some other places we choose to put the ifdef outside the function > > and provide a static inline stub in the header file instead. Either > > way works fine though. > > I don't actually expect anyone will run CONFIG_ARCH_KIRKWOOD_DT > without CACHE_FEROCEON_L2 - I had thought about just forcing kconfig > to include CACHE_FEROCEON_L2, but figured there was some reason they > were separate.. > > Is there anything further I should do to get this patch upstream? nope, it's in the queue. thx, Jason.
diff --git a/arch/arm/mach-kirkwood/board-dt.c b/arch/arm/mach-kirkwood/board-dt.c index e4eb450..d00223e 100644 --- a/arch/arm/mach-kirkwood/board-dt.c +++ b/arch/arm/mach-kirkwood/board-dt.c @@ -50,9 +50,7 @@ static void __init kirkwood_dt_init(void) kirkwood_setup_cpu_mbus(); -#ifdef CONFIG_CACHE_FEROCEON_L2 kirkwood_l2_init(); -#endif /* Setup root of clk tree */ kirkwood_clk_init(); diff --git a/arch/arm/mach-kirkwood/common.c b/arch/arm/mach-kirkwood/common.c index 1201191..4177304 100644 --- a/arch/arm/mach-kirkwood/common.c +++ b/arch/arm/mach-kirkwood/common.c @@ -645,6 +645,7 @@ char * __init kirkwood_id(void) void __init kirkwood_l2_init(void) { +#ifdef CONFIG_CACHE_FEROCEON_L2 #ifdef CONFIG_CACHE_FEROCEON_L2_WRITETHROUGH writel(readl(L2_CONFIG_REG) | L2_WRITETHROUGH, L2_CONFIG_REG); feroceon_l2_init(1); @@ -652,6 +653,7 @@ void __init kirkwood_l2_init(void) writel(readl(L2_CONFIG_REG) & ~L2_WRITETHROUGH, L2_CONFIG_REG); feroceon_l2_init(0); #endif +#endif } void __init kirkwood_init(void) @@ -669,9 +671,7 @@ void __init kirkwood_init(void) kirkwood_setup_cpu_mbus(); -#ifdef CONFIG_CACHE_FEROCEON_L2 kirkwood_l2_init(); -#endif /* Setup root of clk tree */ kirkwood_clk_init();
Move the CACHE_FEROCEON_L2 test to kirkwood_l2_init, since linking fails on the reference to feroceon_l2_init. Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> --- arch/arm/mach-kirkwood/board-dt.c | 2 -- arch/arm/mach-kirkwood/common.c | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-)