[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <875xf1hjb8.fsf@bootlin.com>
Date: Tue, 05 Aug 2025 17:36:27 +0200
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: Mikhail Kshevetskiy <mikhail.kshevetskiy@...sys.eu>
Cc: Richard Weinberger <richard@....at>, Vignesh Raghavendra
<vigneshr@...com>, Lorenzo Bianconi <lorenzo@...nel.org>, Ray Liu
<ray.liu@...oha.com>, Mark Brown <broonie@...nel.org>, Tudor Ambarus
<tudor.ambarus@...aro.org>, Martin Kurbanov
<mmkurbanov@...utedevices.com>, Takahiro Kuwano
<Takahiro.Kuwano@...ineon.com>, Cheng Ming Lin
<chengminglin@...c.com.tw>, linux-mtd@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-spi@...r.kernel.org
Subject: Re: [PATCH 3/4] drivers: mtd: spi-nand: repeat reading in regular
mode if continuous reading fails
On 04/08/2025 at 22:21:31 +03, Mikhail Kshevetskiy <mikhail.kshevetskiy@...sys.eu> wrote:
> Continuous reading may result in multiple flash pages reading in one
> operation. Unfortunately, not all spi-nand controllers support such
> large reading. They will read less data. Unfortunately, the operation
> can't be continued.
>
> In this case:
> * disable continuous reading on this (not good enough) spi controller
> * repeat reading in regular mode.
>
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@...sys.eu>
> ---
> drivers/mtd/nand/spi/core.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mtd/nand/spi/core.c b/drivers/mtd/nand/spi/core.c
> index ff6a1e2fcfdc..88e4c00cccc4 100644
> --- a/drivers/mtd/nand/spi/core.c
> +++ b/drivers/mtd/nand/spi/core.c
> @@ -431,7 +431,7 @@ static int spinand_read_from_cache_op(struct spinand_device *spinand,
> * Toggling the CS during a continuous read is forbidden.
> */
> if (nbytes && req->continuous)
> - return -EIO;
> + return -E2BIG;
> }
>
> if (req->datalen)
> @@ -893,15 +893,26 @@ static int spinand_mtd_read(struct mtd_info *mtd, loff_t from,
> struct spinand_device *spinand = mtd_to_spinand(mtd);
> struct mtd_ecc_stats old_stats;
> unsigned int max_bitflips = 0;
> - int ret;
> + int ret = -E2BIG;
>
> mutex_lock(&spinand->lock);
>
> old_stats = mtd->ecc_stats;
>
> - if (spinand_use_cont_read(mtd, from, ops))
> + if (spinand_use_cont_read(mtd, from, ops)) {
> ret = spinand_mtd_continuous_page_read(mtd, from, ops, &max_bitflips);
> - else
> + if (ret == -E2BIG) {
> + /*
> + * Some spi controllers may not support reading up to
> + * erase block size. They will read less data than
> + * expected. If this happen disable continuous mode
happens,
> + * and repeat reading in normal mode.
> + */
> + spinand->cont_read_possible = false;
> + }
> + }
> +
> + if (ret == -E2BIG)
While I agree with the overall logic, I find pretty unreadable to
perform the regular read upon a return value that is an error code set
in the init sequence. Can you please propose an alternate implementation?
> ret = spinand_mtd_regular_page_read(mtd, from, ops, &max_bitflips);
>
> if (ops->stats) {
Thanks!
Miquèl
Powered by blists - more mailing lists