[PATCH v2 2/5] firmware: scmi: support protocols on sandbox only if enabled
AKASHI Takahiro
takahiro.akashi at linaro.org
Tue Nov 14 02:53:24 CET 2023
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 at 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 at 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
More information about the U-Boot
mailing list