On 12/17/25 14:02, Marek Vasut wrote:
> On 12/15/25 8:38 AM, 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]
>> Reviewed-by: Peng Fan <[email protected]>
>> ---
>> drivers/clk/clk_scmi.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
>> index f6132178205..5c99f3708c3 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,15 @@ static int scmi_clk_gate(struct clk *clk, int enable)
>> in_v2, out);
>> int ret;
>> + /*
>> + * retrieve the correct priv which was set in scmi_clk_probe().
>> + * in case CLK_CCF is set, priv was set in parent_priv
>> + */
>> + if (!CONFIG_IS_ENABLED(CLK_CCF))
>
> Please invert the conditional here , no need for the if (!...) .
Ok
Thanks
Patrice
>
>> + priv = dev_get_priv(clk->dev);
>> + else
>> + priv = dev_get_parent_priv(clk->dev);
>> +
>> ret = devm_scmi_process_msg(clk->dev,
>> (priv->version < CLOCK_PROTOCOL_VERSION_2_1) ?
>> &msg_v1 : &msg_v2);
>>
