[U-Boot] [PATCH] nand: mxs_nand_spl: support use of env in SPL
in order to use env in the SPL (CONFIG_SPL_ENV_SUPPORT) nand_info, mtd_block_isbad, and mtd_read must be available.
Signed-off-by: Tim Harvey tharvey@gateworks.com --- drivers/mtd/nand/mxs_nand_spl.c | 112 ++++++++++++++++++++++------------------ 1 file changed, 61 insertions(+), 51 deletions(-)
diff --git a/drivers/mtd/nand/mxs_nand_spl.c b/drivers/mtd/nand/mxs_nand_spl.c index 0e7c364..d9d1811 100644 --- a/drivers/mtd/nand/mxs_nand_spl.c +++ b/drivers/mtd/nand/mxs_nand_spl.c @@ -8,7 +8,7 @@ #include <nand.h> #include <malloc.h>
-static nand_info_t mtd; +nand_info_t nand_info[1]; static struct nand_chip nand_chip;
static void mxs_nand_command(struct mtd_info *mtd, unsigned int command, @@ -123,14 +123,11 @@ static int mxs_read_page_ecc(struct mtd_info *mtd, void *buf, unsigned int page) return 0; }
-static int is_badblock(struct mtd_info *mtd, loff_t offs, int allowbbt) +int mtd_block_isbad(struct mtd_info *mtd, loff_t offs) { register struct nand_chip *chip = mtd->priv; - unsigned int block = offs >> chip->phys_erase_shift; unsigned int page = offs >> chip->page_shift;
- debug("%s offs=0x%08x block:%d page:%d\n", __func__, (int)offs, block, - page); chip->cmdfunc(mtd, NAND_CMD_READ0, mtd->writesize, page); memset(chip->oob_poi, 0, mtd->oobsize); chip->read_buf(mtd, chip->oob_poi, mtd->oobsize); @@ -138,62 +135,34 @@ static int is_badblock(struct mtd_info *mtd, loff_t offs, int allowbbt) return chip->oob_poi[0] != 0xff; }
-/* setup mtd and nand structs and init mxs_nand driver */ -static int mxs_nand_init(void) -{ - /* return if already initalized */ - if (nand_chip.numchips) - return 0; - - /* init mxs nand driver */ - board_nand_init(&nand_chip); - mtd.priv = &nand_chip; - /* set mtd functions */ - nand_chip.cmdfunc = mxs_nand_command; - nand_chip.numchips = 1; - - /* identify flash device */ - puts("NAND : "); - if (mxs_flash_ident(&mtd)) { - printf("Failed to identify\n"); - return -1; - } - - /* allocate and initialize buffers */ - nand_chip.buffers = memalign(ARCH_DMA_MINALIGN, - sizeof(*nand_chip.buffers)); - nand_chip.oob_poi = nand_chip.buffers->databuf + mtd.writesize; - /* setup flash layout (does not scan as we override that) */ - mtd.size = nand_chip.chipsize; - nand_chip.scan_bbt(&mtd); - - printf("%llu MiB\n", (mtd.size / (1024 * 1024))); - return 0; -} - -int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf) +int mtd_read(struct mtd_info *mtd, loff_t offs, size_t size, size_t *retlen, + uchar *buf) { struct nand_chip *chip; unsigned int page; unsigned int nand_page_per_block; unsigned int sz = 0; + nand_info_t *info = &nand_info[0];
- if (mxs_nand_init()) - return -ENODEV; - chip = mtd.priv; + chip = info->priv; page = offs >> chip->page_shift; - nand_page_per_block = mtd.erasesize / mtd.writesize; + nand_page_per_block = info->erasesize / info->writesize;
- debug("%s offset:0x%08x len:%d page:%d\n", __func__, offs, size, page); + debug("%s offs:0x%08x len:0x%x page:%d\n", __func__, (int)offs, size, + page);
- size = roundup(size, mtd.writesize); + size = roundup(size, info->writesize); + if (retlen) + *retlen = 0; while (sz < size) { - if (mxs_read_page_ecc(&mtd, buf, page) < 0) + if (mxs_read_page_ecc(info, buf, page) < 0) return -1; - sz += mtd.writesize; - offs += mtd.writesize; + sz += info->writesize; + offs += info->writesize; page++; - buf += mtd.writesize; + buf += info->writesize; + if (retlen) + *retlen += info->writesize;
/* * Check if we have crossed a block boundary, and if so @@ -204,10 +173,10 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf) * Yes, new block. See if this block is good. If not, * loop until we find a good block. */ - while (is_badblock(&mtd, offs, 1)) { + while (mtd_block_isbad(info, offs)) { page = page + nand_page_per_block; /* Check i we've reached the end of flash. */ - if (page >= mtd.size >> chip->page_shift) + if (page >= info->size >> chip->page_shift) return -ENOMEM; } } @@ -216,6 +185,46 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf) return 0; }
+/* setup mtd and nand structs and init mxs_nand driver */ +static int mxs_nand_init(void) +{ + nand_info_t *info = &nand_info[0]; + + /* return if already initalized */ + if (nand_chip.numchips) + return 0; + + /* init mxs nand driver */ + board_nand_init(&nand_chip); + info->priv = &nand_chip; + /* set mtd functions */ + nand_chip.cmdfunc = mxs_nand_command; + nand_chip.numchips = 1; + + /* identify flash device */ + puts("NAND : "); + if (mxs_flash_ident(info)) { + printf("Failed to identify\n"); + return -1; + } + + /* allocate and initialize buffers */ + nand_chip.buffers = memalign(ARCH_DMA_MINALIGN, + sizeof(*nand_chip.buffers)); + nand_chip.oob_poi = nand_chip.buffers->databuf + info->writesize; + /* setup flash layout (does not scan as we override that) */ + info->size = nand_chip.chipsize; + nand_chip.scan_bbt(info); + + printf("%llu MiB\n", (info->size / (1024 * 1024))); + return 0; +} + +int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf) +{ + return mtd_read(NULL, offs, size, NULL, buf); +} + int nand_default_bbt(struct mtd_info *mtd) { return 0; @@ -223,6 +232,7 @@ int nand_default_bbt(struct mtd_info *mtd)
void nand_init(void) { + mxs_nand_init(); }
void nand_deselect(void)
On Fri, May 8, 2015 at 2:39 PM, Tim Harvey tharvey@gateworks.com wrote:
in order to use env in the SPL (CONFIG_SPL_ENV_SUPPORT) nand_info, mtd_block_isbad, and mtd_read must be available.
Signed-off-by: Tim Harvey tharvey@gateworks.com
drivers/mtd/nand/mxs_nand_spl.c | 112 ++++++++++++++++++++++------------------ 1 file changed, 61 insertions(+), 51 deletions(-)
diff --git a/drivers/mtd/nand/mxs_nand_spl.c b/drivers/mtd/nand/mxs_nand_spl.c index 0e7c364..d9d1811 100644 --- a/drivers/mtd/nand/mxs_nand_spl.c +++ b/drivers/mtd/nand/mxs_nand_spl.c @@ -8,7 +8,7 @@ #include <nand.h> #include <malloc.h>
-static nand_info_t mtd; +nand_info_t nand_info[1]; static struct nand_chip nand_chip;
static void mxs_nand_command(struct mtd_info *mtd, unsigned int command, @@ -123,14 +123,11 @@ static int mxs_read_page_ecc(struct mtd_info *mtd, void *buf, unsigned int page) return 0; }
-static int is_badblock(struct mtd_info *mtd, loff_t offs, int allowbbt) +int mtd_block_isbad(struct mtd_info *mtd, loff_t offs) { register struct nand_chip *chip = mtd->priv;
unsigned int block = offs >> chip->phys_erase_shift; unsigned int page = offs >> chip->page_shift;debug("%s offs=0x%08x block:%d page:%d\n", __func__, (int)offs, block,page); chip->cmdfunc(mtd, NAND_CMD_READ0, mtd->writesize, page); memset(chip->oob_poi, 0, mtd->oobsize); chip->read_buf(mtd, chip->oob_poi, mtd->oobsize);@@ -138,62 +135,34 @@ static int is_badblock(struct mtd_info *mtd, loff_t offs, int allowbbt) return chip->oob_poi[0] != 0xff; }
-/* setup mtd and nand structs and init mxs_nand driver */ -static int mxs_nand_init(void) -{
/* return if already initalized */if (nand_chip.numchips)return 0;/* init mxs nand driver */board_nand_init(&nand_chip);mtd.priv = &nand_chip;/* set mtd functions */nand_chip.cmdfunc = mxs_nand_command;nand_chip.numchips = 1;/* identify flash device */puts("NAND : ");if (mxs_flash_ident(&mtd)) {printf("Failed to identify\n");return -1;}/* allocate and initialize buffers */nand_chip.buffers = memalign(ARCH_DMA_MINALIGN,sizeof(*nand_chip.buffers));nand_chip.oob_poi = nand_chip.buffers->databuf + mtd.writesize;/* setup flash layout (does not scan as we override that) */mtd.size = nand_chip.chipsize;nand_chip.scan_bbt(&mtd);printf("%llu MiB\n", (mtd.size / (1024 * 1024)));return 0;-}
-int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf) +int mtd_read(struct mtd_info *mtd, loff_t offs, size_t size, size_t *retlen,
uchar *buf){ struct nand_chip *chip; unsigned int page; unsigned int nand_page_per_block; unsigned int sz = 0;
nand_info_t *info = &nand_info[0];
if (mxs_nand_init())return -ENODEV;chip = mtd.priv;
chip = info->priv; page = offs >> chip->page_shift;
nand_page_per_block = mtd.erasesize / mtd.writesize;
nand_page_per_block = info->erasesize / info->writesize;
debug("%s offset:0x%08x len:%d page:%d\n", __func__, offs, size, page);
debug("%s offs:0x%08x len:0x%x page:%d\n", __func__, (int)offs, size,page);
size = roundup(size, mtd.writesize);
size = roundup(size, info->writesize);if (retlen)*retlen = 0; while (sz < size) {
if (mxs_read_page_ecc(&mtd, buf, page) < 0)
if (mxs_read_page_ecc(info, buf, page) < 0) return -1;
sz += mtd.writesize;offs += mtd.writesize;
sz += info->writesize;offs += info->writesize; page++;
buf += mtd.writesize;
buf += info->writesize;if (retlen)*retlen += info->writesize; /* * Check if we have crossed a block boundary, and if so@@ -204,10 +173,10 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf) * Yes, new block. See if this block is good. If not, * loop until we find a good block. */
while (is_badblock(&mtd, offs, 1)) {
while (mtd_block_isbad(info, offs)) { page = page + nand_page_per_block; /* Check i we've reached the end of flash. */
if (page >= mtd.size >> chip->page_shift)
if (page >= info->size >> chip->page_shift) return -ENOMEM; } }@@ -216,6 +185,46 @@ int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf) return 0; }
+/* setup mtd and nand structs and init mxs_nand driver */ +static int mxs_nand_init(void) +{
nand_info_t *info = &nand_info[0];/* return if already initalized */if (nand_chip.numchips)return 0;/* init mxs nand driver */board_nand_init(&nand_chip);info->priv = &nand_chip;/* set mtd functions */nand_chip.cmdfunc = mxs_nand_command;nand_chip.numchips = 1;/* identify flash device */puts("NAND : ");if (mxs_flash_ident(info)) {printf("Failed to identify\n");return -1;}/* allocate and initialize buffers */nand_chip.buffers = memalign(ARCH_DMA_MINALIGN,sizeof(*nand_chip.buffers));nand_chip.oob_poi = nand_chip.buffers->databuf + info->writesize;/* setup flash layout (does not scan as we override that) */info->size = nand_chip.chipsize;nand_chip.scan_bbt(info);printf("%llu MiB\n", (info->size / (1024 * 1024)));return 0;+}
+int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf) +{
return mtd_read(NULL, offs, size, NULL, buf);+}
int nand_default_bbt(struct mtd_info *mtd) { return 0; @@ -223,6 +232,7 @@ int nand_default_bbt(struct mtd_info *mtd)
void nand_init(void) {
mxs_nand_init();}
void nand_deselect(void)
1.9.1
Stefano,
It appears that gw_ventana and cm_fx6 are the only users of this (CONFIG_MX6 && CONFIG_NAND_MXS && building with SPL) so I'm adding Nikita to Cc for him to test/comment.
Cc: Nikita Kiryanov nikita@compulab.co.il
Tim
On Fri, 2015-05-08 at 14:39 -0700, Tim Harvey wrote:
-int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf) +int mtd_read(struct mtd_info *mtd, loff_t offs, size_t size, size_t *retlen,
uchar *buf){ struct nand_chip *chip; unsigned int page; unsigned int nand_page_per_block; unsigned int sz = 0;
- nand_info_t *info = &nand_info[0];
Any reason not to use the passed-in mtd pointer (and fix the nand_spl_load_image wrapper to pass in &nand_info[0])?
+/* setup mtd and nand structs and init mxs_nand driver */ +static int mxs_nand_init(void) +{
- nand_info_t *info = &nand_info[0];
- /* return if already initalized */
- if (nand_chip.numchips)
return 0;- /* init mxs nand driver */
- board_nand_init(&nand_chip);
- info->priv = &nand_chip;
- /* set mtd functions */
- nand_chip.cmdfunc = mxs_nand_command;
- nand_chip.numchips = 1;
- /* identify flash device */
- puts("NAND : ");
- if (mxs_flash_ident(info)) {
printf("Failed to identify\n");return -1;- }
- /* allocate and initialize buffers */
- nand_chip.buffers = memalign(ARCH_DMA_MINALIGN,
sizeof(*nand_chip.buffers));- nand_chip.oob_poi = nand_chip.buffers->databuf + info->writesize;
- /* setup flash layout (does not scan as we override that) */
- info->size = nand_chip.chipsize;
- nand_chip.scan_bbt(info);
- printf("%llu MiB\n", (info->size / (1024 * 1024)));
- return 0;
+}
Why did this function need to be moved?
+int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf) +{
- return mtd_read(NULL, offs, size, NULL, buf);
+}
It'd be nice to keep the wrapper near the function it wraps.
I don't see any other such wrappers; is there no other driver that currently works with SPL env?
int nand_default_bbt(struct mtd_info *mtd) { return 0; @@ -223,6 +232,7 @@ int nand_default_bbt(struct mtd_info *mtd)
void nand_init(void) {
- mxs_nand_init();
}
Do you still need the "return if already initialized" check with this change? How is this change related to the rest?
-Scott
On Wed, May 13, 2015 at 4:38 PM, Scott Wood scottwood@freescale.com wrote:
On Fri, 2015-05-08 at 14:39 -0700, Tim Harvey wrote:
-int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf) +int mtd_read(struct mtd_info *mtd, loff_t offs, size_t size, size_t *retlen,
uchar *buf){ struct nand_chip *chip; unsigned int page; unsigned int nand_page_per_block; unsigned int sz = 0;
nand_info_t *info = &nand_info[0];Any reason not to use the passed-in mtd pointer (and fix the nand_spl_load_image wrapper to pass in &nand_info[0])?
Hi Scott,
No reason - I think I just got confused because I didn't realize nand_info_t was a typedef for struct mtd_info.
+/* setup mtd and nand structs and init mxs_nand driver */ +static int mxs_nand_init(void) +{
nand_info_t *info = &nand_info[0];/* return if already initalized */if (nand_chip.numchips)return 0;/* init mxs nand driver */board_nand_init(&nand_chip);info->priv = &nand_chip;/* set mtd functions */nand_chip.cmdfunc = mxs_nand_command;nand_chip.numchips = 1;/* identify flash device */puts("NAND : ");if (mxs_flash_ident(info)) {printf("Failed to identify\n");return -1;}/* allocate and initialize buffers */nand_chip.buffers = memalign(ARCH_DMA_MINALIGN,sizeof(*nand_chip.buffers));nand_chip.oob_poi = nand_chip.buffers->databuf + info->writesize;/* setup flash layout (does not scan as we override that) */info->size = nand_chip.chipsize;nand_chip.scan_bbt(info);printf("%llu MiB\n", (info->size / (1024 * 1024)));return 0;+}
Why did this function need to be moved?
+int nand_spl_load_image(uint32_t offs, unsigned int size, void *buf) +{
return mtd_read(NULL, offs, size, NULL, buf);+}
It'd be nice to keep the wrapper near the function it wraps.
It doesn't need to be moved and moving the location of the wrapper as well makes the patch much easier to read/review.
I don't see any other such wrappers; is there no other driver that currently works with SPL env?
I don't know - this got me digging into why I needed mtd_read() implemented all of the sudden once I enabled CONFIG_SPL_ENV_SUPPORT.
It turns out that this is needed because env_nand.c:readenv() calls nand_read_skip_bad, which uses the mtd layer. For SPL the functionality of readenv() is pretty much already in nand_spl_load_image() so I find if do the following (instead of making any changes to mxs_nand_spl.c) which allows the mtd layers to not be used I save 4KB in the SPL:
diff --git a/common/env_nand.c b/common/env_nand.c index cc7e979..97d10a2 100644 --- a/common/env_nand.c +++ b/common/env_nand.c @@ -260,6 +260,9 @@ int saveenv(void) } #endif /* CMD_SAVEENV */
+#if defined(CONFIG_SPL_BUILD) +#define readenv(offset, buf) nand_spl_load_image(offset, CONFIG_ENV_SIZE, buf) +#else static int readenv(size_t offset, u_char *buf) { size_t end = offset + CONFIG_ENV_RANGE; @@ -295,6 +298,7 @@ static int readenv(size_t offset, u_char *buf)
return 0; } +#endif /* #if defined(CONFIG_SPL_BUILD) */
#ifdef CONFIG_ENV_OFFSET_OOB int get_nand_env_oob(nand_info_t *nand, unsigned long *result)
I don't think I'm loosing any necessary functionality in the SPL by doing the above and it saves me 4KB in the SPL which is precious space.
int nand_default_bbt(struct mtd_info *mtd) { return 0; @@ -223,6 +232,7 @@ int nand_default_bbt(struct mtd_info *mtd)
void nand_init(void) {
mxs_nand_init();}
Do you still need the "return if already initialized" check with this change? How is this change related to the rest?
The check can now be eliminated as I'm moving the init to where it likely should have been all along instead of in nand_spl_load_image(). In fact, what should be done is to simply rename the static int mxs_nand_init() to nand_init() instead of calling the static function.
So while I could revise the above patch based on some of the things you pointed out here it wouldn't be necessary (although I could do it as a cleanup) with the above patch instead of env_nand.c.
What do you think?
Tim
On Thu, 2015-05-14 at 08:12 -0700, Tim Harvey wrote:
It turns out that this is needed because env_nand.c:readenv() calls nand_read_skip_bad, which uses the mtd layer. For SPL the functionality of readenv() is pretty much already in nand_spl_load_image() so I find if do the following (instead of making any changes to mxs_nand_spl.c) which allows the mtd layers to not be used I save 4KB in the SPL:
diff --git a/common/env_nand.c b/common/env_nand.c index cc7e979..97d10a2 100644 --- a/common/env_nand.c +++ b/common/env_nand.c @@ -260,6 +260,9 @@ int saveenv(void) } #endif /* CMD_SAVEENV */
+#if defined(CONFIG_SPL_BUILD) +#define readenv(offset, buf) nand_spl_load_image(offset, CONFIG_ENV_SIZE, buf) +#else static int readenv(size_t offset, u_char *buf) { size_t end = offset + CONFIG_ENV_RANGE; @@ -295,6 +298,7 @@ static int readenv(size_t offset, u_char *buf)
return 0;} +#endif /* #if defined(CONFIG_SPL_BUILD) */
#ifdef CONFIG_ENV_OFFSET_OOB int get_nand_env_oob(nand_info_t *nand, unsigned long *result)
I don't think I'm loosing any necessary functionality in the SPL by doing the above and it saves me 4KB in the SPL which is precious space.
I'd prefer a function over a macro, but OK.
readenv() itself looks like it could be greatly simplified by dropping the loop and bad block check, since nand_read_skip_bad() handles that.
-Scott
participants (2)
-
Scott Wood -
Tim Harvey