[U-Boot][PATCH v4 0/5] Improve fpga cmd usage information

Current usage information is not read friendly specially when all the options are selected. This series fix some of the issues in the info, aligment and overlapping issues, excess of verbosity on the loads...
Additionally Xilinx exclusive command dependencies are included to remove text stating Xilinx Only
The output looks like this now:
fpga - loadable FPGA image support
Usage: fpga info [dev] List known device information fpga dump <dev> <address> <size> Load device to memory buffer fpga load <dev> <address> <size> Load device from memory buffer fpga loadb <dev> <address> <size> Load device from bitstream buffer fpga loadp <dev> <address> <size> Load device from memory buffer with partial bitstream fpga loadbp <dev> <address> <size> Load device from bitstream buffer with partial bitstream fpga loadfs <dev> <address> <size> <blocksize> <interface> [<dev[:part]>] <filename> Load device from filesystem (FAT by default) fpga loadmk <dev> <address> Load device generated with mkimage NOTE: loadmk operating on FIT must include subimage unit name in the form of addr:<subimg_uname> fpga loads <dev> <address> <size> <authflag> <encflag> [Userkey address] Load device from memory buffer with secure bistream (authenticated/encrypted/both) -authflag: 0 for OCM, 1 for DDR, 2 for no authentication (specifies where to perform authentication) -encflag: 0 for device key, 1 for user key, 2 for no encryption -Userkey address: address where user key is stored NOTE: secure bitstream has to be created using Xilinx bootgen tool
Changes in v4: - Split 4th patch into two, one for existing symbol dependencies and one for the new symbol
Changes in v3: - Split 3rd patch into two, resorting commands and Kconfig changes
Changes in v2: - Tabs replaced by spaces - fpga command added on each option similar to other commands like sf - Dependencies to Xilinx only commands added in Kconfig
Ibai Erkiaga (5): fpga: fix alignment on fpga cmd usage info fpga: improve loads usage information fpga: resort fpga commands fpga: xilinx exclusive commands fpga: add new symbol for fpga_loadb
cmd/Kconfig | 31 ++++++++++++++++----------- cmd/fpga.c | 62 ++++++++++++++++++++++++----------------------------- 2 files changed, 47 insertions(+), 46 deletions(-)

The current implementation generates some alignment issues as well as some overlapping when all the fpga command options are enabled. The fix is intended to improve readability of the usage info.
Signed-off-by: Ibai Erkiaga ibai.erkiaga-elorza@amd.com ---
Changes in v4: - Split 4th patch into two, one for existing symbol dependencies and one for the new symbol
Changes in v3: - Split 3rd patch into two, resorting commands and Kconfig changes
Changes in v2: - Tabs replaced by spaces - fpga command added on each option similar to other commands like sf - Dependencies to Xilinx only commands added in Kconfig
cmd/fpga.c | 61 +++++++++++++++++++++++++----------------------------- 1 file changed, 28 insertions(+), 33 deletions(-)
diff --git a/cmd/fpga.c b/cmd/fpga.c index 93f14098ccb..ae85ff201b8 100644 --- a/cmd/fpga.c +++ b/cmd/fpga.c @@ -408,49 +408,44 @@ U_BOOT_CMD(fpga, 9, 1, do_fpga_wrapper, #else U_BOOT_CMD(fpga, 6, 1, do_fpga_wrapper, #endif - "loadable FPGA image support", - "[operation type] [device number] [image address] [image size]\n" - "fpga operations:\n" - " dump\t[dev] [address] [size]\tLoad device to memory buffer\n" - " info\t[dev]\t\t\tlist known device information\n" - " load\t[dev] [address] [size]\tLoad device from memory buffer\n" + "loadable FPGA image support", + "info [dev] List known device information\n" + "fpga dump <dev> <address> <size> Load device to memory buffer\n" + "fpga load <dev> <address> <size> Load device from memory buffer\n" #if defined(CONFIG_CMD_FPGA_LOADP) - " loadp\t[dev] [address] [size]\t" - "Load device from memory buffer with partial bitstream\n" + "fpga loadp <dev> <address> <size> Load device from memory buffer\n" + " with partial bitstream\n" #endif - " loadb\t[dev] [address] [size]\t" - "Load device from bitstream buffer (Xilinx only)\n" + "fpga loadb <dev> <address> <size> Load device from bitstream buffer\n" + " (Xilinx only)\n" #if defined(CONFIG_CMD_FPGA_LOADBP) - " loadbp\t[dev] [address] [size]\t" - "Load device from bitstream buffer with partial bitstream" - "(Xilinx only)\n" + "fpga loadbp <dev> <address> <size> Load device from bitstream buffer\n" + " with partial bitstream (Xilinx only)\n" #endif #if defined(CONFIG_CMD_FPGA_LOADFS) - "Load device from filesystem (FAT by default) (Xilinx only)\n" - " loadfs [dev] [address] [image size] [blocksize] <interface>\n" - " [<dev[:part]>] <filename>\n" + "fpga loadfs <dev> <address> <size> <blocksize> <interface> [<dev[:part]>] <filename>\n" + " Load device from filesystem (FAT by default) (Xilinx only)\n" #endif #if defined(CONFIG_CMD_FPGA_LOADMK) - " loadmk [dev] [address]\tLoad device generated with mkimage" + "fpga loadmk <dev> <address> Load device generated with mkimage\n" #if defined(CONFIG_FIT) - "\n" - "\tFor loadmk operating on FIT format uImage address must include\n" - "\tsubimage unit name in the form of addr:<subimg_uname>" + " NOTE: loadmk operating on FIT must include subimage unit\n" + " name in the form of addr:<subimg_uname>\n" #endif #endif #if defined(CONFIG_CMD_FPGA_LOAD_SECURE) - "Load encrypted bitstream (Xilinx only)\n" - " loads [dev] [address] [size] [auth-OCM-0/DDR-1/noauth-2]\n" - " [enc-devkey(0)/userkey(1)/nenc(2) [Userkey address]\n" - "Loads the secure bistreams(authenticated/encrypted/both\n" - "authenticated and encrypted) of [size] from [address].\n" - "The auth-OCM/DDR flag specifies to perform authentication\n" - "in OCM or in DDR. 0 for OCM, 1 for DDR, 2 for no authentication.\n" - "The enc flag specifies which key to be used for decryption\n" - "0-device key, 1-user key, 2-no encryption.\n" - "The optional Userkey address specifies from which address key\n" - "has to be used for decryption if user key is selected.\n" - "NOTE: the secure bitstream has to be created using Xilinx\n" - "bootgen tool only.\n" + "fpga loads <dev> <address> <size> <auth-OCM-0/DDR-1/noauth-2>\n" + " <enc-devkey(0)/userkey(1)/nenc(2> [Userkey address]\n" + " Loads the secure bistreams(authenticated/encrypted/both\n" + " authenticated and encrypted) of [size] from [address]\n" + " (Xilinx only)\n" + " -The auth-OCM/DDR flag specifies to perform authentication\n" + " in OCM or in DDR. 0 for OCM, 1 for DDR, 2 for no authentication\n" + " -The enc flag specifies which key to be used for decryption\n" + " 0-device key, 1-user key, 2-no encryption.\n" + " -The optional Userkey address specifies from which address key\n" + " has to be used for decryption if user key is selected.\n" + " NOTE: the secure bitstream has to be created using Xilinx\n" + " bootgen tool only.\n" #endif );

On 1/20/25 13:43, Ibai Erkiaga wrote:
The current implementation generates some alignment issues as well as some overlapping when all the fpga command options are enabled. The fix is intended to improve readability of the usage info.
Signed-off-by: Ibai Erkiaga ibai.erkiaga-elorza@amd.com
You got ack for this one but I can't see it here.
M

Current usage information for loads command is too verbose and long for a command usage prompt. This flag simplifies the text for readability purposes.
Signed-off-by: Ibai Erkiaga ibai.erkiaga-elorza@amd.com ---
(no changes since v1)
cmd/fpga.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-)
diff --git a/cmd/fpga.c b/cmd/fpga.c index ae85ff201b8..91ccbee0fef 100644 --- a/cmd/fpga.c +++ b/cmd/fpga.c @@ -434,18 +434,13 @@ U_BOOT_CMD(fpga, 6, 1, do_fpga_wrapper, #endif #endif #if defined(CONFIG_CMD_FPGA_LOAD_SECURE) - "fpga loads <dev> <address> <size> <auth-OCM-0/DDR-1/noauth-2>\n" - " <enc-devkey(0)/userkey(1)/nenc(2> [Userkey address]\n" - " Loads the secure bistreams(authenticated/encrypted/both\n" - " authenticated and encrypted) of [size] from [address]\n" - " (Xilinx only)\n" - " -The auth-OCM/DDR flag specifies to perform authentication\n" - " in OCM or in DDR. 0 for OCM, 1 for DDR, 2 for no authentication\n" - " -The enc flag specifies which key to be used for decryption\n" - " 0-device key, 1-user key, 2-no encryption.\n" - " -The optional Userkey address specifies from which address key\n" - " has to be used for decryption if user key is selected.\n" - " NOTE: the secure bitstream has to be created using Xilinx\n" - " bootgen tool only.\n" + "fpga loads <dev> <address> <size> <authflag> <encflag> [Userkey address]\n" + " Load device from memory buffer with secure bistream\n" + " (authenticated/encrypted/both)(Xilinx only)\n" + " -authflag: 0 for OCM, 1 for DDR, 2 for no authentication\n" + " (specifies where to perform authentication)\n" + " -encflag: 0 for device key, 1 for user key, 2 for no encryption\n" + " -Userkey address: address where user key is stored\n" + " NOTE: secure bitstream has to be created using Xilinx bootgen tool\n" #endif );

Resort the fpga commands both in the Kconfig and in the source code to list
Signed-off-by: Ibai Erkiaga ibai.erkiaga-elorza@amd.com ---
(no changes since v1)
cmd/Kconfig | 14 +++++++------- cmd/fpga.c | 4 ++-- 2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index 1a0985ca479..b362595af83 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1179,6 +1179,13 @@ config CMD_FPGA help FPGA support.
+config CMD_FPGA_LOADP + bool "fpga loadp - load partial bitstream" + depends on CMD_FPGA + help + Supports loading an FPGA device from a bitstream buffer containing + a partial bitstream. + config CMD_FPGA_LOADBP bool "fpga loadbp - load partial bitstream (Xilinx only)" depends on CMD_FPGA @@ -1198,13 +1205,6 @@ config CMD_FPGA_LOADMK help Supports loading an FPGA device from a image generated by mkimage.
-config CMD_FPGA_LOADP - bool "fpga loadp - load partial bitstream" - depends on CMD_FPGA - help - Supports loading an FPGA device from a bitstream buffer containing - a partial bitstream. - config CMD_FPGA_LOAD_SECURE bool "fpga loads - loads secure bitstreams" depends on CMD_FPGA diff --git a/cmd/fpga.c b/cmd/fpga.c index 91ccbee0fef..13aabc95abc 100644 --- a/cmd/fpga.c +++ b/cmd/fpga.c @@ -412,12 +412,12 @@ U_BOOT_CMD(fpga, 6, 1, do_fpga_wrapper, "info [dev] List known device information\n" "fpga dump <dev> <address> <size> Load device to memory buffer\n" "fpga load <dev> <address> <size> Load device from memory buffer\n" + "fpga loadb <dev> <address> <size> Load device from bitstream buffer\n" + " (Xilinx only)\n" #if defined(CONFIG_CMD_FPGA_LOADP) "fpga loadp <dev> <address> <size> Load device from memory buffer\n" " with partial bitstream\n" #endif - "fpga loadb <dev> <address> <size> Load device from bitstream buffer\n" - " (Xilinx only)\n" #if defined(CONFIG_CMD_FPGA_LOADBP) "fpga loadbp <dev> <address> <size> Load device from bitstream buffer\n" " with partial bitstream (Xilinx only)\n"

Ensure all Xilinx exclusive fpga commands have a KConfig symbol and dependency to FPGA_XILINX listed. Remove (Xilinx only) text from the help command.
Signed-off-by: Ibai Erkiaga ibai.erkiaga-elorza@amd.com ---
(no changes since v1)
cmd/Kconfig | 10 +++++----- cmd/fpga.c | 6 +++--- 2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index b362595af83..c9600498787 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1187,15 +1187,15 @@ config CMD_FPGA_LOADP a partial bitstream.
config CMD_FPGA_LOADBP - bool "fpga loadbp - load partial bitstream (Xilinx only)" - depends on CMD_FPGA + bool "fpga loadbp - load partial bitstream" + depends on CMD_FPGA && FPGA_XILINX help Supports loading an FPGA device from a bitstream buffer containing a partial bitstream.
config CMD_FPGA_LOADFS - bool "fpga loadfs - load bitstream from FAT filesystem (Xilinx only)" - depends on CMD_FPGA + bool "fpga loadfs - load bitstream from FAT filesystem" + depends on CMD_FPGA && FPGA_XILINX help Supports loading an FPGA device from a FAT filesystem.
@@ -1207,7 +1207,7 @@ config CMD_FPGA_LOADMK
config CMD_FPGA_LOAD_SECURE bool "fpga loads - loads secure bitstreams" - depends on CMD_FPGA + depends on CMD_FPGA && FPGA_XILINX select FPGA_LOAD_SECURE help Enables the fpga loads command which is used to load secure diff --git a/cmd/fpga.c b/cmd/fpga.c index 13aabc95abc..75d2f824998 100644 --- a/cmd/fpga.c +++ b/cmd/fpga.c @@ -420,11 +420,11 @@ U_BOOT_CMD(fpga, 6, 1, do_fpga_wrapper, #endif #if defined(CONFIG_CMD_FPGA_LOADBP) "fpga loadbp <dev> <address> <size> Load device from bitstream buffer\n" - " with partial bitstream (Xilinx only)\n" + " with partial bitstream\n" #endif #if defined(CONFIG_CMD_FPGA_LOADFS) "fpga loadfs <dev> <address> <size> <blocksize> <interface> [<dev[:part]>] <filename>\n" - " Load device from filesystem (FAT by default) (Xilinx only)\n" + " Load device from filesystem (FAT by default)\n" #endif #if defined(CONFIG_CMD_FPGA_LOADMK) "fpga loadmk <dev> <address> Load device generated with mkimage\n" @@ -436,7 +436,7 @@ U_BOOT_CMD(fpga, 6, 1, do_fpga_wrapper, #if defined(CONFIG_CMD_FPGA_LOAD_SECURE) "fpga loads <dev> <address> <size> <authflag> <encflag> [Userkey address]\n" " Load device from memory buffer with secure bistream\n" - " (authenticated/encrypted/both)(Xilinx only)\n" + " (authenticated/encrypted/both)\n" " -authflag: 0 for OCM, 1 for DDR, 2 for no authentication\n" " (specifies where to perform authentication)\n" " -encflag: 0 for device key, 1 for user key, 2 for no encryption\n"

Adding new symbol for the fpga loadb command which is exclusive to Xilinx. Default value is y for backward compatibility.
Signed-off-by: Ibai Erkiaga ibai.erkiaga-elorza@amd.com ---
(no changes since v1)
cmd/Kconfig | 7 +++++++ cmd/fpga.c | 8 ++++++-- 2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index c9600498787..55a8d4189a4 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1186,6 +1186,13 @@ config CMD_FPGA_LOADP Supports loading an FPGA device from a bitstream buffer containing a partial bitstream.
+config CMD_FPGA_LOADB + bool "fpga loadb - load bitstream" + default y + depends on CMD_FPGA && FPGA_XILINX + help + Supports loading an FPGA device from a bitstream buffer + config CMD_FPGA_LOADBP bool "fpga loadbp - load partial bitstream" depends on CMD_FPGA && FPGA_XILINX diff --git a/cmd/fpga.c b/cmd/fpga.c index 75d2f824998..212f421739f 100644 --- a/cmd/fpga.c +++ b/cmd/fpga.c @@ -180,6 +180,7 @@ static int do_fpga_load(struct cmd_tbl *cmdtp, int flag, int argc, return fpga_load(dev, (void *)fpga_data, data_size, BIT_FULL, 0); }
+#if defined(CONFIG_CMD_FPGA_LOADB) static int do_fpga_loadb(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) { @@ -194,7 +195,7 @@ static int do_fpga_loadb(struct cmd_tbl *cmdtp, int flag, int argc,
return fpga_loadbitstream(dev, (void *)fpga_data, data_size, BIT_FULL); } - +#endif #if defined(CONFIG_CMD_FPGA_LOADP) static int do_fpga_loadp(struct cmd_tbl *cmdtp, int flag, int argc, char *const argv[]) @@ -356,7 +357,9 @@ static struct cmd_tbl fpga_commands[] = { U_BOOT_CMD_MKENT(info, 1, 1, do_fpga_info, "", ""), U_BOOT_CMD_MKENT(dump, 3, 1, do_fpga_dump, "", ""), U_BOOT_CMD_MKENT(load, 3, 1, do_fpga_load, "", ""), +#if defined(CONFIG_CMD_FPGA_LOADB) U_BOOT_CMD_MKENT(loadb, 3, 1, do_fpga_loadb, "", ""), +#endif #if defined(CONFIG_CMD_FPGA_LOADP) U_BOOT_CMD_MKENT(loadp, 3, 1, do_fpga_loadp, "", ""), #endif @@ -412,8 +415,9 @@ U_BOOT_CMD(fpga, 6, 1, do_fpga_wrapper, "info [dev] List known device information\n" "fpga dump <dev> <address> <size> Load device to memory buffer\n" "fpga load <dev> <address> <size> Load device from memory buffer\n" +#if defined(CONFIG_CMD_FPGA_LOADP) "fpga loadb <dev> <address> <size> Load device from bitstream buffer\n" - " (Xilinx only)\n" +#endif #if defined(CONFIG_CMD_FPGA_LOADP) "fpga loadp <dev> <address> <size> Load device from memory buffer\n" " with partial bitstream\n"

On 1/20/25 13:43, Ibai Erkiaga wrote:
Adding new symbol for the fpga loadb command which is exclusive to Xilinx. Default value is y for backward compatibility.
Signed-off-by: Ibai Erkiaga ibai.erkiaga-elorza@amd.com
(no changes since v1)
cmd/Kconfig | 7 +++++++ cmd/fpga.c | 8 ++++++-- 2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index c9600498787..55a8d4189a4 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1186,6 +1186,13 @@ config CMD_FPGA_LOADP Supports loading an FPGA device from a bitstream buffer containing a partial bitstream.
+config CMD_FPGA_LOADB
- bool "fpga loadb - load bitstream"
- default y
- depends on CMD_FPGA && FPGA_XILINX
- help
Supports loading an FPGA device from a bitstream buffer
Just trying to wrap my head around how you sorted out these Kconfig entries.
you have in help fpga_commands loadb loadp loadbp
But in Kconfig you have LOADP LOADB LOADBP
Also Kconfig description for loadp and loadbp is the same
fpga loadp - load partial bitstream
fpga loadb - load bitstream fpga loadbp - load partial bitstream
That should be also clear. It is loading bitstream in BIT or BIN format.
Thanks, Michal

[AMD Official Use Only - AMD Internal Distribution Only]
Hi Michal
-----Original Message----- From: Simek, Michal michal.simek@amd.com Sent: Monday, January 20, 2025 1:04 PM To: Erkiaga Elorza, Ibai ibai.erkiaga-elorza@amd.com; u-boot@lists.denx.de Cc: ada@thorsis.com Subject: Re: [U-Boot][PATCH v4 5/5] fpga: add new symbol for fpga_loadb
On 1/20/25 13:43, Ibai Erkiaga wrote:
Adding new symbol for the fpga loadb command which is exclusive to Xilinx. Default value is y for backward compatibility.
Signed-off-by: Ibai Erkiaga ibai.erkiaga-elorza@amd.com
(no changes since v1)
cmd/Kconfig | 7 +++++++ cmd/fpga.c | 8 ++++++-- 2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig index c9600498787..55a8d4189a4 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -1186,6 +1186,13 @@ config CMD_FPGA_LOADP Supports loading an FPGA device from a bitstream buffer containing a partial bitstream.
+config CMD_FPGA_LOADB
- bool "fpga loadb - load bitstream"
- default y
- depends on CMD_FPGA && FPGA_XILINX
- help
Supports loading an FPGA device from a bitstream buffer
Just trying to wrap my head around how you sorted out these Kconfig entries.
you have in help fpga_commands loadb loadp loadbp
But in Kconfig you have LOADP LOADB LOADBP
Agree, I think the right order should be loadb, loadp, loadbp in a way that first we have the full bitstream programming commands and then the two partial ones.
Also Kconfig description for loadp and loadbp is the same
fpga loadp - load partial bitstream
fpga loadb - load bitstream fpga loadbp - load partial bitstream
That should be also clear. It is loading bitstream in BIT or BIN format.
Agree
Thanks, Michal
participants (3)
-
Erkiaga Elorza, Ibai
-
Ibai Erkiaga
-
Michal Simek