From patchwork Tue Mar 5 16:38:36 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 2220411 Return-Path: X-Original-To: patchwork-linux-pm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork1.kernel.org (Postfix) with ESMTP id DF7C03FCF6 for ; Tue, 5 Mar 2013 16:38:58 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757425Ab3CEQij (ORCPT ); Tue, 5 Mar 2013 11:38:39 -0500 Received: from mail-oa0-f54.google.com ([209.85.219.54]:48800 "EHLO mail-oa0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752977Ab3CEQih (ORCPT ); Tue, 5 Mar 2013 11:38:37 -0500 Received: by mail-oa0-f54.google.com with SMTP id n12so11189016oag.13 for ; Tue, 05 Mar 2013 08:38:36 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=mime-version:x-received:in-reply-to:references:date:message-id :subject:from:to:cc:content-type:x-gm-message-state; bh=G/jZDcFDP5x35nuBsiyx9VNm76mdh+IPVoRxylTDyno=; b=F08dZ+fxHZvnYKch/2BhouVYg+BodTqLYg+maFgGd07OLFdCw3caDR3qx/iIlyBiGK Eliienkwtyf7XZbtuEv7B6ErpzcNHQg5J3hPk+2hLL9uJ/dqh8EoAMwYYEBQ5mNXHrc9 EDMgjFi4RPQP+xaoqhi+MtPD17/xescVS6SVimgB6ByT8FYnW5Bv8B3zbhuczpapDq4k Pu64wzSA7FZDvQ/75Cfcwo1/nr5H0epTmhcyaNM2lULHg35qllulcAilEJpQ1szNyNWe h/Yw+LstqbjmH4Keq3TDNd8kDXi3DSCk7SdcCfSI90PTeVBluJIJVqwgfUFaNHPxg5mk S2Ag== MIME-Version: 1.0 X-Received: by 10.60.170.177 with SMTP id an17mr19463299oec.62.1362501516537; Tue, 05 Mar 2013 08:38:36 -0800 (PST) Received: by 10.182.69.20 with HTTP; Tue, 5 Mar 2013 08:38:36 -0800 (PST) In-Reply-To: <20130305105251.GL17833@n2100.arm.linux.org.uk> References: <20130305105251.GL17833@n2100.arm.linux.org.uk> Date: Wed, 6 Mar 2013 00:38:36 +0800 Message-ID: Subject: Re: [PATCH] cpufreq: ARM big LITTLE: Add generic cpufreq driver and its DT glue From: Viresh Kumar To: Russell King - ARM Linux Cc: rjw@sisk.pl, Steve.Bannister@arm.com, linux-pm@vger.kernel.org, Sudeep KarkadaNagesha , Liviu.Dudau@arm.com, linux-kernel@vger.kernel.org, cpufreq@vger.kernel.org, robin.randhawa@arm.com, linux-arm-kernel@lists.infradead.org, mark.hambleton@broadcom.com, linaro-kernel@lists.linaro.org, charles.garcia-tobin@arm.com X-Gm-Message-State: ALoCoQlN7Bl7myxjJLS72CMwoJ7ldJEcHtYQCWJPPHyDbPgCYeIFCNPIvq9g7YXz9Jqd5GfZpCG1 Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org On 5 March 2013 18:52, Russell King - ARM Linux wrote: > On Tue, Mar 05, 2013 at 12:52:41PM +0800, Viresh Kumar wrote: >> +static void put_cluster_clk_and_freq_table(u32 cluster) >> +{ >> + if (!atomic_dec_return(&cluster_usage[cluster])) { >> + clk_put(clk[cluster]); >> + clk[cluster] = NULL; > > What's the point in setting the clk to NULL? I couldn't find one and the same is true for freq_table[] too. >> +static int get_cluster_clk_and_freq_table(u32 cluster) >> +{ >> + char name[9] = "cluster"; >> + int count; >> + >> + if (atomic_inc_return(&cluster_usage[cluster]) != 1) >> + return 0; >> + >> + freq_table[cluster] = arm_bL_ops->get_freq_tbl(cluster, &count); >> + if (!freq_table[cluster]) >> + goto atomic_dec; >> + >> + name[7] = cluster + '0'; >> + clk[cluster] = clk_get(NULL, name); >> + if (!IS_ERR_OR_NULL(clk[cluster])) { > > NAK. Two reasons. > > 1. IS_ERR_OR_NULL. You know about this, it's been on the list several > times. AAHHHHHH .. How can i mess up with this concept.. I am really feeling bad now. > 2. Maybe clk_get_sys() rather than clk_get() and use a sensible device > name ("cpu-cluster.%u" maybe) rather than a connection name with a > NULL device? That's a good comment (rather than pointing at some stupid mistake), I will probably keep the same name for the device as well. So how does below fix look to you? ----------x-----------------x----------------- --- 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 0d6de0e..2486b9a 100644 --- a/drivers/cpufreq/arm_big_little.c +++ b/drivers/cpufreq/arm_big_little.c @@ -140,9 +140,7 @@ static void put_cluster_clk_and_freq_table(u32 cluster) { if (!atomic_dec_return(&cluster_usage[cluster])) { clk_put(clk[cluster]); - clk[cluster] = NULL; arm_bL_ops->put_freq_tbl(cluster); - freq_table[cluster] = NULL; pr_debug("%s: cluster: %d\n", __func__, cluster); } } @@ -160,8 +158,8 @@ static int get_cluster_clk_and_freq_table(u32 cluster) goto atomic_dec; name[7] = cluster + '0'; - clk[cluster] = clk_get(NULL, name); - if (!IS_ERR_OR_NULL(clk[cluster])) { + clk[cluster] = clk_get_sys(name, NULL); + if (!IS_ERR(clk[cluster])) { pr_debug("%s: clk: %p & freq table: %p, cluster: %d\n", __func__, clk[cluster], freq_table[cluster], cluster);