Hello Rafael,
On Fri, May 15, 2020 at 05:58:47PM +0200, Rafael J. Wysocki wrote:
On 5/6/2020 7:42 PM, Sergey.Semin@baikalelectronics.ru wrote:
From: Serge Semin Sergey.Semin@baikalelectronics.ru
Recent commit e61a41256edf ("cpufreq: dev_pm_qos_update_request() can return 1 on success") fixed a problem when active policies traverse was falsely stopped due to invalidly treating the non-zero return value from freq_qos_update_request() method as an error. Yes, that function can return positive values if the requested update actually took place. The current problem is that the returned value is then passed to the return cell of the cpufreq_boost_set_sw() (set_boost callback) method. This value is then also analyzed for being non-zero, which is also treated as having an error. As a result during the boost activation we'll get an error returned while having the QOS frequency update successfully performed. Fix this by returning a negative value from the cpufreq_boost_set_sw() if actual error was encountered and zero otherwise treating any positive values as the successful operations completion.
Fixes: 18c49926c4bf ("cpufreq: Add QoS requests for userspace constraints") Signed-off-by: Serge Semin Sergey.Semin@baikalelectronics.ru Acked-by: Viresh Kumar viresh.kumar@linaro.org Cc: Alexey Malahov Alexey.Malahov@baikalelectronics.ru Cc: Thomas Bogendoerfer tsbogend@alpha.franken.de Cc: Paul Burton paulburton@kernel.org Cc: Ralf Baechle ralf@linux-mips.org Cc: Arnd Bergmann arnd@arndb.de Cc: Rob Herring robh+dt@kernel.org Cc: linux-mips@vger.kernel.org Cc: devicetree@vger.kernel.org Cc: stable@vger.kernel.org
drivers/cpufreq/cpufreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 045f9fe157ce..5870cdca88cf 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -2554,7 +2554,7 @@ static int cpufreq_boost_set_sw(int state) break; }
- return ret;
- return ret < 0 ? ret : 0; } int cpufreq_boost_trigger_state(int state)
IMO it is better to update the caller of this function to handle the positive value possibly returned by it correctly.
Could you elaborate why? Viresh seems to be ok with this solution.
As I see it the caller doesn't expect the positive value returned by the original freq_qos_update_request(). It just doesn't need to know whether the effective policy has been updated or not, it only needs to make sure the operations has been successful. Moreover the positive value is related only to the !last! active policy, which doesn't give the caller a full picture of the policy change anyway. So taking all of these into account I'd leave the fix as is.
Regards, -Sergey
Thanks!