mbox series

[0/2] mtd: rawnand: micron: Fix on-die ECC

Message ID 20180709210937.30150-1-boris.brezillon@bootlin.com
Headers show
Series mtd: rawnand: micron: Fix on-die ECC | expand

Message

Boris Brezillon July 9, 2018, 9:09 p.m. UTC
Chris, Bean,

Here are 2 patches for you to review/test. The first one is fixing
the layout definition, and unless I missed something it should be
correct.

The second one is just my understanding of how byte 5 of READ_ID works
based on our experience with the 4bit/512 on-die ECC chip we have
worked on and the other datasheet I had a look at.
I'm not 100% sure this will work for all chips, but might work for the
2 chips we support right now.
So please test and/or review it and let me know if this approach works.

Regards,

Boris

Boris Brezillon (2):
  mtd: rawnand: micron: Define the proper layout for 8bit/512bytes
    on-die ECC
  mtd: rawnand: micron: Fix on-die ECC detection logic

 drivers/mtd/nand/raw/nand_micron.c | 110 +++++++++++++++++++++++++------------
 1 file changed, 75 insertions(+), 35 deletions(-)

Comments

Boris Brezillon July 10, 2018, 8:10 a.m. UTC | #1
On Mon,  9 Jul 2018 23:09:35 +0200
Boris Brezillon <boris.brezillon@bootlin.com> wrote:

> Chris, Bean,
> 
> Here are 2 patches for you to review/test. The first one is fixing
> the layout definition, and unless I missed something it should be
> correct.
> 
> The second one is just my understanding of how byte 5 of READ_ID works
> based on our experience with the 4bit/512 on-die ECC chip we have
> worked on and the other datasheet I had a look at.
> I'm not 100% sure this will work for all chips, but might work for the
> 2 chips we support right now.

I tested it on a MT29F2G08ABAEAH4 (4bit/512bytes on-die ECC) and it
works as expected. When I do the SET_FEATURES(ECC_EN) the ECC enabled
bit in READID byte 5 is set, and when I do SET_FEATURES(ECC_DIS), the
bit is cleared.

Now we need to make sure it works correctly on MT29F1G08ABAFAWP.

> So please test and/or review it and let me know if this approach works.
> 
> Regards,
> 
> Boris
> 
> Boris Brezillon (2):
>   mtd: rawnand: micron: Define the proper layout for 8bit/512bytes
>     on-die ECC
>   mtd: rawnand: micron: Fix on-die ECC detection logic
> 
>  drivers/mtd/nand/raw/nand_micron.c | 110 +++++++++++++++++++++++++------------
>  1 file changed, 75 insertions(+), 35 deletions(-)
>
Chris Packham July 10, 2018, 9:40 p.m. UTC | #2
Hi Boris,

On 10/07/18 20:11, Boris Brezillon wrote:
> On Mon,  9 Jul 2018 23:09:35 +0200
> Boris Brezillon <boris.brezillon@bootlin.com> wrote:
> 
>> Chris, Bean,
>>
>> Here are 2 patches for you to review/test. The first one is fixing
>> the layout definition, and unless I missed something it should be
>> correct.
>>
>> The second one is just my understanding of how byte 5 of READ_ID works
>> based on our experience with the 4bit/512 on-die ECC chip we have
>> worked on and the other datasheet I had a look at.
>> I'm not 100% sure this will work for all chips, but might work for the
>> 2 chips we support right now.
> 
> I tested it on a MT29F2G08ABAEAH4 (4bit/512bytes on-die ECC) and it
> works as expected. When I do the SET_FEATURES(ECC_EN) the ECC enabled
> bit in READID byte 5 is set, and when I do SET_FEATURES(ECC_DIS), the
> bit is cleared.
> 
> Now we need to make sure it works correctly on MT29F1G08ABAFAWP.

I'll test that as soon as I can get access to the hardware (probably 
Monday next week).

> 
>> So please test and/or review it and let me know if this approach works.
>>
>> Regards,
>>
>> Boris
>>
>> Boris Brezillon (2):
>>    mtd: rawnand: micron: Define the proper layout for 8bit/512bytes
>>      on-die ECC
>>    mtd: rawnand: micron: Fix on-die ECC detection logic
>>
>>   drivers/mtd/nand/raw/nand_micron.c | 110 +++++++++++++++++++++++++------------
>>   1 file changed, 75 insertions(+), 35 deletions(-)
>>
> 
>
Chris Packham July 16, 2018, 8:07 a.m. UTC | #3
On 10/07/18 09:09, Boris Brezillon wrote:
> Chris, Bean,
> 
> Here are 2 patches for you to review/test. The first one is fixing
> the layout definition, and unless I missed something it should be
> correct.
> 
> The second one is just my understanding of how byte 5 of READ_ID works
> based on our experience with the 4bit/512 on-die ECC chip we have
> worked on and the other datasheet I had a look at.
> I'm not 100% sure this will work for all chips, but might work for the
> 2 chips we support right now.
> So please test and/or review it and let me know if this approach works.
> 

Hi Boris,

I went to test this today and found that I could no-longer mount the 
Micron MT29F1G08ABAFAWP-ITE chip. However it doesn't appear to these 
changes but d73e5d29 ("mtd: rawnand: micron: Disable ECC earlier in the 
read path").

   [root@linuxbox ~]# ubiformat -q -y /dev/mtd0
   [root@linuxbox ~]# ubiattach -p /dev/mtd0
   ubi0: attaching mtd0
   ubi0: scanning is finished
   ubi0 error: ubi_read_volume_table: the layout volume was not found
   ubi0 error: ubi_attach_mtd_dev: failed to attach mtd0, error -22
   ubiattach: error!: cannot attach "/dev/mtd0"
              error 22 (Invalid argument)
   [root@linuxbox ~]# ubimkvol /dev/ubi0 -N user -m
   libubi: error!: cannot get information about "/dev/ubi0"
           error 2 (No such file or directory)
   ubimkvol: error!: error while probing "/dev/ubi0"
             error 2 (No such file or directory)

I'll try juggling some of the nand/next patches around to see if I can 
test these two without d73e5d29.