From patchwork Tue Oct 13 10:36:25 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sudeep Holla X-Patchwork-Id: 54831 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-wi0-f200.google.com (mail-wi0-f200.google.com [209.85.212.200]) by patches.linaro.org (Postfix) with ESMTPS id 02F6323001 for ; Tue, 13 Oct 2015 10:36:33 +0000 (UTC) Received: by wicid10 with SMTP id id10sf8081647wic.2 for ; Tue, 13 Oct 2015 03:36:32 -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:subject:to:references:cc:from :message-id:date:user-agent:mime-version:in-reply-to:content-type :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=8yOQt6LBwBt3H5ODC9rKqXIsApg3l5Maa8VdBF8PD2s=; b=grIIbQmF6JNvmTfA45hADi4mLEGilJL8UT4HNYm9FeOVWpSDvazS14GyU6DenooYbg vEKPbBq7X6NUdwqHS6GvM+tavFjs5eBpGEaRQEbB2XqWGRpHgQVvVvCTzNUe77mAdcun l4kSy0RGo8zGz6yv4r4AbOGfPR4gQh8HVpd921dG8DaarimXpZFzWLdti24nEp5ee2cb SryaPQazMcpObqxth+gr2fgOiq7z12WUB6ZGt3Ky+D9XeJUY+UrnZ48MjTF93Pq8MMiP L86UkiH3WOLYBJV/kf+Sh52BSAKsMvFZBjskDxrl9IBmjk5pgFeHt5ul/yVTwk25ozh4 gnBg== X-Gm-Message-State: ALoCoQkngwWx4pcCw5nz5E646sc20jyWIt+5QjaWzvh6DerMNcQNEEc1oCLfvbwKEqi3MyPVzfV+ X-Received: by 10.112.158.202 with SMTP id ww10mr6895061lbb.13.1444732592262; Tue, 13 Oct 2015 03:36:32 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.25.156.78 with SMTP id f75ls629245lfe.69.gmail; Tue, 13 Oct 2015 03:36:32 -0700 (PDT) X-Received: by 10.112.131.70 with SMTP id ok6mr14638946lbb.71.1444732592125; Tue, 13 Oct 2015 03:36:32 -0700 (PDT) Received: from mail-lb0-f175.google.com (mail-lb0-f175.google.com. [209.85.217.175]) by mx.google.com with ESMTPS id oi8si1706516lbb.152.2015.10.13.03.36.32 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 13 Oct 2015 03:36:32 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.217.175 as permitted sender) client-ip=209.85.217.175; Received: by lbbk10 with SMTP id k10so14654450lbb.0 for ; Tue, 13 Oct 2015 03:36:32 -0700 (PDT) X-Received: by 10.112.202.35 with SMTP id kf3mr14671722lbc.19.1444732591946; Tue, 13 Oct 2015 03:36:31 -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 w3csp2045576lbq; Tue, 13 Oct 2015 03:36:31 -0700 (PDT) X-Received: by 10.50.117.38 with SMTP id kb6mr18646677igb.92.1444732590938; Tue, 13 Oct 2015 03:36:30 -0700 (PDT) Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id k191si2317850iok.29.2015.10.13.03.36.30; Tue, 13 Oct 2015 03:36:30 -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 S1751819AbbJMKg3 (ORCPT + 12 others); Tue, 13 Oct 2015 06:36:29 -0400 Received: from foss.arm.com ([217.140.101.70]:51754 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751733AbbJMKg2 (ORCPT ); Tue, 13 Oct 2015 06:36:28 -0400 Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.72.51.249]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 6EDC93C; Tue, 13 Oct 2015 03:36:26 -0700 (PDT) Received: from [10.1.207.150] (e103737-lin.cambridge.arm.com [10.1.207.150]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 739223F4F6; Tue, 13 Oct 2015 03:36:27 -0700 (PDT) Subject: Re: [PATCH] cpufreq: arm_big_little: fix frequency check when bL switcher is active To: "Jon Medhurst (Tixy)" References: <1443807532.2845.25.camel@linaro.org> <20151007173920.GG4557@linux> <1444296229.2847.9.camel@linaro.org> <561BB39A.4020400@arm.com> <1444720744.2686.10.camel@linaro.org> Cc: Sudeep Holla , Viresh Kumar , "Rafael J. Wysocki" , linux-pm@vger.kernel.org, linux-kernel@vger.kernel.org From: Sudeep Holla Message-ID: <561CDEA9.7020700@arm.com> Date: Tue, 13 Oct 2015 11:36:25 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 MIME-Version: 1.0 In-Reply-To: <1444720744.2686.10.camel@linaro.org> 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: sudeep.holla@arm.com 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.175 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 13/10/15 08:19, Jon Medhurst (Tixy) wrote: > On Mon, 2015-10-12 at 14:20 +0100, Sudeep Holla wrote: >> >> On 08/10/15 10:23, Jon Medhurst (Tixy) wrote: >> [...] >> >>> 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; >>> } >>> >>> >>> >>> >> >> The above change looks good to me but with minor nit. You can get rid of >> if(!ret) check if you move the hunk after if (WARN_ON(ret)) > > But then we wouldn't get the WARN_ON and pr_err triggered when we detect > the clock rate isn't set, which surely is half the reason for the check > in the first place? > Not sure if I understand what you mean or may be I was not clear, so thought I will put the delta here. Let me know if and how its still a problem. Regards, Sudeep -->8 if (old_cluster != new_cluster) { pr_debug("%s: cpu: %d, old cluster: %d, new cluster: %d\n", @@ -189,15 +199,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; } Acked-by: Sudeep Holla --- 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 i/drivers/cpufreq/arm_big_little.c w/drivers/cpufreq/arm_big_little.c index f1e42f8ce0fc..05e850f80f39 100644 --- i/drivers/cpufreq/arm_big_little.c +++ w/drivers/cpufreq/arm_big_little.c @@ -164,6 +164,16 @@ bL_cpufreq_set_rate(u32 cpu, u32 old_cluster, u32 new_cluster, u32 rate) mutex_unlock(&cluster_lock[new_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; + /* Recalc freq for old cluster when switching clusters */