Build failure CONFIG_ARCH_KIRKWOOD_DT relies on CACHE_FEROCEON_L2

PATCH New Jason Gunthorpe 3 participants 7 messages Sep 30, 2012 – Oct 3, 2012

Subsystems (2)

Arm (32-bit) Arm sub-architectures

lore

Share & tools

lore

Jason Gunthorpe

author

2012-09-30 23:22 UTC #0 lore raw

PATCH 1/1 Build failure CONFIG_ARCH_KIRKWOOD_DT relies on CACHE_FEROCEON_L2

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 <[email protected]>

---
 arch/arm/mach-kirkwood/board-dt.c |    2 --
 arch/arm/mach-kirkwood/common.c   |    4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

arch/arm/mach-kirkwood/board-dt.c +0 -2

index e4eb450..d00223e 100644
@@ -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();

arch/arm/mach-kirkwood/common.c +2 -2

index 1201191..4177304 100644
@@ -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();

Signature · 1 line

1.7.4.1

Olof Johansson

2012-10-01 02:44 UTC #1 lore raw

PATCH 1/1 Build failure CONFIG_ARCH_KIRKWOOD_DT relies on CACHE_FEROCEON_L2

Hi,

Quoted 28 lines · Jason Gunthorpe

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 <[email protected]>

---
 arch/arm/mach-kirkwood/board-dt.c |    2 --
 arch/arm/mach-kirkwood/common.c   |    4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)

arch/arm/mach-kirkwood/board-dt.c +0 -2

index e4eb450..d00223e 100644
@@ -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();

arch/arm/mach-kirkwood/common.c +0 -0

index 1201191..4177304 100644

arch/arm/mach-kirkwood/common.c char * __init kirkwood_id(void) · line 649

Quoted 5 lines

arch/arm/mach-kirkwood/common.c +1 -0

@@ -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

Jason Gunthorpe

author

2012-10-01 04:43 UTC #2 lore raw

PATCH 1/1 Build failure CONFIG_ARCH_KIRKWOOD_DT relies on CACHE_FEROCEON_L2

Quoted 7 lines · Olof Johansson

Quoted 4 lines
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

Olof Johansson

2012-10-01 14:31 UTC #3 lore raw

PATCH 1/1 Build failure CONFIG_ARCH_KIRKWOOD_DT relies on CACHE_FEROCEON_L2

Quoted 21 lines · Jason Gunthorpe

Quoted 7 lines · Olof Johansson
Quoted 4 lines
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);

Quoted 3 lines

#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

Jason Gunthorpe

author

2012-10-02 17:50 UTC #4 lore raw

PATCH 1/1 Build failure CONFIG_ARCH_KIRKWOOD_DT relies on CACHE_FEROCEON_L2

Quoted 6 lines · Olof Johansson

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

Jason Cooper

2012-10-03 13:39 UTC #5 lore raw

PATCH 1/1 Build failure CONFIG_ARCH_KIRKWOOD_DT relies on CACHE_FEROCEON_L2

Quoted 9 lines · Jason Gunthorpe

Quoted 6 lines · Olof Johansson
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.

Quoted 6 lines

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.

Jason Cooper

2012-10-01 10:46 UTC #6 lore raw

PATCH 1/1 Build failure CONFIG_ARCH_KIRKWOOD_DT relies on CACHE_FEROCEON_L2

In reply to Olof Johansson

Quoted 33 lines · Olof Johansson

Hi,
Quoted 29 lines · Jason Gunthorpe
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 <[email protected]>
---
 arch/arm/mach-kirkwood/board-dt.c |    2 --
 arch/arm/mach-kirkwood/common.c   |    4 ++--
 2 files changed, 2 insertions(+), 4 deletions(-)
arch/arm/mach-kirkwood/board-dt.c +0 -2
index e4eb450..d00223e 100644
@@ -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();
arch/arm/mach-kirkwood/common.c +0 -0
index 1201191..4177304 100644
@@ -645,6 +645,7 @@ char * __init kirkwood_id(void)
 

Quoted 7 lines

arch/arm/mach-kirkwood/common.c char * __init kirkwood_id(void) · line 649
Quoted 4 lines
arch/arm/mach-kirkwood/common.c +1 -0
 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.