[PATCH] cmd: mtd: fix speed measurement in the speed benchmark

[PATCH] cmd: mtd: fix speed measurement in the speed benchmark

[Mikhail Kshevetskiy]

The shown speed inverse linearly depends on size of data.
See the output:

  spi-nand: spi_nand nand@0: Micron SPI NAND was found.
  spi-nand: spi_nand nand@0: 256 MiB, block size: 128 KiB, page size: 2048, OOB size: 128
  ...
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x40000
  Reading 262144 byte(s) (128 page(s)) at offset 0x00000000
  Read speed: 63kiB/s
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x20000
  Reading 131072 byte(s) (64 page(s)) at offset 0x00000000
  Read speed: 127kiB/s
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x10000
  Reading 65536 byte(s) (32 page(s)) at offset 0x00000000
  Read speed: 254kiB/s

In the spi-nand case 'io_op.len' is not the same as 'len',
thus we divide a size of the single block on total time.
This is wrong, we should divide on the time for a single
block.

Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
---
 cmd/mtd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cmd/mtd.c b/cmd/mtd.c
index 2520b89eed2..deac7d1f002 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -469,7 +469,7 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
 {
 	bool dump, read, raw, woob, benchmark, write_empty_pages, has_pages = false;
 	u64 start_off, off, len, remaining, default_len;
-	unsigned long bench_start, bench_end;
+	unsigned long bench_start, bench_end, block_time;
 	struct mtd_oob_ops io_op = {};
 	uint user_addr = 0, npages;
 	const char *cmd = argv[0];
@@ -594,9 +594,10 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	if (benchmark && bench_start) {
 		bench_end = timer_get_us();
+		block_time = (bench_end - bench_start) / (len / io_op.len);
 		printf("%s speed: %lukiB/s\n",
 		       read ? "Read" : "Write",
-		       ((io_op.len * 1000000) / (bench_end - bench_start)) / 1024);
+		       ((io_op.len * 1000000) / block_time) / 1024);
 	}
 
 	led_activity_off();
-- 
2.50.1

Re: [PATCH] cmd: mtd: fix speed measurement in the speed benchmark

[Miquel Raynal]

Hello Mikhail,

On 26/08/2025 at 02:48:29 +03, Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> wrote:

> The shown speed inverse linearly depends on size of data.
> See the output:
>
>   spi-nand: spi_nand nand@0: Micron SPI NAND was found.
>   spi-nand: spi_nand nand@0: 256 MiB, block size: 128 KiB, page size: 2048, OOB size: 128
>   ...
>   => mtd read.benchmark spi-nand0 $loadaddr 0 0x40000
>   Reading 262144 byte(s) (128 page(s)) at offset 0x00000000
>   Read speed: 63kiB/s
>   => mtd read.benchmark spi-nand0 $loadaddr 0 0x20000
>   Reading 131072 byte(s) (64 page(s)) at offset 0x00000000
>   Read speed: 127kiB/s
>   => mtd read.benchmark spi-nand0 $loadaddr 0 0x10000
>   Reading 65536 byte(s) (32 page(s)) at offset 0x00000000
>   Read speed: 254kiB/s
>
> In the spi-nand case 'io_op.len' is not the same as 'len',
> thus we divide a size of the single block on total time.
> This is wrong, we should divide on the time for a single
> block.
>
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>

Happy to see this is useful :-) But you're totally right, it didn't use
the correct length. Maybe I would rephrase a bit the last two sentences
to make the commit clearer:

"In the spi-nand case 'io_op.len' is not always the same as 'len', thus
we are using the wrong amount of data to derive the speed."

However, regarding the diff,

> @@ -594,9 +594,10 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
>  
>  	if (benchmark && bench_start) {
>  		bench_end = timer_get_us();
> +		block_time = (bench_end - bench_start) / (len / io_op.len);
>  		printf("%s speed: %lukiB/s\n",
>  		       read ? "Read" : "Write",
> -		       ((io_op.len * 1000000) / (bench_end - bench_start)) / 1024);
> +		       ((io_op.len * 1000000) / block_time) / 1024);

Why not just dividing the length by the benchmark time instead of
reducing and rounding the denominator in the first place, which I
believe makes the final result less precise?

Thanks,
Miquèl

Re: [PATCH] cmd: mtd: fix speed measurement in the speed benchmark

[Michael Nazzareno Trimarchi]

Hi

On Tue, Aug 26, 2025 at 4:23 PM Miquel Raynal <miquel.raynal@bootlin.com>
wrote:

> Hello Mikhail,
>
> On 26/08/2025 at 02:48:29 +03, Mikhail Kshevetskiy <
> mikhail.kshevetskiy@iopsys.eu> wrote:
>
> > The shown speed inverse linearly depends on size of data.
> > See the output:
> >
> >   spi-nand: spi_nand nand@0: Micron SPI NAND was found.
> >   spi-nand: spi_nand nand@0: 256 MiB, block size: 128 KiB, page size:
> 2048, OOB size: 128
> >   ...
> >   => mtd read.benchmark spi-nand0 $loadaddr 0 0x40000
> >   Reading 262144 byte(s) (128 page(s)) at offset 0x00000000
> >   Read speed: 63kiB/s
> >   => mtd read.benchmark spi-nand0 $loadaddr 0 0x20000
> >   Reading 131072 byte(s) (64 page(s)) at offset 0x00000000
> >   Read speed: 127kiB/s
> >   => mtd read.benchmark spi-nand0 $loadaddr 0 0x10000
> >   Reading 65536 byte(s) (32 page(s)) at offset 0x00000000
> >   Read speed: 254kiB/s
> >
> > In the spi-nand case 'io_op.len' is not the same as 'len',
> > thus we divide a size of the single block on total time.
> > This is wrong, we should divide on the time for a single
> > block.
> >
> > Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
>
>
Add a fixes with commit reference, when you post v2 and address al the
other comments

Michael


> Happy to see this is useful :-) But you're totally right, it didn't use
> the correct length. Maybe I would rephrase a bit the last two sentences
> to make the commit clearer:
>
> "In the spi-nand case 'io_op.len' is not always the same as 'len', thus
> we are using the wrong amount of data to derive the speed."
>
> However, regarding the diff,
>
> > @@ -594,9 +594,10 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int
> flag, int argc,
> >
> >       if (benchmark && bench_start) {
> >               bench_end = timer_get_us();
> > +             block_time = (bench_end - bench_start) / (len / io_op.len);
> >               printf("%s speed: %lukiB/s\n",
> >                      read ? "Read" : "Write",
> > -                    ((io_op.len * 1000000) / (bench_end - bench_start))
> / 1024);
> > +                    ((io_op.len * 1000000) / block_time) / 1024);
>
> Why not just dividing the length by the benchmark time instead of
> reducing and rounding the denominator in the first place, which I
> believe makes the final result less precise?
>
> Thanks,
> Miquèl
>


-- 
Michael Nazzareno Trimarchi
Co-Founder & Chief Executive Officer
M. +39 347 913 2170
michael@amarulasolutions.com
__________________________________

Amarula Solutions BV
Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
T. +31 (0)85 111 9172
info@amarulasolutions.com
www.amarulasolutions.com

Re: [PATCH] cmd: mtd: fix speed measurement in the speed benchmark

[Mikhail Kshevetskiy]

and maybe benchmark feature should be documented

On 26.08.2025 17:32, Michael Nazzareno Trimarchi wrote:
> Hi 
>
> On Tue, Aug 26, 2025 at 4:23 PM Miquel Raynal
> <miquel.raynal@bootlin.com> wrote:
>
>     Hello Mikhail,
>
>     On 26/08/2025 at 02:48:29 +03, Mikhail Kshevetskiy
>     <mikhail.kshevetskiy@iopsys.eu> wrote:
>
>     > The shown speed inverse linearly depends on size of data.
>     > See the output:
>     >
>     >   spi-nand: spi_nand nand@0: Micron SPI NAND was found.
>     >   spi-nand: spi_nand nand@0: 256 MiB, block size: 128 KiB, page
>     size: 2048, OOB size: 128
>     >   ...
>     >   => mtd read.benchmark spi-nand0 $loadaddr 0 0x40000
>     >   Reading 262144 byte(s) (128 page(s)) at offset 0x00000000
>     >   Read speed: 63kiB/s
>     >   => mtd read.benchmark spi-nand0 $loadaddr 0 0x20000
>     >   Reading 131072 byte(s) (64 page(s)) at offset 0x00000000
>     >   Read speed: 127kiB/s
>     >   => mtd read.benchmark spi-nand0 $loadaddr 0 0x10000
>     >   Reading 65536 byte(s) (32 page(s)) at offset 0x00000000
>     >   Read speed: 254kiB/s
>     >
>     > In the spi-nand case 'io_op.len' is not the same as 'len',
>     > thus we divide a size of the single block on total time.
>     > This is wrong, we should divide on the time for a single
>     > block.
>     >
>     > Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
>
>
> Add a fixes with commit reference, when you post v2 and address al the
> other comments
>
> Michael
>  
>
>     Happy to see this is useful :-) But you're totally right, it
>     didn't use
>     the correct length. Maybe I would rephrase a bit the last two
>     sentences
>     to make the commit clearer:
>
>     "In the spi-nand case 'io_op.len' is not always the same as 'len',
>     thus
>     we are using the wrong amount of data to derive the speed."
>
>     However, regarding the diff,
>
>     > @@ -594,9 +594,10 @@ static int do_mtd_io(struct cmd_tbl *cmdtp,
>     int flag, int argc,
>     > 
>     >       if (benchmark && bench_start) {
>     >               bench_end = timer_get_us();
>     > +             block_time = (bench_end - bench_start) / (len /
>     io_op.len);
>     >               printf("%s speed: %lukiB/s\n",
>     >                      read ? "Read" : "Write",
>     > -                    ((io_op.len * 1000000) / (bench_end -
>     bench_start)) / 1024);
>     > +                    ((io_op.len * 1000000) / block_time) / 1024);
>
>     Why not just dividing the length by the benchmark time instead of
>     reducing and rounding the denominator in the first place, which I
>     believe makes the final result less precise?
>
>     Thanks,
>     Miquèl
>
>
>
> -- 
> Michael Nazzareno Trimarchi
> Co-Founder & Chief Executive Officer
> M. +39 347 913 2170
> michael@amarulasolutions.com
> __________________________________
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info@amarulasolutions.com
> http://www.amarulasolutions.com/
> <http://www.amarulasolutions.com/>

Re: [PATCH] cmd: mtd: fix speed measurement in the speed benchmark

[Mikhail Kshevetskiy]

On 26.08.2025 17:23, Miquel Raynal wrote:
> Hello Mikhail,
>
> On 26/08/2025 at 02:48:29 +03, Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> wrote:
>
>> The shown speed inverse linearly depends on size of data.
>> See the output:
>>
>>   spi-nand: spi_nand nand@0: Micron SPI NAND was found.
>>   spi-nand: spi_nand nand@0: 256 MiB, block size: 128 KiB, page size: 2048, OOB size: 128
>>   ...
>>   => mtd read.benchmark spi-nand0 $loadaddr 0 0x40000
>>   Reading 262144 byte(s) (128 page(s)) at offset 0x00000000
>>   Read speed: 63kiB/s
>>   => mtd read.benchmark spi-nand0 $loadaddr 0 0x20000
>>   Reading 131072 byte(s) (64 page(s)) at offset 0x00000000
>>   Read speed: 127kiB/s
>>   => mtd read.benchmark spi-nand0 $loadaddr 0 0x10000
>>   Reading 65536 byte(s) (32 page(s)) at offset 0x00000000
>>   Read speed: 254kiB/s
>>
>> In the spi-nand case 'io_op.len' is not the same as 'len',
>> thus we divide a size of the single block on total time.
>> This is wrong, we should divide on the time for a single
>> block.
>>
>> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> Happy to see this is useful :-) But you're totally right, it didn't use
> the correct length. Maybe I would rephrase a bit the last two sentences
> to make the commit clearer:
>
> "In the spi-nand case 'io_op.len' is not always the same as 'len', thus
> we are using the wrong amount of data to derive the speed."
>
> However, regarding the diff,
>
>> @@ -594,9 +594,10 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
>>  
>>  	if (benchmark && bench_start) {
>>  		bench_end = timer_get_us();
>> +		block_time = (bench_end - bench_start) / (len / io_op.len);
>>  		printf("%s speed: %lukiB/s\n",
>>  		       read ? "Read" : "Write",
>> -		       ((io_op.len * 1000000) / (bench_end - bench_start)) / 1024);
>> +		       ((io_op.len * 1000000) / block_time) / 1024);
> Why not just dividing the length by the benchmark time instead of
> reducing and rounding the denominator in the first place, which I
> believe makes the final result less precise?

Do we use 64 bit math? If not we may easily get an overflow.
Actually for 32-bit math it's better use a less precise formula:
(io_op.len * (1000000/1024)) / block_time; thus we will have about 22
bit for length.

>
> Thanks,
> Miquèl

Re: [PATCH] cmd: mtd: fix speed measurement in the speed benchmark

[Miquel Raynal]

On 26/08/2025 at 17:33:40 +03, Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> wrote:

> On 26.08.2025 17:23, Miquel Raynal wrote:
>> Hello Mikhail,
>>
>> On 26/08/2025 at 02:48:29 +03, Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> wrote:
>>
>>> The shown speed inverse linearly depends on size of data.
>>> See the output:
>>>
>>>   spi-nand: spi_nand nand@0: Micron SPI NAND was found.
>>>   spi-nand: spi_nand nand@0: 256 MiB, block size: 128 KiB, page size: 2048, OOB size: 128
>>>   ...
>>>   => mtd read.benchmark spi-nand0 $loadaddr 0 0x40000
>>>   Reading 262144 byte(s) (128 page(s)) at offset 0x00000000
>>>   Read speed: 63kiB/s
>>>   => mtd read.benchmark spi-nand0 $loadaddr 0 0x20000
>>>   Reading 131072 byte(s) (64 page(s)) at offset 0x00000000
>>>   Read speed: 127kiB/s
>>>   => mtd read.benchmark spi-nand0 $loadaddr 0 0x10000
>>>   Reading 65536 byte(s) (32 page(s)) at offset 0x00000000
>>>   Read speed: 254kiB/s
>>>
>>> In the spi-nand case 'io_op.len' is not the same as 'len',
>>> thus we divide a size of the single block on total time.
>>> This is wrong, we should divide on the time for a single
>>> block.
>>>
>>> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
>> Happy to see this is useful :-) But you're totally right, it didn't use
>> the correct length. Maybe I would rephrase a bit the last two sentences
>> to make the commit clearer:
>>
>> "In the spi-nand case 'io_op.len' is not always the same as 'len', thus
>> we are using the wrong amount of data to derive the speed."
>>
>> However, regarding the diff,
>>
>>> @@ -594,9 +594,10 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
>>>  
>>>  	if (benchmark && bench_start) {
>>>  		bench_end = timer_get_us();
>>> +		block_time = (bench_end - bench_start) / (len / io_op.len);
>>>  		printf("%s speed: %lukiB/s\n",
>>>  		       read ? "Read" : "Write",
>>> -		       ((io_op.len * 1000000) / (bench_end - bench_start)) / 1024);
>>> +		       ((io_op.len * 1000000) / block_time) / 1024);
>> Why not just dividing the length by the benchmark time instead of
>> reducing and rounding the denominator in the first place, which I
>> believe makes the final result less precise?
>
> Do we use 64 bit math? If not we may easily get an overflow.
> Actually for 32-bit math it's better use a less precise formula:
> (io_op.len * (1000000/1024)) / block_time; thus we will have about 22
> bit for length.

I considered overflow out of topic (see the v1 of the benchmark
introduction) as we do not run bootloaders for hours. Yes it is
definitely reachable, but for a development/benchmarking tool, I didn't
consider this as a problem.

Thanks,
Miquèl

[PATCH v2] cmd: mtd: fix speed measurement in the speed benchmark

[Mikhail Kshevetskiy]

The shown speed inversely proportional to the data size.
See the output:

  spi-nand: spi_nand nand@0: Micron SPI NAND was found.
  spi-nand: spi_nand nand@0: 256 MiB, block size: 128 KiB, page size: 2048, OOB size: 128
  ...
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x40000
  Reading 262144 byte(s) (128 page(s)) at offset 0x00000000
  Read speed: 63kiB/s
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x20000
  Reading 131072 byte(s) (64 page(s)) at offset 0x00000000
  Read speed: 127kiB/s
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x10000
  Reading 65536 byte(s) (32 page(s)) at offset 0x00000000
  Read speed: 254kiB/s

In the spi-nand case 'io_op.len' is not always the same as 'len', thus
we are using the wrong amount of data to derive the speed.

The patch also updates the help for 'mtd read' command to show the presence
of the benchmark option. The help for the 'mtd write' command help has not
been changed to fit 80 character width restriction.

Fixes: d246e70cf81d0 ("cmd: mtd: Enable speed benchmarking")
Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
---
 cmd/mtd.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

Changes v2:
 * update commit message
 * 'Fixes:' tag was added
 * calculate speed a bit more precisely using a 64-bit math

diff --git a/cmd/mtd.c b/cmd/mtd.c
index cee3a7e43ef..7e022fdc0f7 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -469,7 +469,7 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
 		     char *const argv[])
 {
 	bool dump, read, raw, woob, benchmark, write_empty_pages, has_pages = false;
-	u64 start_off, off, len, remaining, default_len;
+	u64 start_off, off, len, remaining, default_len, speed;
 	unsigned long bench_start, bench_end;
 	struct mtd_oob_ops io_op = {};
 	uint user_addr = 0, npages;
@@ -595,9 +595,10 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	if (benchmark && bench_start) {
 		bench_end = timer_get_us();
+		speed = (len * 1000000) / (bench_end - bench_start);
 		printf("%s speed: %lukiB/s\n",
 		       read ? "Read" : "Write",
-		       ((io_op.len * 1000000) / (bench_end - bench_start)) / 1024);
+		       (unsigned long)(speed / 1024));
 	}
 
 	led_activity_off();
@@ -1210,7 +1211,7 @@ static int mtd_name_complete(int argc, char *const argv[], char last_char,
 U_BOOT_LONGHELP(mtd,
 	"- generic operations on memory technology devices\n\n"
 	"mtd list\n"
-	"mtd read[.raw][.oob]                  <name> <addr> [<off> [<size>]]\n"
+	"mtd read[.raw][.oob][.benchmark]      <name> <addr> [<off> [<size>]]\n"
 	"mtd dump[.raw][.oob]                  <name>        [<off> [<size>]]\n"
 	"mtd write[.raw][.oob][.dontskipff]    <name> <addr> [<off> [<size>]]\n"
 	"mtd erase[.dontskipbad]               <name>        [<off> [<size>]]\n"
-- 
2.50.1

Re: [PATCH v2] cmd: mtd: fix speed measurement in the speed benchmark

[Miquel Raynal]

Hello,

On 26/08/2025 at 22:21:31 +03, Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> wrote:

> The shown speed inversely proportional to the data size.

                 ^ is

> See the output:
>
>   spi-nand: spi_nand nand@0: Micron SPI NAND was found.
>   spi-nand: spi_nand nand@0: 256 MiB, block size: 128 KiB, page size: 2048, OOB size: 128
>   ...
>   => mtd read.benchmark spi-nand0 $loadaddr 0 0x40000
>   Reading 262144 byte(s) (128 page(s)) at offset 0x00000000
>   Read speed: 63kiB/s
>   => mtd read.benchmark spi-nand0 $loadaddr 0 0x20000
>   Reading 131072 byte(s) (64 page(s)) at offset 0x00000000
>   Read speed: 127kiB/s
>   => mtd read.benchmark spi-nand0 $loadaddr 0 0x10000
>   Reading 65536 byte(s) (32 page(s)) at offset 0x00000000
>   Read speed: 254kiB/s
>
> In the spi-nand case 'io_op.len' is not always the same as 'len', thus
> we are using the wrong amount of data to derive the speed.
>
> The patch also updates the help for 'mtd read' command to show the presence
> of the benchmark option. The help for the 'mtd write' command help has not
> been changed to fit 80 character width restriction.

I would put this into a second commit, and anyway adapt the write
command as well. I do not think the 80 chars rule is so important here.

> Fixes: d246e70cf81d0 ("cmd: mtd: Enable speed benchmarking")
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> ---
>  cmd/mtd.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> Changes v2:
>  * update commit message
>  * 'Fixes:' tag was added
>  * calculate speed a bit more precisely using a 64-bit math

This should have been done in a separate patch. In general we prefer
atomic changes. It's okay as the diff is small, but as an example I'd
have instead done that with 3 patches:
- Fix the calculation (sf/io_op.len/len)
- Change the size because you fear 1h is not enough
- Document the commands

Anyway,

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl

[PATCH v3 0/2] cmd: mtd: fix benchmarking

[Mikhail Kshevetskiy]

The patch series fixes speed calculation for nand device case and
updates help to refer benchmark option presence.

Changes v2:
 * update commit message
 * 'Fixes:' tag was added
 * calculate speed a bit more precisely using a 64-bit math

Changes v3:
 * split on 2 patches
 * update help for 'mtd write' command as well
 * format help for 'mtd' command to align device names on the same collumn

Mikhail Kshevetskiy (2):
  cmd: mtd: fix speed measurement in the speed benchmark
  cmd: mtd: add benchmark option to the help

 cmd/mtd.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

-- 
2.50.1

[PATCH v3 1/2] cmd: mtd: fix speed measurement in the speed benchmark

[Mikhail Kshevetskiy]

The shown speed is inversely proportional to the data size.
See the output:

  spi-nand: spi_nand nand@0: Micron SPI NAND was found.
  spi-nand: spi_nand nand@0: 256 MiB, block size: 128 KiB, page size: 2048, OOB size: 128
  ...
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x40000
  Reading 262144 byte(s) (128 page(s)) at offset 0x00000000
  Read speed: 63kiB/s
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x20000
  Reading 131072 byte(s) (64 page(s)) at offset 0x00000000
  Read speed: 127kiB/s
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x10000
  Reading 65536 byte(s) (32 page(s)) at offset 0x00000000
  Read speed: 254kiB/s

In the spi-nand case 'io_op.len' is not always the same as 'len', thus
we are using the wrong amount of data to derive the speed.

Also make sure we are using 64-bit calculation to get a more precise
results.

Fixes: d246e70cf81d0 ("cmd: mtd: Enable speed benchmarking")
Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/mtd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cmd/mtd.c b/cmd/mtd.c
index cee3a7e43ef..d4e1f83b91c 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -469,7 +469,7 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
 		     char *const argv[])
 {
 	bool dump, read, raw, woob, benchmark, write_empty_pages, has_pages = false;
-	u64 start_off, off, len, remaining, default_len;
+	u64 start_off, off, len, remaining, default_len, speed;
 	unsigned long bench_start, bench_end;
 	struct mtd_oob_ops io_op = {};
 	uint user_addr = 0, npages;
@@ -595,9 +595,10 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	if (benchmark && bench_start) {
 		bench_end = timer_get_us();
+		speed = (len * 1000000) / (bench_end - bench_start);
 		printf("%s speed: %lukiB/s\n",
 		       read ? "Read" : "Write",
-		       ((io_op.len * 1000000) / (bench_end - bench_start)) / 1024);
+		       (unsigned long)(speed / 1024));
 	}
 
 	led_activity_off();
-- 
2.50.1

[PATCH v3 2/2] cmd: mtd: add benchmark option to the help

[Mikhail Kshevetskiy]

The patch adds benchmark option to the help of mtd command. For the
'mtd write' case the help line exceed 80 characters. Ignore this issue
as modern terminals are capable to handle more characters.

The patch also formats other command to make sure all device names
starts on the same collumn.

Fixes: d246e70cf81d0 ("cmd: mtd: Enable speed benchmarking")
Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
---
 cmd/mtd.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/cmd/mtd.c b/cmd/mtd.c
index d4e1f83b91c..b072838c97a 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -1211,27 +1211,27 @@ static int mtd_name_complete(int argc, char *const argv[], char last_char,
 U_BOOT_LONGHELP(mtd,
 	"- generic operations on memory technology devices\n\n"
 	"mtd list\n"
-	"mtd read[.raw][.oob]                  <name> <addr> [<off> [<size>]]\n"
-	"mtd dump[.raw][.oob]                  <name>        [<off> [<size>]]\n"
-	"mtd write[.raw][.oob][.dontskipff]    <name> <addr> [<off> [<size>]]\n"
-	"mtd erase[.dontskipbad]               <name>        [<off> [<size>]]\n"
+	"mtd read[.raw][.oob][.benchmark]                <name> <addr> [<off> [<size>]]\n"
+	"mtd dump[.raw][.oob]                            <name>        [<off> [<size>]]\n"
+	"mtd write[.raw][.oob][.dontskipff][.benchmark]  <name> <addr> [<off> [<size>]]\n"
+	"mtd erase[.dontskipbad]                         <name>        [<off> [<size>]]\n"
 	"\n"
 	"Specific functions:\n"
-	"mtd bad                               <name>\n"
+	"mtd bad                                         <name>\n"
 #if CONFIG_IS_ENABLED(CMD_MTD_OTP)
-	"mtd otpread                           <name> [u|f] <off> <size>\n"
-	"mtd otpwrite                          <name> <off> <hex string>\n"
-	"mtd otplock                           <name> <off> <size>\n"
-	"mtd otpinfo                           <name> [u|f]\n"
+	"mtd otpread                                     <name> [u|f] <off> <size>\n"
+	"mtd otpwrite                                    <name> <off> <hex string>\n"
+	"mtd otplock                                     <name> <off> <size>\n"
+	"mtd otpinfo                                     <name> [u|f]\n"
 #endif
 #if CONFIG_IS_ENABLED(CMD_MTD_MARKBAD)
-	"mtd markbad                           <name>         <off> [<off> ...]\n"
+	"mtd markbad                                     <name>         <off> [<off> ...]\n"
 #endif
 #if CONFIG_IS_ENABLED(CMD_MTD_NAND_WRITE_TEST)
-	"mtd nand_write_test                   <name>        [<off> [<size>]]\n"
+	"mtd nand_write_test                             <name>        [<off> [<size>]]\n"
 #endif
 #if CONFIG_IS_ENABLED(CMD_MTD_NAND_READ_TEST)
-	"mtd nand_read_test                    <name>\n"
+	"mtd nand_read_test                              <name>\n"
 #endif
 	"\n"
 	"With:\n"
-- 
2.50.1

[PATCH v4 0/2] cmd: mtd: fix benchmarking

[Mikhail Kshevetskiy]

The patch series fixes speed calculation for nand device case and
updates help to refer benchmark option presence.

Changes v2:
 * update commit message
 * 'Fixes:' tag was added
 * calculate speed a bit more precisely using a 64-bit math

Changes v3:
 * split on 2 patches
 * update help for 'mtd write' command as well
 * format help for 'mtd' command to align device names on the same collumn

Changes v4:
 * rebase patches on top of master branch

Mikhail Kshevetskiy (1):
  cmd: mtd: fix speed measurement in the speed benchmark

Tom Rini (1):
  Prepare v2025.10-rc3

 Makefile                      | 2 +-
 cmd/mtd.c                     | 5 +++--
 doc/develop/release_cycle.rst | 2 +-
 3 files changed, 5 insertions(+), 4 deletions(-)

-- 
2.50.1

[PATCH v4 1/2] Prepare v2025.10-rc3

[Mikhail Kshevetskiy]

From: Tom Rini <trini@konsulko.com>

Signed-off-by: Tom Rini <trini@konsulko.com>
---
 Makefile                      | 2 +-
 doc/develop/release_cycle.rst | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Makefile b/Makefile
index 63e23886cde..b4f10c08d75 100644
--- a/Makefile
+++ b/Makefile
@@ -3,7 +3,7 @@
 VERSION = 2025
 PATCHLEVEL = 10
 SUBLEVEL =
-EXTRAVERSION = -rc2
+EXTRAVERSION = -rc3
 NAME =
 
 # *DOCUMENTATION*
diff --git a/doc/develop/release_cycle.rst b/doc/develop/release_cycle.rst
index 354ab1b5d74..ae6200a1880 100644
--- a/doc/develop/release_cycle.rst
+++ b/doc/develop/release_cycle.rst
@@ -75,7 +75,7 @@ For the next scheduled release, release candidates were made on::
 
 * U-Boot |next_ver|-rc2 was released on Mon 11 August 2025.
 
-.. * U-Boot |next_ver|-rc3 was released on Mon 25 August 2025.
+* U-Boot |next_ver|-rc3 was released on Mon 25 August 2025.
 
 .. * U-Boot |next_ver|-rc4 was released on Mon 08 September 2025.
 
-- 
2.50.1

[PATCH v4 2/2] cmd: mtd: fix speed measurement in the speed benchmark

[Mikhail Kshevetskiy]

The shown speed is inversely proportional to the data size.
See the output:

  spi-nand: spi_nand nand@0: Micron SPI NAND was found.
  spi-nand: spi_nand nand@0: 256 MiB, block size: 128 KiB, page size: 2048, OOB size: 128
  ...
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x40000
  Reading 262144 byte(s) (128 page(s)) at offset 0x00000000
  Read speed: 63kiB/s
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x20000
  Reading 131072 byte(s) (64 page(s)) at offset 0x00000000
  Read speed: 127kiB/s
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x10000
  Reading 65536 byte(s) (32 page(s)) at offset 0x00000000
  Read speed: 254kiB/s

In the spi-nand case 'io_op.len' is not always the same as 'len', thus
we are using the wrong amount of data to derive the speed.

Also make sure we are using 64-bit calculation to get a more precise
results.

Fixes: d246e70cf81d0 ("cmd: mtd: Enable speed benchmarking")
Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/mtd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cmd/mtd.c b/cmd/mtd.c
index 2520b89eed2..57f6c700f1d 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -468,7 +468,7 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
 		     char *const argv[])
 {
 	bool dump, read, raw, woob, benchmark, write_empty_pages, has_pages = false;
-	u64 start_off, off, len, remaining, default_len;
+	u64 start_off, off, len, remaining, default_len, speed;
 	unsigned long bench_start, bench_end;
 	struct mtd_oob_ops io_op = {};
 	uint user_addr = 0, npages;
@@ -594,9 +594,10 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	if (benchmark && bench_start) {
 		bench_end = timer_get_us();
+		speed = (len * 1000000) / (bench_end - bench_start);
 		printf("%s speed: %lukiB/s\n",
 		       read ? "Read" : "Write",
-		       ((io_op.len * 1000000) / (bench_end - bench_start)) / 1024);
+		       (unsigned long)(speed / 1024));
 	}
 
 	led_activity_off();
-- 
2.50.1

[PATCH v5 0/2] cmd: mtd: fix benchmarking

[Mikhail Kshevetskiy]

The patch series fixes speed calculation for nand device case and
updates help to refer benchmark option presence.

Changes v2:
 * update commit message
 * 'Fixes:' tag was added
 * calculate speed a bit more precisely using a 64-bit math

Changes v3:
 * split on 2 patches
 * update help for 'mtd write' command as well
 * format help for 'mtd' command to align device names on the same collumn

Changes v4:
 * attemt to sent incorrect patches (sorry for this)

Changes v5:
 * really rebase on top of master branch

Mikhail Kshevetskiy (2):
  cmd: mtd: fix speed measurement in the speed benchmark
  cmd: mtd: add benchmark option to the help

 cmd/mtd.c | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

-- 
2.50.1

[PATCH v5 1/2] cmd: mtd: fix speed measurement in the speed benchmark

[Mikhail Kshevetskiy]

The shown speed is inversely proportional to the data size.
See the output:

  spi-nand: spi_nand nand@0: Micron SPI NAND was found.
  spi-nand: spi_nand nand@0: 256 MiB, block size: 128 KiB, page size: 2048, OOB size: 128
  ...
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x40000
  Reading 262144 byte(s) (128 page(s)) at offset 0x00000000
  Read speed: 63kiB/s
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x20000
  Reading 131072 byte(s) (64 page(s)) at offset 0x00000000
  Read speed: 127kiB/s
  => mtd read.benchmark spi-nand0 $loadaddr 0 0x10000
  Reading 65536 byte(s) (32 page(s)) at offset 0x00000000
  Read speed: 254kiB/s

In the spi-nand case 'io_op.len' is not always the same as 'len', thus
we are using the wrong amount of data to derive the speed.

Also make sure we are using 64-bit calculation to get a more precise
results.

Fixes: d246e70cf81d0 ("cmd: mtd: Enable speed benchmarking")
Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>
---
 cmd/mtd.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/cmd/mtd.c b/cmd/mtd.c
index 2520b89eed2..57f6c700f1d 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -468,7 +468,7 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
 		     char *const argv[])
 {
 	bool dump, read, raw, woob, benchmark, write_empty_pages, has_pages = false;
-	u64 start_off, off, len, remaining, default_len;
+	u64 start_off, off, len, remaining, default_len, speed;
 	unsigned long bench_start, bench_end;
 	struct mtd_oob_ops io_op = {};
 	uint user_addr = 0, npages;
@@ -594,9 +594,10 @@ static int do_mtd_io(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	if (benchmark && bench_start) {
 		bench_end = timer_get_us();
+		speed = (len * 1000000) / (bench_end - bench_start);
 		printf("%s speed: %lukiB/s\n",
 		       read ? "Read" : "Write",
-		       ((io_op.len * 1000000) / (bench_end - bench_start)) / 1024);
+		       (unsigned long)(speed / 1024));
 	}
 
 	led_activity_off();
-- 
2.50.1

[PATCH v5 2/2] cmd: mtd: add benchmark option to the help

[Mikhail Kshevetskiy]

The patch adds benchmark option to the help of mtd command. For the
'mtd write' case the help line exceed 80 characters. Ignore this issue
as modern terminals are capable to handle more characters.

The patch also formats other command to make sure all device names
starts on the same collumn.

Fixes: d246e70cf81d0 ("cmd: mtd: Enable speed benchmarking")
Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
---
 cmd/mtd.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/cmd/mtd.c b/cmd/mtd.c
index 57f6c700f1d..f053dbadd78 100644
--- a/cmd/mtd.c
+++ b/cmd/mtd.c
@@ -782,18 +782,18 @@ static int mtd_name_complete(int argc, char *const argv[], char last_char,
 U_BOOT_LONGHELP(mtd,
 	"- generic operations on memory technology devices\n\n"
 	"mtd list\n"
-	"mtd read[.raw][.oob]                  <name> <addr> [<off> [<size>]]\n"
-	"mtd dump[.raw][.oob]                  <name>        [<off> [<size>]]\n"
-	"mtd write[.raw][.oob][.dontskipff]    <name> <addr> [<off> [<size>]]\n"
-	"mtd erase[.dontskipbad]               <name>        [<off> [<size>]]\n"
+	"mtd read[.raw][.oob][.benchmark]                <name> <addr> [<off> [<size>]]\n"
+	"mtd dump[.raw][.oob]                            <name>        [<off> [<size>]]\n"
+	"mtd write[.raw][.oob][.dontskipff][.benchmark]  <name> <addr> [<off> [<size>]]\n"
+	"mtd erase[.dontskipbad]                         <name>        [<off> [<size>]]\n"
 	"\n"
 	"Specific functions:\n"
-	"mtd bad                               <name>\n"
+	"mtd bad                                         <name>\n"
 #if CONFIG_IS_ENABLED(CMD_MTD_OTP)
-	"mtd otpread                           <name> [u|f] <off> <size>\n"
-	"mtd otpwrite                          <name> <off> <hex string>\n"
-	"mtd otplock                           <name> <off> <size>\n"
-	"mtd otpinfo                           <name> [u|f]\n"
+	"mtd otpread                                     <name> [u|f] <off> <size>\n"
+	"mtd otpwrite                                    <name> <off> <hex string>\n"
+	"mtd otplock                                     <name> <off> <size>\n"
+	"mtd otpinfo                                     <name> [u|f]\n"
 #endif
 	"\n"
 	"With:\n"
-- 
2.50.1

Re: [PATCH v5 2/2] cmd: mtd: add benchmark option to the help

[Miquel Raynal]

On 29/08/2025 at 10:59:11 +03, Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu> wrote:

> The patch adds benchmark option to the help of mtd command. For the
> 'mtd write' case the help line exceed 80 characters. Ignore this issue
> as modern terminals are capable to handle more characters.
>
> The patch also formats other command to make sure all device names
> starts on the same collumn.
>
> Fixes: d246e70cf81d0 ("cmd: mtd: Enable speed benchmarking")
> Signed-off-by: Mikhail Kshevetskiy <mikhail.kshevetskiy@iopsys.eu>
> ---
>  cmd/mtd.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/cmd/mtd.c b/cmd/mtd.c
> index 57f6c700f1d..f053dbadd78 100644
> --- a/cmd/mtd.c
> +++ b/cmd/mtd.c
> @@ -782,18 +782,18 @@ static int mtd_name_complete(int argc, char *const argv[], char last_char,
>  U_BOOT_LONGHELP(mtd,
>  	"- generic operations on memory technology devices\n\n"
>  	"mtd list\n"
> -	"mtd read[.raw][.oob]                  <name> <addr> [<off> [<size>]]\n"
> -	"mtd dump[.raw][.oob]                  <name>        [<off> [<size>]]\n"
> -	"mtd write[.raw][.oob][.dontskipff]    <name> <addr> [<off> [<size>]]\n"
> -	"mtd erase[.dontskipbad]               <name>        [<off> [<size>]]\n"
> +	"mtd read[.raw][.oob][.benchmark]                <name> <addr> [<off> [<size>]]\n"
> +	"mtd dump[.raw][.oob]                            <name>        [<off> [<size>]]\n"
> +	"mtd write[.raw][.oob][.dontskipff][.benchmark]  <name> <addr> [<off> [<size>]]\n"
> +	"mtd erase[.dontskipbad]                         <name>        [<off> [<size>]]\n"
>  	"\n"
>  	"Specific functions:\n"
> -	"mtd bad                               <name>\n"
> +	"mtd bad                                         <name>\n"
>  #if CONFIG_IS_ENABLED(CMD_MTD_OTP)
> -	"mtd otpread                           <name> [u|f] <off> <size>\n"
> -	"mtd otpwrite                          <name> <off> <hex string>\n"
> -	"mtd otplock                           <name> <off> <size>\n"
> -	"mtd otpinfo                           <name> [u|f]\n"
> +	"mtd otpread                                     <name> [u|f] <off> <size>\n"
> +	"mtd otpwrite                                    <name> <off> <hex string>\n"
> +	"mtd otplock                                     <name> <off> <size>\n"
> +	"mtd otpinfo                                     <name> [u|f]\n"
>  #endif
>  	"\n"
>  	"With:\n"

I'll let the decision to other U-Boot reviewers, but I don't know if
that is relevant to align all the parameters to the longest line.

Either ways, I'm fine,

Reviewed-by: Miquel Raynal <miquel.raynal@bootlin.com>

Thanks,
Miquèl