[U-Boot] [PATCH 0/5] introduce nand write.ubi, and drop ffs for jffs2 too

[Ben Gardiner]

Hi Detlev, Artem,

I'm very sorry that I haven't been back to reply in over a week.
Please accept my apologies.

On Fri, Apr 29, 2011 at 7:58 AM, Detlev Zundel <dzu at denx.de> wrote:
> Hi Ben,
>
>> It was found that on da850evm, where the NAND ECC used does not map all 0xff
>> data to 0xff ECC, that flashing UBI and JFFS2 image from U-boot with nand
>> write[.e] command resulted in alot of ECC errors... for UBI the result was
>> an unmountable filesystem on second attach from linux. For JFFS2 the result was
>> a multitude of ECC errors printed on the cosole on the second mount in Linux --
>> the filesystem remains mountable for awhile but eventually collapses.
>
> I am not sure that I can follow you here so I have to ask you to clarify
> the problem for me.
>
> I understand that a page of 0xffs does _not_ have an ECC of all 0xffs.
> Actually I would be surprised if there was any ECC having this property,
> so I doubt that this is da850 specific, correct?

Thanks for the review, Detlev. I don't have enough experience with
NAND devices or controllers to say whether this property is unique or
not. I trust that yours is sufficient to say so. I think I will remove
this note from the cover letter in v2.

On Fri, Apr 29, 2011 at 9:59 AM, Artem Bityutskiy <dedekind1 at gmail.com> wrote:
>> [...]
>> Can you please enlighten me?
>
> I do not know why I'm in CC, but I see that the code to skip all 0xFFs
> looks like it was copied from UBI. The reason why UBI and UBI user-space
> tools skip NAND pages with all 0xFFs when writing is described here:
>
> http://www.linux-mtd.infradead.org/doc/ubi.html#L_flasher_algo

Artem, thank you for picking up my slack and answering Detlev's questions.

Yes, the drop_ffs() implementation is copied from ubiformat utility.
I'll try to make this clearer in the commit  messages and cover
letters for v2.

On Mon, May 2, 2011 at 9:14 AM, Detlev Zundel <dzu at denx.de> wrote:
> [...]
>>> Can you please enlighten me?
>> [...]
>> If it is too long to read, in short:
>>
>> 1. if we write to flash, we always write to an erased eraseblock,
>> obviously :-)
>
> Yes - I simply wasn't sure if the software layers below (in U-Boot)
> would do an "erase on demand" before writing.  Reading the code, this
> isn't the case (I should have known this), so the skipping should really
> be safe.
>
>> 2. erased erase block contains all 0xFFs, so skipping 0xFF NAND pages is
>> harmless.
>> 3. writing 0xFF pages has side-effects - the ECC bytes in OOB will be
>> used
>> 4. If we are flashing an UBIFS image, UBIFS will use the half-filled
>> eraseblocks, and if the "free" pages were written with 0xFFs, they'll
>> become unusable.
>>
>> So we skip 0xFF pages at the end. Not all, only the last ones. E.g., if
>> your eraseblock consists of 4 pages, and you write "data, 0xFFs, data,
>> 0xFFs", then we'll only write "data, 0xFFs, data", so that the last NAND
>> page can be used later.
>>
>> Sorry for my poor English, and I'm writing in a hurry - have to have to
>> a pub to farewell several colleagues who are leaving the company :)
>
> Thanks for your input.
>
> So I still fail to see where the ECC errors come from.  The closes thing
> that makes sense for me, is that Bens problem very likely wasn't ECC
> connected at all but the result of the "not capable to write twice".
> I.e. his NAND flashes cannot be written to twice.  When he flashed the
> images in U-Boot, there were trailing empty blocks that got programmed
> and UBIFS assumed that it _could_ write to them so it tried and failed
> and somehow got tripped up over this.
>
> If this is the case then we should change the commit message to point to
> the real problem that this patch fixes.

I am planning to respin v2 next week; I will 1) remove mention of the
ECC mapping 0xff 2) add a link to the faq entry provided by Artem and
3) make clear the origin of the drop_ffs function.

I will also rename to nand write.trimffs, thanks for suggestion.

How do you feel about making the write.jffs2 function use trimffs as
in the last patch of the series?

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca