[U-Boot] [PATCH] mtd: nand: new base driver for memory mapped nand devices

[U-Boot] [PATCH] mtd: nand: new base driver for memory mapped nand devices

[Mike Frysinger]

The BF537-STAMP Blackfin board had a driver for working with NAND devices
that are simply memory mapped.  Since there is nothing Blackfin specific
about this, generalize the driver a bit so that everyone can leverage it.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
CC: Scott Wood <scottwood@freescale.com>
---
 board/bf537-stamp/Makefile        |    1 -
 board/bf537-stamp/nand.c          |  100 -------------------------------------
 drivers/mtd/nand/Makefile         |    1 +
 drivers/mtd/nand/nand_plat.c      |   66 ++++++++++++++++++++++++
 include/configs/bf537-stamp.h     |   43 +++++-----------
 include/configs/bfin_adi_common.h |    3 +
 6 files changed, 84 insertions(+), 130 deletions(-)
 delete mode 100644 board/bf537-stamp/nand.c
 create mode 100644 drivers/mtd/nand/nand_plat.c

diff --git a/board/bf537-stamp/Makefile b/board/bf537-stamp/Makefile
index 1dbf406..6da04e3 100644
--- a/board/bf537-stamp/Makefile
+++ b/board/bf537-stamp/Makefile
@@ -32,7 +32,6 @@ LIB	= $(obj)lib$(BOARD).a
 COBJS-y	:= $(BOARD).o cmd_bf537led.o
 COBJS-$(CONFIG_BFIN_IDE)   += ide-cf.o
 COBJS-$(CONFIG_CMD_EEPROM) += spi_flash.o
-COBJS-$(CONFIG_CMD_NAND)   += nand.o
 COBJS-$(CONFIG_POST)       += post.o post-memory.o
 
 SRCS	:= $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
diff --git a/board/bf537-stamp/nand.c b/board/bf537-stamp/nand.c
deleted file mode 100644
index 181e83d..0000000
--- a/board/bf537-stamp/nand.c
+++ /dev/null
@@ -1,100 +0,0 @@
-/*
- * Copyright (c) 2006-2007 Analog Devices Inc.
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-#include <common.h>
-#include <asm/io.h>
-
-#include <nand.h>
-
-#define CONCAT(a,b,c,d) a ## b ## c ## d
-#define PORT(a,b)  CONCAT(pPORT,a,b,)
-
-#ifndef CONFIG_NAND_GPIO_PORT
-#define CONFIG_NAND_GPIO_PORT F
-#endif
-
-/*
- * hardware specific access to control-lines
- */
-static void bfin_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
-{
-	register struct nand_chip *this = mtd->priv;
-	u32 IO_ADDR_W = (u32) this->IO_ADDR_W;
-
-	if (ctrl & NAND_CTRL_CHANGE) {
-		if (ctrl & NAND_CLE)
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE + BFIN_NAND_CLE;
-		else
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE;
-		if (ctrl & NAND_ALE)
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE + BFIN_NAND_ALE;
-		else
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE;
-		this->IO_ADDR_W = (void __iomem *) IO_ADDR_W;
-	}
-	this->IO_ADDR_R = this->IO_ADDR_W;
-
-	/* Drain the writebuffer */
-	SSYNC();
-
-	if (cmd != NAND_CMD_NONE)
-		writeb(cmd, this->IO_ADDR_W);
-}
-
-int bfin_device_ready(struct mtd_info *mtd)
-{
-	int ret = (*PORT(CONFIG_NAND_GPIO_PORT, IO) & BFIN_NAND_READY) ? 1 : 0;
-	SSYNC();
-	return ret;
-}
-
-/*
- * Board-specific NAND initialization. The following members of the
- * argument are board-specific (per include/linux/mtd/nand.h):
- * - IO_ADDR_R?: address to read the 8 I/O lines of the flash device
- * - IO_ADDR_W?: address to write the 8 I/O lines of the flash device
- * - cmd_ctrl: hardwarespecific function for accesing control-lines
- * - dev_ready: hardwarespecific function for  accesing device ready/busy line
- * - enable_hwecc?: function to enable (reset)  hardware ecc generator. Must
- *   only be provided if a hardware ECC is available
- * - ecc.mode: mode of ecc, see defines
- * - chip_delay: chip dependent delay for transfering data from array to
- *   read regs (tR)
- * - options: various chip options. They can partly be set to inform
- *   nand_scan about special functionality. See the defines for further
- *   explanation
- * Members with a "?" were not set in the merged testing-NAND branch,
- * so they are not set here either.
- */
-int board_nand_init(struct nand_chip *nand)
-{
-	*PORT(CONFIG_NAND_GPIO_PORT, _FER) &= ~BFIN_NAND_READY;
-	*PORT(CONFIG_NAND_GPIO_PORT, IO_DIR) &= ~BFIN_NAND_READY;
-	*PORT(CONFIG_NAND_GPIO_PORT, IO_INEN) |= BFIN_NAND_READY;
-
-	nand->cmd_ctrl = bfin_hwcontrol;
-	nand->ecc.mode = NAND_ECC_SOFT;
-	nand->dev_ready = bfin_device_ready;
-	nand->chip_delay = 30;
-
-	return 0;
-}
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 5974d77..46ad901 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -41,6 +41,7 @@ COBJS-$(CONFIG_NAND_FSL_UPM) += fsl_upm.o
 COBJS-$(CONFIG_NAND_NOMADIK) += nomadik.o
 COBJS-$(CONFIG_NAND_S3C64XX) += s3c64xx.o
 COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
+COBJS-$(CONFIG_NAND_PLAT) += nand_plat.o
 endif
 
 COBJS	:= $(COBJS-y)
diff --git a/drivers/mtd/nand/nand_plat.c b/drivers/mtd/nand/nand_plat.c
new file mode 100644
index 0000000..e39503b
--- /dev/null
+++ b/drivers/mtd/nand/nand_plat.c
@@ -0,0 +1,66 @@
+/*
+ * Copyright (c) 2006-2009 Analog Devices Inc.
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <asm/io.h>
+
+#include <nand.h>
+
+#ifdef NAND_PLAT_WRITE_CMD
+static void cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
+{
+	struct nand_chip *this = mtd->priv;
+
+	if (cmd == NAND_CMD_NONE)
+		return;
+
+	if (ctrl & NAND_CLE)
+		NAND_PLAT_WRITE_CMD(cmd, this);
+	else
+		NAND_PLAT_WRITE_ADR(cmd, this);
+
+	/* Drain the writebuffer */
+	sync();
+}
+#else
+# define cmd_ctrl NULL
+#endif
+
+#ifdef NAND_PLAT_DEV_READY
+static int dev_ready(struct mtd_info *mtd)
+{
+	return NAND_PLAT_DEV_READY((struct nand_chip *)mtd->priv);
+}
+#else
+# define dev_ready NULL
+#endif
+
+int board_nand_init(struct nand_chip *nand)
+{
+	NAND_PLAT_INIT();
+
+	nand->cmd_ctrl = cmd_ctrl;
+	nand->dev_ready = dev_ready;
+	nand->ecc.mode = NAND_ECC_SOFT;
+
+	return 0;
+}
diff --git a/include/configs/bf537-stamp.h b/include/configs/bf537-stamp.h
index 3e5862d..91159c0 100644
--- a/include/configs/bf537-stamp.h
+++ b/include/configs/bf537-stamp.h
@@ -136,36 +136,21 @@
 /*
  * NAND Settings
  */
-/* #define CONFIG_BF537_NAND */
-#ifdef CONFIG_BF537_NAND
-# define CONFIG_CMD_NAND
-#endif
-
-#define CONFIG_SYS_NAND_ADDR		0x20212000
-#define CONFIG_SYS_NAND_BASE		CONFIG_SYS_NAND_ADDR
+/* #define CONFIG_NAND_PLAT */
+#define CONFIG_SYS_NAND_BASE		0x20212000
 #define CONFIG_SYS_MAX_NAND_DEVICE	1
-#define SECTORSIZE		512
-#define ADDR_COLUMN		1
-#define ADDR_PAGE		2
-#define ADDR_COLUMN_PAGE	3
-#define NAND_ChipID_UNKNOWN	0x00
-#define NAND_MAX_FLOORS		1
-#define BFIN_NAND_READY		PF3
-
-#define NAND_WAIT_READY(nand) \
-	do { \
-		int timeout = 0; \
-		while (!(*pPORTFIO & PF3)) \
-			if (timeout++ > 100000) \
-				break; \
-	} while (0)
-
-#define BFIN_NAND_CLE		(1 << 2)	/* A2 -> Command Enable */
-#define BFIN_NAND_ALE		(1 << 1)	/* A1 -> Address Enable */
-#define WRITE_NAND_COMMAND(d, adr) bfin_write8(adr | BFIN_NAND_CLE, d)
-#define WRITE_NAND_ADDRESS(d, adr) bfin_write8(adr | BFIN_NAND_ALE, d)
-#define WRITE_NAND(d, adr)         bfin_write8(adr, d)
-#define READ_NAND(adr)             bfin_read8(adr)
+
+#define BFIN_NAND_CLE(chip) ((unsigned long)chip->IO_ADDR_W | (1 << 2))
+#define BFIN_NAND_ALE(chip) ((unsigned long)chip->IO_ADDR_W | (1 << 1))
+#define BFIN_NAND_READY     PF3
+
+#define NAND_PLAT_WRITE_CMD(cmd, chip) bfin_write8(BFIN_NAND_CLE(chip), cmd)
+#define NAND_PLAT_WRITE_ADR(cmd, chip) bfin_write8(BFIN_NAND_ALE(chip), cmd)
+#define NAND_PLAT_DEV_READY(chip)      ((*pPORTFIO & BFIN_NAND_READY) ? 1 : 0)
+#define NAND_PLAT_INIT() \
+	*pPORTF_FER &= ~BFIN_NAND_READY; \
+	*pPORTFIO_DIR &= ~BFIN_NAND_READY; \
+	*pPORTFIO_INEN |= BFIN_NAND_READY;
 
 
 /*
diff --git a/include/configs/bfin_adi_common.h b/include/configs/bfin_adi_common.h
index 030e63c..fa96dcf 100644
--- a/include/configs/bfin_adi_common.h
+++ b/include/configs/bfin_adi_common.h
@@ -38,6 +38,9 @@
 #  define CONFIG_CMD_USB_STORAGE
 #  define CONFIG_DOS_PARTITION
 # endif
+# ifdef CONFIG_NAND_PLAT
+#  define CONFIG_CMD_NAND
+# endif
 # ifdef CONFIG_POST
 #  define CONFIG_CMD_DIAG
 # endif
-- 
1.6.2.2

[U-Boot] [PATCH] mtd: nand: new base driver for memory mapped nand devices

[Scott Wood]

On Sat, Apr 11, 2009 at 09:26:42PM -0400, Mike Frysinger wrote:
> +#ifdef NAND_PLAT_WRITE_CMD

Why would a user select this driver without providing the necessary
definitions -- and if they do, why do you want anything other than
a compilation error to result?

+	/* Drain the writebuffer */
+	sync();

This doesn't look generic to me.

> +#define BFIN_NAND_CLE(chip) ((unsigned long)chip->IO_ADDR_W | (1 << 2))
> +#define BFIN_NAND_ALE(chip) ((unsigned long)chip->IO_ADDR_W | (1 << 1))
> +#define BFIN_NAND_READY     PF3

You have a global variable called "PF3"?

> +#define NAND_PLAT_WRITE_CMD(cmd, chip) bfin_write8(BFIN_NAND_CLE(chip), cmd)
> +#define NAND_PLAT_WRITE_ADR(cmd, chip) bfin_write8(BFIN_NAND_ALE(chip), cmd)
> +#define NAND_PLAT_DEV_READY(chip)      ((*pPORTFIO & BFIN_NAND_READY) ? 1 : 0)
> +#define NAND_PLAT_INIT() \
> +	*pPORTF_FER &= ~BFIN_NAND_READY; \
> +	*pPORTFIO_DIR &= ~BFIN_NAND_READY; \
> +	*pPORTFIO_INEN |= BFIN_NAND_READY;

I'm not too fond of such things being done through header files -- it
means that only one type of so-called "memory mapped" NAND device can be
supported in a given u-boot image.  If it doesn't add too much image size
overhead, I'd prefer having platform code register a struct of callbacks
(or just live with the duplication of 10-15 almost-but-not-quite-generic
lines, and focus on factoring out instances where they're truly
identical).

If we do do it in the header file, though, at least use static inline
functions rather than macros -- besides being less visually obnoxious,
they provide type checking of arguments and avoid problems with name
collisions.  And if you must use macros, always use this:

#define NAND_PLAT_INIT() do { \
	multi; \
	line; \
	macro; \
} while (0) \

rather than this:

#define NAND_PLAT_INIT() \
	multi; \
	line; \
	macro;

The latter will break if you put it in the body of a single-line if
statement.

-Scott

[U-Boot] [PATCH] mtd: nand: new base driver for memory mapped nand devices

[Mike Frysinger]

On Monday 13 April 2009 11:59:30 Scott Wood wrote:
> On Sat, Apr 11, 2009 at 09:26:42PM -0400, Mike Frysinger wrote:
> > +#ifdef NAND_PLAT_WRITE_CMD
>
> Why would a user select this driver without providing the necessary
> definitions -- and if they do, why do you want anything other than
> a compilation error to result?

*shrug* ... i'm not completely familiar with the nand layers and what people 
have done to know exactly what is optional.  easy enough to turn it into:
#ifndef NAND_PLAT_WRITE_CMD
# error "You must define NAND_PLAT_WRITE_CMD"
#endif

> +	/* Drain the writebuffer */
> +	sync();
>
> This doesn't look generic to me.

yes it does.  every arch should define "sync()" in asm/io.h.  if it doesnt, 
your arch is broken.

> > +#define BFIN_NAND_CLE(chip) ((unsigned long)chip->IO_ADDR_W | (1 << 2))
> > +#define BFIN_NAND_ALE(chip) ((unsigned long)chip->IO_ADDR_W | (1 << 1))
> > +#define BFIN_NAND_READY     PF3
>
> You have a global variable called "PF3"?

a global define actually, but yes

> > +#define NAND_PLAT_WRITE_CMD(cmd, chip) bfin_write8(BFIN_NAND_CLE(chip),
> > cmd) +#define NAND_PLAT_WRITE_ADR(cmd, chip)
> > bfin_write8(BFIN_NAND_ALE(chip), cmd) +#define NAND_PLAT_DEV_READY(chip) 
> >     ((*pPORTFIO & BFIN_NAND_READY) ? 1 : 0) +#define NAND_PLAT_INIT() \
> > +	*pPORTF_FER &= ~BFIN_NAND_READY; \
> > +	*pPORTFIO_DIR &= ~BFIN_NAND_READY; \
> > +	*pPORTFIO_INEN |= BFIN_NAND_READY;
>
> I'm not too fond of such things being done through header files -- it
> means that only one type of so-called "memory mapped" NAND device can be
> supported in a given u-boot image.  If it doesn't add too much image size
> overhead, I'd prefer having platform code register a struct of callbacks
> (or just live with the duplication of 10-15 almost-but-not-quite-generic
> lines, and focus on factoring out instances where they're truly
> identical).

doing it in the header follows u-boot convention, and it's much easier than 
creating a dedicated file.  doesnt matter to me.

> If we do do it in the header file, though, at least use static inline
> functions rather than macros -- besides being less visually obnoxious,
> they provide type checking of arguments and avoid problems with name
> collisions.

actually, it kind of does the opposite.  it increases name space pollution.  
if someone does a #define with the same variable name or similar as is used in 
the function, then you can easily get a build failure.  see all the random 
times this has caused a problem with linux/glibc/uClibc and just function 
prototypes let alone function definitions.  plus, not so critically, using 
static inlines would slow down the compiler as it would need to compile & 
optimize & consider it in every single file rather than letting the CPP cull 
it early on.

> The latter will break if you put it in the body of a single-line if
> statement.

i'm fully aware of this, but didnt care since i knew how it was used
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090413/227bf00c/attachment.pgp 

[U-Boot] [PATCH] mtd: nand: new base driver for memory mapped nand devices

[Scott Wood]

Mike Frysinger wrote:
> On Monday 13 April 2009 11:59:30 Scott Wood wrote:
>> On Sat, Apr 11, 2009 at 09:26:42PM -0400, Mike Frysinger wrote:
>>> +#ifdef NAND_PLAT_WRITE_CMD
>> Why would a user select this driver without providing the necessary
>> definitions -- and if they do, why do you want anything other than
>> a compilation error to result?
> 
> *shrug* ... i'm not completely familiar with the nand layers and what people 
> have done to know exactly what is optional.  

You're defining the interface -- there are no existing users.

> easy enough to turn it into:
> #ifndef NAND_PLAT_WRITE_CMD
> # error "You must define NAND_PLAT_WRITE_CMD"
> #endif

Or just let the compiler give an undefined symbol error.

>> +	/* Drain the writebuffer */
>> +	sync();
>>
>> This doesn't look generic to me.
> 
> yes it does.  every arch should define "sync()" in asm/io.h.  if it doesnt, 
> your arch is broken.

I realize that there is a "sync" defined in every architecture 
(otherwise, my comment would have been "this breaks on XXX arch").

However, the need to do a sync in this specific situation is specific to 
how NAND_PLAT_WRITE_* are implemented (in many cases, they will have 
already included a sync or something similar -- they're often included 
in the basic I/O accessors).  And the specific comment about a 
"writebuffer" seems even more out of place in generic code.

>> I'm not too fond of such things being done through header files -- it
>> means that only one type of so-called "memory mapped" NAND device can be
>> supported in a given u-boot image.  If it doesn't add too much image size
>> overhead, I'd prefer having platform code register a struct of callbacks
>> (or just live with the duplication of 10-15 almost-but-not-quite-generic
>> lines, and focus on factoring out instances where they're truly
>> identical).
> 
> doing it in the header follows u-boot convention, and it's much easier than 
> creating a dedicated file.  doesnt matter to me.

That convention has been the subject of some (quite justified, IMHO) 
complaints recently.

>> If we do do it in the header file, though, at least use static inline
>> functions rather than macros -- besides being less visually obnoxious,
>> they provide type checking of arguments and avoid problems with name
>> collisions.
> 
> actually, it kind of does the opposite.  it increases name space pollution.  
> if someone does a #define with the same variable name or similar as is used in 
> the function, then you can easily get a build failure.

The root cause of that is the namespace-polluting #define, not the 
function.  It would just as easily cause problems with code in .c files 
(including when your macros get expanded) as with inline functions in 
headers.

> see all the random times this has caused a problem with linux/glibc/uClibc and just function 
> prototypes let alone function definitions.

This is an internal header file, not a public library header that is 
standards-constrained to accept #define interference from the application.

> plus, not so critically, using 
> static inlines would slow down the compiler as it would need to compile & 
> optimize & consider it in every single file rather than letting the CPP cull 
> it early on.

On the other hand, that means that errors get caught immediately rather 
than when usage changes.

>> The latter will break if you put it in the body of a single-line if
>> statement.
> 
> i'm fully aware of this, but didnt care since i knew how it was used

And maybe it gets used differently in the future?  Or someone copies the 
bad example to somewhere else where it matters?

-Scott

[U-Boot] [PATCH] mtd: nand: new base driver for memory mapped nand devices

[Mike Frysinger]

On Monday 13 April 2009 17:42:00 Scott Wood wrote:
> Mike Frysinger wrote:
> > On Monday 13 April 2009 11:59:30 Scott Wood wrote:
> >> On Sat, Apr 11, 2009 at 09:26:42PM -0400, Mike Frysinger wrote:
> >>> +#ifdef NAND_PLAT_WRITE_CMD
> >>
> >> Why would a user select this driver without providing the necessary
> >> definitions -- and if they do, why do you want anything other than
> >> a compilation error to result?
> >
> > *shrug* ... i'm not completely familiar with the nand layers and what
> > people have done to know exactly what is optional.
>
> You're defining the interface -- there are no existing users.

just because *my* device needs it doesnt mean every device needs it.  i was 
trying to be nice from the get go.

> > easy enough to turn it into:
> > #ifndef NAND_PLAT_WRITE_CMD
> > # error "You must define NAND_PLAT_WRITE_CMD"
> > #endif
>
> Or just let the compiler give an undefined symbol error.

true, but i think that route leads to people grepping files and scratching 
their heads.  an #error would save them some time.

> >> +	/* Drain the writebuffer */
> >> +	sync();
> >>
> >> This doesn't look generic to me.
> >
> > yes it does.  every arch should define "sync()" in asm/io.h.  if it
> > doesnt, your arch is broken.
>
> I realize that there is a "sync" defined in every architecture
> (otherwise, my comment would have been "this breaks on XXX arch").
>
> However, the need to do a sync in this specific situation is specific to
> how NAND_PLAT_WRITE_* are implemented (in many cases, they will have
> already included a sync or something similar -- they're often included
> in the basic I/O accessors).  And the specific comment about a
> "writebuffer" seems even more out of place in generic code.

if they're included in the I/O accessors, then the arch most likely defines 
sync() to nothing, so it doesnt matter.  "write buffer" may not be entirely 
arch independent, but it conveys the exact same thing as "make sure write 
makes it to external device".  this is how sync() is used -- just grep the 
drivers tree and see the smsc driver for example.

> >> If we do do it in the header file, though, at least use static inline
> >> functions rather than macros -- besides being less visually obnoxious,
> >> they provide type checking of arguments and avoid problems with name
> >> collisions.
> >
> > actually, it kind of does the opposite.  it increases name space
> > pollution. if someone does a #define with the same variable name or
> > similar as is used in the function, then you can easily get a build
> > failure.
>
> The root cause of that is the namespace-polluting #define, not the
> function.  It would just as easily cause problems with code in .c files
> (including when your macros get expanded) as with inline functions in
> headers.

or accidental shadowing of global state, but i guess you dont care much about 
that usage either

> > see all the random times this has caused a problem with
> > linux/glibc/uClibc and just function prototypes let alone function
> > definitions.
>
> This is an internal header file, not a public library header that is
> standards-constrained to accept #define interference from the application.

really ?  you call internal kernel headers "standards constrained" ?  my point 
is that it's seen in both scenarios.

> > plus, not so critically, using
> > static inlines would slow down the compiler as it would need to compile &
> > optimize & consider it in every single file rather than letting the CPP
> > cull it early on.
>
> On the other hand, that means that errors get caught immediately rather
> than when usage changes.

indeed

> >> The latter will break if you put it in the body of a single-line if
> >> statement.
> >
> > i'm fully aware of this, but didnt care since i knew how it was used
>
> And maybe it gets used differently in the future?  Or someone copies the
> bad example to somewhere else where it matters?

people should check their work before they hand in their paper ;)
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090413/7d02c21b/attachment.pgp 

[U-Boot] [PATCH] mtd: nand: new base driver for memory mapped nand devices

[Scott Wood]

Mike Frysinger wrote:
> On Monday 13 April 2009 17:42:00 Scott Wood wrote:
>> You're defining the interface -- there are no existing users.
> 
> just because *my* device needs it doesnt mean every device needs it.  i was 
> trying to be nice from the get go.

Every device that is a reasonable candidate for using such a shared 
driver does need it.  If a device is different enough that a "write a 
command byte/word" operation doesn't map well (such as Freescale's 
eLBC), then we shouldn't try to force it into a so-called generic framework.

In other words, this is just to templatize a common NAND driver pattern, 
not to replace the main NAND driver interface.

>>> easy enough to turn it into:
>>> #ifndef NAND_PLAT_WRITE_CMD
>>> # error "You must define NAND_PLAT_WRITE_CMD"
>>> #endif
>> Or just let the compiler give an undefined symbol error.
> 
> true, but i think that route leads to people grepping files and scratching 
> their heads.  an #error would save them some time.

If we did that every place some arch- or platform-defined symbol were 
used, the source code would be (even more of) a mess.

Instead, how about putting a comment at the top of the file stating what 
it requires from the platform, and describing the semantics of each 
item?  That would save more head-scratching than simply turning "not 
defined" into "you must define".

>> However, the need to do a sync in this specific situation is specific to
>> how NAND_PLAT_WRITE_* are implemented (in many cases, they will have
>> already included a sync or something similar -- they're often included
>> in the basic I/O accessors).  And the specific comment about a
>> "writebuffer" seems even more out of place in generic code.
> 
> if they're included in the I/O accessors, then the arch most likely defines 
> sync() to nothing, so it doesnt matter.

Um, no.  See powerpc for example.

Explicit barriers are for when you use raw I/O accessors in optimized 
paths, or when you need to synchronize accesses to main memory (such as 
in a DMA buffer).

> "write buffer" may not be entirely 
> arch independent, but it conveys the exact same thing as "make sure write 
> makes it to external device".  this is how sync() is used -- just grep the 
> drivers tree and see the smsc driver for example.

I did grep, and the only non-arch-specific use I found was in cfi_flash.
I'm not sure what you mean by the "smsc driver".

>> The root cause of that is the namespace-polluting #define, not the
>> function.  It would just as easily cause problems with code in .c files
>> (including when your macros get expanded) as with inline functions in
>> headers.
> 
> or accidental shadowing of global state, but i guess you dont care much about 
> that usage either

I care very much about avoiding accidents.  Preprocessor macros cause 
accidents. :-)

>>> see all the random times this has caused a problem with
>>> linux/glibc/uClibc and just function prototypes let alone function
>>> definitions.
>> This is an internal header file, not a public library header that is
>> standards-constrained to accept #define interference from the application.
> 
> really ?  you call internal kernel headers "standards constrained" ?

Linux contains headers used by glibc; those parts are indeed standards 
constrained.  And while the user-visible headers that aren't directly 
included in a glibc header aren't specifically constrained by the C 
standard, they're intended to be used in more or less arbitrary C 
environments in ways similar to the C library, so 
quality-of-implementation dictates that similar care be taken.

As for the internal headers, any such problem could be remedied by 
fixing the offending #define.  Further, inline functions seem to be 
preferred over macros in Linux, so I'm not sure that they're the best 
example to pick to justify using macros.

Indeed, I'm not sure what relevance this has at all -- the solution to 
the "variable has the same name as a user define" problem in public 
headers is to use a reserved namespace, not to use a macro.  Macros make 
the problem worse by making it possible to alias local variables in the 
expanding context (which may have been used as macro parameters), as 
well as #defines.

Which reminds me:
> +#define BFIN_NAND_CLE(chip) ((unsigned long)chip->IO_ADDR_W | (1 << 2))
> +#define BFIN_NAND_ALE(chip) ((unsigned long)chip->IO_ADDR_W | (1 << 1))

Put parentheses around chip, in case the caller provided a complex 
expression.

-Scott

[U-Boot] [PATCH] mtd: nand: new base driver for memory mapped nand devices

[Mike Frysinger]

On Monday 13 April 2009 19:02:05 Scott Wood wrote:
> Mike Frysinger wrote:
> > On Monday 13 April 2009 17:42:00 Scott Wood wrote:
> >> You're defining the interface -- there are no existing users.
> >
> > just because *my* device needs it doesnt mean every device needs it.  i
> > was trying to be nice from the get go.
>
> Every device that is a reasonable candidate for using such a shared
> driver does need it.  If a device is different enough that a "write a
> command byte/word" operation doesn't map well (such as Freescale's
> eLBC), then we shouldn't try to force it into a so-called generic
> framework.
>
> In other words, this is just to templatize a common NAND driver pattern,
> not to replace the main NAND driver interface.

as i said, i'm not familiar with other use cases, so i cant pick out exactly   
what is common/required and what isnt.  you can just state which is which and 
i'll do it.

> >>> easy enough to turn it into:
> >>> #ifndef NAND_PLAT_WRITE_CMD
> >>> # error "You must define NAND_PLAT_WRITE_CMD"
> >>> #endif
> >>
> >> Or just let the compiler give an undefined symbol error.
> >
> > true, but i think that route leads to people grepping files and
> > scratching their heads.  an #error would save them some time.
>
> If we did that every place some arch- or platform-defined symbol were
> used, the source code would be (even more of) a mess.

there's a difference between "used" and "required".  the #error makes sense in 
the face of no documentation.

> Instead, how about putting a comment at the top of the file stating what
> it requires from the platform, and describing the semantics of each
> item?  That would save more head-scratching than simply turning "not
> defined" into "you must define".

that works for me

> >> However, the need to do a sync in this specific situation is specific to
> >> how NAND_PLAT_WRITE_* are implemented (in many cases, they will have
> >> already included a sync or something similar -- they're often included
> >> in the basic I/O accessors).  And the specific comment about a
> >> "writebuffer" seems even more out of place in generic code.
> >
> > if they're included in the I/O accessors, then the arch most likely
> > defines sync() to nothing, so it doesnt matter.
>
> Um, no.  See powerpc for example.
>
> Explicit barriers are for when you use raw I/O accessors in optimized
> paths, or when you need to synchronize accesses to main memory (such as
> in a DMA buffer).

OK

> > "write buffer" may not be entirely
> > arch independent, but it conveys the exact same thing as "make sure write
> > makes it to external device".  this is how sync() is used -- just grep
> > the drivers tree and see the smsc driver for example.
>
> I did grep, and the only non-arch-specific use I found was in cfi_flash.
> I'm not sure what you mean by the "smsc driver".

it used to be in there, but it isnt anymore.  guess it changed since i last 
read it (but that was back in like 1.1.6 days).

> >>> see all the random times this has caused a problem with
> >>> linux/glibc/uClibc and just function prototypes let alone function
> >>> definitions.
> >>
> >> This is an internal header file, not a public library header that is
> >> standards-constrained to accept #define interference from the
> >> application.
> >
> > really ?  you call internal kernel headers "standards constrained" ?
>
> Linux contains headers used by glibc; those parts are indeed standards
> constrained.  And while the user-visible headers that aren't directly
> included in a glibc header aren't specifically constrained by the C
> standard, they're intended to be used in more or less arbitrary C
> environments in ways similar to the C library, so
> quality-of-implementation dictates that similar care be taken.

which is why so many of the exported headers use macros rather than static 
inlines.  they only get expanded and make a mess when they actually get used.

> As for the internal headers, any such problem could be remedied by
> fixing the offending #define.  Further, inline functions seem to be
> preferred over macros in Linux, so I'm not sure that they're the best
> example to pick to justify using macros.

except using defines avoiding the need to "fix" files constantly

> Indeed, I'm not sure what relevance this has at all -- the solution to
> the "variable has the same name as a user define" problem in public
> headers is to use a reserved namespace, not to use a macro.  Macros make
> the problem worse by making it possible to alias local variables in the
> expanding context (which may have been used as macro parameters), as
> well as #defines.

static inline usage works in the kernel because the headers are separated 
logically -- there is no config.h which gets included by every file and     
contains static inline functions or complex macros.

my point is that "static inline" vs "define" isnt as cut as try as you may 
like.  if we had a dedicated header file that only was included when needed 
(or included with the nand headers), then i'd have no problem with static 
inline.  but considering the board config header is included *everywhere*, 
including assembly files and when building the utilities on the host, C code 
should be avoided in it whenever possible.

> Which reminds me:
> > +#define BFIN_NAND_CLE(chip) ((unsigned long)chip->IO_ADDR_W | (1 << 2))
> > +#define BFIN_NAND_ALE(chip) ((unsigned long)chip->IO_ADDR_W | (1 << 1))
>
> Put parentheses around chip, in case the caller provided a complex
> expression.

thanks
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090413/cc58369b/attachment.pgp 

[U-Boot] [PATCH v2] mtd: nand: new base driver for memory mapped nand devices

[Mike Frysinger]

The BF537-STAMP Blackfin board had a driver for working with NAND devices
that are simply memory mapped.  Since there is nothing Blackfin specific
about this, generalize the driver a bit so that everyone can leverage it.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
CC: Scott Wood <scottwood@freescale.com>
---
v2
	- update based on feedback from Scott Wood

 board/bf537-stamp/Makefile        |    1 -
 board/bf537-stamp/nand.c          |  100 -------------------------------------
 drivers/mtd/nand/Makefile         |    1 +
 drivers/mtd/nand/nand_plat.c      |   53 +++++++++++++++++++
 include/configs/bf537-stamp.h     |   44 +++++++----------
 include/configs/bfin_adi_common.h |    3 +
 6 files changed, 75 insertions(+), 127 deletions(-)
 delete mode 100644 board/bf537-stamp/nand.c
 create mode 100644 drivers/mtd/nand/nand_plat.c

diff --git a/board/bf537-stamp/Makefile b/board/bf537-stamp/Makefile
index 1dbf406..6da04e3 100644
--- a/board/bf537-stamp/Makefile
+++ b/board/bf537-stamp/Makefile
@@ -32,7 +32,6 @@ LIB	= $(obj)lib$(BOARD).a
 COBJS-y	:= $(BOARD).o cmd_bf537led.o
 COBJS-$(CONFIG_BFIN_IDE)   += ide-cf.o
 COBJS-$(CONFIG_CMD_EEPROM) += spi_flash.o
-COBJS-$(CONFIG_CMD_NAND)   += nand.o
 COBJS-$(CONFIG_POST)       += post.o post-memory.o
 
 SRCS	:= $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
diff --git a/board/bf537-stamp/nand.c b/board/bf537-stamp/nand.c
deleted file mode 100644
index 181e83d..0000000
--- a/board/bf537-stamp/nand.c
+++ /dev/null
@@ -1,100 +0,0 @@
-/*
- * Copyright (c) 2006-2007 Analog Devices Inc.
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-#include <common.h>
-#include <asm/io.h>
-
-#include <nand.h>
-
-#define CONCAT(a,b,c,d) a ## b ## c ## d
-#define PORT(a,b)  CONCAT(pPORT,a,b,)
-
-#ifndef CONFIG_NAND_GPIO_PORT
-#define CONFIG_NAND_GPIO_PORT F
-#endif
-
-/*
- * hardware specific access to control-lines
- */
-static void bfin_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
-{
-	register struct nand_chip *this = mtd->priv;
-	u32 IO_ADDR_W = (u32) this->IO_ADDR_W;
-
-	if (ctrl & NAND_CTRL_CHANGE) {
-		if (ctrl & NAND_CLE)
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE + BFIN_NAND_CLE;
-		else
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE;
-		if (ctrl & NAND_ALE)
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE + BFIN_NAND_ALE;
-		else
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE;
-		this->IO_ADDR_W = (void __iomem *) IO_ADDR_W;
-	}
-	this->IO_ADDR_R = this->IO_ADDR_W;
-
-	/* Drain the writebuffer */
-	SSYNC();
-
-	if (cmd != NAND_CMD_NONE)
-		writeb(cmd, this->IO_ADDR_W);
-}
-
-int bfin_device_ready(struct mtd_info *mtd)
-{
-	int ret = (*PORT(CONFIG_NAND_GPIO_PORT, IO) & BFIN_NAND_READY) ? 1 : 0;
-	SSYNC();
-	return ret;
-}
-
-/*
- * Board-specific NAND initialization. The following members of the
- * argument are board-specific (per include/linux/mtd/nand.h):
- * - IO_ADDR_R?: address to read the 8 I/O lines of the flash device
- * - IO_ADDR_W?: address to write the 8 I/O lines of the flash device
- * - cmd_ctrl: hardwarespecific function for accesing control-lines
- * - dev_ready: hardwarespecific function for  accesing device ready/busy line
- * - enable_hwecc?: function to enable (reset)  hardware ecc generator. Must
- *   only be provided if a hardware ECC is available
- * - ecc.mode: mode of ecc, see defines
- * - chip_delay: chip dependent delay for transfering data from array to
- *   read regs (tR)
- * - options: various chip options. They can partly be set to inform
- *   nand_scan about special functionality. See the defines for further
- *   explanation
- * Members with a "?" were not set in the merged testing-NAND branch,
- * so they are not set here either.
- */
-int board_nand_init(struct nand_chip *nand)
-{
-	*PORT(CONFIG_NAND_GPIO_PORT, _FER) &= ~BFIN_NAND_READY;
-	*PORT(CONFIG_NAND_GPIO_PORT, IO_DIR) &= ~BFIN_NAND_READY;
-	*PORT(CONFIG_NAND_GPIO_PORT, IO_INEN) |= BFIN_NAND_READY;
-
-	nand->cmd_ctrl = bfin_hwcontrol;
-	nand->ecc.mode = NAND_ECC_SOFT;
-	nand->dev_ready = bfin_device_ready;
-	nand->chip_delay = 30;
-
-	return 0;
-}
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 471cd6b..3f87b4d 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -44,6 +44,7 @@ COBJS-$(CONFIG_NAND_NOMADIK) += nomadik.o
 COBJS-$(CONFIG_NAND_S3C2410) += s3c2410_nand.c
 COBJS-$(CONFIG_NAND_S3C64XX) += s3c64xx.o
 COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
+COBJS-$(CONFIG_NAND_PLAT) += nand_plat.o
 endif
 
 COBJS	:= $(COBJS-y)
diff --git a/drivers/mtd/nand/nand_plat.c b/drivers/mtd/nand/nand_plat.c
new file mode 100644
index 0000000..9a0e4c0
--- /dev/null
+++ b/drivers/mtd/nand/nand_plat.c
@@ -0,0 +1,53 @@
+/*
+ * Genericish driver for memory mapped NAND devices
+ *
+ * Copyright (c) 2006-2009 Analog Devices Inc.
+ * Licensed under the GPL-2 or later.
+ */
+
+/* Your board must implement the following macros:
+ *  NAND_PLAT_WRITE_CMD(cmd, chip)
+ *  NAND_PLAT_WRITE_ADR(cmd, chip)
+ *  NAND_PLAT_INIT()
+ *
+ * It may also implement the following:
+ *  NAND_PLAT_DEV_READY(chip)
+ */
+
+#include <common.h>
+#include <asm/io.h>
+
+#include <nand.h>
+
+static void cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
+{
+	struct nand_chip *this = mtd->priv;
+
+	if (cmd == NAND_CMD_NONE)
+		return;
+
+	if (ctrl & NAND_CLE)
+		NAND_PLAT_WRITE_CMD(cmd, this);
+	else
+		NAND_PLAT_WRITE_ADR(cmd, this);
+}
+
+#ifdef NAND_PLAT_DEV_READY
+static int dev_ready(struct mtd_info *mtd)
+{
+	return NAND_PLAT_DEV_READY((struct nand_chip *)mtd->priv);
+}
+#else
+# define dev_ready NULL
+#endif
+
+int board_nand_init(struct nand_chip *nand)
+{
+	NAND_PLAT_INIT();
+
+	nand->cmd_ctrl = cmd_ctrl;
+	nand->dev_ready = dev_ready;
+	nand->ecc.mode = NAND_ECC_SOFT;
+
+	return 0;
+}
diff --git a/include/configs/bf537-stamp.h b/include/configs/bf537-stamp.h
index 3e5862d..6dfe15f 100644
--- a/include/configs/bf537-stamp.h
+++ b/include/configs/bf537-stamp.h
@@ -136,36 +136,28 @@
 /*
  * NAND Settings
  */
-/* #define CONFIG_BF537_NAND */
-#ifdef CONFIG_BF537_NAND
-# define CONFIG_CMD_NAND
-#endif
-
-#define CONFIG_SYS_NAND_ADDR		0x20212000
-#define CONFIG_SYS_NAND_BASE		CONFIG_SYS_NAND_ADDR
+/* #define CONFIG_NAND_PLAT */
+#define CONFIG_SYS_NAND_BASE		0x20212000
 #define CONFIG_SYS_MAX_NAND_DEVICE	1
-#define SECTORSIZE		512
-#define ADDR_COLUMN		1
-#define ADDR_PAGE		2
-#define ADDR_COLUMN_PAGE	3
-#define NAND_ChipID_UNKNOWN	0x00
-#define NAND_MAX_FLOORS		1
-#define BFIN_NAND_READY		PF3
-
-#define NAND_WAIT_READY(nand) \
+
+#define BFIN_NAND_CLE(chip) ((unsigned long)(chip)->IO_ADDR_W | (1 << 2))
+#define BFIN_NAND_ALE(chip) ((unsigned long)(chip)->IO_ADDR_W | (1 << 1))
+#define BFIN_NAND_READY     PF3
+#define BFIN_NAND_WRITE(addr, cmd) \
 	do { \
-		int timeout = 0; \
-		while (!(*pPORTFIO & PF3)) \
-			if (timeout++ > 100000) \
-				break; \
+		bfin_write8(addr, cmd); \
+		SSYNC(); \
 	} while (0)
 
-#define BFIN_NAND_CLE		(1 << 2)	/* A2 -> Command Enable */
-#define BFIN_NAND_ALE		(1 << 1)	/* A1 -> Address Enable */
-#define WRITE_NAND_COMMAND(d, adr) bfin_write8(adr | BFIN_NAND_CLE, d)
-#define WRITE_NAND_ADDRESS(d, adr) bfin_write8(adr | BFIN_NAND_ALE, d)
-#define WRITE_NAND(d, adr)         bfin_write8(adr, d)
-#define READ_NAND(adr)             bfin_read8(adr)
+#define NAND_PLAT_WRITE_CMD(cmd, chip) BFIN_NAND_WRITE(BFIN_NAND_CLE(chip), cmd)
+#define NAND_PLAT_WRITE_ADR(cmd, chip) BFIN_NAND_WRITE(BFIN_NAND_ALE(chip), cmd)
+#define NAND_PLAT_DEV_READY(chip)      ((*pPORTFIO & BFIN_NAND_READY) ? 1 : 0)
+#define NAND_PLAT_INIT() \
+	do { \
+		*pPORTF_FER &= ~BFIN_NAND_READY; \
+		*pPORTFIO_DIR &= ~BFIN_NAND_READY; \
+		*pPORTFIO_INEN |= BFIN_NAND_READY; \
+	} while (0)
 
 
 /*
diff --git a/include/configs/bfin_adi_common.h b/include/configs/bfin_adi_common.h
index bfe5376..ac0298d 100644
--- a/include/configs/bfin_adi_common.h
+++ b/include/configs/bfin_adi_common.h
@@ -38,6 +38,9 @@
 #  define CONFIG_CMD_USB_STORAGE
 #  define CONFIG_DOS_PARTITION
 # endif
+# ifdef CONFIG_NAND_PLAT
+#  define CONFIG_CMD_NAND
+# endif
 # ifdef CONFIG_POST
 #  define CONFIG_CMD_DIAG
 # endif
-- 
1.6.2.5

[U-Boot] [PATCH v2] mtd: nand: new base driver for memory mapped nand devices

[Scott Wood]

On Wed, May 06, 2009 at 09:05:21AM -0400, Mike Frysinger wrote:
> + *  NAND_PLAT_WRITE_CMD(cmd, chip)
> + *  NAND_PLAT_WRITE_ADR(cmd, chip)

It seems counterintuitive to have "cmd" before "this" -- it's backwards
from both cmd_ctrl and the blackfin command that you turn it into (yes,
it's like writeb() -- but I always found that to be weird too).

> +/* #define CONFIG_NAND_PLAT */

Why is this commented out?

> +#define NAND_PLAT_DEV_READY(chip)      ((*pPORTFIO & BFIN_NAND_READY) ? 1 : 0)

Why not just (*pPORTFIO & BFIN_NAND_READY)?

-Scott

[U-Boot] [PATCH v2] mtd: nand: new base driver for memory mapped nand devices

[Mike Frysinger]

On Wednesday 06 May 2009 13:35:29 Scott Wood wrote:
> On Wed, May 06, 2009 at 09:05:21AM -0400, Mike Frysinger wrote:
> > + *  NAND_PLAT_WRITE_CMD(cmd, chip)
> > + *  NAND_PLAT_WRITE_ADR(cmd, chip)
>
> It seems counterintuitive to have "cmd" before "this" -- it's backwards
> from both cmd_ctrl and the blackfin command that you turn it into (yes,
> it's like writeb() -- but I always found that to be weird too).

np

> > +/* #define CONFIG_NAND_PLAT */
>
> Why is this commented out?

because it's a driver for an optional add-on card that people usually dont 
have, let alone plugged in

> > +#define NAND_PLAT_DEV_READY(chip)      ((*pPORTFIO & BFIN_NAND_READY) ?
> > 1 : 0)
>
> Why not just (*pPORTFIO & BFIN_NAND_READY)?

i thought the nand/mtd layers expect 1/0 only ?  if the higher layers dont 
care, then there's no reason.
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090506/5352b9e1/attachment.pgp 

[U-Boot] [PATCH v2] mtd: nand: new base driver for memory mapped nand devices

[Scott Wood]

Mike Frysinger wrote:
>>> +/* #define CONFIG_NAND_PLAT */
>> Why is this commented out?
> 
> because it's a driver for an optional add-on card that people usually dont 
> have, let alone plugged in

OK, was hoping there would be at least one config that selects it so it 
gets compilation exposure.

>>> +#define NAND_PLAT_DEV_READY(chip)      ((*pPORTFIO & BFIN_NAND_READY) ?
>>> 1 : 0)
>> Why not just (*pPORTFIO & BFIN_NAND_READY)?
> 
> i thought the nand/mtd layers expect 1/0 only ?  if the higher layers dont 
> care, then there's no reason.

A quick grep doesn't show any non-boolean uses.

-Scott

[U-Boot] [PATCH v2] mtd: nand: new base driver for memory mapped nand devices

[Mike Frysinger]

On Wednesday 06 May 2009 14:19:07 Scott Wood wrote:
> Mike Frysinger wrote:
> >>> +/* #define CONFIG_NAND_PLAT */
> >>
> >> Why is this commented out?
> >
> > because it's a driver for an optional add-on card that people usually
> > dont have, let alone plugged in
>
> OK, was hoping there would be at least one config that selects it so it
> gets compilation exposure.

i have another board that does rely on it that i'll be merging in the next 
cycle

> >>> +#define NAND_PLAT_DEV_READY(chip)      ((*pPORTFIO & BFIN_NAND_READY)
> >>> ? 1 : 0)
> >>
> >> Why not just (*pPORTFIO & BFIN_NAND_READY)?
> >
> > i thought the nand/mtd layers expect 1/0 only ?  if the higher layers
> > dont care, then there's no reason.
>
> A quick grep doesn't show any non-boolean uses.

sounds good, i'll make the change then
-mike

[U-Boot] [PATCH v3] mtd: nand: new base driver for memory mapped nand devices

[Mike Frysinger]

The BF537-STAMP Blackfin board had a driver for working with NAND devices
that are simply memory mapped.  Since there is nothing Blackfin specific
about this, generalize the driver a bit so that everyone can leverage it.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
CC: Scott Wood <scottwood@freescale.com>
---
v3
	- re-order args to NAND_PLAT_WRITE_*
	- dont bother normalizing NAND_PLAT_DEV_READY

 board/bf537-stamp/Makefile        |    1 -
 board/bf537-stamp/nand.c          |  100 -------------------------------------
 drivers/mtd/nand/Makefile         |    1 +
 drivers/mtd/nand/nand_plat.c      |   53 +++++++++++++++++++
 include/configs/bf537-stamp.h     |   44 +++++++----------
 include/configs/bfin_adi_common.h |    3 +
 6 files changed, 75 insertions(+), 127 deletions(-)
 delete mode 100644 board/bf537-stamp/nand.c
 create mode 100644 drivers/mtd/nand/nand_plat.c

diff --git a/board/bf537-stamp/Makefile b/board/bf537-stamp/Makefile
index 1dbf406..6da04e3 100644
--- a/board/bf537-stamp/Makefile
+++ b/board/bf537-stamp/Makefile
@@ -32,7 +32,6 @@ LIB	= $(obj)lib$(BOARD).a
 COBJS-y	:= $(BOARD).o cmd_bf537led.o
 COBJS-$(CONFIG_BFIN_IDE)   += ide-cf.o
 COBJS-$(CONFIG_CMD_EEPROM) += spi_flash.o
-COBJS-$(CONFIG_CMD_NAND)   += nand.o
 COBJS-$(CONFIG_POST)       += post.o post-memory.o
 
 SRCS	:= $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
diff --git a/board/bf537-stamp/nand.c b/board/bf537-stamp/nand.c
deleted file mode 100644
index 181e83d..0000000
--- a/board/bf537-stamp/nand.c
+++ /dev/null
@@ -1,100 +0,0 @@
-/*
- * Copyright (c) 2006-2007 Analog Devices Inc.
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-#include <common.h>
-#include <asm/io.h>
-
-#include <nand.h>
-
-#define CONCAT(a,b,c,d) a ## b ## c ## d
-#define PORT(a,b)  CONCAT(pPORT,a,b,)
-
-#ifndef CONFIG_NAND_GPIO_PORT
-#define CONFIG_NAND_GPIO_PORT F
-#endif
-
-/*
- * hardware specific access to control-lines
- */
-static void bfin_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
-{
-	register struct nand_chip *this = mtd->priv;
-	u32 IO_ADDR_W = (u32) this->IO_ADDR_W;
-
-	if (ctrl & NAND_CTRL_CHANGE) {
-		if (ctrl & NAND_CLE)
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE + BFIN_NAND_CLE;
-		else
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE;
-		if (ctrl & NAND_ALE)
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE + BFIN_NAND_ALE;
-		else
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE;
-		this->IO_ADDR_W = (void __iomem *) IO_ADDR_W;
-	}
-	this->IO_ADDR_R = this->IO_ADDR_W;
-
-	/* Drain the writebuffer */
-	SSYNC();
-
-	if (cmd != NAND_CMD_NONE)
-		writeb(cmd, this->IO_ADDR_W);
-}
-
-int bfin_device_ready(struct mtd_info *mtd)
-{
-	int ret = (*PORT(CONFIG_NAND_GPIO_PORT, IO) & BFIN_NAND_READY) ? 1 : 0;
-	SSYNC();
-	return ret;
-}
-
-/*
- * Board-specific NAND initialization. The following members of the
- * argument are board-specific (per include/linux/mtd/nand.h):
- * - IO_ADDR_R?: address to read the 8 I/O lines of the flash device
- * - IO_ADDR_W?: address to write the 8 I/O lines of the flash device
- * - cmd_ctrl: hardwarespecific function for accesing control-lines
- * - dev_ready: hardwarespecific function for  accesing device ready/busy line
- * - enable_hwecc?: function to enable (reset)  hardware ecc generator. Must
- *   only be provided if a hardware ECC is available
- * - ecc.mode: mode of ecc, see defines
- * - chip_delay: chip dependent delay for transfering data from array to
- *   read regs (tR)
- * - options: various chip options. They can partly be set to inform
- *   nand_scan about special functionality. See the defines for further
- *   explanation
- * Members with a "?" were not set in the merged testing-NAND branch,
- * so they are not set here either.
- */
-int board_nand_init(struct nand_chip *nand)
-{
-	*PORT(CONFIG_NAND_GPIO_PORT, _FER) &= ~BFIN_NAND_READY;
-	*PORT(CONFIG_NAND_GPIO_PORT, IO_DIR) &= ~BFIN_NAND_READY;
-	*PORT(CONFIG_NAND_GPIO_PORT, IO_INEN) |= BFIN_NAND_READY;
-
-	nand->cmd_ctrl = bfin_hwcontrol;
-	nand->ecc.mode = NAND_ECC_SOFT;
-	nand->dev_ready = bfin_device_ready;
-	nand->chip_delay = 30;
-
-	return 0;
-}
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 471cd6b..3f87b4d 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -44,6 +44,7 @@ COBJS-$(CONFIG_NAND_NOMADIK) += nomadik.o
 COBJS-$(CONFIG_NAND_S3C2410) += s3c2410_nand.c
 COBJS-$(CONFIG_NAND_S3C64XX) += s3c64xx.o
 COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
+COBJS-$(CONFIG_NAND_PLAT) += nand_plat.o
 endif
 
 COBJS	:= $(COBJS-y)
diff --git a/drivers/mtd/nand/nand_plat.c b/drivers/mtd/nand/nand_plat.c
new file mode 100644
index 0000000..a2f33a4
--- /dev/null
+++ b/drivers/mtd/nand/nand_plat.c
@@ -0,0 +1,53 @@
+/*
+ * Genericish driver for memory mapped NAND devices
+ *
+ * Copyright (c) 2006-2009 Analog Devices Inc.
+ * Licensed under the GPL-2 or later.
+ */
+
+/* Your board must implement the following macros:
+ *  NAND_PLAT_WRITE_CMD(chip, cmd)
+ *  NAND_PLAT_WRITE_ADR(chip, cmd)
+ *  NAND_PLAT_INIT()
+ *
+ * It may also implement the following:
+ *  NAND_PLAT_DEV_READY(chip)
+ */
+
+#include <common.h>
+#include <asm/io.h>
+
+#include <nand.h>
+
+static void cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
+{
+	struct nand_chip *this = mtd->priv;
+
+	if (cmd == NAND_CMD_NONE)
+		return;
+
+	if (ctrl & NAND_CLE)
+		NAND_PLAT_WRITE_CMD(this, cmd);
+	else
+		NAND_PLAT_WRITE_ADR(this, cmd);
+}
+
+#ifdef NAND_PLAT_DEV_READY
+static int dev_ready(struct mtd_info *mtd)
+{
+	return NAND_PLAT_DEV_READY((struct nand_chip *)mtd->priv);
+}
+#else
+# define dev_ready NULL
+#endif
+
+int board_nand_init(struct nand_chip *nand)
+{
+	NAND_PLAT_INIT();
+
+	nand->cmd_ctrl = cmd_ctrl;
+	nand->dev_ready = dev_ready;
+	nand->ecc.mode = NAND_ECC_SOFT;
+
+	return 0;
+}
diff --git a/include/configs/bf537-stamp.h b/include/configs/bf537-stamp.h
index 3e5862d..baf977b 100644
--- a/include/configs/bf537-stamp.h
+++ b/include/configs/bf537-stamp.h
@@ -136,36 +136,28 @@
 /*
  * NAND Settings
  */
-/* #define CONFIG_BF537_NAND */
-#ifdef CONFIG_BF537_NAND
-# define CONFIG_CMD_NAND
-#endif
-
-#define CONFIG_SYS_NAND_ADDR		0x20212000
-#define CONFIG_SYS_NAND_BASE		CONFIG_SYS_NAND_ADDR
+/* #define CONFIG_NAND_PLAT */
+#define CONFIG_SYS_NAND_BASE		0x20212000
 #define CONFIG_SYS_MAX_NAND_DEVICE	1
-#define SECTORSIZE		512
-#define ADDR_COLUMN		1
-#define ADDR_PAGE		2
-#define ADDR_COLUMN_PAGE	3
-#define NAND_ChipID_UNKNOWN	0x00
-#define NAND_MAX_FLOORS		1
-#define BFIN_NAND_READY		PF3
-
-#define NAND_WAIT_READY(nand) \
+
+#define BFIN_NAND_CLE(chip) ((unsigned long)(chip)->IO_ADDR_W | (1 << 2))
+#define BFIN_NAND_ALE(chip) ((unsigned long)(chip)->IO_ADDR_W | (1 << 1))
+#define BFIN_NAND_READY     PF3
+#define BFIN_NAND_WRITE(addr, cmd) \
 	do { \
-		int timeout = 0; \
-		while (!(*pPORTFIO & PF3)) \
-			if (timeout++ > 100000) \
-				break; \
+		bfin_write8(addr, cmd); \
+		SSYNC(); \
 	} while (0)
 
-#define BFIN_NAND_CLE		(1 << 2)	/* A2 -> Command Enable */
-#define BFIN_NAND_ALE		(1 << 1)	/* A1 -> Address Enable */
-#define WRITE_NAND_COMMAND(d, adr) bfin_write8(adr | BFIN_NAND_CLE, d)
-#define WRITE_NAND_ADDRESS(d, adr) bfin_write8(adr | BFIN_NAND_ALE, d)
-#define WRITE_NAND(d, adr)         bfin_write8(adr, d)
-#define READ_NAND(adr)             bfin_read8(adr)
+#define NAND_PLAT_WRITE_CMD(chip, cmd) BFIN_NAND_WRITE(BFIN_NAND_CLE(chip), cmd)
+#define NAND_PLAT_WRITE_ADR(chip, cmd) BFIN_NAND_WRITE(BFIN_NAND_ALE(chip), cmd)
+#define NAND_PLAT_DEV_READY(chip)      (*pPORTFIO & BFIN_NAND_READY)
+#define NAND_PLAT_INIT() \
+	do { \
+		*pPORTF_FER &= ~BFIN_NAND_READY; \
+		*pPORTFIO_DIR &= ~BFIN_NAND_READY; \
+		*pPORTFIO_INEN |= BFIN_NAND_READY; \
+	} while (0)
 
 
 /*
diff --git a/include/configs/bfin_adi_common.h b/include/configs/bfin_adi_common.h
index bfe5376..ac0298d 100644
--- a/include/configs/bfin_adi_common.h
+++ b/include/configs/bfin_adi_common.h
@@ -38,6 +38,9 @@
 #  define CONFIG_CMD_USB_STORAGE
 #  define CONFIG_DOS_PARTITION
 # endif
+# ifdef CONFIG_NAND_PLAT
+#  define CONFIG_CMD_NAND
+# endif
 # ifdef CONFIG_POST
 #  define CONFIG_CMD_DIAG
 # endif
-- 
1.6.2.5

[U-Boot] [PATCH v3] mtd: nand: new base driver for memory mapped nand devices

[Scott Wood]

On Wed, May 06, 2009 at 03:38:36PM -0400, Mike Frysinger wrote:
> The BF537-STAMP Blackfin board had a driver for working with NAND devices
> that are simply memory mapped.  Since there is nothing Blackfin specific
> about this, generalize the driver a bit so that everyone can leverage it.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> CC: Scott Wood <scottwood@freescale.com>

Applied to u-boot-nand-flash/next.

-Scott

[U-Boot] [PATCH v3] mtd: nand: new base driver for memory mapped nand devices

[Mike Frysinger]

On Wednesday 06 May 2009 15:49:12 Scott Wood wrote:
> On Wed, May 06, 2009 at 03:38:36PM -0400, Mike Frysinger wrote:
> > The BF537-STAMP Blackfin board had a driver for working with NAND devices
> > that are simply memory mapped.  Since there is nothing Blackfin specific
> > about this, generalize the driver a bit so that everyone can leverage it.
> >
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > CC: Scott Wood <scottwood@freescale.com>
>
> Applied to u-boot-nand-flash/next.

thanks !
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090506/0abc71ae/attachment.pgp 

[U-Boot] [PATCH v2] mtd: nand: new base driver for memory mapped nand devices

[Wolfgang Denk]

Dear Mike Frysinger,

In message <1241615121-15945-1-git-send-email-vapier@gentoo.org> you wrote:
> The BF537-STAMP Blackfin board had a driver for working with NAND devices
> that are simply memory mapped.  Since there is nothing Blackfin specific
> about this, generalize the driver a bit so that everyone can leverage it.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
...

> diff --git a/drivers/mtd/nand/nand_plat.c b/drivers/mtd/nand/nand_plat.c
> new file mode 100644
> index 0000000..9a0e4c0
> --- /dev/null
> +++ b/drivers/mtd/nand/nand_plat.c
> @@ -0,0 +1,53 @@
> +/*
> + * Genericish driver for memory mapped NAND devices

Genericish ?

...
> +#define NAND_PLAT_WRITE_CMD(cmd, chip) BFIN_NAND_WRITE(BFIN_NAND_CLE(chip), cmd)
> +#define NAND_PLAT_WRITE_ADR(cmd, chip) BFIN_NAND_WRITE(BFIN_NAND_ALE(chip), cmd)
> +#define NAND_PLAT_DEV_READY(chip)      ((*pPORTFIO & BFIN_NAND_READY) ? 1 : 0)
> +#define NAND_PLAT_INIT() \
> +	do { \
> +		*pPORTF_FER &= ~BFIN_NAND_READY; \
> +		*pPORTFIO_DIR &= ~BFIN_NAND_READY; \
> +		*pPORTFIO_INEN |= BFIN_NAND_READY; \
> +	} while (0)

Please use I/O accessors instead of pointers.


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
All repairs tend to destroy the structure, to  increase  the  entropy
and  disorder  of the system. Less and less effort is spent on fixing
original design flaws; more and more is spent on fixing flaws  intro-
duced by earlier fixes.       - Fred Brooks, "The Mythical Man Month"

[U-Boot] [PATCH v3] mtd: nand: new base driver for memory mapped nand devices

[Wolfgang Denk]

Dear Mike Frysinger,

In message <1241638716-29824-1-git-send-email-vapier@gentoo.org> you wrote:
> The BF537-STAMP Blackfin board had a driver for working with NAND devices
> that are simply memory mapped.  Since there is nothing Blackfin specific
> about this, generalize the driver a bit so that everyone can leverage it.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> CC: Scott Wood <scottwood@freescale.com>
> ---
> v3
> 	- re-order args to NAND_PLAT_WRITE_*
> 	- dont bother normalizing NAND_PLAT_DEV_READY

Please see my comments for v2; they still apply.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
The philosophy exam was a piece of cake  -  which  was  a  bit  of  a
surprise, actually, because I was expecting some questions on a sheet
of paper.

[U-Boot] [PATCH v2] mtd: nand: new base driver for memory mapped nand devices

[Mike Frysinger]

On Wednesday 06 May 2009 16:51:18 Wolfgang Denk wrote:
> In Mike Frysinger wrote:
> > --- /dev/null
> > +++ b/drivers/mtd/nand/nand_plat.c
> > @@ -0,0 +1,53 @@
> > +/*
> > + * Genericish driver for memory mapped NAND devices
>
> Genericish ?

i tried to make it generic.  i dont know if i accomplished that :).

> > +#define NAND_PLAT_WRITE_CMD(cmd, chip)
> > BFIN_NAND_WRITE(BFIN_NAND_CLE(chip), cmd) +#define
> > NAND_PLAT_WRITE_ADR(cmd, chip) BFIN_NAND_WRITE(BFIN_NAND_ALE(chip), cmd)
> > +#define NAND_PLAT_DEV_READY(chip)      ((*pPORTFIO & BFIN_NAND_READY) ?
> > 1 : 0) +#define NAND_PLAT_INIT() \
> > +	do { \
> > +		*pPORTF_FER &= ~BFIN_NAND_READY; \
> > +		*pPORTFIO_DIR &= ~BFIN_NAND_READY; \
> > +		*pPORTFIO_INEN |= BFIN_NAND_READY; \
> > +	} while (0)
>
> Please use I/O accessors instead of pointers.

OK
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090506/af9bcacd/attachment.pgp 

[U-Boot] [PATCH v4] mtd: nand: new base driver for memory mapped nand devices

[Mike Frysinger]

The BF537-STAMP Blackfin board had a driver for working with NAND devices
that are simply memory mapped.  Since there is nothing Blackfin specific
about this, generalize the driver a bit so that everyone can leverage it.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
v4
	- use I/O accessors

 board/bf537-stamp/Makefile        |    1 -
 board/bf537-stamp/nand.c          |  100 -------------------------------------
 drivers/mtd/nand/Makefile         |    1 +
 drivers/mtd/nand/nand_plat.c      |   53 +++++++++++++++++++
 include/configs/bf537-stamp.h     |   44 +++++++----------
 include/configs/bfin_adi_common.h |    3 +
 6 files changed, 75 insertions(+), 127 deletions(-)
 delete mode 100644 board/bf537-stamp/nand.c
 create mode 100644 drivers/mtd/nand/nand_plat.c

diff --git a/board/bf537-stamp/Makefile b/board/bf537-stamp/Makefile
index 1dbf406..6da04e3 100644
--- a/board/bf537-stamp/Makefile
+++ b/board/bf537-stamp/Makefile
@@ -32,7 +32,6 @@ LIB	= $(obj)lib$(BOARD).a
 COBJS-y	:= $(BOARD).o cmd_bf537led.o
 COBJS-$(CONFIG_BFIN_IDE)   += ide-cf.o
 COBJS-$(CONFIG_CMD_EEPROM) += spi_flash.o
-COBJS-$(CONFIG_CMD_NAND)   += nand.o
 COBJS-$(CONFIG_POST)       += post.o post-memory.o
 
 SRCS	:= $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
diff --git a/board/bf537-stamp/nand.c b/board/bf537-stamp/nand.c
deleted file mode 100644
index 181e83d..0000000
--- a/board/bf537-stamp/nand.c
+++ /dev/null
@@ -1,100 +0,0 @@
-/*
- * Copyright (c) 2006-2007 Analog Devices Inc.
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-#include <common.h>
-#include <asm/io.h>
-
-#include <nand.h>
-
-#define CONCAT(a,b,c,d) a ## b ## c ## d
-#define PORT(a,b)  CONCAT(pPORT,a,b,)
-
-#ifndef CONFIG_NAND_GPIO_PORT
-#define CONFIG_NAND_GPIO_PORT F
-#endif
-
-/*
- * hardware specific access to control-lines
- */
-static void bfin_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
-{
-	register struct nand_chip *this = mtd->priv;
-	u32 IO_ADDR_W = (u32) this->IO_ADDR_W;
-
-	if (ctrl & NAND_CTRL_CHANGE) {
-		if (ctrl & NAND_CLE)
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE + BFIN_NAND_CLE;
-		else
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE;
-		if (ctrl & NAND_ALE)
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE + BFIN_NAND_ALE;
-		else
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE;
-		this->IO_ADDR_W = (void __iomem *) IO_ADDR_W;
-	}
-	this->IO_ADDR_R = this->IO_ADDR_W;
-
-	/* Drain the writebuffer */
-	SSYNC();
-
-	if (cmd != NAND_CMD_NONE)
-		writeb(cmd, this->IO_ADDR_W);
-}
-
-int bfin_device_ready(struct mtd_info *mtd)
-{
-	int ret = (*PORT(CONFIG_NAND_GPIO_PORT, IO) & BFIN_NAND_READY) ? 1 : 0;
-	SSYNC();
-	return ret;
-}
-
-/*
- * Board-specific NAND initialization. The following members of the
- * argument are board-specific (per include/linux/mtd/nand.h):
- * - IO_ADDR_R?: address to read the 8 I/O lines of the flash device
- * - IO_ADDR_W?: address to write the 8 I/O lines of the flash device
- * - cmd_ctrl: hardwarespecific function for accesing control-lines
- * - dev_ready: hardwarespecific function for  accesing device ready/busy line
- * - enable_hwecc?: function to enable (reset)  hardware ecc generator. Must
- *   only be provided if a hardware ECC is available
- * - ecc.mode: mode of ecc, see defines
- * - chip_delay: chip dependent delay for transfering data from array to
- *   read regs (tR)
- * - options: various chip options. They can partly be set to inform
- *   nand_scan about special functionality. See the defines for further
- *   explanation
- * Members with a "?" were not set in the merged testing-NAND branch,
- * so they are not set here either.
- */
-int board_nand_init(struct nand_chip *nand)
-{
-	*PORT(CONFIG_NAND_GPIO_PORT, _FER) &= ~BFIN_NAND_READY;
-	*PORT(CONFIG_NAND_GPIO_PORT, IO_DIR) &= ~BFIN_NAND_READY;
-	*PORT(CONFIG_NAND_GPIO_PORT, IO_INEN) |= BFIN_NAND_READY;
-
-	nand->cmd_ctrl = bfin_hwcontrol;
-	nand->ecc.mode = NAND_ECC_SOFT;
-	nand->dev_ready = bfin_device_ready;
-	nand->chip_delay = 30;
-
-	return 0;
-}
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 471cd6b..3f87b4d 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -44,6 +44,7 @@ COBJS-$(CONFIG_NAND_NOMADIK) += nomadik.o
 COBJS-$(CONFIG_NAND_S3C2410) += s3c2410_nand.c
 COBJS-$(CONFIG_NAND_S3C64XX) += s3c64xx.o
 COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
+COBJS-$(CONFIG_NAND_PLAT) += nand_plat.o
 endif
 
 COBJS	:= $(COBJS-y)
diff --git a/drivers/mtd/nand/nand_plat.c b/drivers/mtd/nand/nand_plat.c
new file mode 100644
index 0000000..a2f33a4
--- /dev/null
+++ b/drivers/mtd/nand/nand_plat.c
@@ -0,0 +1,53 @@
+/*
+ * Genericish driver for memory mapped NAND devices
+ *
+ * Copyright (c) 2006-2009 Analog Devices Inc.
+ * Licensed under the GPL-2 or later.
+ */
+
+/* Your board must implement the following macros:
+ *  NAND_PLAT_WRITE_CMD(chip, cmd)
+ *  NAND_PLAT_WRITE_ADR(chip, cmd)
+ *  NAND_PLAT_INIT()
+ *
+ * It may also implement the following:
+ *  NAND_PLAT_DEV_READY(chip)
+ */
+
+#include <common.h>
+#include <asm/io.h>
+
+#include <nand.h>
+
+static void cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
+{
+	struct nand_chip *this = mtd->priv;
+
+	if (cmd == NAND_CMD_NONE)
+		return;
+
+	if (ctrl & NAND_CLE)
+		NAND_PLAT_WRITE_CMD(this, cmd);
+	else
+		NAND_PLAT_WRITE_ADR(this, cmd);
+}
+
+#ifdef NAND_PLAT_DEV_READY
+static int dev_ready(struct mtd_info *mtd)
+{
+	return NAND_PLAT_DEV_READY((struct nand_chip *)mtd->priv);
+}
+#else
+# define dev_ready NULL
+#endif
+
+int board_nand_init(struct nand_chip *nand)
+{
+	NAND_PLAT_INIT();
+
+	nand->cmd_ctrl = cmd_ctrl;
+	nand->dev_ready = dev_ready;
+	nand->ecc.mode = NAND_ECC_SOFT;
+
+	return 0;
+}
diff --git a/include/configs/bf537-stamp.h b/include/configs/bf537-stamp.h
index 3e5862d..2c1e06d 100644
--- a/include/configs/bf537-stamp.h
+++ b/include/configs/bf537-stamp.h
@@ -136,36 +136,28 @@
 /*
  * NAND Settings
  */
-/* #define CONFIG_BF537_NAND */
-#ifdef CONFIG_BF537_NAND
-# define CONFIG_CMD_NAND
-#endif
-
-#define CONFIG_SYS_NAND_ADDR		0x20212000
-#define CONFIG_SYS_NAND_BASE		CONFIG_SYS_NAND_ADDR
+/* #define CONFIG_NAND_PLAT */
+#define CONFIG_SYS_NAND_BASE		0x20212000
 #define CONFIG_SYS_MAX_NAND_DEVICE	1
-#define SECTORSIZE		512
-#define ADDR_COLUMN		1
-#define ADDR_PAGE		2
-#define ADDR_COLUMN_PAGE	3
-#define NAND_ChipID_UNKNOWN	0x00
-#define NAND_MAX_FLOORS		1
-#define BFIN_NAND_READY		PF3
-
-#define NAND_WAIT_READY(nand) \
+
+#define BFIN_NAND_CLE(chip) ((unsigned long)(chip)->IO_ADDR_W | (1 << 2))
+#define BFIN_NAND_ALE(chip) ((unsigned long)(chip)->IO_ADDR_W | (1 << 1))
+#define BFIN_NAND_READY     PF3
+#define BFIN_NAND_WRITE(addr, cmd) \
 	do { \
-		int timeout = 0; \
-		while (!(*pPORTFIO & PF3)) \
-			if (timeout++ > 100000) \
-				break; \
+		bfin_write8(addr, cmd); \
+		SSYNC(); \
 	} while (0)
 
-#define BFIN_NAND_CLE		(1 << 2)	/* A2 -> Command Enable */
-#define BFIN_NAND_ALE		(1 << 1)	/* A1 -> Address Enable */
-#define WRITE_NAND_COMMAND(d, adr) bfin_write8(adr | BFIN_NAND_CLE, d)
-#define WRITE_NAND_ADDRESS(d, adr) bfin_write8(adr | BFIN_NAND_ALE, d)
-#define WRITE_NAND(d, adr)         bfin_write8(adr, d)
-#define READ_NAND(adr)             bfin_read8(adr)
+#define NAND_PLAT_WRITE_CMD(chip, cmd) BFIN_NAND_WRITE(BFIN_NAND_CLE(chip), cmd)
+#define NAND_PLAT_WRITE_ADR(chip, cmd) BFIN_NAND_WRITE(BFIN_NAND_ALE(chip), cmd)
+#define NAND_PLAT_DEV_READY(chip)      (*pPORTFIO & BFIN_NAND_READY)
+#define NAND_PLAT_INIT() \
+	do { \
+		bfin_write_PORTF_FER(bfin_read_PORTF_FER() & ~BFIN_NAND_READY); \
+		bfin_write_PORTFIO_DIR(bfin_read_PORTFIO_DIR() & ~BFIN_NAND_READY); \
+		bfin_write_PORTFIO_INEN(bfin_read_PORTFIO_INEN() | BFIN_NAND_READY); \
+	} while (0)
 
 
 /*
diff --git a/include/configs/bfin_adi_common.h b/include/configs/bfin_adi_common.h
index bfe5376..ac0298d 100644
--- a/include/configs/bfin_adi_common.h
+++ b/include/configs/bfin_adi_common.h
@@ -38,6 +38,9 @@
 #  define CONFIG_CMD_USB_STORAGE
 #  define CONFIG_DOS_PARTITION
 # endif
+# ifdef CONFIG_NAND_PLAT
+#  define CONFIG_CMD_NAND
+# endif
 # ifdef CONFIG_POST
 #  define CONFIG_CMD_DIAG
 # endif
-- 
1.6.2.5

[U-Boot] [PATCH v4] mtd: nand: new base driver for memory mapped nand devices

[Scott Wood]

On Wed, May 06, 2009 at 08:28:39PM -0400, Mike Frysinger wrote:
> The BF537-STAMP Blackfin board had a driver for working with NAND devices
> that are simply memory mapped.  Since there is nothing Blackfin specific
> about this, generalize the driver a bit so that everyone can leverage it.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
> v4
> 	- use I/O accessors
[snip]
> +#define NAND_PLAT_DEV_READY(chip)      (*pPORTFIO & BFIN_NAND_READY)

This is still using direct pointer access.

-Scott

[U-Boot] [PATCH v5] mtd: nand: new base driver for memory mapped nand devices

[Mike Frysinger]

The BF537-STAMP Blackfin board had a driver for working with NAND devices
that are simply memory mapped.  Since there is nothing Blackfin specific
about this, generalize the driver a bit so that everyone can leverage it.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
v5
	- fix one more I/O usage

 board/bf537-stamp/Makefile        |    1 -
 board/bf537-stamp/nand.c          |  100 -------------------------------------
 drivers/mtd/nand/Makefile         |    1 +
 drivers/mtd/nand/nand_plat.c      |   53 +++++++++++++++++++
 include/configs/bf537-stamp.h     |   44 +++++++----------
 include/configs/bfin_adi_common.h |    3 +
 6 files changed, 75 insertions(+), 127 deletions(-)
 delete mode 100644 board/bf537-stamp/nand.c
 create mode 100644 drivers/mtd/nand/nand_plat.c

diff --git a/board/bf537-stamp/Makefile b/board/bf537-stamp/Makefile
index 1dbf406..6da04e3 100644
--- a/board/bf537-stamp/Makefile
+++ b/board/bf537-stamp/Makefile
@@ -32,7 +32,6 @@ LIB	= $(obj)lib$(BOARD).a
 COBJS-y	:= $(BOARD).o cmd_bf537led.o
 COBJS-$(CONFIG_BFIN_IDE)   += ide-cf.o
 COBJS-$(CONFIG_CMD_EEPROM) += spi_flash.o
-COBJS-$(CONFIG_CMD_NAND)   += nand.o
 COBJS-$(CONFIG_POST)       += post.o post-memory.o
 
 SRCS	:= $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
diff --git a/board/bf537-stamp/nand.c b/board/bf537-stamp/nand.c
deleted file mode 100644
index 181e83d..0000000
--- a/board/bf537-stamp/nand.c
+++ /dev/null
@@ -1,100 +0,0 @@
-/*
- * Copyright (c) 2006-2007 Analog Devices Inc.
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-#include <common.h>
-#include <asm/io.h>
-
-#include <nand.h>
-
-#define CONCAT(a,b,c,d) a ## b ## c ## d
-#define PORT(a,b)  CONCAT(pPORT,a,b,)
-
-#ifndef CONFIG_NAND_GPIO_PORT
-#define CONFIG_NAND_GPIO_PORT F
-#endif
-
-/*
- * hardware specific access to control-lines
- */
-static void bfin_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
-{
-	register struct nand_chip *this = mtd->priv;
-	u32 IO_ADDR_W = (u32) this->IO_ADDR_W;
-
-	if (ctrl & NAND_CTRL_CHANGE) {
-		if (ctrl & NAND_CLE)
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE + BFIN_NAND_CLE;
-		else
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE;
-		if (ctrl & NAND_ALE)
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE + BFIN_NAND_ALE;
-		else
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE;
-		this->IO_ADDR_W = (void __iomem *) IO_ADDR_W;
-	}
-	this->IO_ADDR_R = this->IO_ADDR_W;
-
-	/* Drain the writebuffer */
-	SSYNC();
-
-	if (cmd != NAND_CMD_NONE)
-		writeb(cmd, this->IO_ADDR_W);
-}
-
-int bfin_device_ready(struct mtd_info *mtd)
-{
-	int ret = (*PORT(CONFIG_NAND_GPIO_PORT, IO) & BFIN_NAND_READY) ? 1 : 0;
-	SSYNC();
-	return ret;
-}
-
-/*
- * Board-specific NAND initialization. The following members of the
- * argument are board-specific (per include/linux/mtd/nand.h):
- * - IO_ADDR_R?: address to read the 8 I/O lines of the flash device
- * - IO_ADDR_W?: address to write the 8 I/O lines of the flash device
- * - cmd_ctrl: hardwarespecific function for accesing control-lines
- * - dev_ready: hardwarespecific function for  accesing device ready/busy line
- * - enable_hwecc?: function to enable (reset)  hardware ecc generator. Must
- *   only be provided if a hardware ECC is available
- * - ecc.mode: mode of ecc, see defines
- * - chip_delay: chip dependent delay for transfering data from array to
- *   read regs (tR)
- * - options: various chip options. They can partly be set to inform
- *   nand_scan about special functionality. See the defines for further
- *   explanation
- * Members with a "?" were not set in the merged testing-NAND branch,
- * so they are not set here either.
- */
-int board_nand_init(struct nand_chip *nand)
-{
-	*PORT(CONFIG_NAND_GPIO_PORT, _FER) &= ~BFIN_NAND_READY;
-	*PORT(CONFIG_NAND_GPIO_PORT, IO_DIR) &= ~BFIN_NAND_READY;
-	*PORT(CONFIG_NAND_GPIO_PORT, IO_INEN) |= BFIN_NAND_READY;
-
-	nand->cmd_ctrl = bfin_hwcontrol;
-	nand->ecc.mode = NAND_ECC_SOFT;
-	nand->dev_ready = bfin_device_ready;
-	nand->chip_delay = 30;
-
-	return 0;
-}
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 471cd6b..3f87b4d 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -44,6 +44,7 @@ COBJS-$(CONFIG_NAND_NOMADIK) += nomadik.o
 COBJS-$(CONFIG_NAND_S3C2410) += s3c2410_nand.c
 COBJS-$(CONFIG_NAND_S3C64XX) += s3c64xx.o
 COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
+COBJS-$(CONFIG_NAND_PLAT) += nand_plat.o
 endif
 
 COBJS	:= $(COBJS-y)
diff --git a/drivers/mtd/nand/nand_plat.c b/drivers/mtd/nand/nand_plat.c
new file mode 100644
index 0000000..a2f33a4
--- /dev/null
+++ b/drivers/mtd/nand/nand_plat.c
@@ -0,0 +1,53 @@
+/*
+ * Genericish driver for memory mapped NAND devices
+ *
+ * Copyright (c) 2006-2009 Analog Devices Inc.
+ * Licensed under the GPL-2 or later.
+ */
+
+/* Your board must implement the following macros:
+ *  NAND_PLAT_WRITE_CMD(chip, cmd)
+ *  NAND_PLAT_WRITE_ADR(chip, cmd)
+ *  NAND_PLAT_INIT()
+ *
+ * It may also implement the following:
+ *  NAND_PLAT_DEV_READY(chip)
+ */
+
+#include <common.h>
+#include <asm/io.h>
+
+#include <nand.h>
+
+static void cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
+{
+	struct nand_chip *this = mtd->priv;
+
+	if (cmd == NAND_CMD_NONE)
+		return;
+
+	if (ctrl & NAND_CLE)
+		NAND_PLAT_WRITE_CMD(this, cmd);
+	else
+		NAND_PLAT_WRITE_ADR(this, cmd);
+}
+
+#ifdef NAND_PLAT_DEV_READY
+static int dev_ready(struct mtd_info *mtd)
+{
+	return NAND_PLAT_DEV_READY((struct nand_chip *)mtd->priv);
+}
+#else
+# define dev_ready NULL
+#endif
+
+int board_nand_init(struct nand_chip *nand)
+{
+	NAND_PLAT_INIT();
+
+	nand->cmd_ctrl = cmd_ctrl;
+	nand->dev_ready = dev_ready;
+	nand->ecc.mode = NAND_ECC_SOFT;
+
+	return 0;
+}
diff --git a/include/configs/bf537-stamp.h b/include/configs/bf537-stamp.h
index 3e5862d..e8c47ae 100644
--- a/include/configs/bf537-stamp.h
+++ b/include/configs/bf537-stamp.h
@@ -136,36 +136,28 @@
 /*
  * NAND Settings
  */
-/* #define CONFIG_BF537_NAND */
-#ifdef CONFIG_BF537_NAND
-# define CONFIG_CMD_NAND
-#endif
-
-#define CONFIG_SYS_NAND_ADDR		0x20212000
-#define CONFIG_SYS_NAND_BASE		CONFIG_SYS_NAND_ADDR
+/* #define CONFIG_NAND_PLAT */
+#define CONFIG_SYS_NAND_BASE		0x20212000
 #define CONFIG_SYS_MAX_NAND_DEVICE	1
-#define SECTORSIZE		512
-#define ADDR_COLUMN		1
-#define ADDR_PAGE		2
-#define ADDR_COLUMN_PAGE	3
-#define NAND_ChipID_UNKNOWN	0x00
-#define NAND_MAX_FLOORS		1
-#define BFIN_NAND_READY		PF3
-
-#define NAND_WAIT_READY(nand) \
+
+#define BFIN_NAND_CLE(chip) ((unsigned long)(chip)->IO_ADDR_W | (1 << 2))
+#define BFIN_NAND_ALE(chip) ((unsigned long)(chip)->IO_ADDR_W | (1 << 1))
+#define BFIN_NAND_READY     PF3
+#define BFIN_NAND_WRITE(addr, cmd) \
 	do { \
-		int timeout = 0; \
-		while (!(*pPORTFIO & PF3)) \
-			if (timeout++ > 100000) \
-				break; \
+		bfin_write8(addr, cmd); \
+		SSYNC(); \
 	} while (0)
 
-#define BFIN_NAND_CLE		(1 << 2)	/* A2 -> Command Enable */
-#define BFIN_NAND_ALE		(1 << 1)	/* A1 -> Address Enable */
-#define WRITE_NAND_COMMAND(d, adr) bfin_write8(adr | BFIN_NAND_CLE, d)
-#define WRITE_NAND_ADDRESS(d, adr) bfin_write8(adr | BFIN_NAND_ALE, d)
-#define WRITE_NAND(d, adr)         bfin_write8(adr, d)
-#define READ_NAND(adr)             bfin_read8(adr)
+#define NAND_PLAT_WRITE_CMD(chip, cmd) BFIN_NAND_WRITE(BFIN_NAND_CLE(chip), cmd)
+#define NAND_PLAT_WRITE_ADR(chip, cmd) BFIN_NAND_WRITE(BFIN_NAND_ALE(chip), cmd)
+#define NAND_PLAT_DEV_READY(chip)      (bfin_read_PORTFIO() & BFIN_NAND_READY)
+#define NAND_PLAT_INIT() \
+	do { \
+		bfin_write_PORTF_FER(bfin_read_PORTF_FER() & ~BFIN_NAND_READY); \
+		bfin_write_PORTFIO_DIR(bfin_read_PORTFIO_DIR() & ~BFIN_NAND_READY); \
+		bfin_write_PORTFIO_INEN(bfin_read_PORTFIO_INEN() | BFIN_NAND_READY); \
+	} while (0)
 
 
 /*
diff --git a/include/configs/bfin_adi_common.h b/include/configs/bfin_adi_common.h
index bfe5376..ac0298d 100644
--- a/include/configs/bfin_adi_common.h
+++ b/include/configs/bfin_adi_common.h
@@ -38,6 +38,9 @@
 #  define CONFIG_CMD_USB_STORAGE
 #  define CONFIG_DOS_PARTITION
 # endif
+# ifdef CONFIG_NAND_PLAT
+#  define CONFIG_CMD_NAND
+# endif
 # ifdef CONFIG_POST
 #  define CONFIG_CMD_DIAG
 # endif
-- 
1.6.3

[U-Boot] [PATCH v5] mtd: nand: new base driver for memory mapped nand devices

[Scott Wood]

On Wed, May 13, 2009 at 07:45:11PM -0400, Mike Frysinger wrote:
> The BF537-STAMP Blackfin board had a driver for working with NAND devices
> that are simply memory mapped.  Since there is nothing Blackfin specific
> about this, generalize the driver a bit so that everyone can leverage it.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Scott Wood <scottwood@freescale.com>

Applied to u-boot-nand-flash/next

-Scott

[U-Boot] [PATCH v6] mtd: nand: new base driver for memory mapped nand devices

[Mike Frysinger]

The BF537-STAMP Blackfin board had a driver for working with NAND devices
that are simply memory mapped.  Since there is nothing Blackfin specific
about this, generalize the driver a bit so that everyone can leverage it.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>
Signed-off-by: Scott Wood <scottwood@freescale.com>
---
v6
	- add "plat_" prefixes to avoid cpp error due to stubbed functions
	  having the same name as the members of the nand_chip struct

 board/bf537-stamp/Makefile        |    1 -
 board/bf537-stamp/nand.c          |  100 -------------------------------------
 drivers/mtd/nand/Makefile         |    1 +
 drivers/mtd/nand/nand_plat.c      |   53 +++++++++++++++++++
 include/configs/bf537-stamp.h     |   44 +++++++----------
 include/configs/bfin_adi_common.h |    3 +
 6 files changed, 75 insertions(+), 127 deletions(-)
 delete mode 100644 board/bf537-stamp/nand.c
 create mode 100644 drivers/mtd/nand/nand_plat.c

diff --git a/board/bf537-stamp/Makefile b/board/bf537-stamp/Makefile
index 1dbf406..6da04e3 100644
--- a/board/bf537-stamp/Makefile
+++ b/board/bf537-stamp/Makefile
@@ -32,7 +32,6 @@ LIB	= $(obj)lib$(BOARD).a
 COBJS-y	:= $(BOARD).o cmd_bf537led.o
 COBJS-$(CONFIG_BFIN_IDE)   += ide-cf.o
 COBJS-$(CONFIG_CMD_EEPROM) += spi_flash.o
-COBJS-$(CONFIG_CMD_NAND)   += nand.o
 COBJS-$(CONFIG_POST)       += post.o post-memory.o
 
 SRCS	:= $(SOBJS-y:.o=.S) $(COBJS-y:.o=.c)
diff --git a/board/bf537-stamp/nand.c b/board/bf537-stamp/nand.c
deleted file mode 100644
index 181e83d..0000000
--- a/board/bf537-stamp/nand.c
+++ /dev/null
@@ -1,100 +0,0 @@
-/*
- * Copyright (c) 2006-2007 Analog Devices Inc.
- *
- * See file CREDITS for list of people who contributed to this
- * project.
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of the GNU General Public License as
- * published by the Free Software Foundation; either version 2 of
- * the License, or (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
- * MA 02111-1307 USA
- */
-
-#include <common.h>
-#include <asm/io.h>
-
-#include <nand.h>
-
-#define CONCAT(a,b,c,d) a ## b ## c ## d
-#define PORT(a,b)  CONCAT(pPORT,a,b,)
-
-#ifndef CONFIG_NAND_GPIO_PORT
-#define CONFIG_NAND_GPIO_PORT F
-#endif
-
-/*
- * hardware specific access to control-lines
- */
-static void bfin_hwcontrol(struct mtd_info *mtd, int cmd, unsigned int ctrl)
-{
-	register struct nand_chip *this = mtd->priv;
-	u32 IO_ADDR_W = (u32) this->IO_ADDR_W;
-
-	if (ctrl & NAND_CTRL_CHANGE) {
-		if (ctrl & NAND_CLE)
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE + BFIN_NAND_CLE;
-		else
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE;
-		if (ctrl & NAND_ALE)
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE + BFIN_NAND_ALE;
-		else
-			IO_ADDR_W = CONFIG_SYS_NAND_BASE;
-		this->IO_ADDR_W = (void __iomem *) IO_ADDR_W;
-	}
-	this->IO_ADDR_R = this->IO_ADDR_W;
-
-	/* Drain the writebuffer */
-	SSYNC();
-
-	if (cmd != NAND_CMD_NONE)
-		writeb(cmd, this->IO_ADDR_W);
-}
-
-int bfin_device_ready(struct mtd_info *mtd)
-{
-	int ret = (*PORT(CONFIG_NAND_GPIO_PORT, IO) & BFIN_NAND_READY) ? 1 : 0;
-	SSYNC();
-	return ret;
-}
-
-/*
- * Board-specific NAND initialization. The following members of the
- * argument are board-specific (per include/linux/mtd/nand.h):
- * - IO_ADDR_R?: address to read the 8 I/O lines of the flash device
- * - IO_ADDR_W?: address to write the 8 I/O lines of the flash device
- * - cmd_ctrl: hardwarespecific function for accesing control-lines
- * - dev_ready: hardwarespecific function for  accesing device ready/busy line
- * - enable_hwecc?: function to enable (reset)  hardware ecc generator. Must
- *   only be provided if a hardware ECC is available
- * - ecc.mode: mode of ecc, see defines
- * - chip_delay: chip dependent delay for transfering data from array to
- *   read regs (tR)
- * - options: various chip options. They can partly be set to inform
- *   nand_scan about special functionality. See the defines for further
- *   explanation
- * Members with a "?" were not set in the merged testing-NAND branch,
- * so they are not set here either.
- */
-int board_nand_init(struct nand_chip *nand)
-{
-	*PORT(CONFIG_NAND_GPIO_PORT, _FER) &= ~BFIN_NAND_READY;
-	*PORT(CONFIG_NAND_GPIO_PORT, IO_DIR) &= ~BFIN_NAND_READY;
-	*PORT(CONFIG_NAND_GPIO_PORT, IO_INEN) |= BFIN_NAND_READY;
-
-	nand->cmd_ctrl = bfin_hwcontrol;
-	nand->ecc.mode = NAND_ECC_SOFT;
-	nand->dev_ready = bfin_device_ready;
-	nand->chip_delay = 30;
-
-	return 0;
-}
diff --git a/drivers/mtd/nand/Makefile b/drivers/mtd/nand/Makefile
index 471cd6b..3f87b4d 100644
--- a/drivers/mtd/nand/Makefile
+++ b/drivers/mtd/nand/Makefile
@@ -44,6 +44,7 @@ COBJS-$(CONFIG_NAND_NOMADIK) += nomadik.o
 COBJS-$(CONFIG_NAND_S3C2410) += s3c2410_nand.c
 COBJS-$(CONFIG_NAND_S3C64XX) += s3c64xx.o
 COBJS-$(CONFIG_NAND_OMAP_GPMC) += omap_gpmc.o
+COBJS-$(CONFIG_NAND_PLAT) += nand_plat.o
 endif
 
 COBJS	:= $(COBJS-y)
diff --git a/drivers/mtd/nand/nand_plat.c b/drivers/mtd/nand/nand_plat.c
new file mode 100644
index 0000000..b35492b
--- /dev/null
+++ b/drivers/mtd/nand/nand_plat.c
@@ -0,0 +1,53 @@
+/*
+ * Genericish driver for memory mapped NAND devices
+ *
+ * Copyright (c) 2006-2009 Analog Devices Inc.
+ * Licensed under the GPL-2 or later.
+ */
+
+/* Your board must implement the following macros:
+ *  NAND_PLAT_WRITE_CMD(chip, cmd)
+ *  NAND_PLAT_WRITE_ADR(chip, cmd)
+ *  NAND_PLAT_INIT()
+ *
+ * It may also implement the following:
+ *  NAND_PLAT_DEV_READY(chip)
+ */
+
+#include <common.h>
+#include <asm/io.h>
+
+#include <nand.h>
+
+static void plat_cmd_ctrl(struct mtd_info *mtd, int cmd, unsigned int ctrl)
+{
+	struct nand_chip *this = mtd->priv;
+
+	if (cmd == NAND_CMD_NONE)
+		return;
+
+	if (ctrl & NAND_CLE)
+		NAND_PLAT_WRITE_CMD(this, cmd);
+	else
+		NAND_PLAT_WRITE_ADR(this, cmd);
+}
+
+#ifdef NAND_PLAT_DEV_READY
+static int plat_dev_ready(struct mtd_info *mtd)
+{
+	return NAND_PLAT_DEV_READY((struct nand_chip *)mtd->priv);
+}
+#else
+# define plat_dev_ready NULL
+#endif
+
+int board_nand_init(struct nand_chip *nand)
+{
+	NAND_PLAT_INIT();
+
+	nand->cmd_ctrl = plat_cmd_ctrl;
+	nand->dev_ready = plat_dev_ready;
+	nand->ecc.mode = NAND_ECC_SOFT;
+
+	return 0;
+}
diff --git a/include/configs/bf537-stamp.h b/include/configs/bf537-stamp.h
index 3e5862d..e8c47ae 100644
--- a/include/configs/bf537-stamp.h
+++ b/include/configs/bf537-stamp.h
@@ -136,36 +136,28 @@
 /*
  * NAND Settings
  */
-/* #define CONFIG_BF537_NAND */
-#ifdef CONFIG_BF537_NAND
-# define CONFIG_CMD_NAND
-#endif
-
-#define CONFIG_SYS_NAND_ADDR		0x20212000
-#define CONFIG_SYS_NAND_BASE		CONFIG_SYS_NAND_ADDR
+/* #define CONFIG_NAND_PLAT */
+#define CONFIG_SYS_NAND_BASE		0x20212000
 #define CONFIG_SYS_MAX_NAND_DEVICE	1
-#define SECTORSIZE		512
-#define ADDR_COLUMN		1
-#define ADDR_PAGE		2
-#define ADDR_COLUMN_PAGE	3
-#define NAND_ChipID_UNKNOWN	0x00
-#define NAND_MAX_FLOORS		1
-#define BFIN_NAND_READY		PF3
-
-#define NAND_WAIT_READY(nand) \
+
+#define BFIN_NAND_CLE(chip) ((unsigned long)(chip)->IO_ADDR_W | (1 << 2))
+#define BFIN_NAND_ALE(chip) ((unsigned long)(chip)->IO_ADDR_W | (1 << 1))
+#define BFIN_NAND_READY     PF3
+#define BFIN_NAND_WRITE(addr, cmd) \
 	do { \
-		int timeout = 0; \
-		while (!(*pPORTFIO & PF3)) \
-			if (timeout++ > 100000) \
-				break; \
+		bfin_write8(addr, cmd); \
+		SSYNC(); \
 	} while (0)
 
-#define BFIN_NAND_CLE		(1 << 2)	/* A2 -> Command Enable */
-#define BFIN_NAND_ALE		(1 << 1)	/* A1 -> Address Enable */
-#define WRITE_NAND_COMMAND(d, adr) bfin_write8(adr | BFIN_NAND_CLE, d)
-#define WRITE_NAND_ADDRESS(d, adr) bfin_write8(adr | BFIN_NAND_ALE, d)
-#define WRITE_NAND(d, adr)         bfin_write8(adr, d)
-#define READ_NAND(adr)             bfin_read8(adr)
+#define NAND_PLAT_WRITE_CMD(chip, cmd) BFIN_NAND_WRITE(BFIN_NAND_CLE(chip), cmd)
+#define NAND_PLAT_WRITE_ADR(chip, cmd) BFIN_NAND_WRITE(BFIN_NAND_ALE(chip), cmd)
+#define NAND_PLAT_DEV_READY(chip)      (bfin_read_PORTFIO() & BFIN_NAND_READY)
+#define NAND_PLAT_INIT() \
+	do { \
+		bfin_write_PORTF_FER(bfin_read_PORTF_FER() & ~BFIN_NAND_READY); \
+		bfin_write_PORTFIO_DIR(bfin_read_PORTFIO_DIR() & ~BFIN_NAND_READY); \
+		bfin_write_PORTFIO_INEN(bfin_read_PORTFIO_INEN() | BFIN_NAND_READY); \
+	} while (0)
 
 
 /*
diff --git a/include/configs/bfin_adi_common.h b/include/configs/bfin_adi_common.h
index bfe5376..ac0298d 100644
--- a/include/configs/bfin_adi_common.h
+++ b/include/configs/bfin_adi_common.h
@@ -38,6 +38,9 @@
 #  define CONFIG_CMD_USB_STORAGE
 #  define CONFIG_DOS_PARTITION
 # endif
+# ifdef CONFIG_NAND_PLAT
+#  define CONFIG_CMD_NAND
+# endif
 # ifdef CONFIG_POST
 #  define CONFIG_CMD_DIAG
 # endif
-- 
1.6.3.1

[U-Boot] [PATCH v6] mtd: nand: new base driver for memory mapped nand devices

[Scott Wood]

On Mon, May 25, 2009 at 10:42:28PM -0400, Mike Frysinger wrote:
> The BF537-STAMP Blackfin board had a driver for working with NAND devices
> that are simply memory mapped.  Since there is nothing Blackfin specific
> about this, generalize the driver a bit so that everyone can leverage it.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> Signed-off-by: Scott Wood <scottwood@freescale.com>
> ---
> v6
> 	- add "plat_" prefixes to avoid cpp error due to stubbed functions
> 	  having the same name as the members of the nand_chip struct

Applied to u-boot-nand-flash/next.

-Scott