[linux-sunxi] [PATCH 1/2] clk: sunxi-ng: Support fixed post-dividers on MP style clocks

On Tue, Dec 5, 2017 at 7:18 AM, Andr? Przywara [off-list ref] wrote:

Hi Chen-Yu,

On 04/12/17 05:19, Chen-Yu Tsai wrote:

quoted

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 <redacted>
---
 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?

I'll refer to Maxime about this. The feature flag was there from day
one. We only started to implement support for it later on. I'm not
sure if there was a reason to add them as feature flags, instead of
a field that defaults to something (0 even).

Otherwise it's a reasonable change. And we probably don't have to
do a wholesale change for the other clocks in one go. Incidentally
I have a A83T audio series that also adds post-dividers for another
clock type. I'll wait for a conclusion on this end before posting
it.

quoted

+
      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?

Correct. But you can't unset the feature flag. So a copy is needed.

Should be even more straightforward with defaulting postdiv to 1 and
loosing the feature flags.

See above about the feature flag.

ChenYu

Cheers,
Andre.

quoted

 #define SUNXI_CCU_MP_WITH_MUX_GATE(_struct, _name, _parents, _reg,   \
                                 _mshift, _mwidth,                    \
                                 _pshift, _pwidth,                    \