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

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

[Hunter Cobbs]

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)

----------------------------------------------------------

-- 
Hunter Cobbs

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

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Where shall I begin, please your Majesty?" he asked. "Begin  at  the
beginning,"  the  King said, gravely, "and go on till you come to the
end: then stop."    - Alice's Adventures in Wonderland, Lewis Carroll

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

[Scott Wood]

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