Re: [PATCH V2 13/15] cpufreq: mediatek: Link CCI device to CPU

From: Kevin Hilman <khilman@baylibre.com>
To: Rex-BC Chen <rex-bc.chen@mediatek.com>,
	rafael@kernel.org, viresh.kumar@linaro.org, robh+dt@kernel.org,
	krzk+dt@kernel.org
Cc: matthias.bgg@gmail.com, jia-wei.chang@mediatek.com,
	roger.lu@mediatek.com, hsinyi@google.com,
	linux-pm@vger.kernel.org, devicetree@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,
	Project_Global_Chrome_Upstream_Group@mediatek.com
Subject: Re: [PATCH V2 13/15] cpufreq: mediatek: Link CCI device to CPU
Date: Wed, 13 Apr 2022 14:41:27 -0700	[thread overview]
Message-ID: <7hlew83blk.fsf@baylibre.com> (raw)
In-Reply-To: <98957e61b040b6c5b6a6b39e6eb661e07e510277.camel@mediatek.com>

Rex-BC Chen <rex-bc.chen@mediatek.com> writes:

[...]

> From the Chanwoo's devfreq passive govonor series, it's impossible to
> let cci devreq probed done before cpufreq because the passive govonor
> will search for cpufreq node and use it.
>
> Ref: function: cpufreq_passive_register_notifier()
>
> https://git.kernel.org/pub/scm/linux/kernel/git/chanwoo/linux.git/commit/?h=devfreq-testing&id=b670978ddc43eb0c60735c3af6e4a370603ab673

Well this is a problem, because CCI depends on CPUfreq, but CPUfreq
depends on CCI, so one of them has to load and then wait for the other.

> After I discuss with Angelo and Jia-wei, we think we are keeping the
> function in target_index and if the cci is not ready we will use the
> voltage which is set by bootloader to prevent high freqeuncy low
> voltage crash. And then we can keep seting the target frequency.
>
 > We assume the setting of bootloader is correct and we can do this.

I'm still not crazy about this because you're lying to the CPUfreq
framework.  It's requesting one OPP, but you're not setting that, you're
just keeping the bootloader frequency.

In my earlier reply, I gave two other options for handling this.

1) set a (temporary) constraint on the voltage regulator so that it
cannot change.

or more clean, IMO:

2) set a CPUfreq policy that restricts available OPPs to ones that will
not break CCI.

Either of these solutions allow you to load the CPUfreq driver early,
and then wait for the CCI driver to be ready before removing the
restrictions.

> For the SoCs that including ci hardware (8183 and 8186), we think it's
> not ok if we don't probe cci correctly.
> If we failed to get cci node, I think we sould return -ENODEV and the
> probe of cpufreq failed.
>
> What do you think the solution?

I think it would be better if CPUfreq probes sucessfully, but restricts
the OPPs available until CCI is ready.  If CCI fails to probe/load, you
still have a working CPUfreq driver, it just has a restricted set of
OPPs.

Kevin

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek