fsl_upm compile failure.

PATCH Applied Dave Jones 3 participants 6 messages Oct 8, 2008 – Oct 13, 2008 PowerPC

Share & tools

Dave Jones

author

2008-10-08 22:28 UTC #0 lore raw

Hi Anton,
I tried reenabling CONFIG_MTD_NAND_FSL_UPM in the Fedora kernel
today (currently on 2.6.27-rc9-git1), and got the error below.

ERROR: "fsl_lbc_lock" [drivers/mtd/nand/fsl_upm.ko] undefined!

It seems that this isn't exported to modules. diff below fixes this. (Not tested yet, the Fedora ppc
builders are a little slow).

Dave

---

CONFIG_MTD_NAND_FSL_UPM can be built modular, but needs to use fsl_lbc_lock, which isn't currently exported.

Signed-off-by Dave Jones <[email protected]>

arch/powerpc/sysdev/fsl_lbc.c +1 -0

index 422c8fa..1c6c522 100644
@@ -16,6 +16,7 @@
 #include <asm/fsl_lbc.h>
 
 spinlock_t fsl_lbc_lock = __SPIN_LOCK_UNLOCKED(fsl_lbc_lock);
+EXPORT_SYMBOL(fsl_lbc_lock);
 
 struct fsl_lbc_regs __iomem *fsl_lbc_regs;
 EXPORT_SYMBOL(fsl_lbc_regs);

Anton Vorontsov

Acked-by

2008-10-08 23:02 UTC #1 lore raw

Hi,

Quoted 5 lines · Dave Jones

Hi Anton,
I tried reenabling CONFIG_MTD_NAND_FSL_UPM in the Fedora kernel
today (currently on 2.6.27-rc9-git1), and got the error below.
ERROR: "fsl_lbc_lock" [drivers/mtd/nand/fsl_upm.ko] undefined!

Weird. It works for me with mpc836x_rdk_defconfig and MTD_NAND_FSL_UPM=m...

Hm..

CC drivers/mtd/nand/fsl_upm.mod.o
LD [M] drivers/mtd/nand/fsl_upm.ko

Can you send me the .config file you use?

Quoted 18 lines

It seems that this isn't exported to modules. diff below fixes this. (Not tested yet, the Fedora ppc
builders are a little slow).
Dave
---
CONFIG_MTD_NAND_FSL_UPM can be built modular, but needs to use fsl_lbc_lock, which isn't currently exported.
Signed-off-by Dave Jones <[email protected]>
arch/powerpc/sysdev/fsl_lbc.c +0 -0
index 422c8fa..1c6c522 100644

arch/powerpc/sysdev/fsl_lbc.c line 22

Quoted 7 lines

arch/powerpc/sysdev/fsl_lbc.c +1 -0

@@ -16,6 +16,7 @@
 #include <asm/fsl_lbc.h>
 
 spinlock_t fsl_lbc_lock = __SPIN_LOCK_UNLOCKED(fsl_lbc_lock);
+EXPORT_SYMBOL(fsl_lbc_lock);
 
 struct fsl_lbc_regs __iomem *fsl_lbc_regs;
 EXPORT_SYMBOL(fsl_lbc_regs);

But the patch looks obviously correct. Much thanks for catching this.

Though better option would be to uninline the fsl_upm_run_pattern().. it is quite big anyway... Something like this:

arch/powerpc/include/asm/fsl_lbc.h +2 ~~-41~~

index 303f548..af2b535 100644
@@ -24,7 +24,6 @@
 #define __ASM_FSL_LBC_H
 
 #include <linux/types.h>
-#include <linux/spinlock.h>
 #include <asm/io.h>
 
 struct fsl_lbc_bank {

@@ -228,7 +227,6 @@ struct fsl_lbc_regs {
 };
 
 extern struct fsl_lbc_regs __iomem *fsl_lbc_regs;
-extern spinlock_t fsl_lbc_lock;
 
 /*
  * FSL UPM routines

@@ -268,44 +266,7 @@ static inline void fsl_upm_end_pattern(struct fsl_upm *upm)
 		cpu_relax();
 }
 
-/**
- * fsl_upm_run_pattern - actually run an UPM pattern
- * @upm:	pointer to the fsl_upm structure obtained via fsl_upm_find
- * @io_base:	remapped pointer to where memory access should happen
- * @mar:	MAR register content during pattern execution
- *
- * This function triggers dummy write to the memory specified by the io_base,
- * thus UPM pattern actually executed. Note that mar usage depends on the
- * pre-programmed AMX bits in the UPM RAM.
- */
-static inline int fsl_upm_run_pattern(struct fsl_upm *upm,
-				      void __iomem *io_base, u32 mar)
-{
-	int ret = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&fsl_lbc_lock, flags);
-
-	out_be32(&fsl_lbc_regs->mar, mar << (32 - upm->width));
-
-	switch (upm->width) {
-	case 8:
-		out_8(io_base, 0x0);
-		break;
-	case 16:
-		out_be16(io_base, 0x0);
-		break;
-	case 32:
-		out_be32(io_base, 0x0);
-		break;
-	default:
-		ret = -EINVAL;
-		break;
-	}
-
-	spin_unlock_irqrestore(&fsl_lbc_lock, flags);
-
-	return ret;
-}
+extern int fsl_upm_run_pattern(struct fsl_upm *upm, void __iomem *io_base,
+			       u32 mar);
 
 #endif /* __ASM_FSL_LBC_H */

arch/powerpc/sysdev/fsl_lbc.c +41 -0

50 lines not on this page · raw message

(This also adds missing linux/spinlock.h include to the fsl_lbc.c)

Anyhow, whatever approach you chose, feel free to add my:

Acked-by Anton Vorontsov <[email protected]>

Then please resend the patch with added
Cc: Kumar Gala <[email protected]>, [email protected]

^^ Kumar would apply the patch to his powerpc.git tree, powerpc-next branch.

Signature · 3 lines

Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

Anton Vorontsov

2008-10-08 23:17 UTC #2 lore raw

Quoted 2 lines · Anton Vorontsov

Though better option would be to uninline the fsl_upm_run_pattern().. it is quite big anyway... Something like this:

Even better. We don't need exported fsl_lbc_regs too. And also we can mark the lock and regs as static. Yeah, w/o inline things look much better.

arch/powerpc/include/asm/fsl_lbc.h +2 ~~-43~~

index 303f548..375f46c 100644
@@ -24,7 +24,6 @@
 #define __ASM_FSL_LBC_H
 
 #include <linux/types.h>
-#include <linux/spinlock.h>
 #include <asm/io.h>
 
 struct fsl_lbc_bank {

@@ -227,9 +226,6 @@ struct fsl_lbc_regs {
 	u8 res8[0xF00];
 };
 
-extern struct fsl_lbc_regs __iomem *fsl_lbc_regs;
-extern spinlock_t fsl_lbc_lock;
-
 /*
  * FSL UPM routines
  */

@@ -268,44 +264,7 @@ static inline void fsl_upm_end_pattern(struct fsl_upm *upm)
 		cpu_relax();
 }
 
-/**
- * fsl_upm_run_pattern - actually run an UPM pattern
- * @upm:	pointer to the fsl_upm structure obtained via fsl_upm_find
- * @io_base:	remapped pointer to where memory access should happen
- * @mar:	MAR register content during pattern execution
- *
- * This function triggers dummy write to the memory specified by the io_base,
- * thus UPM pattern actually executed. Note that mar usage depends on the
- * pre-programmed AMX bits in the UPM RAM.
- */
-static inline int fsl_upm_run_pattern(struct fsl_upm *upm,
-				      void __iomem *io_base, u32 mar)
-{
-	int ret = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&fsl_lbc_lock, flags);
-
-	out_be32(&fsl_lbc_regs->mar, mar << (32 - upm->width));
-
-	switch (upm->width) {
-	case 8:
-		out_8(io_base, 0x0);
-		break;
-	case 16:
-		out_be16(io_base, 0x0);
-		break;
-	case 32:
-		out_be32(io_base, 0x0);
-		break;
-	default:
-		ret = -EINVAL;
-		break;
-	}
-
-	spin_unlock_irqrestore(&fsl_lbc_lock, flags);
-
-	return ret;
-}
+extern int fsl_upm_run_pattern(struct fsl_upm *upm, void __iomem *io_base,
+			       u32 mar);
 
 #endif /* __ASM_FSL_LBC_H */

arch/powerpc/sysdev/fsl_lbc.c +43 -3

58 lines not on this page · raw message

Dave Jones

author

2008-10-08 23:33 UTC #3 lore raw

In reply to Anton Vorontsov

Quoted 12 lines · Anton Vorontsov

Hi,
Quoted 5 lines · Dave Jones
Hi Anton,
I tried reenabling CONFIG_MTD_NAND_FSL_UPM in the Fedora kernel
today (currently on 2.6.27-rc9-git1), and got the error below.
ERROR: "fsl_lbc_lock" [drivers/mtd/nand/fsl_upm.ko] undefined!
Weird. It works for me with mpc836x_rdk_defconfig and MTD_NAND_FSL_UPM=m...

Quoted 6 lines

Hm..
CC drivers/mtd/nand/fsl_upm.mod.o
LD [M] drivers/mtd/nand/fsl_upm.ko
Can you send me the .config file you use?

http://davej.fedorapeople.org/ppc.config

Quoted 4 lines

But the patch looks obviously correct. Much thanks for catching this.
Though better option would be to uninline the fsl_upm_run_pattern().. it is quite big anyway... Something like this:

I'll leave it to you to decide what to push to Linus

Dave

Signature · 1 line

http://www.codemonkey.org.uk

Anton Vorontsov

2008-10-09 00:33 UTC #4 lore raw

PATCH 1/1 powerpc: fix fsl_upm nand driver modular build

The fsl_upm nand driver fails to build because fsl_lbc_lock isn't exported, the lock is needed by the inlined fsl_upm_run_pattern() function:

ERROR: "fsl_lbc_lock" [drivers/mtd/nand/fsl_upm.ko] undefined!

Dave Jones purposed to export the lock, but it is better to just uninline the fsl_upm_run_pattern().

When uninlined we also no longer need the exported fsl_lbc_regs, and both fsl_lbc_lock and fsl_lbc_regs could be marked static.

While at it, also add some missing includes that we should have included explicitly.

Reported-by Dave Jones <[email protected]>

Signed-off-by Anton Vorontsov <[email protected]>

---

On Wed, Oct 08, 2008 at 07:33:06PM -0400, Dave Jones wrote:
[...]

Quoted 5 lines

Quoted 5 lines
Quoted 1 line
ERROR: "fsl_lbc_lock" [drivers/mtd/nand/fsl_upm.ko] undefined!
Weird. It works for me with mpc836x_rdk_defconfig and MTD_NAND_FSL_UPM=m...

Quoted 8 lines

Quoted 6 lines
Hm..
CC drivers/mtd/nand/fsl_upm.mod.o
LD [M] drivers/mtd/nand/fsl_upm.ko
Can you send me the .config file you use?
http://davej.fedorapeople.org/ppc.config

Still works here. :-/ I'm using latest Linus' git tree, pulled few minutes ago just in case.

gcc version 4.2.0 (MontaVista 4.2.0-16.0.20.0800760 2008-04-05) GNU ld (GNU Binutils) 2.17.50.20070611

Most probably something on my side, maybe need to update the toolchain.

Quoted 6 lines

Quoted 4 lines
But the patch looks obviously correct. Much thanks for catching this.
Though better option would be to uninline the fsl_upm_run_pattern().. it is quite big anyway... Something like this:
I'll leave it to you to decide what to push to Linus

Ok, here it is. Since I'm unable to reproduce the error, I'd appreciate Tested-by: tag if you could find time to build-test the patch. It works for me though...

Thanks again,

 arch/powerpc/include/asm/fsl_lbc.h |   48 +++-----------------------------
 arch/powerpc/sysdev/fsl_lbc.c      |   53 +++++++++++++++++++++++++++++++++---
 2 files changed, 53 insertions(+), 48 deletions(-)

arch/powerpc/include/asm/fsl_lbc.h +4 ~~-44~~

index 303f548..63a4f77 100644
@@ -23,9 +23,9 @@
 #ifndef __ASM_FSL_LBC_H
 #define __ASM_FSL_LBC_H
 
+#include <linux/compiler.h>
 #include <linux/types.h>
-#include <linux/spinlock.h>
-#include <asm/io.h>
+#include <linux/io.h>
 
 struct fsl_lbc_bank {
 	__be32 br;             /**< Base Register  */

@@ -227,9 +227,6 @@ struct fsl_lbc_regs {
 	u8 res8[0xF00];
 };
 
-extern struct fsl_lbc_regs __iomem *fsl_lbc_regs;
-extern spinlock_t fsl_lbc_lock;
-
 /*
  * FSL UPM routines
  */

@@ -268,44 +265,7 @@ static inline void fsl_upm_end_pattern(struct fsl_upm *upm)
 		cpu_relax();
 }
 
-/**
- * fsl_upm_run_pattern - actually run an UPM pattern
- * @upm:	pointer to the fsl_upm structure obtained via fsl_upm_find
- * @io_base:	remapped pointer to where memory access should happen
- * @mar:	MAR register content during pattern execution
- *
- * This function triggers dummy write to the memory specified by the io_base,
- * thus UPM pattern actually executed. Note that mar usage depends on the
- * pre-programmed AMX bits in the UPM RAM.
- */
-static inline int fsl_upm_run_pattern(struct fsl_upm *upm,
-				      void __iomem *io_base, u32 mar)
-{
-	int ret = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&fsl_lbc_lock, flags);
-
-	out_be32(&fsl_lbc_regs->mar, mar << (32 - upm->width));
-
-	switch (upm->width) {
-	case 8:
-		out_8(io_base, 0x0);
-		break;
-	case 16:
-		out_be16(io_base, 0x0);
-		break;
-	case 32:
-		out_be32(io_base, 0x0);
-		break;
-	default:
-		ret = -EINVAL;
-		break;
-	}
-
-	spin_unlock_irqrestore(&fsl_lbc_lock, flags);
-
-	return ret;
-}
+extern int fsl_upm_run_pattern(struct fsl_upm *upm, void __iomem *io_base,
+			       u32 mar);
 
 #endif /* __ASM_FSL_LBC_H */

arch/powerpc/sysdev/fsl_lbc.c +49 -4

66 lines not on this page · raw message

Signature · 1 line

1.5.6.3

Kumar Gala

Applied

2008-10-13 16:48 UTC #5 lore raw

Re: [PATCH] powerpc: fix fsl_upm nand driver modular build

Quoted 12 lines · Anton Vorontsov

The fsl_upm nand driver fails to build because fsl_lbc_lock isn't exported, the lock is needed by the inlined fsl_upm_run_pattern() function:
ERROR: "fsl_lbc_lock" [drivers/mtd/nand/fsl_upm.ko] undefined!
Dave Jones purposed to export the lock, but it is better to just uninline the fsl_upm_run_pattern().
When uninlined we also no longer need the exported fsl_lbc_regs, and both fsl_lbc_lock and fsl_lbc_regs could be marked static.

Quoted 6 lines

While at it, also add some missing includes that we should have included explicitly.
Reported-by Dave Jones <[email protected]>
Signed-off-by Anton Vorontsov <[email protected]>
---

applied.

- k