All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>
Cc: linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/4] mtd: nand: mxc_nand: disable subpage reads
Date: Thu, 18 Apr 2024 08:48:08 +0200	[thread overview]
Message-ID: <ZiDCKGlG4MZ23Tqo@pengutronix.de> (raw)
In-Reply-To: <20240417-mtd-nand-mxc-nand-exec-op-v1-4-d12564fe54e9@pengutronix.de>

On Wed, Apr 17, 2024 at 09:13:31AM +0200, Sascha Hauer wrote:
> The NAND core enabled subpage reads when a largepage NAND is used with
> SOFT_ECC. The i.MX NAND controller doesn't support subpage reads, so
> clear the flag again.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mtd/nand/raw/mxc_nand.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/mxc_nand.c b/drivers/mtd/nand/raw/mxc_nand.c
> index f44c130dca18d..19b46210bd194 100644
> --- a/drivers/mtd/nand/raw/mxc_nand.c
> +++ b/drivers/mtd/nand/raw/mxc_nand.c
> @@ -1667,6 +1667,8 @@ static int mxcnd_probe(struct platform_device *pdev)
>  	if (err)
>  		goto escan;
>  
> +	this->options &= ~NAND_SUBPAGE_READ;
> +

Nah, it doesn't work like this. It turns out the BBT is read using
subpage reads before we can disable them here.

This is the code in nand_scan_tail() we stumble upon:

	/* Large page NAND with SOFT_ECC should support subpage reads */
	switch (ecc->engine_type) {
	case NAND_ECC_ENGINE_TYPE_SOFT:
		if (chip->page_shift > 9)
			chip->options |= NAND_SUBPAGE_READ;
		break;

	default:
		break;
	}

So the code assumes subpage reads are ok when SOFT_ECC is in use, which
in my case is not true. I guess some drivers depend on the
NAND_SUBPAGE_READ bit magically be set, so simply removing this code is
likely not an option.  Any ideas what to do?

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

WARNING: multiple messages have this Message-ID (diff)
From: Sascha Hauer <s.hauer@pengutronix.de>
To: Miquel Raynal <miquel.raynal@bootlin.com>,
	Richard Weinberger <richard@nod.at>,
	Vignesh Raghavendra <vigneshr@ti.com>
Cc: linux-mtd@lists.infradead.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/4] mtd: nand: mxc_nand: disable subpage reads
Date: Thu, 18 Apr 2024 08:48:08 +0200	[thread overview]
Message-ID: <ZiDCKGlG4MZ23Tqo@pengutronix.de> (raw)
In-Reply-To: <20240417-mtd-nand-mxc-nand-exec-op-v1-4-d12564fe54e9@pengutronix.de>

On Wed, Apr 17, 2024 at 09:13:31AM +0200, Sascha Hauer wrote:
> The NAND core enabled subpage reads when a largepage NAND is used with
> SOFT_ECC. The i.MX NAND controller doesn't support subpage reads, so
> clear the flag again.
> 
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/mtd/nand/raw/mxc_nand.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/nand/raw/mxc_nand.c b/drivers/mtd/nand/raw/mxc_nand.c
> index f44c130dca18d..19b46210bd194 100644
> --- a/drivers/mtd/nand/raw/mxc_nand.c
> +++ b/drivers/mtd/nand/raw/mxc_nand.c
> @@ -1667,6 +1667,8 @@ static int mxcnd_probe(struct platform_device *pdev)
>  	if (err)
>  		goto escan;
>  
> +	this->options &= ~NAND_SUBPAGE_READ;
> +

Nah, it doesn't work like this. It turns out the BBT is read using
subpage reads before we can disable them here.

This is the code in nand_scan_tail() we stumble upon:

	/* Large page NAND with SOFT_ECC should support subpage reads */
	switch (ecc->engine_type) {
	case NAND_ECC_ENGINE_TYPE_SOFT:
		if (chip->page_shift > 9)
			chip->options |= NAND_SUBPAGE_READ;
		break;

	default:
		break;
	}

So the code assumes subpage reads are ok when SOFT_ECC is in use, which
in my case is not true. I guess some drivers depend on the
NAND_SUBPAGE_READ bit magically be set, so simply removing this code is
likely not an option.  Any ideas what to do?

Sascha


-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

  reply	other threads:[~2024-04-18  6:48 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-17  7:13 [PATCH 0/4] mtd: nand: mxc_nand: Convert to exec_op Sascha Hauer
2024-04-17  7:13 ` Sascha Hauer
2024-04-17  7:13 ` [PATCH 1/4] mtd: nand: mxc_nand: separate page read from ecc calc Sascha Hauer
2024-04-17  7:13   ` Sascha Hauer
2024-04-17  7:13 ` [PATCH 2/4] mtd: nand: mxc_nand: implement exec_op Sascha Hauer
2024-04-17  7:13   ` Sascha Hauer
2024-05-06 14:02   ` Miquel Raynal
2024-05-06 14:02     ` Miquel Raynal
2024-04-17  7:13 ` [PATCH 3/4] mtd: nand: mxc_nand: support software ECC Sascha Hauer
2024-04-17  7:13   ` Sascha Hauer
2024-05-06 14:05   ` Miquel Raynal
2024-05-06 14:05     ` Miquel Raynal
2024-05-06 15:51     ` Miquel Raynal
2024-05-06 15:51       ` Miquel Raynal
2024-05-07  7:12       ` Sascha Hauer
2024-05-07  7:12         ` Sascha Hauer
2024-05-07  7:45         ` Miquel Raynal
2024-05-07  7:45           ` Miquel Raynal
2024-05-07 10:33           ` Sascha Hauer
2024-05-07 10:33             ` Sascha Hauer
2024-05-07 14:02             ` Miquel Raynal
2024-05-07 14:02               ` Miquel Raynal
2024-04-17  7:13 ` [PATCH 4/4] mtd: nand: mxc_nand: disable subpage reads Sascha Hauer
2024-04-17  7:13   ` Sascha Hauer
2024-04-18  6:48   ` Sascha Hauer [this message]
2024-04-18  6:48     ` Sascha Hauer
2024-04-18  9:32     ` Miquel Raynal
2024-04-18  9:32       ` Miquel Raynal
2024-04-18 11:43       ` Sascha Hauer
2024-04-18 11:43         ` Sascha Hauer
2024-04-19  9:46         ` Miquel Raynal
2024-04-19  9:46           ` Miquel Raynal
2024-04-22 10:53           ` Sascha Hauer
2024-04-22 10:53             ` Sascha Hauer
2024-05-06 16:41             ` Miquel Raynal
2024-05-06 16:41               ` Miquel Raynal
2024-05-07  7:28               ` Sascha Hauer
2024-05-07  7:28                 ` Sascha Hauer

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=ZiDCKGlG4MZ23Tqo@pengutronix.de \
    --to=s.hauer@pengutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=miquel.raynal@bootlin.com \
    --cc=richard@nod.at \
    --cc=vigneshr@ti.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.