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

[Scott Wood]

On Mon, 2015-03-30 at 23:26 +0200, Stefan Agner wrote:
> On 2015-03-30 22:48, Scott Wood wrote:
> > What is special about this controller, that caching makes sense here but
> > not on other controllers?  If it makes sense everywhere, then the upper
> > layer is the place to do it.
> > 
> 
> Well, its not a caching which could be handled by upper layers: The NFC
> reads the data into SRAM, where it gets hardware ECC checked. After
> that, the hardware would be capable of copy the data into main memory
> using DMA, however, the driver currently is not using this
> functionality. Instead, we use memcpy, and copy only the data which are
> requested.

The upper layer could choose to read the whole page at once.

> >> However, in order to avoid a full page getting allocated and memcpy'ed
> >> when only writing a part of a page, we actually could re-enable that
> >> feature. But I'm not sure about the semantics of a subpage writes: Does
> >> the stack makes sure that the rest of the bytes are in the cache (e.g.
> >> read before write)? From what I understand, a subpage write would only
> >> copy (via vf610_nfc_write_buf) the data required into the the page
> >> cache, which then gets written to the device. Who makes sure that the
> >> rest of the page contains sound data?
> > 
> > If the rest of the page is all 0xff it shouldn't affect the existing
> > data -- as long as the controller isn't writing some non-0xff ECC bytes
> > as a result.
> > 
> > It's moot if subpage writes are disabled, though.
> > 
> 
> Hm, and if its not 0xff? Is a sub page write valid in that case?

It's also OK if it matches what's already there.  Otherwise, it will
corrupt.

> If a subpage write means that the rest of the page is 0xff, we could
> also memset the whole SRAM 0xff and copy only the subpage data into the
> buffer. We would still need to write the whole page, but only read parts
> of it from main memory.

The generic NAND code already does this -- see the memset() in
nand_do_write_ops().  In Linux we rely on this in the eLBC driver
because it worked by accident and disabling subpages now would break UBI
compatibility.

>  OTH, when we disable subpage write, and the stack prepares the whole
> page, the page is probably still in the CPU cache anyway, and hence the
> copy operation would be nearly as fast as memset/and partly memcpy....
> Probably not worth the whole effort.

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?

> >> I think it mainly makes sense when the higher layer reads OOB and then
> >> the main data or visa-verse. I saw such access patterns at least in
> >> U-Boot.
> > 
> > Wouldn't it make more sense to not read a full page every time OOB is
> > read?
> 
> I think when ECC is enabled this is not possible. However, since OOB is
> not ECC checked, we could also turn off ECC and read only the OOB.
> However, I'm also not sure if that is supported by the controller, I
> need to check that. If it both is not possible, I would argue that
> remembering the page in SRAM is worth to effort to speed up page => OOB
> or OOB => page access...

If the controller can't support reading OOB by itself, that would be an
answer to "what is different about this controller"...  Though if
caching is done to deal with that, it should be limited to only dealing
with consecutive reads.  Don't cache writes.

-Scott