From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1AAD9C433E3 for ; Sat, 16 May 2020 12:52:14 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 063F9207CB for ; Sat, 16 May 2020 12:52:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726249AbgEPMwK (ORCPT ); Sat, 16 May 2020 08:52:10 -0400 Received: from mail.baikalelectronics.com ([87.245.175.226]:40058 "EHLO mail.baikalelectronics.ru" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726237AbgEPMwK (ORCPT ); Sat, 16 May 2020 08:52:10 -0400 Received: from localhost (unknown [127.0.0.1]) by mail.baikalelectronics.ru (Postfix) with ESMTP id 43AD88030802; Sat, 16 May 2020 12:52:07 +0000 (UTC) X-Virus-Scanned: amavisd-new at baikalelectronics.ru Received: from mail.baikalelectronics.ru ([127.0.0.1]) by localhost (mail.baikalelectronics.ru [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id PpvYz0io-J5L; Sat, 16 May 2020 15:52:05 +0300 (MSK) Date: Sat, 16 May 2020 15:52:03 +0300 From: Serge Semin To: "Rafael J. Wysocki" CC: Serge Semin , Thomas Bogendoerfer , "Rafael J. Wysocki" , Viresh Kumar , Ulf Hansson , Matthias Kaehlcke , Alexey Malahov , Paul Burton , Ralf Baechle , Arnd Bergmann , Rob Herring , , , , Frederic Weisbecker , Ingo Molnar , Yue Hu , , Subject: Re: [PATCH v2 20/20] cpufreq: Return zero on success in boost sw setting Message-ID: <20200516125203.et5gkv6ullkerjyd@mobilestation> References: <20200306124807.3596F80307C2@mail.baikalelectronics.ru> <20200506174238.15385-1-Sergey.Semin@baikalelectronics.ru> <20200506174238.15385-21-Sergey.Semin@baikalelectronics.ru> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: X-ClientProxiedBy: MAIL.baikal.int (192.168.51.25) To mail (192.168.51.25) Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org 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 > > > > 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 > > Acked-by: Viresh Kumar > > Cc: Alexey Malahov > > Cc: Thomas Bogendoerfer > > Cc: Paul Burton > > Cc: Ralf Baechle > > Cc: Arnd Bergmann > > Cc: Rob Herring > > 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! > >