public inbox for linux-mtd@lists.infradead.org
 help / color / mirror / Atom feed
From: "ada@thorsis.com" <ada@thorsis.com>
To: Steven Seeger <steven.seeger@flightsystems.net>
Cc: "linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	"ada@thorsis.com" <ada@thorsis.com>,
	Miquel Raynal <miquel.raynal@bootlin.com>,
	"richard@nod.at" <richard@nod.at>
Subject: Re: NAND parameter page read fails with bitflip and no column change support
Date: Thu, 11 Apr 2024 10:52:48 +0200	[thread overview]
Message-ID: <20240411-candied-plod-8e55373d63cd@thorsis.com> (raw)
In-Reply-To: <DM6PR05MB4506C6DFD7ED777C5896B83BF7062@DM6PR05MB4506.namprd05.prod.outlook.com>

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.


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

  reply	other threads:[~2024-04-11  8:53 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <mailman.25709.1712772710.1280.linux-mtd@lists.infradead.org>
2024-04-10 18:45 ` NAND parameter page read fails with bitflip and no column change support Steven Seeger
2024-04-11  8:52   ` ada [this message]
2024-05-07 16:08     ` Miquel Raynal
2024-04-10 18:11 Steven Seeger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240411-candied-plod-8e55373d63cd@thorsis.com \
    --to=ada@thorsis.com \
    --cc=linux-mtd@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --cc=steven.seeger@flightsystems.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox