Re: [PATCH 4/5] mtd: nand: add support for Micron on-die ECC

[Boris Brezillon <boris.brezillon-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>]

On Tue, 11 Apr 2017 17:01:51 +0000
"Bean Huo (beanhuo)" <beanhuo-AL4WhLSQfzjQT0dZR+AlfA@public.gmane.org> wrote:

> >On Tue, 11 Apr 2017 15:02:22 +0000
> >"Bean Huo (beanhuo)" <beanhuo-AL4WhLSQfzjQT0dZR+AlfA@public.gmane.org> wrote:
> >  
> >> Hi, Boris and Thomas
> >> Let me do some explanation.
> >>  
> >> >> if (NAND == SLC ) { // on-die ECC only exists in SLC //check device
> >> >> ID byte 4
> >> >>      if ((ID.byte4 & 0x02) == 0x02) {// internal ECC level ==10b  
> >> >
> >> >So here the MT29F1G08ABADAWP datasheet says 0x2 <=> 4bit/512bytes ECC.
> >> >  
> >>
> >> If the NAND supports on-die ECC, here should be 10b, not matter it is
> >> 8bit or 4bit, You are correct, MT29F1G08ABADAWP is 0x2, its explanation is  
> >4bit/512bytes ECC.  
> >> But for the 70s, it is 8bit on-die ECC, but it is still 10b.
> >> So that why here using these two bits to determine if exist on-die ECC.
> >> What's more, for some old products, they don't support on-die ECC,
> >> Sometimes, here is still 01b, so still need following codes to do
> >> further determinations.  
> >
> >Okay, then here is the differentiator. Did you check that on SLC NANDs there's no
> >collision on ID[4].bits[1:0]. I've seen NAND vendors changing their ID scheme in
> >incompatible ways (old fields were replaced by new ones with completely
> >different meanings).  
> 
> 
> Yes, this is true, there is no one standard to define and formalize ID.byte4,
> It is always changing. Also, sometimes it definitely conflicts with other NAND without
> On-die ECC. For the Micron both serials SLC NAND with on-die ECC, bits[1:0] is defined
> Internal ECC level. 
> 
> >I'd really like to make sure we're not mis-interpreting READ_ID information, so
> >maybe we should restrict the test on ONFI NANDs if all NANDs supporting on-die
> >ECC are ONFI compliant. We should probably also check that chip->id.len >= 5.
> >
> >  
> >>  
> >> >> 	if (ID.byte4 & 0x80) {//on-Die ECC enabled  
> >> >
> >> >Did you read my last reply?
> >> >Thomas discovered that ID[4].bit7 is actually reflecting the ECC
> >> >engine state (1 if the engine is enabled, 0 if it's disabled), not
> >> >whether the NAND supports on-die ECC or not, so no this test is not reliable.
> >> >  
> >> For the on-die ECC, it is not always default enabled. It depends on requirement  
> >from costumers.  
> >> If on-die ECC is not enabled, bit7 is 0. It can be switched through "Feature  
> >Operations".
> >
> >So this check is not needed, right?  
> 
> Here is much complicated. One question is that what main purpose of on-die ECC.
> there are two types of usage model:
> 1.  on-die ECC default enabled:
> Normally before bootloader and kernel, there is no any ECC to correct and maintain
> Bootloader reliability.  For this kind of customer, I think, they mainly want to have reliable booting.
> Rather than for store user data. Per this kind of condition, we don't check, because on-die ECC
> Always be enabled, cannot be disabled.
> 
> 2. on-die ECC default disabled:
> I think this is used for some important user data. Unless the bootrom of CPU can issue 
> SET_FEATURE to enable on-die ECC, and until Linux running, on-die ECC is still enabled.
> Otherwise, we need to check if it enables or not.

Well, knowing whether the NAND has on-die ECC or not and determining if
it's enabled by default are 2 different things. Until now, we were
trying to detect the former.

> 
> >BTW, do you have NANDs where the on-die ECC is always enabled, and if this is
> >the case, what happens when you call
> >SET_FEATURE(disable/enable-ECC) on these NANDs?  
> 
> If this NAND is on-die ECC defaulted enabled, the on-die ECC cannot
> Disabled later. Why? This is related to specific user model.

Erf, this changes a bit what Thomas and I had in mind, because that
means read/write_page_raw() are not supported in this case, and more
importantly, that means users should by no mean enable external ECC
engines.

> We have one PPT on Micron domain website, it is "on die ECC training",
> It opens and can freely download. It clearly describes this.

Okay, I'll try to download this document.

One last question. Is it dangerous to call
SET_FEATURE(disable/enable-ECC) on a NAND that has ECC enabled by
default? We could use that to detect whether on-die ECC can be turned
off or not and adjust the chip->ecc init steps accordingly.

Thanks,

Boris
--
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