spi: sf: add support for throughput mesurement of sf read/write

This patch adds time measurement and throughput calculation for sf read/write commands.

The output of sf read changes from

---8<---
SF: 4096 bytes @ 0x0 Read: OK
--->8---

to

---8<---
SF: 4096 bytes @ 0x0 Read: OK in 6 ms (666 KiB/s)
--->8---

Signed-off-by Mugunthan V N <[email protected]>

---
 common/cmd_sf.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

common/cmd_sf.c +15 -3

index ac7f5df..8687f3a 100644
@@ -15,6 +15,7 @@
 #include <spi_flash.h>
 #include <jffs2/jffs2.h>
 #include <linux/mtd/mtd.h>
+#include <linux/math64.h>
 
 #include <asm/io.h>
 #include <dm/device-internal.h>

@@ -266,6 +267,7 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 	int ret = 1;
 	int dev = 0;
 	loff_t offset, len, maxsize;
+	unsigned long time;
 
 	if (argc < 3)
 		return -1;

@@ -298,17 +300,27 @@ static int do_spi_flash_read_write(int argc, char * const argv[])
 		int read;
 
 		read = strncmp(argv[0], "read", 4) == 0;
+
+		time = get_timer(0);
 		if (read)
 			ret = spi_flash_read(flash, offset, len, buf);
 		else
 			ret = spi_flash_write(flash, offset, len, buf);
+		time = get_timer(time);
 
 		printf("SF: %zu bytes @ %#x %s: ", (size_t)len, (u32)offset,
 		       read ? "Read" : "Written");
-		if (ret)
+		if (ret) {
 			printf("ERROR %d\n", ret);
-		else
-			printf("OK\n");
+		} else {
+			printf("OK in %lu ms", time);
+			if (time > 0) {
+				puts(" (");
+				print_size(div_u64(len, time) * 1000, "/s");
+				puts(")");
+			}
+			puts("\n");
+		}
 	}
 
 	unmap_physmem(buf, len);

Jagan

Quoted 16 lines · Mugunthan V N

This patch adds time measurement and throughput calculation for sf read/write commands.

The output of sf read changes from

---8<---
SF: 4096 bytes @ 0x0 Read: OK
--->8---

to

---8<---
SF: 4096 bytes @ 0x0 Read: OK in 6 ms (666 KiB/s)
--->8---

Signed-off-by Mugunthan V N <[email protected]>

A gentle ping

Quoted 14 lines · Mugunthan V N

Jagan

Quoted 11 lines · Mugunthan V N

This patch adds time measurement and throughput calculation for sf read/write commands.

The output of sf read changes from

---8<---
SF: 4096 bytes @ 0x0 Read: OK
--->8---

to

Quoted 5 lines

Quoted 5 lines

---8<---
SF: 4096 bytes @ 0x0 Read: OK in 6 ms (666 KiB/s)
--->8---

Signed-off-by Mugunthan V N <[email protected]>

Was it similar to 'sf update' ? please check it once.

Quoted 19 lines · Jagan Teki

Quoted 18 lines · Mugunthan V N

Jagan

Quoted 15 lines · Mugunthan V N

This patch adds time measurement and throughput calculation for sf read/write commands.

The output of sf read changes from

---8<---
SF: 4096 bytes @ 0x0 Read: OK
--->8---

to

---8<---
SF: 4096 bytes @ 0x0 Read: OK in 6 ms (666 KiB/s)
--->8---

Quoted 4 lines

Quoted 1 line

Quoted 1 line

Signed-off-by Mugunthan V N <[email protected]>

Was it similar to 'sf update' ? please check it once.

sf update out similar but also uses progressive output, in read/write case it can't be done. The final throughput measurement is similar on both update and read/write.

Quoted 25 lines · Mugunthan V N

Quoted 23 lines · Jagan Teki

Quoted 19 lines · Mugunthan V N

Jagan

Quoted 16 lines · Mugunthan V N

This patch adds time measurement and throughput calculation for sf read/write commands.

The output of sf read changes from

---8<---
SF: 4096 bytes @ 0x0 Read: OK
--->8---

to

---8<---
SF: 4096 bytes @ 0x0 Read: OK in 6 ms (666 KiB/s)
--->8---

Signed-off-by Mugunthan V N <[email protected]>

Was it similar to 'sf update' ? please check it once.

Quoted 3 lines

sf update out similar but also uses progressive output, in read/write case it can't be done. The final throughput measurement is similar on both update and read/write.

True, that's what if we need a progressed throughput just use 'sf update' else normal 'sf read/write' It's look not good to me to add extra code on top of generic commands. What ever we wanted to extend features let's added it on 'sf update' than sf read/write, Sorry.

Quoted 27 lines · Jagan Teki

Quoted 26 lines · Mugunthan V N

Quoted 24 lines · Jagan Teki

Quoted 19 lines · Mugunthan V N

Jagan

Quoted 16 lines · Mugunthan V N

This patch adds time measurement and throughput calculation for sf read/write commands.

The output of sf read changes from

---8<---
SF: 4096 bytes @ 0x0 Read: OK
--->8---

to

---8<---
SF: 4096 bytes @ 0x0 Read: OK in 6 ms (666 KiB/s)
--->8---

Signed-off-by Mugunthan V N <[email protected]>

Was it similar to 'sf update' ? please check it once.

Quoted 8 lines

Quoted 3 lines

sf update out similar but also uses progressive output, in read/write case it can't be done. The final throughput measurement is similar on both update and read/write.

True, that's what if we need a progressed throughput just use 'sf update' else normal 'sf read/write' It's look not good to me to add extra code on top of generic commands. What ever we wanted to extend features let's added it on 'sf update' than sf read/write, Sorry.

If I need to measure the time of commands, I use the "time" command ("time sf write ...") by enabling it via CONFIG_CMD_TIME. This provides all the needed information to detect performance changes.

Quoted 37 lines · Stefan Roese

Quoted 27 lines · Jagan Teki

Quoted 26 lines · Mugunthan V N

Quoted 24 lines · Jagan Teki

Quoted 19 lines · Mugunthan V N

Jagan

Quoted 16 lines · Mugunthan V N

This patch adds time measurement and throughput calculation for sf read/write commands.

The output of sf read changes from

---8<---
SF: 4096 bytes @ 0x0 Read: OK
--->8---

to

---8<---
SF: 4096 bytes @ 0x0 Read: OK in 6 ms (666 KiB/s)
--->8---

Signed-off-by Mugunthan V N <[email protected]>

Was it similar to 'sf update' ? please check it once.

Quoted 8 lines

Quoted 3 lines

sf update out similar but also uses progressive output, in read/write case it can't be done. The final throughput measurement is similar on both update and read/write.

True, that's what if we need a progressed throughput just use 'sf update' else normal 'sf read/write' It's look not good to me to add extra code on top of generic commands. What ever we wanted to extend features let's added it on 'sf update' than sf read/write, Sorry.

Quoted 5 lines

If I need to measure the time of commands, I use the "time" command ("time sf write ...") by enabling it via CONFIG_CMD_TIME. This provides all the needed information to detect performance changes.

But similar kind of implementations is present for fatload and tftp. So I thought having similar performance log for sf read/write will be good as well.

Quoted 54 lines · Mugunthan V N

Quoted 46 lines · Stefan Roese

Quoted 38 lines · Jagan Teki

Quoted 28 lines · Mugunthan V N

Quoted 26 lines · Jagan Teki

Quoted 21 lines · Mugunthan V N

Jagan

Quoted 17 lines · Mugunthan V N

This patch adds time measurement and throughput calculation for sf read/write commands.

The output of sf read changes from

---8<---
SF: 4096 bytes @ 0x0 Read: OK
--->8---

to

---8<---
SF: 4096 bytes @ 0x0 Read: OK in 6 ms (666 KiB/s)
--->8---

Signed-off-by Mugunthan V N <[email protected]>

Quoted 6 lines

Quoted 2 lines

Was it similar to 'sf update' ? please check it once.

sf update out similar but also uses progressive output, in read/write case it can't be done. The final throughput measurement is similar on both update and read/write.

Quoted 4 lines

True, that's what if we need a progressed throughput just use 'sf update' else normal 'sf read/write' It's look not good to me to add extra code on top of generic commands. What ever we wanted to extend features let's added it on 'sf update' than sf read/write, Sorry.

Quoted 5 lines

If I need to measure the time of commands, I use the "time" command ("time sf write ...") by enabling it via CONFIG_CMD_TIME. This provides all the needed information to detect performance changes.

Quoted 3 lines

But similar kind of implementations is present for fatload and tftp. So I thought having similar performance log for sf read/write will be good as well.

Yes, I understand. I also find this output helpful. But I'm not sure, if we should add this timing code to all these functions. Instead of just using this common time command, if needed. To keep the code size at a minimum.

Of course its no big code addition, so I don't really have any bigger reservations against it.