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

---
 drivers/mtd/nand/mxs_nand_spl.c | 112 ++++++++++++++++++++++------------------
 1 file changed, 61 insertions(+), 51 deletions(-)

drivers/mtd/nand/mxs_nand_spl.c +61 -52

index 0e7c364..d9d1811 100644
@@ -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

drivers/mtd/nand/mxs_nand_spl.c +24 -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 */

Review

Quoted 9 lines · Tim Harvey

(quoted diff) +3 -1

-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])?

Quoted 25 lines

(quoted diff) +24 -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 */

Review

Quoted 10 lines

(quoted diff) +10 -0

+     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?

Review

Quoted 4 lines

(quoted diff) +4 -0

+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?

Quoted 6 lines

int nand_default_bbt(struct mtd_info *mtd)

 {
      return 0;

(quoted diff) +0 -0

@@ -223,6 +232,7 @@ int nand_default_bbt(struct mtd_info *mtd)
 

Review int nand_default_bbt(struct mtd_info *mtd) · line 236

Quoted 4 lines

(quoted diff) +1 -0

 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?

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:

common/env_nand.c +4 -0

index cc7e979..97d10a2 100644
@@ -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.