[PATCH v2] cmd: add a fetch utility
Heinrich Schuchardt
xypron.glpk at gmx.de
Wed Nov 13 15:40:01 CET 2024
Am 13. November 2024 14:32:57 MEZ schrieb Neil Armstrong <neil.armstrong at linaro.org>:
>On 13/11/2024 05:22, Caleb Connolly wrote:
>> Add a small utility for displaying some information about U-Boot and the
>> hardware it's running on in a similar fashion to the popular neofetch
>> tool for Linux [1].
>>
>> While the output is meant to be useful, it should also be pleasing to
>> look at and perhaps entertaining. The ufetch command aims to bring this
>> to U-Boot, featuring a colorful ASCII art version of the U-Boot logo.
>>
>> [1]: https://en.wikipedia.org/wiki/Neofetch
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>
>> ---
>> Ephemeral screenshot: https://0x0.st/XkQU.png
>>
>> Changes since v1:
>> * Rework storage info to be more dynamic
>> * use print_size() helper everywhere
>> * manually walk RAM banks to report memory size correctly
>> * minor formatting changes and fixes
>> * MAINTAINERS entry
>> * V1: https://lore.kernel.org/u-boot/20240808163153.2069650-1-caleb.connolly@linaro.org
>> ---
>> MAINTAINERS | 5 ++
>> cmd/Kconfig | 7 ++
>> cmd/Makefile | 1 +
>> cmd/ufetch.c | 224 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 237 insertions(+)
>> create mode 100644 cmd/ufetch.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 38c714cf46a6..d1eb164ad590 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -1724,8 +1724,13 @@ M: Heiko Schocher <hs at denx.de>
>> S: Maintained
>> T: git https://source.denx.de/u-boot/custodians/u-boot-ubi.git
>> F: drivers/mtd/ubi/
>> +UFETCH
>> +M: Caleb Connolly <caleb.connolly at linaro.org>
>> +S: Maintained
>> +F: cmd/ufetch.c
>> +
>> UFS
>> M: Neil Armstrong <neil.armstrong at linaro.org>
>> M: Bhupesh Sharma <bhupesh.linux at gmail.com>
>> M: Neha Malcom Francis <n-francis at ti.com>
>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>> index 4fba9fe67034..da736249a3cf 100644
>> --- a/cmd/Kconfig
>> +++ b/cmd/Kconfig
>> @@ -175,8 +175,15 @@ config CMD_CPU
>> number of CPUs, type (e.g. manufacturer, architecture, product or
>> internal name) and clock frequency. Other information may be
>> available depending on the CPU driver.
>> +config CMD_UFETCH
>> + bool "U-Boot fetch"
>> + depends on BLK
>> + help
>> + Fetch utility for U-Boot (akin to neofetch). Prints information
>> + about U-Boot and the board it is running on in a pleasing format.
The information is already available in other commands with greater detail.
What is your use case?
>> +
>> config CMD_FWU_METADATA
>> bool "fwu metadata read"
>> depends on FWU_MULTI_BANK_UPDATE
>> help
>> diff --git a/cmd/Makefile b/cmd/Makefile
>> index d1f369deec0a..1e6d3128c8ca 100644
>> --- a/cmd/Makefile
>> +++ b/cmd/Makefile
>> @@ -52,8 +52,9 @@ obj-$(CONFIG_CMD_CONSOLE) += console.o
>> obj-$(CONFIG_CMD_CPU) += cpu.o
>> obj-$(CONFIG_CMD_DATE) += date.o
>> obj-$(CONFIG_CMD_DEMO) += demo.o
>> obj-$(CONFIG_CMD_DM) += dm.o
>> +obj-$(CONFIG_CMD_UFETCH) += ufetch.o
>> obj-$(CONFIG_CMD_SOUND) += sound.o
>> ifdef CONFIG_POST
>> obj-$(CONFIG_CMD_DIAG) += diag.o
>> endif
>> diff --git a/cmd/ufetch.c b/cmd/ufetch.c
>> new file mode 100644
>> index 000000000000..1bd0565b9f08
>> --- /dev/null
>> +++ b/cmd/ufetch.c
>> @@ -0,0 +1,224 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +/* Small "fetch" utility for U-Boot */
>> +
>> +#ifdef CONFIG_ARM
>> +#include <asm/system.h>
>> +#endif
>> +#include <dm/device.h>
>> +#include <dm/uclass-internal.h>
>> +#include <display_options.h>
>> +#include <mmc.h>
>> +#include <time.h>
>> +#include <asm/global_data.h>
>> +#include <cli.h>
>> +#include <command.h>
>> +#include <dm/ofnode.h>
>> +#include <env.h>
>> +#include <rand.h>
>> +#include <vsprintf.h>
>> +#include <linux/delay.h>
>> +#include <linux/kernel.h>
>> +#include <version.h>
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +#define LINE_WIDTH 40
>> +#define BLUE "\033[38;5;4m"
>> +#define YELLOW "\033[38;5;11m"
>> +#define BOLD "\033[1m"
>> +#define RESET "\033[0m"
>> +static const char * const logo_lines[] = {
>> + BLUE BOLD " ......::...... ",
>> + BLUE BOLD " ...::::::::::::::::::... ",
>> + BLUE BOLD " ..::::::::::::::::::::::::::.. ",
>> + BLUE BOLD " .::::.:::::::::::::::...::::.::::. ",
>> + BLUE BOLD " .::::::::::::::::::::..::::::::::::::. ",
>> + BLUE BOLD " .::.:::::::::::::::::::" YELLOW "=*%#*" BLUE "::::::::::.::. ",
>> + BLUE BOLD " .:::::::::::::::::....." YELLOW "*%%*-" BLUE ":....::::::::::. ",
>> + BLUE BOLD " .:.:::...:::::::::.:-" YELLOW "===##*---==-" BLUE "::::::::::.:. ",
>> + BLUE BOLD " .::::..::::........" YELLOW "-***#****###****-" BLUE "...::::::.:. ",
>> + BLUE BOLD " ::.:.-" YELLOW "+***+=" BLUE "::-" YELLOW "=+**#%%%%%%%%%%%%###*= " BLUE "-::...::::. ",
>> + BLUE BOLD ".:.::-" YELLOW "*****###%%%%%%%%%%%%%%%%%%%%%%%%%%#*=" BLUE ":..:::: ",
>> + BLUE BOLD ".::" YELLOW "##" BLUE ":" YELLOW "***#%%%%%%#####%%%%%%%####%%%%%####%%%*" BLUE "-.::. ",
>> + BLUE BOLD ":.:" YELLOW "#%" BLUE "::" YELLOW "*%%%%%%%#*****##%%%#*****##%%##*****#%%+" BLUE ".::.",
>> + BLUE BOLD ".::" YELLOW "**==#%%%%%%%##****#%%%%##****#%%%%#****###%%" BLUE ":.. ",
>> + BLUE BOLD "..:" YELLOW "#%" BLUE "::" YELLOW "*%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%#%%%%%+ " BLUE ".:.",
>> + BLUE BOLD " ::" YELLOW "##" BLUE ":" YELLOW "+**#%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%* " BLUE "-.:: ",
>> + BLUE BOLD " ..::-" YELLOW "#****#%#%%%%%%%%%%%%%%%%%%%%%%%%%%#*=" BLUE "-..::. ",
>> + BLUE BOLD " ...:=" YELLOW "*****=" BLUE "::-" YELLOW "=+**###%%%%%%%%###**+= " BLUE "--:...::: ",
>> + BLUE BOLD " .::.::--:........::::::--::::::......::::::. ",
>> + BLUE BOLD " .::.....::::::::::...........:::::::::.::. ",
>> + BLUE BOLD " .::::::::::::::::::::::::::::::::::::. ",
>> + BLUE BOLD " .::::.::::::::::::::::::::::.::::. ",
>> + BLUE BOLD " ..::::::::::::::::::::::::::.. ",
>> + BLUE BOLD " ...::::::::::::::::::... ",
>> + BLUE BOLD " ......::...... ",
>> +};
The lines above are not informative but increase code size. They should be removable from code.
>> +
>> +enum output_lines {
>> + FIRST,
>> + SECOND,
>> + KERNEL,
>> + SYSINFO,
>> + HOST,
>> + UPTIME,
>> + IP,
>> + CMDS,
>> + CONSOLES,
>> + DEVICES,
>> + FEATURES,
>> + RELOCATION,
>> + CORES,
>> + MEMORY,
>> + STORAGE,
>> +
>> + /* Up to 10 storage devices... Should be enough for anyone right? */
>> + _LAST_LINE = (STORAGE + 10),
>> +#define LAST_LINE (_LAST_LINE - 1UL)
>> +};
Please, avoid unnecessary assumptions.
What stops me from having a few dozen drives on my NAS or in my VM?
>> +
>> +static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
>> + char *const argv[])
>> +{
>> + int num_lines = max(LAST_LINE + 1, ARRAY_SIZE(logo_lines));
>> + const char *model, *compatible;
>> + char *ipaddr;
>> + int n_cmds, n_cpus = 0, ret, compatlen;
>> + size_t size;
>> + ofnode np;
>> + struct udevice *dev;
>> + struct blk_desc *desc;
>> + bool skip_ascii = false;
>> +
>> + if (argc > 1 && strcmp(argv[1], "-n") == 0) {
>> + skip_ascii = true;
>> + num_lines = LAST_LINE;
>> + }
>> +
>> + for (int line = 0; line < num_lines; line++) {
This loop is makes adjustments to the code difficult especially for output where there are multiple devices like network and storage.
If the board supports ANSI you can easily position the cursor whereever you want.
>> + if (!skip_ascii) {
>> + if (line < ARRAY_SIZE(logo_lines))
>> + printf("%s ", logo_lines[line]);
>> + else
>> + printf("%*c ", LINE_WIDTH, ' ');
>> + }
>> + switch (line) {
>> + case FIRST:
>> + compatible = ofnode_read_string(ofnode_root(), "compatible");
>> + if (!compatible)
>> + compatible = "unknown";
>> + printf(RESET "%s\n", compatible);
>> + compatlen = strlen(compatible);
>> + break;
>> + case SECOND:
>> + for (int j = 0; j < compatlen; j++)
>> + putc('-');
>> + putc('\n');
>> + break;
>> + case KERNEL:
>> + printf("Kernel:" RESET " %s\n", U_BOOT_VERSION);
>> + break;
>> + case SYSINFO:
>> + printf("Config:" RESET " %s_defconfig\n", CONFIG_SYS_CONFIG_NAME);
>> + break;
>> + case HOST:
>> + model = ofnode_read_string(ofnode_root(), "model");
>> + if (model)
>> + printf("Host:" RESET " %s\n", model);
>> + break;
>> + case UPTIME:
>> + printf("Uptime:" RESET " %ld seconds\n", get_timer(0) / 1000);
>> + break;
>> + case IP:
>> + ipaddr = env_get("ipvaddr");
>> + if (!ipaddr)
>> + ipaddr = "none";
>> + printf("IP Address:" RESET " %s", ipaddr);
>> + ipaddr = env_get("ipv6addr");
>> + if (ipaddr)
Do your boards only have a single network interface? Mine have multiple. Why wouldn't you list them all?
>> + printf(", %s\n", ipaddr);
>> + else
>> + putc('\n');
Why waste an output line on something that does not exist. Get rid of the loop!
>> + break;
>> + case CMDS:
>> + n_cmds = ll_entry_count(struct cmd_tbl, cmd);
>> + printf("Commands:" RESET " %d (help)\n", n_cmds);
Who cares about the number of commands if the one he needs is not provided.
This information is not helpful.
>> + break;
>> + case CONSOLES:
>> + printf("Consoles:" RESET " %s (%d baud)\n", env_get("stdout"),
>> + gd->baudrate);
Please, do not assume that stdout relates to a serial console.
>> + break;
>> + case DEVICES:
>> + printf("Devices:" RESET " ");
>> + /* TODO: walk the DM tree */
>> + printf("Uncountable!\n");
Please, do not write non-information to the screen.
>> + break;
>> + case FEATURES:
>> + printf("Features:" RESET " ");
>> + if (IS_ENABLED(CONFIG_NET))
>> + printf("Net");
>> + if (IS_ENABLED(CONFIG_EFI_LOADER))
>> + printf(", EFI");
>> + if (IS_ENABLED(CONFIG_CMD_CAT))
>> + printf(", cat :3");
:3 relates to what in U-Boot?
No clue, why you consider the cat command the relevant one, needing special mention.
>> +#ifdef CONFIG_ARM
>> + switch (current_el()) {
>> + case 2:
>> + printf(", VMs");
Neither full control nor VMs are provided by U-Boot.
Why don't you simply write "EL%d"?
>> + break;
>> + case 3:
>> + printf(", full control!");
>> + break;
>> + }
>> +#endif
>> + printf("\n");
>> + break;
>> + case RELOCATION:
>> + if (gd->flags & GD_FLG_SKIP_RELOC)
>> + printf("Relocated:" RESET " no\n");
>> + else
>> + printf("Relocated:" RESET " to %#011lx\n", gd->relocaddr);
>> + break;
>> + case CORES:
>> + ofnode_for_each_subnode(np, ofnode_path("/cpus")) {
>> + if (ofnode_name_eq(np, "cpu"))
>> + n_cpus++;
>> + }
>> + printf("CPU:" RESET " %d (1 in use)\n", n_cpus);
Who knows how many cores are used in the secure world?
This (1 in use) is not oroviding information.
>> + break;
>> + case MEMORY:
>> + for (int j = 0; j < CONFIG_NR_DRAM_BANKS && gd->bd->bi_dram[j].size; j++)
>> + size += gd->bd->bi_dram[j].size;
>> + printf("Memory:" RESET " ");
>> + print_size(size, "\n");
>> + break;
>> + case STORAGE:
>> + default:
>> + ret = uclass_find_device_by_seq(UCLASS_BLK, line - STORAGE, &dev);
>> + if (!ret && dev) {
>> + desc = dev_get_uclass_plat(dev);
>> + size = desc->lba * desc->blksz;
>> + printf("%s %d: " RESET, desc->uclass_id == UCLASS_SCSI ? "SCSI" :
>> + desc->uclass_id == UCLASS_MMC
>> + ? "MMC" : "Storage",
SCSI is so old fashioned. How about NVMe?
There is a library function to get the display name for a block device.
>> + desc->lun);
>> + if (size)
>> + print_size(size, "");
>> + else
>> + printf("No media");
>> + }
>> + printf("\n");
>> + }
>> + }
>> +
>> + printf(RESET "\n\n");
>> +
>> + return 0;
>> +}
>> +
>> +U_BOOT_CMD(ufetch, 2, 1, do_ufetch,
>> + "U-Boot fetch utility",
>> + "Print information about your device.\n"
>> + " -n Don't print the ASCII logo"
And how I do I get rid of the other ANSI output?
Best regards
Heinrich
>> +);
>
>Tested-by: Neil Armstrong <neil.armstrong at linaro.org> # on SM8560-QRD
>
>Ephemeral screenshot: https://0x0.st/Xk2N.png
>
>Thanks,
>Neil
More information about the U-Boot
mailing list