brcmnand (iproc): bitflips and ECC errors due to ignored ECC config

brcmnand (iproc): bitflips and ECC errors due to ignored ECC config

[Rafał Miłecki]

Hi,

I've a D-Link DIR-885L router with following NAND:
nand: device found, Manufacturer ID: 0x01, Chip ID: 0xf1
nand: AMD/Spansion NAND 128MiB 3,3V 8-bit
nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64

It uses ECC strength 1 with step size 512 so I created DT with:
nand-ecc-strength = <1>;
nand-ecc-step-size = <512>;

Unfortunately all I get from mtd_read are -EUCLEAN (6%) and -EBADMSG
(94%) errors. It's becase of the following code in brcmnand_setup_dev:
if (chip->ecc.strength == 1) /* Hamming */
cfg->ecc_level = 15;

After removing this special handling of strength 1 I can use my NAND
as expected.

Is there a good reason for this hamming condition? Can we add some way
of using strength 1?

-- 
Rafał

Re: brcmnand (iproc): bitflips and ECC errors due to ignored ECC config

[Rafał Miłecki]

On 25 January 2016 at 13:59, Rafał Miłecki <zajec5@gmail.com> wrote:
> I've a D-Link DIR-885L router with following NAND:
> nand: device found, Manufacturer ID: 0x01, Chip ID: 0xf1
> nand: AMD/Spansion NAND 128MiB 3,3V 8-bit
> nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
>
> It uses ECC strength 1 with step size 512 so I created DT with:
> nand-ecc-strength = <1>;
> nand-ecc-step-size = <512>;
>
> Unfortunately all I get from mtd_read are -EUCLEAN (6%) and -EBADMSG
> (94%) errors. It's becase of the following code in brcmnand_setup_dev:
> if (chip->ecc.strength == 1) /* Hamming */
> cfg->ecc_level = 15;
>
> After removing this special handling of strength 1 I can use my NAND
> as expected.
>
> Is there a good reason for this hamming condition? Can we add some way
> of using strength 1?

Ping?

-- 
Rafał

Re: brcmnand (iproc): bitflips and ECC errors due to ignored ECC config

[Kamal Dasu]

When SECTOR_SIZE = 512B, SPARE_AREA_SIZE  = 16, ECC_LEVEL = 15 enables
1-bit Hamming ECC protection. So the ecc_level field is the SoC
internal representation for 1-bit hamming. If you mean by "removing
handling" you made strength=ecc_level=1, then you are now using 1-bit
BCH.

Kamal

On Mon, Feb 8, 2016 at 4:54 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
> On 25 January 2016 at 13:59, Rafał Miłecki <zajec5@gmail.com> wrote:
>> I've a D-Link DIR-885L router with following NAND:
>> nand: device found, Manufacturer ID: 0x01, Chip ID: 0xf1
>> nand: AMD/Spansion NAND 128MiB 3,3V 8-bit
>> nand: 128 MiB, SLC, erase size: 128 KiB, page size: 2048, OOB size: 64
>>
>> It uses ECC strength 1 with step size 512 so I created DT with:
>> nand-ecc-strength = <1>;
>> nand-ecc-step-size = <512>;
>>
>> Unfortunately all I get from mtd_read are -EUCLEAN (6%) and -EBADMSG
>> (94%) errors. It's becase of the following code in brcmnand_setup_dev:
>> if (chip->ecc.strength == 1) /* Hamming */
>> cfg->ecc_level = 15;
>>
>> After removing this special handling of strength 1 I can use my NAND
>> as expected.
>>
>> Is there a good reason for this hamming condition? Can we add some way
>> of using strength 1?
>
> Ping?
>
> --
> Rafał

Re: brcmnand (iproc): bitflips and ECC errors due to ignored ECC config

[Brian Norris]

On Mon, Feb 08, 2016 at 02:15:16PM -0500, Kamal Dasu wrote:
> When SECTOR_SIZE = 512B, SPARE_AREA_SIZE  = 16, ECC_LEVEL = 15 enables
> 1-bit Hamming ECC protection. So the ecc_level field is the SoC
> internal representation for 1-bit hamming. If you mean by "removing
> handling" you made strength=ecc_level=1, then you are now using 1-bit
> BCH.

It kinda sounds like Rafal is suggesting this product uses 1-bit BCH.
I didn't even remember such a thing existed on this controller.
Unfortunately, we don't really have a way to clearly differentiate 1-bit
Hamming and 1-bit BCH in DT, do we? Both could equally well be described
by:

	nand-ecc-strength = <1>;
	nand-ecc-step-size = <512>;

Maybe we need a custom BRCM property?

Brian

Re: brcmnand (iproc): bitflips and ECC errors due to ignored ECC config

[Kamal Dasu]

Brian,

Current driver  defaults to using hamming when we see 1 bit ecc strength.

"It kinda sounds like Rafal is suggesting this product uses 1-bit BCH."

That explains the read errors, if the image that is being read back
was written with 1-bit BCH encoding.

"Unfortunately, we don't really have a way to clearly differentiate 1-bit
Hamming and 1-bit BCH in DT, do we?"

Yes hamming is a special case in that sense. Adding an optional
"ecc_algo"  field would help.
If Rafal has the ability to  create an image with BCH-2 through BCH-8
that can also work on this part. However the current issue needs to
get fixed to have 1-bit hamming along side 1-bit BCH support. And
maybe we default to using BCH, unless hamming is chosen for 1-bit.

Kamal

On Mon, Feb 8, 2016 at 2:21 PM, Brian Norris
<computersforpeace@gmail.com> wrote:
> On Mon, Feb 08, 2016 at 02:15:16PM -0500, Kamal Dasu wrote:
>> When SECTOR_SIZE = 512B, SPARE_AREA_SIZE  = 16, ECC_LEVEL = 15 enables
>> 1-bit Hamming ECC protection. So the ecc_level field is the SoC
>> internal representation for 1-bit hamming. If you mean by "removing
>> handling" you made strength=ecc_level=1, then you are now using 1-bit
>> BCH.
>
> It kinda sounds like Rafal is suggesting this product uses 1-bit BCH.
> I didn't even remember such a thing existed on this controller.
> Unfortunately, we don't really have a way to clearly differentiate 1-bit
> Hamming and 1-bit BCH in DT, do we? Both could equally well be described
> by:
>
>         nand-ecc-strength = <1>;
>         nand-ecc-step-size = <512>;
>
> Maybe we need a custom BRCM property?
>
> Brian

Re: brcmnand (iproc): bitflips and ECC errors due to ignored ECC config

[Rafał Miłecki]

On 8 February 2016 at 21:05, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
> "Unfortunately, we don't really have a way to clearly differentiate 1-bit
> Hamming and 1-bit BCH in DT, do we?"
>
> Yes hamming is a special case in that sense. Adding an optional
> "ecc_algo"  field would help.
> If Rafal has the ability to  create an image with BCH-2 through BCH-8
> that can also work on this part. However the current issue needs to
> get fixed to have 1-bit hamming along side 1-bit BCH support. And
> maybe we default to using BCH, unless hamming is chosen for 1-bit.

I can't really change the way bootloader (CFE) works and how it
flashes image to the NAND. So I'm stuck with BCH-1.

Re: brcmnand (iproc): bitflips and ECC errors due to ignored ECC config

[Kamal Dasu]

Rafal,

Thanks for pointing this out, will send a patch with the above solution.

Kamal

On Fri, Feb 12, 2016 at 7:06 AM, Rafał Miłecki <zajec5@gmail.com> wrote:
> On 8 February 2016 at 21:05, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
>> "Unfortunately, we don't really have a way to clearly differentiate 1-bit
>> Hamming and 1-bit BCH in DT, do we?"
>>
>> Yes hamming is a special case in that sense. Adding an optional
>> "ecc_algo"  field would help.
>> If Rafal has the ability to  create an image with BCH-2 through BCH-8
>> that can also work on this part. However the current issue needs to
>> get fixed to have 1-bit hamming along side 1-bit BCH support. And
>> maybe we default to using BCH, unless hamming is chosen for 1-bit.
>
> I can't really change the way bootloader (CFE) works and how it
> flashes image to the NAND. So I'm stuck with BCH-1.

Re: brcmnand (iproc): bitflips and ECC errors due to ignored ECC config

[Rafał Miłecki]

On 12 February 2016 at 17:16, Kamal Dasu <kdasu.kdev@gmail.com> wrote:
> Thanks for pointing this out, will send a patch with the above solution.

Oh, I already started working on this. Let me send patches so you can
see if you like it.