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

[Scott Wood]

On Fri, 2015-07-31 at 16:41 +0200, Boris Brezillon wrote:
> 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.

So, how do you prevent the bad block check from happening in non-SPL NAND 
code?

> 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.

Are you talking about using scrub only on blocks that read as valid when 
unrandomized?  Does the tool that writes these randomized images skip blocks 
that are marked bad?

-Scott