From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752025AbdLDXSl (ORCPT ); Mon, 4 Dec 2017 18:18:41 -0500 Received: from foss.arm.com ([217.140.101.70]:42774 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751380AbdLDXSh (ORCPT ); Mon, 4 Dec 2017 18:18:37 -0500 Subject: Re: [linux-sunxi] [PATCH 1/2] clk: sunxi-ng: Support fixed post-dividers on MP style clocks To: wens@csie.org, Maxime Ripard , Michael Turquette , Stephen Boyd References: <20171204051912.7485-1-wens@csie.org> <20171204051912.7485-2-wens@csie.org> Cc: linux-arm-kernel@lists.infradead.org, linux-clk@vger.kernel.org, linux-kernel@vger.kernel.org, linux-sunxi@googlegroups.com From: =?UTF-8?Q?Andr=c3=a9_Przywara?= Organization: ARM Ltd. Message-ID: Date: Mon, 4 Dec 2017 23:18:03 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: <20171204051912.7485-2-wens@csie.org> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Chen-Yu, On 04/12/17 05:19, Chen-Yu Tsai wrote: > On the A64, the MMC module clocks are fixed in the new timing mode, > i.e. they do not have a bit to select the mode. These clocks have > a 2x divider somewhere between the clock and the MMC module. > > To be consistent with other SoCs supporting the new timing mode, > we model the 2x divider as a fixed post-divider on the MMC module > clocks. > > To do this, we first add fixed post-divider to the MP style clocks, > which the MMC module clocks are. > > Signed-off-by: Chen-Yu Tsai > --- > drivers/clk/sunxi-ng/ccu_mp.c | 20 ++++++++++++++++++-- > drivers/clk/sunxi-ng/ccu_mp.h | 24 ++++++++++++++++++++++++ > 2 files changed, 42 insertions(+), 2 deletions(-) > > diff --git a/drivers/clk/sunxi-ng/ccu_mp.c b/drivers/clk/sunxi-ng/ccu_mp.c > index 688855e7dc8c..5d0af4051737 100644 > --- a/drivers/clk/sunxi-ng/ccu_mp.c > +++ b/drivers/clk/sunxi-ng/ccu_mp.c > @@ -50,12 +50,19 @@ static unsigned long ccu_mp_round_rate(struct ccu_mux_internal *mux, > unsigned int max_m, max_p; > unsigned int m, p; > > + if (cmp->common.features & CCU_FEATURE_FIXED_POSTDIV) > + rate *= cmp->fixed_post_div; Can't you just initialise fixed_post_div to 1 normally and save the CCU_FEATURE_FIXED_POSTDIV? > + > max_m = cmp->m.max ?: 1 << cmp->m.width; > max_p = cmp->p.max ?: 1 << ((1 << cmp->p.width) - 1); > > ccu_mp_find_best(*parent_rate, rate, max_m, max_p, &m, &p); > + rate = *parent_rate / p / m; > + > + if (cmp->common.features & CCU_FEATURE_FIXED_POSTDIV) > + rate /= cmp->fixed_post_div; > > - return *parent_rate / p / m; > + return rate; > } > > static void ccu_mp_disable(struct clk_hw *hw) > @@ -83,6 +90,7 @@ static unsigned long ccu_mp_recalc_rate(struct clk_hw *hw, > unsigned long parent_rate) > { > struct ccu_mp *cmp = hw_to_ccu_mp(hw); > + unsigned long rate; > unsigned int m, p; > u32 reg; > > @@ -101,7 +109,11 @@ static unsigned long ccu_mp_recalc_rate(struct clk_hw *hw, > p = reg >> cmp->p.shift; > p &= (1 << cmp->p.width) - 1; > > - return (parent_rate >> p) / m; > + rate = (parent_rate >> p) / m; > + if (cmp->common.features & CCU_FEATURE_FIXED_POSTDIV) > + rate /= cmp->fixed_post_div; > + > + return rate; > } > > static int ccu_mp_determine_rate(struct clk_hw *hw, > @@ -129,6 +141,10 @@ static int ccu_mp_set_rate(struct clk_hw *hw, unsigned long rate, > max_m = cmp->m.max ?: 1 << cmp->m.width; > max_p = cmp->p.max ?: 1 << ((1 << cmp->p.width) - 1); > > + /* Adjust target rate according to post-dividers */ > + if (cmp->common.features & CCU_FEATURE_FIXED_POSTDIV) > + rate = rate * cmp->fixed_post_div; > + > ccu_mp_find_best(parent_rate, rate, max_m, max_p, &m, &p); > > spin_lock_irqsave(cmp->common.lock, flags); > diff --git a/drivers/clk/sunxi-ng/ccu_mp.h b/drivers/clk/sunxi-ng/ccu_mp.h > index aaef11d747ea..5107635e61de 100644 > --- a/drivers/clk/sunxi-ng/ccu_mp.h > +++ b/drivers/clk/sunxi-ng/ccu_mp.h > @@ -33,9 +33,33 @@ struct ccu_mp { > struct ccu_div_internal m; > struct ccu_div_internal p; > struct ccu_mux_internal mux; > + > + unsigned int fixed_post_div; > + > struct ccu_common common; > }; > > +#define SUNXI_CCU_MP_WITH_MUX_GATE_POSTDIV(_struct, _name, _parents, _reg, \ > + _mshift, _mwidth, \ > + _pshift, _pwidth, \ > + _muxshift, _muxwidth, \ > + _gate, _postdiv, _flags) \ > + struct ccu_mp _struct = { \ > + .enable = _gate, \ > + .m = _SUNXI_CCU_DIV(_mshift, _mwidth), \ > + .p = _SUNXI_CCU_DIV(_pshift, _pwidth), \ > + .mux = _SUNXI_CCU_MUX(_muxshift, _muxwidth), \ > + .fixed_post_div = _postdiv, \ > + .common = { \ > + .reg = _reg, \ > + .features = CCU_FEATURE_FIXED_POSTDIV, \ > + .hw.init = CLK_HW_INIT_PARENTS(_name, \ > + _parents, \ > + &ccu_mp_ops, \ > + _flags), \ > + } \ > + } > + This looks suspiciously like a copy of the macro below. What about you define the one below as a special case of this new one above? Should be even more straightforward with defaulting postdiv to 1 and loosing the feature flags. Cheers, Andre. > #define SUNXI_CCU_MP_WITH_MUX_GATE(_struct, _name, _parents, _reg, \ > _mshift, _mwidth, \ > _pshift, _pwidth, \ >