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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20241129040234.enfb2vpehpzhmtmc@vireshk-i7>
Date: Fri, 29 Nov 2024 09:32:34 +0530
From: Viresh Kumar <viresh.kumar@...aro.org>
To: Christian Marangi <ansuelsmth@...il.com>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, linux-kernel@...r.kernel.org,
	linux-pm@...r.kernel.org, Lorenzo Bianconi <lorenzo@...nel.org>,
	upstream@...oha.com, ulf.hansson@...aro.org
Subject: Re: [PATCH v2] cpufreq: airoha: Add EN7581 Cpufreq SMC driver

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