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
