Re: NAND parameter page read fails with bitflip and no column change support

[Miquel Raynal <miquel.raynal@bootlin.com>]

Hello,

ada@thorsis.com wrote on Thu, 11 Apr 2024 10:52:48 +0200:

> Hello Steven, Miquel,
> 
> Am Wed, Apr 10, 2024 at 06:45:58PM +0000 schrieb Steven Seeger:
> > >I had reached out to Miquel privately with this issue, only due to the desire to
> > >help inform maintainers of the issue. I was too busy at the time to join the list
> > >and write up something more formal. Miquel provided a small patch for me
> > >that did not work at the time, and then I lost access to the board for a while.
> > >Now I have a little time and a board, so I am here to report the issue and make it public.  
> > 
> > I tried the patch from Miquel back in February and it works correctly with 6.6.12
> > and my older driver without column change support.
> > 
> > Here is h is patch for reference:
> > 
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -2123,7 +2123,7 @@ EXPORT_SYMBOL_GPL(nand_reset_op);
> >  int nand_read_data_op(struct nand_chip *chip, void *buf, unsigned int len,
> >                       bool force_8bit, bool check_only)
> >  {
> > -       if (!len || !buf)
> > +       if (!len || (!check_only && !buf))
> >                 return -EINVAL;
> >  
> >         if (nand_has_exec_op(chip)) {  
> 
> Looking at the context aka the whole function nand_read_data_op() this
> looks fine.  I had reported the same issue in March, referencing the
> same commit 9f820fc0651c ("mtd: rawnand: Check the data only read
> pattern only once") you mentioned in your other mail:
> 
> Link: https://lore.kernel.org/linux-mtd/20240307-pantry-deceit-78ce20f47899@thorsis.com/
> 
> With nand_read_data_op() fixed in nand_onfi_detect() the boolean
> `use_datain` is true now instead of false (because
> rawnand_check_data_only_read_support() calls that function, too).  So
> in nand_onfi_detect() code jumps to nand_read_data_op() in the second
> loop run now instead of nand_change_read_column_op() which probably
> masks that other problem with mtd->writesize not set mentioned in our
> earlier discussion.
> 
> Long story short: the diff above in itself seems correct to me.  A
> test with that fix applied on our sam9x60 board using
> nand_soft_waitrdy instead of RB# gpio successfully gets ONFI param
> page 1 with nand_read_data_op() after getting ONFI param page 0 with
> nand_read_param_page_op() failed.
> 
> You said this is a patch/diff from Miquel.  Miquel do you send this
> as a proper patch then, or should someone else?
> 
> Greets
> Alex
> 
> P.S.: still busy with other things, no time yet for proper timing
> analysis of our other NAND flash problems with logic analyzer etc.
> 

Sorry for the delay, here is the proposal for fixing this mess:
https://lore.kernel.org/linux-mtd/20240507160546.130255-1-miquel.raynal@bootlin.com/T/#t

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/