[PATCH 1/4] clk: scmi: support set parent

From: Peng Fan peng.fan@nxp.com
Support SCMI CLK set parent.
Signed-off-by: Peng Fan peng.fan@nxp.com Signed-off-by: Alice Guo alice.guo@nxp.com Reviewed-by: Ye Li ye.li@nxp.com --- drivers/clk/clk_scmi.c | 20 ++++++++++++++++++++ include/scmi_protocols.h | 19 +++++++++++++++++++ 2 files changed, 39 insertions(+)
diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c index e42d2032d4..84333cdd0c 100644 --- a/drivers/clk/clk_scmi.c +++ b/drivers/clk/clk_scmi.c @@ -178,11 +178,31 @@ static int scmi_clk_probe(struct udevice *dev) return 0; }
+static int scmi_clk_set_parent(struct clk *clk, struct clk *parent) +{ + struct scmi_clk_parent_set_in in = { + .clock_id = clk->id, + .parent_clk = parent->id, + }; + struct scmi_clk_parent_set_out out; + struct scmi_msg msg = SCMI_MSG_IN(SCMI_PROTOCOL_ID_CLOCK, + SCMI_CLOCK_PARENT_SET, + in, out); + int ret; + + ret = devm_scmi_process_msg(clk->dev, &msg); + if (ret < 0) + return ret; + + return scmi_to_linux_errno(out.status); +} + static const struct clk_ops scmi_clk_ops = { .enable = scmi_clk_enable, .disable = scmi_clk_disable, .get_rate = scmi_clk_get_rate, .set_rate = scmi_clk_set_rate, + .set_parent = scmi_clk_set_parent, };
U_BOOT_DRIVER(scmi_clock) = { diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h index e52c740cb3..e1e4117e11 100644 --- a/include/scmi_protocols.h +++ b/include/scmi_protocols.h @@ -732,6 +732,7 @@ enum scmi_clock_message_id { SCMI_CLOCK_RATE_SET = 0x5, SCMI_CLOCK_RATE_GET = 0x6, SCMI_CLOCK_CONFIG_SET = 0x7, + SCMI_CLOCK_PARENT_SET = 0x10 };
#define SCMI_CLK_PROTO_ATTR_COUNT_MASK GENMASK(15, 0) @@ -835,6 +836,24 @@ struct scmi_clk_rate_set_out { s32 status; };
+/** + * struct scmi_clk_parent_state_in - Message payload for CLOCK_PARENT_SET command + * @clock_id: SCMI clock ID + * @parent_clk: SCMI clock ID + */ +struct scmi_clk_parent_set_in { + u32 clock_id; + u32 parent_clk; +}; + +/** + * struct scmi_clk_parent_set_out - Response payload for CLOCK_PARENT_SET command + * @status: SCMI command status + */ +struct scmi_clk_parent_set_out { + s32 status; +}; + /* * SCMI Reset Domain Protocol */

From: Ye Li ye.li@nxp.com
According to SCMI v3.2, switch to use 0xD for parent clock set.
Signed-off-by: Ye Li ye.li@nxp.com Signed-off-by: Alice Guo alice.guo@nxp.com Reviewed-by: Peng Fan peng.fan@nxp.com --- include/scmi_protocols.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/scmi_protocols.h b/include/scmi_protocols.h index e1e4117e11..36e46f4072 100644 --- a/include/scmi_protocols.h +++ b/include/scmi_protocols.h @@ -732,7 +732,7 @@ enum scmi_clock_message_id { SCMI_CLOCK_RATE_SET = 0x5, SCMI_CLOCK_RATE_GET = 0x6, SCMI_CLOCK_CONFIG_SET = 0x7, - SCMI_CLOCK_PARENT_SET = 0x10 + SCMI_CLOCK_PARENT_SET = 0xD };
#define SCMI_CLK_PROTO_ATTR_COUNT_MASK GENMASK(15, 0)

From: Peng Fan peng.fan@nxp.com
Support the clk scmi driver probe in pre-reloc stage.
Signed-off-by: Peng Fan peng.fan@nxp.com Signed-off-by: Alice Guo alice.guo@nxp.com Reviewed-by: Ye Li ye.li@nxp.com --- drivers/clk/clk_scmi.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c index 84333cdd0c..a01292c479 100644 --- a/drivers/clk/clk_scmi.c +++ b/drivers/clk/clk_scmi.c @@ -210,4 +210,5 @@ U_BOOT_DRIVER(scmi_clock) = { .id = UCLASS_CLK, .ops = &scmi_clk_ops, .probe = scmi_clk_probe, + .flags = DM_FLAG_PRE_RELOC, };

From: Ye Li ye.li@nxp.com
Add workaround to set_rate/enable/disable to bus clock that SM will reply DENIED error.
Signed-off-by: Ye Li ye.li@nxp.com Signed-off-by: Alice Guo alice.guo@nxp.com Reviewed-by: Peng Fan peng.fan@nxp.com --- drivers/clk/clk_scmi.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c index a01292c479..a860a653ba 100644 --- a/drivers/clk/clk_scmi.c +++ b/drivers/clk/clk_scmi.c @@ -73,7 +73,13 @@ static int scmi_clk_gate(struct clk *clk, int enable) if (ret) return ret;
- return scmi_to_linux_errno(out.status); + ret = scmi_to_linux_errno(out.status); + if (ret == -EACCES) { + debug("Ignore %s enable failure\n", clk_hw_get_name(clk)); + ret = 0; + } + + return ret; }
static int scmi_clk_enable(struct clk *clk) @@ -108,7 +114,7 @@ static ulong scmi_clk_get_rate(struct clk *clk) return (ulong)(((u64)out.rate_msb << 32) | out.rate_lsb); }
-static ulong scmi_clk_set_rate(struct clk *clk, ulong rate) +static ulong __scmi_clk_set_rate(struct clk *clk, ulong rate) { struct scmi_clk_rate_set_in in = { .clock_id = clk->id, @@ -133,6 +139,17 @@ static ulong scmi_clk_set_rate(struct clk *clk, ulong rate) return scmi_clk_get_rate(clk); }
+static ulong scmi_clk_set_rate(struct clk *clk, ulong rate) +{ + ulong orig_rate; + + orig_rate = scmi_clk_get_rate(clk); + if (orig_rate == rate) + return orig_rate; + + return __scmi_clk_set_rate(clk, rate); +} + static int scmi_clk_probe(struct udevice *dev) { struct clk *clk;

On Sun, Oct 06, 2024 at 05:38:25PM +0800, alice.guo@oss.nxp.com wrote:
From: Ye Li ye.li@nxp.com
Add workaround to set_rate/enable/disable to bus clock that SM will reply DENIED error.
Signed-off-by: Ye Li ye.li@nxp.com Signed-off-by: Alice Guo alice.guo@nxp.com Reviewed-by: Peng Fan peng.fan@nxp.com
In general, please include a cover letter so it will be clearer what the overall goal is, especially once merged.
drivers/clk/clk_scmi.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c index a01292c479..a860a653ba 100644 --- a/drivers/clk/clk_scmi.c +++ b/drivers/clk/clk_scmi.c @@ -73,7 +73,13 @@ static int scmi_clk_gate(struct clk *clk, int enable) if (ret) return ret;
- return scmi_to_linux_errno(out.status);
- ret = scmi_to_linux_errno(out.status);
- if (ret == -EACCES) {
debug("Ignore %s enable failure\n", clk_hw_get_name(clk));
ret = 0;
- }
- return ret;
}
This seems like a generic change being made globally and not a work-around for a specific problem on (some?) iMX families. Has this been tested on other platforms?

Subject: Re: [PATCH 4/4] clk: scmi: Add workaround for set_rate/enable/disable
On Sun, Oct 06, 2024 at 05:38:25PM +0800, alice.guo@oss.nxp.com wrote:
From: Ye Li ye.li@nxp.com
Add workaround to set_rate/enable/disable to bus clock that SM will reply DENIED error.
Signed-off-by: Ye Li ye.li@nxp.com Signed-off-by: Alice Guo alice.guo@nxp.com Reviewed-by: Peng Fan peng.fan@nxp.com
In general, please include a cover letter so it will be clearer what the overall goal is, especially once merged.
drivers/clk/clk_scmi.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c index a01292c479..a860a653ba 100644 --- a/drivers/clk/clk_scmi.c +++ b/drivers/clk/clk_scmi.c @@ -73,7 +73,13 @@ static int scmi_clk_gate(struct clk *clk, int
enable)
if (ret) return ret;
- return scmi_to_linux_errno(out.status);
- ret = scmi_to_linux_errno(out.status);
- if (ret == -EACCES) {
debug("Ignore %s enable failure\n",
clk_hw_get_name(clk));
ret = 0;
- }
- return ret;
}
This seems like a generic change being made globally and not a work- around for a specific problem on (some?) iMX families.
We have changed Linux upstream to this behavior, but better in firmware/clock.c as linux, https://elixir.bootlin.com/linux/v6.11.2/source/drivers/firmware/arm_scmi/cl...
Regards, Peng.
Has this been
tested on other platforms?
-- Tom

On Tue, Oct 08, 2024 at 03:02:13AM +0000, Peng Fan wrote:
Subject: Re: [PATCH 4/4] clk: scmi: Add workaround for set_rate/enable/disable
On Sun, Oct 06, 2024 at 05:38:25PM +0800, alice.guo@oss.nxp.com wrote:
From: Ye Li ye.li@nxp.com
Add workaround to set_rate/enable/disable to bus clock that SM will reply DENIED error.
Signed-off-by: Ye Li ye.li@nxp.com Signed-off-by: Alice Guo alice.guo@nxp.com Reviewed-by: Peng Fan peng.fan@nxp.com
In general, please include a cover letter so it will be clearer what the overall goal is, especially once merged.
drivers/clk/clk_scmi.c | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c index a01292c479..a860a653ba 100644 --- a/drivers/clk/clk_scmi.c +++ b/drivers/clk/clk_scmi.c @@ -73,7 +73,13 @@ static int scmi_clk_gate(struct clk *clk, int
enable)
if (ret) return ret;
- return scmi_to_linux_errno(out.status);
- ret = scmi_to_linux_errno(out.status);
- if (ret == -EACCES) {
debug("Ignore %s enable failure\n",
clk_hw_get_name(clk));
ret = 0;
- }
- return ret;
}
This seems like a generic change being made globally and not a work- around for a specific problem on (some?) iMX families.
We have changed Linux upstream to this behavior, but better in firmware/clock.c as linux, https://elixir.bootlin.com/linux/v6.11.2/source/drivers/firmware/arm_scmi/cl...
Please mention stuff like that in the commit message, as well as the cover letter overview of what you're changing / why, thanks.
participants (3)
-
alice.guo@oss.nxp.com
-
Peng Fan
-
Tom Rini