Message ID | 1507209837-16938-2-git-send-email-horms+renesas@verge.net.au (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Geert Uytterhoeven |
Headers | show |
Hi Simon, On Thu, Oct 5, 2017 at 3:23 PM, Simon Horman <horms+renesas@verge.net.au> wrote: > From: Takeshi Kihara <takeshi.kihara.df@renesas.com> > > This patch adds Z clock divider support for R-Car Gen3 SoC. > > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au> > --- > > v2 [Simon Horman] > * Use DIV_ROUND_CLOSEST_ULL instead of open-coding the same behaviour > using div_u64() > * Do not round rate to 100MHz in cpg_z_clk_recalc_rate > * Remove calculation for PLL post-divider, this is bogus. > Instead do not round to closest in cpg_z_clk_round_rate() > * Drop check for !prate in cpg_z_clk_round_rate Thanks for the updates! > --- a/drivers/clk/renesas/rcar-gen3-cpg.c > +++ b/drivers/clk/renesas/rcar-gen3-cpg.c > +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; > + > + mult = 32 - FIELD_GET(CPG_FRQCRC_ZFC_MASK, clk_readl(zclk->reg)); > + return DIV_ROUND_CLOSEST_ULL(parent_rate * mult, 32); While parent_rate is unsigned long and thus 64-bit on arm64, this will work fine on R-Car Gen3. However, if someone ever tries to reuse this on a 32-bit platform, the multiplication may overflow. Hence I recommend to make this a 64-bit multiplication explicitly, i.e. return DIV_ROUND_CLOSEST_ULL((u64)parent_rate * mult, 32); > +} > + > +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; > + > + mult = div_u64((u64)rate * 32, prate); You can avoid the cast by making the constant 64-bit: mult = div_u64(rate * 32ULL, prate); > + mult = clamp(mult, 1U, 32U); > + > + return *parent_rate * mult / 32; Again, *parent_rate is 64-bit on arm64, but not on 32-bit platforms. > + /* > + * Note: There is no HW information about the worst case latency. > + * > + * Using experimental measurements, it seems that no more than > + * ~10 iterations are needed, independently of the CPU rate. > + * Since this value might be dependent of external xtal rate, pll1 > + * rate or even the other emulation clocks rate, use 1000 as a another emulated clock rate? (Yeah, copied from Gen2 ;-) Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> 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
On Mon, Oct 09, 2017 at 10:02:34AM +0200, Geert Uytterhoeven wrote: > Hi Simon, > > On Thu, Oct 5, 2017 at 3:23 PM, Simon Horman <horms+renesas@verge.net.au> wrote: > > From: Takeshi Kihara <takeshi.kihara.df@renesas.com> > > > > This patch adds Z clock divider support for R-Car Gen3 SoC. > > > > Signed-off-by: Takeshi Kihara <takeshi.kihara.df@renesas.com> > > Signed-off-by: Simon Horman <horms+renesas@verge.net.au> > > --- > > > > v2 [Simon Horman] > > * Use DIV_ROUND_CLOSEST_ULL instead of open-coding the same behaviour > > using div_u64() > > * Do not round rate to 100MHz in cpg_z_clk_recalc_rate > > * Remove calculation for PLL post-divider, this is bogus. > > Instead do not round to closest in cpg_z_clk_round_rate() > > * Drop check for !prate in cpg_z_clk_round_rate > > Thanks for the updates! > > > --- a/drivers/clk/renesas/rcar-gen3-cpg.c > > +++ b/drivers/clk/renesas/rcar-gen3-cpg.c > > > +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; > > + > > + mult = 32 - FIELD_GET(CPG_FRQCRC_ZFC_MASK, clk_readl(zclk->reg)); > > + return DIV_ROUND_CLOSEST_ULL(parent_rate * mult, 32); > > While parent_rate is unsigned long and thus 64-bit on arm64, this will work > fine on R-Car Gen3. However, if someone ever tries to reuse this on a > 32-bit platform, the multiplication may overflow. > Hence I recommend to make this a 64-bit multiplication explicitly, i.e. > > return DIV_ROUND_CLOSEST_ULL((u64)parent_rate * mult, 32); Thanks, will do. > > +} > > + > > +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; > > + > > + mult = div_u64((u64)rate * 32, prate); > > You can avoid the cast by making the constant 64-bit: > > mult = div_u64(rate * 32ULL, prate); Thanks, that looks a bit nicer. > > + mult = clamp(mult, 1U, 32U); > > + > > + return *parent_rate * mult / 32; > > Again, *parent_rate is 64-bit on arm64, but not on 32-bit platforms. > > > + /* > > + * Note: There is no HW information about the worst case latency. > > + * > > + * Using experimental measurements, it seems that no more than > > + * ~10 iterations are needed, independently of the CPU rate. > > + * Since this value might be dependent of external xtal rate, pll1 > > + * rate or even the other emulation clocks rate, use 1000 as a > > another emulated clock rate? > (Yeah, copied from Gen2 ;-) It seem so. > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c index 951105816547..d98272a6075f 100644 --- a/drivers/clk/renesas/rcar-gen3-cpg.c +++ b/drivers/clk/renesas/rcar-gen3-cpg.c @@ -13,6 +13,7 @@ */ #include <linux/bug.h> +#include <linux/bitfield.h> #include <linux/clk.h> #include <linux/clk-provider.h> #include <linux/device.h> @@ -29,6 +30,129 @@ #define CPG_PLL2CR 0x002c #define CPG_PLL4CR 0x01f4 +/* + * 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 ) / 2 + * 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 GENMASK(12, 8) + +struct cpg_z_clk { + struct clk_hw hw; + void __iomem *reg; + void __iomem *kick_reg; +}; + +#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; + + mult = 32 - FIELD_GET(CPG_FRQCRC_ZFC_MASK, clk_readl(zclk->reg)); + return DIV_ROUND_CLOSEST_ULL(parent_rate * mult, 32); +} + +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; + + mult = div_u64((u64)rate * 32, prate); + mult = clamp(mult, 1U, 32U); + + return *parent_rate * mult / 32; +} + +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; + unsigned int i; + u32 val, kick; + + mult = DIV_ROUND_CLOSEST_ULL(rate * 32ULL, parent_rate); + mult = clamp(mult, 1U, 32U); + + if (clk_readl(zclk->kick_reg) & CPG_FRQCRB_KICK) + return -EBUSY; + + val = clk_readl(zclk->reg) & ~CPG_FRQCRC_ZFC_MASK; + val |= FIELD_PREP(CPG_FRQCRC_ZFC_MASK, 32 - mult); + clk_writel(val, zclk->reg); + + /* + * Set KICK bit in FRQCRB to update hardware setting and wait for + * clock change completion. + */ + kick = clk_readl(zclk->kick_reg); + kick |= CPG_FRQCRB_KICK; + clk_writel(kick, zclk->kick_reg); + + /* + * Note: There is no HW information about the worst case latency. + * + * Using experimental measurements, it seems that no more than + * ~10 iterations are needed, independently of the CPU rate. + * Since this value might be dependent of external xtal rate, pll1 + * rate or even the other emulation clocks rate, use 1000 as a + * "super" safe value. + */ + for (i = 1000; i; i--) { + if (!(clk_readl(zclk->kick_reg) & CPG_FRQCRB_KICK)) + return 0; + + cpu_relax(); + } + + return -ETIMEDOUT; +} + +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 char *name, + const char *parent_name, + void __iomem *reg) +{ + 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 = name; + init.ops = &cpg_z_clk_ops; + init.flags = 0; + init.parent_names = &parent_name; + init.num_parents = 1; + + zclk->reg = reg + CPG_FRQCRC; + zclk->kick_reg = reg + CPG_FRQCRB; + zclk->hw.init = &init; + + clk = clk_register(NULL, &zclk->hw); + if (IS_ERR(clk)) + kfree(zclk); + + return clk; +} /* * SDn Clock @@ -373,6 +497,10 @@ struct clk * __init rcar_gen3_cpg_clk_register(struct device *dev, mult = 1; break; + case CLK_TYPE_GEN3_Z: + return cpg_z_clk_register(core->name, __clk_get_name(parent), + base); + default: return ERR_PTR(-EINVAL); } diff --git a/drivers/clk/renesas/rcar-gen3-cpg.h b/drivers/clk/renesas/rcar-gen3-cpg.h index d756ef8b78eb..89da28f6f71b 100644 --- a/drivers/clk/renesas/rcar-gen3-cpg.h +++ b/drivers/clk/renesas/rcar-gen3-cpg.h @@ -21,6 +21,7 @@ enum rcar_gen3_clk_types { CLK_TYPE_GEN3_SD, CLK_TYPE_GEN3_R, CLK_TYPE_GEN3_PE, + CLK_TYPE_GEN3_Z, }; #define DEF_GEN3_SD(_name, _id, _parent, _offset) \