[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 at bootlin.com> wrote:
>
>     Hello Mikhail,
>
>     On 26/08/2025 at 02:48:29 +03, Mikhail Kshevetskiy
>     <mikhail.kshevetskiy at iopsys.eu> wrote:
>
>     > The shown speed inverse linearly depends on size of data.
>     > See the output:
>     >
>     >   spi-nand: spi_nand nand at 0: Micron SPI NAND was found.
>     >   spi-nand: spi_nand nand at 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 at 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 at amarulasolutions.com
> __________________________________
>
> Amarula Solutions BV
> Joop Geesinkweg 125, 1114 AB, Amsterdam, NL
> T. +31 (0)85 111 9172
> info at amarulasolutions.com
> http://www.amarulasolutions.com/
> <http://www.amarulasolutions.com/>