From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: Simon Glass <sjg@chromium.org>
Cc: trini@konsulko.com, etienne.carriere@st.com,
michal.simek@amd.com, u-boot@lists.denx.de
Subject: Re: [PATCH v2 2/5] firmware: scmi: support protocols on sandbox only if enabled
Date: Tue, 14 Nov 2023 10:53:24 +0900 [thread overview]
Message-ID: <ZVLTFCZJ8mUvv5UJ@octopus> (raw)
In-Reply-To: <CAPnjgZ0dnRJxorwAyp1M=aHtaxWOEcwErrWex8ur38iK5+GYNg@mail.gmail.com>
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
next prev parent reply other threads:[~2023-11-14 1:53 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
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 18:01 ` Simon Glass
2023-11-14 1:44 ` 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 18:01 ` Simon Glass
2023-11-14 1:53 ` AKASHI Takahiro [this message]
2023-11-13 1:49 ` [PATCH v2 3/5] cmd: add scmi command for SCMI firmware 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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZVLTFCZJ8mUvv5UJ@octopus \
--to=takahiro.akashi@linaro.org \
--cc=etienne.carriere@st.com \
--cc=michal.simek@amd.com \
--cc=sjg@chromium.org \
--cc=trini@konsulko.com \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.