[PATCH 4/4] mtd: rawnand: Clarify conditions to enable continuous reads

[Christophe Kerello]

Hi Miquel,

On 2/21/24 12:20, Miquel Raynal wrote:
> Hi Christophe,
> 
> christophe.kerello at foss.st.com wrote on Fri, 9 Feb 2024 14:35:44 +0100:
> 
>> Hi Miquel,
>>
>> I am testing last nand/next branch with the MP1 board, and i get an issue since this patch was applied.
>>
>> When I read the SLC NAND using nandump tool (reading page 0 and page 1), the OOB is not displayed at expected. For page 1, oob is displayed when for page 0 the first data of the page are displayed.
>>
>> The nanddump command used is: nanddump -c -o -l 0x2000 /dev/mtd9
> 
> I believe the issue is not in the indexes but related to the OOB. I
> currently test on a device on which I would prefer not to smash the
> content, so this is just compile tested and not run time verified, but
> could you tell me if this solves the issue:
> 
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -3577,7 +3577,8 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
>          oob = ops->oobbuf;
>          oob_required = oob ? 1 : 0;
>   
> -       rawnand_enable_cont_reads(chip, page, readlen, col);
> +       if (!oob_required)
> +               rawnand_enable_cont_reads(chip, page, readlen, col);

I am still able to reproduce the problem with the patch applied.
In fact, when nanddump reads the OOB, nand_do_read_ops is not called, 
but nand_read_oob_op is called, and as cont_read.ongoing=1, we are not 
dumping the oob but the first data of the page.

page 0:
[   57.642144] rawnand_enable_cont_reads: page=0, col=0, readlen=4096, 
mtd->writesize=4096
[   57.650210] rawnand_enable_cont_reads: end_page=1
[   57.654858] nand_do_read_ops: cont_read.ongoing=1
[   59.352562] nand_read_oob_op
page 1:
[   59.355966] rawnand_enable_cont_reads: page=1, col=0, readlen=4096, 
mtd->writesize=4096
[   59.364045] rawnand_enable_cont_reads: end_page=1
[   59.368757] nand_do_read_ops: cont_read.ongoing=0
[   61.390098] nand_read_oob_op

I have not currently bandwidth to work on this topic and I need to 
understand how continuous read is working, but I have made a patch and I 
do not have issues with it when I am using nanddump or mtd_debug tools.

I have not tested it on a file system, so it is just a proposal.

--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -3466,22 +3466,18 @@ 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 end_page;

  	chip->cont_read.ongoing = false;

-	if (!chip->controller->supported_op.cont_read)
+	if (!chip->controller->supported_op.cont_read || col + readlen <= 
mtd->writesize)
  		return;

-	end_page = DIV_ROUND_UP(col + readlen, mtd->writesize);
-	end_col = (col + readlen) % mtd->writesize;
+	end_page = page + DIV_ROUND_UP(col + readlen, mtd->writesize) - 1;

  	if (col)
  		page++;

-	if (end_col && end_page)
-		end_page--;
-
  	if (page + 1 > end_page)
  		return;

Tell me if this patch is breaking the continuous read feature or if it 
can be pushed on the mailing list.

Regards,
Christophe Kerello.

>   
>          while (1) {
>                  struct mtd_ecc_stats ecc_stats = mtd->ecc_stats;
> 
> 
> If that does not work, I'll destroy the content of the flash and
> properly reproduce.
> 
> Thanks,
> Miquèl