From patchwork Tue Apr 16 19:58:26 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Mike Turquette X-Patchwork-Id: 2450941 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork1.kernel.org Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) by patchwork1.kernel.org (Postfix) with ESMTP id D5FD13FD40 for ; Tue, 16 Apr 2013 19:58:42 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1USC1X-00085o-9E; Tue, 16 Apr 2013 19:58:39 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1USC1U-0006Ee-Pb; Tue, 16 Apr 2013 19:58:36 +0000 Received: from mail-da0-x231.google.com ([2607:f8b0:400e:c00::231]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1USC1R-0006EB-V6 for linux-arm-kernel@lists.infradead.org; Tue, 16 Apr 2013 19:58:34 +0000 Received: by mail-da0-f49.google.com with SMTP id t11so398598daj.22 for ; Tue, 16 Apr 2013 12:58:31 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20120113; h=x-received:content-type:mime-version:content-transfer-encoding:to :from:in-reply-to:cc:references:message-id:user-agent:subject:date :x-gm-message-state; bh=qFyNC5Kw8s3bcpVDcUVnFc/DXpjL70tJzubcesecTIw=; b=kM09J2TsAFiIn8/Zs3i4ciJBMghZvcqfEbDJmVRO540ompnx705E5/ByGU/JRQivAa 4WaUpbyHzlEIkDOkHDwBf+gcT3qfT8ffS++SWdzmz6xKrkExsOeCqgCqjrQ+m5q2c0Dl yJQ/4SIBrCP5rziIcvVSoYU07fTsaGOMJK3S7peMO3T7IO++N/3xhuEi1QX/htndD5jl sf4PPzi6HwCb03RmzPjKMU/Kh+5MdWKrTVva3fQhnviOASrKclwQ4/5nPGQI4tTTNJYL hJX+iOfnu3B9bJ3QJtSJbULDvAMmVjxmrTZ6IaQinfZLDHRzNEcgtSDKA6iHozV/H587 4fdQ== X-Received: by 10.68.238.38 with SMTP id vh6mr5362210pbc.63.1366142311700; Tue, 16 Apr 2013 12:58:31 -0700 (PDT) Received: from localhost (adsl-69-228-93-79.dsl.pltn13.pacbell.net. [69.228.93.79]) by mx.google.com with ESMTPS id ak5sm3897773pac.4.2013.04.16.12.58.29 (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Tue, 16 Apr 2013 12:58:30 -0700 (PDT) MIME-Version: 1.0 To: Soren Brinkmann , From: Mike Turquette In-Reply-To: References: Message-ID: <20130416195826.19887.78037@quantum> User-Agent: alot/0.3.4 Subject: Re: [PATCH] clk: Always notify whole subtree when reparenting Date: Tue, 16 Apr 2013 12:58:26 -0700 X-Gm-Message-State: ALoCoQmSmkj9gp0Y0lqW8QSn/L+fwjK9PaenNawc3nF6zl+70KiGCWvwRoVKjUVyKt82YsWm1hS9 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20130416_155834_106603_047FF76E X-CRM114-Status: GOOD ( 18.82 ) X-Spam-Score: -1.9 (-) X-Spam-Report: SpamAssassin version 3.3.2 on merlin.infradead.org summary: Content analysis details: (-1.9 points) pts rule name description ---- ---------------------- -------------------------------------------------- -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] Cc: Soren Brinkmann , linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org Quoting Soren Brinkmann (2013-04-16 10:06:50) > A clock's notifier count only reflects notifiers which are registered > directly for that clock. A reparent operation though affects the whole > subtree because of a potential rate change. > When issuing the pre rate change notifications only the notifier count > for the clock to be changed is considered and notifiers for subclocks > may never be called. Resulting in clocks in the subtree which have > registered notifiers, may receive a POST_- or ABORT_RATE_CHANGE > notification, without a PRE_RATE_CHANGE_NOTIFICATION. > Therefore always traverse the whole subtree when issueing pre rate > change notifications during a reparent operation. > > Signed-off-by: Soren Brinkmann > --- > This should probably be considered an RFC. There may be smarter ways to > resolve this issue. E.g. forward notifier counts upstream the way it is done > with enable counts. > Hi Soren, Thanks for the fix. Some of the core code has changed enough lately that it is worth going through and consolidating/cleaning up. For instance another nice optimization that is related might be something like: However it remains to be seen whether any drivers rely on PRE_RATE_CHANGE notifiers even if the clock rate stays the same. I don't think I'll take your patch for 3.10 since no one has reported a bug with it yet, but I'll roll it into a larger cleanup series after the merge window. Thanks, Mike > Sören > > > drivers/clk/clk.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 20ce67f..1179cb5 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1481,8 +1481,7 @@ int clk_set_parent(struct clk *clk, struct clk *parent) > } > > /* propagate PRE_RATE_CHANGE notifications */ > - if (clk->notifier_count) > - ret = __clk_speculate_rates(clk, p_rate); > + ret = __clk_speculate_rates(clk, p_rate); > > /* abort if a driver objects */ > if (ret & NOTIFY_STOP_MASK) > -- > 1.8.2.1 diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c index 20ce67f..15e8b41 100644 --- a/drivers/clk/clk.c +++ b/drivers/clk/clk.c @@ -1028,6 +1028,9 @@ static int __clk_speculate_rates(struct clk *clk, unsigned long parent_rate) else new_rate = parent_rate; + if (new_rate == clk->rate) + return ret; + /* abort rate change if a driver returns NOTIFY_BAD or NOTIFY_STOP */ if (clk->notifier_count) ret = __clk_notify(clk, PRE_RATE_CHANGE, clk->rate, new_rate);