Re: [PATCH v10 5/6] soc: qcom: Add support for Core Power Reduction v3, v4 and Hardened

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Il 27/02/23 14:20, Dmitry Baryshkov ha scritto:
On Mon, 27 Feb 2023 at 15:06, AngeloGioacchino Del Regno
<angelogioacchino.delregno@xxxxxxxxxxxxx> wrote:

Il 27/02/23 13:01, Dmitry Baryshkov ha scritto:

I took a glance at the 'cpufreq: qcom-hw: Implement CPRh aware OSM programming'
patch, it doesn't seem to use the header (maybe I checked the older version of the
patch). As for me, this is another signal that cpr_ext_data should come together
with the LUT programming rather than with the CPRh itself.

Konrad, perhaps you can send the cpufreq-hw commits in a separate series, in
which cover letter you mention a dependency on this one?
That would *clearly* show the full picture to reviewers.

Yes, that would be great. A small note regarding those patches. I see that you
patched the qcom-cpufreq-hw.c. This way first the driver programs the LUT, then it
reads it back to setup the OPPs. Would it be easier to split OSM-not-programmed
driver?


When I engineered that solution, I kept the cpufreq-hw reading *again* the values
from OSM to keep the driver *fully* compatible with the bootloader-programmed OSM
flow, which makes one thing (in my opinion) perfectly clear: that programming
sequence is exactly the same as what happens "under the hood" on SDM845 (and later)
but performed here-instead-of-there (linux instead of bootloader), with the actual
scaling driver being 100% the same between the two flows in the end.

Having two drivers as you suggested would indeed achieve the same, but wouldn't be
any easier... if you do that, you'd have to *somehow* make sure that the
programming driver does its job before the cpufreq driver tries to read the OSM
status, adding one more link to an already long chain.

Besides, I remember that this question got asked a while ago on the mailing lists
and there was a short discussion about it:

https://www.mail-archive.com/linux-kernel@xxxxxxxxxxxxxxx/msg2555580.html

Ack, I see. Maybe splitting LUT programming to a separate source file
would emphasise the fact that it is only required for some (older)

Maybe. I'm not sure it's worth adding a new helper file, but I don't really have
any strong arguments against...

Konrad, your call.

Cheers!
Angelo

SoCs. Other than that, I have no additional comments for that series.




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux