Re: [PATCH 0/3] mtd: rawnand: More continuous read fixes

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

Hi Miquel,

On 2/23/24 12:55, Miquel Raynal wrote:
> Hello,
> 
> Following Christophe report I manually inserted many different
> conditions to test the logic enablingand configuring continuous reads in
> the core, trying to clarify the core and hopefully fix it for real. I am
> pretty confident regarding the first patch but a bit more in the fog for
> the second/third. Even though I'm pretty sure they improve the situation
> there might still be corner cases I don't have in mind.

I have tested the patchset and the issue is fixed, so I will send a 
tested-by on patch 1.

But, I think that there is still an issue using FMC2 and probably others 
drivers.

FMC2 driver has 2 modes: polling mode that will called 
nand_read_page_op and the sequencer mode that is defining its own HW 
read algorithm.

The FMC2 sequencer do not support continuous read feature.

I have added basic logs in nand_do_read_ops. My understanding is that 
the continuous read feature should be disabled at the end of this function.

FMC2 polling mode:
root@stm32mp1:~# mtd_debug read /dev/mtd9 0 0x2000 /tmp/read.hex
[   41.083132] nand_do_read_ops starts: cont_read.ongoing=1
[   41.086410] nand_do_read_ops starts: cont_read.first_page=0, 
cont_read.last_page=1
[   41.094797] nand_do_read_ops ends: cont_read.ongoing=0
[   41.098111] nand_do_read_ops ends: cont_read.first_page=0, 
cont_read.last_page=1
Copied 8192 bytes from address 0x00000000 in flash to /tmp/read.hex

It is OK. In polling mode, con_read.ongoing is set to false before 
leaving nand_do_read_ops function.

FMC2 sequencer:
root@stm32mp1:~# mtd_debug read /dev/mtd9 0 0x2000 /tmp/read.hex
[   57.143059] nand_do_read_ops starts: cont_read.ongoing=1
[   57.146370] nand_do_read_ops starts: cont_read.first_page=0, 
cont_read.last_page=1
[   57.154469] nand_do_read_ops ends: cont_read.ongoing=1
[   57.158020] nand_do_read_ops ends: cont_read.first_page=0, 
cont_read.last_page=1
Copied 8192 bytes from address 0x00000000 in flash to /tmp/read.hex

KO, con_read.ongoing is set to true before leaving nand_do_read_ops 
function. That means that read_oob can returned wrong data (similar 
issue as the initial reported issue).

So, I see 2 ways to fix this issue.

On framework side by adding a callback (rawnand_disable_cont_reads) that 
will set to false con_read.ongoing before leaving nand_do_read_ops function.

Or

On FMC2 driver side by disabling the continuous read feature in case of 
the sequencer is used like it is done in nandsim.c driver.
Something like:
if (check_only) {
		for (op_id = 0; op_id < op->ninstrs; op_id++) {
			instr = &op->instrs[op_id];
			if (instr->type == NAND_OP_CMD_INSTR &&
			    (instr->ctx.cmd.opcode == NAND_CMD_READCACHEEND ||
			     instr->ctx.cmd.opcode == NAND_CMD_READCACHESEQ))
				return -EOPNOTSUPP;
		}

		return 0;
	}

So, should it be fixed on driver side or framework side?

Regards,
Christophe Kerello.

> 
> Link: https://lore.kernel.org/linux-mtd/20240221175327.42f7076d@xps-13/T/#m399bacb10db8f58f6b1f0149a1df867ec086bb0a
> 
> Cheers,
> Miquèl
> 
> Miquel Raynal (3):
>    mtd: rawnand: Fix and simplify again the continuous read derivations
>    mtd: rawnand: Add a helper for calculating a page index
>    mtd: rawnand: Ensure all continuous terms are always in sync
> 
>   drivers/mtd/nand/raw/nand_base.c | 77 +++++++++++++++++++-------------
>   1 file changed, 47 insertions(+), 30 deletions(-)
> 

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