Re: [U-Boot-Users] [PATCH] JFFS2 command support on OneNAND

On Sat, Jul 12, 2008 at 6:43 AM, Scott Wood scottwood@freescale.com wrote:

On Mon, Jul 07, 2008 at 11:22:56AM +0900, Kyungmin Park wrote:

...

+static int part_validate_onenand(struct mtdids *id, struct part_info *part) +{ +#if defined(CONFIG_CMD_ONENAND)

• /* info for OneNAND chips */
  

• struct mtd_info *mtd;
  

• mtd = &onenand_mtd;
  

• if ((unsigned long)(part->offset) % mtd->erasesize) {
  

•         printf("%s%d: partition (%s) start offset"
  

•                 "alignment incorrect\n",
  

•                         MTD_DEV_TYPE(id->type), id->num, part->name);
  

•         return 1;
  

• }
  

• if (part->size % mtd->erasesize) {
  

•         printf("%s%d: partition (%s) size alignment incorrect\n",
  

•                         MTD_DEV_TYPE(id->type), id->num, part->name);
  

•         return 1;
  

• }
  

• return 0;
  

+#else

• return 1;
  

+#endif +}

This looks like a duplicate of part_validate_nand (note that nand_info_t is just an obfuscatory alias of struct mtd_info).

...

+static int read_onenand_cached(u32 off, u32 size, u_char *buf) +{

• u32 bytes_read = 0;
  

• size_t retlen;
  

• int cpy_bytes;
  

• while (bytes_read < size) {
  

•         if ((off + bytes_read < onenand_cache_off) ||
  

•             (off + bytes_read >= onenand_cache_off + ONENAND_CACHE_SIZE)) {
  

•                 onenand_cache_off = (off + bytes_read) & ONENAND_PAGE_MASK;
  

•                 if (!onenand_cache) {
  

•                         /* This memory never gets freed but 'cause
  

•                            it's a bootloader, nobody cares */
  

•                         onenand_cache = malloc(ONENAND_CACHE_SIZE);
  

•                         if (!onenand_cache) {
  

•                                 printf("read_onenand_cached: can't alloc cache size %d bytes\n",
  

•                                        ONENAND_CACHE_SIZE);
  

•                                 return -1;
  

•                         }
  

•                 }
  

•                 retlen = ONENAND_CACHE_SIZE;
  

•                 if (onenand_read(&onenand_mtd, onenand_cache_off, retlen,
  

•                                         &retlen, onenand_cache) != 0 ||
  

•                                 retlen != ONENAND_CACHE_SIZE) {
  

•                         printf("read_onenand_cached: error reading nand off %#x size %d bytes\n",
  

•                                 onenand_cache_off, ONENAND_CACHE_SIZE);
  

•                         return -1;
  

•                 }
  

•         }
  

•         cpy_bytes = onenand_cache_off + ONENAND_CACHE_SIZE - (off + bytes_read);
  

•         if (cpy_bytes > size - bytes_read)
  

•                 cpy_bytes = size - bytes_read;
  

•         memcpy(buf + bytes_read,
  

•                onenand_cache + off + bytes_read - onenand_cache_off,
  

•                cpy_bytes);
  

•         bytes_read += cpy_bytes;
  

• }
  

• return bytes_read;
  

+}

I really would rather not duplicate all of this, which looks extremely similar to regular NAND. Is there reason why we don't use the mtd_info function pointer interface?

Agreed, It's almost same as NAND code. Now nand code uses two modes, legacy and mtd. Because I don't want to break the NAND code , I used the duplicated code. Basically it should be used the common mtd style code except legacy.

So first it added the current code, next time it tries to use the common interface and some code clean up.

Thank you, Kyungmin Park