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

From: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
To: AngeloGioacchino Del Regno  <angelogioacchino.delregno@collabora.com>
Cc: Konrad Dybcio <konrad.dybcio@linaro.org>,
	Andy Gross <agross@kernel.org>,
	Bjorn Andersson <andersson@kernel.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Viresh Kumar <vireshk@kernel.org>, Nishanth Menon <nm@ti.com>,
	Stephen Boyd <sboyd@kernel.org>, Niklas Cassel <nks@flawful.org>,
	Liam Girdwood <lgirdwood@gmail.com>,
	Mark Brown <broonie@kernel.org>,
	Robert Marko <robimarko@gmail.com>,
	linux-kernel@vger.kernel.org, linux-arm-msm@vger.kernel.org,
	devicetree@vger.kernel.org, linux-pm@vger.kernel.org,
	AngeloGioacchino Del Regno 
	<angelogioacchino.delregno@somainline.org>
Subject: Re: [PATCH v10 5/6] soc: qcom: Add support for Core Power Reduction v3, v4 and Hardened
Date: Mon, 27 Feb 2023 15:20:39 +0200	[thread overview]
Message-ID: <CAA8EJpprXe3k6Kecg6v-QHT-qP=QjimFZFpLWjPqky3M=J+x+A@mail.gmail.com> (raw)
In-Reply-To: <8a813713-c60d-4726-0c62-de032db99ede@collabora.com>

On Mon, 27 Feb 2023 at 15:06, AngeloGioacchino Del Regno
<angelogioacchino.delregno@collabora.com> 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@vger.kernel.org/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)
SoCs. Other than that, I have no additional comments for that series.

-- 
With best wishes
Dmitry