Re: [PATCH] Newly erased page read workaround

[Vipin Kumar <vipin.kumar@st.com>]

On 3/31/2011 7:21 PM, Artem Bityutskiy wrote:
> Hi,
> 

Hello Artem,

> sorry for late reply, was too busy and also missed your patch somehow.
> 
> On Thu, 2011-02-24 at 11:40 +0530, Viresh Kumar wrote:
>> From: Vipin Kumar <vipin.kumar@st.com>
>>
>> A newly erased page contains ff in data as well as spare area. While reading an
>> erased page, the read out ecc from spare area does not match the ecc generated
>> by fsmc ecc hardware accelarator. This is because ecc of data ff ff is not ff
>> ff. This leads to errors when jffs2 fs erases and reads back the pages to
>> ensure consistency.
>>
>> This patch adds a software workaround to ensure that the ecc check is not
>> performed for erased pages. An erased page is checked by checking data as ff ff.
>>
>> Signed-off-by: Vipin Kumar <vipin.kumar@st.com>
>> Signed-off-by: Viresh Kumar <viresh.kumar@st.com>
>> Acked-by: Linus Walleij <linus.walleij@stericsson.com>
> 
> ...
> 
>> +		/*
>> +		 * This is a temporary erase check. A newly erased page read
>> +		 * would result in an ecc error because the oob data is also
>> +		 * erased to FF and the calculated ecc for an FF data is not
>> +		 * FF..FF.
>> +		 * This is a workaround to skip performing correction in case
>> +		 * data is FF..FF
>> +		 */
>> +		for (k = 0; k < eccsize; k++) {
>> +			if (*(p + k) != 0xff)
>> +				break;
>> +		}
>> +
> 
> So eccsize is 512, and you are going to compare 512 bytes on every read?
> Isn't it a very bad and wasteful solution? May be you could invent
> something less straight-forward but more optimal?
> 
> Let's suppose ECC corresponding to all 0xFFs is P. May be you could do
> this wasteful check only if ECC == P, not all the time?
> 

Basically, the hardware only reports the flipped bit offset and not the actual ecc 
at the time of reading. It reports ecc only at the time of writing. So, comparing 
ECCs is not a possibility while reading.

The other solution can be to write P into OOB every time a page is erased. In that 
case we put a hook in erase. But since the read implementation was anyway specific 
for fsmc bch ecc4 scheme, this implementation was more convinient...

> Also, Ivan pointed you the right thing - you might have bit-flips on an
> erased eraseblock. If not on freshly, then on an erasblock which was
> erased and then not used for long time. If this is not of your concern,

In that case an ecc error would be reported since the ecc wont match the stored 
ecc i.e FFFF and the driver would mark it as a normal corrupted page

> then at least write this in the comments and tell that you know about
> this issue but for reasons X and Y you do not handle them.
> 

OK. I would add this in the patch comments

> Thanks!
> 

Regards
Vipin