From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754873AbdDLQqI (ORCPT ); Wed, 12 Apr 2017 12:46:08 -0400 Received: from smtp.codeaurora.org ([198.145.29.96]:55424 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752608AbdDLQqH (ORCPT ); Wed, 12 Apr 2017 12:46:07 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 smtp.codeaurora.org DDBF560D0C Authentication-Results: pdx-caf-mail.web.codeaurora.org; dmarc=none (p=none dis=none) header.from=codeaurora.org Authentication-Results: pdx-caf-mail.web.codeaurora.org; spf=none smtp.mailfrom=sboyd@codeaurora.org Date: Wed, 12 Apr 2017 09:46:05 -0700 From: Stephen Boyd To: Peter De Schrijver Cc: Michael Turquette , linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] clk: Re-evaluate clock rate on min/max update Message-ID: <20170412164605.GO7065@codeaurora.org> References: <1490103807-21821-1-git-send-email-pdeschrijver@nvidia.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1490103807-21821-1-git-send-email-pdeschrijver@nvidia.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 03/21, Peter De Schrijver wrote: > Whenever a user change its min or max rate limit of a clock, we need to > re-evaluate the current clock rate and possibly change it if the new limits > require so. To do this clk_set_rate_range() already calls > clk_core_set_rate_nolock, however this won't have the intended effect > because the core clock rate hasn't changed. To fix this, move the test to > avoid setting the same core clock rate again, to clk_set_rate() so > clk_core_set_rate_nolock() can change the clock rate when min or max have > been updated, even when the core clock rate has not changed. I'd expect some sort of Fixes: tag here? Or it never worked!? > > Signed-off-by: Peter De Schrijver I seem to recall some problems here around rate aggregation that we fixed after the patches merged. Sorry, but I have to go back and look at those conversations to refresh my memory and make sure this is all fine. Are you relying on the rate setting op to be called with the new min/max requirements if the aggregated rate is the same? I don't understand why clk drivers care. > --- > drivers/clk/clk.c | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index 2fa2fb8..0b815d1 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1569,10 +1569,6 @@ static int clk_core_set_rate_nolock(struct clk_core *core, > if (!core) > return 0; > > - /* bail early if nothing to do */ > - if (rate == clk_core_get_rate_nolock(core)) > - return 0; > - > if ((core->flags & CLK_SET_RATE_GATE) && core->prepare_count) > return -EBUSY; > > @@ -1621,16 +1617,21 @@ static int clk_core_set_rate_nolock(struct clk_core *core, > */ > int clk_set_rate(struct clk *clk, unsigned long rate) > { > - int ret; > + int ret = 0; > > if (!clk) > - return 0; > + return ret; Why? Noise? > > /* prevent racing with updates to the clock topology */ > clk_prepare_lock(); > > + /* bail early if nothing to do */ > + if (rate == clk_core_get_rate_nolock(clk->core)) > + goto out; > + > ret = clk_core_set_rate_nolock(clk->core, rate); > > +out: > clk_prepare_unlock(); > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project