Re: [PATCH v4 0/2] mtd: hisilicon: add a new driver for NAND controller of hisilicon hip04 Soc

[Zhou Wang <wangzhou.bry-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

On 2014年12月17日 14:23, Brian Norris wrote:
> On Thu, Dec 11, 2014 at 08:02:27PM +0800, Zhou Wang wrote:
>> I have tested this NAND controller driver on the MTD test modules
>> (drivers/mtd/tests/*). All tests passed except mtd_nandbiterrs.ko.
>>
>> Here is the test log for mtd_nandbiterrs:
>> /home # insmod mtd_nandbiterrs.ko dev=2 page_offset=1 seed=110 mode=0
>> [  100.484995]
>> [  100.490082] ==================================================
>> [  100.509657] mtd_nandbiterrs: MTD device: 2
>> [  100.523413] mtd_nandbiterrs: MTD device size 8388608,
>> eraseblock=131072, page=2048, oob=64
>> [  100.551134] mtd_nandbiterrs: Device uses 2 subpages of 1024 bytes
>> [  100.571585] mtd_nandbiterrs: Using page=1, offset=2048, eraseblock=0
>> [  104.431136] mtd_nandbiterrs: incremental biterrors test
>> [  104.448872] mtd_nandbiterrs: write_page
>> [  104.463193] mtd_nandbiterrs: rewrite page
>> [  104.477620] mtd_nandbiterrs: read_page
>> [  104.490898] mtd_nandbiterrs: verify_page
>> [  104.504338] mtd_nandbiterrs: Successfully corrected 0 bit errors
>> per subpage
>> [  104.527985] mtd_nandbiterrs: Inserted biterror @ 0/5
>> [  104.544673] mtd_nandbiterrs: Inserted biterror @ 1024/2
>> [  104.562197] mtd_nandbiterrs: rewrite page
>> [  104.576766] mtd_nandbiterrs: read_page
>> [  104.590052] mtd_nandbiterrs: verify_page
>> [  104.603252] mtd_nandbiterrs: Error: page offset 0, expected 23, got 03
>> [  104.625203] mtd_nandbiterrs: Error: page offset 1024, expected 06, got 02
>> [  104.648056] mtd_nandbiterrs: ECC failure, read data is incorrect
>> despite read success
>> insmod: can't insert 'mtd_nandbiterrs.ko': Input/output error
>>
>> The reason for above failure is that:
>> In ECC mode, when rewriting page data to NAND flash, the NAND
>> controller will also produce ECC code and write them to NAND flash
>> as well. So when we read data from NAND flash, there is no need to
>> correct the error bit. We read what we write to the flash.
>
> BTW, your explanation doesn't seem quite right. The problem is that
> even though mtd_read() didn't report errors, the data doesn't match
> what's written. It's not that there was "no need to correct the error
> bit".

Maybe I did not express clearly. In the nandbiterrs test, firstly
write data to flash with ECC code in oob area, then change some bits
and rewrite data to flash with old ECC code in oob area, at last read
data out with ECC to test if the "error bits" can be corrected. My
explanation is that in rewriting process NAND controller also produces
new ECC code of the data and write both data and new ECC code to flash.
So in next step we will get what was writen without "correction".

>
> I'd recommend digging a little more to figure out what's wrong here. You
> might need to instrument the nandbiterrs test. This is possibly

Thanks, I will do it.

> highlighting a driver bug [1].
>
> Brian
>
> [1] Besised simply that you didn't implement write_page_raw(). The
> default nand_write_page_raw() implementation looks just like your
> non-raw version.

Yes. In ECC mode, as the NAND controller must write page as a whole
with ECC code, the default nand_write_page_raw() looks just like
non-raw version.

Thanks a lot,
Zhou

>

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html