[PATCH 0/4] sf: Add documentation and an 'sf mmap' command

This little series adds documentation and a few other tidy-ups to the 'sf' command.
It also provides a way to access memory-mapped SPI via the command line.
Simon Glass (4): command: Use a constant pointer for the help sf: Tidy up code to avoid #ifdef sf: doc: Add documentation for the 'sf' command sf: Provide a command to access memory-mapped SPI
arch/Kconfig | 2 + cmd/Kconfig | 8 ++ cmd/sf.c | 67 ++++++++--- doc/usage/index.rst | 1 + doc/usage/sf.rst | 288 ++++++++++++++++++++++++++++++++++++++++++++ include/command.h | 2 +- 6 files changed, 349 insertions(+), 19 deletions(-) create mode 100644 doc/usage/sf.rst

This text should never change during execution, so it makes sense to use a const char * so that it can be declared as const in the code. Update struct cmd_tbl with a const char * pointer for 'help'.
Signed-off-by: Simon Glass sjg@chromium.org ---
include/command.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/command.h b/include/command.h index 747f8f80958..675c16a1249 100644 --- a/include/command.h +++ b/include/command.h @@ -45,7 +45,7 @@ struct cmd_tbl { char *const argv[]); char *usage; /* Usage message (short) */ #ifdef CONFIG_SYS_LONGHELP - char *help; /* Help message (long) */ + const char *help; /* Help message (long) */ #endif #ifdef CONFIG_AUTO_COMPLETE /* do auto completion on the arguments */

On 4/6/21 12:30 AM, Simon Glass wrote:
This text should never change during execution, so it makes sense to use a const char * so that it can be declared as const in the code. Update struct cmd_tbl with a const char * pointer for 'help'.
Signed-off-by: Simon Glass sjg@chromium.org
include/command.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/command.h b/include/command.h index 747f8f80958..675c16a1249 100644 --- a/include/command.h +++ b/include/command.h @@ -45,7 +45,7 @@ struct cmd_tbl { char *const argv[]); char *usage; /* Usage message (short) */
Should usage also be const?
--Sean
#ifdef CONFIG_SYS_LONGHELP
- char *help; /* Help message (long) */
- const char *help; /* Help message (long) */ #endif #ifdef CONFIG_AUTO_COMPLETE /* do auto completion on the arguments */

Hi Sean,
On Fri, 9 Apr 2021 at 04:32, Sean Anderson seanga2@gmail.com wrote:
On 4/6/21 12:30 AM, Simon Glass wrote:
This text should never change during execution, so it makes sense to use a const char * so that it can be declared as const in the code. Update struct cmd_tbl with a const char * pointer for 'help'.
Signed-off-by: Simon Glass sjg@chromium.org
include/command.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/command.h b/include/command.h index 747f8f80958..675c16a1249 100644 --- a/include/command.h +++ b/include/command.h @@ -45,7 +45,7 @@ struct cmd_tbl { char *const argv[]); char *usage; /* Usage message (short) */
Should usage also be const?
I think so, but would need to check it builds, etc.
Regards, Simon

Update this code to use IS_ENABLED() instead.
Signed-off-by: Simon Glass sjg@chromium.org ---
cmd/sf.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 46346fb9d43..d4f5ecee38c 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -384,7 +384,6 @@ static int do_spi_protect(int argc, char *const argv[]) return ret == 0 ? 0 : 1; }
-#ifdef CONFIG_CMD_SF_TEST enum { STAGE_ERASE, STAGE_CHECK, @@ -394,7 +393,7 @@ enum { STAGE_COUNT, };
-static char *stage_name[STAGE_COUNT] = { +static const char *stage_name[STAGE_COUNT] = { "erase", "check", "write", @@ -548,7 +547,6 @@ static int do_spi_flash_test(int argc, char *const argv[])
return 0; } -#endif /* CONFIG_CMD_SF_TEST */
static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -582,10 +580,8 @@ static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, int argc, ret = do_spi_flash_erase(argc, argv); else if (strcmp(cmd, "protect") == 0) ret = do_spi_protect(argc, argv); -#ifdef CONFIG_CMD_SF_TEST - else if (!strcmp(cmd, "test")) + else if (IS_ENABLED(CONFIG_CMD_SF_TEST) && !strcmp(cmd, "test")) ret = do_spi_flash_test(argc, argv); -#endif else ret = -1;
@@ -597,16 +593,8 @@ usage: return CMD_RET_USAGE; }
-#ifdef CONFIG_CMD_SF_TEST -#define SF_TEST_HELP "\nsf test offset len " \ - "- run a very basic destructive test" -#else -#define SF_TEST_HELP -#endif - -U_BOOT_CMD( - sf, 5, 1, do_spi_flash, - "SPI flash sub-system", +#ifdef CONFIG_SYS_LONGHELP +static const char long_help[] = "probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus\n" " and chip select\n" "sf read addr offset|partition len - read `len' bytes starting at\n" @@ -622,6 +610,14 @@ U_BOOT_CMD( " at `addr' to flash at `offset'\n" " or to start of mtd `partition'\n" "sf protect lock/unlock sector len - protect/unprotect 'len' bytes starting\n" - " at address 'sector'\n" - SF_TEST_HELP + " at address 'sector'" +#ifdef CONFIG_CMD_SF_TEST + "\nsf test offset len - run a very basic destructive test" +#endif +#endif /* CONFIG_SYS_LONGHELP */ + ; + +U_BOOT_CMD( + sf, 5, 1, do_spi_flash, + "SPI flash sub-system", long_help );

On 06/04/21 04:30PM, Simon Glass wrote:
Update this code to use IS_ENABLED() instead.
Signed-off-by: Simon Glass sjg@chromium.org
cmd/sf.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 46346fb9d43..d4f5ecee38c 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -384,7 +384,6 @@ static int do_spi_protect(int argc, char *const argv[]) return ret == 0 ? 0 : 1; }
-#ifdef CONFIG_CMD_SF_TEST enum { STAGE_ERASE, STAGE_CHECK, @@ -394,7 +393,7 @@ enum { STAGE_COUNT, };
-static char *stage_name[STAGE_COUNT] = { +static const char *stage_name[STAGE_COUNT] = { "erase", "check", "write", @@ -548,7 +547,6 @@ static int do_spi_flash_test(int argc, char *const argv[])
return 0; } -#endif /* CONFIG_CMD_SF_TEST */
Won't this cause all the test related functions to always be compiled into the binary? Then what's the point of having it behind a config at all?
static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -582,10 +580,8 @@ static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, int argc, ret = do_spi_flash_erase(argc, argv); else if (strcmp(cmd, "protect") == 0) ret = do_spi_protect(argc, argv); -#ifdef CONFIG_CMD_SF_TEST
- else if (!strcmp(cmd, "test"))
- else if (IS_ENABLED(CONFIG_CMD_SF_TEST) && !strcmp(cmd, "test")) ret = do_spi_flash_test(argc, argv);
-#endif else ret = -1;
@@ -597,16 +593,8 @@ usage: return CMD_RET_USAGE; }
-#ifdef CONFIG_CMD_SF_TEST -#define SF_TEST_HELP "\nsf test offset len " \
"- run a very basic destructive test"
-#else -#define SF_TEST_HELP -#endif
-U_BOOT_CMD(
- sf, 5, 1, do_spi_flash,
- "SPI flash sub-system",
+#ifdef CONFIG_SYS_LONGHELP +static const char long_help[] = "probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus\n" " and chip select\n" "sf read addr offset|partition len - read `len' bytes starting at\n" @@ -622,6 +610,14 @@ U_BOOT_CMD( " at `addr' to flash at `offset'\n" " or to start of mtd `partition'\n" "sf protect lock/unlock sector len - protect/unprotect 'len' bytes starting\n"
- " at address 'sector'\n"
- SF_TEST_HELP
- " at address 'sector'"
+#ifdef CONFIG_CMD_SF_TEST
- "\nsf test offset len - run a very basic destructive test"
+#endif +#endif /* CONFIG_SYS_LONGHELP */
- ;
+U_BOOT_CMD(
- sf, 5, 1, do_spi_flash,
- "SPI flash sub-system", long_help
);
2.31.0.208.g409f899ff0-goog

On 06.04.21 12:22, Pratyush Yadav wrote:
On 06/04/21 04:30PM, Simon Glass wrote:
Update this code to use IS_ENABLED() instead.
Signed-off-by: Simon Glass sjg@chromium.org
cmd/sf.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 46346fb9d43..d4f5ecee38c 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -384,7 +384,6 @@ static int do_spi_protect(int argc, char *const argv[]) return ret == 0 ? 0 : 1; }
-#ifdef CONFIG_CMD_SF_TEST enum { STAGE_ERASE, STAGE_CHECK, @@ -394,7 +393,7 @@ enum { STAGE_COUNT, };
-static char *stage_name[STAGE_COUNT] = { +static const char *stage_name[STAGE_COUNT] = { "erase", "check", "write", @@ -548,7 +547,6 @@ static int do_spi_flash_test(int argc, char *const argv[])
return 0; } -#endif /* CONFIG_CMD_SF_TEST */
Won't this cause all the test related functions to always be compiled into the binary? Then what's the point of having it behind a config at all?
We compile all functions into separate sections. The linker eliminates unused functions. Look for -ffunction-sections in files arch/<arch>/config.mk.
Best regards
Heinrich
static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -582,10 +580,8 @@ static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, int argc, ret = do_spi_flash_erase(argc, argv); else if (strcmp(cmd, "protect") == 0) ret = do_spi_protect(argc, argv); -#ifdef CONFIG_CMD_SF_TEST
- else if (!strcmp(cmd, "test"))
- else if (IS_ENABLED(CONFIG_CMD_SF_TEST) && !strcmp(cmd, "test")) ret = do_spi_flash_test(argc, argv);
-#endif else ret = -1;
@@ -597,16 +593,8 @@ usage: return CMD_RET_USAGE; }
-#ifdef CONFIG_CMD_SF_TEST -#define SF_TEST_HELP "\nsf test offset len " \
"- run a very basic destructive test"
-#else -#define SF_TEST_HELP -#endif
-U_BOOT_CMD(
- sf, 5, 1, do_spi_flash,
- "SPI flash sub-system",
+#ifdef CONFIG_SYS_LONGHELP +static const char long_help[] = "probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus\n" " and chip select\n" "sf read addr offset|partition len - read `len' bytes starting at\n" @@ -622,6 +610,14 @@ U_BOOT_CMD( " at `addr' to flash at `offset'\n" " or to start of mtd `partition'\n" "sf protect lock/unlock sector len - protect/unprotect 'len' bytes starting\n"
- " at address 'sector'\n"
- SF_TEST_HELP
- " at address 'sector'"
+#ifdef CONFIG_CMD_SF_TEST
- "\nsf test offset len - run a very basic destructive test"
+#endif +#endif /* CONFIG_SYS_LONGHELP */
- ;
+U_BOOT_CMD(
- sf, 5, 1, do_spi_flash,
- "SPI flash sub-system", long_help
);
2.31.0.208.g409f899ff0-goog

On 06/04/21 01:32PM, Heinrich Schuchardt wrote:
On 06.04.21 12:22, Pratyush Yadav wrote:
On 06/04/21 04:30PM, Simon Glass wrote:
Update this code to use IS_ENABLED() instead.
Signed-off-by: Simon Glass sjg@chromium.org
cmd/sf.c | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-)
diff --git a/cmd/sf.c b/cmd/sf.c index 46346fb9d43..d4f5ecee38c 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -384,7 +384,6 @@ static int do_spi_protect(int argc, char *const argv[]) return ret == 0 ? 0 : 1; }
-#ifdef CONFIG_CMD_SF_TEST enum { STAGE_ERASE, STAGE_CHECK, @@ -394,7 +393,7 @@ enum { STAGE_COUNT, };
-static char *stage_name[STAGE_COUNT] = { +static const char *stage_name[STAGE_COUNT] = { "erase", "check", "write", @@ -548,7 +547,6 @@ static int do_spi_flash_test(int argc, char *const argv[])
return 0; } -#endif /* CONFIG_CMD_SF_TEST */
Won't this cause all the test related functions to always be compiled into the binary? Then what's the point of having it behind a config at all?
We compile all functions into separate sections. The linker eliminates unused functions. Look for -ffunction-sections in files arch/<arch>/config.mk.
Ah, clever! This would rely on the compiler eliminating the dead if branch but the build uses -O2 or -Os, both of which should eliminate dead code.
Best regards
Heinrich
static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -582,10 +580,8 @@ static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, int argc, ret = do_spi_flash_erase(argc, argv); else if (strcmp(cmd, "protect") == 0) ret = do_spi_protect(argc, argv); -#ifdef CONFIG_CMD_SF_TEST
- else if (!strcmp(cmd, "test"))
- else if (IS_ENABLED(CONFIG_CMD_SF_TEST) && !strcmp(cmd, "test")) ret = do_spi_flash_test(argc, argv);
-#endif else ret = -1;
@@ -597,16 +593,8 @@ usage: return CMD_RET_USAGE; }
-#ifdef CONFIG_CMD_SF_TEST -#define SF_TEST_HELP "\nsf test offset len " \
"- run a very basic destructive test"
-#else -#define SF_TEST_HELP -#endif
-U_BOOT_CMD(
- sf, 5, 1, do_spi_flash,
- "SPI flash sub-system",
+#ifdef CONFIG_SYS_LONGHELP +static const char long_help[] = "probe [[bus:]cs] [hz] [mode] - init flash device on given SPI bus\n" " and chip select\n" "sf read addr offset|partition len - read `len' bytes starting at\n" @@ -622,6 +610,14 @@ U_BOOT_CMD( " at `addr' to flash at `offset'\n" " or to start of mtd `partition'\n" "sf protect lock/unlock sector len - protect/unprotect 'len' bytes starting\n"
- " at address 'sector'\n"
- SF_TEST_HELP
- " at address 'sector'"
+#ifdef CONFIG_CMD_SF_TEST
- "\nsf test offset len - run a very basic destructive test"
+#endif +#endif /* CONFIG_SYS_LONGHELP */
- ;
+U_BOOT_CMD(
- sf, 5, 1, do_spi_flash,
- "SPI flash sub-system", long_help
);
2.31.0.208.g409f899ff0-goog

This command is fairly complicated so documentation is useful. Unfortunately I an not sure how the MTD side of things works and cannot find information about that.
Signed-off-by: Simon Glass sjg@chromium.org ---
doc/usage/index.rst | 1 + doc/usage/sf.rst | 238 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 239 insertions(+) create mode 100644 doc/usage/sf.rst
diff --git a/doc/usage/index.rst b/doc/usage/index.rst index 35c515f8b59..a5d31bfae0d 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -34,5 +34,6 @@ Shell commands pstore qfw sbi + sf true scp03 diff --git a/doc/usage/sf.rst b/doc/usage/sf.rst new file mode 100644 index 00000000000..7ed077b89a7 --- /dev/null +++ b/doc/usage/sf.rst @@ -0,0 +1,238 @@ +.. SPDX-License-Identifier: GPL-2.0+: + +md command +========== + +Synopis +------- + +:: + + sf probe [[[<bus>:]<cs>] [<hz> [<mode>]]] + sf read <addr> <offset>|<partition> <len> + sf write <addr> <offset>|<partition> <len> + sf erase <offset>|<partition> <len> + sf update <addr> <offset>|<partition> <len> + sf protect lock|unlock <sector> <len> + sf test <offset>|<partition> <len> + +Description +----------- + +The *sf* command is used to access SPI flash, supporting read/write/erase and +a few other functions. + +The flash must first be probed with *sf probe* before any of the other +subcommands can be used. All of the parameters are optional: + +bus + SPI bus number containing the SPI-flash chip, e.g. 0. If you don't know + the number, you can use 'dm uclass' to see all the spi devices, + and check the value for 'seq' for each one (here 0 and 2):: + + uclass 89: spi + 0 spi@0 @ 05484960, seq 0 + 1 spi@1 @ 05484b40, seq 2 + +cs + SPI chip-select to use for the chip. This is often 0 and can be omitted, + but in some cases multiple slaves are attached to a SPI controller, + selected by a chip-select line for each one. + +hz + Speed of the SPI bus in hertz. This normally defaults to 100000, i.e. + 100KHz, which is very slow. Note that if the device exists in the + device tree, there might be a speed provided there, in which case this + setting is ignored. + +mode + SPI mode to use: + + ===== ================ + Mode Meaning + ===== ================ + 0 CPOL=0, CPHA=0 + 1 CPOL=0, CPHA=1 + 2 CPOL=1, CPHA=0 + 3 CPOL=1, CPHA=1 + ===== ================ + + CPHA 0 means that data is transferred (sampled) on the first clock + edge; 1 means the second. + + CPOL 0 controls the idle state of the clock, 0 for low, 1 for high. + The active state is the opposite of idle. + + You may find this `SPI documentation`_ useful. + +Once probed, the other subcommands can be used. Parameters are as follows: + +addr + Memory address to start transfer + +offset + Flash offset to start transfer + +partition + If the parameter is not numeric, it is assumed to be a partition + description in the format <dev_type><dev_num>,<part_num> which is not + covered here. This requires CONFIG_CMD_MTDPARTS. + +len + Number of bytes to transfer + + +Read +---- + +Use *sf read* to read from SPI flash to memory. The read will fail if an +attempt is made to read past the end of the flash. + + +Write +----- + +Use *sf write* to write from memory to SPI flash. The SPI flash should be +erased first, since otherwise the result is undefined. + +The write will fail if an attempt is made to read past the end of the flash. + + +Erase +----- + +Use *sf erase* to erase a region of SPI flash. The erase will fail if any part +of the region to be erased is protected or lies past the end of the flash. It +may also fail if the start offset or length are not aligned to an erase region +(e.g. 256 bytes). + + +Update +------ + +Use *sf update* to automatically erase and update a region of SPI flash from +memory. This works a sector at a time (typical 256 bytes or 4KB). For each +sector it first checks if the sector already has the right data. If so it is +skipped. If not, the sector is erased and the new data written. Note that if +the length is not a multiple of the erase size, the space after the data in +the last sector will be erased. If the offset does not start at the beginning +of an erase block, the operation will fail. + +Speed statistics are shown including the number of bytes that were already +correct. + + +Protect +------- + +SPI-flash chips often have a protection feature where the chip is split up into +regions which can be locked or unlocked. With *sf protect* it is possible to +change these settings, if supported by the driver. + +lock|unlock + Selects whether to lock or unlock the sectors + +<sector> + Start sector number to lock/unlock. This may be the byte offset or some + other value, depending on the chip. + +<len> + Number of bytes to lock/unlock + + +Test +---- + +A convenient and fast *sf test* subcommand provides a way to check that SPI +flash is working as expected. This works in four stages: + + * erase - erases the entire region + * check - checks that the region is erased + * write - writes a test pattern to the region, consisting of the U-Boot code + * read - reads back the test pattern to check that it was written correctly + +Memory is allocated for two buffers, each <len> bytes in size. At typical +size is 64KB to 1MB. The offset and size must be aligned to an erase boundary. + +Note that this test will fail if any part of the SPI flash is write-protected. + + +Example +------- + +:: + + # This first part uses sandbox + => sf read 1000 1100 80000 + device 0 offset 0x1100, size 0x80000 + SF: 524288 bytes @ 0x1100 Read: OK + => md 1000 + 00001000: edfe0dd0 f33a0000 78000000 84250000 ......:....x..%. + 00001010: 28000000 11000000 10000000 00000000 ...(............ + 00001020: 6f050000 0c250000 00000000 00000000 ...o..%......... + 00001030: 00000000 00000000 00000000 00000000 ................ + 00001040: 00000000 00000000 00000000 00000000 ................ + 00001050: 00000000 00000000 00000000 00000000 ................ + 00001060: 00000000 00000000 00000000 00000000 ................ + 00001070: 00000000 00000000 01000000 00000000 ................ + 00001080: 03000000 04000000 00000000 01000000 ................ + 00001090: 03000000 04000000 0f000000 01000000 ................ + 000010a0: 03000000 08000000 1b000000 646e6173 ............sand + 000010b0: 00786f62 03000000 08000000 21000000 box............! + 000010c0: 646e6173 00786f62 01000000 61696c61 sandbox.....alia + 000010d0: 00736573 03000000 07000000 2c000000 ses............, + 000010e0: 6332692f 00003040 03000000 07000000 /i2c@0.......... + 000010f0: 31000000 6963702f 00003040 03000000 ...1/pci@0...... + => sf erase 0 80000 + SF: 524288 bytes @ 0x0 Erased: OK + => sf read 1000 1100 80000 + device 0 offset 0x1100, size 0x80000 + SF: 524288 bytes @ 0x1100 Read: OK + => md 1000 + 00001000: ffffffff ffffffff ffffffff ffffffff ................ + 00001010: ffffffff ffffffff ffffffff ffffffff ................ + 00001020: ffffffff ffffffff ffffffff ffffffff ................ + 00001030: ffffffff ffffffff ffffffff ffffffff ................ + 00001040: ffffffff ffffffff ffffffff ffffffff ................ + 00001050: ffffffff ffffffff ffffffff ffffffff ................ + 00001060: ffffffff ffffffff ffffffff ffffffff ................ + 00001070: ffffffff ffffffff ffffffff ffffffff ................ + 00001080: ffffffff ffffffff ffffffff ffffffff ................ + 00001090: ffffffff ffffffff ffffffff ffffffff ................ + 000010a0: ffffffff ffffffff ffffffff ffffffff ................ + 000010b0: ffffffff ffffffff ffffffff ffffffff ................ + 000010c0: ffffffff ffffffff ffffffff ffffffff ................ + 000010d0: ffffffff ffffffff ffffffff ffffffff ................ + 000010e0: ffffffff ffffffff ffffffff ffffffff ................ + 000010f0: ffffffff ffffffff ffffffff ffffffff ................ + + # This next part is running on coral, an x86 Chromebook + => sf erase 300000 80000 + SF: 524288 bytes @ 0x300000 Erased: OK + => sf update 1110000 300000 80000 + device 0 offset 0x300000, size 0x80000 + 524288 bytes written, 0 bytes skipped in 0.457s, speed 1164578 B/s + + # This does nothing as the flash is already updated + => sf update 1110000 300000 80000 + device 0 offset 0x300000, size 0x80000 + 0 bytes written, 524288 bytes skipped in 0.196s, speed 2684354 B/s + => sf test 00000 80000 # try a protected region + SPI flash test: + Erase failed (err = -5) + Test failed + => sf test 800000 80000 + SPI flash test: + 0 erase: 18 ticks, 28444 KiB/s 227.552 Mbps + 1 check: 192 ticks, 2666 KiB/s 21.328 Mbps + 2 write: 227 ticks, 2255 KiB/s 18.040 Mbps + 3 read: 189 ticks, 2708 KiB/s 21.664 Mbps + Test passed + 0 erase: 18 ticks, 28444 KiB/s 227.552 Mbps + 1 check: 192 ticks, 2666 KiB/s 21.328 Mbps + 2 write: 227 ticks, 2255 KiB/s 18.040 Mbps + 3 read: 189 ticks, 2708 KiB/s 21.664 Mbps + + +.. _SPI documentation: + https://en.wikipedia.org/wiki/Serial_Peripheral_Interface

On 06.04.21 06:30, Simon Glass wrote:
This command is fairly complicated so documentation is useful. Unfortunately I an not sure how the MTD side of things works and cannot find information about that.
Signed-off-by: Simon Glass sjg@chromium.org
doc/usage/index.rst | 1 + doc/usage/sf.rst | 238 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 239 insertions(+) create mode 100644 doc/usage/sf.rst
diff --git a/doc/usage/index.rst b/doc/usage/index.rst index 35c515f8b59..a5d31bfae0d 100644 --- a/doc/usage/index.rst +++ b/doc/usage/index.rst @@ -34,5 +34,6 @@ Shell commands pstore qfw sbi
- sf true scp03
diff --git a/doc/usage/sf.rst b/doc/usage/sf.rst new file mode 100644 index 00000000000..7ed077b89a7 --- /dev/null +++ b/doc/usage/sf.rst @@ -0,0 +1,238 @@ +.. SPDX-License-Identifier: GPL-2.0+:
+md command
Thank you for documenting the sf command.
%/md/sf/
+==========
+Synopis +-------
+::
- sf probe [[[<bus>:]<cs>] [<hz> [<mode>]]]
- sf read <addr> <offset>|<partition> <len>
- sf write <addr> <offset>|<partition> <len>
- sf erase <offset>|<partition> <len>
- sf update <addr> <offset>|<partition> <len>
- sf protect lock|unlock <sector> <len>
- sf test <offset>|<partition> <len>
+Description +-----------
+The *sf* command is used to access SPI flash, supporting read/write/erase and +a few other functions.
For all subcommands except 'sf probe' you have a heading. This somehow looks inconsistent. Maybe you want to add a heading after all arguments.
+The flash must first be probed with *sf probe* before any of the other +subcommands can be used. All of the parameters are optional:
+bus
- SPI bus number containing the SPI-flash chip, e.g. 0. If you don't know
- the number, you can use 'dm uclass' to see all the spi devices,
- and check the value for 'seq' for each one (here 0 and 2)::
uclass 89: spi
0 spi@0 @ 05484960, seq 0
1 spi@1 @ 05484b40, seq 2
+cs
- SPI chip-select to use for the chip. This is often 0 and can be omitted,
- but in some cases multiple slaves are attached to a SPI controller,
- selected by a chip-select line for each one.
+hz
- Speed of the SPI bus in hertz. This normally defaults to 100000, i.e.
- 100KHz, which is very slow. Note that if the device exists in the
- device tree, there might be a speed provided there, in which case this
- setting is ignored.
+mode
- SPI mode to use:
- ===== ================
- Mode Meaning
- ===== ================
- 0 CPOL=0, CPHA=0
- 1 CPOL=0, CPHA=1
- 2 CPOL=1, CPHA=0
- 3 CPOL=1, CPHA=1
- ===== ================
- CPHA 0 means that data is transferred (sampled) on the first clock
The clock phase (CPHA) ...
- edge; 1 means the second.
- CPOL 0 controls the idle state of the clock, 0 for low, 1 for high.
The clock polarity (CPOL) controls ...
- The active state is the opposite of idle.
- You may find this `SPI documentation`_ useful.
+Once probed, the other subcommands can be used. Parameters are as follows:
This sentence is redundant. You stated already above that probe is the first subcommand to be executed.
+addr
- Memory address to start transfer
+offset
- Flash offset to start transfer
+partition
- If the parameter is not numeric, it is assumed to be a partition
- description in the format <dev_type><dev_num>,<part_num> which is not
- covered here. This requires CONFIG_CMD_MTDPARTS.
+len
- Number of bytes to transfer
+Read +----
Please, use a lower heading level than for "Description". Using 'sf read' as text makes it clear that this is about the sub-command:
sf read ~~~~~~~
+Use *sf read* to read from SPI flash to memory. The read will fail if an +attempt is made to read past the end of the flash.
+Write +-----
sf write ~~~~~~~~
+Use *sf write* to write from memory to SPI flash. The SPI flash should be +erased first, since otherwise the result is undefined.
+The write will fail if an attempt is made to read past the end of the flash.
+Erase +-----
sf erase ~~~~~~~~
+Use *sf erase* to erase a region of SPI flash. The erase will fail if any part +of the region to be erased is protected or lies past the end of the flash. It +may also fail if the start offset or length are not aligned to an erase region +(e.g. 256 bytes).
+Update +------
sf update ~~~~~~~~~
+Use *sf update* to automatically erase and update a region of SPI flash from +memory. This works a sector at a time (typical 256 bytes or 4KB). For each +sector it first checks if the sector already has the right data. If so it is +skipped. If not, the sector is erased and the new data written. Note that if +the length is not a multiple of the erase size, the space after the data in +the last sector will be erased. If the offset does not start at the beginning +of an erase block, the operation will fail.
+Speed statistics are shown including the number of bytes that were already +correct.
+Protect +-------
sf protect ~~~~~~~~~~
+SPI-flash chips often have a protection feature where the chip is split up into +regions which can be locked or unlocked. With *sf protect* it is possible to +change these settings, if supported by the driver.
+lock|unlock
- Selects whether to lock or unlock the sectors
+<sector>
- Start sector number to lock/unlock. This may be the byte offset or some
- other value, depending on the chip.
+<len>
- Number of bytes to lock/unlock
+Test +----
+A convenient and fast *sf test* subcommand provides a way to check that SPI +flash is working as expected. This works in four stages:
- erase - erases the entire region
- check - checks that the region is erased
- write - writes a test pattern to the region, consisting of the U-Boot code
- read - reads back the test pattern to check that it was written correctly
+Memory is allocated for two buffers, each <len> bytes in size. At typical +size is 64KB to 1MB. The offset and size must be aligned to an erase boundary.
+Note that this test will fail if any part of the SPI flash is write-protected.
+Example +-------
You have two examples.
Examples --------
+::
- # This first part uses sandbox
This sentence is not coding. Please, place it above the code block.
This first example uses the sandbox
::
- => sf read 1000 1100 80000
- device 0 offset 0x1100, size 0x80000
- SF: 524288 bytes @ 0x1100 Read: OK
- => md 1000
- 00001000: edfe0dd0 f33a0000 78000000 84250000 ......:....x..%.
- 00001010: 28000000 11000000 10000000 00000000 ...(............
- 00001020: 6f050000 0c250000 00000000 00000000 ...o..%.........
- 00001030: 00000000 00000000 00000000 00000000 ................
- 00001040: 00000000 00000000 00000000 00000000 ................
- 00001050: 00000000 00000000 00000000 00000000 ................
- 00001060: 00000000 00000000 00000000 00000000 ................
- 00001070: 00000000 00000000 01000000 00000000 ................
- 00001080: 03000000 04000000 00000000 01000000 ................
- 00001090: 03000000 04000000 0f000000 01000000 ................
- 000010a0: 03000000 08000000 1b000000 646e6173 ............sand
- 000010b0: 00786f62 03000000 08000000 21000000 box............!
- 000010c0: 646e6173 00786f62 01000000 61696c61 sandbox.....alia
- 000010d0: 00736573 03000000 07000000 2c000000 ses............,
- 000010e0: 6332692f 00003040 03000000 07000000 /i2c@0..........
- 000010f0: 31000000 6963702f 00003040 03000000 ...1/pci@0......
- => sf erase 0 80000
- SF: 524288 bytes @ 0x0 Erased: OK
- => sf read 1000 1100 80000
- device 0 offset 0x1100, size 0x80000
- SF: 524288 bytes @ 0x1100 Read: OK
- => md 1000
- 00001000: ffffffff ffffffff ffffffff ffffffff ................
- 00001010: ffffffff ffffffff ffffffff ffffffff ................
- 00001020: ffffffff ffffffff ffffffff ffffffff ................
- 00001030: ffffffff ffffffff ffffffff ffffffff ................
- 00001040: ffffffff ffffffff ffffffff ffffffff ................
- 00001050: ffffffff ffffffff ffffffff ffffffff ................
- 00001060: ffffffff ffffffff ffffffff ffffffff ................
- 00001070: ffffffff ffffffff ffffffff ffffffff ................
- 00001080: ffffffff ffffffff ffffffff ffffffff ................
- 00001090: ffffffff ffffffff ffffffff ffffffff ................
- 000010a0: ffffffff ffffffff ffffffff ffffffff ................
- 000010b0: ffffffff ffffffff ffffffff ffffffff ................
- 000010c0: ffffffff ffffffff ffffffff ffffffff ................
- 000010d0: ffffffff ffffffff ffffffff ffffffff ................
- 000010e0: ffffffff ffffffff ffffffff ffffffff ................
- 000010f0: ffffffff ffffffff ffffffff ffffffff ................
- # This next part is running on coral, an x86 Chromebook
This is a second independent example. So end the code block here. Put the description before the next code block:
This second example is running on coral, an x86 Chromebook
::
Best regards
Heinrich
- => sf erase 300000 80000
- SF: 524288 bytes @ 0x300000 Erased: OK
- => sf update 1110000 300000 80000
- device 0 offset 0x300000, size 0x80000
- 524288 bytes written, 0 bytes skipped in 0.457s, speed 1164578 B/s
- # This does nothing as the flash is already updated
- => sf update 1110000 300000 80000
- device 0 offset 0x300000, size 0x80000
- 0 bytes written, 524288 bytes skipped in 0.196s, speed 2684354 B/s
- => sf test 00000 80000 # try a protected region
- SPI flash test:
- Erase failed (err = -5)
- Test failed
- => sf test 800000 80000
- SPI flash test:
- 0 erase: 18 ticks, 28444 KiB/s 227.552 Mbps
- 1 check: 192 ticks, 2666 KiB/s 21.328 Mbps
- 2 write: 227 ticks, 2255 KiB/s 18.040 Mbps
- 3 read: 189 ticks, 2708 KiB/s 21.664 Mbps
- Test passed
- 0 erase: 18 ticks, 28444 KiB/s 227.552 Mbps
- 1 check: 192 ticks, 2666 KiB/s 21.328 Mbps
- 2 write: 227 ticks, 2255 KiB/s 18.040 Mbps
- 3 read: 189 ticks, 2708 KiB/s 21.664 Mbps
+.. _SPI documentation:

Add a new 'sf mmap' function to show the address of a SPI offset, if the hardware supports it. This is useful on x86 systems.
Signed-off-by: Simon Glass sjg@chromium.org ---
arch/Kconfig | 2 ++ cmd/Kconfig | 8 ++++++++ cmd/sf.c | 35 +++++++++++++++++++++++++++++++++ doc/usage/sf.rst | 50 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 95 insertions(+)
diff --git a/arch/Kconfig b/arch/Kconfig index 70232239278..55f86898e0e 100644 --- a/arch/Kconfig +++ b/arch/Kconfig @@ -131,6 +131,7 @@ config SANDBOX imply CMD_LZMADEC imply CMD_SATA imply CMD_SF + imply CMD_SF_MMAP imply CMD_SF_TEST imply CRC32_VERIFY imply FAT_WRITE @@ -192,6 +193,7 @@ config X86 imply CMD_IRQ imply CMD_PCI imply CMD_SF + imply CMD_SF_MMAP imply CMD_SF_TEST imply CMD_ZBOOT imply DM_ETH diff --git a/cmd/Kconfig b/cmd/Kconfig index e4bb1d4c4a7..85d83155dac 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1260,6 +1260,14 @@ config CMD_SF help SPI Flash support
+config CMD_SF_MMAP + bool "sf mmap - Access memory-mapped SPI flash" + depends on CMD_SF + help + On some systems part of the SPI flash is mapped into mmemory. This + command provides a way to map a SPI-flash offset to a memory address, + so that the contents can be browsed using 'md', for example. + config CMD_SF_TEST bool "sf test - Allow testing of SPI flash" depends on CMD_SF diff --git a/cmd/sf.c b/cmd/sf.c index d4f5ecee38c..28b480acc58 100644 --- a/cmd/sf.c +++ b/cmd/sf.c @@ -384,6 +384,36 @@ static int do_spi_protect(int argc, char *const argv[]) return ret == 0 ? 0 : 1; }
+static int do_spi_flash_mmap(int argc, char *const argv[]) +{ + loff_t offset, len, maxsize; + uint map_offset, map_size; + ulong map_base; + int dev = 0; + int ret; + + if (argc < 2) + return CMD_RET_USAGE; + + ret = mtd_arg_off_size(argc - 1, &argv[1], &dev, &offset, &len, + &maxsize, MTD_DEV_TYPE_NOR, flash->size); + if (ret) + return ret; + + ret = dm_spi_get_mmap(flash->dev, &map_base, &map_size, &map_offset); + if (ret) { + printf("Mapping not available (err=%d)\n", ret); + return CMD_RET_FAILURE; + } + if (offset < 0 || offset + len > map_size) { + printf("Offset out of range (map size %x)\n", map_size); + return CMD_RET_FAILURE; + } + printf("%lx\n", map_base + (ulong)offset); + + return 0; +} + enum { STAGE_ERASE, STAGE_CHECK, @@ -580,6 +610,8 @@ static int do_spi_flash(struct cmd_tbl *cmdtp, int flag, int argc, ret = do_spi_flash_erase(argc, argv); else if (strcmp(cmd, "protect") == 0) ret = do_spi_protect(argc, argv); + else if (IS_ENABLED(CONFIG_CMD_SF_MMAP) && !strcmp(cmd, "mmap")) + ret = do_spi_flash_mmap(argc, argv); else if (IS_ENABLED(CONFIG_CMD_SF_TEST) && !strcmp(cmd, "test")) ret = do_spi_flash_test(argc, argv); else @@ -611,6 +643,9 @@ static const char long_help[] = " or to start of mtd `partition'\n" "sf protect lock/unlock sector len - protect/unprotect 'len' bytes starting\n" " at address 'sector'" +#ifdef CONFIG_CMD_SF_MMAP + "\nsf mmap offset len - get memory address of SPI-flash offset\n" +#endif #ifdef CONFIG_CMD_SF_TEST "\nsf test offset len - run a very basic destructive test" #endif diff --git a/doc/usage/sf.rst b/doc/usage/sf.rst index 7ed077b89a7..c1ef728f714 100644 --- a/doc/usage/sf.rst +++ b/doc/usage/sf.rst @@ -14,6 +14,7 @@ Synopis sf erase <offset>|<partition> <len> sf update <addr> <offset>|<partition> <len> sf protect lock|unlock <sector> <len> + sf mmap <offset>|<partition> <len> sf test <offset>|<partition> <len>
Description @@ -140,6 +141,16 @@ lock|unlock Number of bytes to lock/unlock
+Memory-mapped flash +------------------- + +On some systems part of the SPI flash is mapped into mmemory. With *sf mmap* +you can map a SPI-flash offset to a memory address, so that the contents can be +browsed using 'md', for example. + +The command will fail if this is not supported by the hardware or driver. + + Test ----
@@ -233,6 +244,45 @@ Example 2 write: 227 ticks, 2255 KiB/s 18.040 Mbps 3 read: 189 ticks, 2708 KiB/s 21.664 Mbps
+ # On coral, SPI flash offset 0 corresponds to address ff081000 in memory + => sf mmap 0 1000 + device 0 offset 0x0, size 0x1000 + ff081000 + + # See below for how this address was obtained + => sf mmap e80000 11e18 + device 0 offset 0xe80000, size 0x11e18 + fff01000 + => md fff01000 + fff01000: b2e8e089 89000030 30b4e8c4 c0310000 ....0......0..1. + fff01010: 002c95e8 2ce8e800 feeb0000 dfe8c489 ..,....,........ + fff01020: f400002c 83f4fdeb d4e80cec 3100001e ,..............1 + fff01030: 0cc483c0 f883c3c3 8b0b770a df408504 .........w....@. + fff01040: c085fef1 c8b80575 c3fef1e5 53565755 ....u.......UWVS + fff01050: 89c38951 2404c7c5 80000002 8924048b Q......$......$. + fff01060: 89a20fdf 89fb89de 75890045 084d8904 ........E..u..M. + fff01070: ff0c5589 c5832404 243c8110 80000005 .U...$....<$.... + fff01080: 43c6da75 3b800030 43037520 d889f8eb u..C0..; u.C.... + fff01090: 5f5e5b5a 80e6c35d 535657c3 e7e8c689 Z[^_]....WVS.... + fff010a0: 89000069 00a164c3 8b000000 408b4c56 i....d......VL.@ + fff010b0: 0cec837c ddb9ff6a e8fef1e5 00004e01 |...j........N.. + fff010c0: a1640389 00000000 1c80b60f 66000001 ..d............f + fff010d0: b80c4389 00000001 a20fdf89 fb89de89 .C.............. + fff010e0: 89104389 c4831453 5bc03110 56c35f5e .C..S....1.[^_.V + fff010f0: 14ec8353 ce89d389 0000a164 b60f0000 S.......d....... + + +The offset e80000 was obtained using the following binman command, to find the +location of U-Boot SPL:: + + $ binman ls -i /tmp/b/chromebook_coral/u-boot.rom + Name Image-pos Size Entry-type Offset Uncomp-size + ------------------------------------------------------------------------------------------------------ + main-section 0 1000000 section 0 + spl e80000 11e18 u-boot-spl ffe80000 + u-boot d00000 9106e u-boot ffd00000 + ... +
.. _SPI documentation: https://en.wikipedia.org/wiki/Serial_Peripheral_Interface
participants (4)
-
Heinrich Schuchardt
-
Pratyush Yadav
-
Sean Anderson
-
Simon Glass