diff mbox series

[v4,3/5] clk: renesas: rcar-gen3: Support Z and Z2 clocks with high frequency parents

Message ID 20190207133550.13967-4-horms+renesas@verge.net.au (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show
Series clk: renesas: r8a77990, r8a774c0: Add Z2 clock | expand

Commit Message

Simon Horman Feb. 7, 2019, 1:35 p.m. UTC
Support Z and Z2 clocks with parent frequencies greater than UINT32_MAX Hz
(~4.29GHz).

The DIV_ROUND_CLOSEST_ULL() macro accepts a 64bit dividend and 32bit
divisor. This leads to truncation of the dividend, which is the Z or Z2
parent clock frequency in HZ, on platforms where frequency of that clock is
greater than UINT32_MAX Hz.

To resolve this problem the DIV64_U64_ROUND_CLOSEST() macro, which an
unsigned 32bit dividend and divisor, is introduced.

An earlier version of this patch made use of the existing
DIV_ROUND_CLOSEST() macro, which accepts the prevailing type of the
dividend and divisor. However, this does not compile on 32bit systems, such
as i386 and mips, when called with the types used at this callsite, an
unsigned long long dividend and unsigned long divisor.

This work is in preparation for supporting the Z2 cloco on the R-Car Gen3
E3 (r8a77990) SoC which has a 4.8GHz parent clock.

Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v4: Add and use DIV64_U64_ROUND_CLOSEST

v2: New patch
---
 drivers/clk/renesas/rcar-gen3-cpg.c |  4 ++--
 include/linux/math64.h              | 13 +++++++++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

Comments

Geert Uytterhoeven Feb. 7, 2019, 3:05 p.m. UTC | #1
Hi Simon,

On Thu, Feb 7, 2019 at 2:36 PM Simon Horman <horms+renesas@verge.net.au> wrote:
> Support Z and Z2 clocks with parent frequencies greater than UINT32_MAX Hz
> (~4.29GHz).
>
> The DIV_ROUND_CLOSEST_ULL() macro accepts a 64bit dividend and 32bit
> divisor. This leads to truncation of the dividend, which is the Z or Z2

truncation of the divisor.

> parent clock frequency in HZ, on platforms where frequency of that clock is
> greater than UINT32_MAX Hz.
>
> To resolve this problem the DIV64_U64_ROUND_CLOSEST() macro, which an

which takes an

> unsigned 32bit dividend and divisor, is introduced.

64-bit

> An earlier version of this patch made use of the existing
> DIV_ROUND_CLOSEST() macro, which accepts the prevailing type of the
> dividend and divisor. However, this does not compile on 32bit systems, such
> as i386 and mips, when called with the types used at this callsite, an
> unsigned long long dividend and unsigned long divisor.

Thanks for fixing this!

> This work is in preparation for supporting the Z2 cloco on the R-Car Gen3
> E3 (r8a77990) SoC which has a 4.8GHz parent clock.
>
> Signed-off-by: Simon Horman <horms+renesas@verge.net.au>

With the above fixed:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

> ---
> v4: Add and use DIV64_U64_ROUND_CLOSEST
>
> v2: New patch
> ---
>  drivers/clk/renesas/rcar-gen3-cpg.c |  4 ++--
>  include/linux/math64.h              | 13 +++++++++++++

While I have no issue taking this change through the clk-renesas tree if
Andrew provides his Acked-by, I think the introduction of
DIV64_U64_ROUND_CLOSEST() should be a separate patch.

However, given commit 68600f623d69da42 ("mm: don't miss the last page
because of round-off error") added both DIV64_U64_ROUND_UP() and its
user under mm/, there's precedence for not splitting it off...

Gr{oetje,eeting}s,

                        Geert
Simon Horman Feb. 7, 2019, 3:22 p.m. UTC | #2
On Thu, Feb 07, 2019 at 04:05:58PM +0100, Geert Uytterhoeven wrote:
> Hi Simon,
> 
> On Thu, Feb 7, 2019 at 2:36 PM Simon Horman <horms+renesas@verge.net.au> wrote:
> > Support Z and Z2 clocks with parent frequencies greater than UINT32_MAX Hz
> > (~4.29GHz).
> >
> > The DIV_ROUND_CLOSEST_ULL() macro accepts a 64bit dividend and 32bit
> > divisor. This leads to truncation of the dividend, which is the Z or Z2
> 
> truncation of the divisor.
> 
> > parent clock frequency in HZ, on platforms where frequency of that clock is
> > greater than UINT32_MAX Hz.
> >
> > To resolve this problem the DIV64_U64_ROUND_CLOSEST() macro, which an
> 
> which takes an
> 
> > unsigned 32bit dividend and divisor, is introduced.
> 
> 64-bit
> 
> > An earlier version of this patch made use of the existing
> > DIV_ROUND_CLOSEST() macro, which accepts the prevailing type of the
> > dividend and divisor. However, this does not compile on 32bit systems, such
> > as i386 and mips, when called with the types used at this callsite, an
> > unsigned long long dividend and unsigned long divisor.
> 
> Thanks for fixing this!
> 
> > This work is in preparation for supporting the Z2 cloco on the R-Car Gen3
> > E3 (r8a77990) SoC which has a 4.8GHz parent clock.
> >
> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
> 
> With the above fixed:
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> > ---
> > v4: Add and use DIV64_U64_ROUND_CLOSEST
> >
> > v2: New patch
> > ---
> >  drivers/clk/renesas/rcar-gen3-cpg.c |  4 ++--
> >  include/linux/math64.h              | 13 +++++++++++++
> 
> While I have no issue taking this change through the clk-renesas tree if
> Andrew provides his Acked-by, I think the introduction of
> DIV64_U64_ROUND_CLOSEST() should be a separate patch.
> 
> However, given commit 68600f623d69da42 ("mm: don't miss the last page
> because of round-off error") added both DIV64_U64_ROUND_UP() and its
> user under mm/, there's precedence for not splitting it off...

I also assumed a separate patch would be better,
but I found little evidence of that practice being
recently used in math64.h.

In any case, as I need to respin I'll break it out into
a separate patch.
diff mbox series

Patch

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index 6b146c2cf6a3..4513bb705b03 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -120,8 +120,8 @@  static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned int i;
 	u32 val, kick;
 
-	mult = DIV_ROUND_CLOSEST_ULL(rate * 32ULL * zclk->fixed_div,
-				     parent_rate);
+	mult = DIV64_U64_ROUND_CLOSEST(rate * 32ULL * zclk->fixed_div,
+				       parent_rate);
 	mult = clamp(mult, 1U, 32U);
 
 	if (readl(zclk->kick_reg) & CPG_FRQCRB_KICK)
diff --git a/include/linux/math64.h b/include/linux/math64.h
index bb2c84afb80c..65bef21cdddb 100644
--- a/include/linux/math64.h
+++ b/include/linux/math64.h
@@ -284,4 +284,17 @@  static inline u64 mul_u64_u32_div(u64 a, u32 mul, u32 divisor)
 #define DIV64_U64_ROUND_UP(ll, d)	\
 	({ u64 _tmp = (d); div64_u64((ll) + _tmp - 1, _tmp); })
 
+/**
+ * DIV64_U64_ROUND_CLOSEST - unsigned 64bit divide with 64bit divisor rounded to nearest integer
+ * @dividend: unsigned 64bit dividend
+ * @divisor: unsigned 64bit divisor
+ *
+ * Divide unsigned 64bit dividend by unsigned 64bit divisor
+ * and round to closest integer.
+ *
+ * Return: dividend / divisor rounded to nearest integer
+ */
+#define DIV64_U64_ROUND_CLOSEST(dividend, divisor)	\
+	({ u64 _tmp = (divisor); div64_u64((dividend) + _tmp / 2, _tmp); })
+
 #endif /* _LINUX_MATH64_H */