[PATCH v4 0/6] Introduce the sysinfo command

[PATCH v4 0/6] Introduce the sysinfo command

[Detlev Casanova]

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 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   | 141 ++++++++++++++++++++++++++------------
 drivers/sysinfo/sandbox.c |  17 +++++
 include/sysinfo.h         |   5 ++
 test/cmd/Makefile         |   1 +
 test/cmd/test_sysinfo.c   |  51 ++++++++++++++
 10 files changed, 370 insertions(+), 42 deletions(-)
 create mode 100644 cmd/sysinfo.c
 create mode 100644 doc/usage/cmd/sysinfo.rst
 create mode 100644 test/cmd/test_sysinfo.c

-- 
2.41.0

[PATCH v4 1/6] sysinfo: Add IDs for board id and revision

[Detlev Casanova]

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,
 };
-- 
2.41.0

[PATCH v4 2/6] cmd: Add a sysinfo command

[Detlev Casanova]

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 ecfd5752377..796eedba194 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 9f8c0b058be..87a196b4a03 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -166,6 +166,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),
+);
-- 
2.41.0

[PATCH v4 3/6] sysinfo: Add a test

[Detlev Casanova]

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 19cc6701e62..a51762aa3ae 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -130,6 +130,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 a3cf983739e..d3ac5bf2d5e 100644
--- a/test/cmd/Makefile
+++ b/test/cmd/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_CMD_SEAMA) += seama.o
 ifdef CONFIG_SANDBOX
 obj-$(CONFIG_CMD_READ) += rw.o
 obj-$(CONFIG_CMD_SETEXPR) += setexpr.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);
-- 
2.41.0

[PATCH v4 4/6] sysinfo: Add documentation

[Detlev Casanova]

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).
-- 
2.41.0

[PATCH v4 5/6] sysinfo: rcar3: Use int instead of char for revision

[Detlev Casanova]

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 | 104 ++++++++++++++++++++++++----------------
 1 file changed, 62 insertions(+), 42 deletions(-)

diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c
index 7b127986da7..450a4c26773 100644
--- a/drivers/sysinfo/rcar3.c
+++ b/drivers/sysinfo/rcar3.c
@@ -68,90 +68,110 @@ 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 model[64];
+	char rev[4] = "?.?";
+	u8 rev_major = 0;
+	u8 rev_minor = 0;
 
 	switch (board_id) {
 	case BOARD_SALVATOR_XS:
 		salvator_xs = true;
 		fallthrough;
 	case BOARD_SALVATOR_X:
+		snprintf(model, sizeof(model),
+			 "Renesas Salvator-X%s board", salvator_xs ? "S" : "");
 		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);
+
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev %s",
+			 model, rev);
+
 		return;
 	case BOARD_STARTER_KIT:
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+			 "Renesas Starter Kit board");
 		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);
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev %s",
+			 model, rev);
 		return;
 	case BOARD_STARTER_KIT_PRE:
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+			 "Renesas Starter Kit Premier board");
 		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);
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev %s",
+			 model, rev);
 		return;
 	case BOARD_EAGLE:
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+			 "Renesas Eagle board");
 		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);
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev %s",
+			 model, rev);
 		return;
 	case BOARD_EBISU_4D:
 		ebisu_4d = true;
 		fallthrough;
 	case BOARD_EBISU:
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+			 "Renesas Ebisu%s board", ebisu_4d ? "-4D" : "");
 		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);
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev %s",
+			 model, rev);
 		return;
 	case BOARD_DRAAK:
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+			 "Renesas Draak board");
 		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);
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev %s",
+			 model, rev);
 		return;
 	case BOARD_KRIEK:
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+			 "Renesas Kriek board");
 		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);
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev %s",
+			 model, rev);
 		return;
 	case BOARD_CONDOR_I:
 		condor_i = true;
 		fallthrough;
 	case BOARD_CONDOR:
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+			 "Renesas Condor%s board", condor_i ? "-I" : "");
 		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);
+		snprintf(priv->boardmodel, sizeof(priv->boardmodel), "%s rev %s",
+			 model, rev);
 		return;
 	default:
 		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
-- 
2.41.0

[PATCH v4 6/6] sysinfo: rcar3: Implement BOARD_ID and BOARD_REV_*

[Detlev Casanova]

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 | 103 +++++++++++++++++++++++++++-------------
 1 file changed, 70 insertions(+), 33 deletions(-)

diff --git a/drivers/sysinfo/rcar3.c b/drivers/sysinfo/rcar3.c
index 450a4c26773..1a99642bf75 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_REVISION_MAJOR:
+		if (!priv->has_rev)
+			return -EINVAL;
+		*val = priv->rev_major;
+		return 0;
+	case SYSINFO_ID_BOARD_REVISION_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)
@@ -70,8 +98,9 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv)
 	bool condor_i = false;
 	char model[64];
 	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:
@@ -81,9 +110,10 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv)
 		snprintf(model, sizeof(model),
 			 "Renesas Salvator-X%s board", salvator_xs ? "S" : "");
 		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), "%s rev %s",
@@ -91,34 +121,37 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv)
 
 		return;
 	case BOARD_STARTER_KIT:
-		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+		snprintf(model, sizeof(model),
 			 "Renesas Starter Kit board");
 		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), "%s rev %s",
 			 model, rev);
 		return;
 	case BOARD_STARTER_KIT_PRE:
-		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+		snprintf(model, sizeof(model),
 			 "Renesas Starter Kit Premier board");
 		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), "%s rev %s",
 			 model, rev);
 		return;
 	case BOARD_EAGLE:
-		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+		snprintf(model, sizeof(model),
 			 "Renesas Eagle board");
 		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), "%s rev %s",
 			 model, rev);
@@ -127,34 +160,37 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv)
 		ebisu_4d = true;
 		fallthrough;
 	case BOARD_EBISU:
-		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+		snprintf(model, sizeof(model),
 			 "Renesas Ebisu%s board", ebisu_4d ? "-4D" : "");
 		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), "%s rev %s",
 			 model, rev);
 		return;
 	case BOARD_DRAAK:
-		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+		snprintf(model, sizeof(model),
 			 "Renesas Draak board");
 		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), "%s rev %s",
 			 model, rev);
 		return;
 	case BOARD_KRIEK:
-		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+		snprintf(model, sizeof(model),
 			 "Renesas Kriek board");
 		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), "%s rev %s",
 			 model, rev);
@@ -163,12 +199,13 @@ static void sysinfo_rcar_parse(struct sysinfo_rcar_priv *priv)
 		condor_i = true;
 		fallthrough;
 	case BOARD_CONDOR:
-		snprintf(priv->boardmodel, sizeof(priv->boardmodel),
+		snprintf(model, sizeof(model),
 			 "Renesas Condor%s board", condor_i ? "-I" : "");
 		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), "%s rev %s",
 			 model, rev);
-- 
2.41.0

Re: [PATCH v4 0/6] Introduce the sysinfo command

[Peter Robinson]

On Thu, Jul 20, 2023 at 3:50 PM Detlev Casanova
<detlev.casanova@collabora.com> wrote:
>
> 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.

How does this differ from the bdinfo and some of the other commands
that provide similar info?

> 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   | 141 ++++++++++++++++++++++++++------------
>  drivers/sysinfo/sandbox.c |  17 +++++
>  include/sysinfo.h         |   5 ++
>  test/cmd/Makefile         |   1 +
>  test/cmd/test_sysinfo.c   |  51 ++++++++++++++
>  10 files changed, 370 insertions(+), 42 deletions(-)
>  create mode 100644 cmd/sysinfo.c
>  create mode 100644 doc/usage/cmd/sysinfo.rst
>  create mode 100644 test/cmd/test_sysinfo.c
>
> --
> 2.41.0
>

Re: [PATCH v4 0/6] Introduce the sysinfo command

[Detlev Casanova]

On Tuesday, August 8, 2023 10:21:11 A.M. EDT Peter Robinson wrote:
> On Thu, Jul 20, 2023 at 3:50 PM Detlev Casanova
> 
> <detlev.casanova@collabora.com> wrote:
> > 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.
> 
> How does this differ from the bdinfo and some of the other commands
> that provide similar info?

If I'm not wrong, the bdinfo command is about showing what hardware is
present on the board. The sysinfo command is about showing board
identification information.
 
> > 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   | 141 ++++++++++++++++++++++++++------------
> >  drivers/sysinfo/sandbox.c |  17 +++++
> >  include/sysinfo.h         |   5 ++
> >  test/cmd/Makefile         |   1 +
> >  test/cmd/test_sysinfo.c   |  51 ++++++++++++++
> >  10 files changed, 370 insertions(+), 42 deletions(-)
> >  create mode 100644 cmd/sysinfo.c
> >  create mode 100644 doc/usage/cmd/sysinfo.rst
> >  create mode 100644 test/cmd/test_sysinfo.c
> > 
> > --
> > 2.41.0

Re: [PATCH v4 0/6] Introduce the sysinfo command

[Peter Robinson]

On Tue, Aug 8, 2023 at 3:27 PM Detlev Casanova
<detlev.casanova@collabora.com> wrote:
>
> On Tuesday, August 8, 2023 10:21:11 A.M. EDT Peter Robinson wrote:
> > On Thu, Jul 20, 2023 at 3:50 PM Detlev Casanova
> >
> > <detlev.casanova@collabora.com> wrote:
> > > 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.
> >
> > How does this differ from the bdinfo and some of the other commands
> > that provide similar info?
>
> If I'm not wrong, the bdinfo command is about showing what hardware is
> present on the board. The sysinfo command is about showing board
> identification information.

Why can't bdinfo be extended rather than adding yet another command?

> > > 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   | 141 ++++++++++++++++++++++++++------------
> > >  drivers/sysinfo/sandbox.c |  17 +++++
> > >  include/sysinfo.h         |   5 ++
> > >  test/cmd/Makefile         |   1 +
> > >  test/cmd/test_sysinfo.c   |  51 ++++++++++++++
> > >  10 files changed, 370 insertions(+), 42 deletions(-)
> > >  create mode 100644 cmd/sysinfo.c
> > >  create mode 100644 doc/usage/cmd/sysinfo.rst
> > >  create mode 100644 test/cmd/test_sysinfo.c
> > >
> > > --
> > > 2.41.0
>
>
>
>

Re: [PATCH v4 0/6] Introduce the sysinfo command

[Marek Vasut]

On 8/8/23 16:31, Peter Robinson wrote:
> On Tue, Aug 8, 2023 at 3:27 PM Detlev Casanova
> <detlev.casanova@collabora.com> wrote:
>>
>> On Tuesday, August 8, 2023 10:21:11 A.M. EDT Peter Robinson wrote:
>>> On Thu, Jul 20, 2023 at 3:50 PM Detlev Casanova
>>>
>>> <detlev.casanova@collabora.com> wrote:
>>>> 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.
>>>
>>> How does this differ from the bdinfo and some of the other commands
>>> that provide similar info?
>>
>> If I'm not wrong, the bdinfo command is about showing what hardware is
>> present on the board. The sysinfo command is about showing board
>> identification information.
> 
> Why can't bdinfo be extended rather than adding yet another command?

It could , but it feels like overloading bdinfo .
Sysinfo is system identification (i.e. stuff in eeprom), bdinfo is 
things like memory layout.