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

[Zhongqiu Han]

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@xxxxxx>
> > > ---
> > >   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