回复: [PATCH v3] firmware: scmi: Add clock v3.2 CONFIG_SET support
Alice Guo (OSS)
alice.guo at oss.nxp.com
Tue Nov 4 04:56:33 CET 2025
> -----邮件原件-----
> 发件人: U-Boot <u-boot-bounces at lists.denx.de> 代表 Marek Vasut
> 发送时间: 2025年11月4日 6:56
> 收件人: u-boot at lists.denx.de
> 抄送: Vinh Nguyen <vinh.nguyen.xz at renesas.com>; Marek Vasut
> <marek.vasut+renesas at mailbox.org>; Alice Guo <alice.guo at nxp.com>;
> Cristian Marussi <cristian.marussi at arm.com>; Patrice Chotard
> <patrice.chotard at foss.st.com>; Patrick Delaunay
> <patrick.delaunay at foss.st.com>; Peng Fan <peng.fan at nxp.com>; Sean
> Anderson <seanga2 at gmail.com>; Tom Rini <trini at konsulko.com>; Valentin
> Caron <valentin.caron at foss.st.com>; Ye Li <ye.li at nxp.com>
> 主题: [PATCH v3] firmware: scmi: Add clock v3.2 CONFIG_SET support
>
> From: Vinh Nguyen <vinh.nguyen.xz at renesas.com>
>
> SCMI v3.2 introduces a new clock CONFIG_SET message format that can
> optionally carry also OEM specific configuration values beside the usual clock
> enable/disable requests. Add support to use such new format when talking to a
> v3.2 compliant SCMI platform.
>
> Support existing enable/disable operations across different clock protocol
> versions: this patch still does not add protocol operations to support the new
> OEM specific optional configuration capabilities.
>
> No functional change for the SCMI drivers users of the related enable and
> disable clock operations.
>
> Signed-off-by: Vinh Nguyen <vinh.nguyen.xz at renesas.com>
> Signed-off-by: Marek Vasut <marek.vasut+renesas at mailbox.org>
> [Marek: Remodel after Linux e49e314a2cf7 ("firmware: arm_scmi: Add clock
> v3.2 CONFIG_SET support")
> Support both old < 2.1 and new >= 2.1 protocol versions.
> Update commit message based on Linux one]
> ---
> Cc: Alice Guo <alice.guo at nxp.com>
> Cc: Cristian Marussi <cristian.marussi at arm.com>
> Cc: Patrice Chotard <patrice.chotard at foss.st.com>
> Cc: Patrick Delaunay <patrick.delaunay at foss.st.com>
> Cc: Peng Fan <peng.fan at nxp.com>
> Cc: Sean Anderson <seanga2 at gmail.com>
> Cc: Tom Rini <trini at konsulko.com>
> Cc: Valentin Caron <valentin.caron at foss.st.com>
> Cc: Vinh Nguyen <vinh.nguyen.xz at renesas.com>
> Cc: Ye Li <ye.li at nxp.com>
> Cc: u-boot at lists.denx.de
> ---
> V2: Fix up iMX95 and sandbox after rework
> V3: Cache pointer to top-level SCMI clock device in each struct clk_scmi
> and use it to extract struct scmi_clock_priv private data which
> contain protocol version. This is necessary, because clk_scmi .dev
> may not necessarily point to direct subdevice of the top level SCMI
> clock device.
> ---
> arch/arm/mach-imx/imx9/scmi/clock_scmi.c | 2 +-
> drivers/clk/clk_scmi.c | 35 ++++++++++++++++++----
> drivers/firmware/scmi/sandbox-scmi_agent.c | 4 +--
> include/scmi_protocols.h | 17 +++++++++--
> 4 files changed, 48 insertions(+), 10 deletions(-)
>
> diff --git a/arch/arm/mach-imx/imx9/scmi/clock_scmi.c
> b/arch/arm/mach-imx/imx9/scmi/clock_scmi.c
> index fa15b5f8df9..fc1d5d77799 100644
> --- a/arch/arm/mach-imx/imx9/scmi/clock_scmi.c
> +++ b/arch/arm/mach-imx/imx9/scmi/clock_scmi.c
> @@ -10,7 +10,7 @@
>
> int imx_clk_scmi_enable(u32 clock_id, bool enable) {
> - struct scmi_clk_state_in in = {
> + struct scmi_clk_state_in_v1 in = {
> .clock_id = clock_id,
> .attributes = (enable) ? 1 : 0,
> };
> diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c index
> a7d89f32cd7..cdbf7e6df52 100644
> --- a/drivers/clk/clk_scmi.c
> +++ b/drivers/clk/clk_scmi.c
> @@ -16,6 +16,7 @@
>
> struct clk_scmi {
> struct clk clk;
> + struct udevice *sdev; /* SCMI top-level clock device */
> u32 ctrl_flags;
> };
>
> @@ -134,17 +135,40 @@ static int scmi_clk_get_attibute(struct udevice *dev,
> int clkid, char **name,
>
> static int scmi_clk_gate(struct clk *clk, int enable) {
> - struct scmi_clk_state_in in = {
> + struct scmi_clk_state_in_v1 in_v1 = {
> + .clock_id = clk_get_id(clk),
> + .attributes = enable,
> + };
> + /* Valid only from SCMI clock v2.1 */
> + struct scmi_clk_state_in_v2 in_v2 = {
> .clock_id = clk_get_id(clk),
> .attributes = enable,
> };
> struct scmi_clk_state_out out;
> - struct scmi_msg msg = SCMI_MSG_IN(SCMI_PROTOCOL_ID_CLOCK,
> - SCMI_CLOCK_CONFIG_SET,
> - in, out);
> + struct scmi_msg msg_v1 = SCMI_MSG_IN(SCMI_PROTOCOL_ID_CLOCK,
> + SCMI_CLOCK_CONFIG_SET,
> + in_v1, out);
> + struct scmi_msg msg_v2 = SCMI_MSG_IN(SCMI_PROTOCOL_ID_CLOCK,
> + SCMI_CLOCK_CONFIG_SET,
> + in_v2, out);
> + struct scmi_clock_priv *priv;
> + struct clk_scmi *clkscmi;
> + struct clk *c;
> int ret;
>
> - ret = devm_scmi_process_msg(clk->dev, &msg);
> + ret = clk_get_by_id(clk->id, &c);
> + if (ret)
> + return ret;
> +
> + clkscmi = container_of(c, struct clk_scmi, clk);
> +
> + priv = dev_get_priv(clkscmi->sdev);
> + if (!priv)
> + return -EINVAL;
> +
> + ret = devm_scmi_process_msg(clk->dev,
> + (priv->version < CLOCK_PROTOCOL_VERSION_2_1) ?
> + &msg_v1 : &msg_v2);
> if (ret)
> return ret;
>
> @@ -319,6 +343,7 @@ static int scmi_clk_probe(struct udevice *dev)
> }
>
> dev_clk_dm(dev, i, &clk_scmi->clk);
> + clk_scmi->sdev = dev;
>
> if (CLK_HAS_RESTRICTIONS(attributes)) {
> u32 perm;
> diff --git a/drivers/firmware/scmi/sandbox-scmi_agent.c
> b/drivers/firmware/scmi/sandbox-scmi_agent.c
> index 74a87832dcb..5b242a039c2 100644
> --- a/drivers/firmware/scmi/sandbox-scmi_agent.c
> +++ b/drivers/firmware/scmi/sandbox-scmi_agent.c
> @@ -828,7 +828,7 @@ static int sandbox_scmi_clock_rate_get(struct udevice
> *dev,
>
> static int sandbox_scmi_clock_gate(struct udevice *dev, struct scmi_msg *msg)
> {
> - struct scmi_clk_state_in *in = NULL;
> + struct scmi_clk_state_in_v1 *in = NULL;
> struct scmi_clk_state_out *out = NULL;
> struct sandbox_scmi_clk *clk_state = NULL;
>
> @@ -836,7 +836,7 @@ static int sandbox_scmi_clock_gate(struct udevice *dev,
> struct scmi_msg *msg)
> !msg->out_msg || msg->out_msg_sz < sizeof(*out))
> return -EINVAL;
>
> - in = (struct scmi_clk_state_in *)msg->in_msg;
> + in = (struct scmi_clk_state_in_v1 *)msg->in_msg;
> out = (struct scmi_clk_state_out *)msg->out_msg;
>
> clk_state = get_scmi_clk_state(in->clock_id); diff --git
> a/include/scmi_protocols.h b/include/scmi_protocols.h index
> bcd8e671149..ecab021b472 100644
> --- a/include/scmi_protocols.h
> +++ b/include/scmi_protocols.h
> @@ -733,6 +733,7 @@ int scmi_pwd_name_get(struct udevice *dev, u32
> domain_id, u8 **name);
> /*
> * SCMI Clock Protocol
> */
> +#define CLOCK_PROTOCOL_VERSION_2_1 0x20001
> #define CLOCK_PROTOCOL_VERSION_3_0 0x30000
>
> enum scmi_clock_message_id {
> @@ -800,15 +801,27 @@ struct scmi_clk_attribute_out_v2 { };
>
> /**
> - * struct scmi_clk_state_in - Message payload for CLOCK_CONFIG_SET
> command
> + * struct scmi_clk_state_in_v1 - Message payload for CLOCK_CONFIG_SET
> + command for protocol < 2.1
> * @clock_id: SCMI clock ID
> * @attributes: Attributes of the targets clock state
> */
> -struct scmi_clk_state_in {
> +struct scmi_clk_state_in_v1 {
> u32 clock_id;
> u32 attributes;
> };
>
> +/**
> + * struct scmi_clk_state_in_v2 - Message payload for CLOCK_CONFIG_SET
> command for protocol >= 2.1
> + * @clock_id: SCMI clock ID
> + * @attributes: Attributes of the targets clock state
> + * @extended_config_val: Extended and OEM specific configuration */
> +struct scmi_clk_state_in_v2 {
> + u32 clock_id;
> + u32 attributes;
> + u32 extended_config_val;
> +};
> +
> /**
> * struct scmi_clk_state_out - Response payload for CLOCK_CONFIG_SET
> command
> * @status: SCMI command status
> --
> 2.51.0
Hi Marek,
I tested PATCH v3 on i.MX95, and it works as expected. However, I noticed that PATCH v3 increases runtime memory usage. I'm not sure whether this is a concern we need to address, so I want to discuss it with you. Here is the change I made based on PATCH v2:
static int scmi_clk_gate(struct clk *clk, int enable)
{
- struct scmi_clock_priv *priv = dev_get_priv(clk->dev->parent);
+ struct scmi_clock_priv *priv = clk->dev->parent_priv_;
struct scmi_clk_state_in_v1 in_v1 = {
.clock_id = clk_get_id(clk),
.attributes = enable,
@@ -331,6 +332,7 @@ static int scmi_clk_probe(struct udevice *dev)
}
dev_clk_dm(dev, i, &clk_scmi->clk);
+ dev_set_parent_priv(clk_scmi->clk.dev, priv);
if (CLK_HAS_RESTRICTIONS(attributes)) {
u32 perm;
Best regards,
Alice Guo
More information about the U-Boot
mailing list