[PATCH V2] drivers/mtd: rawnand: nandsim: Fix stale NAND_ROW_ADDR_3 with overridesize

[PATCH V2] drivers/mtd: rawnand: nandsim: Fix stale NAND_ROW_ADDR_3 with overridesize

[wang wei]

The overridesize module parameter changes the size of the simulated
device after nand_scan() has completed. It updates nsmtd->size,
memorg->eraseblocks_per_lun, chip->chip_shift and chip->pagemask to
match the new geometry, but leaves the NAND_ROW_ADDR_3 option
untouched, even though nand_scan_ident() set it from the geometry
decoded out of the ID bytes.

When the ID bytes describe a device larger than 128 MiB and
overridesize shrinks the simulation to 128 MiB or less, the stale
option makes the core emit one extra row address byte (page >> 16,
always zero given the reduced page count) in every read, program and
erase operation, while the simulator state machine expects one byte
less. All accesses then fail with:

    nandsim: error: write_byte: address (0x0) isn't expected, expected
    state is STATE_CMD_READSTART, switch to STATE_READY

The opposite direction is equally broken: growing a small device past
128 MiB keeps NAND_ROW_ADDR_3 cleared, so the third row address byte
is dropped and the wrong pages are silently addressed.

This used to work before commit 14157f861437 ("mtd: nand: introduce
NAND_ROW_ADDR_3 flag"). Back then nandsim kept chip->chipsize in sync
with the override, and nand_command_lp() decided at run time, per
command, whether the third row address cycle was needed by testing
chip->chipsize against 128 MiB. This code thus always saw the
overridden size. The above commit moved the decision to
nand_scan_ident(), which encodes it once into NAND_ROW_ADDR_3 at
scan time -- before nandsim applies the override -- and the
overridesize path was never taught to re-evaluate the flag. 

Re-evaluate NAND_ROW_ADDR_3 right after overriding chip_shift, using
the same test as nand_scan_ident(), so that the address width emitted
by the core always matches ns->geom.pgaddrbytes, which ns_init()
derives from the overridden total size.

Fixes: 14157f861437 ("mtd: nand: introduce NAND_ROW_ADDR_3 flag")
Signed-off-by: wang wei <a929244872@163.com>
---
 drivers/mtd/nand/raw/nandsim.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
index fe96803..ac82dd7 100644
--- a/drivers/mtd/nand/raw/nandsim.c
+++ b/drivers/mtd/nand/raw/nandsim.c
@@ -2359,6 +2359,11 @@ static int __init ns_init_module(void)
 		targetsize = nanddev_target_size(&chip->base);
 		chip->chip_shift = ffs(nsmtd->erasesize) + overridesize - 1;
 		chip->pagemask = (targetsize >> chip->page_shift) - 1;
+
+		if (chip->chip_shift - chip->page_shift > 16)
+			chip->options |= NAND_ROW_ADDR_3;
+		else
+			chip->options &= ~NAND_ROW_ADDR_3;
 	}
 
 	ret = ns_setup_wear_reporting(nsmtd);

-- 
2.55.0.windows.3

Re: [PATCH V2] drivers/mtd: rawnand: nandsim: Fix stale NAND_ROW_ADDR_3 with overridesize

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mtd: nandsim: address cycle mismatch for 256 MiB / 4096-byte page devices
--

commit d11867536ecbb0d0243c052069d60f5f1a2c4afe
Author: wang wei <a929244872@163.com>

drivers/mtd: rawnand: nandsim: Fix stale NAND_ROW_ADDR_3 with overridesize

The commit fixes a stale NAND_ROW_ADDR_3 flag by re-evaluating it after
changing the size of the simulated device when using the overridesize
parameter. It updates the flag based on the core logic: chip_shift -
page_shift > 16.

> diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
> index fe968037f75a2..ac82dd7e18a93 100644
> --- a/drivers/mtd/nand/raw/nandsim.c
> +++ b/drivers/mtd/nand/raw/nandsim.c
> @@ -2359,6 +2359,11 @@ static int __init ns_init_module(void)
>  		targetsize = nanddev_target_size(&chip->base);
>  		chip->chip_shift = ffs(nsmtd->erasesize) + overridesize - 1;
>  		chip->pagemask = (targetsize >> chip->page_shift) - 1;
> +
> +		if (chip->chip_shift - chip->page_shift > 16)
> +			chip->options |= NAND_ROW_ADDR_3;
> +		else
> +			chip->options &= ~NAND_ROW_ADDR_3;

[Severity: Medium]
Does this logic conflict with the address byte expectation in ns_init()?

For a 256 MiB device with 4096-byte pages, chip_shift - page_shift evaluates
to 28 - 12 = 16. This causes the code here to clear NAND_ROW_ADDR_3, so the
core emits 4 address bytes.

However, ns_init() has hardcoded logic expecting 5 address bytes for devices
larger than 128 MiB:

drivers/mtd/nand/raw/nandsim.c:ns_init() {
    ...
		if (ns->geom.totsz <= (128 << 20)) {
			ns->geom.pgaddrbytes  = 4;
			ns->geom.secaddrbytes = 2;
		} else {
			ns->geom.pgaddrbytes  = 5;
			ns->geom.secaddrbytes = 3;
		}
    ...
}

Will this mismatch stall the nandsim state machine and fail operations due to
an address cycle mismatch?

Prior to this change, shrinking a 512 MiB device to 256 MiB left the stale
flag set, which serendipitously matched nandsim's expectation of 5 bytes.

>  	}
>  
>  	ret = ns_setup_wear_reporting(nsmtd);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001124733.43559-1-a929244872@163.com?part=1