Re: [PATCH 1/3] mtd: rawnand: Fix and simplify again the continuous read derivations

[Christophe Kerello <christophe.kerello@foss.st.com>]

Hi Miquel,

On 2/23/24 12:55, Miquel Raynal wrote:
> We need to avoid the first page if we don't read it entirely.
> We need to avoid the last page if we don't read it entirely.
> While rather simple, this logic has been failed in the previous
> fix. This time I wrote about 30 unit tests locally to check each
> possible condition, hopefully I covered them all.
> 
> Reported-by: Christophe Kerello <christophe.kerello@foss.st.com>
> Closes: https://lore.kernel.org/linux-mtd/20240221175327.42f7076d@xps-13/T/#m399bacb10db8f58f6b1f0149a1df867ec086bb0a
> Suggested-by: Christophe Kerello <christophe.kerello@foss.st.com>
> Fixes: 828f6df1bcba ("mtd: rawnand: Clarify conditions to enable continuous reads")
> Cc: stable@vger.kernel.org
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

Tested-by: Christophe Kerello <christophe.kerello@foss.st.com>

Regards,
Christophe Kerello.

> ---
>   drivers/mtd/nand/raw/nand_base.c | 38 ++++++++++++++++++--------------
>   1 file changed, 22 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 3b3ce2926f5d..bcfd99a1699f 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -3466,30 +3466,36 @@ static void rawnand_enable_cont_reads(struct nand_chip *chip, unsigned int page,
>   				      u32 readlen, int col)
>   {
>   	struct mtd_info *mtd = nand_to_mtd(chip);
> -	unsigned int end_page, end_col;
> +	unsigned int first_page, last_page;
>   
>   	chip->cont_read.ongoing = false;
>   
>   	if (!chip->controller->supported_op.cont_read)
>   		return;
>   
> -	end_page = DIV_ROUND_UP(col + readlen, mtd->writesize);
> -	end_col = (col + readlen) % mtd->writesize;
> +	/*
> +	 * Don't bother making any calculations if the length is too small.
> +	 * Side effect: avoids possible integer underflows below.
> +	 */
> +	if (readlen < (2 * mtd->writesize))
> +		return;
>   
> +	/* Derive the page where continuous read should start (the first full page read) */
> +	first_page = page;
>   	if (col)
> -		page++;
> -
> -	if (end_col && end_page)
> -		end_page--;
> -
> -	if (page + 1 > end_page)
> -		return;
> -
> -	chip->cont_read.first_page = page;
> -	chip->cont_read.last_page = end_page;
> -	chip->cont_read.ongoing = true;
> -
> -	rawnand_cap_cont_reads(chip);
> +		first_page++;
> +
> +	/* Derive the page where continuous read should stop (the last full page read) */
> +	last_page = page + ((col + readlen) / mtd->writesize) - 1;
> +
> +	/* Configure and enable continuous read when suitable */
> +	if (first_page < last_page) {
> +		chip->cont_read.first_page = first_page;
> +		chip->cont_read.last_page = last_page;
> +		chip->cont_read.ongoing = true;
> +		/* May reset the ongoing flag */
> +		rawnand_cap_cont_reads(chip);
> +	}
>   }
>   
>   static void rawnand_cont_read_skip_first_page(struct nand_chip *chip, unsigned int page)

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