Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751435AbdFZKPg (ORCPT ); Mon, 26 Jun 2017 06:15:36 -0400 Received: from mail-it0-f67.google.com ([209.85.214.67]:36430 "EHLO mail-it0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751353AbdFZKPa (ORCPT ); Mon, 26 Jun 2017 06:15:30 -0400 MIME-Version: 1.0 In-Reply-To: <20170626055331.GB31066@plaes.org> References: <20170626055331.GB31066@plaes.org> From: Jonathan Liu Date: Mon, 26 Jun 2017 20:15:28 +1000 Message-ID: Subject: Re: [linux-sunxi] [PATCH v4 1/6] clk: sunxi-ng: div: Add support for fixed post-divider To: Priit Laes Cc: Michael Turquette , Stephen Boyd , Rob Herring , Mark Rutland , Maxime Ripard , Chen-Yu Tsai , Russell King , Philipp Zabel , linux-clk@vger.kernel.org, devicetree , linux-arm-kernel , linux-kernel , linux-sunxi Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3986 Lines: 109 Hi Priit, On 26 June 2017 at 15:53, Priit Laes wrote: > On Mon, Jun 26, 2017 at 08:05:16AM +1000, Jonathan Liu wrote: >> Hi Priit, >> >> This is showing from clock rate of 171428572 in the output of "cat >> /sys/kernel/debug/clk/clk_summary" for pll-periph-sata. >> The clock rate should be 100000000 (100 MHz) when read from the hardware. > > This is what I see on Cubietruck. Can you check with sunxi-ccu-wip branch? > > $ cat /sys/kernel/debug/clk/clk_summary |grep -i sata > pll-periph-sata 1 1 100000000 0 0 > sata 1 1 100000000 0 0 > ahb-sata 1 1 300000000 0 0 > I checked with your sunxi-ccu-wip branch and I get the same results as you. My branch seems out of date. >> >> On 26 June 2017 at 06:45, Priit Laes wrote: >> > SATA clock on sun4i/sun7i is of type (parent) / M / 6 where >> > 6 is fixed post-divider. >> > >> > Signed-off-by: Priit Laes >> > --- >> > drivers/clk/sunxi-ng/ccu_div.c | 12 ++++++++++-- >> > drivers/clk/sunxi-ng/ccu_div.h | 3 ++- >> > 2 files changed, 12 insertions(+), 3 deletions(-) >> > >> > diff --git a/drivers/clk/sunxi-ng/ccu_div.c b/drivers/clk/sunxi-ng/ccu_div.c >> > index c0e5c10..de30e15 100644 >> > --- a/drivers/clk/sunxi-ng/ccu_div.c >> > +++ b/drivers/clk/sunxi-ng/ccu_div.c >> >> Missing handling of fixed_post_div in ccu_div_round_rate. >> ccu_div_round_rate should multiply the rate by the postdiv before >> looking up the divider using divider_get_val and divide val by postdiv >> just before returning. >> Ignore the above comment, my branch is out of date. >> > @@ -62,8 +62,13 @@ static unsigned long ccu_div_recalc_rate(struct clk_hw *hw, >> > parent_rate = ccu_mux_helper_apply_prediv(&cd->common, &cd->mux, -1, >> > parent_rate); >> > >> > - return divider_recalc_rate(hw, parent_rate, val, cd->div.table, >> > - cd->div.flags); >> > + val = divider_recalc_rate(hw, parent_rate, val, cd->div.table, >> > + cd->div.flags); >> > + >> > + if (cd->common.features & CCU_FEATURE_FIXED_POSTDIV) >> > + val /= cd->fixed_post_div; >> > + >> > + return val; >> > } >> > >> > static int ccu_div_determine_rate(struct clk_hw *hw, >> >> > @@ -89,6 +94,9 @@ static int ccu_div_set_rate(struct clk_hw *hw, unsigned long rate, >> > val = divider_get_val(rate, parent_rate, cd->div.table, cd->div.width, >> > cd->div.flags); >> > >> > + if (cd->common.features & CCU_FEATURE_FIXED_POSTDIV) >> > + val *= cd->fixed_post_div; >> > + >> > spin_lock_irqsave(cd->common.lock, flags); >> > >> > reg = readl(cd->common.base + cd->common.reg); >> >> val here is the divider value that is stored in the clock register >> field not the actual rate so this is incorrect. Instead the rate needs >> to be multiplied by postdiv just before divider_get_val. > > OK, thanks, this one I wasn't really sure about. > Please do fix setting the rate though. >> >> > diff --git a/drivers/clk/sunxi-ng/ccu_div.h b/drivers/clk/sunxi-ng/ccu_div.h >> > index 08d0744..f3a5028 100644 >> > --- a/drivers/clk/sunxi-ng/ccu_div.h >> > +++ b/drivers/clk/sunxi-ng/ccu_div.h >> > @@ -86,9 +86,10 @@ struct ccu_div_internal { >> > struct ccu_div { >> > u32 enable; >> > >> > - struct ccu_div_internal div; >> > + struct ccu_div_internal div; >> > struct ccu_mux_internal mux; >> > struct ccu_common common; >> > + unsigned int fixed_post_div; >> > }; >> > >> > #define SUNXI_CCU_DIV_TABLE_WITH_GATE(_struct, _name, _parent, _reg, \ >> > -- >> > git-series 0.9.1 >> >> Thanks. >> >> Regards, >> Jonathan Regards, Jonathan