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

[Michal Suchanek]

On 31 July 2015 at 16:41, Boris Brezillon
<boris.brezillon at free-electrons.com> 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.
>
> 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.

If u-boot takes into account the broken bad block markers generated by
Allwinner tools it will make perfectly unusable image with invalid bad
block markers.

I tried flashing my image by booting Linux from SD card and formatting
the nand and it certainly did not go well with some past version of
the driver.

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

That would be good if it just detected bad blocks whatever format the
flash was in previously.

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

We probably want to support no-nonsense reimaging regardless of the
previous nand content. You cannot expect users to track what they used
last to flash their device.

Thanks

Michal