From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752547AbcBYXPB (ORCPT ); Thu, 25 Feb 2016 18:15:01 -0500 Received: from smtp.codeaurora.org ([198.145.29.96]:53128 "EHLO smtp.codeaurora.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751332AbcBYXPA (ORCPT ); Thu, 25 Feb 2016 18:15:00 -0500 Date: Thu, 25 Feb 2016 15:14:58 -0800 From: Stephen Boyd To: Shawn Lin Cc: Michael Turquette , linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2] clk: check the actual phase if get_phase is provided Message-ID: <20160225231458.GH28849@codeaurora.org> References: <1455759499-9622-1-git-send-email-shawn.lin@rock-chips.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1455759499-9622-1-git-send-email-shawn.lin@rock-chips.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 02/18, Shawn Lin wrote: > set_phase does sanity checking of degree and ask sub-driver > to set the degree. If set_phase is limited to support the > degree what the caller need, sub-driver may select a > approximate value and return success state. In this case, it's > inappropriate to assign the degree directly to clk->core->phase. > We should ask sub-driver to decide the strategy. If sub-driver just > want to support accurate degree, it can fail the set_phase. Otherwise, > store the actual degree provided by sub-driver into clk->core->phase > if get_phase is provided. Another improvemnt by this patch is that > we can avoid to do unnecessary set_phase if the request defrees is > already there. > > Signed-off-by: Shawn Lin > > --- Knee jerk reaction is why does the provider code set a phase that isn't requested? Do we need some sort of clk_round_phase() API that parallels clk_round_rate() so that drivers know what phase they're going to get? Or do drivers not care what phase they get when they call clk_set_phase()? > > Changes in v2: > - remove actual_degree to simplify the changes > - bail early if nothing to to > > drivers/clk/clk.c | 12 ++++++++++-- > 1 file changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c > index b4db67a..275e70f 100644 > --- a/drivers/clk/clk.c > +++ b/drivers/clk/clk.c > @@ -1902,6 +1902,10 @@ int clk_set_phase(struct clk *clk, int degrees) > > clk_prepare_lock(); > > + /* bail early if nothing to do */ > + if (degrees == clk->core->phase) > + goto out; > + This could be split out into a different "optimization" patch and applied today. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project