* [PATCH 0/3] cmd: ufetch improvements
@ 2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
0 siblings, 0 replies; 14+ messages in thread
From: J. Neuschäfer @ 2024-12-05 18:35 UTC (permalink / raw)
To: Caleb Connolly, Tom Rini; +Cc: u-boot, J. Neuschäfer
Following the recent addition of the "ufetch" command, this patchset
improves it in a few ways.
Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
J. Neuschäfer (3):
cmd: ufetch: Fix type mismatch on 32-bit
cmd: Allow building ufetch without CONFIG_BLK
cmd: ufetch: Show CPU architecture under "CPU"
cmd/Kconfig | 1 -
cmd/ufetch.c | 14 +++++++++-----
2 files changed, 9 insertions(+), 6 deletions(-)
---
base-commit: e5c1c2c6d9da79d874015f6d33fa2c7a34899733
change-id: 20241203-ufetch-326e771b26ad
Best regards,
--
J. Neuschäfer <j.ne@posteo.net>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/3] cmd: ufetch improvements
@ 2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
0 siblings, 0 replies; 14+ messages in thread
From: J. Neuschäfer via B4 Relay @ 2024-12-05 18:35 UTC (permalink / raw)
To: Caleb Connolly, Tom Rini; +Cc: u-boot, J. Neuschäfer
Following the recent addition of the "ufetch" command, this patchset
improves it in a few ways.
Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
J. Neuschäfer (3):
cmd: ufetch: Fix type mismatch on 32-bit
cmd: Allow building ufetch without CONFIG_BLK
cmd: ufetch: Show CPU architecture under "CPU"
cmd/Kconfig | 1 -
cmd/ufetch.c | 14 +++++++++-----
2 files changed, 9 insertions(+), 6 deletions(-)
---
base-commit: e5c1c2c6d9da79d874015f6d33fa2c7a34899733
change-id: 20241203-ufetch-326e771b26ad
Best regards,
--
J. Neuschäfer <j.ne@posteo.net>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/3] cmd: ufetch: Fix type mismatch on 32-bit
2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
@ 2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
-1 siblings, 0 replies; 14+ messages in thread
From: J. Neuschäfer @ 2024-12-05 18:35 UTC (permalink / raw)
To: Caleb Connolly, Tom Rini; +Cc: u-boot, J. Neuschäfer
On 32-bit architectures, LAST_LINE (_LAST_LINE - 1UL) is 64 bits long,
but size_t (from ARRAY_SIZE(...)) is 32 bits. This results in a warning
because the max() macro expects the same type on both sides:
cmd/ufetch.c: In function ‘do_ufetch’:
include/linux/kernel.h:179:24: warning: comparison of distinct pointer types lacks a cast [-Wcompare-distinct-pointer-types]
179 | (void) (&_max1 == &_max2); \
| ^~
cmd/ufetch.c:92:25: note: in expansion of macro ‘max’
92 | int num_lines = max(LAST_LINE + 1, ARRAY_SIZE(logo_lines));
| ^~~
Fix this by casting LAST_LINE to size_t.
Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
cmd/ufetch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/ufetch.c b/cmd/ufetch.c
index 0b825d7e8c75f3b18933d3e3f77e5f40f2c7b658..5f3ef847b268dc384271fc6774720e5fd2337157 100644
--- a/cmd/ufetch.c
+++ b/cmd/ufetch.c
@@ -89,7 +89,7 @@ enum output_lines {
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));
+ int num_lines = max((size_t)LAST_LINE + 1, ARRAY_SIZE(logo_lines));
const char *model, *compatible;
char *ipaddr;
int n_cmds, n_cpus = 0, ret, compatlen;
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 1/3] cmd: ufetch: Fix type mismatch on 32-bit
@ 2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
0 siblings, 0 replies; 14+ messages in thread
From: J. Neuschäfer via B4 Relay @ 2024-12-05 18:35 UTC (permalink / raw)
To: Caleb Connolly, Tom Rini; +Cc: u-boot, J. Neuschäfer
From: "J. Neuschäfer" <j.ne@posteo.net>
On 32-bit architectures, LAST_LINE (_LAST_LINE - 1UL) is 64 bits long,
but size_t (from ARRAY_SIZE(...)) is 32 bits. This results in a warning
because the max() macro expects the same type on both sides:
cmd/ufetch.c: In function ‘do_ufetch’:
include/linux/kernel.h:179:24: warning: comparison of distinct pointer types lacks a cast [-Wcompare-distinct-pointer-types]
179 | (void) (&_max1 == &_max2); \
| ^~
cmd/ufetch.c:92:25: note: in expansion of macro ‘max’
92 | int num_lines = max(LAST_LINE + 1, ARRAY_SIZE(logo_lines));
| ^~~
Fix this by casting LAST_LINE to size_t.
Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
cmd/ufetch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/ufetch.c b/cmd/ufetch.c
index 0b825d7e8c75f3b18933d3e3f77e5f40f2c7b658..5f3ef847b268dc384271fc6774720e5fd2337157 100644
--- a/cmd/ufetch.c
+++ b/cmd/ufetch.c
@@ -89,7 +89,7 @@ enum output_lines {
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));
+ int num_lines = max((size_t)LAST_LINE + 1, ARRAY_SIZE(logo_lines));
const char *model, *compatible;
char *ipaddr;
int n_cmds, n_cpus = 0, ret, compatlen;
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] cmd: Allow building ufetch without CONFIG_BLK
2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
@ 2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
-1 siblings, 0 replies; 14+ messages in thread
From: J. Neuschäfer @ 2024-12-05 18:35 UTC (permalink / raw)
To: Caleb Connolly, Tom Rini; +Cc: u-boot, J. Neuschäfer
The ufetch command is still quite useful on systems without block
device support; remove the CONFIG_BLK dependency and make sure the code
compiles/works with and without CONFIG_BLK.
Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
cmd/Kconfig | 1 -
cmd/ufetch.c | 10 +++++++---
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 4936a70f3ef16ddb093ceafa12d011ca1b89e95c..547fd2a91f7883e2ae5982897ec93c4483d67852 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -178,7 +178,6 @@ config CMD_CPU
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.
diff --git a/cmd/ufetch.c b/cmd/ufetch.c
index 5f3ef847b268dc384271fc6774720e5fd2337157..7aed0b447bda104b837d37c6adcbb21b80237aba 100644
--- a/cmd/ufetch.c
+++ b/cmd/ufetch.c
@@ -92,11 +92,9 @@ static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
int num_lines = max((size_t)LAST_LINE + 1, ARRAY_SIZE(logo_lines));
const char *model, *compatible;
char *ipaddr;
- int n_cmds, n_cpus = 0, ret, compatlen;
+ int n_cmds, n_cpus = 0, compatlen;
size_t size;
ofnode np;
- struct udevice *dev;
- struct blk_desc *desc;
bool skip_ascii = false;
if (argc > 1 && strcmp(argv[1], "-n") == 0) {
@@ -200,6 +198,11 @@ static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
break;
case STORAGE:
default:
+#ifdef CONFIG_BLK
+ struct udevice *dev;
+ struct blk_desc *desc;
+ int ret;
+
ret = uclass_find_device_by_seq(UCLASS_BLK, line - STORAGE, &dev);
if (!ret && dev) {
desc = dev_get_uclass_plat(dev);
@@ -213,6 +216,7 @@ static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
} else if (ret == -ENODEV && (skip_ascii || line > ARRAY_SIZE(logo_lines))) {
break;
}
+#endif
printf("\n");
}
}
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] cmd: Allow building ufetch without CONFIG_BLK
@ 2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
0 siblings, 0 replies; 14+ messages in thread
From: J. Neuschäfer via B4 Relay @ 2024-12-05 18:35 UTC (permalink / raw)
To: Caleb Connolly, Tom Rini; +Cc: u-boot, J. Neuschäfer
From: "J. Neuschäfer" <j.ne@posteo.net>
The ufetch command is still quite useful on systems without block
device support; remove the CONFIG_BLK dependency and make sure the code
compiles/works with and without CONFIG_BLK.
Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
cmd/Kconfig | 1 -
cmd/ufetch.c | 10 +++++++---
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/cmd/Kconfig b/cmd/Kconfig
index 4936a70f3ef16ddb093ceafa12d011ca1b89e95c..547fd2a91f7883e2ae5982897ec93c4483d67852 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -178,7 +178,6 @@ config CMD_CPU
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.
diff --git a/cmd/ufetch.c b/cmd/ufetch.c
index 5f3ef847b268dc384271fc6774720e5fd2337157..7aed0b447bda104b837d37c6adcbb21b80237aba 100644
--- a/cmd/ufetch.c
+++ b/cmd/ufetch.c
@@ -92,11 +92,9 @@ static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
int num_lines = max((size_t)LAST_LINE + 1, ARRAY_SIZE(logo_lines));
const char *model, *compatible;
char *ipaddr;
- int n_cmds, n_cpus = 0, ret, compatlen;
+ int n_cmds, n_cpus = 0, compatlen;
size_t size;
ofnode np;
- struct udevice *dev;
- struct blk_desc *desc;
bool skip_ascii = false;
if (argc > 1 && strcmp(argv[1], "-n") == 0) {
@@ -200,6 +198,11 @@ static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
break;
case STORAGE:
default:
+#ifdef CONFIG_BLK
+ struct udevice *dev;
+ struct blk_desc *desc;
+ int ret;
+
ret = uclass_find_device_by_seq(UCLASS_BLK, line - STORAGE, &dev);
if (!ret && dev) {
desc = dev_get_uclass_plat(dev);
@@ -213,6 +216,7 @@ static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
} else if (ret == -ENODEV && (skip_ascii || line > ARRAY_SIZE(logo_lines))) {
break;
}
+#endif
printf("\n");
}
}
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] cmd: ufetch: Show CPU architecture under "CPU"
2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
@ 2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
-1 siblings, 0 replies; 14+ messages in thread
From: J. Neuschäfer @ 2024-12-05 18:35 UTC (permalink / raw)
To: Caleb Connolly, Tom Rini; +Cc: u-boot, J. Neuschäfer
When looking at ufetch output it isn't immediately obvious which CPU
architecture the presented board has. This patch therefore adds the CPU
architecture string (for example "powerpc") to the "CPU:" line.
Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
cmd/ufetch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/ufetch.c b/cmd/ufetch.c
index 7aed0b447bda104b837d37c6adcbb21b80237aba..0ee50bfb606aa2223072c63cd46b3a70ef5449fd 100644
--- a/cmd/ufetch.c
+++ b/cmd/ufetch.c
@@ -188,7 +188,7 @@ static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
if (ofnode_name_eq(np, "cpu"))
n_cpus++;
}
- printf("CPU:" RESET " %d (1 in use)\n", n_cpus);
+ printf("CPU: " RESET CONFIG_SYS_ARCH ", %d (1 in use)\n", n_cpus);
break;
case MEMORY:
for (int j = 0; j < CONFIG_NR_DRAM_BANKS && gd->bd->bi_dram[j].size; j++)
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] cmd: ufetch: Show CPU architecture under "CPU"
@ 2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
0 siblings, 0 replies; 14+ messages in thread
From: J. Neuschäfer via B4 Relay @ 2024-12-05 18:35 UTC (permalink / raw)
To: Caleb Connolly, Tom Rini; +Cc: u-boot, J. Neuschäfer
From: "J. Neuschäfer" <j.ne@posteo.net>
When looking at ufetch output it isn't immediately obvious which CPU
architecture the presented board has. This patch therefore adds the CPU
architecture string (for example "powerpc") to the "CPU:" line.
Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
---
cmd/ufetch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/cmd/ufetch.c b/cmd/ufetch.c
index 7aed0b447bda104b837d37c6adcbb21b80237aba..0ee50bfb606aa2223072c63cd46b3a70ef5449fd 100644
--- a/cmd/ufetch.c
+++ b/cmd/ufetch.c
@@ -188,7 +188,7 @@ static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
if (ofnode_name_eq(np, "cpu"))
n_cpus++;
}
- printf("CPU:" RESET " %d (1 in use)\n", n_cpus);
+ printf("CPU: " RESET CONFIG_SYS_ARCH ", %d (1 in use)\n", n_cpus);
break;
case MEMORY:
for (int j = 0; j < CONFIG_NR_DRAM_BANKS && gd->bd->bi_dram[j].size; j++)
--
2.45.2
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/3] cmd: ufetch: Fix type mismatch on 32-bit
2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
(?)
@ 2024-12-09 14:58 ` Caleb Connolly
-1 siblings, 0 replies; 14+ messages in thread
From: Caleb Connolly @ 2024-12-09 14:58 UTC (permalink / raw)
To: j.ne, Tom Rini; +Cc: u-boot
Hi J,
Thanks for the patch!
On 05/12/2024 19:35, J. Neuschäfer via B4 Relay wrote:
> From: "J. Neuschäfer" <j.ne@posteo.net>
>
> On 32-bit architectures, LAST_LINE (_LAST_LINE - 1UL) is 64 bits long,
> but size_t (from ARRAY_SIZE(...)) is 32 bits. This results in a warning
> because the max() macro expects the same type on both sides:
>
> cmd/ufetch.c: In function ‘do_ufetch’:
> include/linux/kernel.h:179:24: warning: comparison of distinct pointer types lacks a cast [-Wcompare-distinct-pointer-types]
> 179 | (void) (&_max1 == &_max2); \
> | ^~
> cmd/ufetch.c:92:25: note: in expansion of macro ‘max’
> 92 | int num_lines = max(LAST_LINE + 1, ARRAY_SIZE(logo_lines));
> | ^~~
>
> Fix this by casting LAST_LINE to size_t.
>
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> cmd/ufetch.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/cmd/ufetch.c b/cmd/ufetch.c
> index 0b825d7e8c75f3b18933d3e3f77e5f40f2c7b658..5f3ef847b268dc384271fc6774720e5fd2337157 100644
> --- a/cmd/ufetch.c
> +++ b/cmd/ufetch.c
> @@ -89,7 +89,7 @@ enum output_lines {
> 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));
> + int num_lines = max((size_t)LAST_LINE + 1, ARRAY_SIZE(logo_lines));
> const char *model, *compatible;
> char *ipaddr;
> int n_cmds, n_cpus = 0, ret, compatlen;
>
--
// Caleb (they/them)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] cmd: Allow building ufetch without CONFIG_BLK
2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
(?)
@ 2024-12-09 15:04 ` Caleb Connolly
2024-12-09 16:51 ` J. Neuschäfer
-1 siblings, 1 reply; 14+ messages in thread
From: Caleb Connolly @ 2024-12-09 15:04 UTC (permalink / raw)
To: j.ne, Tom Rini; +Cc: u-boot
On 05/12/2024 19:35, J. Neuschäfer via B4 Relay wrote:
> From: "J. Neuschäfer" <j.ne@posteo.net>
>
> The ufetch command is still quite useful on systems without block
> device support; remove the CONFIG_BLK dependency and make sure the code
> compiles/works with and without CONFIG_BLK.
>
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
Small nit below, but with that:
Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
> cmd/Kconfig | 1 -
> cmd/ufetch.c | 10 +++++++---
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 4936a70f3ef16ddb093ceafa12d011ca1b89e95c..547fd2a91f7883e2ae5982897ec93c4483d67852 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -178,7 +178,6 @@ config CMD_CPU
>
> 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.
> diff --git a/cmd/ufetch.c b/cmd/ufetch.c
> index 5f3ef847b268dc384271fc6774720e5fd2337157..7aed0b447bda104b837d37c6adcbb21b80237aba 100644
> --- a/cmd/ufetch.c
> +++ b/cmd/ufetch.c
> @@ -92,11 +92,9 @@ static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
> int num_lines = max((size_t)LAST_LINE + 1, ARRAY_SIZE(logo_lines));
> const char *model, *compatible;
> char *ipaddr;
> - int n_cmds, n_cpus = 0, ret, compatlen;
> + int n_cmds, n_cpus = 0, compatlen;
> size_t size;
> ofnode np;
> - struct udevice *dev;
> - struct blk_desc *desc;
> bool skip_ascii = false;
>
> if (argc > 1 && strcmp(argv[1], "-n") == 0) {
> @@ -200,6 +198,11 @@ static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
> break;
> case STORAGE:
> default:
> +#ifdef CONFIG_BLK
> + struct udevice *dev;
At least without the #ifdef, I see a warning "A label followed by a
declaration is a C23 extension". So to be on the safe side please put
this section in a block:
default: {
...
}
Kind regards,
> + struct blk_desc *desc;
> + int ret;
> +
> ret = uclass_find_device_by_seq(UCLASS_BLK, line - STORAGE, &dev);
> if (!ret && dev) {
> desc = dev_get_uclass_plat(dev);
> @@ -213,6 +216,7 @@ static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
> } else if (ret == -ENODEV && (skip_ascii || line > ARRAY_SIZE(logo_lines))) {
> break;
> }
> +#endif
> printf("\n");
> }
> }
>
--
// Caleb (they/them)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] cmd: ufetch: Show CPU architecture under "CPU"
2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
(?)
@ 2024-12-09 15:09 ` Caleb Connolly
2024-12-09 16:58 ` J. Neuschäfer
-1 siblings, 1 reply; 14+ messages in thread
From: Caleb Connolly @ 2024-12-09 15:09 UTC (permalink / raw)
To: j.ne, Tom Rini; +Cc: u-boot
Thanks for the patch!
On 05/12/2024 19:35, J. Neuschäfer via B4 Relay wrote:
> From: "J. Neuschäfer" <j.ne@posteo.net>
>
> When looking at ufetch output it isn't immediately obvious which CPU
> architecture the presented board has. This patch therefore adds the CPU
> architecture string (for example "powerpc") to the "CPU:" line.
It would be nice to have the proper fancy name, but this is an
improvement nonetheless.
>
> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> ---
> cmd/ufetch.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/cmd/ufetch.c b/cmd/ufetch.c
> index 7aed0b447bda104b837d37c6adcbb21b80237aba..0ee50bfb606aa2223072c63cd46b3a70ef5449fd 100644
> --- a/cmd/ufetch.c
> +++ b/cmd/ufetch.c
> @@ -188,7 +188,7 @@ static int do_ufetch(struct cmd_tbl *cmdtp, int flag, int argc,
> if (ofnode_name_eq(np, "cpu"))
> n_cpus++;
> }
> - printf("CPU:" RESET " %d (1 in use)\n", n_cpus);
> + printf("CPU: " RESET CONFIG_SYS_ARCH ", %d (1 in use)\n", n_cpus);
This will read like
CPU: arm 4 (1 in use)
which is a bit hard to parse at a glance. How about
CPU: arm [4 cores]
or some other separator between SYS_ARCH and the core count?
Kind regards,
> break;
> case MEMORY:
> for (int j = 0; j < CONFIG_NR_DRAM_BANKS && gd->bd->bi_dram[j].size; j++)
>
--
// Caleb (they/them)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] cmd: Allow building ufetch without CONFIG_BLK
2024-12-09 15:04 ` Caleb Connolly
@ 2024-12-09 16:51 ` J. Neuschäfer
2024-12-09 16:55 ` Caleb Connolly
0 siblings, 1 reply; 14+ messages in thread
From: J. Neuschäfer @ 2024-12-09 16:51 UTC (permalink / raw)
To: Caleb Connolly; +Cc: j.ne, Tom Rini, u-boot
On Mon, Dec 09, 2024 at 04:04:50PM +0100, Caleb Connolly wrote:
> On 05/12/2024 19:35, J. Neuschäfer via B4 Relay wrote:
> > From: "J. Neuschäfer" <j.ne@posteo.net>
> >
> > The ufetch command is still quite useful on systems without block
> > device support; remove the CONFIG_BLK dependency and make sure the code
> > compiles/works with and without CONFIG_BLK.
> >
> > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
>
> Small nit below, but with that:
>
> Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org>
> > ---
[...]
> > default:
> > +#ifdef CONFIG_BLK
> > + struct udevice *dev;
>
> At least without the #ifdef, I see a warning "A label followed by a
> declaration is a C23 extension".
Ah, I wondered about this. Apparently I have a compiler that uses C23 as
the default, so I didn't see the warning. I'll fix it in the next revision.
> So to be on the safe side please put this section in a block:
>
> default: {
An alternative that I prefer because it's a bit less intrusive with
regards to indentation, is to use a semicolon:
default:;
/* code with the same indentation as usual */
/* and no closing brace to remember */
What do you think?
-- jn
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/3] cmd: Allow building ufetch without CONFIG_BLK
2024-12-09 16:51 ` J. Neuschäfer
@ 2024-12-09 16:55 ` Caleb Connolly
0 siblings, 0 replies; 14+ messages in thread
From: Caleb Connolly @ 2024-12-09 16:55 UTC (permalink / raw)
To: J. Neuschäfer; +Cc: Tom Rini, u-boot
On 09/12/2024 17:51, J. Neuschäfer wrote:
> On Mon, Dec 09, 2024 at 04:04:50PM +0100, Caleb Connolly wrote:
>> On 05/12/2024 19:35, J. Neuschäfer via B4 Relay wrote:
>>> From: "J. Neuschäfer" <j.ne@posteo.net>
>>>
>>> The ufetch command is still quite useful on systems without block
>>> device support; remove the CONFIG_BLK dependency and make sure the code
>>> compiles/works with and without CONFIG_BLK.
>>>
>>> Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
>>
>> Small nit below, but with that:
>>
>> Reviewed-by: Caleb Connolly <caleb.connolly@linaro.org>
>>> ---
> [...]
>>> default:
>>> +#ifdef CONFIG_BLK
>>> + struct udevice *dev;
>>
>> At least without the #ifdef, I see a warning "A label followed by a
>> declaration is a C23 extension".
>
> Ah, I wondered about this. Apparently I have a compiler that uses C23 as
> the default, so I didn't see the warning. I'll fix it in the next revision.
>
>> So to be on the safe side please put this section in a block:
>>
>> default: {
>
> An alternative that I prefer because it's a bit less intrusive with
> regards to indentation, is to use a semicolon:
>
> default:;
> /* code with the same indentation as usual */
> /* and no closing brace to remember */
Hmm, haven't seen that one before. fwiw I think you can leave the
indentation as-is and just add the brackets.
Kind regards,
>
> What do you think?
>
>
> -- jn
--
// Caleb (they/them)
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] cmd: ufetch: Show CPU architecture under "CPU"
2024-12-09 15:09 ` Caleb Connolly
@ 2024-12-09 16:58 ` J. Neuschäfer
0 siblings, 0 replies; 14+ messages in thread
From: J. Neuschäfer @ 2024-12-09 16:58 UTC (permalink / raw)
To: Caleb Connolly; +Cc: j.ne, Tom Rini, u-boot
On Mon, Dec 09, 2024 at 04:09:30PM +0100, Caleb Connolly wrote:
> Thanks for the patch!
>
> On 05/12/2024 19:35, J. Neuschäfer via B4 Relay wrote:
> > From: "J. Neuschäfer" <j.ne@posteo.net>
> >
> > When looking at ufetch output it isn't immediately obvious which CPU
> > architecture the presented board has. This patch therefore adds the CPU
> > architecture string (for example "powerpc") to the "CPU:" line.
>
> It would be nice to have the proper fancy name, but this is an
> improvement nonetheless.
True
> >
> > Signed-off-by: J. Neuschäfer <j.ne@posteo.net>
> > ---
[...]
> > - printf("CPU:" RESET " %d (1 in use)\n", n_cpus);
> > + printf("CPU: " RESET CONFIG_SYS_ARCH ", %d (1 in use)\n", n_cpus);
>
> This will read like
>
> CPU: arm 4 (1 in use)
>
> which is a bit hard to parse at a glance. How about
>
> CPU: arm [4 cores]
>
> or some other separator between SYS_ARCH and the core count?
Ah yup, good idea. I also struggled with reading this line as it was;
"4 cores" or similar makes it clear what the number means.
-- jn
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2024-12-09 18:13 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-12-05 18:35 [PATCH 0/3] cmd: ufetch improvements J. Neuschäfer
2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
2024-12-05 18:35 ` [PATCH 1/3] cmd: ufetch: Fix type mismatch on 32-bit J. Neuschäfer
2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
2024-12-09 14:58 ` Caleb Connolly
2024-12-05 18:35 ` [PATCH 2/3] cmd: Allow building ufetch without CONFIG_BLK J. Neuschäfer
2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
2024-12-09 15:04 ` Caleb Connolly
2024-12-09 16:51 ` J. Neuschäfer
2024-12-09 16:55 ` Caleb Connolly
2024-12-05 18:35 ` [PATCH 3/3] cmd: ufetch: Show CPU architecture under "CPU" J. Neuschäfer
2024-12-05 18:35 ` J. Neuschäfer via B4 Relay
2024-12-09 15:09 ` Caleb Connolly
2024-12-09 16:58 ` J. Neuschäfer
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.