Re: [v5, PATCH 4/5] cpufreq: mediatek: add opp notification for SVS support

On 09-12-19, 14:56, andrew-sh.cheng wrote:

On Wed, 2019-11-27 at 14:06 +0530, Viresh Kumar wrote:

quoted

On 26-11-19, 19:50, Andrew-sh.Cheng wrote:

quoted

+		if (!IS_ERR(opp_item))
+			dev_pm_opp_put(opp_item);
+		else
+			freq = 0;
+

What is the purpose of the above code ?

When dev_pm_opp_find_freq_ceil() doesn't find matching opp item, freq
value won't be set.
Set it as 0 for below checking

quoted

quoted

+		/* case of current opp is disabled */
+		if (freq == 0 || freq != info->opp_freq) {
+			// find an enable opp item
+			freq = 1;
+			opp_item = dev_pm_opp_find_freq_ceil(info->cpu_dev,
+							     &freq);
+			if (!IS_ERR(opp_item)) {
+				dev_pm_opp_put(opp_item);
+				policy = cpufreq_cpu_get(info->opp_cpu);
+				if (policy) {
+					cpufreq_driver_target(policy,
+						freq / 1000,
+						CPUFREQ_RELATION_L);

Why don't you simply call this instead of all the code in the else
block ?

These else code is used to check "current opp item is disabled or not".
If not, do nothing.
If current opp item is disabled, need to find an not-disabled opp item,
and set frequency to it.

Right. So this notifier helper of yours receive the opp which is getting
disabled, why don't you compare its frequency directly to see if the current OPP
is getting disabled ?

-- 
viresh