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

[Liang Yang]

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