On 12/11/25 1:45 PM, Peng Fan wrote:
On Tue, Dec 09, 2025 at 12:54:15PM +0100, Marek Vasut wrote:
On 12/8/25 2:51 PM, Patrice Chotard wrote:
In scmi_clk_probe(), in case of CLK_CCF is not enabled, parent private
data is not set, so in scmi_clk_gate(), an uninitialized priv struct is
retrieved.
SCMI request is performed either using scmi_clk_state_in_v1 or
scmi_clk_state_in_v2 struct depending of the unpredictable value of
priv->version which leads to error during SCMI clock enable.
Issue detected on STM32MP157C-DK2 board using the SCMI device tree
stm32mp157c-dk2-scmi.dts.
Fixes: 0619cb32030b ("firmware: scmi: Add clock v3.2 CONFIG_SET support")
Signed-off-by: Patrice Chotard <[email protected]>
Cc: Alice Guo <[email protected]>
Cc: Marek Vasut <[email protected]>
Cc: Patrick Delaunay <[email protected]>
Cc: Peng Fan <[email protected]>
Cc: Sean Anderson <[email protected]>
Cc: Tom Rini <[email protected]>
Cc: Valentin Caron <[email protected]>
Cc: Vinh Nguyen <[email protected]>
Cc: [email protected]
---
drivers/clk/clk_scmi.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
index f6132178205..e1c20f2c47c 100644
--- a/drivers/clk/clk_scmi.c
+++ b/drivers/clk/clk_scmi.c
@@ -137,7 +137,7 @@ static int scmi_clk_get_attribute(struct udevice *dev, int
clkid, char *name,
static int scmi_clk_gate(struct clk *clk, int enable)
{
- struct scmi_clock_priv *priv = dev_get_parent_priv(clk->dev);
+ struct scmi_clock_priv *priv;
struct scmi_clk_state_in_v1 in_v1 = {
.clock_id = clk_get_id(clk),
.attributes = enable,
@@ -156,6 +156,11 @@ static int scmi_clk_gate(struct clk *clk, int enable)
in_v2, out);
int ret;
+ if (!CONFIG_IS_ENABLED(CLK_CCF))
+ priv = dev_get_priv(clk->dev);
+ else
+ priv = dev_get_parent_priv(clk->dev);
+
Shouldn't this be doing similar resolution like existing code ?
168 static int scmi_clk_get_ctrl_flags(struct clk *clk, u32 *ctrl_flags)
169 {
170 struct clk_scmi *clkscmi;
171 struct udevice *dev;
172 u32 attributes;
173 struct clk *c;
174 int ret;
175
176 ret = clk_get_by_id(clk->id, &c);
177 if (ret)
178 return ret;
179
180 dev = c->dev->parent; // <-----------------------------
No need. The current clk code is indeed confusing.
In drivers/clk/clk.c, clk_get_by_id is ready called, so the first param passed
to scmi_clk_gate is the clk that registered during clk driver probe.
Please add a code comment.
