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

[Akashdeep Kaur <a-kaur@ti.com>]

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.
> 
>>   };
>>   /**
>> @@ -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) {
> 
>