Re: [PATCH v2] cpufreq: airoha: Add EN7581 Cpufreq SMC driver

[Viresh Kumar]

On 25-11-24, 13:43, Christian Marangi wrote:
> sorry for the delay... I checked the example and other cpufreq driver
> that register a simple cpufreq-dt. None of the current driver implements
> a full clk provider internally

Yeah, that may be done from platform code for those drivers instead of the
cpufreq driver.

> and I have some fear it might be
> problematic to have mixed stuff, eventually I feel I should implement a
> small clk driver that implements determine_rate, set_rate, a compatible
> and all sort. And still we would have the double reference of OPP
> Index->Freq Clock->OPP index.

One way to avoid that would be to use performance-state stuff and model this as
a genpd. In that case, opp->level field (which you can set as index only in your
case) will be programmed as is by the genpd core. That's why I cc'd Ulf earlier
as it looked more suited to your case.

> I wonder if a much easier and better solution for this is (similar to
> how we do with suspend and resume) add entry in the struct
> cpufreq_dt_platform_data, to permit to define simple .target_index and
> .get and overwrite the default one cpufreq-dt.

Easier, sure. Better, I doubt that :(

The OPP core currently configures a lot of stuff from set-opp API, like clk,
regulators, genpd (performance state), bandwidth, etc and I really want to use
that infrastructure for everyone instead of starting to open code that in
drivers. The suspend/resume callbacks are special as the OPP core doesn't know
what to do in that case and so it was left for the drivers to handle that.

> This permits to both reduce greatly the airoha-cpufreq driver, register
> a simple cpufreq-dt and prevent any kind of overhead. After all the
> .target_index and .get doesn't do anything fancy, they just call the OPP
> set and clk get rate.

Yeah, clk-get is pretty simple but opp-set isn't and then there is scope of
further enhancements / improvements in the future which can be best handled if
we keep that code common for everyone.

> What do you think? Changes are really trivial since this is already
> implemented for suspend and resume.

I think you can model it as a performance state (which lot of platforms are
doing as well), and then you won't have the index->clk->index issue anymore.

-- 
viresh