All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] clk: renesas: rcar-gen3: Add Z clock divider support
@ 2017-04-19 17:46 Yoshihiro Kaneko
  2017-04-21 13:59 ` Geert Uytterhoeven
  0 siblings, 1 reply; 10+ messages in thread
From: Yoshihiro Kaneko @ 2017-04-19 17:46 UTC (permalink / raw)
  To: linux-clk
  Cc: Michael Turquette, Stephen Boyd, Geert Uytterhoeven,
	Simon Horman, Magnus Damm, linux-renesas-soc

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: Yoshihiro Kaneko <ykaneko0929@gmail.com>
---
This patch is based on the clk-next branch of linux-clk tree.

 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 3dee900..8419f27 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -29,6 +29,145 @@
 #define CPG_PLL2CR		0x002c
 #define CPG_PLL4CR		0x01f4
 
+/** Modify for Z-clock
+ * -----------------------------------------------------------------------------
+ * 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;
+};
+
+#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);
+	/* Round to closest value at 100MHz unit */
+	rate = 100000000*DIV_ROUND_CLOSEST(rate, 100000000);
+	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);
+	mult = clamp(mult, 1U, 32U);
+
+	return *parent_rate / 32 * mult;
+}
+
+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);
+	mult = clamp(mult, 1U, 32U);
+
+	if (clk_readl(zclk->kick_reg) & CPG_FRQCRB_KICK)
+		return -EBUSY;
+
+	val = clk_readl(zclk->reg);
+	val &= ~CPG_FRQCRC_ZFC_MASK;
+	val |= (32 - mult) << CPG_FRQCRC_ZFC_SHIFT;
+	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;
+}
+/** End of modifying for Z-clock */
 
 /*
  * SDn Clock
@@ -360,6 +499,10 @@ struct clk * __init rcar_gen3_cpg_clk_register(struct device *dev,
 			parent = clks[cpg_clk_extalr];
 		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 073be54..b1e883d 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.h
+++ b/drivers/clk/renesas/rcar-gen3-cpg.h
@@ -20,6 +20,7 @@ enum rcar_gen3_clk_types {
 	CLK_TYPE_GEN3_PLL4,
 	CLK_TYPE_GEN3_SD,
 	CLK_TYPE_GEN3_R,
+	CLK_TYPE_GEN3_Z,
 };
 
 #define DEF_GEN3_SD(_name, _id, _parent, _offset)	\
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] clk: renesas: rcar-gen3: Add Z clock divider support
  2017-04-19 17:46 [PATCH] clk: renesas: rcar-gen3: Add Z clock divider support Yoshihiro Kaneko
@ 2017-04-21 13:59 ` Geert Uytterhoeven
  2017-04-22  1:27   ` Stephen Boyd
  2017-04-24  8:03   ` Peter De Schrijver
  0 siblings, 2 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2017-04-21 13:59 UTC (permalink / raw)
  To: Yoshihiro Kaneko, Michael Turquette, Stephen Boyd
  Cc: linux-clk, Geert Uytterhoeven, Simon Horman, Magnus Damm, Linux-Renesas

Hi Kaneko-san, and Mike, Stephen (see below),

On Wed, Apr 19, 2017 at 7:46 PM, Yoshihiro Kaneko <ykaneko0929@gmail.com> 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: Yoshihiro Kaneko <ykaneko0929@gmail.com>

I gave this patch a try on Salvator-X.  On both R-Car H3 and M3-W,
/sys/kernel/debug/clk/clk_summary reports:

       .pll0                              0            0  2999999880
       0 0
          z                               0            0  3000000000
       0 0

The Z clock runs at 1.5 GHz, so the numbers are off by a factor of two.
It seems the PLL post-divider of 1/2 is not taken into account.

> --- 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;
> +       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);
> +       /* Round to closest value at 100MHz unit */
> +       rate = 100000000*DIV_ROUND_CLOSEST(rate, 100000000);

Mike, Stephen: what's your opinion about such rounding tricks?

Thanks!

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] clk: renesas: rcar-gen3: Add Z clock divider support
  2017-04-21 13:59 ` Geert Uytterhoeven
@ 2017-04-22  1:27   ` Stephen Boyd
  2017-04-23 20:20     ` Geert Uytterhoeven
  2017-04-24  8:03   ` Peter De Schrijver
  1 sibling, 1 reply; 10+ messages in thread
From: Stephen Boyd @ 2017-04-22  1:27 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Yoshihiro Kaneko, Michael Turquette, linux-clk,
	Geert Uytterhoeven, Simon Horman, Magnus Damm, Linux-Renesas

On 04/21, Geert Uytterhoeven wrote:
> 
> > --- 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;
> > +       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);
> > +       /* Round to closest value at 100MHz unit */
> > +       rate = 100000000*DIV_ROUND_CLOSEST(rate, 100000000);
> 
> Mike, Stephen: what's your opinion about such rounding tricks?
> 

What's wrong with the true rate? I'd prefer we calculate the true
rate unless there's some reason. Typically that's what a comment
is for, not to explain what the code is doing which is usually
self evident from reading it.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] clk: renesas: rcar-gen3: Add Z clock divider support
  2017-04-22  1:27   ` Stephen Boyd
@ 2017-04-23 20:20     ` Geert Uytterhoeven
  0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2017-04-23 20:20 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Yoshihiro Kaneko, Michael Turquette, linux-clk,
	Geert Uytterhoeven, Simon Horman, Magnus Damm, Linux-Renesas

Hi Stephen,

On Sat, Apr 22, 2017 at 3:27 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 04/21, Geert Uytterhoeven wrote:
>> > --- 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;
>> > +       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);
>> > +       /* Round to closest value at 100MHz unit */
>> > +       rate = 100000000*DIV_ROUND_CLOSEST(rate, 100000000);
>>
>> Mike, Stephen: what's your opinion about such rounding tricks?
>
> What's wrong with the true rate? I'd prefer we calculate the true
> rate unless there's some reason. Typically that's what a comment
> is for, not to explain what the code is doing which is usually
> self evident from reading it.

The (intermediate) true rate may be a fractional number.
Since clock rate division and multiplication operate on integer numbers
(unsigned long), the calculated rate may deviate from the true rate due to
rounding or truncation.

The hack above is an attempt to correct that.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] clk: renesas: rcar-gen3: Add Z clock divider support
  2017-04-21 13:59 ` Geert Uytterhoeven
  2017-04-22  1:27   ` Stephen Boyd
@ 2017-04-24  8:03   ` Peter De Schrijver
  2017-04-24  8:09     ` Geert Uytterhoeven
  1 sibling, 1 reply; 10+ messages in thread
From: Peter De Schrijver @ 2017-04-24  8:03 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Yoshihiro Kaneko, Michael Turquette, Stephen Boyd, linux-clk,
	Geert Uytterhoeven, Simon Horman, Magnus Damm, Linux-Renesas

On Fri, Apr 21, 2017 at 03:59:10PM +0200, Geert Uytterhoeven wrote:
> Hi Kaneko-san, and Mike, Stephen (see below),
> 
> On Wed, Apr 19, 2017 at 7:46 PM, Yoshihiro Kaneko <ykaneko0929@gmail.com> 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: Yoshihiro Kaneko <ykaneko0929@gmail.com>
> 
> I gave this patch a try on Salvator-X.  On both R-Car H3 and M3-W,
> /sys/kernel/debug/clk/clk_summary reports:
> 
>        .pll0                              0            0  2999999880
>        0 0
>           z                               0            0  3000000000
>        0 0
> 
> The Z clock runs at 1.5 GHz, so the numbers are off by a factor of two.
> It seems the PLL post-divider of 1/2 is not taken into account.
> 
> > --- 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;
> > +       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);
> > +       /* Round to closest value at 100MHz unit */
> > +       rate = 100000000*DIV_ROUND_CLOSEST(rate, 100000000);
> 
> Mike, Stephen: what's your opinion about such rounding tricks?
> 

Is this an actual divider or a pulse skipper?

Peter.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] clk: renesas: rcar-gen3: Add Z clock divider support
  2017-04-24  8:03   ` Peter De Schrijver
@ 2017-04-24  8:09     ` Geert Uytterhoeven
  2017-04-25 10:18       ` Peter De Schrijver
  0 siblings, 1 reply; 10+ messages in thread
From: Geert Uytterhoeven @ 2017-04-24  8:09 UTC (permalink / raw)
  To: Peter De Schrijver
  Cc: Yoshihiro Kaneko, Michael Turquette, Stephen Boyd, linux-clk,
	Geert Uytterhoeven, Simon Horman, Magnus Damm, Linux-Renesas

Hi Peter,

On Mon, Apr 24, 2017 at 10:03 AM, Peter De Schrijver
<pdeschrijver@nvidia.com> wrote:
>> > --- 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;
>> > +       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);
>> > +       /* Round to closest value at 100MHz unit */
>> > +       rate = 100000000*DIV_ROUND_CLOSEST(rate, 100000000);
>>
>> Mike, Stephen: what's your opinion about such rounding tricks?
>
> Is this an actual divider or a pulse skipper?

Forgive my ignorance, but what is the difference?

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] clk: renesas: rcar-gen3: Add Z clock divider support
  2017-04-24  8:09     ` Geert Uytterhoeven
@ 2017-04-25 10:18       ` Peter De Schrijver
  2017-04-25 10:28         ` Geert Uytterhoeven
  0 siblings, 1 reply; 10+ messages in thread
From: Peter De Schrijver @ 2017-04-25 10:18 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Yoshihiro Kaneko, Michael Turquette, Stephen Boyd, linux-clk,
	Geert Uytterhoeven, Simon Horman, Magnus Damm, Linux-Renesas

Hi Geert,

On Mon, Apr 24, 2017 at 10:09:10AM +0200, Geert Uytterhoeven wrote:
> Hi Peter,
> 
> On Mon, Apr 24, 2017 at 10:03 AM, Peter De Schrijver
> <pdeschrijver@nvidia.com> wrote:
> >> > --- 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;
> >> > +       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);
> >> > +       /* Round to closest value at 100MHz unit */
> >> > +       rate = 100000000*DIV_ROUND_CLOSEST(rate, 100000000);
> >>
> >> Mike, Stephen: what's your opinion about such rounding tricks?
> >
> > Is this an actual divider or a pulse skipper?
> 
> Forgive my ignorance, but what is the difference?

A pulse skipper, as the name says, skips pulses. Eg, if you configure the
skipper to 3/4, it will skip 1 input clock pulse out of 4. The pulse width
will be the same as the parent clock pulse width though. A divider stretches
the clock pulse. So the output of a divider configured to 1/2 will have a
pulse width which is twice as large as the pulse width of the input clock.

Cheers,

Peter.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] clk: renesas: rcar-gen3: Add Z clock divider support
  2017-04-25 10:18       ` Peter De Schrijver
@ 2017-04-25 10:28         ` Geert Uytterhoeven
  2017-04-25 11:02           ` Peter De Schrijver
  0 siblings, 1 reply; 10+ messages in thread
From: Geert Uytterhoeven @ 2017-04-25 10:28 UTC (permalink / raw)
  To: Peter De Schrijver
  Cc: Yoshihiro Kaneko, Michael Turquette, Stephen Boyd, linux-clk,
	Geert Uytterhoeven, Simon Horman, Magnus Damm, Linux-Renesas

Hi Peter,

On Tue, Apr 25, 2017 at 12:18 PM, Peter De Schrijver
<pdeschrijver@nvidia.com> wrote:
> On Mon, Apr 24, 2017 at 10:09:10AM +0200, Geert Uytterhoeven wrote:
>> On Mon, Apr 24, 2017 at 10:03 AM, Peter De Schrijver
>> <pdeschrijver@nvidia.com> wrote:
>> >> > --- 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;
>> >> > +       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);
>> >> > +       /* Round to closest value at 100MHz unit */
>> >> > +       rate = 100000000*DIV_ROUND_CLOSEST(rate, 100000000);
>> >>
>> >> Mike, Stephen: what's your opinion about such rounding tricks?
>> >
>> > Is this an actual divider or a pulse skipper?
>>
>> Forgive my ignorance, but what is the difference?
>
> A pulse skipper, as the name says, skips pulses. Eg, if you configure the
> skipper to 3/4, it will skip 1 input clock pulse out of 4. The pulse width
> will be the same as the parent clock pulse width though. A divider stretches
> the clock pulse. So the output of a divider configured to 1/2 will have a
> pulse width which is twice as large as the pulse width of the input clock.

OK, thanks! So a pulse skipper changes the duty cycle of the signal.

I would expect this to be a real divider, as that would allow the Cortex A53
core to do work on both edges of the clock signal.
It's a bit difficult to measure, though ;-)

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] clk: renesas: rcar-gen3: Add Z clock divider support
  2017-04-25 10:28         ` Geert Uytterhoeven
@ 2017-04-25 11:02           ` Peter De Schrijver
  2017-04-25 11:39             ` Geert Uytterhoeven
  0 siblings, 1 reply; 10+ messages in thread
From: Peter De Schrijver @ 2017-04-25 11:02 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Yoshihiro Kaneko, Michael Turquette, Stephen Boyd, linux-clk,
	Geert Uytterhoeven, Simon Horman, Magnus Damm, Linux-Renesas

Hi Geert,

On Tue, Apr 25, 2017 at 12:28:29PM +0200, Geert Uytterhoeven wrote:
> Hi Peter,
> 
> On Tue, Apr 25, 2017 at 12:18 PM, Peter De Schrijver
> <pdeschrijver@nvidia.com> wrote:
> > On Mon, Apr 24, 2017 at 10:09:10AM +0200, Geert Uytterhoeven wrote:
> >> On Mon, Apr 24, 2017 at 10:03 AM, Peter De Schrijver
> >> <pdeschrijver@nvidia.com> wrote:
> >> >> > --- 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;
> >> >> > +       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);
> >> >> > +       /* Round to closest value at 100MHz unit */
> >> >> > +       rate = 100000000*DIV_ROUND_CLOSEST(rate, 100000000);
> >> >>
> >> >> Mike, Stephen: what's your opinion about such rounding tricks?
> >> >
> >> > Is this an actual divider or a pulse skipper?
> >>
> >> Forgive my ignorance, but what is the difference?
> >
> > A pulse skipper, as the name says, skips pulses. Eg, if you configure the
> > skipper to 3/4, it will skip 1 input clock pulse out of 4. The pulse width
> > will be the same as the parent clock pulse width though. A divider stretches
> > the clock pulse. So the output of a divider configured to 1/2 will have a
> > pulse width which is twice as large as the pulse width of the input clock.
> 
> OK, thanks! So a pulse skipper changes the duty cycle of the signal.
> 

Yes, it does.

> I would expect this to be a real divider, as that would allow the Cortex A53
> core to do work on both edges of the clock signal.

Maybe. Although I know we've run A53s from skippers. One difference is that a
real divider allows you to lower the voltage, while a skipper does not,
because it only changes the duty cycle as you mentioned.

> It's a bit difficult to measure, though ;-)
> 

Maybe there's a way to expose the clock signal on an external pin? Or you can
use a full chip emulator :)

Cheers,

Peter.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] clk: renesas: rcar-gen3: Add Z clock divider support
  2017-04-25 11:02           ` Peter De Schrijver
@ 2017-04-25 11:39             ` Geert Uytterhoeven
  0 siblings, 0 replies; 10+ messages in thread
From: Geert Uytterhoeven @ 2017-04-25 11:39 UTC (permalink / raw)
  To: Peter De Schrijver
  Cc: Yoshihiro Kaneko, Michael Turquette, Stephen Boyd, linux-clk,
	Geert Uytterhoeven, Simon Horman, Magnus Damm, Linux-Renesas

Hi Peter,

On Tue, Apr 25, 2017 at 1:02 PM, Peter De Schrijver
<pdeschrijver@nvidia.com> wrote:
> On Tue, Apr 25, 2017 at 12:28:29PM +0200, Geert Uytterhoeven wrote:
>> On Tue, Apr 25, 2017 at 12:18 PM, Peter De Schrijver
>> <pdeschrijver@nvidia.com> wrote:
>> > On Mon, Apr 24, 2017 at 10:09:10AM +0200, Geert Uytterhoeven wrote:
>> >> On Mon, Apr 24, 2017 at 10:03 AM, Peter De Schrijver
>> >> <pdeschrijver@nvidia.com> wrote:
>> >> >> > --- 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;
>> >> >> > +       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);
>> >> >> > +       /* Round to closest value at 100MHz unit */
>> >> >> > +       rate = 100000000*DIV_ROUND_CLOSEST(rate, 100000000);
>> >> >>
>> >> >> Mike, Stephen: what's your opinion about such rounding tricks?
>> >> >
>> >> > Is this an actual divider or a pulse skipper?
>> >>
>> >> Forgive my ignorance, but what is the difference?
>> >
>> > A pulse skipper, as the name says, skips pulses. Eg, if you configure the
>> > skipper to 3/4, it will skip 1 input clock pulse out of 4. The pulse width
>> > will be the same as the parent clock pulse width though. A divider stretches
>> > the clock pulse. So the output of a divider configured to 1/2 will have a
>> > pulse width which is twice as large as the pulse width of the input clock.
>>
>> OK, thanks! So a pulse skipper changes the duty cycle of the signal.
>
> Yes, it does.
>
>> I would expect this to be a real divider, as that would allow the Cortex A53
>> core to do work on both edges of the clock signal.
>
> Maybe. Although I know we've run A53s from skippers. One difference is that a
> real divider allows you to lower the voltage, while a skipper does not,
> because it only changes the duty cycle as you mentioned.

Then it must be a real divider, as it supports DVFS.

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-04-25 11:39 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-19 17:46 [PATCH] clk: renesas: rcar-gen3: Add Z clock divider support Yoshihiro Kaneko
2017-04-21 13:59 ` Geert Uytterhoeven
2017-04-22  1:27   ` Stephen Boyd
2017-04-23 20:20     ` Geert Uytterhoeven
2017-04-24  8:03   ` Peter De Schrijver
2017-04-24  8:09     ` Geert Uytterhoeven
2017-04-25 10:18       ` Peter De Schrijver
2017-04-25 10:28         ` Geert Uytterhoeven
2017-04-25 11:02           ` Peter De Schrijver
2017-04-25 11:39             ` Geert Uytterhoeven

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.