[U-Boot] [PATCH] mtd: nand: new base driver for memory mapped nand devices
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
participants (3)
-
Mike Frysinger -
Scott Wood -
Wolfgang Denk