[PATCH 2/3] clk: scmi: Fix priv initialization in scmi_clk_gate()

[Marek Vasut]

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 <patrice.chotard at foss.st.com>
>>>
>>> Cc: Alice Guo <alice.guo at nxp.com>
>>> Cc: Marek Vasut <marek.vasut+renesas at mailbox.org>
>>> 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: u-boot at lists.denx.de
>>> ---
>>>    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.