[U-Boot] JFFS2 Loading Error on non 8k blocksize NAND [DEMO PATCH 1/1]

Hello everyone. Well, this is my first post on the list and its to announce a small bug that I've found when using JFFS2 on NAND in UBoot.
This issue was seen only once volume production was started on a new device. However, its a simple fix and I'm including my temporary patch for it at the end
Basically, when using the "fsload" command to read our kernel from NAND flash in preparation for boot, a small percentage of our partitions were displaying a CRC error on boot. Upon investigation, I noticed the fsload application skipping over bad erase blocks of 8k in size. This is to be expected, except that our NAND flash has 128k block sizes. In certain cases, we get a bad eraseblock in just the wrong location that then causes us to read invalid information for a kernel image.
Upon tracking this down, it appears that the jffs2_1pass.c is using older system configuration variables instead of the newer CONFIG_SYS_ prefixed variables. It also had the Page size hard coded to 512. Included is my patch that makes the code functional for properly-configured boards with new code, but its a demo only as this code probably needs a little bit of refactoring and cleanup so that its more generic in nature.
--------------------- Patch Snip --------------------- diff -ruN u-boot/fs/jffs2/jffs2_1pass.c uboot/fs/jffs2/jffs2_1pass.c --- u-boot/fs/jffs2/jffs2_1pass.c 2009-12-15 14:20:33.000000000 -0600 +++ uboot/fs/jffs2/jffs2_1pass.c 2009-12-15 14:19:27.000000000 -0600 @@ -158,12 +158,12 @@ * */
-#define NAND_PAGE_SIZE 512 +#define NAND_PAGE_SIZE CONFIG_SYS_NAND_PAGE_SIZE #define NAND_PAGE_SHIFT 9 #define NAND_PAGE_MASK (~(NAND_PAGE_SIZE-1))
#ifndef NAND_CACHE_PAGES -#define NAND_CACHE_PAGES 16 +#define NAND_CACHE_PAGES CONFIG_SYS_NAND_PAGE_COUNT #endif #define NAND_CACHE_SIZE (NAND_CACHE_PAGES*NAND_PAGE_SIZE)
----------------------------------------------------------

Dear Hunter Cobbs,
In message ad84a70a0912151341h115e4c59n9494caef776ea92b@mail.gmail.com you wrote:
Hello everyone. Well, this is my first post on the list and its to announce a small bug that I've found when using JFFS2 on NAND in UBoot.
Thanks.
diff -ruN u-boot/fs/jffs2/jffs2_1pass.c uboot/fs/jffs2/jffs2_1pass.c
Ideally you would use git to track source code changes, and then use "git format-patch" to extract a patch from the git repository and "git send-email" to submit it to the mailing list.
In any case, we need your Signed-off-by: line for such a submission.
--- u-boot/fs/jffs2/jffs2_1pass.c 2009-12-15 14:20:33.000000000 -0600 +++ uboot/fs/jffs2/jffs2_1pass.c 2009-12-15 14:19:27.000000000 -0600 @@ -158,12 +158,12 @@
*/
-#define NAND_PAGE_SIZE 512 +#define NAND_PAGE_SIZE CONFIG_SYS_NAND_PAGE_SIZE #define NAND_PAGE_SHIFT 9
If you change the definition of NAND_PAGE_SIZE, then the value of NAND_PAGE_SHIFT makes no longer sense. Having a close look it is not used anywhere in the code, so I recommend to simply delete this line. While doing this, please also delete the (likewise unsued) definition of ONENAND_PAGE_SHIFT.
#define NAND_PAGE_MASK (~(NAND_PAGE_SIZE-1))
#ifndef NAND_CACHE_PAGES -#define NAND_CACHE_PAGES 16 +#define NAND_CACHE_PAGES CONFIG_SYS_NAND_PAGE_COUNT #endif #define NAND_CACHE_SIZE (NAND_CACHE_PAGES*NAND_PAGE_SIZE)
I think here we should remove the "#ifndef NAND_CACHE_PAGES" / "#endif" lines and change all remaining definitions of NAND_CACHE_PAGES in old board config files (include/configs/CATcenter.h, include/configs/PPChameleonEVB.h, include/configs/NC650.h, and include/configs/SIMPC8313.h) into CONFIG_SYS_NAND_PAGE_COUNT.
Scott, what do you think?
Best regards,
Wolfgang Denk

On Mon, Dec 21, 2009 at 09:37:02PM +0100, Wolfgang Denk wrote:
--- u-boot/fs/jffs2/jffs2_1pass.c 2009-12-15 14:20:33.000000000 -0600 +++ uboot/fs/jffs2/jffs2_1pass.c 2009-12-15 14:19:27.000000000 -0600 @@ -158,12 +158,12 @@
*/
-#define NAND_PAGE_SIZE 512 +#define NAND_PAGE_SIZE CONFIG_SYS_NAND_PAGE_SIZE #define NAND_PAGE_SHIFT 9
If you change the definition of NAND_PAGE_SIZE, then the value of NAND_PAGE_SHIFT makes no longer sense. Having a close look it is not used anywhere in the code, so I recommend to simply delete this line. While doing this, please also delete the (likewise unsued) definition of ONENAND_PAGE_SHIFT.
Also note that the page size is sometimes determined dynamically -- not all boards define CONFIG_SYS_NAND_PAGE_SIZE, and it's only used by some NAND boot code.
If this code really needs to know the page size, it should use mtd->writesize -- but it looks like it doesn't use it for anything other than calculating the size of the cache.
#define NAND_PAGE_MASK (~(NAND_PAGE_SIZE-1))
#ifndef NAND_CACHE_PAGES -#define NAND_CACHE_PAGES 16 +#define NAND_CACHE_PAGES CONFIG_SYS_NAND_PAGE_COUNT #endif #define NAND_CACHE_SIZE (NAND_CACHE_PAGES*NAND_PAGE_SIZE)
I think here we should remove the "#ifndef NAND_CACHE_PAGES" / "#endif" lines and change all remaining definitions of NAND_CACHE_PAGES in old board config files (include/configs/CATcenter.h, include/configs/PPChameleonEVB.h, include/configs/NC650.h, and include/configs/SIMPC8313.h) into CONFIG_SYS_NAND_PAGE_COUNT.
This doesn't seem to be CONFIG_SYS material, but rather a software choice of how large a cache for JFFS2 to create.
It should perhaps be renamed something like CONFIG_JFFS2_NAND_CACHE_SIZE -- and maybe specified in terms of bytes rather than number of pages? If it even needs to be tunable at all...
-Scott
participants (3)
-
Hunter Cobbs
-
Scott Wood
-
Wolfgang Denk