[PATCH 0/3] clk: scmi: various fixes

[PATCH 0/3] clk: scmi: various fixes

[Patrice Chotard]

- Fix typo scmi_clk_get_attibute.
- Fix priv initialization in scmi_clk_gate() which leads to
  clock_enable() error when CONFIG_CLK_CCF is not set.
- Remove duplicated scmi_generic_protocol_version() call in 
  scmi_clk_probe().
 

Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
---
Patrice Chotard (3):
      clk: scmi: Fix typo scmi_clk_get_attibute
      clk: scmi: Fix priv initialization in scmi_clk_gate()
      clk: scmi: Remove duplicated scmi_generic_protocol_version() request

 drivers/clk/clk_scmi.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)
---
base-commit: c5e6d2ab7eba68cbfb600cdc131c0c375ced2ec9
change-id: 20251208-sqqs-7884dbfd4fa7

Best regards,
-- 
Patrice Chotard <patrice.chotard@foss.st.com>

[PATCH 1/3] clk: scmi: Fix typo scmi_clk_get_attibute

[Patrice Chotard]

%s/scmi_clk_get_attibute/scmi_clk_get_attribute/

Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>

Cc: Alice Guo <alice.guo@nxp.com>
Cc: Marek Vasut <marek.vasut+renesas@mailbox.org>
Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Sean Anderson <seanga2@gmail.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Valentin Caron <valentin.caron@foss.st.com>
Cc: Vinh Nguyen <vinh.nguyen.xz@renesas.com>
Cc: u-boot@lists.denx.de
---
 drivers/clk/clk_scmi.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
index 683ac822a01..f6132178205 100644
--- a/drivers/clk/clk_scmi.c
+++ b/drivers/clk/clk_scmi.c
@@ -87,8 +87,8 @@ static int scmi_clk_get_num_clock(struct udevice *dev, size_t *num_clocks)
 	return 0;
 }
 
-static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char *name,
-				 u32 *attr)
+static int scmi_clk_get_attribute(struct udevice *dev, int clkid, char *name,
+				  u32 *attr)
 {
 	struct scmi_clock_priv *priv = dev_get_priv(dev);
 	struct scmi_clk_attribute_in in = {
@@ -183,8 +183,8 @@ static int scmi_clk_get_ctrl_flags(struct clk *clk, u32 *ctrl_flags)
 
 	if (!clkscmi->attrs_resolved) {
 		char name[SCMI_CLOCK_NAME_LENGTH_MAX];
-		ret = scmi_clk_get_attibute(dev, clk->id & CLK_ID_MSK,
-					    name, &attributes);
+		ret = scmi_clk_get_attribute(dev, clk->id & CLK_ID_MSK,
+					     name, &attributes);
 		if (ret)
 			return ret;
 

-- 
2.43.0

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

[Patrice Chotard]

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@foss.st.com>

Cc: Alice Guo <alice.guo@nxp.com>
Cc: Marek Vasut <marek.vasut+renesas@mailbox.org>
Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Sean Anderson <seanga2@gmail.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Valentin Caron <valentin.caron@foss.st.com>
Cc: Vinh Nguyen <vinh.nguyen.xz@renesas.com>
Cc: u-boot@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);
+
 	ret = devm_scmi_process_msg(clk->dev,
 				    (priv->version < CLOCK_PROTOCOL_VERSION_2_1) ?
 				    &msg_v1 : &msg_v2);

-- 
2.43.0

[PATCH 3/3] clk: scmi: Remove duplicated scmi_generic_protocol_version() request

[Patrice Chotard]

scmi_generic_protocol_version() request is done twice in scmi_clk_probe().
Remove first call which is useless.

Fixes: ae7e0330ce22 ("clk: scmi: add compatibility with clock protocol 2.0")

Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>

Cc: Alice Guo <alice.guo@nxp.com>
Cc: Marek Vasut <marek.vasut+renesas@mailbox.org>
Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Sean Anderson <seanga2@gmail.com>
Cc: Tom Rini <trini@konsulko.com>
Cc: Valentin Caron <valentin.caron@foss.st.com>
Cc: Vinh Nguyen <vinh.nguyen.xz@renesas.com>
Cc: u-boot@lists.denx.de
---
 drivers/clk/clk_scmi.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/clk/clk_scmi.c b/drivers/clk/clk_scmi.c
index e1c20f2c47c..cbd44f310b1 100644
--- a/drivers/clk/clk_scmi.c
+++ b/drivers/clk/clk_scmi.c
@@ -339,9 +339,6 @@ static int scmi_clk_probe(struct udevice *dev)
 	if (!CONFIG_IS_ENABLED(CLK_CCF))
 		return 0;
 
-	ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_CLOCK,
-					    &priv->version);
-
 	/* register CCF children: CLK UCLASS, no probed again */
 	if (device_get_uclass_id(dev->parent) == UCLASS_CLK)
 		return 0;

-- 
2.43.0

Re: [PATCH 1/3] clk: scmi: Fix typo scmi_clk_get_attibute

[Peng Fan]

On Mon, Dec 08, 2025 at 02:51:25PM +0100, Patrice Chotard wrote:
>%s/scmi_clk_get_attibute/scmi_clk_get_attribute/
>
>Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
>

Reviewed-by: Peng Fan <peng.fan@nxp.com>

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

[Peng Fan]

On Mon, Dec 08, 2025 at 02:51:26PM +0100, 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@foss.st.com>
>

Reviewed-by: Peng Fan <peng.fan@nxp.com>

Re: [PATCH 3/3] clk: scmi: Remove duplicated scmi_generic_protocol_version() request

[Peng Fan]

On Mon, Dec 08, 2025 at 02:51:27PM +0100, Patrice Chotard wrote:
>scmi_generic_protocol_version() request is done twice in scmi_clk_probe().
>Remove first call which is useless.
>
>Fixes: ae7e0330ce22 ("clk: scmi: add compatibility with clock protocol 2.0")
>
>Signed-off-by: Patrice Chotard <patrice.chotard@foss.st.com>
>

Reviewed-by: Peng Fan <peng.fan@nxp.com>

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

[Marek Vasut]

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@foss.st.com>
> 
> Cc: Alice Guo <alice.guo@nxp.com>
> Cc: Marek Vasut <marek.vasut+renesas@mailbox.org>
> Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Sean Anderson <seanga2@gmail.com>
> Cc: Tom Rini <trini@konsulko.com>
> Cc: Valentin Caron <valentin.caron@foss.st.com>
> Cc: Vinh Nguyen <vinh.nguyen.xz@renesas.com>
> Cc: u-boot@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; // <-----------------------------
181
182         clkscmi = container_of(c, struct clk_scmi, clk);
183
184         if (!clkscmi->attrs_resolved) {
185                 char name[SCMI_CLOCK_NAME_LENGTH_MAX];
186                 ret = scmi_clk_get_attibute(dev,
                           ^^^^^^^^^^^^^^^^^^^^^^^^^
...
  90 static int scmi_clk_get_attibute(struct udevice *dev, int clkid, 
char *name,
  91                                  u32 *attr)
  92 {
  93         struct scmi_clock_priv *priv = dev_get_priv(dev);
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

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

[Peng Fan]

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@foss.st.com>
>> 
>> Cc: Alice Guo <alice.guo@nxp.com>
>> Cc: Marek Vasut <marek.vasut+renesas@mailbox.org>
>> Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
>> Cc: Peng Fan <peng.fan@nxp.com>
>> Cc: Sean Anderson <seanga2@gmail.com>
>> Cc: Tom Rini <trini@konsulko.com>
>> Cc: Valentin Caron <valentin.caron@foss.st.com>
>> Cc: Vinh Nguyen <vinh.nguyen.xz@renesas.com>
>> Cc: u-boot@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.

Regards
Peng

>181
>182         clkscmi = container_of(c, struct clk_scmi, clk);
>183
>184         if (!clkscmi->attrs_resolved) {
>185                 char name[SCMI_CLOCK_NAME_LENGTH_MAX];
>186                 ret = scmi_clk_get_attibute(dev,
>                          ^^^^^^^^^^^^^^^^^^^^^^^^^
>...
> 90 static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char
>*name,
> 91                                  u32 *attr)
> 92 {
> 93         struct scmi_clock_priv *priv = dev_get_priv(dev);
>            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>

Re: [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@foss.st.com>
>>>
>>> Cc: Alice Guo <alice.guo@nxp.com>
>>> Cc: Marek Vasut <marek.vasut+renesas@mailbox.org>
>>> Cc: Patrick Delaunay <patrick.delaunay@foss.st.com>
>>> Cc: Peng Fan <peng.fan@nxp.com>
>>> Cc: Sean Anderson <seanga2@gmail.com>
>>> Cc: Tom Rini <trini@konsulko.com>
>>> Cc: Valentin Caron <valentin.caron@foss.st.com>
>>> Cc: Vinh Nguyen <vinh.nguyen.xz@renesas.com>
>>> Cc: u-boot@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.