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

[PATCH] 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. The decision 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. The
commit that introduced the regression is:

    https://git.kernel.org/torvalds/c/14157f861437ebe2d624b0a845b91bbdf8ca9a2d

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 | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
index fe96803..176db89 100644
--- a/drivers/mtd/nand/raw/nandsim.c
+++ b/drivers/mtd/nand/raw/nandsim.c
@@ -2359,6 +2359,18 @@ 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;
+
+		/*
+		 * NAND_ROW_ADDR_3 was set by nand_scan_ident() from the
+		 * geometry decoded out of the ID bytes, which the size
+		 * override has just made stale. Re-evaluate it against
+		 * the new geometry, otherwise the core emits one more (or
+		 * one less) row address byte than the simulator expects.
+		 */
+		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] drivers/mtd: rawnand: nandsim: Fix stale NAND_ROW_ADDR_3 with overridesize

[Miquel Raynal]

Hi Wang,

On 01/10/2026 at 18:33:59 +08, wang wei <a929244872@163.com> wrote:

> 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.

Fine until here.

> 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. The decision thus always saw the

"The decision"?

> 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. The
> commit that introduced the regression is:
>
>     https://git.kernel.org/torvalds/c/14157f861437ebe2d624b0a845b91bbdf8ca9a2d

This is not needed, you mention it above and below already.

> 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")

Cc: stable

> Signed-off-by: wang wei <a929244872@163.com>
> ---
>  drivers/mtd/nand/raw/nandsim.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
> index fe96803..176db89 100644
> --- a/drivers/mtd/nand/raw/nandsim.c
> +++ b/drivers/mtd/nand/raw/nandsim.c
> @@ -2359,6 +2359,18 @@ 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;
> +
> +		/*
> +		 * NAND_ROW_ADDR_3 was set by nand_scan_ident() from the
> +		 * geometry decoded out of the ID bytes, which the size
> +		 * override has just made stale. Re-evaluate it against
> +		 * the new geometry, otherwise the core emits one more (or
> +		 * one less) row address byte than the simulator expects.
> +		 */

Everything above this line can just be deleted. Tell your LLM that this
is obvious enough and does not require any particular comment.

> +		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);

Instead of repeating the operation in nandsim, why not calling
nand_scan() after over writing the size? (inverting the two blocks)

Thanks,
Miquèl

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

[Miquel Raynal]

>>> +		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);
>>
>>Instead of repeating the operation in nandsim, why not calling
>>nand_scan() after over writing the size? (inverting the two blocks)
>
> The override cannot be moved before nand_scan(): In nand_detect(), 
> chip ->chipushift will be set to the actual flash size. In addition,  
> In addition, expressed in erase blocks (new_size = nsmtd->erasesize << overridesize)
> and erasesize is what nand_scan_ident() decodes from the ID bytes, so
> the override has a hard data dependency on the scan.

Ok, fine then.

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

[wang wei]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 4276 bytes --]

>Hi Wang,
>
>On 01/10/2026 at 18:33:59 +08, wang wei <a929244872@163.com> wrote:
>
>> 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.
>
>Fine until here.

Okay

>
>> 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. The decision thus always saw the
>
>"The decision"?

What I mean is the semantics of this code

>
>> 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. The
>> commit that introduced the regression is:
>>
>>     https://git.kernel.org/torvalds/c/14157f861437ebe2d624b0a845b91bbdf8ca9a2d
>
>This is not needed, you mention it above and below already.

Okay

>
>> 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")
>
>Cc: stable
>
>> Signed-off-by: wang wei <a929244872@163.com>
>> ---
>>  drivers/mtd/nand/raw/nandsim.c | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/raw/nandsim.c b/drivers/mtd/nand/raw/nandsim.c
>> index fe96803..176db89 100644
>> --- a/drivers/mtd/nand/raw/nandsim.c
>> +++ b/drivers/mtd/nand/raw/nandsim.c
>> @@ -2359,6 +2359,18 @@ 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;
>> +
>> +		/*
>> +		 * NAND_ROW_ADDR_3 was set by nand_scan_ident() from the
>> +		 * geometry decoded out of the ID bytes, which the size
>> +		 * override has just made stale. Re-evaluate it against
>> +		 * the new geometry, otherwise the core emits one more (or
>> +		 * one less) row address byte than the simulator expects.
>> +		 */
>
>Everything above this line can just be deleted. Tell your LLM that this
>is obvious enough and does not require any particular comment.

Okay, will be modified in V2

>
>> +		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);
>
>Instead of repeating the operation in nandsim, why not calling
>nand_scan() after over writing the size? (inverting the two blocks)

The override cannot be moved before nand_scan(): In nand_detect(),
chip ->chipushift will be set to the actual flash size. In addition,
In addition, expressed in erase blocks (new_size = nsmtd->erasesize << overridesize)
and erasesize is what nand_scan_ident() decodes from the ID bytes, so
the override has a hard data dependency on the scan.

>
>Thanks,
>Miquèl