[PATCH v1 0/4] Introduce the sysinfo command
The command can be used to show various information that can be used to identify the running system.
Currently supported subcommands are: * model: A string representing the model * id: The id of the board * revision: The revision of this board id.
I'm not sure about the last commit, if it needs to be here upstream or if it chould be something done by packagers.
I'm also not sure if a test should be added and where to start to test this kind of command.
Detlev Casanova (4): sysinfo: Add IDs for board id and revision cmd: Add a sysinfo command sysinfo: rcar3: Implement BOARD_ID and BOARD_REVISION configs: rcar3: Add shell function to select the linux devicetree
cmd/Kconfig | 6 ++ cmd/Makefile | 1 + cmd/sysinfo.c | 124 +++++++++++++++++++++++++++++ configs/rcar3_ulcb_defconfig | 1 + drivers/sysinfo/rcar3.c | 46 ++++++++--- include/configs/rcar-gen3-common.h | 9 ++- include/sysinfo.h | 4 + 7 files changed, 180 insertions(+), 11 deletions(-) create mode 100644 cmd/sysinfo.c
These IDs will be used by the sysinfo command. The new IDs are: * SYSINFO_ID_BOARD_ID: The board ID as an integer * SYSINFO_ID_BOARD_REVISION: The board revision as a string
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com --- include/sysinfo.h | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/include/sysinfo.h b/include/sysinfo.h index b140d742e93..e2a7b10b6f6 100644 --- a/include/sysinfo.h +++ b/include/sysinfo.h @@ -47,6 +47,10 @@ enum sysinfo_id { /* For show_board_info() */ SYSINFO_ID_BOARD_MODEL,
+ /* For sysinfo command */ + SYSINFO_ID_BOARD_ID, + SYSINFO_ID_BOARD_REVISION, + /* First value available for downstream/board used */ SYSINFO_ID_USER = 0x1000, };
The command is able to show different information for the running system: * Model name * Board ID * Revision
This command can be used by boot shell scripts to select configurations depending on the specific running system.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com --- cmd/Kconfig | 6 +++ cmd/Makefile | 1 + cmd/sysinfo.c | 124 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 131 insertions(+) create mode 100644 cmd/sysinfo.c
diff --git a/cmd/Kconfig b/cmd/Kconfig index 365371fb511..ba4844853a1 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -210,6 +210,12 @@ config CMD_SBI help Display information about the SBI implementation.
+config CMD_SYSINFO + bool "sysinfo" + depends on SYSINFO + help + Display information about the system. + endmenu
menu "Boot commands" diff --git a/cmd/Makefile b/cmd/Makefile index 6c37521b4e2..ba4d6de9a1b 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -165,6 +165,7 @@ obj-$(CONFIG_CMD_SPI) += spi.o obj-$(CONFIG_CMD_STRINGS) += strings.o obj-$(CONFIG_CMD_SMC) += smccc.o obj-$(CONFIG_CMD_SYSBOOT) += sysboot.o +obj-$(CONFIG_CMD_SYSINFO) += sysinfo.o obj-$(CONFIG_CMD_STACKPROTECTOR_TEST) += stackprot_test.o obj-$(CONFIG_CMD_TEMPERATURE) += temperature.o obj-$(CONFIG_CMD_TERMINAL) += terminal.o diff --git a/cmd/sysinfo.c b/cmd/sysinfo.c new file mode 100644 index 00000000000..31cb27ae46e --- /dev/null +++ b/cmd/sysinfo.c @@ -0,0 +1,124 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2023 + * Detlev Casanova detlev.casanova@collabora.com + */ + +#include <command.h> +#include <env.h> +#include <sysinfo.h> +#include <vsprintf.h> + +static int get_sysinfo(struct udevice **dev) +{ + int ret = sysinfo_get(dev); + + if (ret) { + debug("Cannot get sysinfo: %d\n", ret); + return ret; + } + + ret = sysinfo_detect(*dev); + if (ret) { + debug("Cannot detect sysinfo: %d\n", ret); + return ret; + } + + return 0; +} + +static int do_sysinfo_model(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct udevice *dev; + char model[64]; + int ret = get_sysinfo(&dev); + + if (ret) + return ret; + + ret = sysinfo_get_str(dev, + SYSINFO_ID_BOARD_MODEL, + 64, + model); + + if (ret) { + debug("Cannot get sysinfo str: %d\n", ret); + return ret; + } + + if (argc == 2) + env_set(argv[1], model); + else + printf("%s\n", model); + + return 0; +} + +static int do_sysinfo_id(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct udevice *dev; + u32 board_id; + char board_id_str[5] = { '\0' }; + int ret = get_sysinfo(&dev); + + if (ret) + return ret; + + ret = sysinfo_get_int(dev, + SYSINFO_ID_BOARD_ID, + &board_id); + + if (ret) { + debug("Cannot get sysinfo int: %d\n", ret); + return ret; + } + + sprintf(board_id_str, "0x%02x", board_id); + if (argc == 2) + env_set(argv[1], board_id_str); + else + printf("%s\n", board_id_str); + + return 0; +} + +static int do_sysinfo_revision(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct udevice *dev; + char rev[4]; + int ret = get_sysinfo(&dev); + + if (ret) + return ret; + + ret = sysinfo_get_str(dev, + SYSINFO_ID_BOARD_REVISION, + 4, + rev); + + if (ret) { + debug("Cannot get sysinfo str: %d\n", ret); + return ret; + } + + if (argc == 2) + env_set(argv[1], rev); + else + printf("%s\n", rev); + + return 0; +} + +static char sysinfo_help_text[] = + "model <varname> - Show or set the board model in varname\n" + "sysinfo id <varname> - Show or set the board id in varname (in format 0xHH)\n" + "sysinfo revision <varname> - Show or set the board revision in varname"; + +U_BOOT_CMD_WITH_SUBCMDS(sysinfo, "System information", sysinfo_help_text, + U_BOOT_SUBCMD_MKENT(model, 2, 1, do_sysinfo_model), + U_BOOT_SUBCMD_MKENT(id, 2, 1, do_sysinfo_id), + U_BOOT_SUBCMD_MKENT(revision, 2, 1, do_sysinfo_revision), +);
On 6/16/23 17:21, Detlev Casanova wrote:
[...]
+static int do_sysinfo_id(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])+{
- struct udevice *dev;
- u32 board_id;
- char board_id_str[5] = { '\0' };
- int ret = get_sysinfo(&dev);
- if (ret)
return ret;- ret = sysinfo_get_int(dev,
SYSINFO_ID_BOARD_ID,&board_id);- if (ret) {
debug("Cannot get sysinfo int: %d\n", ret);return ret;- }
- sprintf(board_id_str, "0x%02x", board_id);
- if (argc == 2)
env_set(argv[1], board_id_str);
env_set_hex()
- else
printf("%s\n", board_id_str);
printf(...%02x...
- return 0;
+}
+static int do_sysinfo_revision(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])+{
- struct udevice *dev;
- char rev[4];
- int ret = get_sysinfo(&dev);
- if (ret)
return ret;- ret = sysinfo_get_str(dev,
SYSINFO_ID_BOARD_REVISION,4,rev);- if (ret) {
debug("Cannot get sysinfo str: %d\n", ret);return ret;
Commands always return CMD_RET_* , not errno .
Expose that information to the command shell to let scripts select the correct devicetree name.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com --- drivers/sysinfo/rcar3.c | 46 ++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-)
diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c index 7b127986da7..89ad46c5422 100644 --- a/drivers/sysinfo/rcar3.c +++ b/drivers/sysinfo/rcar3.c @@ -32,6 +32,8 @@ */ struct sysinfo_rcar_priv { char boardmodel[64]; + u8 id; + char revision[4]; u8 val; };
@@ -48,17 +50,37 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, size_t size, char *
switch (id) { case SYSINFO_ID_BOARD_MODEL: - strncpy(val, priv->boardmodel, size); - val[size - 1] = '\0'; + strlcpy(val, priv->boardmodel, size); + break; + case SYSINFO_ID_BOARD_REVISION: + strlcpy(val, priv->revision, size); + break; + default: + return -EINVAL; + }; + + val[size - 1] = '\0'; + return 0; +} + +static int sysinfo_rcar_get_int(struct udevice *dev, int id, int *val) +{ + struct sysinfo_rcar_priv *priv = dev_get_priv(dev); + + switch (id) { + case SYSINFO_ID_BOARD_ID: + *val = priv->id; return 0; default: return -EINVAL; }; + }
static const struct sysinfo_ops sysinfo_rcar_ops = { .detect = sysinfo_rcar_detect, .get_str = sysinfo_rcar_get_str, + .get_int = sysinfo_rcar_get_int, };
static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) @@ -83,7 +105,7 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Salvator-X%s board rev %c.%c", salvator_xs ? "S" : "", rev_major, rev_minor); - return; + break; case BOARD_STARTER_KIT: if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */ rev_major = (board_rev & BIT(0)) ? '3' : '1'; @@ -92,7 +114,7 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Starter Kit board rev %c.%c", rev_major, rev_minor); - return; + break; case BOARD_STARTER_KIT_PRE: if (!(board_rev & ~3)) { /* Only rev 0..3 is valid */ rev_major = (board_rev & BIT(1)) ? '2' : '1'; @@ -101,7 +123,7 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Starter Kit Premier board rev %c.%c", rev_major, rev_minor); - return; + break; case BOARD_EAGLE: if (!board_rev) { /* Only rev 0 is valid */ rev_major = '1'; @@ -110,7 +132,7 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Eagle board rev %c.%c", rev_major, rev_minor); - return; + break; case BOARD_EBISU_4D: ebisu_4d = true; fallthrough; @@ -122,7 +144,7 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Ebisu%s board rev %c.%c", ebisu_4d ? "-4D" : "", rev_major, rev_minor); - return; + break; case BOARD_DRAAK: if (!board_rev) { /* Only rev 0 is valid */ rev_major = '1'; @@ -131,7 +153,7 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Draak board rev %c.%c", rev_major, rev_minor); - return; + break; case BOARD_KRIEK: if (!board_rev) { /* Only rev 0 is valid */ rev_major = '1'; @@ -140,7 +162,7 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Kriek board rev %c.%c", rev_major, rev_minor); - return; + break; case BOARD_CONDOR_I: condor_i = true; fallthrough; @@ -152,13 +174,17 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Condor%s board rev %c.%c", condor_i ? "-I" : "", rev_major, rev_minor); - return; + break; default: snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas -Unknown- board rev ?.?"); priv->val = 0xff; return; } + + snprintf(priv->revision, sizeof(priv->revision), "%c.%c", rev_major, + rev_minor); + priv->id = board_id; }
static int sysinfo_rcar_probe(struct udevice *dev)
On 6/16/23 17:21, Detlev Casanova wrote:
Expose that information to the command shell to let scripts select the correct devicetree name.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
drivers/sysinfo/rcar3.c | 46 ++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-)
diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c index 7b127986da7..89ad46c5422 100644 --- a/drivers/sysinfo/rcar3.c +++ b/drivers/sysinfo/rcar3.c @@ -32,6 +32,8 @@ */ struct sysinfo_rcar_priv { char boardmodel[64];
- u8 id;
- char revision[4]; u8 val; };
@@ -48,17 +50,37 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, size_t size, char *
switch (id) { case SYSINFO_ID_BOARD_MODEL:
strncpy(val, priv->boardmodel, size);val[size - 1] = '\0';
strlcpy(val, priv->boardmodel, size);break;- case SYSINFO_ID_BOARD_REVISION:
strlcpy(val, priv->revision, size);break;- default:
return -EINVAL;- };
- val[size - 1] = '\0';
- return 0;
+}
+static int sysinfo_rcar_get_int(struct udevice *dev, int id, int *val) +{
- struct sysinfo_rcar_priv *priv = dev_get_priv(dev);
- switch (id) {
- case SYSINFO_ID_BOARD_ID:
*val = priv->id;
Why not return SYSINFO_ID_BOARD_REVISION as integer here ?
[...]
On Friday, June 16, 2023 8:43:33 P.M. EDT Marek Vasut wrote:
On 6/16/23 17:21, Detlev Casanova wrote:
Expose that information to the command shell to let scripts select the correct devicetree name.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
drivers/sysinfo/rcar3.c | 46 ++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-)
diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c index 7b127986da7..89ad46c5422 100644 --- a/drivers/sysinfo/rcar3.c +++ b/drivers/sysinfo/rcar3.c @@ -32,6 +32,8 @@
*/
struct sysinfo_rcar_priv {
char boardmodel[64];
u8 id;
char revision[4];
u8 val;
};
@@ -48,17 +50,37 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, size_t size, char *> switch (id) {
case SYSINFO_ID_BOARD_MODEL:
strncpy(val, priv->boardmodel, size);val[size - 1] = '\0';
strlcpy(val, priv->boardmodel, size);break;- case SYSINFO_ID_BOARD_REVISION:
strlcpy(val, priv->revision, size);break;- default:
return -EINVAL;- };
- val[size - 1] = '\0';
- return 0;
+}
+static int sysinfo_rcar_get_int(struct udevice *dev, int id, int *val) +{
struct sysinfo_rcar_priv *priv = dev_get_priv(dev);
switch (id) {
case SYSINFO_ID_BOARD_ID:
*val = priv->id;return 0;
Why not return SYSINFO_ID_BOARD_REVISION as integer here ?
Because the revision (on r-car3 boards at least) is in the format X.Y. It could be returned as "(X << 8) | Y" or split in major/minor. But different boards will use different revisions and I think that having a str is easier to deal with in a shell script.
[...]
On 6/19/23 16:42, Detlev Casanova wrote:
On Friday, June 16, 2023 8:43:33 P.M. EDT Marek Vasut wrote:
On 6/16/23 17:21, Detlev Casanova wrote:
Expose that information to the command shell to let scripts select the correct devicetree name.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
drivers/sysinfo/rcar3.c | 46 ++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-)
diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c index 7b127986da7..89ad46c5422 100644 --- a/drivers/sysinfo/rcar3.c +++ b/drivers/sysinfo/rcar3.c @@ -32,6 +32,8 @@
*/struct sysinfo_rcar_priv {
char boardmodel[64];
u8 id;
char revision[4];
u8 val;
};
@@ -48,17 +50,37 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, size_t size, char *> switch (id) {
case SYSINFO_ID_BOARD_MODEL:
strncpy(val, priv->boardmodel, size);val[size - 1] = '\0';
strlcpy(val, priv->boardmodel, size);break;- case SYSINFO_ID_BOARD_REVISION:
strlcpy(val, priv->revision, size);break;- default:
return -EINVAL;- };
- val[size - 1] = '\0';
- return 0;
+}
+static int sysinfo_rcar_get_int(struct udevice *dev, int id, int *val) +{
struct sysinfo_rcar_priv *priv = dev_get_priv(dev);
switch (id) {
case SYSINFO_ID_BOARD_ID:
*val = priv->id; return 0;Why not return SYSINFO_ID_BOARD_REVISION as integer here ?
Because the revision (on r-car3 boards at least) is in the format X.Y. It could be returned as "(X << 8) | Y" or split in major/minor. But different boards will use different revisions and I think that having a str is easier to deal with in a shell script.
With rcar they are numbers, so lets go with major/minor integers please.
On Monday, June 19, 2023 12:11:18 P.M. EDT Marek Vasut wrote:
On 6/19/23 16:42, Detlev Casanova wrote:
On Friday, June 16, 2023 8:43:33 P.M. EDT Marek Vasut wrote:
On 6/16/23 17:21, Detlev Casanova wrote:
Expose that information to the command shell to let scripts select the correct devicetree name.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
drivers/sysinfo/rcar3.c | 46 ++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-)
diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c index 7b127986da7..89ad46c5422 100644 --- a/drivers/sysinfo/rcar3.c +++ b/drivers/sysinfo/rcar3.c @@ -32,6 +32,8 @@
*/struct sysinfo_rcar_priv {
char boardmodel[64];
u8 id;
char revision[4];
u8 val;
};
@@ -48,17 +50,37 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, size_t size, char *>
switch (id) { case SYSINFO_ID_BOARD_MODEL:
strncpy(val, priv->boardmodel, size);val[size - 1] = '\0';
strlcpy(val, priv->boardmodel, size);break;- case SYSINFO_ID_BOARD_REVISION:
strlcpy(val, priv->revision, size);break;- default:
return -EINVAL;- };
- val[size - 1] = '\0';
- return 0;
+}
+static int sysinfo_rcar_get_int(struct udevice *dev, int id, int *val) +{
struct sysinfo_rcar_priv *priv = dev_get_priv(dev);
switch (id) {
case SYSINFO_ID_BOARD_ID:
*val = priv->id; return 0;Why not return SYSINFO_ID_BOARD_REVISION as integer here ?
Because the revision (on r-car3 boards at least) is in the format X.Y. It could be returned as "(X << 8) | Y" or split in major/minor. But different boards will use different revisions and I think that having a str is easier to deal with in a shell script.
With rcar they are numbers, so lets go with major/minor integers please.
Ok for this part, but shouldn't the sysinfo command use a common interface for all boards ? Or should it also have rev_major/rev_minor arguments ?
On 6/19/23 20:27, Detlev Casanova wrote:
On Monday, June 19, 2023 12:11:18 P.M. EDT Marek Vasut wrote:
On 6/19/23 16:42, Detlev Casanova wrote:
On Friday, June 16, 2023 8:43:33 P.M. EDT Marek Vasut wrote:
On 6/16/23 17:21, Detlev Casanova wrote:
Expose that information to the command shell to let scripts select the correct devicetree name.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
drivers/sysinfo/rcar3.c | 46 ++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-)diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c index 7b127986da7..89ad46c5422 100644 --- a/drivers/sysinfo/rcar3.c +++ b/drivers/sysinfo/rcar3.c @@ -32,6 +32,8 @@
*/ struct sysinfo_rcar_priv { char boardmodel[64];
u8 id;
char revision[4];
u8 val;};
@@ -48,17 +50,37 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, size_t size, char *>
switch (id) { case SYSINFO_ID_BOARD_MODEL:
strncpy(val, priv->boardmodel, size);val[size - 1] = '\0';
strlcpy(val, priv->boardmodel, size);break;- case SYSINFO_ID_BOARD_REVISION:
strlcpy(val, priv->revision, size);break;- default:
return -EINVAL;- };
- val[size - 1] = '\0';
- return 0;
+}
+static int sysinfo_rcar_get_int(struct udevice *dev, int id, int *val) +{
struct sysinfo_rcar_priv *priv = dev_get_priv(dev);
switch (id) {
case SYSINFO_ID_BOARD_ID:
*val = priv->id; return 0;Why not return SYSINFO_ID_BOARD_REVISION as integer here ?
Because the revision (on r-car3 boards at least) is in the format X.Y. It could be returned as "(X << 8) | Y" or split in major/minor. But different boards will use different revisions and I think that having a str is easier to deal with in a shell script.
With rcar they are numbers, so lets go with major/minor integers please.
Ok for this part, but shouldn't the sysinfo command use a common interface for all boards ? Or should it also have rev_major/rev_minor arguments ?
I would expect other boards to either report rev_major/rev_minor if implemented, or errno if those boards don't implement this property.
On Monday, June 19, 2023 5:54:44 P.M. EDT Marek Vasut wrote:
On 6/19/23 20:27, Detlev Casanova wrote:
On Monday, June 19, 2023 12:11:18 P.M. EDT Marek Vasut wrote:
On 6/19/23 16:42, Detlev Casanova wrote:
On Friday, June 16, 2023 8:43:33 P.M. EDT Marek Vasut wrote:
On 6/16/23 17:21, Detlev Casanova wrote:
Expose that information to the command shell to let scripts select the correct devicetree name.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
drivers/sysinfo/rcar3.c | 46 ++++++++++++++++++++++++++++++++--------- 1 file changed, 36 insertions(+), 10 deletions(-)diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c index 7b127986da7..89ad46c5422 100644 --- a/drivers/sysinfo/rcar3.c +++ b/drivers/sysinfo/rcar3.c @@ -32,6 +32,8 @@
*/ struct sysinfo_rcar_priv { char boardmodel[64];
u8 id;
char revision[4];
u8 val;};
@@ -48,17 +50,37 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, size_t size, char *>
switch (id) { case SYSINFO_ID_BOARD_MODEL:
strncpy(val, priv->boardmodel, size);val[size - 1] = '\0';
strlcpy(val, priv->boardmodel, size);break;- case SYSINFO_ID_BOARD_REVISION:
strlcpy(val, priv->revision, size);break;- default:
return -EINVAL;- };
- val[size - 1] = '\0';
- return 0;
+}
+static int sysinfo_rcar_get_int(struct udevice *dev, int id, int *val) +{
struct sysinfo_rcar_priv *priv = dev_get_priv(dev);
switch (id) {
case SYSINFO_ID_BOARD_ID:
*val = priv->id; return 0;Why not return SYSINFO_ID_BOARD_REVISION as integer here ?
Because the revision (on r-car3 boards at least) is in the format X.Y. It could be returned as "(X << 8) | Y" or split in major/minor. But different boards will use different revisions and I think that having a str is easier to deal with in a shell script.
With rcar they are numbers, so lets go with major/minor integers please.
Ok for this part, but shouldn't the sysinfo command use a common interface for all boards ? Or should it also have rev_major/rev_minor arguments ?
I would expect other boards to either report rev_major/rev_minor if implemented, or errno if those boards don't implement this property.
Another thing on rcar is that the revision is stored as 2 char values. Would you oppose a change form using a char (e.g. rev_major = '1') to using u8 values (e.g. rev_major = 1) instead ?
On 6/20/23 19:49, Detlev Casanova wrote:
On Monday, June 19, 2023 5:54:44 P.M. EDT Marek Vasut wrote:
On 6/19/23 20:27, Detlev Casanova wrote:
On Monday, June 19, 2023 12:11:18 P.M. EDT Marek Vasut wrote:
On 6/19/23 16:42, Detlev Casanova wrote:
On Friday, June 16, 2023 8:43:33 P.M. EDT Marek Vasut wrote:
On 6/16/23 17:21, Detlev Casanova wrote: > Expose that information to the command shell to let scripts select the > correct devicetree name. > > Signed-off-by: Detlev Casanova detlev.casanova@collabora.com > --- > > drivers/sysinfo/rcar3.c | 46 > ++++++++++++++++++++++++++++++++--------- > 1 file changed, 36 insertions(+), 10 deletions(-) > > diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c > index 7b127986da7..89ad46c5422 100644 > --- a/drivers/sysinfo/rcar3.c > +++ b/drivers/sysinfo/rcar3.c > @@ -32,6 +32,8 @@ > > */ > > struct sysinfo_rcar_priv { > > char boardmodel[64]; > > + u8 id; > + char revision[4]; > > u8 val; > > }; > > @@ -48,17 +50,37 @@ static int sysinfo_rcar_get_str(struct udevice > *dev, > int id, size_t size, char *> > > switch (id) { > > case SYSINFO_ID_BOARD_MODEL: > - strncpy(val, priv->boardmodel, size); > - val[size - 1] = '\0'; > + strlcpy(val, priv->boardmodel, size); > + break; > + case SYSINFO_ID_BOARD_REVISION: > + strlcpy(val, priv->revision, size); > + break; > + default: > + return -EINVAL; > + }; > + > + val[size - 1] = '\0'; > + return 0; > +} > + > +static int sysinfo_rcar_get_int(struct udevice *dev, int id, int > *val) > +{ > + struct sysinfo_rcar_priv *priv = dev_get_priv(dev); > + > + switch (id) { > + case SYSINFO_ID_BOARD_ID: > + *val = priv->id; > > return 0;
Why not return SYSINFO_ID_BOARD_REVISION as integer here ?
Because the revision (on r-car3 boards at least) is in the format X.Y. It could be returned as "(X << 8) | Y" or split in major/minor. But different boards will use different revisions and I think that having a str is easier to deal with in a shell script.
With rcar they are numbers, so lets go with major/minor integers please.
Ok for this part, but shouldn't the sysinfo command use a common interface for all boards ? Or should it also have rev_major/rev_minor arguments ?
I would expect other boards to either report rev_major/rev_minor if implemented, or errno if those boards don't implement this property.
Another thing on rcar is that the revision is stored as 2 char values. Would you oppose a change form using a char (e.g. rev_major = '1') to using u8 values (e.g. rev_major = 1) instead ?
Shouldn't those rev fields just be integer(s) to cover the generic case?
On Tuesday, June 20, 2023 4:03:18 P.M. EDT Marek Vasut wrote:
On 6/20/23 19:49, Detlev Casanova wrote:
On Monday, June 19, 2023 5:54:44 P.M. EDT Marek Vasut wrote:
On 6/19/23 20:27, Detlev Casanova wrote:
On Monday, June 19, 2023 12:11:18 P.M. EDT Marek Vasut wrote:
On 6/19/23 16:42, Detlev Casanova wrote:
On Friday, June 16, 2023 8:43:33 P.M. EDT Marek Vasut wrote: > On 6/16/23 17:21, Detlev Casanova wrote: >> Expose that information to the command shell to let scripts select >> the >> correct devicetree name. >> >> Signed-off-by: Detlev Casanova detlev.casanova@collabora.com >> --- >> >> drivers/sysinfo/rcar3.c | 46 >> ++++++++++++++++++++++++++++++++--------- >> 1 file changed, 36 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c >> index 7b127986da7..89ad46c5422 100644 >> --- a/drivers/sysinfo/rcar3.c >> +++ b/drivers/sysinfo/rcar3.c >> @@ -32,6 +32,8 @@ >> >> */ >> >> struct sysinfo_rcar_priv { >> >> char boardmodel[64]; >> >> + u8 id; >> + char revision[4]; >> >> u8 val; >> >> }; >> >> @@ -48,17 +50,37 @@ static int sysinfo_rcar_get_str(struct udevice >> *dev, >> int id, size_t size, char *> >> >> switch (id) { >> >> case SYSINFO_ID_BOARD_MODEL: >> - strncpy(val, priv->boardmodel, size); >> - val[size - 1] = '\0'; >> + strlcpy(val, priv->boardmodel, size); >> + break; >> + case SYSINFO_ID_BOARD_REVISION: >> + strlcpy(val, priv->revision, size); >> + break; >> + default: >> + return -EINVAL; >> + }; >> + >> + val[size - 1] = '\0'; >> + return 0; >> +} >> + >> +static int sysinfo_rcar_get_int(struct udevice *dev, int id, int >> *val) >> +{ >> + struct sysinfo_rcar_priv *priv = dev_get_priv(dev); >> + >> + switch (id) { >> + case SYSINFO_ID_BOARD_ID: >> + *val = priv->id; >> >> return 0; > > Why not return SYSINFO_ID_BOARD_REVISION as integer here ?
Because the revision (on r-car3 boards at least) is in the format X.Y. It could be returned as "(X << 8) | Y" or split in major/minor. But different boards will use different revisions and I think that having a str is easier to deal with in a shell script.
With rcar they are numbers, so lets go with major/minor integers please.
Ok for this part, but shouldn't the sysinfo command use a common interface for all boards ? Or should it also have rev_major/rev_minor arguments ?
I would expect other boards to either report rev_major/rev_minor if implemented, or errno if those boards don't implement this property.
Another thing on rcar is that the revision is stored as 2 char values. Would you oppose a change form using a char (e.g. rev_major = '1') to using u8 values (e.g. rev_major = 1) instead ?
Shouldn't those rev fields just be integer(s) to cover the generic case?
On rcar, they are chars. I don't really see a reason for this except to show the '?.?' on unknown board ids. But that can be managed in other ways.
On 6/20/23 22:40, Detlev Casanova wrote:
On Tuesday, June 20, 2023 4:03:18 P.M. EDT Marek Vasut wrote:
On 6/20/23 19:49, Detlev Casanova wrote:
On Monday, June 19, 2023 5:54:44 P.M. EDT Marek Vasut wrote:
On 6/19/23 20:27, Detlev Casanova wrote:
On Monday, June 19, 2023 12:11:18 P.M. EDT Marek Vasut wrote:
On 6/19/23 16:42, Detlev Casanova wrote: > On Friday, June 16, 2023 8:43:33 P.M. EDT Marek Vasut wrote: >> On 6/16/23 17:21, Detlev Casanova wrote: >>> Expose that information to the command shell to let scripts select >>> the >>> correct devicetree name. >>> >>> Signed-off-by: Detlev Casanova detlev.casanova@collabora.com >>> --- >>> >>> drivers/sysinfo/rcar3.c | 46 >>> ++++++++++++++++++++++++++++++++--------- >>> 1 file changed, 36 insertions(+), 10 deletions(-) >>> >>> diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c >>> index 7b127986da7..89ad46c5422 100644 >>> --- a/drivers/sysinfo/rcar3.c >>> +++ b/drivers/sysinfo/rcar3.c >>> @@ -32,6 +32,8 @@ >>> >>> */ >>> >>> struct sysinfo_rcar_priv { >>> >>> char boardmodel[64]; >>> >>> + u8 id; >>> + char revision[4]; >>> >>> u8 val; >>> >>> }; >>> >>> @@ -48,17 +50,37 @@ static int sysinfo_rcar_get_str(struct udevice >>> *dev, >>> int id, size_t size, char *> >>> >>> switch (id) { >>> >>> case SYSINFO_ID_BOARD_MODEL: >>> - strncpy(val, priv->boardmodel, size); >>> - val[size - 1] = '\0'; >>> + strlcpy(val, priv->boardmodel, size); >>> + break; >>> + case SYSINFO_ID_BOARD_REVISION: >>> + strlcpy(val, priv->revision, size); >>> + break; >>> + default: >>> + return -EINVAL; >>> + }; >>> + >>> + val[size - 1] = '\0'; >>> + return 0; >>> +} >>> + >>> +static int sysinfo_rcar_get_int(struct udevice *dev, int id, int >>> *val) >>> +{ >>> + struct sysinfo_rcar_priv *priv = dev_get_priv(dev); >>> + >>> + switch (id) { >>> + case SYSINFO_ID_BOARD_ID: >>> + *val = priv->id; >>> >>> return 0; >> >> Why not return SYSINFO_ID_BOARD_REVISION as integer here ? > > Because the revision (on r-car3 boards at least) is in the format X.Y. > It > could be returned as "(X << 8) | Y" or split in major/minor. But > different > boards will use different revisions and I think that having a str is > easier to deal with in a shell script.
With rcar they are numbers, so lets go with major/minor integers please.
Ok for this part, but shouldn't the sysinfo command use a common interface for all boards ? Or should it also have rev_major/rev_minor arguments ?
I would expect other boards to either report rev_major/rev_minor if implemented, or errno if those boards don't implement this property.
Another thing on rcar is that the revision is stored as 2 char values. Would you oppose a change form using a char (e.g. rev_major = '1') to using u8 values (e.g. rev_major = 1) instead ?
Shouldn't those rev fields just be integer(s) to cover the generic case?
On rcar, they are chars. I don't really see a reason for this except to show the '?.?' on unknown board ids. But that can be managed in other ways.
Yes I know, I am more concerned about other boards which might not have such short revision number, so why not just make the revision fields integer which covers very much anything ? Also, if you have those fields set to integer, then -EINVAL could be printed as '?' .
This function uses the sysinfo command to determine which linux device tree is selected for the running board.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com --- configs/rcar3_ulcb_defconfig | 1 + include/configs/rcar-gen3-common.h | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/configs/rcar3_ulcb_defconfig b/configs/rcar3_ulcb_defconfig index b8fdb5e3826..bb5f8f742ea 100644 --- a/configs/rcar3_ulcb_defconfig +++ b/configs/rcar3_ulcb_defconfig @@ -47,6 +47,7 @@ CONFIG_CMD_EXT4=y CONFIG_CMD_EXT4_WRITE=y CONFIG_CMD_FAT=y CONFIG_CMD_FS_GENERIC=y +CONFIG_CMD_SYSINFO=y CONFIG_OF_CONTROL=y CONFIG_OF_LIST="r8a77950-ulcb-u-boot r8a77960-ulcb-u-boot r8a77965-ulcb-u-boot" CONFIG_MULTI_DTB_FIT_LZO=y diff --git a/include/configs/rcar-gen3-common.h b/include/configs/rcar-gen3-common.h index 213caa75238..b278eb85a81 100644 --- a/include/configs/rcar-gen3-common.h +++ b/include/configs/rcar-gen3-common.h @@ -33,6 +33,13 @@ /* ENV setting */
#define CFG_EXTRA_ENV_SETTINGS \ - "bootm_size=0x10000000\0" + "bootm_size=0x10000000\0" \ + "set_board_fdt=sysinfo revision board_rev; " \ + "sysinfo id board_id; " \ + "if test ${board_rev} = 2.1 && test ${board_id} = 0x0b; then " \ + "setenv fdtfile renesas/r8a779m1-ulcb.dtb; " \ + "elif test ${board_rev} = 2.0 && test ${board_id} = 0x0b; then "\ + "setenv fdtfile renesas/r8a77951-ulcb.dtb; " \ + "fi\0" \
#endif /* __RCAR_GEN3_COMMON_H */
On 6/16/23 17:21, Detlev Casanova wrote:
This function uses the sysinfo command to determine which linux device tree is selected for the running board.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
configs/rcar3_ulcb_defconfig | 1 + include/configs/rcar-gen3-common.h | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/configs/rcar3_ulcb_defconfig b/configs/rcar3_ulcb_defconfig index b8fdb5e3826..bb5f8f742ea 100644 --- a/configs/rcar3_ulcb_defconfig +++ b/configs/rcar3_ulcb_defconfig @@ -47,6 +47,7 @@ CONFIG_CMD_EXT4=y CONFIG_CMD_EXT4_WRITE=y CONFIG_CMD_FAT=y CONFIG_CMD_FS_GENERIC=y +CONFIG_CMD_SYSINFO=y CONFIG_OF_CONTROL=y CONFIG_OF_LIST="r8a77950-ulcb-u-boot r8a77960-ulcb-u-boot r8a77965-ulcb-u-boot" CONFIG_MULTI_DTB_FIT_LZO=y diff --git a/include/configs/rcar-gen3-common.h b/include/configs/rcar-gen3-common.h index 213caa75238..b278eb85a81 100644 --- a/include/configs/rcar-gen3-common.h +++ b/include/configs/rcar-gen3-common.h @@ -33,6 +33,13 @@ /* ENV setting */
#define CFG_EXTRA_ENV_SETTINGS \
- "bootm_size=0x10000000\0"
- "bootm_size=0x10000000\0" \
- "set_board_fdt=sysinfo revision board_rev; " \
- "sysinfo id board_id; " \
- "if test ${board_rev} = 2.1 && test ${board_id} = 0x0b; then " \
"setenv fdtfile renesas/r8a779m1-ulcb.dtb; " \- "elif test ${board_rev} = 2.0 && test ${board_id} = 0x0b; then "\
"setenv fdtfile renesas/r8a77951-ulcb.dtb; " \- "fi\0" \
You must also consider M3W and M3N ULCB, i.e. 77960/77965 ones.
You must also consider all the other boards which use this header file, salvator-xs comes to mind. That one also comes with M3W/M3N SoC. Also, all the other Gen3 boards use this header, E3 Ebise, V3* Condor/Eagle ...
On Friday, June 16, 2023 8:45:31 P.M. EDT Marek Vasut wrote:
On 6/16/23 17:21, Detlev Casanova wrote:
This function uses the sysinfo command to determine which linux device tree is selected for the running board.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
configs/rcar3_ulcb_defconfig | 1 + include/configs/rcar-gen3-common.h | 9 ++++++++- 2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/configs/rcar3_ulcb_defconfig b/configs/rcar3_ulcb_defconfig index b8fdb5e3826..bb5f8f742ea 100644 --- a/configs/rcar3_ulcb_defconfig +++ b/configs/rcar3_ulcb_defconfig @@ -47,6 +47,7 @@ CONFIG_CMD_EXT4=y
CONFIG_CMD_EXT4_WRITE=y CONFIG_CMD_FAT=y CONFIG_CMD_FS_GENERIC=y
+CONFIG_CMD_SYSINFO=y
CONFIG_OF_CONTROL=y CONFIG_OF_LIST="r8a77950-ulcb-u-boot r8a77960-ulcb-u-boot r8a77965-ulcb-u-boot" CONFIG_MULTI_DTB_FIT_LZO=y
diff --git a/include/configs/rcar-gen3-common.h b/include/configs/rcar-gen3-common.h index 213caa75238..b278eb85a81 100644 --- a/include/configs/rcar-gen3-common.h +++ b/include/configs/rcar-gen3-common.h @@ -33,6 +33,13 @@
/* ENV setting */
#define CFG_EXTRA_ENV_SETTINGS \
- "bootm_size=0x10000000\0"
- "bootm_size=0x10000000\0"
\
- "set_board_fdt=sysinfo revision board_rev; "
\
- "sysinfo id board_id; "
\
- "if test ${board_rev} = 2.1 && test ${board_id} = 0x0b; then "
\
"setenv fdtfile renesas/r8a779m1-ulcb.dtb; "
\
- "elif test ${board_rev} = 2.0 && test ${board_id} = 0x0b; then "\
"setenv fdtfile renesas/r8a77951-ulcb.dtb; "
\
- "fi\0"
\
You must also consider M3W and M3N ULCB, i.e. 77960/77965 ones.
You must also consider all the other boards which use this header file, salvator-xs comes to mind. That one also comes with M3W/M3N SoC. Also, all the other Gen3 boards use this header, E3 Ebise, V3* Condor/Eagle ...
Yeah, that's why I'm not sure this function should be here. I don't have the boards to check the revision values.
participants (2)
-
Detlev Casanova -
Marek Vasut