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.aka...@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.aka...@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