[PATCH v3 2/2] clk: scmi: add compatibility for clock version 3.0 and onwards

[Kamlesh Gurudasani]

Kamlesh Gurudasani <kamlesh at ti.com> writes:

> Maxim Kochetkov <fido_max at inbox.ru> writes:
>
>> 08.10.2025 15:27, Kamlesh Gurudasani wrote:
>>
>>>>> Could you elaborate?
>>>>> I think clk->dev should be fine, similar to other instances in the same
>>>>> file. Maybe I am missing something?
>>>>>
>>>>
>>>> clk->dev points to hw clock device. It has no struct scmi_clock_priv
>>>> stored in priv data. So we have to step up to parent device to get SCMI
>>>> protocol device to get struct scmi_clock_priv to get protocol version
>>>> priv->version.
>>> Hi Maxim,
>>> 
>>> in probe function, we are making sure that priv data is available
>>> 	struct scmi_clock_priv *priv = dev_get_priv(dev);
>>> 	size_t num_clocks, i;
>>> 	int ret;
>>> 
>>> 	ret = devm_scmi_of_get_channel(dev);
>>> 	if (ret)
>>> 		return ret;
>>> 
>>> 	ret = scmi_generic_protocol_version(dev, SCMI_PROTOCOL_ID_CLOCK,
>>> 					    &priv->version);
>>> 
>>> 
>>> Also, the code is working just fine and returns correct version with
>>> clk->dev
>>> 
>>> Let me know if I missed something
>>
>> Here is my debug path:
>>
>> --- a/drivers/clk/clk_scmi.c
>> +++ b/drivers/clk/clk_scmi.c
>> @@ -135,6 +135,10 @@ static int scmi_clk_gate(struct clk *clk, int enable)
>>          struct scmi_clk_state_out out;
>>          int ret;
>>
>> +       printf("%s name:%s dev:0x%px dev->priv:0x%px     parent->name:%s 
>> parent->dev:0x%px parent->dev->priv:0x%px\n", __FUNCTION__,
>> +               clk->dev->name, clk->dev, dev_get_priv(clk->dev),
>> +               clk->dev->parent->name, clk->dev->parent, 
>> dev_get_priv(clk->dev->parent));
>> +
>>          if (priv->version >= CLOCK_PROTOCOL_VERSION_3_0) {
>>                  struct scmi_clk_state_in_v2 in = {
>>                          .clock_id = clk_get_id(clk),
>> @@ -288,6 +292,8 @@ static int scmi_clk_probe(struct udevice *dev)
>>          size_t num_clocks, i;
>>          int ret;
>>
>> +       printf("%s name:%s dev:0x%px dev->priv:0x%px\n", __FUNCTION__, 
>> dev->name, dev, priv);
>> +
>>          ret = devm_scmi_of_get_channel(dev);
>>          if (ret)
>>                  return ret;
>>
>> And output:
>>
>> DRAM:  510 MiB
>> scmi_clk_probe name:protocol at 14 dev:0x000000041f51fc50 
>> dev->priv:0x000000041f520ae0
>> scmi_clk_gate name:UART0 dev:0x000000041f522710 
>> dev->priv:0x0000000000000000     parent->name:protocol at 14 
>> parent->dev:0x000000041f51fc50 parent->dev->priv:0x000000041f520ae0
>> Core:  132 devices, 22 uclasses, devicetree: board
>> MMC:   scmi_clk_gate name:EMMC_S dev:0x000000041f521950 
>> dev->priv:0x0000000000000000     parent->name:protocol at 14 
>> parent->dev:0x000000041f51fc50 parent->dev->priv:0x000000041f520ae0
>> sdhci at 0: 0
>> Loading Environment from nowhere... OK
>> In:    serial
>> Out:   serial
>> Err:   serial
>>
>> As we can see here, we have two different devs in scmi_clk_probe() and 
>> scmi_clk_gate(). dev from scmi_clk_gate() have NULL in dev->priv. So we 
>> can't get scmi version form clk->dev direcrly.
>>
>> P.S.: I have CONFIG_CLK_CCF=y
> Thanks for the logs Maxim.
>
> Looks like the behaviour is different for CCF and non CCF
>
> We have parent->dev->priv as NULL. (and no prints from probe func for r5)
>
> *r5 core logs*
> U-Boot SPL 2025.01-00667-g8d2958a533ce-dirty (Oct 28 2025 - 18:43:57 +0530)
> SPL initial stack usage: 1984 bytes
> Trying to boot from DFU
> scmi_clk_gate name:protocol at 14 dev:0x82b7a400x dev->priv:0x82b7a588x parent->name:scmi parent->dev:0x82b7a1d0x parent->dev->pr\
> iv:0x0x
> #####DOWNLOAD ... OK
> Ctrl+C to exit ...
> ERROR:   Agent 0 Protocol 0x10 Message 0x7: not supported
My bad both are a53 logs, spl and uboot


>
> *A53 core logs*
> U-Boot 2025.01-00667-g8d2958a533ce-dirty (Oct 28 2025 - 18:43:57 +0530)
>
> SoC:   AM62LX SR1.0 HS-FS
> Model: Texas Instruments AM62L3 Evaluation Module
> DRAM:  2 GiB
> ERROR:   Agent 0 Protocol 0x10 Message 0x7: not supported
> scmi_clk_probe name:protocol at 14 dev:0x00000000fdeb65a0 dev->priv:0x00000000fdeb6f30
> Core:  84 devices, 32 uclasses, devicetree: separate
> MMC:   mmc at fa10000: 0, mmc at fa00000: 1
> Loading Environment from nowhere... OK
>
>
> If you have suggestion for solutions please let me know.
I'm not able to test CCF enabled but I have following that might be a
potential solution

modified   drivers/clk/clk_scmi.c
@@ -130,10 +130,18 @@ static int scmi_clk_get_attibute(struct udevice *dev, int clkid, char **name,
 
 static int scmi_clk_gate(struct clk *clk, int enable)
 {
-	struct scmi_clock_priv *priv = dev_get_priv(clk->dev);
+	struct scmi_clock_priv *priv;
         struct scmi_clk_state_out out;
         int ret;
 
+	if (CONFIG_IS_ENABLED(CLK_CCF)) {
+		/* CCF: version info is in parent device */
+		priv = dev_get_priv(clk->dev->parent);
+	} else {
+		/* Non-CCF: version info is in current device */
+		priv = dev_get_priv(clk->dev);
+	}
+
         printf("%s name:%s dev:0x%px dev->priv:0x%px parent->name:%s parent->dev:0x%px parent->dev->priv:0x%px\n", __FUNCTION__, clk->dev->name, clk->dev, dev_get_priv(clk->dev), clk->dev->parent->name, clk->dev->parent, dev_get_priv(clk->dev->parent));
 	if (priv->version >= CLOCK_PROTOCOL_VERSION_3_0) {
 		struct scmi_clk_state_in_v2 in = {


I have checked with CCF disabled.
Let me know if this works for you as well.

Regards,
Kamlesh