* [PATCH v2 1/5] test: dm: skip scmi tests against disabled protocols
2023-11-13 1:49 [PATCH v2 0/5] cmd: add scmi command AKASHI Takahiro
@ 2023-11-13 1:49 ` AKASHI Takahiro
2023-11-13 18:01 ` Simon Glass
2023-11-13 1:49 ` [PATCH v2 2/5] firmware: scmi: support protocols on sandbox only if enabled AKASHI Takahiro
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: AKASHI Takahiro @ 2023-11-13 1:49 UTC (permalink / raw)
To: trini, sjg; +Cc: etienne.carriere, michal.simek, u-boot, AKASHI Takahiro
This is a precautionary 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 1/5] test: dm: skip scmi tests against disabled protocols
2023-11-13 1:49 ` [PATCH v2 1/5] test: dm: skip scmi tests against disabled protocols AKASHI Takahiro
@ 2023-11-13 18:01 ` Simon Glass
2023-11-14 1:44 ` AKASHI Takahiro
0 siblings, 1 reply; 10+ messages in thread
From: Simon Glass @ 2023-11-13 18:01 UTC (permalink / raw)
To: AKASHI Takahiro; +Cc: trini, etienne.carriere, michal.simek, u-boot
Hi AKASHI,
On Sun, 12 Nov 2023 at 18:49, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> This is a precautionary 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;
-EAGAIN to skip a test
Please update a comment if this needs to be documented better
> +
> /* 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
>
Regards,
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/5] test: dm: skip scmi tests against disabled protocols
2023-11-13 18:01 ` Simon Glass
@ 2023-11-14 1:44 ` AKASHI Takahiro
0 siblings, 0 replies; 10+ messages in thread
From: AKASHI Takahiro @ 2023-11-14 1:44 UTC (permalink / raw)
To: Simon Glass; +Cc: trini, etienne.carriere, michal.simek, u-boot
On Mon, Nov 13, 2023 at 11:01:18AM -0700, Simon Glass wrote:
> Hi AKASHI,
>
> On Sun, 12 Nov 2023 at 18:49, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > This is a precautionary 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;
>
> -EAGAIN to skip a test
Ah, I didn't notice such a common practice as it is rarely seen
under ut. Will fix.
-Takahiro Akashi
> Please update a comment if this needs to be documented better
>
> > +
> > /* 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
> >
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/5] firmware: scmi: support protocols on sandbox only if enabled
2023-11-13 1:49 [PATCH v2 0/5] cmd: add scmi command AKASHI Takahiro
2023-11-13 1:49 ` [PATCH v2 1/5] test: dm: skip scmi tests against disabled protocols AKASHI Takahiro
@ 2023-11-13 1:49 ` AKASHI Takahiro
2023-11-13 18:01 ` Simon Glass
2023-11-13 1:49 ` [PATCH v2 3/5] cmd: add scmi command for SCMI firmware AKASHI Takahiro
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: AKASHI Takahiro @ 2023-11-13 1:49 UTC (permalink / raw)
To: trini, sjg; +Cc: etienne.carriere, michal.simek, u-boot, 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 d13180962662..1fc9a0f4ea7e 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v2 2/5] firmware: scmi: support protocols on sandbox only if enabled
2023-11-13 1:49 ` [PATCH v2 2/5] firmware: scmi: support protocols on sandbox only if enabled AKASHI Takahiro
@ 2023-11-13 18:01 ` Simon Glass
2023-11-14 1:53 ` AKASHI Takahiro
0 siblings, 1 reply; 10+ messages in thread
From: Simon Glass @ 2023-11-13 18:01 UTC (permalink / raw)
To: AKASHI Takahiro; +Cc: trini, etienne.carriere, michal.simek, u-boot
Hi AKASHI,
On Sun, 12 Nov 2023 at 18:49, AKASHI Takahiro
<takahiro.akashi@linaro.org> wrote:
>
> 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 d13180962662..1fc9a0f4ea7e 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,
Is this better? Perhaps not!
CONFIG_IS_ENABLED(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;
How about putting this all in a function and avoiding the goto?
> +
> 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
>
Regards,
Simon
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH v2 2/5] firmware: scmi: support protocols on sandbox only if enabled
2023-11-13 18:01 ` Simon Glass
@ 2023-11-14 1:53 ` AKASHI Takahiro
0 siblings, 0 replies; 10+ messages in thread
From: AKASHI Takahiro @ 2023-11-14 1:53 UTC (permalink / raw)
To: Simon Glass; +Cc: trini, etienne.carriere, michal.simek, u-boot
On Mon, Nov 13, 2023 at 11:01:20AM -0700, Simon Glass wrote:
> Hi AKASHI,
>
> On Sun, 12 Nov 2023 at 18:49, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
> >
> > 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 d13180962662..1fc9a0f4ea7e 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,
>
> Is this better? Perhaps not!
>
> CONFIG_IS_ENABLED(SCMI_POWER_DOMAIN, (SCMI_PROTOCOL_ID_POWER_DOMAIN,))
Ah, good notation.
> > +#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;
>
> How about putting this all in a function and avoiding the goto?
Okay, will do.
Thanks,
-Takahiro Akashi
> > +
> > 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
> >
>
> Regards,
> Simon
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 3/5] cmd: add scmi command for SCMI firmware
2023-11-13 1:49 [PATCH v2 0/5] cmd: add scmi command AKASHI Takahiro
2023-11-13 1:49 ` [PATCH v2 1/5] test: dm: skip scmi tests against disabled protocols AKASHI Takahiro
2023-11-13 1:49 ` [PATCH v2 2/5] firmware: scmi: support protocols on sandbox only if enabled AKASHI Takahiro
@ 2023-11-13 1:49 ` AKASHI Takahiro
2023-11-13 1:49 ` [PATCH v2 4/5] doc: cmd: add documentation for scmi AKASHI Takahiro
2023-11-13 1:49 ` [PATCH v2 5/5] test: dm: add scmi command test AKASHI Takahiro
4 siblings, 0 replies; 10+ messages in thread
From: AKASHI Takahiro @ 2023-11-13 1:49 UTC (permalink / raw)
To: trini, sjg
Cc: etienne.carriere, michal.simek, u-boot, AKASHI Takahiro,
Etienne Carriere
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>
---
v8 (actually v2 as SCMI cmd)
* localize global variables to avoid pytest errors
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 | 384 +++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 394 insertions(+)
create mode 100644 cmd/scmi.c
diff --git a/cmd/Kconfig b/cmd/Kconfig
index df6d71c103f9..ca9f742dcf78 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -2559,6 +2559,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..664062c4eff5
--- /dev/null
+++ b/cmd/scmi.c
@@ -0,0 +1,384 @@
+// 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>
+
+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_agent() - get SCMI agent device
+ *
+ * Return: Pointer to SCMI agent device on success, NULL on failure
+ */
+static struct udevice *get_agent(void)
+{
+ struct udevice *agent;
+
+ if (uclass_get_device(UCLASS_SCMI_AGENT, 0, &agent)) {
+ printf("Cannot find any SCMI agent\n");
+ return NULL;
+ }
+
+ return agent;
+}
+
+/**
+ * get_base_proto() - get SCMI base protocol device
+ * @agent: SCMI agent device
+ *
+ * Return: Pointer to SCMI base protocol device on success,
+ * NULL on failure
+ */
+static struct udevice *get_base_proto(struct udevice *agent)
+{
+ struct udevice *base_proto;
+
+ if (!agent) {
+ agent = get_agent();
+ if (!agent)
+ return NULL;
+ }
+
+ base_proto = scmi_get_protocol(agent, SCMI_PROTOCOL_ID_BASE);
+ if (!base_proto) {
+ printf("SCMI base protocol not found\n");
+ return NULL;
+ }
+
+ return base_proto;
+}
+
+/**
+ * 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[])
+{
+ struct udevice *agent, *base_proto;
+ u32 agent_id, num_protocols;
+ u8 *agent_name, *protocols;
+ int i, ret;
+
+ if (argc != 1)
+ return CMD_RET_USAGE;
+
+ agent = get_agent();
+ if (!agent)
+ return CMD_RET_FAILURE;
+ base_proto = get_base_proto(agent);
+ if (!base_proto)
+ return CMD_RET_FAILURE;
+
+ 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;
+ struct udevice *base_proto;
+ 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;
+
+ base_proto = get_base_proto(NULL);
+ if (!base_proto)
+ return CMD_RET_FAILURE;
+
+ 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;
+ struct udevice *base_proto;
+ 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;
+
+ base_proto = get_base_proto(NULL);
+ if (!base_proto)
+ return CMD_RET_FAILURE;
+
+ 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;
+ struct udevice *base_proto;
+ 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;
+
+ base_proto = get_base_proto(NULL);
+ if (!base_proto)
+ return CMD_RET_FAILURE;
+
+ 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++;
+
+ 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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 4/5] doc: cmd: add documentation for scmi
2023-11-13 1:49 [PATCH v2 0/5] cmd: add scmi command AKASHI Takahiro
` (2 preceding siblings ...)
2023-11-13 1:49 ` [PATCH v2 3/5] cmd: add scmi command for SCMI firmware AKASHI Takahiro
@ 2023-11-13 1:49 ` AKASHI Takahiro
2023-11-13 1:49 ` [PATCH v2 5/5] test: dm: add scmi command test AKASHI Takahiro
4 siblings, 0 replies; 10+ messages in thread
From: AKASHI Takahiro @ 2023-11-13 1:49 UTC (permalink / raw)
To: trini, sjg
Cc: etienne.carriere, michal.simek, u-boot, AKASHI Takahiro,
Etienne Carriere
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 d8e23fcacffb..1a626c03c237 100644
--- a/doc/usage/index.rst
+++ b/doc/usage/index.rst
@@ -94,6 +94,7 @@ Shell commands
cmd/rng
cmd/saves
cmd/sbi
+ cmd/scmi
cmd/scp03
cmd/seama
cmd/setexpr
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v2 5/5] test: dm: add scmi command test
2023-11-13 1:49 [PATCH v2 0/5] cmd: add scmi command AKASHI Takahiro
` (3 preceding siblings ...)
2023-11-13 1:49 ` [PATCH v2 4/5] doc: cmd: add documentation for scmi AKASHI Takahiro
@ 2023-11-13 1:49 ` AKASHI Takahiro
4 siblings, 0 replies; 10+ messages in thread
From: AKASHI Takahiro @ 2023-11-13 1:49 UTC (permalink / raw)
To: trini, sjg
Cc: etienne.carriere, michal.simek, u-boot, AKASHI Takahiro,
Etienne Carriere
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 bc5bcb2a6237..c550af93b0ca 100644
--- a/configs/sandbox_defconfig
+++ b/configs/sandbox_defconfig
@@ -121,6 +121,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
^ permalink raw reply related [flat|nested] 10+ messages in thread