[PATCH v2] cmd: add a fetch utility
Neil Armstrong
neil.armstrong at linaro.org
Wed Nov 13 16:01:07 CET 2024
On 13/11/2024 15:40, Heinrich Schuchardt wrote:
> 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?
SCSI is used with UFS, so it's far from being old-fashioned.
Neil
>
> 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