Re: [PATCH v1 4/5] mtd: rawnand: meson: clear OOB buffer before read

[Liang Yang <liang.yang@amlogic.com>]

Hi,

On 2023/4/12 18:51, Liang Yang wrote:
>>>>>> diff --git a/drivers/mtd/nand/raw/meson_nand.c 
>>>>>> b/drivers/mtd/nand/raw/meson_nand.c
>>>>>> index f84a10238e4d..f2f2472cb511 100644
>>>>>> --- a/drivers/mtd/nand/raw/meson_nand.c
>>>>>> +++ b/drivers/mtd/nand/raw/meson_nand.c
>>>>>> @@ -858,9 +858,12 @@ static int meson_nfc_read_page_sub(struct 
>>>>>> nand_chip *nand,
>>>>>>   static int meson_nfc_read_page_raw(struct nand_chip *nand, u8 *buf,
>>>>>>                      int oob_required, int page)
>>>>>>   {
>>>>>> +    struct mtd_info *mtd = nand_to_mtd(nand);
>>>>>>       u8 *oob_buf = nand->oob_poi;
>>>>>>       int ret;
>>>>>> +    memset(oob_buf, 0, mtd->oobsize);
>>>>>
>>>>> I'm surprised raw reads do not read the entire OOB?
>>>>
>>>> Yes! Seems in case of raw access (what i see in this driver) number 
>>>> of OOB bytes read
>>>> still depends on ECC parameters: for each portion of data covered 
>>>> with ECC code we can
>>>> read it's ECC code and "user bytes" from OOB - it is what i see by 
>>>> dumping DMA buffer by
>>>> printk(). For example I'm working with 2K NAND pages, each page has 
>>>> 2 x 1K ECC blocks.
>>>> For each ECC block I have 16 OOB bytes which I can access by 
>>>> read/write. Each 16 bytes
>>>> contains 2 bytes of user's data and 14 bytes ECC codes. So when I 
>>>> read page in raw mode
>>>> controller returns 32 bytes (2 x (2 + 14)) of OOB. While OOB is 
>>>> reported as 64 bytes.
>>>
>>> In all modes, when you read OOB, you should get the full OOB. The fact
>>> that ECC correction is enabled or disabled does not matter. If the NAND
>>> features OOB sections of 64 bytes, you should get the 64 bytes.
>>>
>>> What happens sometimes, is that some of the bytes are not protected
>>> against bitflips, but the policy is to return the full buffer.
>>
>> Ok, so to clarify case for this NAND controller:
>> 1) In both ECC and raw modes i need to return the same raw OOB data 
>> (e.g. user bytes
>>     + ECC codes)?
>> 2) If I have access to only 32 bytes of OOB (in case above), I must 
>> report that size
>>     of OOB is only 32 bytes during initialization?
>>
>> Thanks, Arseniy
> 
> Yes. it should return all the OOB data. i make a mistake on raw read and 
> there is wrong code in meson_nfc_read_page_raw().
>      meson_nfc_get_data_oob(nand, buf, oob_buf);
> changed to:
>      if (oob_required)
>          memcpy(oob_buf, buf + mtd->writesize, mtd->oobsize)

Sorry, please ignore this. the previous code is right.

the controller changes the layout of one page; the physical layout is 
2048 main data + 64 oob data. after writing into NAND page, it is stored
like this: 1024 main data + 2 user bytes + 14 ECC parity bytes + 1024 
main data + 2 user bytes + 14 ECC parity bytes. so that is right we only 
get 4 user bytes and 28 ECC parity bytes, total 32 bytes. that is the 
behavior of the controller that transferring one ECC page(1KB) brings 
back only 2 user bytes.

because layout is changed by controller, so go back to the function. 
meson_nfc_get_data_oob(nand, buf, oob_buf) try to get the right user and 
ecc parity bytes from the right pos. after that, the other oob bytes is 
not reading from NAND flash.


-- 
Thanks,
Liang

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic