From patchwork Thu Oct 8 09:23:49 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Jon Medhurst \(Tixy\)" X-Patchwork-Id: 54650 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wi0-f197.google.com (mail-wi0-f197.google.com [209.85.212.197]) by patches.linaro.org (Postfix) with ESMTPS id E3D6922FF8 for ; Thu, 8 Oct 2015 09:24:01 +0000 (UTC) Received: by wisv5 with SMTP id v5sf7573705wis.0 for ; Thu, 08 Oct 2015 02:24:01 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:message-id:subject:from:to:cc:date :in-reply-to:references:content-type:mime-version :content-transfer-encoding:sender:precedence:list-id :x-original-sender:x-original-authentication-results:mailing-list :list-post:list-help:list-archive:list-unsubscribe; bh=sj/hyizHLe42nB1FpFp0Zi6JybxG55jz4kzlE4f15og=; b=iOgkgpZKpsTWesy1WCT7fm6sPVY03P8Uopz6b805tM06AEKwuA83tfIpiQ6QYntxWY dZ/NoFVI7Z+m96KbO7mSbdKmjMVptpKZ6wKNz4O2yoRJNfiiJvQVGvY+6Zf+4t0IDlhY 0L6/er9BpJa6shjTd8o2vWJgtOZb8eiSH4E+Df2ywxLFjX6XiV3RteXSPJJF2CbcD649 7HsOJ9lbjhpqWYL8fhOaHtSaSmd1BCiNJCI84p/2HMi1y7plCSUfNz1gyo0vXuZJNRlY GxBwP/30q/NmmaVcxpsOV0UrJkXVpoOaUBzDbCh63A3l1r1rjqnotVM/GsaCRtBUHrD1 S3+g== X-Gm-Message-State: ALoCoQkpdi58U/nyUCG4C9ET+G+AVSis5+xPc3Og5wdLKeRUdoqi/tZ3fZxjjfY3j9uLkUS3Cdj/ X-Received: by 10.112.138.4 with SMTP id qm4mr1170100lbb.6.1444296240852; Thu, 08 Oct 2015 02:24:00 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.25.152.199 with SMTP id a190ls174873lfe.62.gmail; Thu, 08 Oct 2015 02:24:00 -0700 (PDT) X-Received: by 10.112.150.97 with SMTP id uh1mr2522492lbb.53.1444296240712; Thu, 08 Oct 2015 02:24:00 -0700 (PDT) Received: from mail-lb0-f181.google.com (mail-lb0-f181.google.com. [209.85.217.181]) by mx.google.com with ESMTPS id j9si14742742lbc.171.2015.10.08.02.24.00 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 08 Oct 2015 02:24:00 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.181 as permitted sender) client-ip=209.85.217.181; Received: by lbcao8 with SMTP id ao8so40445100lbc.3 for ; Thu, 08 Oct 2015 02:24:00 -0700 (PDT) X-Received: by 10.25.38.9 with SMTP id m9mr1986127lfm.112.1444296240578; Thu, 08 Oct 2015 02:24:00 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.112.59.35 with SMTP id w3csp463471lbq; Thu, 8 Oct 2015 02:23:59 -0700 (PDT) X-Received: by 10.107.16.158 with SMTP id 30mr7330071ioq.50.1444296239602; Thu, 08 Oct 2015 02:23:59 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id q192si30899296ioe.201.2015.10.08.02.23.59; Thu, 08 Oct 2015 02:23:59 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-pm-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754758AbbJHJX6 (ORCPT + 12 others); Thu, 8 Oct 2015 05:23:58 -0400 Received: from queue01a.mail.zen.net.uk ([212.23.3.234]:56790 "EHLO queue01a.mail.zen.net.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754138AbbJHJXz (ORCPT ); Thu, 8 Oct 2015 05:23:55 -0400 Received: from [212.23.1.1] (helo=smarthost01a.mail.zen.net.uk) by queue01a.mail.zen.net.uk with esmtp (Exim 4.72) (envelope-from ) id 1Zk7QX-0006ZO-Rx; Thu, 08 Oct 2015 09:23:53 +0000 Received: from [82.69.122.217] (helo=linaro1) by smarthost01a.mail.zen.net.uk with esmtpsa (TLS1.2:RSA_AES_128_CBC_SHA1:128) (Exim 4.80) (envelope-from ) id 1Zk7QT-00075X-LV; Thu, 08 Oct 2015 09:23:49 +0000 Message-ID: <1444296229.2847.9.camel@linaro.org> Subject: Re: [PATCH] cpufreq: arm_big_little: fix frequency check when bL switcher is active From: "Jon Medhurst (Tixy)" To: Viresh Kumar Cc: Sudeep Holla , "Rafael J. Wysocki" , linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org, Michael Turquette Date: Thu, 08 Oct 2015 10:23:49 +0100 In-Reply-To: <20151007173920.GG4557@linux> References: <1443807532.2845.25.camel@linaro.org> <20151007173920.GG4557@linux> X-Mailer: Evolution 3.12.9-1+b1 Mime-Version: 1.0 X-Originating-smarthost01a-IP: [82.69.122.217] Sender: linux-pm-owner@vger.kernel.org Precedence: list List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: tixy@linaro.org X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.181 as permitted sender) smtp.mailfrom=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 List-Post: , List-Help: , List-Archive: List-Unsubscribe: , On Wed, 2015-10-07 at 23:09 +0530, Viresh Kumar wrote: [...] > And why not fix it properly by doing this: > > diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c > index f1e42f8ce0fc..5b36657a76d6 100644 > --- a/drivers/cpufreq/arm_big_little.c > +++ b/drivers/cpufreq/arm_big_little.c > @@ -128,7 +128,7 @@ static unsigned int bL_cpufreq_get_rate(unsigned int cpu) > static unsigned int > bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) > { > - u32 new_rate, prev_rate; > + u32 new_rate, prev_rate, target_rate; > int ret; > bool bLs = is_bL_switching_enabled(); > > @@ -140,9 +140,11 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) > per_cpu(physical_cluster, cpu) = new_cluster; > > new_rate = find_cluster_maxfreq(new_cluster); > + target_rate = new_rate; > new_rate = ACTUAL_FREQ(new_cluster, new_rate); > } else { > new_rate = rate; > + target_rate = new_rate; > } > > pr_debug("%s: cpu: %d, old cluster: %d, new cluster: %d, freq: %d\n", > @@ -196,7 +198,7 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) > * be reading only the cached value anyway. This needs to be removed > * once clk core is fixed. > */ > - if (bL_cpufreq_get_rate(cpu) != new_rate) > + if (bL_cpufreq_get_rate(cpu) != target_rate) > return -EIO; > return 0; > } You call that a proper fix? ;-) It's comparing an 'virtual' frequency to an 'actual' frequency. If the real intent is to check that clk_set_rate works I would have thought the patch below would be correct. But I didn't propose that as it's the obvious implementation and I assumed the original patch didn't do it that way for a reason. --- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/drivers/cpufreq/arm_big_little.c b/drivers/cpufreq/arm_big_little.c index f1e42f8..59115a4 100644 --- a/drivers/cpufreq/arm_big_little.c +++ b/drivers/cpufreq/arm_big_little.c @@ -149,6 +149,18 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) __func__, cpu, old_cluster, new_cluster, new_rate); ret = clk_set_rate(clk[new_cluster], new_rate * 1000); + if (!ret) { + /* + * FIXME: clk_set_rate has to handle the case where clk_change_rate + * can fail due to hardware or firmware issues. Until the clk core + * layer is fixed, we can check here. In most of the cases we will + * be reading only the cached value anyway. This needs to be removed + * once clk core is fixed. + */ + if (clk_get_rate(clk[new_cluster]) != new_rate * 1000) + ret = -EIO; + } + if (WARN_ON(ret)) { pr_err("clk_set_rate failed: %d, new cluster: %d\n", ret, new_cluster); @@ -189,15 +201,6 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) mutex_unlock(&cluster_lock[old_cluster]); } - /* - * FIXME: clk_set_rate has to handle the case where clk_change_rate - * can fail due to hardware or firmware issues. Until the clk core - * layer is fixed, we can check here. In most of the cases we will - * be reading only the cached value anyway. This needs to be removed - * once clk core is fixed. - */ - if (bL_cpufreq_get_rate(cpu) != new_rate) - return -EIO; return 0; }