[U-Boot] [PATCH 1/2] mtd: vf610_nfc: mark page as dirty on block erase

[Stefan Agner]

On 2015-04-03 22:15, Scott Wood wrote:
> On Fri, 2015-04-03 at 20:09 +0200, Stefan Agner wrote:
>> On 2015-04-03 01:48, Scott Wood wrote:
>> > On Tue, 2015-03-31 at 11:02 -0400, Bill Pringlemeir wrote:
>> >> On 2015-03-31 00:15, Scott Wood wrote:
>> >>
>> >> > Especially since you'd be doing one write rather than four full-page
>> >> > "partial" writes.  Surely the bottleneck here is the NAND chip itself,
>> >> > not copying data to the buffer?
>> >>
>> >> The AHB bus that the NFC controller is on is relatively slow.  Here are
>> >> some numbers from 'AN4947-vybrid-bus-architechure',
>> >>
>> >> Vybrid Cortex A5 to DDR (in CPU clocks 400/500MHz),
>> >>
>> >>    First read     Subsequent
>> >>    285            8              all caches on
>> >>    345            269            no cache, mmu
>> >>    437            371            no cache, no mmu
>> >>
>> >> The NFC is on an AHB bus 32bit, 66MHz (not AXI 64bit, 133-166MHz like
>> >> DDR).  The AHB will be about four times slower.  Also the reads and
>> >> writes to the physical NAND must take place serially.  Here are the
>> >> program page steps.
>> >>
>> >>   1. Issue controller Read full page to NFC buffer.
>> >>   2. Copy update partial page from DDR to NFC buffer.
>> >>   3. Issue write NAND page.
>> >
>> > Why is any sort of read part of the write process?
>>
>> To recalculate the correct ECC, which is done in the controller, the
>> controller has to have the page in the SRAM. I will send out a patch
>> which implements vf610_nfc_write_subpage. And the read part is done when
>> the MTD subsystem calls NAND_CMD_SEQIN.
> 
> Again, if this is the only way you can do subpage accesses then you
> should not do them.

Why not? IMHO, there are valid reason to do it, since we save coping
data over the bus (we save copying page size - write len of bytes)...
Also, I guess all NAND controller which do HW ECC need to read at least
ECC step size back to the controller... Maybe we can move the discussion
to the actual code (see "mtd: vf610_nfc: support subpage write").
 
>> Actually, the Linux NAND driver supports subpage writes already by using
>> the generic nand_write_subpage_hwecc function. However, in U-Boot I
>> added driver specific page_read/page_write due to performance reasons:
>> http://lists.denx.de/pipermail/u-boot/2014-August/186293.html
>>
>> However, I tried driver specific page_read/page_write functions for
>> Linux too, but I couldn't measure noticeable performance improvements. I
>> think the reason why it lead to noticeable improvements for U-Boot was
>> because U-Boot does not use caches so far.
> 
> If you care about U-Boot's performance at all, enabling cache would be
> the first thing I'd try...

Yes, we have that in our downstream since some months, and patch is
pending, see:
http://article.gmane.org/gmane.comp.boot-loaders.u-boot/215896

But it was not the first thing we did, we started with the functionality
needed to boot, such as reading from NAND... Hence we could now go back
and use the MTD subsystems generic functions. I do not have a strong
opinion on this...

--
Stefan