Hi Khiem,

On Tue, May 10, 2016 at 6:42 AM, Khiem Nguyen
[off-list ref] wrote:

Base on Dien Pham work.

Signed-off-by: Khiem Nguyen <redacted>

Thanks for your patch!

Please drop this comment

+ *
-----------------------------------------------------------------------------
+ * Z Clock
+ *
+ * Traits of this clock:
+ * prepare - clk_prepare only ensures that parents are prepared
+ * enable - clk_enable only ensures that parents are enabled
+ * rate - rate is adjustable.  clk->rate = parent->rate * mult / 32
+ * parent - fixed parent.  No clk_set_parent support
+ */
+#define CPG_FRQCRB                     0x00000004
+#define CPG_FRQCRB_KICK                        BIT(31)
+#define CPG_FRQCRC                     0x000000e0
+#define CPG_FRQCRC_ZFC_MASK            (0x1f << 8)
+#define CPG_FRQCRC_ZFC_SHIFT           8
+
+
+struct cpg_z_clk {
+       struct clk_hw hw;
+       void __iomem *reg;
+       void __iomem *kick_reg;

I would just store the base address, and always use "zclk->base + CPG_FRQCRB"
or "zclk->base + CPG_FRQCRC".

+};
+
+#define to_z_clk(_hw)  container_of(_hw, struct cpg_z_clk, hw)
+
+static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
+                                          unsigned long parent_rate)
+{
+       struct cpg_z_clk *zclk = to_z_clk(hw);
+       unsigned int mult;
+       unsigned int val;
+       unsigned long rate;
+
+       val = (clk_readl(zclk->reg) & CPG_FRQCRC_ZFC_MASK)
+           >> CPG_FRQCRC_ZFC_SHIFT;
+       mult = 32 - val;
+
+       rate = div_u64((u64)parent_rate * mult + 16, 32);

The above also does rounding.

+       /* Round to closest value at 100MHz unit */
+       rate = 100000000*DIV_ROUND_CLOSEST(rate, 100000000);

Why rounding to the closest 100MHz unit?

+       return rate;
+}
+
+static long cpg_z_clk_round_rate(struct clk_hw *hw, unsigned long rate,
+                                unsigned long *parent_rate)
+{
+       unsigned long prate  = *parent_rate;
+       unsigned int mult;
+
+       if (!prate)
+               prate = 1;
+
+       mult = div_u64((u64)rate * 32 + prate/2, prate);

DIV_ROUND_CLOSEST_ULL()

+       mult = clamp(mult, 1U, 32U);
+
+       return *parent_rate / 32 * mult;

Doing the division first reduces accuracy.
Please do the multiplication first (in 64-bit arithmetic).

+}
+
+static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
+                             unsigned long parent_rate)
+{
+       struct cpg_z_clk *zclk = to_z_clk(hw);
+       unsigned int mult;
+       u32 val, kick;
+       unsigned int i;
+
+       mult = div_u64((u64)rate * 32 + parent_rate/2, parent_rate);

DIV_ROUND_CLOSEST_ULL()

+}
+
+static const struct clk_ops cpg_z_clk_ops = {
+       .recalc_rate = cpg_z_clk_recalc_rate,
+       .round_rate = cpg_z_clk_round_rate,
+       .set_rate = cpg_z_clk_set_rate,
+};
+
+static struct clk * __init cpg_z_clk_register(const struct cpg_core_clk
*core,
+                                             void __iomem *base,
+                                             const char *parent_name)
+{
+       struct clk_init_data init;
+       struct cpg_z_clk *zclk;
+       struct clk *clk;
+
+       zclk = kzalloc(sizeof(*zclk), GFP_KERNEL);
+       if (!zclk)
+               return ERR_PTR(-ENOMEM);
+
+       init.name = core->name;
+       init.ops = &cpg_z_clk_ops;
+       init.flags = CLK_SET_RATE_GATE;

#define CLK_SET_RATE_GATE       BIT(0) /* must be gated across rate change */

Given you don't implement clk_ops.disable() and clk_ops.unprepare(),
CLK_SET_RATE_GATE doesn't sound like the right flag to use.

+       init.parent_names = &parent_name;
+       init.num_parents = 1;
+
+       zclk->reg = base + CPG_FRQCRC;
+       zclk->kick_reg = base + CPG_FRQCRB;
+       zclk->hw.init = &init;
+
+       clk = clk_register(NULL, &zclk->hw);
+       if (IS_ERR(clk))
+               kfree(zclk);
+
+       return clk;
+}
+
+/** End of modifying for Z-clock */

Please drop this comment

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds