[PATCH v5 0/6] 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.
Changes since v4: - Use less snprintf calls in drivers/sysinfo/rcar3.c - Rebase on master Changes since v3: - Fix documentation typo. Changes since v2: - Fix code style. - Use printf() instead of debug(). - Clarify sysinfo new ids types (int). - Add a test for sysinfo command. - Add documentation for sysinfo command. Changes since v1: - Removed shell function to select linux device tree. This will be distributions job. - Break revision in rev_major and rev_minor in the sysinfo driver.
Detlev Casanova (6): sysinfo: Add IDs for board id and revision cmd: Add a sysinfo command sysinfo: Add a test sysinfo: Add documentation sysinfo: rcar3: Use int instead of char for revision sysinfo: rcar3: Implement BOARD_ID and BOARD_REV_*
cmd/Kconfig | 6 ++ cmd/Makefile | 1 + cmd/sysinfo.c | 133 ++++++++++++++++++++++++++++++++++++++ configs/sandbox_defconfig | 1 + doc/usage/cmd/sysinfo.rst | 56 ++++++++++++++++ drivers/sysinfo/rcar3.c | 110 +++++++++++++++++++++---------- drivers/sysinfo/sandbox.c | 17 +++++ include/sysinfo.h | 5 ++ test/cmd/Makefile | 1 + test/cmd/test_sysinfo.c | 51 +++++++++++++++ 10 files changed, 347 insertions(+), 34 deletions(-) create mode 100644 cmd/sysinfo.c create mode 100644 doc/usage/cmd/sysinfo.rst create mode 100644 test/cmd/test_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_REV_MAJOR: The board major revision as int * SYSINFO_ID_BOARD_REV_MINOR: The board minor revision as int
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org Signed-off-by: Detlev Casanova detlev.casanova@collabora.com --- include/sysinfo.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/sysinfo.h b/include/sysinfo.h index b140d742e93..13815600ae6 100644 --- a/include/sysinfo.h +++ b/include/sysinfo.h @@ -47,6 +47,11 @@ enum sysinfo_id { /* For show_board_info() */ SYSINFO_ID_BOARD_MODEL,
+ /* For sysinfo command (all int) */ + SYSINFO_ID_BOARD_ID, + SYSINFO_ID_BOARD_REV_MAJOR, + SYSINFO_ID_BOARD_REV_MINOR, + /* First value available for downstream/board used */ SYSINFO_ID_USER = 0x1000, };
On Mon, 2 Oct 2023 at 09:21, Detlev Casanova detlev.casanova@collabora.com wrote:
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_REV_MAJOR: The board major revision as int
- SYSINFO_ID_BOARD_REV_MINOR: The board minor revision as int
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
include/sysinfo.h | 5 +++++ 1 file changed, 5 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
On 10/2/23 17:20, Detlev Casanova wrote:
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_REV_MAJOR: The board major revision as int
- SYSINFO_ID_BOARD_REV_MINOR: The board minor revision as int
Integers will not work in the generic case. E.g. for the VisionFive 2 board we have seen board revisions:
1.2A, 1.2B, 1.3B
For the Wandboard:
B1, D1
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
include/sysinfo.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/sysinfo.h b/include/sysinfo.h index b140d742e93..13815600ae6 100644 --- a/include/sysinfo.h +++ b/include/sysinfo.h @@ -47,6 +47,11 @@ enum sysinfo_id { /* For show_board_info() */ SYSINFO_ID_BOARD_MODEL,
- /* For sysinfo command (all int) */
Please, provide a Sphinx style documentation for every value. Cf. https://www.kernel.org/doc/html/v5.3/doc-guide/kernel-doc.html#structure-uni...
Best regards
Heinrich
- SYSINFO_ID_BOARD_ID,
- SYSINFO_ID_BOARD_REV_MAJOR,
- SYSINFO_ID_BOARD_REV_MINOR,
- /* First value available for downstream/board used */ SYSINFO_ID_USER = 0x1000, };
On 10/16/23 19:37, Heinrich Schuchardt wrote:
On 10/2/23 17:20, Detlev Casanova wrote:
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_REV_MAJOR: The board major revision as int * SYSINFO_ID_BOARD_REV_MINOR: The board minor revision as int
Integers will not work in the generic case. E.g. for the VisionFive 2 board we have seen board revisions:
1.2A, 1.2B, 1.3B
For the Wandboard:
B1, D1
Dang, I literally had this in my upstreaming queue today, so, sigh. Dropped for now :-(
On Monday, October 16, 2023 1:37:40 P.M. EDT Heinrich Schuchardt wrote:
On 10/2/23 17:20, Detlev Casanova wrote:
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_REV_MAJOR: The board major revision as int
- SYSINFO_ID_BOARD_REV_MINOR: The board minor revision as int
Integers will not work in the generic case. E.g. for the VisionFive 2 board we have seen board revisions:
1.2A, 1.2B, 1.3B
For the Wandboard:
B1, D1
From this, I gather that the revision should just be a string value, as there
is no generic way to represent all variations of the revision "numbers".
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
include/sysinfo.h | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/include/sysinfo.h b/include/sysinfo.h index b140d742e93..13815600ae6 100644 --- a/include/sysinfo.h +++ b/include/sysinfo.h @@ -47,6 +47,11 @@ enum sysinfo_id {
/* For show_board_info() */ SYSINFO_ID_BOARD_MODEL,
- /* For sysinfo command (all int) */
Please, provide a Sphinx style documentation for every value. Cf. https://www.kernel.org/doc/html/v5.3/doc-guide/kernel-doc.html#structure-uni on-and-enumeration-documentation
Best regards
Heinrich
SYSINFO_ID_BOARD_ID,
SYSINFO_ID_BOARD_REV_MAJOR,
SYSINFO_ID_BOARD_REV_MINOR,
/* 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.
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org Signed-off-by: Detlev Casanova detlev.casanova@collabora.com --- cmd/Kconfig | 6 +++ cmd/Makefile | 1 + cmd/sysinfo.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+) create mode 100644 cmd/sysinfo.c
diff --git a/cmd/Kconfig b/cmd/Kconfig index 43ca10f69cc..67d019aa795 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -218,6 +218,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 9bebf321c39..972f3a4720e 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -167,6 +167,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..46369ff9ac7 --- /dev/null +++ b/cmd/sysinfo.c @@ -0,0 +1,133 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright 2023 + * Detlev Casanova detlev.casanova@collabora.com + */ + +#include <command.h> +#include <env.h> +#include <sysinfo.h> +#include <exports.h> + +static int get_sysinfo(struct udevice **devp) +{ + int ret = sysinfo_get(devp); + + if (ret) { + printf("Cannot get sysinfo: %d\n", ret); + return ret; + } + + ret = sysinfo_detect(*devp); + if (ret) { + printf("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 CMD_RET_FAILURE; + + ret = sysinfo_get_str(dev, + SYSINFO_ID_BOARD_MODEL, + sizeof(model), + model); + if (ret) { + printf("Cannot get sysinfo str: %d\n", ret); + return CMD_RET_FAILURE; + } + + if (argc == 2) + ret = env_set(argv[1], model); + else + printf("%s\n", model); + + if (ret) + return CMD_RET_FAILURE; + else + return CMD_RET_SUCCESS; +} + +static int do_sysinfo_id(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct udevice *dev; + u32 board_id; + int ret = get_sysinfo(&dev); + + if (ret) + return CMD_RET_FAILURE; + + ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_ID, &board_id); + if (ret) { + printf("Cannot get sysinfo int: %d\n", ret); + return CMD_RET_FAILURE; + } + + if (argc == 2) + ret = env_set_hex(argv[1], board_id); + else + printf("0x%02x\n", board_id); + + if (ret) + return CMD_RET_FAILURE; + else + return CMD_RET_SUCCESS; +} + +static int do_sysinfo_revision(struct cmd_tbl *cmdtp, int flag, int argc, + char *const argv[]) +{ + struct udevice *dev; + int rev_major; + int rev_minor; + char rev[64]; + int ret = get_sysinfo(&dev); + + if (ret) + return CMD_RET_FAILURE; + + ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_REV_MAJOR, &rev_major); + if (ret) { + printf("Cannot get sysinfo int: %d\n", ret); + return CMD_RET_FAILURE; + } + + ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_REV_MINOR, &rev_minor); + if (ret) { + printf("Cannot get sysinfo int: %d\n", ret); + return CMD_RET_FAILURE; + } + + snprintf(rev, sizeof(rev), "%d.%d", rev_major, rev_minor); + + if (argc == 2) + ret = env_set(argv[1], rev); + else + printf("%s\n", rev); + + if (ret) + return CMD_RET_FAILURE; + else + return CMD_RET_SUCCESS; +} + +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 Mon, 2 Oct 2023 at 09:21, Detlev Casanova detlev.casanova@collabora.com wrote:
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.
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
cmd/Kconfig | 6 +++ cmd/Makefile | 1 + cmd/sysinfo.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+) create mode 100644 cmd/sysinfo.c
Reviewed-by: Simon Glass sjg@chromium.org
Would prefer more help text in the Kconfig as this is pretty vague
diff --git a/cmd/Kconfig b/cmd/Kconfig index 43ca10f69cc..67d019aa795 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -218,6 +218,12 @@ config CMD_SBI help Display information about the SBI implementation.
+config CMD_SYSINFO
bool "sysinfo"depends on SYSINFOhelpDisplay information about the system.endmenu
menu "Boot commands" diff --git a/cmd/Makefile b/cmd/Makefile index 9bebf321c39..972f3a4720e 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -167,6 +167,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..46369ff9ac7 --- /dev/null +++ b/cmd/sysinfo.c @@ -0,0 +1,133 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2023
- Detlev Casanova detlev.casanova@collabora.com
- */
+#include <command.h> +#include <env.h> +#include <sysinfo.h> +#include <exports.h>
+static int get_sysinfo(struct udevice **devp) +{
int ret = sysinfo_get(devp);if (ret) {printf("Cannot get sysinfo: %d\n", ret);return ret;}ret = sysinfo_detect(*devp);if (ret) {printf("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 CMD_RET_FAILURE;ret = sysinfo_get_str(dev,SYSINFO_ID_BOARD_MODEL,sizeof(model),model);if (ret) {printf("Cannot get sysinfo str: %d\n", ret);return CMD_RET_FAILURE;}if (argc == 2)ret = env_set(argv[1], model);elseprintf("%s\n", model);if (ret)return CMD_RET_FAILURE;elsereturn CMD_RET_SUCCESS;+}
+static int do_sysinfo_id(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])+{
struct udevice *dev;u32 board_id;int ret = get_sysinfo(&dev);if (ret)return CMD_RET_FAILURE;ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_ID, &board_id);if (ret) {printf("Cannot get sysinfo int: %d\n", ret);return CMD_RET_FAILURE;}if (argc == 2)ret = env_set_hex(argv[1], board_id);elseprintf("0x%02x\n", board_id);if (ret)return CMD_RET_FAILURE;elsereturn CMD_RET_SUCCESS;+}
+static int do_sysinfo_revision(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])+{
struct udevice *dev;int rev_major;int rev_minor;char rev[64];int ret = get_sysinfo(&dev);if (ret)return CMD_RET_FAILURE;ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_REV_MAJOR, &rev_major);if (ret) {printf("Cannot get sysinfo int: %d\n", ret);return CMD_RET_FAILURE;}ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_REV_MINOR, &rev_minor);if (ret) {printf("Cannot get sysinfo int: %d\n", ret);return CMD_RET_FAILURE;}snprintf(rev, sizeof(rev), "%d.%d", rev_major, rev_minor);if (argc == 2)ret = env_set(argv[1], rev);elseprintf("%s\n", rev);if (ret)return CMD_RET_FAILURE;elsereturn CMD_RET_SUCCESS;+}
+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),+);
2.41.0
On Monday, October 2, 2023 2:56:28 P.M. EDT Simon Glass wrote:
On Mon, 2 Oct 2023 at 09:21, Detlev Casanova
detlev.casanova@collabora.com wrote:
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.
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
cmd/Kconfig | 6 +++ cmd/Makefile | 1 + cmd/sysinfo.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+) create mode 100644 cmd/sysinfo.c
Reviewed-by: Simon Glass sjg@chromium.org
Would prefer more help text in the Kconfig as this is pretty vague
Indeed, how would that sound ?
Display information to identify the system. It supports showing a board id, a revision number as well as a human readable name.
diff --git a/cmd/Kconfig b/cmd/Kconfig index 43ca10f69cc..67d019aa795 100644 --- a/cmd/Kconfig +++ b/cmd/Kconfig @@ -218,6 +218,12 @@ config CMD_SBI
help Display information about the SBI implementation.+config CMD_SYSINFO
bool "sysinfo"depends on SYSINFOhelpDisplay information about the system.endmenu
menu "Boot commands"
diff --git a/cmd/Makefile b/cmd/Makefile index 9bebf321c39..972f3a4720e 100644 --- a/cmd/Makefile +++ b/cmd/Makefile @@ -167,6 +167,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..46369ff9ac7 --- /dev/null +++ b/cmd/sysinfo.c @@ -0,0 +1,133 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright 2023
- Detlev Casanova detlev.casanova@collabora.com
- */
+#include <command.h> +#include <env.h> +#include <sysinfo.h> +#include <exports.h>
+static int get_sysinfo(struct udevice **devp) +{
int ret = sysinfo_get(devp);if (ret) {printf("Cannot get sysinfo: %d\n", ret);return ret;}ret = sysinfo_detect(*devp);if (ret) {printf("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 CMD_RET_FAILURE;ret = sysinfo_get_str(dev,SYSINFO_ID_BOARD_MODEL,sizeof(model),model);if (ret) {printf("Cannot get sysinfo str: %d\n", ret);return CMD_RET_FAILURE;}if (argc == 2)ret = env_set(argv[1], model);elseprintf("%s\n", model);if (ret)return CMD_RET_FAILURE;elsereturn CMD_RET_SUCCESS;+}
+static int do_sysinfo_id(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])+{
struct udevice *dev;u32 board_id;int ret = get_sysinfo(&dev);if (ret)return CMD_RET_FAILURE;ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_ID, &board_id);if (ret) {printf("Cannot get sysinfo int: %d\n", ret);return CMD_RET_FAILURE;}if (argc == 2)ret = env_set_hex(argv[1], board_id);elseprintf("0x%02x\n", board_id);if (ret)return CMD_RET_FAILURE;elsereturn CMD_RET_SUCCESS;+}
+static int do_sysinfo_revision(struct cmd_tbl *cmdtp, int flag, int argc,
char *const argv[])+{
struct udevice *dev;int rev_major;int rev_minor;char rev[64];int ret = get_sysinfo(&dev);if (ret)return CMD_RET_FAILURE;ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_REV_MAJOR,&rev_major);
if (ret) {printf("Cannot get sysinfo int: %d\n", ret);return CMD_RET_FAILURE;}ret = sysinfo_get_int(dev, SYSINFO_ID_BOARD_REV_MINOR,&rev_minor);
if (ret) {printf("Cannot get sysinfo int: %d\n", ret);return CMD_RET_FAILURE;}snprintf(rev, sizeof(rev), "%d.%d", rev_major, rev_minor);if (argc == 2)ret = env_set(argv[1], rev);elseprintf("%s\n", rev);if (ret)return CMD_RET_FAILURE;elsereturn CMD_RET_SUCCESS;+}
+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), +);
2.41.0
Hi Detlev,
On Mon, 16 Oct 2023 at 08:21, Detlev Casanova detlev.casanova@collabora.com wrote:
On Monday, October 2, 2023 2:56:28 P.M. EDT Simon Glass wrote:
On Mon, 2 Oct 2023 at 09:21, Detlev Casanova
detlev.casanova@collabora.com wrote:
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.
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
cmd/Kconfig | 6 +++ cmd/Makefile | 1 + cmd/sysinfo.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 140 insertions(+) create mode 100644 cmd/sysinfo.c
Reviewed-by: Simon Glass sjg@chromium.org
Would prefer more help text in the Kconfig as this is pretty vague
Indeed, how would that sound ?
Display information to identify the system. It supports showing a board id, a revision number as well as a human readable name.
Also it uses the sysinfo driver for your board to get the information. You could link to the sysinfo docs or header file.
Regards, Simon
The test runs one of each subcommand and checks that the output matches the values in the sandbox sysinfo driver.
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org Signed-off-by: Detlev Casanova detlev.casanova@collabora.com --- configs/sandbox_defconfig | 1 + drivers/sysinfo/sandbox.c | 17 +++++++++++++ test/cmd/Makefile | 1 + test/cmd/test_sysinfo.c | 51 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+) create mode 100644 test/cmd/test_sysinfo.c
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 62bc182ca16..409e3d88012 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -129,6 +129,7 @@ CONFIG_CMD_EROFS=y CONFIG_CMD_EXT4_WRITE=y CONFIG_CMD_SQUASHFS=y CONFIG_CMD_MTDPARTS=y +CONFIG_CMD_SYSINFO=y CONFIG_CMD_STACKPROTECTOR_TEST=y CONFIG_MAC_PARTITION=y CONFIG_AMIGA_PARTITION=y diff --git a/drivers/sysinfo/sandbox.c b/drivers/sysinfo/sandbox.c index d270a26aa43..cc7783907a9 100644 --- a/drivers/sysinfo/sandbox.c +++ b/drivers/sysinfo/sandbox.c @@ -7,9 +7,14 @@ #include <common.h> #include <dm.h> #include <sysinfo.h> +#include <version.h>
#include "sandbox.h"
+#define SANDBOX_BOARD_ID 0x42 +#define SANDBOX_BOARD_REV_MAJOR U_BOOT_VERSION_NUM +#define SANDBOX_BOARD_REV_MINOR U_BOOT_VERSION_NUM_PATCH + struct sysinfo_sandbox_priv { bool called_detect; int test_i1; @@ -48,6 +53,15 @@ int sysinfo_sandbox_get_int(struct udevice *dev, int id, int *val) struct sysinfo_sandbox_priv *priv = dev_get_priv(dev);
switch (id) { + case SYSINFO_ID_BOARD_ID: + *val = SANDBOX_BOARD_ID; + return 0; + case SYSINFO_ID_BOARD_REV_MAJOR: + *val = SANDBOX_BOARD_REV_MAJOR; + return 0; + case SYSINFO_ID_BOARD_REV_MINOR: + *val = SANDBOX_BOARD_REV_MINOR; + return 0; case INT_TEST1: *val = priv->test_i1; /* Increments with every call */ @@ -71,6 +85,9 @@ int sysinfo_sandbox_get_str(struct udevice *dev, int id, size_t size, char *val) int index = (i1 * i2) % ARRAY_SIZE(vacation_spots);
switch (id) { + case SYSINFO_ID_BOARD_MODEL: + snprintf(val, size, "sandbox"); + return 0; case STR_VACATIONSPOT: /* Picks a vacation spot depending on i1 and i2 */ snprintf(val, size, vacation_spots[index]); diff --git a/test/cmd/Makefile b/test/cmd/Makefile index 6e3d7e919ef..dc8f59d87f2 100644 --- a/test/cmd/Makefile +++ b/test/cmd/Makefile @@ -26,6 +26,7 @@ ifdef CONFIG_SANDBOX obj-$(CONFIG_CMD_READ) += rw.o obj-$(CONFIG_CMD_SETEXPR) += setexpr.o obj-$(CONFIG_ARM_FFA_TRANSPORT) += armffa.o +obj-$(CONFIG_CMD_SYSINFO) += test_sysinfo.o endif obj-$(CONFIG_CMD_TEMPERATURE) += temperature.o obj-$(CONFIG_CMD_WGET) += wget.o diff --git a/test/cmd/test_sysinfo.c b/test/cmd/test_sysinfo.c new file mode 100644 index 00000000000..7ba6dd0df89 --- /dev/null +++ b/test/cmd/test_sysinfo.c @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Tests for sysinfo command + * + * Copyright 2023, Detlev Casanova detlev.casanova@collabora.com + */ + +#include <common.h> +#include <command.h> +#include <asm/global_data.h> +#include <display_options.h> +#include <test/lib.h> +#include <test/test.h> +#include <test/ut.h> +#include <version.h> + +DECLARE_GLOBAL_DATA_PTR; + +#define REV_(x, y) #x "." #y +#define REV(x, y) REV_(x, y) + +struct test_data { + char *cmd; + char *expected; +}; + +static struct test_data sysinfo_data[] = { + {"sysinfo model", "sandbox"}, + {"sysinfo id", "0x42"}, + {"sysinfo revision", REV(U_BOOT_VERSION_NUM, U_BOOT_VERSION_NUM_PATCH)}, +}; + +static int lib_test_sysinfo(struct unit_test_state *uts) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(sysinfo_data); ++i) { + ut_silence_console(uts); + console_record_reset_enable(); + ut_assertok(run_command(sysinfo_data[i].cmd, 0)); + ut_unsilence_console(uts); + console_record_readline(uts->actual_str, + sizeof(uts->actual_str)); + ut_asserteq_str(sysinfo_data[i].expected, uts->actual_str); + ut_assertok(ut_check_console_end(uts)); + } + + return 0; +} + +LIB_TEST(lib_test_sysinfo, 0);
Hi Detlev,
On Mon, 2 Oct 2023 at 09:22, Detlev Casanova detlev.casanova@collabora.com wrote:
The test runs one of each subcommand and checks that the output matches the values in the sandbox sysinfo driver.
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
configs/sandbox_defconfig | 1 + drivers/sysinfo/sandbox.c | 17 +++++++++++++ test/cmd/Makefile | 1 + test/cmd/test_sysinfo.c | 51 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 70 insertions(+) create mode 100644 test/cmd/test_sysinfo.c
Reviewed-by: Simon Glass sjg@chromium.org
nits below
diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig index 62bc182ca16..409e3d88012 100644 --- a/configs/sandbox_defconfig +++ b/configs/sandbox_defconfig @@ -129,6 +129,7 @@ CONFIG_CMD_EROFS=y CONFIG_CMD_EXT4_WRITE=y CONFIG_CMD_SQUASHFS=y CONFIG_CMD_MTDPARTS=y +CONFIG_CMD_SYSINFO=y CONFIG_CMD_STACKPROTECTOR_TEST=y CONFIG_MAC_PARTITION=y CONFIG_AMIGA_PARTITION=y diff --git a/drivers/sysinfo/sandbox.c b/drivers/sysinfo/sandbox.c index d270a26aa43..cc7783907a9 100644 --- a/drivers/sysinfo/sandbox.c +++ b/drivers/sysinfo/sandbox.c @@ -7,9 +7,14 @@ #include <common.h> #include <dm.h> #include <sysinfo.h> +#include <version.h>
#include "sandbox.h"
+#define SANDBOX_BOARD_ID 0x42 +#define SANDBOX_BOARD_REV_MAJOR U_BOOT_VERSION_NUM +#define SANDBOX_BOARD_REV_MINOR U_BOOT_VERSION_NUM_PATCH
struct sysinfo_sandbox_priv { bool called_detect; int test_i1; @@ -48,6 +53,15 @@ int sysinfo_sandbox_get_int(struct udevice *dev, int id, int *val) struct sysinfo_sandbox_priv *priv = dev_get_priv(dev);
switch (id) {
case SYSINFO_ID_BOARD_ID:*val = SANDBOX_BOARD_ID;return 0;case SYSINFO_ID_BOARD_REV_MAJOR:*val = SANDBOX_BOARD_REV_MAJOR;return 0;case SYSINFO_ID_BOARD_REV_MINOR:*val = SANDBOX_BOARD_REV_MINOR;return 0; case INT_TEST1: *val = priv->test_i1; /* Increments with every call */@@ -71,6 +85,9 @@ int sysinfo_sandbox_get_str(struct udevice *dev, int id, size_t size, char *val) int index = (i1 * i2) % ARRAY_SIZE(vacation_spots);
switch (id) {
case SYSINFO_ID_BOARD_MODEL:snprintf(val, size, "sandbox");return 0; case STR_VACATIONSPOT: /* Picks a vacation spot depending on i1 and i2 */ snprintf(val, size, vacation_spots[index]);diff --git a/test/cmd/Makefile b/test/cmd/Makefile index 6e3d7e919ef..dc8f59d87f2 100644 --- a/test/cmd/Makefile +++ b/test/cmd/Makefile @@ -26,6 +26,7 @@ ifdef CONFIG_SANDBOX obj-$(CONFIG_CMD_READ) += rw.o obj-$(CONFIG_CMD_SETEXPR) += setexpr.o obj-$(CONFIG_ARM_FFA_TRANSPORT) += armffa.o +obj-$(CONFIG_CMD_SYSINFO) += test_sysinfo.o endif obj-$(CONFIG_CMD_TEMPERATURE) += temperature.o obj-$(CONFIG_CMD_WGET) += wget.o diff --git a/test/cmd/test_sysinfo.c b/test/cmd/test_sysinfo.c new file mode 100644 index 00000000000..7ba6dd0df89 --- /dev/null +++ b/test/cmd/test_sysinfo.c @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Tests for sysinfo command
- Copyright 2023, Detlev Casanova detlev.casanova@collabora.com
- */
+#include <common.h> +#include <command.h> +#include <asm/global_data.h> +#include <display_options.h> +#include <test/lib.h> +#include <test/test.h> +#include <test/ut.h> +#include <version.h>
+DECLARE_GLOBAL_DATA_PTR;
+#define REV_(x, y) #x "." #y +#define REV(x, y) REV_(x, y)
+struct test_data {
char *cmd;char *expected;+};
+static struct test_data sysinfo_data[] = {
{"sysinfo model", "sandbox"},{"sysinfo id", "0x42"},{"sysinfo revision", REV(U_BOOT_VERSION_NUM, U_BOOT_VERSION_NUM_PATCH)},
I would prefer that write out the lines in code. I don't think it helps much to put these in a table.
+};
+static int lib_test_sysinfo(struct unit_test_state *uts) +{
int i;for (i = 0; i < ARRAY_SIZE(sysinfo_data); ++i) {ut_silence_console(uts);console_record_reset_enable();
drop those two lines
ut_assertok(run_command(sysinfo_data[i].cmd, 0));ut_unsilence_console(uts);console_record_readline(uts->actual_str,sizeof(uts->actual_str));ut_asserteq_str(sysinfo_data[i].expected, uts->actual_str);
ut_assert_nextline(sysinfo_data[i].expected, uts->actual_str);
ut_assertok(ut_check_console_end(uts));}return 0;+}
+LIB_TEST(lib_test_sysinfo, 0);
Add UT_TESTF_CONSOLE_REC
-- 2.41.0
Regards, Simon
Add documentation for the sysinfo command with examples.
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org Signed-off-by: Detlev Casanova detlev.casanova@collabora.com --- doc/usage/cmd/sysinfo.rst | 56 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 doc/usage/cmd/sysinfo.rst
diff --git a/doc/usage/cmd/sysinfo.rst b/doc/usage/cmd/sysinfo.rst new file mode 100644 index 00000000000..1660b2aa1a6 --- /dev/null +++ b/doc/usage/cmd/sysinfo.rst @@ -0,0 +1,56 @@ +.. SPDX-License-Identifier: GPL-2.0+: + +sysinfo command +=============== + +Synopis +------- + +:: + + sysinfo id <varname> + sysinfo model <varname> + sysinfo revision <varname> + +Description +----------- + +The `sysinfo` command is used to show information about the running system + +The `sysinfo id` command prints or sets an environment variable to the board id +as an hex value. + + varname + an optional environment variable to store the board id into. + +The `sysinfo model` command prints or sets an environment variable to the board +model name as a string value. + + varname + an optional environment variable to store the board model name into. + +The `sysinfo revision` command prints or sets an environment variable to the +board revision in the <MAJOR>.<MINOR> format, where MINOR and MINOR are int +values. + + varname + an optional environment variable to store the board revision into. + +Examples +-------- + +:: + + => sysinfo id + 0x0b + => sysinfo model + Renesas Starter Kit Premier board rev 2.1 + => sysinfo revision varname + => env print varname + 2.1 + +Return value +------------ + +The return value $? is set to 0 (true) if the command succeeded. If an +error occurs, the return value $? is set to 1 (false).
Hi Detlev,
On Mon, 2 Oct 2023 at 09:22, Detlev Casanova detlev.casanova@collabora.com wrote:
Add documentation for the sysinfo command with examples.
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
doc/usage/cmd/sysinfo.rst | 56 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 doc/usage/cmd/sysinfo.rst
Reviewed-by: Simon Glass sjg@chromium.org
diff --git a/doc/usage/cmd/sysinfo.rst b/doc/usage/cmd/sysinfo.rst new file mode 100644 index 00000000000..1660b2aa1a6 --- /dev/null +++ b/doc/usage/cmd/sysinfo.rst @@ -0,0 +1,56 @@ +.. SPDX-License-Identifier: GPL-2.0+:
+sysinfo command +===============
+Synopis +-------
+::
- sysinfo id <varname>
[<varname>]
I think, since the params are optional?
- sysinfo model <varname>
- sysinfo revision <varname>
+Description +-----------
+The `sysinfo` command is used to show information about the running system
+The `sysinfo id` command prints or sets an environment variable to the board id +as an hex value.
- varname
an optional environment variable to store the board id into.+The `sysinfo model` command prints or sets an environment variable to the board +model name as a string value.
- varname
an optional environment variable to store the board model name into.+The `sysinfo revision` command prints or sets an environment variable to the +board revision in the <MAJOR>.<MINOR> format, where MINOR and MINOR are int +values.
- varname
an optional environment variable to store the board revision into.+Examples +--------
+::
- => sysinfo id
- 0x0b
- => sysinfo model
- Renesas Starter Kit Premier board rev 2.1
- => sysinfo revision varname
- => env print varname
- 2.1
+Return value +------------
+The return value $? is set to 0 (true) if the command succeeded. If an
+error occurs, the return value $? is set to 1 (false).
2.41.0
Regards.
Simon
On 10/2/23 17:20, Detlev Casanova wrote:
Add documentation for the sysinfo command with examples.
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
doc/usage/cmd/sysinfo.rst | 56 +++++++++++++++++++++++++++++++++++++++
Thank you for providing a man-page.
Unfortunately this does not build: usage/cmd/sysinfo.rst:document isn't included in any toctree
Please, add sysinfo to doc/usage/index.rst.
Please, use run 'make htmldocs' before resubmitting.
1 file changed, 56 insertions(+) create mode 100644 doc/usage/cmd/sysinfo.rst
diff --git a/doc/usage/cmd/sysinfo.rst b/doc/usage/cmd/sysinfo.rst new file mode 100644 index 00000000000..1660b2aa1a6 --- /dev/null +++ b/doc/usage/cmd/sysinfo.rst @@ -0,0 +1,56 @@ +.. SPDX-License-Identifier: GPL-2.0+:
+sysinfo command +===============
+Synopis
%s/Synopis/Synopsis/
+-------
+::
- sysinfo id <varname>
- sysinfo model <varname>
- sysinfo revision <varname>
+Description +-----------
+The `sysinfo` command is used to show information about the running system
+The `sysinfo id` command prints or sets an environment variable to the board id +as an hex value.
%s/an hex/a hexadecimal/
- varname
an optional environment variable to store the board id into.+The `sysinfo model` command prints or sets an environment variable to the board +model name as a string value.
- varname
an optional environment variable to store the board model name into.
If varname were optional, the synopsis would be
sysinfo model [varname]
+The `sysinfo revision` command prints or sets an environment variable to the +board revision in the <MAJOR>.<MINOR> format, where MINOR and MINOR are int +values.
- varname
an optional environment variable to store the board revision into.+Examples +--------
+::
- => sysinfo id
- 0x0b
- => sysinfo model
- Renesas Starter Kit Premier board rev 2.1
- => sysinfo revision varname
- => env print varname
- 2.1
This is too much hassle to print the information.
We should make the parameters optional:
'sysinfo' should print all information. 'sysinfo revision' should print the revision.
+Return value +------------
+The return value $? is set to 0 (true) if the command succeeded. If an +error occurs, the return value $? is set to 1 (false).
Please, use the same tense:
%s/succeeded/succeeds/
Best regards
Heinrich
To be used with the sysinfo command, revision values must be considered as integers, not chars as some boards will implement BOARD_REVISION_* and might use numbers greater than 9.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com --- drivers/sysinfo/rcar3.c | 73 ++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 34 deletions(-)
diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c index 7b127986da7..633e80bc19b 100644 --- a/drivers/sysinfo/rcar3.c +++ b/drivers/sysinfo/rcar3.c @@ -68,8 +68,9 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) bool salvator_xs = false; bool ebisu_4d = false; bool condor_i = false; - char rev_major = '?'; - char rev_minor = '?'; + char rev[4] = "?.?"; + u8 rev_major = 0; + u8 rev_minor = 0;
switch (board_id) { case BOARD_SALVATOR_XS: @@ -77,81 +78,85 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) fallthrough; case BOARD_SALVATOR_X: if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */ - rev_major = '1'; - rev_minor = '0' + (board_rev & BIT(0)); + rev_major = 1; + rev_minor = board_rev & BIT(0); + snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); } snprintf(priv->boardmodel, sizeof(priv->boardmodel), - "Renesas Salvator-X%s board rev %c.%c", - salvator_xs ? "S" : "", rev_major, rev_minor); + "Renesas Salvator-X%s board rev %s", + salvator_xs ? "S" : "", rev); + return; case BOARD_STARTER_KIT: if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */ - rev_major = (board_rev & BIT(0)) ? '3' : '1'; - rev_minor = '0'; + rev_major = (board_rev & BIT(0)) ? 3 : 1; + rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); } snprintf(priv->boardmodel, sizeof(priv->boardmodel), - "Renesas Starter Kit board rev %c.%c", - rev_major, rev_minor); + "Renesas Starter Kit board rev %s", rev); return; case BOARD_STARTER_KIT_PRE: if (!(board_rev & ~3)) { /* Only rev 0..3 is valid */ - rev_major = (board_rev & BIT(1)) ? '2' : '1'; - rev_minor = (board_rev == 3) ? '1' : '0'; + rev_major = (board_rev & BIT(1)) ? 2 : 1; + rev_minor = (board_rev == 3) ? 1 : 0; + snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); } snprintf(priv->boardmodel, sizeof(priv->boardmodel), - "Renesas Starter Kit Premier board rev %c.%c", - rev_major, rev_minor); + "Renesas Starter Kit Premier board rev %s", rev); return; case BOARD_EAGLE: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = '1'; - rev_minor = '0'; + rev_major = 1; + rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); } snprintf(priv->boardmodel, sizeof(priv->boardmodel), - "Renesas Eagle board rev %c.%c", - rev_major, rev_minor); + "Renesas Eagle board rev %s", rev); return; case BOARD_EBISU_4D: ebisu_4d = true; fallthrough; case BOARD_EBISU: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = '1'; - rev_minor = '0'; + rev_major = 1; + rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); } snprintf(priv->boardmodel, sizeof(priv->boardmodel), - "Renesas Ebisu%s board rev %c.%c", - ebisu_4d ? "-4D" : "", rev_major, rev_minor); + "Renesas Ebisu%s board rev %s", + ebisu_4d ? "-4D" : "", rev); return; case BOARD_DRAAK: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = '1'; - rev_minor = '0'; + rev_major = 1; + rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); } snprintf(priv->boardmodel, sizeof(priv->boardmodel), - "Renesas Draak board rev %c.%c", - rev_major, rev_minor); + "Renesas Draak board rev %s", rev); return; case BOARD_KRIEK: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = '1'; - rev_minor = '0'; + rev_major = 1; + rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); } snprintf(priv->boardmodel, sizeof(priv->boardmodel), - "Renesas Kriek board rev %c.%c", - rev_major, rev_minor); + "Renesas Kriek board rev %s", rev); return; case BOARD_CONDOR_I: condor_i = true; fallthrough; case BOARD_CONDOR: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = '1'; - rev_minor = '0'; + rev_major = 1; + rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); } snprintf(priv->boardmodel, sizeof(priv->boardmodel), - "Renesas Condor%s board rev %c.%c", - condor_i ? "-I" : "", rev_major, rev_minor); + "Renesas Condor%s board rev %s", + condor_i ? "-I" : "", rev); return; default: snprintf(priv->boardmodel, sizeof(priv->boardmodel),
On 10/2/23 17:20, Detlev Casanova wrote:
To be used with the sysinfo command, revision values must be considered as integers, not chars as some boards will implement BOARD_REVISION_* and might use numbers greater than 9.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
drivers/sysinfo/rcar3.c | 73 ++++++++++++++++++++++------------------- 1 file changed, 39 insertions(+), 34 deletions(-)
diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c index 7b127986da7..633e80bc19b 100644 --- a/drivers/sysinfo/rcar3.c +++ b/drivers/sysinfo/rcar3.c @@ -68,8 +68,9 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) bool salvator_xs = false; bool ebisu_4d = false; bool condor_i = false;
- char rev_major = '?';
- char rev_minor = '?';
char rev[4] = "?.?";
u8 rev_major = 0;
u8 rev_minor = 0;
switch (board_id) { case BOARD_SALVATOR_XS:
@@ -77,81 +78,85 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) fallthrough; case BOARD_SALVATOR_X: if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */
rev_major = '1';rev_minor = '0' + (board_rev & BIT(0));
rev_major = 1;rev_minor = board_rev & BIT(0);snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor);
"Renesas Salvator-X%s board rev %c.%c",salvator_xs ? "S" : "", rev_major, rev_minor);
"Renesas Salvator-X%s board rev %s",salvator_xs ? "S" : "", rev);- return;
Extra newline got added just before the return here. If you want to do V6 with that fixed, that would be nice. In either case:
Reviewed-by: Marek Vasut marek.vasut+renesas@mailbox.org
Thanks !
Expose that information to the sysinfo command to let scripts make decisions based on the board id and revision.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com --- drivers/sysinfo/rcar3.c | 89 +++++++++++++++++++++++++++++------------ 1 file changed, 63 insertions(+), 26 deletions(-)
diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c index 633e80bc19b..a554323506a 100644 --- a/drivers/sysinfo/rcar3.c +++ b/drivers/sysinfo/rcar3.c @@ -32,6 +32,10 @@ */ struct sysinfo_rcar_priv { char boardmodel[64]; + u8 id; + u8 rev_major; + u8 rev_minor; + bool has_rev; u8 val; };
@@ -56,9 +60,33 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, size_t size, char * }; }
+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; + case SYSINFO_ID_BOARD_REV_MAJOR: + if (!priv->has_rev) + return -EINVAL; + *val = priv->rev_major; + return 0; + case SYSINFO_ID_BOARD_REV_MINOR: + if (!priv->has_rev) + return -EINVAL; + *val = priv->rev_minor; + 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) @@ -69,8 +97,9 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) bool ebisu_4d = false; bool condor_i = false; char rev[4] = "?.?"; - u8 rev_major = 0; - u8 rev_minor = 0; + + priv->id = board_id; + priv->has_rev = false;
switch (board_id) { case BOARD_SALVATOR_XS: @@ -78,9 +107,10 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) fallthrough; case BOARD_SALVATOR_X: if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */ - rev_major = 1; - rev_minor = board_rev & BIT(0); - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = 1; + priv->rev_minor = board_rev & BIT(0); + priv->has_rev = true; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Salvator-X%s board rev %s", @@ -89,27 +119,30 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) return; case BOARD_STARTER_KIT: if (!(board_rev & ~1)) { /* Only rev 0 and 1 is valid */ - rev_major = (board_rev & BIT(0)) ? 3 : 1; - rev_minor = 0; - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = (board_rev & BIT(0)) ? 3 : 1; + priv->rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); + priv->has_rev = true; } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Starter Kit board rev %s", rev); return; case BOARD_STARTER_KIT_PRE: if (!(board_rev & ~3)) { /* Only rev 0..3 is valid */ - rev_major = (board_rev & BIT(1)) ? 2 : 1; - rev_minor = (board_rev == 3) ? 1 : 0; - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = (board_rev & BIT(1)) ? 2 : 1; + priv->rev_minor = (board_rev == 3) ? 1 : 0; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); + priv->has_rev = true; } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Starter Kit Premier board rev %s", rev); return; case BOARD_EAGLE: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = 1; - rev_minor = 0; - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = 1; + priv->rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); + priv->has_rev = true; } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Eagle board rev %s", rev); @@ -119,9 +152,10 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) fallthrough; case BOARD_EBISU: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = 1; - rev_minor = 0; - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = 1; + priv->rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); + priv->has_rev = true; } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Ebisu%s board rev %s", @@ -129,18 +163,20 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) return; case BOARD_DRAAK: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = 1; - rev_minor = 0; - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = 1; + priv->rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); + priv->has_rev = true; } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Draak board rev %s", rev); return; case BOARD_KRIEK: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = 1; - rev_minor = 0; - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = 1; + priv->rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); + priv->has_rev = true; } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Kriek board rev %s", rev); @@ -150,9 +186,10 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) fallthrough; case BOARD_CONDOR: if (!board_rev) { /* Only rev 0 is valid */ - rev_major = 1; - rev_minor = 0; - snprintf(rev, sizeof(rev), "%u.%u", rev_major, rev_minor); + priv->rev_major = 1; + priv->rev_minor = 0; + snprintf(rev, sizeof(rev), "%u.%u", priv->rev_major, priv->rev_minor); + priv->has_rev = true; } snprintf(priv->boardmodel, sizeof(priv->boardmodel), "Renesas Condor%s board rev %s",
On 10/2/23 17:20, Detlev Casanova wrote:
Expose that information to the sysinfo command to let scripts make decisions based on the board id and revision.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
drivers/sysinfo/rcar3.c | 89 +++++++++++++++++++++++++++++------------ 1 file changed, 63 insertions(+), 26 deletions(-)
diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c index 633e80bc19b..a554323506a 100644 --- a/drivers/sysinfo/rcar3.c +++ b/drivers/sysinfo/rcar3.c @@ -32,6 +32,10 @@ */ struct sysinfo_rcar_priv { char boardmodel[64];
- u8 id;
- u8 rev_major;
- u8 rev_minor;
- bool has_rev; u8 val; };
@@ -56,9 +60,33 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, size_t size, char * }; }
+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;- case SYSINFO_ID_BOARD_REV_MAJOR:
if (!priv->has_rev)return -EINVAL;*val = priv->rev_major;return 0;- case SYSINFO_ID_BOARD_REV_MINOR:
if (!priv->has_rev)return -EINVAL;*val = priv->rev_minor;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)
@@ -69,8 +97,9 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv) bool ebisu_4d = false; bool condor_i = false; char rev[4] = "?.?";
- u8 rev_major = 0;
- u8 rev_minor = 0;
- priv->id = board_id;
- priv->has_rev = false;
Would it make more sense to assign
priv->rev_major = 1; priv->rev_minor = 0;
And get rid of priv->has_rev entirely ?
Basically say that the default case is rev. 1.0 board.
[...]
On Saturday, October 7, 2023 5:35:27 P.M. EDT Marek Vasut wrote:
On 10/2/23 17:20, Detlev Casanova wrote:
Expose that information to the sysinfo command to let scripts make decisions based on the board id and revision.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
drivers/sysinfo/rcar3.c | 89 +++++++++++++++++++++++++++++------------ 1 file changed, 63 insertions(+), 26 deletions(-)
diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c index 633e80bc19b..a554323506a 100644 --- a/drivers/sysinfo/rcar3.c +++ b/drivers/sysinfo/rcar3.c @@ -32,6 +32,10 @@
*/
struct sysinfo_rcar_priv {
char boardmodel[64];
u8 id;
u8 rev_major;
u8 rev_minor;
bool has_rev;
u8 val;
};
@@ -56,9 +60,33 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, size_t size, char *> };
}
+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;- case SYSINFO_ID_BOARD_REV_MAJOR:
if (!priv->has_rev)return -EINVAL;*val = priv->rev_major;return 0;- case SYSINFO_ID_BOARD_REV_MINOR:
if (!priv->has_rev)return -EINVAL;*val = priv->rev_minor;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)
@@ -69,8 +97,9 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv)> bool ebisu_4d = false; bool condor_i = false; char rev[4] = "?.?";
- u8 rev_major = 0;
- u8 rev_minor = 0;
- priv->id = board_id;
- priv->has_rev = false;
Would it make more sense to assign
priv->rev_major = 1; priv->rev_minor = 0;
And get rid of priv->has_rev entirely ?
Basically say that the default case is rev. 1.0 board.
[...]
I'm not really sure about this has it doesn't differentiate between rev 1.0 and unknown revision.
On 10/16/23 19:03, Detlev Casanova wrote:
On Saturday, October 7, 2023 5:35:27 P.M. EDT Marek Vasut wrote:
On 10/2/23 17:20, Detlev Casanova wrote:
Expose that information to the sysinfo command to let scripts make decisions based on the board id and revision.
Signed-off-by: Detlev Casanova detlev.casanova@collabora.com
drivers/sysinfo/rcar3.c | 89 +++++++++++++++++++++++++++++------------ 1 file changed, 63 insertions(+), 26 deletions(-)
diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c index 633e80bc19b..a554323506a 100644 --- a/drivers/sysinfo/rcar3.c +++ b/drivers/sysinfo/rcar3.c @@ -32,6 +32,10 @@
*/struct sysinfo_rcar_priv {
char boardmodel[64];
u8 id;
u8 rev_major;
u8 rev_minor;
bool has_rev;
u8 val;
};
@@ -56,9 +60,33 @@ static int sysinfo_rcar_get_str(struct udevice *dev, int id, size_t size, char *> };
}
+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;- case SYSINFO_ID_BOARD_REV_MAJOR:
if (!priv->has_rev)return -EINVAL;*val = priv->rev_major;return 0;- case SYSINFO_ID_BOARD_REV_MINOR:
if (!priv->has_rev)return -EINVAL;*val = priv->rev_minor;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)
@@ -69,8 +97,9 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv)> bool ebisu_4d = false; bool condor_i = false; char rev[4] = "?.?";
- u8 rev_major = 0;
- u8 rev_minor = 0;
- priv->id = board_id;
- priv->has_rev = false;
Would it make more sense to assign
priv->rev_major = 1; priv->rev_minor = 0;
And get rid of priv->has_rev entirely ?
Basically say that the default case is rev. 1.0 board.
[...]
I'm not really sure about this has it doesn't differentiate between rev 1.0 and unknown revision.
Good point, please document this in the commit message.
participants (4)
-
Detlev Casanova -
Heinrich Schuchardt -
Marek Vasut -
Simon Glass