On Wed, 2015-03-18 at 10:04 +0100, Albert ARIBAUD (3ADEV) wrote:
> +int nand_spl_load_image(uint32_t offs, unsigned int size, void *dst)
> +{
> +     int block_good;

bool?

> +     struct lpc32xx_oob oob;
> +     unsigned int page, left;
> +
> +     /* if starting mid-block consider block good */
> +     block_good = 1;
> +
> +     for (page = offs / BYTES_PER_PAGE,
> +          left = DIV_ROUND_UP(size, BYTES_PER_PAGE);
> +          left && (page < PAGES_PER_CHIP_MAX); page++) {
> +             /* read inband and oob page data anyway */
> +             int res = read_single_page(dst, page, &oob);
> +             /* return error if a good block's page was not readable */
> +             if ((res < 0) && block_good)
> +                     return -1;

Why even try to read it if it's been marked bad?

> +             /* if first page in a block, update 'block good' status */

The non-SPL driver appears to be checking the last two pages in the
block, not the first page.

> +             if ((page % PAGES_PER_BLOCK) == 0) {
> +                     /* assume block is good */
> +                     block_good = 1;
> +                     /* if page could not be read, assume block is bad */
> +                     if (res < 0)
> +                             block_good = 0;

No.  A block is to be skipped if and only if it has a bad block marker.
ECC errors should not be silently ignored just because they happen on
the first page of a block.  If the writer of this data didn't skip the
block, then skipping it when reading will result in unusable data
regardless of the underlying ECC problem.

-Scott


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to