Re: [PATCH v4 1/1] cpufreq: ti: Add EPROBE_DEFER for K3 SoCs

[Zhongqiu Han <zhongqiu.han@oss.qualcomm.com>]

On 6/3/2026 3:23 PM, Akashdeep Kaur wrote:
> Hi Zhongqiu,
> 
> Thanks for the feedback!
> 
> On 01/06/26 08:58, Zhongqiu Han wrote:
>> On 5/28/2026 5:05 PM, Akashdeep Kaur wrote:
>>> On K3 SoCs, ti-cpufreq relies on k3-socinfo to register the SoC
>>> device before soc_device_match() can return valid revision
>>> information. If ti-cpufreq probes before k3-socinfo,
>>> soc_device_match() returns NULL, leading to incorrect CPU frequency
>>> scaling behavior.
>>> Defer probe when k3-socinfo hasn't registered the SoC device yet.
>>
>>
>> Hi Akashdeep,
>>
>> Thanks for the update.
>>
>>
>>>
>>> Signed-off-by: Akashdeep Kaur <a-kaur@ti.com>
>>> ---
>>>   drivers/cpufreq/ti-cpufreq.c | 12 ++++++++++++
>>>   1 file changed, 12 insertions(+)
>>>
>>> diff --git a/drivers/cpufreq/ti-cpufreq.c b/drivers/cpufreq/ti-cpufreq.c
>>> index a01abc1622eb..8219751da175 100644
>>> --- a/drivers/cpufreq/ti-cpufreq.c
>>> +++ b/drivers/cpufreq/ti-cpufreq.c
>>> @@ -99,6 +99,7 @@ struct ti_cpufreq_soc_data {
>>>       unsigned long efuse_shift;
>>>       unsigned long rev_offset;
>>>       bool multi_regulator;
>>> +    bool needs_k3_socinfo;
>>>   /* Backward compatibility hack: Might have missing syscon */
>>>   #define TI_QUIRK_SYSCON_MAY_BE_MISSING    0x1
>>>   /* Backward compatibility hack: new syscon size is 1 register wide */
>>> @@ -347,6 +348,7 @@ static struct ti_cpufreq_soc_data am625_soc_data = {
>>>       .efuse_mask = 0x07c0,
>>>       .efuse_shift = 0x6,
>>>       .multi_regulator = false,
>>> +    .needs_k3_socinfo = true,
>>>       .quirks = TI_QUIRK_SYSCON_IS_SINGLE_REG,
>>>   };
>>> @@ -356,6 +358,7 @@ static struct ti_cpufreq_soc_data am62a7_soc_data 
>>> = {
>>>       .efuse_mask = 0x07c0,
>>>       .efuse_shift = 0x6,
>>>       .multi_regulator = false,
>>> +    .needs_k3_socinfo = true,
>>>   };
>>>   static struct ti_cpufreq_soc_data am62l3_soc_data = {
>>> @@ -364,6 +367,7 @@ static struct ti_cpufreq_soc_data am62l3_soc_data 
>>> = {
>>>       .efuse_mask = 0x07c0,
>>>       .efuse_shift = 0x6,
>>>       .multi_regulator = false,
>>> +    .needs_k3_socinfo = true,
>>>   };
>>>   static struct ti_cpufreq_soc_data am62p5_soc_data = {
>>> @@ -372,6 +376,7 @@ static struct ti_cpufreq_soc_data am62p5_soc_data 
>>> = {
>>>       .efuse_mask = 0x07c0,
>>>       .efuse_shift = 0x6,
>>>       .multi_regulator = false,
>>> +    .needs_k3_socinfo = true,
>>
>>
>> It seems that `ti_cpufreq_soc_data` is a hardware description–only
>> structure.
>>
>> After further consideration, may I know would it be reasonable to
>> consider using of_machine_get_match() instead, for example:
>>
>> static const struct of_device_id ti_k3_cpufreq_of_match[] = {
>>               { .compatible = "ti,am625"  },
>>               { .compatible = "ti,am62xx" },
>>                 ......
>>               {}
>>       };
>>
>> if (soc_device_match(k3_cpufreq_soc)) {
>>      *revision_value = 0x1;
>>      goto done;
>> } else if (of_machine_get_match(ti_k3_of_match)) {
>>      return dev_err_probe(xxx);
>> }
>>
>> Just a thought — you might want to consider this if it makes sense.
>> Sorry for not pointing this out earlier, I may have missed it befor
> 
> The structure already contains driver behavior flags alongside hardware 
> descriptions:
> 
>    1. multi_regulator (bool) - Controls whether the driver uses multiple 
> regulators (driver behavior, not HW description)
>    2. quirks (u8) - Explicitly documented as "Backward compatibility 
> hack" flags that control driver behavior:
>      - TI_QUIRK_SYSCON_MAY_BE_MISSING
>      - TI_QUIRK_SYSCON_IS_SINGLE_REG
> 
>    The needs_k3_socinfo flag follows this existing pattern - it's a 
> driver behavior flag that controls probe deferral logic, similar to how 
> multi_regulator controls regulator configuration and quirks control 
> backward compatibility handling.


Thanks for the clarification.


>>
>>>   };
>>>   /**
>>> @@ -443,6 +448,12 @@ static int ti_cpufreq_get_rev(struct 
>>> ti_cpufreq_data *opp_data,
>>>           goto done;
>>>       }
>>>
>>> +    /* Defer if k3-socinfo hasn't registered the SoC device yet */
>>> +    if (opp_data->soc_data->needs_k3_socinfo) {
>>> +        dev_dbg(opp_data->cpu_dev,
>>> +            "SoC info not ready yet, deferring probe\n");
>>> +        return -EPROBE_DEFER;
>>
>>
>> It would be better to use dev_err_probe() here and return the original
>> error, instead of using dev_dbg() with -EPROBE_DEFER.
>>
>> Also, the message "SoC info not ready yet" seems a bit too generic and
>> not very descriptive. Which SoC is this referring to?
> 
> Thats good suggestion, will update both in next version.
> 
> Thanks,
> Akashdeep Kaur
>>
>>
>>> +    }
>>> +
>>>       ret = regmap_read(opp_data->syscon, opp_data->soc_data- 
>>> >rev_offset,
>>>                 &revision);
>>>       if (opp_data->soc_data->quirks & TI_QUIRK_SYSCON_MAY_BE_MISSING 
>>> && ret == -EIO) {
>>
>>
> 


-- 
Thx and BRs,
Zhongqiu Han