[U-Boot] [PATCH v3 2/4] sunxi: nand: Add basic sunxi NAND driver for SPL with DMA support

[Boris Brezillon]

Hi Michal,

On Fri, 31 Jul 2015 16:25:00 +0200
Michal Suchanek <hramrach at gmail.com> wrote:

> On 31 July 2015 at 11:24, Boris Brezillon
> <boris.brezillon at free-electrons.com> wrote:
> > Hi Hans,
> >
> > On Fri, 31 Jul 2015 10:36:43 +0200
> > Hans de Goede <hdegoede at redhat.com> wrote:
> >
> >> Hi,
> >>
> >> On 31-07-15 02:47, Scott Wood wrote:
> >> > On Thu, 2015-07-23 at 14:33 +0200, Piotr Zierhoffer wrote:
> >> >> +int nand_spl_load_image(uint32_t offs, unsigned int size, void *dest)
> >> >> +{
> >> >> +     void *current_dest;
> >> >> +     uint32_t count;
> >> >> +     uint32_t current_count;
> >> >> +     uint32_t ecc_errors = 0;
> >> >> +
> >> >> +     memset(dest, 0x0, size); /* clean destination memory */
> >> >> +     for (current_dest = dest;
> >> >> +                     current_dest < (dest + size);
> >> >> +                     current_dest += CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE) {
> >> >> +             nand_read_page(offs, offs
> >> >> +                             < CONFIG_NAND_SUNXI_SPL_SYNDROME_PARTITIONS_END,
> >> >> +                            &ecc_errors);
> >> >> +             count = current_dest - dest;
> >> >> +
> >> >> +             if (size - count > CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE)
> >> >> +                     current_count = CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE;
> >> >> +             else
> >> >> +                     current_count = size - count;
> >> >> +
> >> >> +             memcpy(current_dest,
> >> >> +                    temp_buf,
> >> >> +                    current_count);
> >> >> +             offs += CONFIG_NAND_SUNXI_SPL_ECC_PAGE_SIZE;
> >> >> +     }
> >> >> +     return ecc_errors ? -1 : 0;
> >> >> +}
> >> >
> >> > No bad block marker handling?
> >>
> >> The bootrom does not use bad block marker handling (and allwinner's
> >> own FTL does neither for the non boot area, the actually mess up
> >> things by writing metadata which looks like classic bad block
> >> markers).
> >
> > Hm, checking for bad block markers (and skipping bad blocks) is always a
> > good thing, even if it does not by itself guarantee that the data
> > stored in there are not corrupted.
> 
> Not on Allwinner hardware. Allwinner tools write data to the nand
> which looks like bad block markers so skipping blocks which appear
> marked as bad will inevitably skip valid blocks on many (most ?)
> Allwinner devices.

First of all, that's not exactly true. The allwinner NAND layer (and
probably the tools allowing you to flash a new image) is actually
writing the appropriate good block pattern (0xff), but this pattern is
then randomized by the hardware randomizer, which makes it look like a
bad block (!= 0xff).

The other aspect is, do we really have to support images flashed with
this tool? I mean, I'm flashing my images using fel and a standard
u-boot, and they generate perfectly valid images with valid bad block
markers.

> 
> Only in the case you soldered a new nand chip yourself and never used
> Allwinner tools with it will the bad block markers remain valid. This
> is overall very unlikely so it should not be something SPL handles.

Here you're talking about the bad block markers 'corrupted' by the
libnand layer (or the flashing tool).
To restore valid bad block markers you just have to launch nand scrub
in u-boot.
I'm also thinking about a way to retrieve the real bad blocks even when
the NAND has been used with an allwinner kernel (the idea is to first
check without the randomizer disabled, and if it does not return 0xff,
try to enable the randomizer, then if it still does not return 0xff,
it's a real bad block).

But unless we want to support both mainline and non-mainline systems
using the same SPL, that aspect is just something that should be handled
when your moving from an allwinner image to a mainline one.

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com