diff mbox series

[v5,14/16] cmd: add scmi command for SCMI firmware

Message ID 20230926065750.734440-15-takahiro.akashi@linaro.org
State Superseded
Headers show
Series firmware: scmi: add SCMI base protocol support | expand

Commit Message

AKASHI Takahiro Sept. 26, 2023, 6:57 a.m. UTC
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   | 337 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 347 insertions(+)
 create mode 100644 cmd/scmi.c

Comments

Michal Simek Oct. 24, 2023, 8:27 a.m. UTC | #1
Hi Takahiro,

út 26. 9. 2023 v 9:00 odesílatel AKASHI Takahiro
<takahiro.akashi@linaro.org> napsal:
>
> 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   | 337 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 347 insertions(+)
>  create mode 100644 cmd/scmi.c
>
> diff --git a/cmd/Kconfig b/cmd/Kconfig
> index 43ca10f69ccf..f46152ace7d8 100644
> --- a/cmd/Kconfig
> +++ b/cmd/Kconfig
> @@ -2533,6 +2533,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

This line above is wrong and this was removed from v6 with
"drop scmi command which was intended to be used for debugging".
It is fine that it shouldn't be used on production system but it
doesn't mean that
it should be actually removed.
It is useful for bring ups. Can we get this patch merged? It was
already reviewed
anyway.

Thanks,
Michal
Tom Rini Oct. 24, 2023, 10:24 p.m. UTC | #2
On Tue, Oct 24, 2023 at 10:27:44AM +0200, Michal Simek wrote:
> Hi Takahiro,
> 
> út 26. 9. 2023 v 9:00 odesílatel AKASHI Takahiro
> <takahiro.akashi@linaro.org> napsal:
> >
> > 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   | 337 +++++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 347 insertions(+)
> >  create mode 100644 cmd/scmi.c
> >
> > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > index 43ca10f69ccf..f46152ace7d8 100644
> > --- a/cmd/Kconfig
> > +++ b/cmd/Kconfig
> > @@ -2533,6 +2533,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
> 
> This line above is wrong and this was removed from v6 with
> "drop scmi command which was intended to be used for debugging".
> It is fine that it shouldn't be used on production system but it
> doesn't mean that
> it should be actually removed.
> It is useful for bring ups. Can we get this patch merged? It was
> already reviewed
> anyway.

There was then also some conflict with the follow-up series here. I
would be fine with the command being introduced again after I merge that
second series, which I am testing now.
AKASHI Takahiro Oct. 25, 2023, 1:14 a.m. UTC | #3
Hi Tom, Michal,

On Tue, Oct 24, 2023 at 06:24:07PM -0400, Tom Rini wrote:
> On Tue, Oct 24, 2023 at 10:27:44AM +0200, Michal Simek wrote:
> > Hi Takahiro,
> > 
> > ?t 26. 9. 2023 v 9:00 odes?latel AKASHI Takahiro
> > <takahiro.akashi@linaro.org> napsal:
> > >
> > > 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   | 337 +++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 347 insertions(+)
> > >  create mode 100644 cmd/scmi.c
> > >
> > > diff --git a/cmd/Kconfig b/cmd/Kconfig
> > > index 43ca10f69ccf..f46152ace7d8 100644
> > > --- a/cmd/Kconfig
> > > +++ b/cmd/Kconfig
> > > @@ -2533,6 +2533,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
> > 
> > This line above is wrong and this was removed from v6 with

@Michal, do you mean the default should be 'y'?

> > "drop scmi command which was intended to be used for debugging".
> > It is fine that it shouldn't be used on production system but it
> > doesn't mean that
> > it should be actually removed.
> > It is useful for bring ups. Can we get this patch merged? It was
> > already reviewed
> > anyway.
> 
> There was then also some conflict with the follow-up series here. I
> would be fine with the command being introduced again after I merge that
> second series, which I am testing now.

I don't mind neither.
My concern, however, was a test, "ut dm scmi_cmd".
The output from "scmi info" command varies depending on what SCMI protocols
are provided by SCMI firmware (sandbox fake server in this case).

I will try to keep it updated.

-Takahiro Akashi

> 
> -- 
> Tom
diff mbox series

Patch

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 43ca10f69ccf..f46152ace7d8 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2533,6 +2533,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 9bebf321c397..ad3810b17e93 100644
--- a/cmd/Makefile
+++ b/cmd/Makefile
@@ -158,6 +158,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..5efec8ad87fd
--- /dev/null
+++ b/cmd/scmi.c
@@ -0,0 +1,337 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ *  SCMI (System Control and Management Interface) utility command
+ *
+ *  Copyright (c) 2023 Linaro Limited
+ *		Author: AKASHI Takahiro
+ */
+
+#include <common.h>
+#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);