[PATCH 0/5] cmd: add scmi command

[PATCH 0/5] cmd: add scmi command

[AKASHI Takahiro]

"Scmi" command will be re-introduced per Michal's request.
The functionality is the same as I put it in my patch set of adding
SCMI base protocol support, but made some tweak to make UT, "ut dm
scmi_cmd," more flexible and tolerable when enabling/disabling a specific
SCMI protocol for test purpose.

Each commit may have some change history inherited from the preceding
patch series.

Test
====
The patch series was tested on the following platforms:
* sandbox


Prerequisite:
=============
* This patch series is based on the latest master.

AKASHI Takahiro (5):
  test: dm: skip scmi tests against disabled protocols
  firmware: scmi: support protocols on sandbox only if enabled
  cmd: add scmi command for SCMI firmware
  doc: cmd: add documentation for scmi
  test: dm: add scmi command test

 cmd/Kconfig                                  |   9 +
 cmd/Makefile                                 |   1 +
 cmd/scmi.c                                   | 335 +++++++++++++++++++
 configs/sandbox_defconfig                    |   1 +
 doc/usage/cmd/scmi.rst                       | 126 +++++++
 doc/usage/index.rst                          |   1 +
 drivers/firmware/scmi/sandbox-scmi_agent.c   |  27 +-
 drivers/firmware/scmi/sandbox-scmi_devices.c |  78 +++--
 test/dm/scmi.c                               |  93 +++++
 9 files changed, 638 insertions(+), 33 deletions(-)
 create mode 100644 cmd/scmi.c
 create mode 100644 doc/usage/cmd/scmi.rst

-- 
2.34.1

[PATCH 1/5] test: dm: skip scmi tests against disabled protocols

[AKASHI Takahiro]

This is a precautious change to make scmi tests workable whether or not
a specific protocol be enabled.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 test/dm/scmi.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/test/dm/scmi.c b/test/dm/scmi.c
index da45314f2e4c..2f63f2da16fb 100644
--- a/test/dm/scmi.c
+++ b/test/dm/scmi.c
@@ -217,6 +217,9 @@ static int dm_test_scmi_power_domains(struct unit_test_state *uts)
 	u8 *name;
 	int ret;
 
+	if (!IS_ENABLED(CONFIG_SCMI_POWER_DOMAIN))
+		return 0;
+
 	/* preparation */
 	ut_assertok(load_sandbox_scmi_test_devices(uts, &agent, &dev));
 	ut_assertnonnull(agent);
@@ -317,6 +320,9 @@ static int dm_test_scmi_clocks(struct unit_test_state *uts)
 	int ret_dev;
 	int ret;
 
+	if (!IS_ENABLED(CONFIG_CLK_SCMI))
+		return 0;
+
 	ret = load_sandbox_scmi_test_devices(uts, &agent, &dev);
 	if (ret)
 		return ret;
@@ -382,6 +388,9 @@ static int dm_test_scmi_resets(struct unit_test_state *uts)
 	struct udevice *agent_dev, *reset_dev, *dev = NULL;
 	int ret;
 
+	if (!IS_ENABLED(CONFIG_RESET_SCMI))
+		return 0;
+
 	ret = load_sandbox_scmi_test_devices(uts, &agent, &dev);
 	if (ret)
 		return ret;
@@ -418,6 +427,9 @@ static int dm_test_scmi_voltage_domains(struct unit_test_state *uts)
 	struct udevice *dev;
 	struct udevice *regul0_dev;
 
+	if (!IS_ENABLED(CONFIG_DM_REGULATOR_SCMI))
+		return 0;
+
 	ut_assertok(load_sandbox_scmi_test_devices(uts, &agent, &dev));
 
 	scmi_devices = sandbox_scmi_devices_ctx(dev);
-- 
2.34.1

[PATCH 2/5] firmware: scmi: support protocols on sandbox only if enabled

[AKASHI Takahiro]

This change will be useful when we manually test SCMI on sandbox
by enabling/disabling a specific SCMI protocol.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
---
 drivers/firmware/scmi/sandbox-scmi_agent.c   | 27 ++++++-
 drivers/firmware/scmi/sandbox-scmi_devices.c | 78 ++++++++++++--------
 2 files changed, 72 insertions(+), 33 deletions(-)

diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c b/drivers/firmware/scmi/sandbox-scmi_agent.c
index 9f5f497e0a6c..e1b9b5f5f2d8 100644
--- a/drivers/firmware/scmi/sandbox-scmi_agent.c
+++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
@@ -66,10 +66,18 @@ struct scmi_channel {
 };
 
 static u8 protocols[] = {
+#if IS_ENABLED(CONFIG_SCMI_POWER_DOMAIN)
 	SCMI_PROTOCOL_ID_POWER_DOMAIN,
+#endif
+#if IS_ENABLED(CONFIG_CLK_SCMI)
 	SCMI_PROTOCOL_ID_CLOCK,
+#endif
+#if IS_ENABLED(CONFIG_RESET_SCMI)
 	SCMI_PROTOCOL_ID_RESET_DOMAIN,
+#endif
+#if IS_ENABLED(CONFIG_DM_REGULATOR_SCMI)
 	SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN,
+#endif
 };
 
 #define NUM_PROTOCOLS ARRAY_SIZE(protocols)
@@ -1160,6 +1168,9 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev,
 		}
 		break;
 	case SCMI_PROTOCOL_ID_POWER_DOMAIN:
+		if (!IS_ENABLED(CONFIG_SCMI_POWER_DOMAIN))
+			goto not_supported;
+
 		switch (msg->message_id) {
 		case SCMI_PROTOCOL_VERSION:
 			return sandbox_scmi_pwd_protocol_version(dev, msg);
@@ -1180,6 +1191,9 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev,
 		}
 		break;
 	case SCMI_PROTOCOL_ID_CLOCK:
+		if (!IS_ENABLED(CONFIG_CLK_SCMI))
+			goto not_supported;
+
 		switch (msg->message_id) {
 		case SCMI_PROTOCOL_ATTRIBUTES:
 			return sandbox_scmi_clock_protocol_attribs(dev, msg);
@@ -1196,6 +1210,9 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev,
 		}
 		break;
 	case SCMI_PROTOCOL_ID_RESET_DOMAIN:
+		if (!IS_ENABLED(CONFIG_RESET_SCMI))
+			goto not_supported;
+
 		switch (msg->message_id) {
 		case SCMI_RESET_DOMAIN_ATTRIBUTES:
 			return sandbox_scmi_rd_attribs(dev, msg);
@@ -1206,6 +1223,9 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev,
 		}
 		break;
 	case SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN:
+		if (!IS_ENABLED(CONFIG_DM_REGULATOR_SCMI))
+			goto not_supported;
+
 		switch (msg->message_id) {
 		case SCMI_VOLTAGE_DOMAIN_ATTRIBUTES:
 			return sandbox_scmi_voltd_attribs(dev, msg);
@@ -1224,8 +1244,7 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev,
 	case SCMI_PROTOCOL_ID_SYSTEM:
 	case SCMI_PROTOCOL_ID_PERF:
 	case SCMI_PROTOCOL_ID_SENSOR:
-		*(u32 *)msg->out_msg = SCMI_NOT_SUPPORTED;
-		return 0;
+		goto not_supported;
 	default:
 		break;
 	}
@@ -1239,6 +1258,10 @@ static int sandbox_scmi_test_process_msg(struct udevice *dev,
 	/* Intentionnaly report unhandled IDs through the SCMI return code */
 	*(u32 *)msg->out_msg = SCMI_PROTOCOL_ERROR;
 	return 0;
+
+not_supported:
+	*(u32 *)msg->out_msg = SCMI_NOT_SUPPORTED;
+	return 0;
 }
 
 static int sandbox_scmi_test_remove(struct udevice *dev)
diff --git a/drivers/firmware/scmi/sandbox-scmi_devices.c b/drivers/firmware/scmi/sandbox-scmi_devices.c
index facb5b06ffb5..0519cf889aa9 100644
--- a/drivers/firmware/scmi/sandbox-scmi_devices.c
+++ b/drivers/firmware/scmi/sandbox-scmi_devices.c
@@ -62,12 +62,13 @@ static int sandbox_scmi_devices_remove(struct udevice *dev)
 	if (!devices)
 		return 0;
 
-	for (n = 0; n < SCMI_TEST_DEVICES_RD_COUNT; n++) {
-		int ret2 = reset_free(devices->reset + n);
+	if (IS_ENABLED(CONFIG_RESET_SCMI))
+		for (n = 0; n < SCMI_TEST_DEVICES_RD_COUNT; n++) {
+			int ret2 = reset_free(devices->reset + n);
 
-		if (ret2 && !ret)
-			ret = ret2;
-	}
+			if (ret2 && !ret)
+				ret = ret2;
+		}
 
 	return ret;
 }
@@ -89,39 +90,53 @@ static int sandbox_scmi_devices_probe(struct udevice *dev)
 		.regul_count = SCMI_TEST_DEVICES_VOLTD_COUNT,
 	};
 
-	ret = power_domain_get_by_index(dev, priv->devices.pwdom, 0);
-	if (ret) {
-		dev_err(dev, "%s: Failed on power domain\n", __func__);
-		return ret;
-	}
-
-	for (n = 0; n < SCMI_TEST_DEVICES_CLK_COUNT; n++) {
-		ret = clk_get_by_index(dev, n, priv->devices.clk + n);
+	if (IS_ENABLED(CONFIG_SCMI_POWER_DOMAIN)) {
+		ret = power_domain_get_by_index(dev, priv->devices.pwdom, 0);
 		if (ret) {
-			dev_err(dev, "%s: Failed on clk %zu\n", __func__, n);
+			dev_err(dev, "%s: Failed on power domain\n", __func__);
 			return ret;
 		}
 	}
 
-	for (n = 0; n < SCMI_TEST_DEVICES_RD_COUNT; n++) {
-		ret = reset_get_by_index(dev, n, priv->devices.reset + n);
-		if (ret) {
-			dev_err(dev, "%s: Failed on reset %zu\n", __func__, n);
-			goto err_reset;
+	if (IS_ENABLED(CONFIG_CLK_SCMI)) {
+		for (n = 0; n < SCMI_TEST_DEVICES_CLK_COUNT; n++) {
+			ret = clk_get_by_index(dev, n, priv->devices.clk + n);
+			if (ret) {
+				dev_err(dev, "%s: Failed on clk %zu\n",
+					__func__, n);
+				return ret;
+			}
 		}
 	}
 
-	for (n = 0; n < SCMI_TEST_DEVICES_VOLTD_COUNT; n++) {
-		char name[32];
-
-		ret = snprintf(name, sizeof(name), "regul%zu-supply", n);
-		assert(ret >= 0 && ret < sizeof(name));
+	if (IS_ENABLED(CONFIG_RESET_SCMI)) {
+		for (n = 0; n < SCMI_TEST_DEVICES_RD_COUNT; n++) {
+			ret = reset_get_by_index(dev, n,
+						 priv->devices.reset + n);
+			if (ret) {
+				dev_err(dev, "%s: Failed on reset %zu\n",
+					__func__, n);
+				goto err_reset;
+			}
+		}
+	}
 
-		ret = device_get_supply_regulator(dev, name,
-						  priv->devices.regul + n);
-		if (ret) {
-			dev_err(dev, "%s: Failed on voltd %zu\n", __func__, n);
-			goto err_regul;
+	if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI)) {
+		for (n = 0; n < SCMI_TEST_DEVICES_VOLTD_COUNT; n++) {
+			char name[32];
+
+			ret = snprintf(name, sizeof(name), "regul%zu-supply",
+				       n);
+			assert(ret >= 0 && ret < sizeof(name));
+
+			ret = device_get_supply_regulator(dev, name,
+							  priv->devices.regul
+								+ n);
+			if (ret) {
+				dev_err(dev, "%s: Failed on voltd %zu\n",
+					__func__, n);
+				goto err_regul;
+			}
 		}
 	}
 
@@ -130,8 +145,9 @@ static int sandbox_scmi_devices_probe(struct udevice *dev)
 err_regul:
 	n = SCMI_TEST_DEVICES_RD_COUNT;
 err_reset:
-	for (; n > 0; n--)
-		reset_free(priv->devices.reset + n - 1);
+	if (IS_ENABLED(CONFIG_RESET_SCMI))
+		for (; n > 0; n--)
+			reset_free(priv->devices.reset + n - 1);
 
 	return ret;
 }
-- 
2.34.1

[PATCH 3/5] cmd: add scmi command for SCMI firmware

[AKASHI Takahiro]

This command, "scmi", may provide a command line interface to various SCMI
protocols. It supports at least initially SCMI base protocol and is
intended mainly for debug purpose.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
---
v3
* describe that arguments are in hex at a help message
* modify the code for dynamically allocated agent names
v2
* remove sub command category, 'scmi base', for simplicity
---
 cmd/Kconfig  |   9 ++
 cmd/Makefile |   1 +
 cmd/scmi.c   | 335 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 345 insertions(+)
 create mode 100644 cmd/scmi.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 205df2f1fb65..c940051eba91 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2550,6 +2550,15 @@ config CMD_CROS_EC
 	  a number of sub-commands for performing EC tasks such as
 	  updating its flash, accessing a small saved context area
 	  and talking to the I2C bus behind the EC (if there is one).
+
+config CMD_SCMI
+	bool "Enable scmi command"
+	depends on SCMI_FIRMWARE
+	default n
+	help
+	  This command provides user interfaces to several SCMI (System
+	  Control and Management Interface) protocols available on Arm
+	  platforms to manage system resources.
 endmenu
 
 menu "Filesystem commands"
diff --git a/cmd/Makefile b/cmd/Makefile
index 9a6790cc1708..320f0b5266eb 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -159,6 +159,7 @@ obj-$(CONFIG_CMD_SATA) += sata.o
 obj-$(CONFIG_CMD_NVME) += nvme.o
 obj-$(CONFIG_SANDBOX) += sb.o
 obj-$(CONFIG_CMD_SF) += sf.o
+obj-$(CONFIG_CMD_SCMI) += scmi.o
 obj-$(CONFIG_CMD_SCSI) += scsi.o disk.o
 obj-$(CONFIG_CMD_SHA1SUM) += sha1sum.o
 obj-$(CONFIG_CMD_SEAMA) += seama.o
diff --git a/cmd/scmi.c b/cmd/scmi.c
new file mode 100644
index 000000000000..f8f54f84cff8
--- /dev/null
+++ b/cmd/scmi.c
@@ -0,0 +1,335 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  SCMI (System Control and Management Interface) utility command
+ *
+ *  Copyright (c) 2023 Linaro Limited
+ *		Author: AKASHI Takahiro
+ */
+
+#include <command.h>
+#include <exports.h>
+#include <scmi_agent.h>
+#include <scmi_agent-uclass.h>
+#include <stdlib.h>
+#include <asm/types.h>
+#include <dm/device.h>
+#include <dm/uclass.h> /* uclass_get_device */
+#include <linux/bitfield.h>
+#include <linux/bitops.h>
+
+static struct udevice *agent;
+static struct udevice *base_proto;
+
+struct {
+	enum scmi_std_protocol id;
+	const char *name;
+} protocol_name[] = {
+	{SCMI_PROTOCOL_ID_BASE, "Base"},
+	{SCMI_PROTOCOL_ID_POWER_DOMAIN, "Power domain management"},
+	{SCMI_PROTOCOL_ID_SYSTEM, "System power management"},
+	{SCMI_PROTOCOL_ID_PERF, "Performance domain management"},
+	{SCMI_PROTOCOL_ID_CLOCK, "Clock management"},
+	{SCMI_PROTOCOL_ID_SENSOR, "Sensor management"},
+	{SCMI_PROTOCOL_ID_RESET_DOMAIN, "Reset domain management"},
+	{SCMI_PROTOCOL_ID_VOLTAGE_DOMAIN, "Voltage domain management"},
+};
+
+/**
+ * get_proto_name() - get the name of SCMI protocol
+ *
+ * @id:		SCMI Protocol ID
+ *
+ * Get the printable name of the protocol, @id
+ *
+ * Return:	Name string on success, NULL on failure
+ */
+static const char *get_proto_name(enum scmi_std_protocol id)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(protocol_name); i++)
+		if (id == protocol_name[i].id)
+			return protocol_name[i].name;
+
+	return NULL;
+}
+
+/**
+ * do_scmi_info() - get the information of SCMI services
+ *
+ * @cmdtp:	Command table
+ * @flag:	Command flag
+ * @argc:	Number of arguments
+ * @argv:	Argument array
+ *
+ * Get the information of SCMI services using various interfaces
+ * provided by the Base protocol.
+ *
+ * Return:	CMD_RET_SUCCESS on success, CMD_RET_RET_FAILURE on failure
+ */
+static int do_scmi_info(struct cmd_tbl *cmdtp, int flag, int argc,
+			char * const argv[])
+{
+	u32 agent_id, num_protocols;
+	u8 *agent_name, *protocols;
+	int i, ret;
+
+	if (argc != 1)
+		return CMD_RET_USAGE;
+
+	printf("SCMI device: %s\n", agent->name);
+	printf("  protocol version: 0x%x\n", scmi_version(agent));
+	printf("  # of agents: %d\n", scmi_num_agents(agent));
+	for (i = 0; i < scmi_num_agents(agent); i++) {
+		ret = scmi_base_discover_agent(base_proto, i, &agent_id,
+					       &agent_name);
+		if (ret) {
+			if (ret != -EOPNOTSUPP)
+				printf("base_discover_agent() failed for id: %d (%d)\n",
+				       i, ret);
+			break;
+		}
+		printf("    %c%2d: %s\n", i == scmi_agent_id(agent) ? '>' : ' ',
+		       i, agent_name);
+		free(agent_name);
+	}
+	printf("  # of protocols: %d\n", scmi_num_protocols(agent));
+	num_protocols = scmi_num_protocols(agent);
+	protocols = scmi_protocols(agent);
+	if (protocols)
+		for (i = 0; i < num_protocols; i++)
+			printf("      %s\n", get_proto_name(protocols[i]));
+	printf("  vendor: %s\n", scmi_vendor(agent));
+	printf("  sub vendor: %s\n", scmi_sub_vendor(agent));
+	printf("  impl version: 0x%x\n", scmi_impl_version(agent));
+
+	return CMD_RET_SUCCESS;
+}
+
+/**
+ * do_scmi_set_dev() - set access permission to device
+ *
+ * @cmdtp:	Command table
+ * @flag:	Command flag
+ * @argc:	Number of arguments
+ * @argv:	Argument array
+ *
+ * Set access permission to device with SCMI_BASE_SET_DEVICE_PERMISSIONS
+ *
+ * Return:	CMD_RET_SUCCESS on success, CMD_RET_RET_FAILURE on failure
+ */
+static int do_scmi_set_dev(struct cmd_tbl *cmdtp, int flag, int argc,
+			   char * const argv[])
+{
+	u32 agent_id, device_id, flags, attributes;
+	char *end;
+	int ret;
+
+	if (argc != 4)
+		return CMD_RET_USAGE;
+
+	agent_id = simple_strtoul(argv[1], &end, 16);
+	if (*end != '\0')
+		return CMD_RET_USAGE;
+
+	device_id = simple_strtoul(argv[2], &end, 16);
+	if (*end != '\0')
+		return CMD_RET_USAGE;
+
+	flags = simple_strtoul(argv[3], &end, 16);
+	if (*end != '\0')
+		return CMD_RET_USAGE;
+
+	ret = scmi_base_protocol_message_attrs(base_proto,
+					       SCMI_BASE_SET_DEVICE_PERMISSIONS,
+					       &attributes);
+	if (ret) {
+		printf("This operation is not supported\n");
+		return CMD_RET_FAILURE;
+	}
+
+	ret = scmi_base_set_device_permissions(base_proto, agent_id,
+					       device_id, flags);
+	if (ret) {
+		printf("%s access to device:%u failed (%d)\n",
+		       flags ? "Allowing" : "Denying", device_id, ret);
+		return CMD_RET_FAILURE;
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+/**
+ * do_scmi_set_proto() - set protocol permission to device
+ *
+ * @cmdtp:	Command table
+ * @flag:	Command flag
+ * @argc:	Number of arguments
+ * @argv:	Argument array
+ *
+ * Set protocol permission to device with SCMI_BASE_SET_PROTOCOL_PERMISSIONS
+ *
+ * Return:	CMD_RET_SUCCESS on success, CMD_RET_RET_FAILURE on failure
+ */
+static int do_scmi_set_proto(struct cmd_tbl *cmdtp, int flag, int argc,
+			     char * const argv[])
+{
+	u32 agent_id, device_id, protocol_id, flags, attributes;
+	char *end;
+	int ret;
+
+	if (argc != 5)
+		return CMD_RET_USAGE;
+
+	agent_id = simple_strtoul(argv[1], &end, 16);
+	if (*end != '\0')
+		return CMD_RET_USAGE;
+
+	device_id = simple_strtoul(argv[2], &end, 16);
+	if (*end != '\0')
+		return CMD_RET_USAGE;
+
+	protocol_id = simple_strtoul(argv[3], &end, 16);
+	if (*end != '\0')
+		return CMD_RET_USAGE;
+
+	flags = simple_strtoul(argv[4], &end, 16);
+	if (*end != '\0')
+		return CMD_RET_USAGE;
+
+	ret = scmi_base_protocol_message_attrs(base_proto,
+					       SCMI_BASE_SET_PROTOCOL_PERMISSIONS,
+					       &attributes);
+	if (ret) {
+		printf("This operation is not supported\n");
+		return CMD_RET_FAILURE;
+	}
+
+	ret = scmi_base_set_protocol_permissions(base_proto, agent_id,
+						 device_id, protocol_id,
+						 flags);
+	if (ret) {
+		printf("%s access to protocol:0x%x on device:%u failed (%d)\n",
+		       flags ? "Allowing" : "Denying", protocol_id, device_id,
+		       ret);
+		return CMD_RET_FAILURE;
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+/**
+ * do_scmi_reset() - reset platform resource settings
+ *
+ * @cmdtp:	Command table
+ * @flag:	Command flag
+ * @argc:	Number of arguments
+ * @argv:	Argument array
+ *
+ * Reset platform resource settings with BASE_RESET_AGENT_CONFIGURATION
+ *
+ * Return:	CMD_RET_SUCCESS on success, CMD_RET_RET_FAILURE on failure
+ */
+static int do_scmi_reset(struct cmd_tbl *cmdtp, int flag, int argc,
+			 char * const argv[])
+{
+	u32 agent_id, flags, attributes;
+	char *end;
+	int ret;
+
+	if (argc != 3)
+		return CMD_RET_USAGE;
+
+	agent_id = simple_strtoul(argv[1], &end, 16);
+	if (*end != '\0')
+		return CMD_RET_USAGE;
+
+	flags = simple_strtoul(argv[2], &end, 16);
+	if (*end != '\0')
+		return CMD_RET_USAGE;
+
+	ret = scmi_base_protocol_message_attrs(base_proto,
+					       SCMI_BASE_RESET_AGENT_CONFIGURATION,
+					       &attributes);
+	if (ret) {
+		printf("Reset is not supported\n");
+		return CMD_RET_FAILURE;
+	}
+
+	ret = scmi_base_reset_agent_configuration(base_proto, agent_id, flags);
+	if (ret) {
+		printf("Reset failed (%d)\n", ret);
+		return CMD_RET_FAILURE;
+	}
+
+	return CMD_RET_SUCCESS;
+}
+
+static struct cmd_tbl cmd_scmi_sub[] = {
+	U_BOOT_CMD_MKENT(info, CONFIG_SYS_MAXARGS, 1,
+			 do_scmi_info, "", ""),
+	U_BOOT_CMD_MKENT(perm_dev, CONFIG_SYS_MAXARGS, 1,
+			 do_scmi_set_dev, "", ""),
+	U_BOOT_CMD_MKENT(perm_proto, CONFIG_SYS_MAXARGS, 1,
+			 do_scmi_set_proto, "", ""),
+	U_BOOT_CMD_MKENT(reset, CONFIG_SYS_MAXARGS, 1,
+			 do_scmi_reset, "", ""),
+};
+
+/**
+ * do_scmi() - SCMI utility
+ *
+ * @cmdtp:	Command table
+ * @flag:	Command flag
+ * @argc:	Number of arguments
+ * @argv:	Argument array
+ *
+ * Provide user interfaces to SCMI protocols.
+ *
+ * Return:	CMD_RET_SUCCESS on success,
+ *		CMD_RET_USAGE or CMD_RET_RET_FAILURE on failure
+ */
+static int do_scmi(struct cmd_tbl *cmdtp, int flag,
+		   int argc, char *const argv[])
+{
+	struct cmd_tbl *cp;
+
+	if (argc < 2)
+		return CMD_RET_USAGE;
+
+	argc--; argv++;
+
+	if (!agent) {
+		if (uclass_get_device(UCLASS_SCMI_AGENT, 0, &agent)) {
+			printf("Cannot find any SCMI agent\n");
+			return CMD_RET_FAILURE;
+		}
+	}
+
+	if (!base_proto)
+		base_proto = scmi_get_protocol(agent, SCMI_PROTOCOL_ID_BASE);
+	if (!base_proto) {
+		printf("SCMI base protocol not found\n");
+		return CMD_RET_FAILURE;
+	}
+
+	cp = find_cmd_tbl(argv[0], cmd_scmi_sub, ARRAY_SIZE(cmd_scmi_sub));
+	if (!cp)
+		return CMD_RET_USAGE;
+
+	return cp->cmd(cmdtp, flag, argc, argv);
+}
+
+static char scmi_help_text[] =
+	" - SCMI utility\n"
+	" info - get the info of SCMI services\n"
+	" perm_dev <agent-id in hex> <device-id in hex> <flags in hex>\n"
+	"   - set access permission to device\n"
+	" perm_proto <agent-id in hex> <device-id in hex> <protocol-id in hex> <flags in hex>\n"
+	"   - set protocol permission to device\n"
+	" reset <agent-id in hex> <flags in hex>\n"
+	"   - reset platform resource settings\n"
+	"";
+
+U_BOOT_CMD(scmi, CONFIG_SYS_MAXARGS, 0, do_scmi, "SCMI utility",
+	   scmi_help_text);
-- 
2.34.1

Re: [PATCH 3/5] cmd: add scmi command for SCMI firmware

[Michal Simek]

On 10/25/23 07:14, AKASHI Takahiro wrote:
> This command, "scmi", may provide a command line interface to various SCMI
> protocols. It supports at least initially SCMI base protocol and is
> intended mainly for debug purpose.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
> ---
> v3
> * describe that arguments are in hex at a help message
> * modify the code for dynamically allocated agent names
> v2
> * remove sub command category, 'scmi base', for simplicity
> ---
>   cmd/Kconfig  |   9 ++
>   cmd/Makefile |   1 +
>   cmd/scmi.c   | 335 +++++++++++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 345 insertions(+)
>   create mode 100644 cmd/scmi.c
> 
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 205df2f1fb65..c940051eba91 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -2550,6 +2550,15 @@ config CMD_CROS_EC
>   	  a number of sub-commands for performing EC tasks such as
>   	  updating its flash, accessing a small saved context area
>   	  and talking to the I2C bus behind the EC (if there is one).
> +
> +config CMD_SCMI
> +	bool "Enable scmi command"
> +	depends on SCMI_FIRMWARE
> +	default n

As I said in previous email. default n should be removed.
It is default option.

You can take a look at least on this commit.

commit b4c2c151b14b59a2403675526adf666710cade67
Author:     Michal Simek <michal.simek@amd.com>
AuthorDate: Fri Aug 27 08:48:10 2021 +0200
Commit:     Tom Rini <trini@konsulko.com>
CommitDate: Tue Aug 31 17:47:49 2021 -0400

     Kconfig: Remove all default n/no options

     default n/no doesn't need to be specified. It is default option anyway.

     Signed-off-by: Michal Simek <michal.simek@xilinx.com>
     [trini: Rework FSP_USE_UPD portion]
     Signed-off-by: Tom Rini <trini@konsulko.com>

I have also sent one more resync patch and CC you there too.

Thanks,
Michal

Re: [PATCH 3/5] cmd: add scmi command for SCMI firmware

[AKASHI Takahiro]

Hi Michal,

On Wed, Oct 25, 2023 at 09:29:04AM +0200, Michal Simek wrote:
> 
> 
> On 10/25/23 07:14, AKASHI Takahiro wrote:
> > This command, "scmi", may provide a command line interface to various SCMI
> > protocols. It supports at least initially SCMI base protocol and is
> > intended mainly for debug purpose.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
> > ---
> > v3
> > * describe that arguments are in hex at a help message
> > * modify the code for dynamically allocated agent names
> > v2
> > * remove sub command category, 'scmi base', for simplicity
> > ---
> >   cmd/Kconfig  |   9 ++
> >   cmd/Makefile |   1 +
> >   cmd/scmi.c   | 335 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >   3 files changed, 345 insertions(+)
> >   create mode 100644 cmd/scmi.c
> > 
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index 205df2f1fb65..c940051eba91 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -2550,6 +2550,15 @@ config CMD_CROS_EC
> >   	  a number of sub-commands for performing EC tasks such as
> >   	  updating its flash, accessing a small saved context area
> >   	  and talking to the I2C bus behind the EC (if there is one).
> > +
> > +config CMD_SCMI
> > +	bool "Enable scmi command"
> > +	depends on SCMI_FIRMWARE
> > +	default n
> 
> As I said in previous email. default n should be removed.
> It is default option.

I see.
I hope that Tom will take care of that when merging if there is no
other comment.

-Takahiro Akashi

> You can take a look at least on this commit.
> 
> commit b4c2c151b14b59a2403675526adf666710cade67
> Author:     Michal Simek <michal.simek@amd.com>
> AuthorDate: Fri Aug 27 08:48:10 2021 +0200
> Commit:     Tom Rini <trini@konsulko.com>
> CommitDate: Tue Aug 31 17:47:49 2021 -0400
> 
>     Kconfig: Remove all default n/no options
> 
>     default n/no doesn't need to be specified. It is default option anyway.
> 
>     Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>     [trini: Rework FSP_USE_UPD portion]
>     Signed-off-by: Tom Rini <trini@konsulko.com>
> 
> I have also sent one more resync patch and CC you there too.
> 
> Thanks,
> Michal
> 
> 
> 
> 

[PATCH 4/5] doc: cmd: add documentation for scmi

[AKASHI Takahiro]

This is a help text for scmi command.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
---
v6
* add the manual to doc/usage/index.rst
v4
* s/tranport/transport/
v2
* add more descriptions about SCMI
---
 doc/usage/cmd/scmi.rst | 126 +++++++++++++++++++++++++++++++++++++++++
 doc/usage/index.rst    |   1 +
 2 files changed, 127 insertions(+)
 create mode 100644 doc/usage/cmd/scmi.rst

diff --git a/doc/usage/cmd/scmi.rst b/doc/usage/cmd/scmi.rst
new file mode 100644
index 000000000000..9ea7e0e41dad
--- /dev/null
+++ b/doc/usage/cmd/scmi.rst
@@ -0,0 +1,126 @@
+.. SPDX-License-Identifier: GPL-2.0+:
+
+scmi command
+============
+
+Synopsis
+--------
+
+::
+
+    scmi info
+    scmi perm_dev <agent id> <device id> <flags>
+    scmi perm_proto <agent id> <device id> <protocol id> <flags>
+    scmi reset <agent id> <flags>
+
+Description
+-----------
+
+Arm System Control and Management Interface (SCMI hereafter) is a set of
+standardised interfaces to manage system resources, like clocks, power
+domains, pin controls, reset and so on, in a system-wide manner.
+
+An entity which provides those services is called a SCMI firmware (or
+SCMI server if you like) may be placed/implemented by EL3 software or
+by a dedicated system control processor (SCP) or else.
+
+A user of SCMI interfaces, including U-Boot, is called a SCMI agent and
+may issues commands, which are defined in each protocol for specific system
+resources, to SCMI server via a communication channel, called a transport.
+Those interfaces are independent from the server's implementation thanks to
+a transport layer.
+
+For more details, see the `SCMI specification`_.
+
+While most of system resources managed under SCMI protocols are implemented
+and handled as standard U-Boot devices, for example clk_scmi, scmi command
+provides additional management functionality against SCMI server.
+
+scmi info
+~~~~~~~~~
+    Show base information about SCMI server and supported protocols
+
+scmi perm_dev
+~~~~~~~~~~~~~
+    Allow or deny access permission to the device
+
+scmi perm_proto
+~~~~~~~~~~~~~~~
+    Allow or deny access to the protocol on the device
+
+scmi reset
+~~~~~~~~~~
+    Reset the already-configured permissions against the device
+
+Parameters are used as follows:
+
+<agent id>
+    SCMI Agent ID, hex value
+
+<device id>
+    SCMI Device ID, hex value
+
+    Please note that what a device means is not defined
+    in the specification.
+
+<protocol id>
+    SCMI Protocol ID, hex value
+
+    It must not be 0x10 (base protocol)
+
+<flags>
+    Flags to control the action, hex value
+
+    0 to deny, 1 to allow. The other values are reserved and allowed
+    values may depend on the implemented version of SCMI server in
+    the future. See SCMI specification for more details.
+
+Example
+-------
+
+Obtain basic information about SCMI server:
+
+::
+
+    => scmi info
+    SCMI device: scmi
+      protocol version: 0x20000
+      # of agents: 3
+          0: platform
+        > 1: OSPM
+          2: PSCI
+      # of protocols: 4
+          Power domain management
+          Performance domain management
+          Clock management
+          Sensor management
+      vendor: Linaro
+      sub vendor: PMWG
+      impl version: 0x20b0000
+
+Ask for access permission to device#0:
+
+::
+
+    => scmi perm_dev 1 0 1
+
+Reset configurations with all access permission settings retained:
+
+::
+
+    => scmi reset 1 0
+
+Configuration
+-------------
+
+The scmi command is only available if CONFIG_CMD_SCMI=y.
+Default n because this command is mainly for debug purpose.
+
+Return value
+------------
+
+The return value ($?) is set to 0 if the operation succeeded,
+1 if the operation failed or -1 if the operation failed due to
+a syntax error.
+
+.. _`SCMI specification`: https://developer.arm.com/documentation/den0056/e/?lang=en
diff --git a/doc/usage/index.rst b/doc/usage/index.rst
index 98b4719c4088..9a65b50aeed7 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -94,6 +94,7 @@ Shell commands
    cmd/saves
    cmd/sbi
    cmd/sf
+   cmd/scmi
    cmd/scp03
    cmd/seama
    cmd/setexpr
-- 
2.34.1

[PATCH 5/5] test: dm: add scmi command test

[AKASHI Takahiro]

In this test, "scmi" command is tested against different sub-commands.
Please note that scmi command is for debug purpose and is not intended
in production system.

Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
Reviewed-by: Simon Glass <sjg@chromium.org>
Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
---
v7
* make test assertions more flexible depending on the number of provided
  protocols
v4
* move 'base'-related changes to the prior commit
* add CONFIG_CMD_SCMI to sandbox_defconfig
v3
* change char to u8 in vendor/agent names
v2
* use helper functions, removing direct uses of ops
---
 configs/sandbox_defconfig |  1 +
 test/dm/scmi.c            | 81 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+)

diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
index e3a2f9eb1708..34e3cc281d7f 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -120,6 +120,7 @@ CONFIG_CMD_REGULATOR=y
 CONFIG_CMD_AES=y
 CONFIG_CMD_TPM=y
 CONFIG_CMD_TPM_TEST=y
+CONFIG_CMD_SCMI=y
 CONFIG_CMD_BTRFS=y
 CONFIG_CMD_CBFS=y
 CONFIG_CMD_CRAMFS=y
diff --git a/test/dm/scmi.c b/test/dm/scmi.c
index 2f63f2da16fb..2bcf7ac6fcc3 100644
--- a/test/dm/scmi.c
+++ b/test/dm/scmi.c
@@ -19,6 +19,7 @@
 #include <scmi_agent.h>
 #include <scmi_agent-uclass.h>
 #include <scmi_protocols.h>
+#include <vsprintf.h>
 #include <asm/scmi_test.h>
 #include <dm/device-internal.h>
 #include <dm/test.h>
@@ -206,6 +207,86 @@ static int dm_test_scmi_base(struct unit_test_state *uts)
 
 DM_TEST(dm_test_scmi_base, UT_TESTF_SCAN_FDT);
 
+static int dm_test_scmi_cmd(struct unit_test_state *uts)
+{
+	struct udevice *agent_dev;
+	int num_proto = 0;
+	char cmd_out[30];
+
+	if (!IS_ENABLED(CONFIG_CMD_SCMI))
+		return 0;
+
+	/* preparation */
+	ut_assertok(uclass_get_device_by_name(UCLASS_SCMI_AGENT, "scmi",
+					      &agent_dev));
+	ut_assertnonnull(agent_dev);
+
+	/*
+	 * Estimate the number of provided protocols.
+	 * This estimation is correct as far as a corresponding
+	 * protocol support is added to sandbox fake serer.
+	 */
+	if (IS_ENABLED(CONFIG_SCMI_POWER_DOMAIN))
+		num_proto++;
+	if (IS_ENABLED(CONFIG_CLK_SCMI))
+		num_proto++;
+	if (IS_ENABLED(CONFIG_RESET_SCMI))
+		num_proto++;
+	if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI))
+		num_proto++;
+
+	/* scmi info */
+	ut_assertok(run_command("scmi info", 0));
+
+	ut_assert_nextline("SCMI device: scmi");
+	snprintf(cmd_out, 30, "  protocol version: 0x%x",
+		 SCMI_BASE_PROTOCOL_VERSION);
+	ut_assert_nextline(cmd_out);
+	ut_assert_nextline("  # of agents: 2");
+	ut_assert_nextline("      0: platform");
+	ut_assert_nextline("    > 1: OSPM");
+	snprintf(cmd_out, 30, "  # of protocols: %d", num_proto);
+	ut_assert_nextline(cmd_out);
+	if (IS_ENABLED(CONFIG_SCMI_POWER_DOMAIN))
+		ut_assert_nextline("      Power domain management");
+	if (IS_ENABLED(CONFIG_CLK_SCMI))
+		ut_assert_nextline("      Clock management");
+	if (IS_ENABLED(CONFIG_RESET_SCMI))
+		ut_assert_nextline("      Reset domain management");
+	if (IS_ENABLED(CONFIG_DM_REGULATOR_SCMI))
+		ut_assert_nextline("      Voltage domain management");
+	ut_assert_nextline("  vendor: U-Boot");
+	ut_assert_nextline("  sub vendor: Sandbox");
+	ut_assert_nextline("  impl version: 0x1");
+
+	ut_assert_console_end();
+
+	/* scmi perm_dev */
+	ut_assertok(run_command("scmi perm_dev 1 0 1", 0));
+	ut_assert_console_end();
+
+	ut_assert(run_command("scmi perm_dev 1 0 0", 0));
+	ut_assert_nextline("Denying access to device:0 failed (-13)");
+	ut_assert_console_end();
+
+	/* scmi perm_proto */
+	ut_assertok(run_command("scmi perm_proto 1 0 14 1", 0));
+	ut_assert_console_end();
+
+	ut_assert(run_command("scmi perm_proto 1 0 14 0", 0));
+	ut_assert_nextline("Denying access to protocol:0x14 on device:0 failed (-13)");
+	ut_assert_console_end();
+
+	/* scmi reset */
+	ut_assert(run_command("scmi reset 1 1", 0));
+	ut_assert_nextline("Reset failed (-13)");
+	ut_assert_console_end();
+
+	return 0;
+}
+
+DM_TEST(dm_test_scmi_cmd, UT_TESTF_SCAN_FDT);
+
 static int dm_test_scmi_power_domains(struct unit_test_state *uts)
 {
 	struct sandbox_scmi_agent *agent;
-- 
2.34.1

Re: [PATCH 5/5] test: dm: add scmi command test

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2216 bytes --]

On Wed, Oct 25, 2023 at 02:14:27PM +0900, AKASHI Takahiro wrote:

> In this test, "scmi" command is tested against different sub-commands.
> Please note that scmi command is for debug purpose and is not intended
> in production system.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> Reviewed-by: Simon Glass <sjg@chromium.org>
> Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>

The test part of this still fails:
https://source.denx.de/u-boot/u-boot/-/jobs/732077

I don't know why more output wasn't captured, when I run it locally
instead I get:
========================================== FAILURES ===========================================
_______________________________ test_ut[ut_dm_dm_test_scmi_cmd] _______________________________
test/py/u_boot_spawn.py:195: in expect
    c = os.read(self.fd, 1024).decode(errors='replace')
E   OSError: [Errno 5] Input/output error

During handling of the above exception, another exception occurred:
test/py/tests/test_ut.py:502: in test_ut
    output = u_boot_console.run_command('ut ' + ut_subtest)
test/py/u_boot_console_base.py:266: in run_command
    m = self.p.expect([self.prompt_compiled] + self.bad_patterns)
test/py/u_boot_spawn.py:204: in expect
    raise ValueError('U-Boot exited with %s' % info)
E   ValueError: U-Boot exited with signal 11 (SIGSEGV)
------------------------------------ Captured stdout call -------------------------------------
=> ut dm dm_test_scmi_cmd
Test: dm_test_scmi_cmd: scmi.c
SCMI device: scmi
  protocol version: 0x20000
  # of agents: 2
      0: platform
    > 1: OSPM
  # of protocols: 4
      Power domain management
      Clock management
      Reset domain management
      Voltage domain management
  vendor: U-Boot
  sub vendor: Sandbox
  impl version: 0x1
Denying access to device:0 failed (-13)
Denying access to protocol:0x14 on device:0 failed (-13)
Reset failed (-13)
Test: dm_test_scmi_cmd: scmi.c (flat tree)
SCMI device: Q
=================================== short test summary info ===================================
FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_scmi_cmd] - ValueError: U-Boot exited...

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH 5/5] test: dm: add scmi command test

[AKASHI Takahiro]

[-- Attachment #1: Type: text/plain, Size: 2762 bytes --]

Hi Tom,

On Fri, Nov 10, 2023 at 01:21:37PM -0500, Tom Rini wrote:
> On Wed, Oct 25, 2023 at 02:14:27PM +0900, AKASHI Takahiro wrote:
> 
> > In this test, "scmi" command is tested against different sub-commands.
> > Please note that scmi command is for debug purpose and is not intended
> > in production system.
> > 
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > Reviewed-by: Simon Glass <sjg@chromium.org>
> > Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
> 
> The test part of this still fails:
> https://source.denx.de/u-boot/u-boot/-/jobs/732077
> 
> I don't know why more output wasn't captured, when I run it locally
> instead I get:
> ========================================== FAILURES ===========================================
> _______________________________ test_ut[ut_dm_dm_test_scmi_cmd] _______________________________
> test/py/u_boot_spawn.py:195: in expect
>     c = os.read(self.fd, 1024).decode(errors='replace')
> E   OSError: [Errno 5] Input/output error
> 
> During handling of the above exception, another exception occurred:
> test/py/tests/test_ut.py:502: in test_ut
>     output = u_boot_console.run_command('ut ' + ut_subtest)
> test/py/u_boot_console_base.py:266: in run_command
>     m = self.p.expect([self.prompt_compiled] + self.bad_patterns)
> test/py/u_boot_spawn.py:204: in expect
>     raise ValueError('U-Boot exited with %s' % info)
> E   ValueError: U-Boot exited with signal 11 (SIGSEGV)


The command uses global variables which hold pointers to 'struct udevice'
which are to be shared between the main and the sub-commands.
Since pytest framework executes ut tests twice, once with a (normal?) device
tree and once with a flat tree,  udevices will be *voided* between
two executions.

I will fix it in v2.

Thanks,
-Takahiro Akashi


> ------------------------------------ Captured stdout call -------------------------------------
> => ut dm dm_test_scmi_cmd
> Test: dm_test_scmi_cmd: scmi.c
> SCMI device: scmi
>   protocol version: 0x20000
>   # of agents: 2
>       0: platform
>     > 1: OSPM
>   # of protocols: 4
>       Power domain management
>       Clock management
>       Reset domain management
>       Voltage domain management
>   vendor: U-Boot
>   sub vendor: Sandbox
>   impl version: 0x1
> Denying access to device:0 failed (-13)
> Denying access to protocol:0x14 on device:0 failed (-13)
> Reset failed (-13)
> Test: dm_test_scmi_cmd: scmi.c (flat tree)
> SCMI device: Q
> =================================== short test summary info ===================================
> FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_scmi_cmd] - ValueError: U-Boot exited...
> 
> -- 
> Tom



[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH 5/5] test: dm: add scmi command test

[Simon Glass]

Hi,

On Sun, 12 Nov 2023 at 18:46, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> Hi Tom,
>
> On Fri, Nov 10, 2023 at 01:21:37PM -0500, Tom Rini wrote:
> > On Wed, Oct 25, 2023 at 02:14:27PM +0900, AKASHI Takahiro wrote:
> >
> > > In this test, "scmi" command is tested against different sub-commands.
> > > Please note that scmi command is for debug purpose and is not intended
> > > in production system.
> > >
> > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
> >
> > The test part of this still fails:
> > https://source.denx.de/u-boot/u-boot/-/jobs/732077
> >
> > I don't know why more output wasn't captured, when I run it locally
> > instead I get:
> > ========================================== FAILURES ===========================================
> > _______________________________ test_ut[ut_dm_dm_test_scmi_cmd] _______________________________
> > test/py/u_boot_spawn.py:195: in expect
> >     c = os.read(self.fd, 1024).decode(errors='replace')
> > E   OSError: [Errno 5] Input/output error
> >
> > During handling of the above exception, another exception occurred:
> > test/py/tests/test_ut.py:502: in test_ut
> >     output = u_boot_console.run_command('ut ' + ut_subtest)
> > test/py/u_boot_console_base.py:266: in run_command
> >     m = self.p.expect([self.prompt_compiled] + self.bad_patterns)
> > test/py/u_boot_spawn.py:204: in expect
> >     raise ValueError('U-Boot exited with %s' % info)
> > E   ValueError: U-Boot exited with signal 11 (SIGSEGV)
>
>
> The command uses global variables which hold pointers to 'struct udevice'
> which are to be shared between the main and the sub-commands.
> Since pytest framework executes ut tests twice, once with a (normal?) device
> tree and once with a flat tree,  udevices will be *voided* between
> two executions.

Are you able to put the var in the uclass-priv data instead? The state
should be cleared before running each DM test.

Regards,
Simon


>
> I will fix it in v2.
>
> Thanks,
> -Takahiro Akashi
>
>
> > ------------------------------------ Captured stdout call -------------------------------------
> > => ut dm dm_test_scmi_cmd
> > Test: dm_test_scmi_cmd: scmi.c
> > SCMI device: scmi
> >   protocol version: 0x20000
> >   # of agents: 2
> >       0: platform
> >     > 1: OSPM
> >   # of protocols: 4
> >       Power domain management
> >       Clock management
> >       Reset domain management
> >       Voltage domain management
> >   vendor: U-Boot
> >   sub vendor: Sandbox
> >   impl version: 0x1
> > Denying access to device:0 failed (-13)
> > Denying access to protocol:0x14 on device:0 failed (-13)
> > Reset failed (-13)
> > Test: dm_test_scmi_cmd: scmi.c (flat tree)
> > SCMI device: Q
> > =================================== short test summary info ===================================
> > FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_scmi_cmd] - ValueError: U-Boot exited...
> >
> > --
> > Tom
>
>

Re: [PATCH 5/5] test: dm: add scmi command test

[AKASHI Takahiro]

On Mon, Nov 13, 2023 at 11:01:17AM -0700, Simon Glass wrote:
> Hi,
> 
> On Sun, 12 Nov 2023 at 18:46, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > Hi Tom,
> >
> > On Fri, Nov 10, 2023 at 01:21:37PM -0500, Tom Rini wrote:
> > > On Wed, Oct 25, 2023 at 02:14:27PM +0900, AKASHI Takahiro wrote:
> > >
> > > > In this test, "scmi" command is tested against different sub-commands.
> > > > Please note that scmi command is for debug purpose and is not intended
> > > > in production system.
> > > >
> > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > > Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
> > >
> > > The test part of this still fails:
> > > https://source.denx.de/u-boot/u-boot/-/jobs/732077
> > >
> > > I don't know why more output wasn't captured, when I run it locally
> > > instead I get:
> > > ========================================== FAILURES ===========================================
> > > _______________________________ test_ut[ut_dm_dm_test_scmi_cmd] _______________________________
> > > test/py/u_boot_spawn.py:195: in expect
> > >     c = os.read(self.fd, 1024).decode(errors='replace')
> > > E   OSError: [Errno 5] Input/output error
> > >
> > > During handling of the above exception, another exception occurred:
> > > test/py/tests/test_ut.py:502: in test_ut
> > >     output = u_boot_console.run_command('ut ' + ut_subtest)
> > > test/py/u_boot_console_base.py:266: in run_command
> > >     m = self.p.expect([self.prompt_compiled] + self.bad_patterns)
> > > test/py/u_boot_spawn.py:204: in expect
> > >     raise ValueError('U-Boot exited with %s' % info)
> > > E   ValueError: U-Boot exited with signal 11 (SIGSEGV)
> >
> >
> > The command uses global variables which hold pointers to 'struct udevice'
> > which are to be shared between the main and the sub-commands.
> > Since pytest framework executes ut tests twice, once with a (normal?) device
> > tree and once with a flat tree,  udevices will be *voided* between
> > two executions.
> 
> Are you able to put the var in the uclass-priv data instead? The state
> should be cleared before running each DM test.

Well, I don't think we need such a trick.
As you can see, we may simply fetch/find necessary udevices
every time the command is called.
It is enough given that the command is mainly for debug purpose.

-Takahiro Akashi


> Regards,
> Simon
> 
> 
> >
> > I will fix it in v2.
> >
> > Thanks,
> > -Takahiro Akashi
> >
> >
> > > ------------------------------------ Captured stdout call -------------------------------------
> > > => ut dm dm_test_scmi_cmd
> > > Test: dm_test_scmi_cmd: scmi.c
> > > SCMI device: scmi
> > >   protocol version: 0x20000
> > >   # of agents: 2
> > >       0: platform
> > >     > 1: OSPM
> > >   # of protocols: 4
> > >       Power domain management
> > >       Clock management
> > >       Reset domain management
> > >       Voltage domain management
> > >   vendor: U-Boot
> > >   sub vendor: Sandbox
> > >   impl version: 0x1
> > > Denying access to device:0 failed (-13)
> > > Denying access to protocol:0x14 on device:0 failed (-13)
> > > Reset failed (-13)
> > > Test: dm_test_scmi_cmd: scmi.c (flat tree)
> > > SCMI device: Q
> > > =================================== short test summary info ===================================
> > > FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_scmi_cmd] - ValueError: U-Boot exited...
> > >
> > > --
> > > Tom
> >
> >

Re: [PATCH 5/5] test: dm: add scmi command test

[Simon Glass]

Hi,

On Mon, 13 Nov 2023 at 18:41, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> On Mon, Nov 13, 2023 at 11:01:17AM -0700, Simon Glass wrote:
> > Hi,
> >
> > On Sun, 12 Nov 2023 at 18:46, AKASHI Takahiro
> > <takahiro.akashi@linaro.org> wrote:
> > >
> > > Hi Tom,
> > >
> > > On Fri, Nov 10, 2023 at 01:21:37PM -0500, Tom Rini wrote:
> > > > On Wed, Oct 25, 2023 at 02:14:27PM +0900, AKASHI Takahiro wrote:
> > > >
> > > > > In this test, "scmi" command is tested against different sub-commands.
> > > > > Please note that scmi command is for debug purpose and is not intended
> > > > > in production system.
> > > > >
> > > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > > > Reviewed-by: Simon Glass <sjg@chromium.org>
> > > > > Reviewed-by: Etienne Carriere <etienne.carriere@foss.st.com>
> > > >
> > > > The test part of this still fails:
> > > > https://source.denx.de/u-boot/u-boot/-/jobs/732077
> > > >
> > > > I don't know why more output wasn't captured, when I run it locally
> > > > instead I get:
> > > > ========================================== FAILURES ===========================================
> > > > _______________________________ test_ut[ut_dm_dm_test_scmi_cmd] _______________________________
> > > > test/py/u_boot_spawn.py:195: in expect
> > > >     c = os.read(self.fd, 1024).decode(errors='replace')
> > > > E   OSError: [Errno 5] Input/output error
> > > >
> > > > During handling of the above exception, another exception occurred:
> > > > test/py/tests/test_ut.py:502: in test_ut
> > > >     output = u_boot_console.run_command('ut ' + ut_subtest)
> > > > test/py/u_boot_console_base.py:266: in run_command
> > > >     m = self.p.expect([self.prompt_compiled] + self.bad_patterns)
> > > > test/py/u_boot_spawn.py:204: in expect
> > > >     raise ValueError('U-Boot exited with %s' % info)
> > > > E   ValueError: U-Boot exited with signal 11 (SIGSEGV)
> > >
> > >
> > > The command uses global variables which hold pointers to 'struct udevice'
> > > which are to be shared between the main and the sub-commands.
> > > Since pytest framework executes ut tests twice, once with a (normal?) device
> > > tree and once with a flat tree,  udevices will be *voided* between
> > > two executions.
> >
> > Are you able to put the var in the uclass-priv data instead? The state
> > should be cleared before running each DM test.
>
> Well, I don't think we need such a trick.
> As you can see, we may simply fetch/find necessary udevices
> every time the command is called.
> It is enough given that the command is mainly for debug purpose.

OK. Let's see how it goes. When you mention global vars I get a bit
nervous, but we do have these in some places.

Regards,
Simon


>
> -Takahiro Akashi
>
>
> > Regards,
> > Simon
> >
> >
> > >
> > > I will fix it in v2.
> > >
> > > Thanks,
> > > -Takahiro Akashi
> > >
> > >
> > > > ------------------------------------ Captured stdout call -------------------------------------
> > > > => ut dm dm_test_scmi_cmd
> > > > Test: dm_test_scmi_cmd: scmi.c
> > > > SCMI device: scmi
> > > >   protocol version: 0x20000
> > > >   # of agents: 2
> > > >       0: platform
> > > >     > 1: OSPM
> > > >   # of protocols: 4
> > > >       Power domain management
> > > >       Clock management
> > > >       Reset domain management
> > > >       Voltage domain management
> > > >   vendor: U-Boot
> > > >   sub vendor: Sandbox
> > > >   impl version: 0x1
> > > > Denying access to device:0 failed (-13)
> > > > Denying access to protocol:0x14 on device:0 failed (-13)
> > > > Reset failed (-13)
> > > > Test: dm_test_scmi_cmd: scmi.c (flat tree)
> > > > SCMI device: Q
> > > > =================================== short test summary info ===================================
> > > > FAILED test/py/tests/test_ut.py::test_ut[ut_dm_dm_test_scmi_cmd] - ValueError: U-Boot exited...
> > > >
> > > > --
> > > > Tom
> > >
> > >