diff mbox series

cmd: add a fetch utility

Message ID 20240808163153.2069650-1-caleb.connolly@linaro.org
State Superseded
Headers show
Series cmd: add a fetch utility | expand

Commit Message

Caleb Connolly Aug. 8, 2024, 4:24 p.m. UTC
While U-Boot does a pretty good job at printing information at startup
about what hardware it's running on, it can be hard to take pretty
pictures of this to show off on the internet (by far the most
important aspect of porting a device in 2024!).

Add a small utility "ufetch" for displaying some information about U-Boot and
the hardware it's running on in a similar fashion to the popular neofetch tool
for *nix's [1].

While the output is meant to be useful, it should also be pleasing to look at
and look good in screenshots. The ufetch command brings this to U-Boot,
featuring a colorful ASCII art version of the U-Boot logo and a fancy layout.

Finally, tireless device porters can properly show off their hard work and get
the internet points they so deserve!

Not everything is fully supported yet, but this seemed good enough for initial
inclusion. Only one MMC device is detected, and other than SCSI other storage
devices aren't supported.

[1]: https://en.wikipedia.org/wiki/Neofetch

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
Ephemeral demo image: https://0x0.st/XVUa.png
---
 cmd/Kconfig  |   7 ++
 cmd/Makefile |   1 +
 cmd/ufetch.c | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 209 insertions(+)
 create mode 100644 cmd/ufetch.c

Comments

Simon Glass Aug. 9, 2024, 1:55 a.m. UTC | #1
Hi Caleb,

On Thu, 8 Aug 2024 at 10:32, Caleb Connolly <caleb.connolly@linaro.org>
wrote:
>
> While U-Boot does a pretty good job at printing information at startup
> about what hardware it's running on, it can be hard to take pretty
> pictures of this to show off on the internet (by far the most
> important aspect of porting a device in 2024!).
>
> Add a small utility "ufetch" for displaying some information about U-Boot
and
> the hardware it's running on in a similar fashion to the popular neofetch
tool
> for *nix's [1].
>
> While the output is meant to be useful, it should also be pleasing to
look at
> and look good in screenshots. The ufetch command brings this to U-Boot,
> featuring a colorful ASCII art version of the U-Boot logo and a fancy
layout.
>
> Finally, tireless device porters can properly show off their hard work
and get
> the internet points they so deserve!
>
> Not everything is fully supported yet, but this seemed good enough for
initial
> inclusion. Only one MMC device is detected, and other than SCSI other
storage
> devices aren't supported.
>
> [1]: https://en.wikipedia.org/wiki/Neofetch
>
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> Ephemeral demo image: https://0x0.st/XVUa.png
> ---
>  cmd/Kconfig  |   7 ++
>  cmd/Makefile |   1 +
>  cmd/ufetch.c | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 209 insertions(+)
>  create mode 100644 cmd/ufetch.c

Very cute!

>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 978f44eda426..70acb5727b04 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"

How about default y if SANDBOX so that this gets built for that. You should
also add doc/ and test/ for the command (although the test can be very
basic).

> +       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.
> +
>  config CMD_FWU_METADATA
>         bool "fwu metadata read"
>         depends on FWU_MULTI_BANK_UPDATE
>         help
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 87133cc27a8a..ffb04c8f2e0a 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -53,8 +53,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..f7374eb2e364
> --- /dev/null
> +++ b/cmd/ufetch.c
> @@ -0,0 +1,201 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/* Small "fetch" utility for U-Boot */

Isn't it a command, rather than a utility? I think of a utility as a
program I run.

> +
> +#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>

Please check [1]

> +
> +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 *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
"::-\033[38;5;7m=+**###%%%%%%%%###**+=  " BLUE "--:...:::  ",
> +       BLUE BOLD "   .::.::--:........::::::--::::::......::::::.    ",
> +       BLUE BOLD "    .::.....::::::::::...........:::::::::.::.     ",
> +       BLUE BOLD "      .::::::::::::::::::::::::::::::::::::.       ",
> +       BLUE BOLD "        .::::.::::::::::::::::::::::.::::.         ",
> +       BLUE BOLD "          ..::::::::::::::::::::::::::..           ",
> +       BLUE BOLD "             ...::::::::::::::::::...              ",
> +       BLUE BOLD "                  ......::......                   ",
> +};
> +
> +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)
> +};
> +
> +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;
> +       ofnode np;
> +       struct mmc *mmc;
> +       struct blk_desc *scsi_desc;
> +       bool skip_ascii = false;
> +
> +       if (argc > 1 && strcmp(argv[1], "-n") == 0) {
> +               skip_ascii = true;
> +               num_lines = LAST_LINE;
> +       }
> +
> +       for (int i = 0; i < num_lines; i++) {
> +               if (!skip_ascii) {
> +                       if (i < ARRAY_SIZE(logo_lines))
> +                               printf("%s ", logo_lines[i]);
> +                       else
> +                               printf("%*c ", LINE_WIDTH, ' ');
> +               }
> +               switch (i) {
> +               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)
> +                               printf(", %s\n", ipaddr);
> +                       else
> +                               putc('\n');
> +                       break;
> +               case CMDS:
> +                       n_cmds = ll_entry_count(struct cmd_tbl, cmd);
> +                       printf("Commands:" RESET " %d (help)\n", n_cmds);
> +                       break;
> +               case CONSOLES:
> +                       printf("Consoles:" RESET " %s (%d baud)\n",
env_get("stdout"),
> +                              gd->baudrate);
> +                       break;
> +               case DEVICES:
> +                       printf("Devices:" RESET " ");
> +                       /* TODO: walk the DM tree */
> +                       printf("Uncountable!\n");

See dm_announce() for how to do that.

> +                       break;
> +               case FEATURES:
> +                       printf("Features:" RESET " ");
> +                       if (IS_ENABLED(CONFIG_NET))
> +                               printf("Net");
> +                       if (IS_ENABLED(CONFIG_EFI_LOADER))
> +                               printf(", EFI");

How about EFI_LOADER ? After all, U-Boot can run as an EFI app so 'EFI'
might be better reserved for that.

> +                       printf("\n");
> +                       break;
> +               case RELOCATION:
> +                       if (gd->flags & GD_FLG_SKIP_RELOC)
> +                               printf("Relocated:" RESET " no\n");
> +                       else
> +                               printf("Relocated:" RESET " to %#09lx\n",
gd->relocaddr);

Not for this patch but I'd really like to figure out a way to
enable/disable ANSI codes globally in U-Boot. For example, we should
disable them in tests, or when redirecting sandbox to a file. It affects
EFI tests too...so if you have any ideas... :-)

> +                       break;
> +               case CORES:
> +                       ofnode_for_each_subnode(np, ofnode_path("/cpus"))
{
> +                               if (ofnode_name_eq(np, "cpu"))
> +                                       n_cpus++;
> +                       }

uclass_id_count(UCLASS_CPU)

> +                       printf("CPU:" RESET " %d (1 in use)\n", n_cpus);
> +                       break;
> +               case MEMORY:
> +                       printf("Memory:" RESET " %lu MB\n",
(ulong)gd->ram_size >> 20);
> +                       break;
> +               case STORAGE:
> +               default:
> +                       if (i == STORAGE && get_mmc_num() > 0) {
> +                               mmc = find_mmc_device(0);
> +                               if (mmc)
> +                                       printf("Storage:" RESET "  mmc 0:
%llu MB", mmc->capacity >> 20);
> +                       }

How about iterating through UCLASS_BLK ?

> +                       if (i >= STORAGE + 1) {
> +                               ret = blk_get_desc(UCLASS_SCSI, i -
(STORAGE + 1), &scsi_desc);
> +                               if (!ret && scsi_desc->type !=
DEV_TYPE_UNKNOWN)
> +                                       /* The calculation here is really
lossy but close enough */
> +                                       printf("Storage:" RESET " scsi
%d: %lu MB", i - (STORAGE + 1),
> +                                              ((scsi_desc->lba >> 9) *
scsi_desc->blksz) >> 11);

You can use print_size() - and same above I suppose

> +                       }
> +                       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"
> +);
> --
> 2.46.0
>

Regards,
Simon


[1] https://docs.u-boot.org/en/latest/develop/codingstyle.html#include-files
Michal Simek Aug. 9, 2024, 6:03 a.m. UTC | #2
On 8/8/24 18:24, Caleb Connolly wrote:
> While U-Boot does a pretty good job at printing information at startup
> about what hardware it's running on, it can be hard to take pretty
> pictures of this to show off on the internet (by far the most
> important aspect of porting a device in 2024!).
> 
> Add a small utility "ufetch" for displaying some information about U-Boot and
> the hardware it's running on in a similar fashion to the popular neofetch tool
> for *nix's [1].
> 
> While the output is meant to be useful, it should also be pleasing to look at
> and look good in screenshots. The ufetch command brings this to U-Boot,
> featuring a colorful ASCII art version of the U-Boot logo and a fancy layout.
> 
> Finally, tireless device porters can properly show off their hard work and get
> the internet points they so deserve!
> 
> Not everything is fully supported yet, but this seemed good enough for initial
> inclusion. Only one MMC device is detected, and other than SCSI other storage
> devices aren't supported.
> 
> [1]: https://en.wikipedia.org/wiki/Neofetch
> 
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> Ephemeral demo image: https://0x0.st/XVUa.png
> ---
>   cmd/Kconfig  |   7 ++
>   cmd/Makefile |   1 +
>   cmd/ufetch.c | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 209 insertions(+)
>   create mode 100644 cmd/ufetch.c
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 978f44eda426..70acb5727b04 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.
> +
>   config CMD_FWU_METADATA
>   	bool "fwu metadata read"
>   	depends on FWU_MULTI_BANK_UPDATE
>   	help
> diff --git a/cmd/Makefile b/cmd/Makefile
> index 87133cc27a8a..ffb04c8f2e0a 100644
> --- a/cmd/Makefile
> +++ b/cmd/Makefile
> @@ -53,8 +53,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..f7374eb2e364
> --- /dev/null
> +++ b/cmd/ufetch.c
> @@ -0,0 +1,201 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/* Small "fetch" utility for U-Boot */
> +
> +#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 *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 "::-\033[38;5;7m=+**###%%%%%%%%###**+=  " BLUE "--:...:::  ",
> +	BLUE BOLD "   .::.::--:........::::::--::::::......::::::.    ",
> +	BLUE BOLD "    .::.....::::::::::...........:::::::::.::.     ",
> +	BLUE BOLD "      .::::::::::::::::::::::::::::::::::::.       ",
> +	BLUE BOLD "        .::::.::::::::::::::::::::::.::::.         ",
> +	BLUE BOLD "          ..::::::::::::::::::::::::::..           ",
> +	BLUE BOLD "             ...::::::::::::::::::...              ",
> +	BLUE BOLD "                  ......::......                   ",
> +};
> +
> +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)
> +};
> +
> +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;
> +	ofnode np;
> +	struct mmc *mmc;
> +	struct blk_desc *scsi_desc;
> +	bool skip_ascii = false;
> +
> +	if (argc > 1 && strcmp(argv[1], "-n") == 0) {
> +		skip_ascii = true;
> +		num_lines = LAST_LINE;
> +	}
> +
> +	for (int i = 0; i < num_lines; i++) {
> +		if (!skip_ascii) {
> +			if (i < ARRAY_SIZE(logo_lines))
> +				printf("%s ", logo_lines[i]);
> +			else
> +				printf("%*c ", LINE_WIDTH, ' ');
> +		}
> +		switch (i) {
> +		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");


ipaddr here.

> +			if (!ipaddr)
> +				ipaddr = "none";
> +			printf("IP Address:" RESET " %s", ipaddr);
> +			ipaddr = env_get("ipv6addr");
> +			if (ipaddr)
> +				printf(", %s\n", ipaddr);
> +			else
> +				putc('\n');
> +			break;
> +		case CMDS:
> +			n_cmds = ll_entry_count(struct cmd_tbl, cmd);
> +			printf("Commands:" RESET " %d (help)\n", n_cmds);
> +			break;
> +		case CONSOLES:
> +			printf("Consoles:" RESET " %s (%d baud)\n", env_get("stdout"),
> +			       gd->baudrate);
> +			break;
> +		case DEVICES:
> +			printf("Devices:" RESET " ");
> +			/* TODO: walk the DM tree */
> +			printf("Uncountable!\n");
> +			break;
> +		case FEATURES:
> +			printf("Features:" RESET " ");
> +			if (IS_ENABLED(CONFIG_NET))
> +				printf("Net");
> +			if (IS_ENABLED(CONFIG_EFI_LOADER))
> +				printf(", EFI");
> +			printf("\n");
> +			break;
> +		case RELOCATION:
> +			if (gd->flags & GD_FLG_SKIP_RELOC)
> +				printf("Relocated:" RESET " no\n");
> +			else
> +				printf("Relocated:" RESET " to %#09lx\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);
> +			break;
> +		case MEMORY:
> +			printf("Memory:" RESET " %lu MB\n", (ulong)gd->ram_size >> 20);

This is kind of shortcut (and nothing wrong on it)

In my case I see 2GB here but board has 4GB which is properly shown via
show_dram_config() as

DRAM:  2 GiB (effective 4 GiB)

Maybe you can simply call this function to get the same information.
Also I see that there is board_add_ram_info() called which can provide more 
information.

Thanks,
Michal
Heinrich Schuchardt Aug. 9, 2024, 6:18 a.m. UTC | #3
Am 8. August 2024 18:24:24 MESZ schrieb Caleb Connolly <caleb.connolly@linaro.org>:
>While U-Boot does a pretty good job at printing information at startup
>about what hardware it's running on, it can be hard to take pretty
>pictures of this to show off on the internet (by far the most
>important aspect of porting a device in 2024!).
>
>Add a small utility "ufetch" for displaying some information about U-Boot and
>the hardware it's running on in a similar fashion to the popular neofetch tool
>for *nix's [1].
>
>While the output is meant to be useful, it should also be pleasing to look at
>and look good in screenshots. The ufetch command brings this to U-Boot,
>featuring a colorful ASCII art version of the U-Boot logo and a fancy layout.
>
>Finally, tireless device porters can properly show off their hard work and get
>the internet points they so deserve!
>
>Not everything is fully supported yet, but this seemed good enough for initial
>inclusion. Only one MMC device is detected, and other than SCSI other storage
>devices aren't supported.


This command is not very generic. E.g. it supports SCSI drives but not SATA. Many boards have eMMC and SD-card. This lks more ike an RFC tan something ready to merge.

Why don't you use a script or an EFI program to print whatever information you need?

>
>[1]: https://en.wikipedia.org/wiki/Neofetch
>
>Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
>---
>Ephemeral demo image: https://0x0.st/XVUa.png
>---
> cmd/Kconfig  |   7 ++
> cmd/Makefile |   1 +
> cmd/ufetch.c | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 209 insertions(+)
> create mode 100644 cmd/ufetch.c
>
>diff --git a/cmd/Kconfig b/cmd/Kconfig
>index 978f44eda426..70acb5727b04 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.
>+
> config CMD_FWU_METADATA
> 	bool "fwu metadata read"
> 	depends on FWU_MULTI_BANK_UPDATE
> 	help
>diff --git a/cmd/Makefile b/cmd/Makefile
>index 87133cc27a8a..ffb04c8f2e0a 100644
>--- a/cmd/Makefile
>+++ b/cmd/Makefile
>@@ -53,8 +53,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..f7374eb2e364
>--- /dev/null
>+++ b/cmd/ufetch.c
>@@ -0,0 +1,201 @@
>+// SPDX-License-Identifier: GPL-2.0
>+
>+/* Small "fetch" utility for U-Boot */
>+
>+#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 *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 "::-\033[38;5;7m=+**###%%%%%%%%###**+=  " BLUE "--:...:::  ",
>+	BLUE BOLD "   .::.::--:........::::::--::::::......::::::.    ",
>+	BLUE BOLD "    .::.....::::::::::...........:::::::::.::.     ",
>+	BLUE BOLD "      .::::::::::::::::::::::::::::::::::::.       ",
>+	BLUE BOLD "        .::::.::::::::::::::::::::::.::::.         ",
>+	BLUE BOLD "          ..::::::::::::::::::::::::::..           ",
>+	BLUE BOLD "             ...::::::::::::::::::...              ",
>+	BLUE BOLD "                  ......::......                   ",
>+};
>+
>+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)
>+};
>+
>+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;
>+	ofnode np;
>+	struct mmc *mmc;
>+	struct blk_desc *scsi_desc;
>+	bool skip_ascii = false;
>+
>+	if (argc > 1 && strcmp(argv[1], "-n") == 0) {
>+		skip_ascii = true;
>+		num_lines = LAST_LINE;
>+	}
>+
>+	for (int i = 0; i < num_lines; i++) {
>+		if (!skip_ascii) {
>+			if (i < ARRAY_SIZE(logo_lines))
>+				printf("%s ", logo_lines[i]);
>+			else
>+				printf("%*c ", LINE_WIDTH, ' ');
>+		}
>+		switch (i) {
>+		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:

Please, do not use misleading variable names.


>+			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)
>+				printf(", %s\n", ipaddr);
>+			else
>+				putc('\n');
>+			break;
>+		case CMDS:
>+			n_cmds = ll_entry_count(struct cmd_tbl, cmd);

Who cares if you have 50 commands or 51 if you don't know which?


>+			printf("Commands:" RESET " %d (help)\n", n_cmds);
>+			break;
>+		case CONSOLES:
>+			printf("Consoles:" RESET " %s (%d baud)\n", env_get("stdout"),
>+			       gd->baudrate);
>+			break;
>+		case DEVICES:
>+			printf("Devices:" RESET " ");
>+			/* TODO: walk the DM tree */
>+			printf("Uncountable!\n");

Nothing we should merge.

>+			break;
>+		case FEATURES:
>+			printf("Features:" RESET " ");
>+			if (IS_ENABLED(CONFIG_NET))
>+				printf("Net");

Wouuldn't you want to know which devices where detected?

>+			if (IS_ENABLED(CONFIG_EFI_LOADER))
>+				printf(", EFI");
>+			printf("\n");
>+			break;
>+		case RELOCATION:
>+			if (gd->flags & GD_FLG_SKIP_RELOC)
>+				printf("Relocated:" RESET " no\n");
>+			else
>+				printf("Relocated:" RESET " to %#09lx\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);
>+			break;
>+		case MEMORY:
>+			printf("Memory:" RESET " %lu MB\n", (ulong)gd->ram_size >> 20);
>+			break;
>+		case STORAGE:
>+		default:
>+			if (i == STORAGE && get_mmc_num() > 0) {
>+				mmc = find_mmc_device(0);
>+				if (mmc)
>+					printf("Storage:" RESET "  mmc 0: %llu MB", mmc->capacity >> 20);

And if my eMMC is device 1 and SD card is 0?

Why wouldn't you iterate over all UCLASS_BLK devices?

Best regards

Heinrich

>+			}
>+			if (i >= STORAGE + 1) {
>+				ret = blk_get_desc(UCLASS_SCSI, i - (STORAGE + 1), &scsi_desc);
>+				if (!ret && scsi_desc->type != DEV_TYPE_UNKNOWN)
>+					/* The calculation here is really lossy but close enough */
>+					printf("Storage:" RESET " scsi %d: %lu MB", i - (STORAGE + 1),
>+					       ((scsi_desc->lba >> 9) * scsi_desc->blksz) >> 11);
>+			}
>+			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"
>+);
Caleb Connolly Aug. 9, 2024, 5 p.m. UTC | #4
Hi Simon,

On 09/08/2024 03:55, Simon Glass wrote:
> Hi Caleb,
> 
> On Thu, 8 Aug 2024 at 10:32, Caleb Connolly <caleb.connolly@linaro.org 
> <mailto:caleb.connolly@linaro.org>> wrote:
>  >
>  > While U-Boot does a pretty good job at printing information at startup
>  > about what hardware it's running on, it can be hard to take pretty
>  > pictures of this to show off on the internet (by far the most
>  > important aspect of porting a device in 2024!).
>  >
>  > Add a small utility "ufetch" for displaying some information about 
> U-Boot and
>  > the hardware it's running on in a similar fashion to the popular 
> neofetch tool
>  > for *nix's [1].
>  >
>  > While the output is meant to be useful, it should also be pleasing to 
> look at
>  > and look good in screenshots. The ufetch command brings this to U-Boot,
>  > featuring a colorful ASCII art version of the U-Boot logo and a fancy 
> layout.
>  >
>  > Finally, tireless device porters can properly show off their hard 
> work and get
>  > the internet points they so deserve!
>  >
>  > Not everything is fully supported yet, but this seemed good enough 
> for initial
>  > inclusion. Only one MMC device is detected, and other than SCSI other 
> storage
>  > devices aren't supported.
>  >
>  > [1]: https://en.wikipedia.org/wiki/Neofetch 
> <https://en.wikipedia.org/wiki/Neofetch>
>  >
>  > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org 
> <mailto:caleb.connolly@linaro.org>>
>  > ---
>  > Ephemeral demo image: https://0x0.st/XVUa.png <https://0x0.st/XVUa.png>
>  > ---
>  >  cmd/Kconfig  |   7 ++
>  >  cmd/Makefile |   1 +
>  >  cmd/ufetch.c | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  >  3 files changed, 209 insertions(+)
>  >  create mode 100644 cmd/ufetch.c
> 
> Very cute!
> 
>  >
>  > diff --git a/cmd/Kconfig b/cmd/Kconfig
>  > index 978f44eda426..70acb5727b04 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"
> 
> How about default y if SANDBOX so that this gets built for that. You 
> should also add doc/ and test/ for the command (although the test can be 
> very basic).

I was really aiming for cmd/2048.c levels of expectation and usability 
here, not to make this a rugged part of U-Boot's feature set (if someone 
else would like to go the extra mile they're certainly welcome to).

Making it default y for sandbox so we can at least make sure it compiles 
sounds reasonable though.
> 
>  > +       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.
>  > +
>  >  config CMD_FWU_METADATA
>  >         bool "fwu metadata read"
>  >         depends on FWU_MULTI_BANK_UPDATE
>  >         help
>  > diff --git a/cmd/Makefile b/cmd/Makefile
>  > index 87133cc27a8a..ffb04c8f2e0a 100644
>  > --- a/cmd/Makefile
>  > +++ b/cmd/Makefile
>  > @@ -53,8 +53,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..f7374eb2e364
>  > --- /dev/null
>  > +++ b/cmd/ufetch.c
>  > @@ -0,0 +1,201 @@
>  > +// SPDX-License-Identifier: GPL-2.0
>  > +
>  > +/* Small "fetch" utility for U-Boot */
> 
> Isn't it a command, rather than a utility? I think of a utility as a 
> program I run.

I feel like you could ask 5 different people about this and get 5 
different answers. As a native british english speaker I would view 
"utilities" as a subset of "commands" in this context. This isn't a high 
effort contribution for me so I'm really fine with whatever.
> 
>  > +
>  > +#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>
> 
> Please check [1]

Sure, I'll order these alphabetically.
> 
>  > +
>  > +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 *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 
> "::-\033[38;5;7m=+**###%%%%%%%%###**+=  " BLUE "--:...:::  ",
>  > +       BLUE BOLD "   .::.::--:........::::::--::::::......::::::.    ",
>  > +       BLUE BOLD "    .::.....::::::::::...........:::::::::.::.     ",
>  > +       BLUE BOLD "      .::::::::::::::::::::::::::::::::::::.       ",
>  > +       BLUE BOLD "        .::::.::::::::::::::::::::::.::::.         ",
>  > +       BLUE BOLD "          ..::::::::::::::::::::::::::..           ",
>  > +       BLUE BOLD "             ...::::::::::::::::::...              ",
>  > +       BLUE BOLD "                  ......::......                   ",
>  > +};
>  > +
>  > +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)
>  > +};
>  > +
>  > +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;
>  > +       ofnode np;
>  > +       struct mmc *mmc;
>  > +       struct blk_desc *scsi_desc;
>  > +       bool skip_ascii = false;
>  > +
>  > +       if (argc > 1 && strcmp(argv[1], "-n") == 0) {
>  > +               skip_ascii = true;
>  > +               num_lines = LAST_LINE;
>  > +       }
>  > +
>  > +       for (int i = 0; i < num_lines; i++) {
>  > +               if (!skip_ascii) {
>  > +                       if (i < ARRAY_SIZE(logo_lines))
>  > +                               printf("%s ", logo_lines[i]);
>  > +                       else
>  > +                               printf("%*c ", LINE_WIDTH, ' ');
>  > +               }
>  > +               switch (i) {
>  > +               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)
>  > +                               printf(", %s\n", ipaddr);
>  > +                       else
>  > +                               putc('\n');
>  > +                       break;
>  > +               case CMDS:
>  > +                       n_cmds = ll_entry_count(struct cmd_tbl, cmd);
>  > +                       printf("Commands:" RESET " %d (help)\n", n_cmds);
>  > +                       break;
>  > +               case CONSOLES:
>  > +                       printf("Consoles:" RESET " %s (%d baud)\n", 
> env_get("stdout"),
>  > +                              gd->baudrate);
>  > +                       break;
>  > +               case DEVICES:
>  > +                       printf("Devices:" RESET " ");
>  > +                       /* TODO: walk the DM tree */
>  > +                       printf("Uncountable!\n");
> 
> See dm_announce() for how to do that.
> 
>  > +                       break;
>  > +               case FEATURES:
>  > +                       printf("Features:" RESET " ");
>  > +                       if (IS_ENABLED(CONFIG_NET))
>  > +                               printf("Net");
>  > +                       if (IS_ENABLED(CONFIG_EFI_LOADER))
>  > +                               printf(", EFI");
> 
> How about EFI_LOADER ? After all, U-Boot can run as an EFI app so 'EFI' 
> might be better reserved for that.
> 
>  > +                       printf("\n");
>  > +                       break;
>  > +               case RELOCATION:
>  > +                       if (gd->flags & GD_FLG_SKIP_RELOC)
>  > +                               printf("Relocated:" RESET " no\n");
>  > +                       else
>  > +                               printf("Relocated:" RESET " to 
> %#09lx\n", gd->relocaddr);
> 
> Not for this patch but I'd really like to figure out a way to 
> enable/disable ANSI codes globally in U-Boot. For example, we should 
> disable them in tests, or when redirecting sandbox to a file. It affects 
> EFI tests too...so if you have any ideas... :-)

Only thing that comes to mind would be adding a compile time flag to 
printf which teaches it to parse and skip ANSI escape codes.
> 
>  > +                       break;
>  > +               case CORES:
>  > +                       ofnode_for_each_subnode(np, 
> ofnode_path("/cpus")) {
>  > +                               if (ofnode_name_eq(np, "cpu"))
>  > +                                       n_cpus++;
>  > +                       }
> 
> uclass_id_count(UCLASS_CPU)
> 
>  > +                       printf("CPU:" RESET " %d (1 in use)\n", n_cpus);
>  > +                       break;
>  > +               case MEMORY:
>  > +                       printf("Memory:" RESET " %lu MB\n", 
> (ulong)gd->ram_size >> 20);
>  > +                       break;
>  > +               case STORAGE:
>  > +               default:
>  > +                       if (i == STORAGE && get_mmc_num() > 0) {
>  > +                               mmc = find_mmc_device(0);
>  > +                               if (mmc)
>  > +                                       printf("Storage:" RESET "  
> mmc 0: %llu MB", mmc->capacity >> 20);
>  > +                       }
> 
> How about iterating through UCLASS_BLK ?

Probably a much more sensible approach tbh, can you easily determine the 
size of a block device?
> 
>  > +                       if (i >= STORAGE + 1) {
>  > +                               ret = blk_get_desc(UCLASS_SCSI, i - 
> (STORAGE + 1), &scsi_desc);
>  > +                               if (!ret && scsi_desc->type != 
> DEV_TYPE_UNKNOWN)
>  > +                                       /* The calculation here is 
> really lossy but close enough */
>  > +                                       printf("Storage:" RESET " 
> scsi %d: %lu MB", i - (STORAGE + 1),
>  > +                                              ((scsi_desc->lba >> 9) 
> * scsi_desc->blksz) >> 11);
> 
> You can use print_size() - and same above I suppose

haha yeahp i should really do that instead.

Thanks for the review!
> 
>  > +                       }
>  > +                       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"
>  > +);
>  > --
>  > 2.46.0
>  >
> 
> Regards,
> Simon
> 
> 
> [1] 
> https://docs.u-boot.org/en/latest/develop/codingstyle.html#include-files 
> <https://docs.u-boot.org/en/latest/develop/codingstyle.html#include-files>
Caleb Connolly Aug. 9, 2024, 5:23 p.m. UTC | #5
On 09/08/2024 08:18, Heinrich Schuchardt wrote:
> 
> 
> Am 8. August 2024 18:24:24 MESZ schrieb Caleb Connolly <caleb.connolly@linaro.org>:
>> While U-Boot does a pretty good job at printing information at startup
>> about what hardware it's running on, it can be hard to take pretty
>> pictures of this to show off on the internet (by far the most
>> important aspect of porting a device in 2024!).
>>
>> Add a small utility "ufetch" for displaying some information about U-Boot and
>> the hardware it's running on in a similar fashion to the popular neofetch tool
>> for *nix's [1].
>>
>> While the output is meant to be useful, it should also be pleasing to look at
>> and look good in screenshots. The ufetch command brings this to U-Boot,
>> featuring a colorful ASCII art version of the U-Boot logo and a fancy layout.
>>
>> Finally, tireless device porters can properly show off their hard work and get
>> the internet points they so deserve!
>>
>> Not everything is fully supported yet, but this seemed good enough for initial
>> inclusion. Only one MMC device is detected, and other than SCSI other storage
>> devices aren't supported.
> 
> 
> This command is not very generic. E.g. it supports SCSI drives but not SATA. Many boards have eMMC and SD-card. This lks more ike an RFC tan something ready to merge.

Honestly it didn't even occur to me to send an RFC since this is purely 
a vanity thing...

I'll make it generic over UCLASS_BLK if possible for the next revision.
> 
> Why don't you use a script or an EFI program to print whatever information you need?

Well , then you'd need to have a way to load the script or EFI binary, 
which might not be possible for an early U-Boot port which doesn't have 
storage support yet (or at least, be a bunch of effort).

The only motivation to have this in U-Boot is really because

a) it's cute
b) it motivates "reward oriented" people like myself to get U-Boot 
booting on new/interesting devices because now it can be really easily 
shown off.

Maybe it's a cynical take, but I think stuff like this really has the 
potential to get more folks interested. Nobody likes doing the invisible 
background work, so why not bring some visibility to things?

Assuming I did a bad job at conveying my intent in the patch 
description: the point here is not to convey useful debugging 
information to developers, it's to show off, look pretty, and be 
amusing. Plenty of other U-Boot commands can convey information, this 
one is more like the 2048 port.
> 
>>
>> [1]: https://en.wikipedia.org/wiki/Neofetch
>>
>> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
>> ---
>> Ephemeral demo image: https://0x0.st/XVUa.png
>> ---
>> cmd/Kconfig  |   7 ++
>> cmd/Makefile |   1 +
>> cmd/ufetch.c | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 209 insertions(+)
>> create mode 100644 cmd/ufetch.c
>>
>> diff --git a/cmd/Kconfig b/cmd/Kconfig
>> index 978f44eda426..70acb5727b04 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.
>> +
>> config CMD_FWU_METADATA
>> 	bool "fwu metadata read"
>> 	depends on FWU_MULTI_BANK_UPDATE
>> 	help
>> diff --git a/cmd/Makefile b/cmd/Makefile
>> index 87133cc27a8a..ffb04c8f2e0a 100644
>> --- a/cmd/Makefile
>> +++ b/cmd/Makefile
>> @@ -53,8 +53,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..f7374eb2e364
>> --- /dev/null
>> +++ b/cmd/ufetch.c
>> @@ -0,0 +1,201 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +/* Small "fetch" utility for U-Boot */
>> +
>> +#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 *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 "::-\033[38;5;7m=+**###%%%%%%%%###**+=  " BLUE "--:...:::  ",
>> +	BLUE BOLD "   .::.::--:........::::::--::::::......::::::.    ",
>> +	BLUE BOLD "    .::.....::::::::::...........:::::::::.::.     ",
>> +	BLUE BOLD "      .::::::::::::::::::::::::::::::::::::.       ",
>> +	BLUE BOLD "        .::::.::::::::::::::::::::::.::::.         ",
>> +	BLUE BOLD "          ..::::::::::::::::::::::::::..           ",
>> +	BLUE BOLD "             ...::::::::::::::::::...              ",
>> +	BLUE BOLD "                  ......::......                   ",
>> +};
>> +
>> +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)
>> +};
>> +
>> +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;
>> +	ofnode np;
>> +	struct mmc *mmc;
>> +	struct blk_desc *scsi_desc;
>> +	bool skip_ascii = false;
>> +
>> +	if (argc > 1 && strcmp(argv[1], "-n") == 0) {
>> +		skip_ascii = true;
>> +		num_lines = LAST_LINE;
>> +	}
>> +
>> +	for (int i = 0; i < num_lines; i++) {
>> +		if (!skip_ascii) {
>> +			if (i < ARRAY_SIZE(logo_lines))
>> +				printf("%s ", logo_lines[i]);
>> +			else
>> +				printf("%*c ", LINE_WIDTH, ' ');
>> +		}
>> +		switch (i) {
>> +		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:
> 
> Please, do not use misleading variable names.

it is meant to be a little satirical, but I get it.
> 
> 
>> +			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)
>> +				printf(", %s\n", ipaddr);
>> +			else
>> +				putc('\n');
>> +			break;
>> +		case CMDS:
>> +			n_cmds = ll_entry_count(struct cmd_tbl, cmd);
> 
> Who cares if you have 50 commands or 51 if you don't know which?

This is a play on the neofetch "packages" counter.
> 
> 
>> +			printf("Commands:" RESET " %d (help)\n", n_cmds);
>> +			break;
>> +		case CONSOLES:
>> +			printf("Consoles:" RESET " %s (%d baud)\n", env_get("stdout"),
>> +			       gd->baudrate);
>> +			break;
>> +		case DEVICES:
>> +			printf("Devices:" RESET " ");
>> +			/* TODO: walk the DM tree */
>> +			printf("Uncountable!\n");
> 
> Nothing we should merge.
> 
>> +			break;
>> +		case FEATURES:
>> +			printf("Features:" RESET " ");
>> +			if (IS_ENABLED(CONFIG_NET))
>> +				printf("Net");
> 
> Wouuldn't you want to know which devices where detected?
> 
>> +			if (IS_ENABLED(CONFIG_EFI_LOADER))
>> +				printf(", EFI");
>> +			printf("\n");
>> +			break;
>> +		case RELOCATION:
>> +			if (gd->flags & GD_FLG_SKIP_RELOC)
>> +				printf("Relocated:" RESET " no\n");
>> +			else
>> +				printf("Relocated:" RESET " to %#09lx\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);
>> +			break;
>> +		case MEMORY:
>> +			printf("Memory:" RESET " %lu MB\n", (ulong)gd->ram_size >> 20);
>> +			break;
>> +		case STORAGE:
>> +		default:
>> +			if (i == STORAGE && get_mmc_num() > 0) {
>> +				mmc = find_mmc_device(0);
>> +				if (mmc)
>> +					printf("Storage:" RESET "  mmc 0: %llu MB", mmc->capacity >> 20);
> 
> And if my eMMC is device 1 and SD card is 0?
> 
> Why wouldn't you iterate over all UCLASS_BLK devices?
> 
> Best regards
> 
> Heinrich
> 
>> +			}
>> +			if (i >= STORAGE + 1) {
>> +				ret = blk_get_desc(UCLASS_SCSI, i - (STORAGE + 1), &scsi_desc);
>> +				if (!ret && scsi_desc->type != DEV_TYPE_UNKNOWN)
>> +					/* The calculation here is really lossy but close enough */
>> +					printf("Storage:" RESET " scsi %d: %lu MB", i - (STORAGE + 1),
>> +					       ((scsi_desc->lba >> 9) * scsi_desc->blksz) >> 11);
>> +			}
>> +			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"
>> +);
Simon Glass Aug. 9, 2024, 6:34 p.m. UTC | #6
Hi Caleb,

On Fri, 9 Aug 2024 at 11:00, Caleb Connolly <caleb.connolly@linaro.org> wrote:
>
> Hi Simon,
>
> On 09/08/2024 03:55, Simon Glass wrote:
> > Hi Caleb,
> >
> > On Thu, 8 Aug 2024 at 10:32, Caleb Connolly <caleb.connolly@linaro.org
> > <mailto:caleb.connolly@linaro.org>> wrote:
> >  >
> >  > While U-Boot does a pretty good job at printing information at startup
> >  > about what hardware it's running on, it can be hard to take pretty
> >  > pictures of this to show off on the internet (by far the most
> >  > important aspect of porting a device in 2024!).
> >  >
> >  > Add a small utility "ufetch" for displaying some information about
> > U-Boot and
> >  > the hardware it's running on in a similar fashion to the popular
> > neofetch tool
> >  > for *nix's [1].
> >  >
> >  > While the output is meant to be useful, it should also be pleasing to
> > look at
> >  > and look good in screenshots. The ufetch command brings this to U-Boot,
> >  > featuring a colorful ASCII art version of the U-Boot logo and a fancy
> > layout.
> >  >
> >  > Finally, tireless device porters can properly show off their hard
> > work and get
> >  > the internet points they so deserve!
> >  >
> >  > Not everything is fully supported yet, but this seemed good enough
> > for initial
> >  > inclusion. Only one MMC device is detected, and other than SCSI other
> > storage
> >  > devices aren't supported.
> >  >
> >  > [1]: https://en.wikipedia.org/wiki/Neofetch
> > <https://en.wikipedia.org/wiki/Neofetch>
> >  >
> >  > Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org
> > <mailto:caleb.connolly@linaro.org>>
> >  > ---
> >  > Ephemeral demo image: https://0x0.st/XVUa.png <https://0x0.st/XVUa.png>
> >  > ---
> >  >  cmd/Kconfig  |   7 ++
> >  >  cmd/Makefile |   1 +
> >  >  cmd/ufetch.c | 201 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  >  3 files changed, 209 insertions(+)
> >  >  create mode 100644 cmd/ufetch.c
> >
> > Very cute!
> >
> >  >
> >  > diff --git a/cmd/Kconfig b/cmd/Kconfig
> >  > index 978f44eda426..70acb5727b04 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"
> >
> > How about default y if SANDBOX so that this gets built for that. You
> > should also add doc/ and test/ for the command (although the test can be
> > very basic).
>
> I was really aiming for cmd/2048.c levels of expectation and usability
> here, not to make this a rugged part of U-Boot's feature set (if someone
> else would like to go the extra mile they're certainly welcome to).
>
> Making it default y for sandbox so we can at least make sure it compiles
> sounds reasonable though.
> >
> >  > +       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.
> >  > +
> >  >  config CMD_FWU_METADATA
> >  >         bool "fwu metadata read"
> >  >         depends on FWU_MULTI_BANK_UPDATE
> >  >         help
> >  > diff --git a/cmd/Makefile b/cmd/Makefile
> >  > index 87133cc27a8a..ffb04c8f2e0a 100644
> >  > --- a/cmd/Makefile
> >  > +++ b/cmd/Makefile
> >  > @@ -53,8 +53,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..f7374eb2e364
> >  > --- /dev/null
> >  > +++ b/cmd/ufetch.c
> >  > @@ -0,0 +1,201 @@
> >  > +// SPDX-License-Identifier: GPL-2.0
> >  > +
> >  > +/* Small "fetch" utility for U-Boot */
> >
> > Isn't it a command, rather than a utility? I think of a utility as a
> > program I run.
>
> I feel like you could ask 5 different people about this and get 5
> different answers. As a native british english speaker I would view
> "utilities" as a subset of "commands" in this context. This isn't a high
> effort contribution for me so I'm really fine with whatever.

OK I see, fair enough.

> >
> >  > +
> >  > +#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>
> >
> > Please check [1]
>
> Sure, I'll order these alphabetically.
> >
> >  > +
> >  > +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 *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
> > "::-\033[38;5;7m=+**###%%%%%%%%###**+=  " BLUE "--:...:::  ",
> >  > +       BLUE BOLD "   .::.::--:........::::::--::::::......::::::.    ",
> >  > +       BLUE BOLD "    .::.....::::::::::...........:::::::::.::.     ",
> >  > +       BLUE BOLD "      .::::::::::::::::::::::::::::::::::::.       ",
> >  > +       BLUE BOLD "        .::::.::::::::::::::::::::::.::::.         ",
> >  > +       BLUE BOLD "          ..::::::::::::::::::::::::::..           ",
> >  > +       BLUE BOLD "             ...::::::::::::::::::...              ",
> >  > +       BLUE BOLD "                  ......::......                   ",
> >  > +};
> >  > +
> >  > +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)
> >  > +};
> >  > +
> >  > +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;
> >  > +       ofnode np;
> >  > +       struct mmc *mmc;
> >  > +       struct blk_desc *scsi_desc;
> >  > +       bool skip_ascii = false;
> >  > +
> >  > +       if (argc > 1 && strcmp(argv[1], "-n") == 0) {
> >  > +               skip_ascii = true;
> >  > +               num_lines = LAST_LINE;
> >  > +       }
> >  > +
> >  > +       for (int i = 0; i < num_lines; i++) {
> >  > +               if (!skip_ascii) {
> >  > +                       if (i < ARRAY_SIZE(logo_lines))
> >  > +                               printf("%s ", logo_lines[i]);
> >  > +                       else
> >  > +                               printf("%*c ", LINE_WIDTH, ' ');
> >  > +               }
> >  > +               switch (i) {
> >  > +               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)
> >  > +                               printf(", %s\n", ipaddr);
> >  > +                       else
> >  > +                               putc('\n');
> >  > +                       break;
> >  > +               case CMDS:
> >  > +                       n_cmds = ll_entry_count(struct cmd_tbl, cmd);
> >  > +                       printf("Commands:" RESET " %d (help)\n", n_cmds);
> >  > +                       break;
> >  > +               case CONSOLES:
> >  > +                       printf("Consoles:" RESET " %s (%d baud)\n",
> > env_get("stdout"),
> >  > +                              gd->baudrate);
> >  > +                       break;
> >  > +               case DEVICES:
> >  > +                       printf("Devices:" RESET " ");
> >  > +                       /* TODO: walk the DM tree */
> >  > +                       printf("Uncountable!\n");
> >
> > See dm_announce() for how to do that.
> >
> >  > +                       break;
> >  > +               case FEATURES:
> >  > +                       printf("Features:" RESET " ");
> >  > +                       if (IS_ENABLED(CONFIG_NET))
> >  > +                               printf("Net");
> >  > +                       if (IS_ENABLED(CONFIG_EFI_LOADER))
> >  > +                               printf(", EFI");
> >
> > How about EFI_LOADER ? After all, U-Boot can run as an EFI app so 'EFI'
> > might be better reserved for that.
> >
> >  > +                       printf("\n");
> >  > +                       break;
> >  > +               case RELOCATION:
> >  > +                       if (gd->flags & GD_FLG_SKIP_RELOC)
> >  > +                               printf("Relocated:" RESET " no\n");
> >  > +                       else
> >  > +                               printf("Relocated:" RESET " to
> > %#09lx\n", gd->relocaddr);
> >
> > Not for this patch but I'd really like to figure out a way to
> > enable/disable ANSI codes globally in U-Boot. For example, we should
> > disable them in tests, or when redirecting sandbox to a file. It affects
> > EFI tests too...so if you have any ideas... :-)
>
> Only thing that comes to mind would be adding a compile time flag to
> printf which teaches it to parse and skip ANSI escape codes.

Ideally it would have the option to be run-time, if we want to pay an
extra penalty. Then we can use the same sandbox build with tests as we
do for normal usage.

> >
> >  > +                       break;
> >  > +               case CORES:
> >  > +                       ofnode_for_each_subnode(np,
> > ofnode_path("/cpus")) {
> >  > +                               if (ofnode_name_eq(np, "cpu"))
> >  > +                                       n_cpus++;
> >  > +                       }
> >
> > uclass_id_count(UCLASS_CPU)
> >
> >  > +                       printf("CPU:" RESET " %d (1 in use)\n", n_cpus);
> >  > +                       break;
> >  > +               case MEMORY:
> >  > +                       printf("Memory:" RESET " %lu MB\n",
> > (ulong)gd->ram_size >> 20);
> >  > +                       break;
> >  > +               case STORAGE:
> >  > +               default:
> >  > +                       if (i == STORAGE && get_mmc_num() > 0) {
> >  > +                               mmc = find_mmc_device(0);
> >  > +                               if (mmc)
> >  > +                                       printf("Storage:" RESET "
> > mmc 0: %llu MB", mmc->capacity >> 20);
> >  > +                       }
> >
> > How about iterating through UCLASS_BLK ?
>
> Probably a much more sensible approach tbh, can you easily determine the
> size of a block device?

You can do something like this:

struct udevice *dev;   // block device
struct blk_desc *desc = dev_get_uclass_plat(dev);

size = desc->lba * desc->blksz

> >
> >  > +                       if (i >= STORAGE + 1) {
> >  > +                               ret = blk_get_desc(UCLASS_SCSI, i -
> > (STORAGE + 1), &scsi_desc);
> >  > +                               if (!ret && scsi_desc->type !=
> > DEV_TYPE_UNKNOWN)
> >  > +                                       /* The calculation here is
> > really lossy but close enough */
> >  > +                                       printf("Storage:" RESET "
> > scsi %d: %lu MB", i - (STORAGE + 1),
> >  > +                                              ((scsi_desc->lba >> 9)
> > * scsi_desc->blksz) >> 11);
> >
> > You can use print_size() - and same above I suppose
>
> haha yeahp i should really do that instead.
>
> Thanks for the review!
> >
> >  > +                       }
> >  > +                       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"
> >  > +);
> >  > --
> >  > 2.46.0
> >

> >
> > [1]
> > https://docs.u-boot.org/en/latest/develop/codingstyle.html#include-files
> > <https://docs.u-boot.org/en/latest/develop/codingstyle.html#include-files>


Regards,
Simon
Tom Rini Aug. 9, 2024, 10:02 p.m. UTC | #7
On Fri, Aug 09, 2024 at 07:23:42PM +0200, Caleb Connolly wrote:
> 
> 
> On 09/08/2024 08:18, Heinrich Schuchardt wrote:
> > 
> > 
> > Am 8. August 2024 18:24:24 MESZ schrieb Caleb Connolly <caleb.connolly@linaro.org>:
> > > While U-Boot does a pretty good job at printing information at startup
> > > about what hardware it's running on, it can be hard to take pretty
> > > pictures of this to show off on the internet (by far the most
> > > important aspect of porting a device in 2024!).
> > > 
> > > Add a small utility "ufetch" for displaying some information about U-Boot and
> > > the hardware it's running on in a similar fashion to the popular neofetch tool
> > > for *nix's [1].
> > > 
> > > While the output is meant to be useful, it should also be pleasing to look at
> > > and look good in screenshots. The ufetch command brings this to U-Boot,
> > > featuring a colorful ASCII art version of the U-Boot logo and a fancy layout.
> > > 
> > > Finally, tireless device porters can properly show off their hard work and get
> > > the internet points they so deserve!
> > > 
> > > Not everything is fully supported yet, but this seemed good enough for initial
> > > inclusion. Only one MMC device is detected, and other than SCSI other storage
> > > devices aren't supported.
> > 
> > 
> > This command is not very generic. E.g. it supports SCSI drives but not SATA. Many boards have eMMC and SD-card. This lks more ike an RFC tan something ready to merge.
> 
> Honestly it didn't even occur to me to send an RFC since this is purely a
> vanity thing...
> 
> I'll make it generic over UCLASS_BLK if possible for the next revision.
> > 
> > Why don't you use a script or an EFI program to print whatever information you need?
> 
> Well , then you'd need to have a way to load the script or EFI binary, which
> might not be possible for an early U-Boot port which doesn't have storage
> support yet (or at least, be a bunch of effort).
> 
> The only motivation to have this in U-Boot is really because
> 
> a) it's cute
> b) it motivates "reward oriented" people like myself to get U-Boot booting
> on new/interesting devices because now it can be really easily shown off.
> 
> Maybe it's a cynical take, but I think stuff like this really has the
> potential to get more folks interested. Nobody likes doing the invisible
> background work, so why not bring some visibility to things?
> 
> Assuming I did a bad job at conveying my intent in the patch description:
> the point here is not to convey useful debugging information to developers,
> it's to show off, look pretty, and be amusing. Plenty of other U-Boot
> commands can convey information, this one is more like the 2048 port.

So, at the high level, yes, this is something that ought to get merged
as it's handy enough, but not intended to replace for example smbios
information.
Tony Dinh Nov. 13, 2024, 12:40 a.m. UTC | #8
Hi Tom,

On Fri, Aug 9, 2024 at 3:41 PM Tom Rini <trini@konsulko.com> wrote:
>
> On Fri, Aug 09, 2024 at 07:23:42PM +0200, Caleb Connolly wrote:
> >
> >
> > On 09/08/2024 08:18, Heinrich Schuchardt wrote:
> > >
> > >
> > > Am 8. August 2024 18:24:24 MESZ schrieb Caleb Connolly <caleb.connolly@linaro.org>:
> > > > While U-Boot does a pretty good job at printing information at startup
> > > > about what hardware it's running on, it can be hard to take pretty
> > > > pictures of this to show off on the internet (by far the most
> > > > important aspect of porting a device in 2024!).
> > > >
> > > > Add a small utility "ufetch" for displaying some information about U-Boot and
> > > > the hardware it's running on in a similar fashion to the popular neofetch tool
> > > > for *nix's [1].
> > > >
> > > > While the output is meant to be useful, it should also be pleasing to look at
> > > > and look good in screenshots. The ufetch command brings this to U-Boot,
> > > > featuring a colorful ASCII art version of the U-Boot logo and a fancy layout.
> > > >
> > > > Finally, tireless device porters can properly show off their hard work and get
> > > > the internet points they so deserve!
> > > >
> > > > Not everything is fully supported yet, but this seemed good enough for initial
> > > > inclusion. Only one MMC device is detected, and other than SCSI other storage
> > > > devices aren't supported.
> > >
> > >
> > > This command is not very generic. E.g. it supports SCSI drives but not SATA. Many boards have eMMC and SD-card. This lks more ike an RFC tan something ready to merge.
> >
> > Honestly it didn't even occur to me to send an RFC since this is purely a
> > vanity thing...
> >
> > I'll make it generic over UCLASS_BLK if possible for the next revision.
> > >
> > > Why don't you use a script or an EFI program to print whatever information you need?
> >
> > Well , then you'd need to have a way to load the script or EFI binary, which
> > might not be possible for an early U-Boot port which doesn't have storage
> > support yet (or at least, be a bunch of effort).
> >
> > The only motivation to have this in U-Boot is really because
> >
> > a) it's cute
> > b) it motivates "reward oriented" people like myself to get U-Boot booting
> > on new/interesting devices because now it can be really easily shown off.
> >
> > Maybe it's a cynical take, but I think stuff like this really has the
> > potential to get more folks interested. Nobody likes doing the invisible
> > background work, so why not bring some visibility to things?
> >
> > Assuming I did a bad job at conveying my intent in the patch description:
> > the point here is not to convey useful debugging information to developers,
> > it's to show off, look pretty, and be amusing. Plenty of other U-Boot
> > commands can convey information, this one is more like the 2048 port.
>
> So, at the high level, yes, this is something that ought to get merged
> as it's handy enough, but not intended to replace for example smbios
> information.

Up vote for this patch. Can we merge this to next?

All the best,
Tony

>
> --
> Tom
Tom Rini Nov. 13, 2024, 12:55 a.m. UTC | #9
On Tue, Nov 12, 2024 at 04:40:44PM -0800, Tony Dinh wrote:
> Hi Tom,
> 
> On Fri, Aug 9, 2024 at 3:41 PM Tom Rini <trini@konsulko.com> wrote:
> >
> > On Fri, Aug 09, 2024 at 07:23:42PM +0200, Caleb Connolly wrote:
> > >
> > >
> > > On 09/08/2024 08:18, Heinrich Schuchardt wrote:
> > > >
> > > >
> > > > Am 8. August 2024 18:24:24 MESZ schrieb Caleb Connolly <caleb.connolly@linaro.org>:
> > > > > While U-Boot does a pretty good job at printing information at startup
> > > > > about what hardware it's running on, it can be hard to take pretty
> > > > > pictures of this to show off on the internet (by far the most
> > > > > important aspect of porting a device in 2024!).
> > > > >
> > > > > Add a small utility "ufetch" for displaying some information about U-Boot and
> > > > > the hardware it's running on in a similar fashion to the popular neofetch tool
> > > > > for *nix's [1].
> > > > >
> > > > > While the output is meant to be useful, it should also be pleasing to look at
> > > > > and look good in screenshots. The ufetch command brings this to U-Boot,
> > > > > featuring a colorful ASCII art version of the U-Boot logo and a fancy layout.
> > > > >
> > > > > Finally, tireless device porters can properly show off their hard work and get
> > > > > the internet points they so deserve!
> > > > >
> > > > > Not everything is fully supported yet, but this seemed good enough for initial
> > > > > inclusion. Only one MMC device is detected, and other than SCSI other storage
> > > > > devices aren't supported.
> > > >
> > > >
> > > > This command is not very generic. E.g. it supports SCSI drives but not SATA. Many boards have eMMC and SD-card. This lks more ike an RFC tan something ready to merge.
> > >
> > > Honestly it didn't even occur to me to send an RFC since this is purely a
> > > vanity thing...
> > >
> > > I'll make it generic over UCLASS_BLK if possible for the next revision.
> > > >
> > > > Why don't you use a script or an EFI program to print whatever information you need?
> > >
> > > Well , then you'd need to have a way to load the script or EFI binary, which
> > > might not be possible for an early U-Boot port which doesn't have storage
> > > support yet (or at least, be a bunch of effort).
> > >
> > > The only motivation to have this in U-Boot is really because
> > >
> > > a) it's cute
> > > b) it motivates "reward oriented" people like myself to get U-Boot booting
> > > on new/interesting devices because now it can be really easily shown off.
> > >
> > > Maybe it's a cynical take, but I think stuff like this really has the
> > > potential to get more folks interested. Nobody likes doing the invisible
> > > background work, so why not bring some visibility to things?
> > >
> > > Assuming I did a bad job at conveying my intent in the patch description:
> > > the point here is not to convey useful debugging information to developers,
> > > it's to show off, look pretty, and be amusing. Plenty of other U-Boot
> > > commands can convey information, this one is more like the 2048 port.
> >
> > So, at the high level, yes, this is something that ought to get merged
> > as it's handy enough, but not intended to replace for example smbios
> > information.
> 
> Up vote for this patch. Can we merge this to next?

I think I was expecting another iteration from Caleb based on some minor
feedback?
diff mbox series

Patch

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 978f44eda426..70acb5727b04 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.
+
 config CMD_FWU_METADATA
 	bool "fwu metadata read"
 	depends on FWU_MULTI_BANK_UPDATE
 	help
diff --git a/cmd/Makefile b/cmd/Makefile
index 87133cc27a8a..ffb04c8f2e0a 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -53,8 +53,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..f7374eb2e364
--- /dev/null
+++ b/cmd/ufetch.c
@@ -0,0 +1,201 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+/* Small "fetch" utility for U-Boot */
+
+#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 *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 "::-\033[38;5;7m=+**###%%%%%%%%###**+=  " BLUE "--:...:::  ",
+	BLUE BOLD "   .::.::--:........::::::--::::::......::::::.    ",
+	BLUE BOLD "    .::.....::::::::::...........:::::::::.::.     ",
+	BLUE BOLD "      .::::::::::::::::::::::::::::::::::::.       ",
+	BLUE BOLD "        .::::.::::::::::::::::::::::.::::.         ",
+	BLUE BOLD "          ..::::::::::::::::::::::::::..           ",
+	BLUE BOLD "             ...::::::::::::::::::...              ",
+	BLUE BOLD "                  ......::......                   ",
+};
+
+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)
+};
+
+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;
+	ofnode np;
+	struct mmc *mmc;
+	struct blk_desc *scsi_desc;
+	bool skip_ascii = false;
+
+	if (argc > 1 && strcmp(argv[1], "-n") == 0) {
+		skip_ascii = true;
+		num_lines = LAST_LINE;
+	}
+
+	for (int i = 0; i < num_lines; i++) {
+		if (!skip_ascii) {
+			if (i < ARRAY_SIZE(logo_lines))
+				printf("%s ", logo_lines[i]);
+			else
+				printf("%*c ", LINE_WIDTH, ' ');
+		}
+		switch (i) {
+		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)
+				printf(", %s\n", ipaddr);
+			else
+				putc('\n');
+			break;
+		case CMDS:
+			n_cmds = ll_entry_count(struct cmd_tbl, cmd);
+			printf("Commands:" RESET " %d (help)\n", n_cmds);
+			break;
+		case CONSOLES:
+			printf("Consoles:" RESET " %s (%d baud)\n", env_get("stdout"),
+			       gd->baudrate);
+			break;
+		case DEVICES:
+			printf("Devices:" RESET " ");
+			/* TODO: walk the DM tree */
+			printf("Uncountable!\n");
+			break;
+		case FEATURES:
+			printf("Features:" RESET " ");
+			if (IS_ENABLED(CONFIG_NET))
+				printf("Net");
+			if (IS_ENABLED(CONFIG_EFI_LOADER))
+				printf(", EFI");
+			printf("\n");
+			break;
+		case RELOCATION:
+			if (gd->flags & GD_FLG_SKIP_RELOC)
+				printf("Relocated:" RESET " no\n");
+			else
+				printf("Relocated:" RESET " to %#09lx\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);
+			break;
+		case MEMORY:
+			printf("Memory:" RESET " %lu MB\n", (ulong)gd->ram_size >> 20);
+			break;
+		case STORAGE:
+		default:
+			if (i == STORAGE && get_mmc_num() > 0) {
+				mmc = find_mmc_device(0);
+				if (mmc)
+					printf("Storage:" RESET "  mmc 0: %llu MB", mmc->capacity >> 20);
+			}
+			if (i >= STORAGE + 1) {
+				ret = blk_get_desc(UCLASS_SCSI, i - (STORAGE + 1), &scsi_desc);
+				if (!ret && scsi_desc->type != DEV_TYPE_UNKNOWN)
+					/* The calculation here is really lossy but close enough */
+					printf("Storage:" RESET " scsi %d: %lu MB", i - (STORAGE + 1),
+					       ((scsi_desc->lba >> 9) * scsi_desc->blksz) >> 11);
+			}
+			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"
+);