Hi Khiem, On Tue, May 10, 2016 at 6:42 AM, Khiem Nguyen <khiem.nguyen.xt@xxxxxxxxxxxxxxx> wrote: > Base on Dien Pham work. > > Signed-off-by: Khiem Nguyen <khiem.nguyen.xt@xxxxxxxxxxxxxxx> Thanks for your patch! > --- > drivers/clk/renesas/rcar-gen3-cpg.c | 143 > ++++++++++++++++++++++++++++++++++++ > drivers/clk/renesas/rcar-gen3-cpg.h | 1 + > 2 files changed, 144 insertions(+) > > diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c > b/drivers/clk/renesas/rcar-gen3-cpg.c > index bb4f2f9..45209ac 100644 > --- a/drivers/clk/renesas/rcar-gen3-cpg.c > +++ b/drivers/clk/renesas/rcar-gen3-cpg.c > @@ -28,6 +28,146 @@ > #define CPG_PLL2CR 0x002c > #define CPG_PLL4CR 0x01f4 > > +/** Modify for Z-clock 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@xxxxxxxxxxxxxx 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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html