linux-clk.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate()
@ 2019-08-30 13:45 Geert Uytterhoeven
  2019-08-30 13:45 ` [PATCH v2 1/8] clk: renesas: rcar-gen2: Improve arithmetic divisions Geert Uytterhoeven
                   ` (8 more replies)
  0 siblings, 9 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2019-08-30 13:45 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-renesas-soc, Geert Uytterhoeven

	Hi Mike, Stephen,

As the .round_rate() callback returns a long clock rate, it cannot
return clock rates that do not fit in signed long, but do fit in
unsigned long.  The newer .determine_rate() callback does not suffer
from this limitation.  In addition, .determine_rate() provides the
ability to specify a rate range.

This patch series performs the customary preparatory cleanups, and
switches the Z (CPU) and SD clocks in the R-Car Gen2 and Gen3 clock
drivers from the .round_rate() to the .determine_rate() callback.
Note that the "div6" clock driver hasn't been converted yet, so div6
clocks still use .round_rate().

Changes compared to v1[1]:
  - Add preparatory arithmetic division improvements
  - Split off cpg_sd_clock_calc_div() absorption and SD clock best rate
    calculation,
  - Use div_u64() for division by unsigned long,

This has been tested on R-Car M2-W and various R-Car Gen3, and should
have no behavioral impact.

To be queued in clk-renesas-for-v5.5.

Thanks for your comments!

[1] [PATCH 0/5] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate()
    https://lore.kernel.org/linux-clk/20190617125238.13761-1-geert+renesas@glider.be/

Geert Uytterhoeven (8):
  clk: renesas: rcar-gen2: Improve arithmetic divisions
  clk: renesas: rcar-gen3: Improve arithmetic divisions
  clk: renesas: rcar-gen3: Avoid double table iteration in SD
    .set_rate()
  clk: renesas: rcar-gen3: Absorb cpg_sd_clock_calc_div()
  clk: renesas: rcar-gen3: Loop to find best rate in
    cpg_sd_clock_round_rate()
  clk: renesas: rcar-gen2: Switch Z clock to .determine_rate()
  clk: renesas: rcar-gen3: Switch Z clocks to .determine_rate()
  clk: renesas: rcar-gen3: Switch SD clocks to .determine_rate()

 drivers/clk/renesas/rcar-gen2-cpg.c | 25 ++++++-----
 drivers/clk/renesas/rcar-gen3-cpg.c | 64 ++++++++++++++++-------------
 2 files changed, 49 insertions(+), 40 deletions(-)

-- 
2.17.1

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] 18+ messages in thread

* [PATCH v2 1/8] clk: renesas: rcar-gen2: Improve arithmetic divisions
  2019-08-30 13:45 [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate() Geert Uytterhoeven
@ 2019-08-30 13:45 ` Geert Uytterhoeven
  2019-08-30 21:42   ` Niklas Söderlund
  2019-08-30 13:45 ` [PATCH v2 2/8] clk: renesas: rcar-gen3: " Geert Uytterhoeven
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2019-08-30 13:45 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-renesas-soc, Geert Uytterhoeven

  - Use div64_ul() instead of div_u64() if the divisor is unsigned long,
    to avoid truncation to 32-bit on 64-bit platforms,
  - Prefer ULL constant suffixes over casts to u64,
  - Prioritize multiplication over division, to increase accuracy.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.
---
 drivers/clk/renesas/rcar-gen2-cpg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen2-cpg.c b/drivers/clk/renesas/rcar-gen2-cpg.c
index f596a2dafcf4d8d1..c378505830f0bacc 100644
--- a/drivers/clk/renesas/rcar-gen2-cpg.c
+++ b/drivers/clk/renesas/rcar-gen2-cpg.c
@@ -72,10 +72,10 @@ static long cpg_z_clk_round_rate(struct clk_hw *hw, unsigned long rate,
 	if (!prate)
 		prate = 1;
 
-	mult = div_u64((u64)rate * 32, prate);
+	mult = div64_ul(rate * 32ULL, prate);
 	mult = clamp(mult, 1U, 32U);
 
-	return *parent_rate / 32 * mult;
+	return div_u64((u64)*parent_rate * mult, 32);
 }
 
 static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -86,7 +86,7 @@ static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 	u32 val, kick;
 	unsigned int i;
 
-	mult = div_u64((u64)rate * 32, parent_rate);
+	mult = div64_ul(rate * 32ULL, parent_rate);
 	mult = clamp(mult, 1U, 32U);
 
 	if (readl(zclk->kick_reg) & CPG_FRQCRB_KICK)
-- 
2.17.1


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

* [PATCH v2 2/8] clk: renesas: rcar-gen3: Improve arithmetic divisions
  2019-08-30 13:45 [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate() Geert Uytterhoeven
  2019-08-30 13:45 ` [PATCH v2 1/8] clk: renesas: rcar-gen2: Improve arithmetic divisions Geert Uytterhoeven
@ 2019-08-30 13:45 ` Geert Uytterhoeven
  2019-08-30 21:57   ` Niklas Söderlund
  2019-08-30 13:45 ` [PATCH v2 3/8] clk: renesas: rcar-gen3: Avoid double table iteration in SD .set_rate() Geert Uytterhoeven
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2019-08-30 13:45 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-renesas-soc, Geert Uytterhoeven

  - Use div64_ul() instead of div_u64() if the divisor is unsigned long,
    to avoid truncation to 32-bit on 64-bit platforms,
  - Use div_u64() for 64-by-32 divisions.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - New.
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index 043ab6ed9d550732..3480284a08308134 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -122,10 +122,10 @@ static long cpg_z_clk_round_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned int mult;
 
 	prate = *parent_rate / zclk->fixed_div;
-	mult = div_u64(rate * 32ULL, prate);
+	mult = div64_ul(rate * 32ULL, prate);
 	mult = clamp(mult, 1U, 32U);
 
-	return (u64)prate * mult / 32;
+	return div_u64((u64)prate * mult, 32);
 }
 
 static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
-- 
2.17.1


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

* [PATCH v2 3/8] clk: renesas: rcar-gen3: Avoid double table iteration in SD .set_rate()
  2019-08-30 13:45 [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate() Geert Uytterhoeven
  2019-08-30 13:45 ` [PATCH v2 1/8] clk: renesas: rcar-gen2: Improve arithmetic divisions Geert Uytterhoeven
  2019-08-30 13:45 ` [PATCH v2 2/8] clk: renesas: rcar-gen3: " Geert Uytterhoeven
@ 2019-08-30 13:45 ` Geert Uytterhoeven
  2019-08-30 22:06   ` Niklas Söderlund
  2019-08-30 13:45 ` [PATCH v2 4/8] clk: renesas: rcar-gen3: Absorb cpg_sd_clock_calc_div() Geert Uytterhoeven
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2019-08-30 13:45 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-renesas-soc, Geert Uytterhoeven

The .set_rate() callback for the SD clocks is always called with a valid
clock rate, returned by .round_rate().  Hence there is no need to
iterate through the divider table twice: once to repeat the work done by
.round_rate(), and a second time to find the corresponding divider
entry.

Just iterate once, looking for the divider that matches the passed clock
rate.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - No changes.
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index 3480284a08308134..9f457411984b1ca4 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -339,14 +339,14 @@ static long cpg_sd_clock_round_rate(struct clk_hw *hw, unsigned long rate,
 }
 
 static int cpg_sd_clock_set_rate(struct clk_hw *hw, unsigned long rate,
-				   unsigned long parent_rate)
+				 unsigned long parent_rate)
 {
 	struct sd_clock *clock = to_sd_clock(hw);
-	unsigned int div = cpg_sd_clock_calc_div(clock, rate, parent_rate);
 	unsigned int i;
 
 	for (i = 0; i < clock->div_num; i++)
-		if (div == clock->div_table[i].div)
+		if (rate == DIV_ROUND_CLOSEST(parent_rate,
+					      clock->div_table[i].div))
 			break;
 
 	if (i >= clock->div_num)
-- 
2.17.1


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

* [PATCH v2 4/8] clk: renesas: rcar-gen3: Absorb cpg_sd_clock_calc_div()
  2019-08-30 13:45 [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate() Geert Uytterhoeven
                   ` (2 preceding siblings ...)
  2019-08-30 13:45 ` [PATCH v2 3/8] clk: renesas: rcar-gen3: Avoid double table iteration in SD .set_rate() Geert Uytterhoeven
@ 2019-08-30 13:45 ` Geert Uytterhoeven
  2019-08-30 22:14   ` Niklas Söderlund
  2019-08-30 13:45 ` [PATCH v2 5/8] clk: renesas: rcar-gen3: Loop to find best rate in cpg_sd_clock_round_rate() Geert Uytterhoeven
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2019-08-30 13:45 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-renesas-soc, Geert Uytterhoeven

cpg_sd_clock_round_rate() is the sole caller of cpg_sd_clock_calc_div(),
hence absorb the latter into the former.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Split off from "clk: renesas: rcar-gen3: Switch SD clocks to
    .determine_rate()".
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index 9f457411984b1ca4..12ea5c9a671de788 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -309,15 +309,15 @@ static unsigned long cpg_sd_clock_recalc_rate(struct clk_hw *hw,
 				 clock->div_table[clock->cur_div_idx].div);
 }
 
-static unsigned int cpg_sd_clock_calc_div(struct sd_clock *clock,
-					  unsigned long rate,
-					  unsigned long parent_rate)
+static long cpg_sd_clock_round_rate(struct clk_hw *hw, unsigned long rate,
+				      unsigned long *parent_rate)
 {
 	unsigned long calc_rate, diff, diff_min = ULONG_MAX;
+	struct sd_clock *clock = to_sd_clock(hw);
 	unsigned int i, best_div = 0;
 
 	for (i = 0; i < clock->div_num; i++) {
-		calc_rate = DIV_ROUND_CLOSEST(parent_rate,
+		calc_rate = DIV_ROUND_CLOSEST(*parent_rate,
 					      clock->div_table[i].div);
 		diff = calc_rate > rate ? calc_rate - rate : rate - calc_rate;
 		if (diff < diff_min) {
@@ -326,16 +326,7 @@ static unsigned int cpg_sd_clock_calc_div(struct sd_clock *clock,
 		}
 	}
 
-	return best_div;
-}
-
-static long cpg_sd_clock_round_rate(struct clk_hw *hw, unsigned long rate,
-				      unsigned long *parent_rate)
-{
-	struct sd_clock *clock = to_sd_clock(hw);
-	unsigned int div = cpg_sd_clock_calc_div(clock, rate, *parent_rate);
-
-	return DIV_ROUND_CLOSEST(*parent_rate, div);
+	return DIV_ROUND_CLOSEST(*parent_rate, best_div);
 }
 
 static int cpg_sd_clock_set_rate(struct clk_hw *hw, unsigned long rate,
-- 
2.17.1


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

* [PATCH v2 5/8] clk: renesas: rcar-gen3: Loop to find best rate in cpg_sd_clock_round_rate()
  2019-08-30 13:45 [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate() Geert Uytterhoeven
                   ` (3 preceding siblings ...)
  2019-08-30 13:45 ` [PATCH v2 4/8] clk: renesas: rcar-gen3: Absorb cpg_sd_clock_calc_div() Geert Uytterhoeven
@ 2019-08-30 13:45 ` Geert Uytterhoeven
  2019-08-30 22:23   ` Niklas Söderlund
  2019-08-30 13:45 ` [PATCH v2 6/8] clk: renesas: rcar-gen2: Switch Z clock to .determine_rate() Geert Uytterhoeven
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2019-08-30 13:45 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-renesas-soc, Geert Uytterhoeven

cpg_sd_clock_round_rate() really needs the best rate, not the best
divider.  Hence change the iteration to find the former, and get rid of
the final division.

Add an out-of-range rate check while at it.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Split off from "clk: renesas: rcar-gen3: Switch SD clocks to
    .determine_rate()".
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index 12ea5c9a671de788..a612045cba7d97b7 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -312,21 +312,25 @@ static unsigned long cpg_sd_clock_recalc_rate(struct clk_hw *hw,
 static long cpg_sd_clock_round_rate(struct clk_hw *hw, unsigned long rate,
 				      unsigned long *parent_rate)
 {
-	unsigned long calc_rate, diff, diff_min = ULONG_MAX;
+	unsigned long best_rate = ULONG_MAX, diff_min = ULONG_MAX;
 	struct sd_clock *clock = to_sd_clock(hw);
-	unsigned int i, best_div = 0;
+	unsigned long calc_rate, diff;
+	unsigned int i;
 
 	for (i = 0; i < clock->div_num; i++) {
 		calc_rate = DIV_ROUND_CLOSEST(*parent_rate,
 					      clock->div_table[i].div);
 		diff = calc_rate > rate ? calc_rate - rate : rate - calc_rate;
 		if (diff < diff_min) {
-			best_div = clock->div_table[i].div;
+			best_rate = calc_rate;
 			diff_min = diff;
 		}
 	}
 
-	return DIV_ROUND_CLOSEST(*parent_rate, best_div);
+	if (best_rate > LONG_MAX)
+		return -EINVAL;
+
+	return best_rate;
 }
 
 static int cpg_sd_clock_set_rate(struct clk_hw *hw, unsigned long rate,
-- 
2.17.1


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

* [PATCH v2 6/8] clk: renesas: rcar-gen2: Switch Z clock to .determine_rate()
  2019-08-30 13:45 [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate() Geert Uytterhoeven
                   ` (4 preceding siblings ...)
  2019-08-30 13:45 ` [PATCH v2 5/8] clk: renesas: rcar-gen3: Loop to find best rate in cpg_sd_clock_round_rate() Geert Uytterhoeven
@ 2019-08-30 13:45 ` Geert Uytterhoeven
  2019-08-30 13:45 ` [PATCH v2 7/8] clk: renesas: rcar-gen3: Switch Z clocks " Geert Uytterhoeven
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2019-08-30 13:45 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-renesas-soc, Geert Uytterhoeven

As the .round_rate() callback returns a long clock rate, it cannot
return clock rates that do not fit in signed long, but do fit in
unsigned long.  Hence switch the Z clock on R-Car Gen2 from the old
.round_rate() callback to the newer .determine_rate() callback, which
does not suffer from this limitation.

This includes implementing range checking.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Use div_u64() for division by unsigned long,
  - Rebased.
---
 drivers/clk/renesas/rcar-gen2-cpg.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen2-cpg.c b/drivers/clk/renesas/rcar-gen2-cpg.c
index c378505830f0bacc..d4fa3dc3e2a2632f 100644
--- a/drivers/clk/renesas/rcar-gen2-cpg.c
+++ b/drivers/clk/renesas/rcar-gen2-cpg.c
@@ -63,19 +63,22 @@ static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
 	return div_u64((u64)parent_rate * mult, 32);
 }
 
-static long cpg_z_clk_round_rate(struct clk_hw *hw, unsigned long rate,
-				 unsigned long *parent_rate)
+static int cpg_z_clk_determine_rate(struct clk_hw *hw,
+				    struct clk_rate_request *req)
 {
-	unsigned long prate  = *parent_rate;
-	unsigned int mult;
+	unsigned long prate = req->best_parent_rate;
+	unsigned int min_mult, max_mult, mult;
 
-	if (!prate)
-		prate = 1;
+	min_mult = max(div64_ul(req->min_rate * 32ULL, prate), 1ULL);
+	max_mult = min(div64_ul(req->max_rate * 32ULL, prate), 32ULL);
+	if (max_mult < min_mult)
+		return -EINVAL;
 
-	mult = div64_ul(rate * 32ULL, prate);
-	mult = clamp(mult, 1U, 32U);
+	mult = div64_ul(req->rate * 32ULL, prate);
+	mult = clamp(mult, min_mult, max_mult);
 
-	return div_u64((u64)*parent_rate * mult, 32);
+	req->rate = div_u64((u64)prate * mult, 32);
+	return 0;
 }
 
 static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -126,7 +129,7 @@ static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 
 static const struct clk_ops cpg_z_clk_ops = {
 	.recalc_rate = cpg_z_clk_recalc_rate,
-	.round_rate = cpg_z_clk_round_rate,
+	.determine_rate = cpg_z_clk_determine_rate,
 	.set_rate = cpg_z_clk_set_rate,
 };
 
-- 
2.17.1


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

* [PATCH v2 7/8] clk: renesas: rcar-gen3: Switch Z clocks to .determine_rate()
  2019-08-30 13:45 [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate() Geert Uytterhoeven
                   ` (5 preceding siblings ...)
  2019-08-30 13:45 ` [PATCH v2 6/8] clk: renesas: rcar-gen2: Switch Z clock to .determine_rate() Geert Uytterhoeven
@ 2019-08-30 13:45 ` Geert Uytterhoeven
  2019-08-30 13:45 ` [PATCH v2 8/8] clk: renesas: rcar-gen3: Switch SD " Geert Uytterhoeven
  2019-09-03 22:09 ` [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch " Stephen Boyd
  8 siblings, 0 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2019-08-30 13:45 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-renesas-soc, Geert Uytterhoeven

As the .round_rate() callback returns a long clock rate, it cannot
return clock rates that do not fit in signed long, but do fit in
unsigned long.  Hence switch the Z clocks on R-Car Gen3 from the old
.round_rate() callback to the newer .determine_rate() callback, which
does not suffer from this limitation.

This includes implementing range checking.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Use div_u64() for division by unsigned long,
  - Rebased.
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index a612045cba7d97b7..176f908929b3920f 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -114,18 +114,24 @@ static unsigned long cpg_z_clk_recalc_rate(struct clk_hw *hw,
 				     32 * zclk->fixed_div);
 }
 
-static long cpg_z_clk_round_rate(struct clk_hw *hw, unsigned long rate,
-				 unsigned long *parent_rate)
+static int cpg_z_clk_determine_rate(struct clk_hw *hw,
+				    struct clk_rate_request *req)
 {
 	struct cpg_z_clk *zclk = to_z_clk(hw);
+	unsigned int min_mult, max_mult, mult;
 	unsigned long prate;
-	unsigned int mult;
 
-	prate = *parent_rate / zclk->fixed_div;
-	mult = div64_ul(rate * 32ULL, prate);
-	mult = clamp(mult, 1U, 32U);
+	prate = req->best_parent_rate / zclk->fixed_div;
+	min_mult = max(div64_ul(req->min_rate * 32ULL, prate), 1ULL);
+	max_mult = min(div64_ul(req->max_rate * 32ULL, prate), 32ULL);
+	if (max_mult < min_mult)
+		return -EINVAL;
 
-	return div_u64((u64)prate * mult, 32);
+	mult = div64_ul(req->rate * 32ULL, prate);
+	mult = clamp(mult, min_mult, max_mult);
+
+	req->rate = div_u64((u64)prate * mult, 32);
+	return 0;
 }
 
 static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -172,7 +178,7 @@ static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
 
 static const struct clk_ops cpg_z_clk_ops = {
 	.recalc_rate = cpg_z_clk_recalc_rate,
-	.round_rate = cpg_z_clk_round_rate,
+	.determine_rate = cpg_z_clk_determine_rate,
 	.set_rate = cpg_z_clk_set_rate,
 };
 
-- 
2.17.1


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

* [PATCH v2 8/8] clk: renesas: rcar-gen3: Switch SD clocks to .determine_rate()
  2019-08-30 13:45 [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate() Geert Uytterhoeven
                   ` (6 preceding siblings ...)
  2019-08-30 13:45 ` [PATCH v2 7/8] clk: renesas: rcar-gen3: Switch Z clocks " Geert Uytterhoeven
@ 2019-08-30 13:45 ` Geert Uytterhoeven
  2019-09-03 22:09 ` [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch " Stephen Boyd
  8 siblings, 0 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2019-08-30 13:45 UTC (permalink / raw)
  To: Michael Turquette, Stephen Boyd
  Cc: linux-clk, linux-renesas-soc, Geert Uytterhoeven

As the .round_rate() callback returns a long clock rate, it cannot
return clock rates that do not fit in signed long, but do fit in
unsigned long.  Hence switch the SD clocks on R-Car Gen3 from the old
.round_rate() callback to the newer .determine_rate() callback, which
does not suffer from this limitation.

This includes implementing range checking.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
v2:
  - Extract cpg_sd_clock_calc_div() absorption and best rate calculation
    into their own patches.
---
 drivers/clk/renesas/rcar-gen3-cpg.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
index 176f908929b3920f..7b47743097395102 100644
--- a/drivers/clk/renesas/rcar-gen3-cpg.c
+++ b/drivers/clk/renesas/rcar-gen3-cpg.c
@@ -315,8 +315,8 @@ static unsigned long cpg_sd_clock_recalc_rate(struct clk_hw *hw,
 				 clock->div_table[clock->cur_div_idx].div);
 }
 
-static long cpg_sd_clock_round_rate(struct clk_hw *hw, unsigned long rate,
-				      unsigned long *parent_rate)
+static int cpg_sd_clock_determine_rate(struct clk_hw *hw,
+				       struct clk_rate_request *req)
 {
 	unsigned long best_rate = ULONG_MAX, diff_min = ULONG_MAX;
 	struct sd_clock *clock = to_sd_clock(hw);
@@ -324,19 +324,24 @@ static long cpg_sd_clock_round_rate(struct clk_hw *hw, unsigned long rate,
 	unsigned int i;
 
 	for (i = 0; i < clock->div_num; i++) {
-		calc_rate = DIV_ROUND_CLOSEST(*parent_rate,
+		calc_rate = DIV_ROUND_CLOSEST(req->best_parent_rate,
 					      clock->div_table[i].div);
-		diff = calc_rate > rate ? calc_rate - rate : rate - calc_rate;
+		if (calc_rate < req->min_rate || calc_rate > req->max_rate)
+			continue;
+
+		diff = calc_rate > req->rate ? calc_rate - req->rate
+					     : req->rate - calc_rate;
 		if (diff < diff_min) {
 			best_rate = calc_rate;
 			diff_min = diff;
 		}
 	}
 
-	if (best_rate > LONG_MAX)
+	if (best_rate == ULONG_MAX)
 		return -EINVAL;
 
-	return best_rate;
+	req->rate = best_rate;
+	return 0;
 }
 
 static int cpg_sd_clock_set_rate(struct clk_hw *hw, unsigned long rate,
@@ -367,7 +372,7 @@ static const struct clk_ops cpg_sd_clock_ops = {
 	.disable = cpg_sd_clock_disable,
 	.is_enabled = cpg_sd_clock_is_enabled,
 	.recalc_rate = cpg_sd_clock_recalc_rate,
-	.round_rate = cpg_sd_clock_round_rate,
+	.determine_rate = cpg_sd_clock_determine_rate,
 	.set_rate = cpg_sd_clock_set_rate,
 };
 
-- 
2.17.1


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

* Re: [PATCH v2 1/8] clk: renesas: rcar-gen2: Improve arithmetic divisions
  2019-08-30 13:45 ` [PATCH v2 1/8] clk: renesas: rcar-gen2: Improve arithmetic divisions Geert Uytterhoeven
@ 2019-08-30 21:42   ` Niklas Söderlund
  0 siblings, 0 replies; 18+ messages in thread
From: Niklas Söderlund @ 2019-08-30 21:42 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Turquette, Stephen Boyd, linux-clk, linux-renesas-soc

Hi Geert,

Thanks for your work.

On 2019-08-30 15:45:08 +0200, Geert Uytterhoeven wrote:
>   - Use div64_ul() instead of div_u64() if the divisor is unsigned long,
>     to avoid truncation to 32-bit on 64-bit platforms,
>   - Prefer ULL constant suffixes over casts to u64,
>   - Prioritize multiplication over division, to increase accuracy.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> v2:
>   - New.
> ---
>  drivers/clk/renesas/rcar-gen2-cpg.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/renesas/rcar-gen2-cpg.c b/drivers/clk/renesas/rcar-gen2-cpg.c
> index f596a2dafcf4d8d1..c378505830f0bacc 100644
> --- a/drivers/clk/renesas/rcar-gen2-cpg.c
> +++ b/drivers/clk/renesas/rcar-gen2-cpg.c
> @@ -72,10 +72,10 @@ static long cpg_z_clk_round_rate(struct clk_hw *hw, unsigned long rate,
>  	if (!prate)
>  		prate = 1;
>  
> -	mult = div_u64((u64)rate * 32, prate);
> +	mult = div64_ul(rate * 32ULL, prate);
>  	mult = clamp(mult, 1U, 32U);
>  
> -	return *parent_rate / 32 * mult;
> +	return div_u64((u64)*parent_rate * mult, 32);
>  }
>  
>  static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> @@ -86,7 +86,7 @@ static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
>  	u32 val, kick;
>  	unsigned int i;
>  
> -	mult = div_u64((u64)rate * 32, parent_rate);
> +	mult = div64_ul(rate * 32ULL, parent_rate);
>  	mult = clamp(mult, 1U, 32U);
>  
>  	if (readl(zclk->kick_reg) & CPG_FRQCRB_KICK)
> -- 
> 2.17.1
> 

-- 
Regards,
Niklas Söderlund

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

* Re: [PATCH v2 2/8] clk: renesas: rcar-gen3: Improve arithmetic divisions
  2019-08-30 13:45 ` [PATCH v2 2/8] clk: renesas: rcar-gen3: " Geert Uytterhoeven
@ 2019-08-30 21:57   ` Niklas Söderlund
  0 siblings, 0 replies; 18+ messages in thread
From: Niklas Söderlund @ 2019-08-30 21:57 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Turquette, Stephen Boyd, linux-clk, linux-renesas-soc

Hi Geert,

Thanks for your work.

On 2019-08-30 15:45:09 +0200, Geert Uytterhoeven wrote:
>   - Use div64_ul() instead of div_u64() if the divisor is unsigned long,
>     to avoid truncation to 32-bit on 64-bit platforms,
>   - Use div_u64() for 64-by-32 divisions.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> v2:
>   - New.
> ---
>  drivers/clk/renesas/rcar-gen3-cpg.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
> index 043ab6ed9d550732..3480284a08308134 100644
> --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> @@ -122,10 +122,10 @@ static long cpg_z_clk_round_rate(struct clk_hw *hw, unsigned long rate,
>  	unsigned int mult;
>  
>  	prate = *parent_rate / zclk->fixed_div;
> -	mult = div_u64(rate * 32ULL, prate);
> +	mult = div64_ul(rate * 32ULL, prate);
>  	mult = clamp(mult, 1U, 32U);
>  
> -	return (u64)prate * mult / 32;
> +	return div_u64((u64)prate * mult, 32);
>  }
>  
>  static int cpg_z_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> -- 
> 2.17.1
> 

-- 
Regards,
Niklas Söderlund

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

* Re: [PATCH v2 3/8] clk: renesas: rcar-gen3: Avoid double table iteration in SD .set_rate()
  2019-08-30 13:45 ` [PATCH v2 3/8] clk: renesas: rcar-gen3: Avoid double table iteration in SD .set_rate() Geert Uytterhoeven
@ 2019-08-30 22:06   ` Niklas Söderlund
  0 siblings, 0 replies; 18+ messages in thread
From: Niklas Söderlund @ 2019-08-30 22:06 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Turquette, Stephen Boyd, linux-clk, linux-renesas-soc

Hi Geert,

Thanks for your patch.

On 2019-08-30 15:45:10 +0200, Geert Uytterhoeven wrote:
> The .set_rate() callback for the SD clocks is always called with a valid
> clock rate, returned by .round_rate().  Hence there is no need to
> iterate through the divider table twice: once to repeat the work done by
> .round_rate(), and a second time to find the corresponding divider
> entry.
> 
> Just iterate once, looking for the divider that matches the passed clock
> rate.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Clever :-)

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> v2:
>   - No changes.
> ---
>  drivers/clk/renesas/rcar-gen3-cpg.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
> index 3480284a08308134..9f457411984b1ca4 100644
> --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> @@ -339,14 +339,14 @@ static long cpg_sd_clock_round_rate(struct clk_hw *hw, unsigned long rate,
>  }
>  
>  static int cpg_sd_clock_set_rate(struct clk_hw *hw, unsigned long rate,
> -				   unsigned long parent_rate)
> +				 unsigned long parent_rate)
>  {
>  	struct sd_clock *clock = to_sd_clock(hw);
> -	unsigned int div = cpg_sd_clock_calc_div(clock, rate, parent_rate);
>  	unsigned int i;
>  
>  	for (i = 0; i < clock->div_num; i++)
> -		if (div == clock->div_table[i].div)
> +		if (rate == DIV_ROUND_CLOSEST(parent_rate,
> +					      clock->div_table[i].div))
>  			break;
>  
>  	if (i >= clock->div_num)
> -- 
> 2.17.1
> 

-- 
Regards,
Niklas Söderlund

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

* Re: [PATCH v2 4/8] clk: renesas: rcar-gen3: Absorb cpg_sd_clock_calc_div()
  2019-08-30 13:45 ` [PATCH v2 4/8] clk: renesas: rcar-gen3: Absorb cpg_sd_clock_calc_div() Geert Uytterhoeven
@ 2019-08-30 22:14   ` Niklas Söderlund
  0 siblings, 0 replies; 18+ messages in thread
From: Niklas Söderlund @ 2019-08-30 22:14 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Turquette, Stephen Boyd, linux-clk, linux-renesas-soc

Hi Geert,

Thanks for your work.

On 2019-08-30 15:45:11 +0200, Geert Uytterhoeven wrote:
> cpg_sd_clock_round_rate() is the sole caller of cpg_sd_clock_calc_div(),
> hence absorb the latter into the former.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> v2:
>   - Split off from "clk: renesas: rcar-gen3: Switch SD clocks to
>     .determine_rate()".
> ---
>  drivers/clk/renesas/rcar-gen3-cpg.c | 19 +++++--------------
>  1 file changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
> index 9f457411984b1ca4..12ea5c9a671de788 100644
> --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> @@ -309,15 +309,15 @@ static unsigned long cpg_sd_clock_recalc_rate(struct clk_hw *hw,
>  				 clock->div_table[clock->cur_div_idx].div);
>  }
>  
> -static unsigned int cpg_sd_clock_calc_div(struct sd_clock *clock,
> -					  unsigned long rate,
> -					  unsigned long parent_rate)
> +static long cpg_sd_clock_round_rate(struct clk_hw *hw, unsigned long rate,
> +				      unsigned long *parent_rate)
>  {
>  	unsigned long calc_rate, diff, diff_min = ULONG_MAX;
> +	struct sd_clock *clock = to_sd_clock(hw);
>  	unsigned int i, best_div = 0;
>  
>  	for (i = 0; i < clock->div_num; i++) {
> -		calc_rate = DIV_ROUND_CLOSEST(parent_rate,
> +		calc_rate = DIV_ROUND_CLOSEST(*parent_rate,
>  					      clock->div_table[i].div);
>  		diff = calc_rate > rate ? calc_rate - rate : rate - calc_rate;
>  		if (diff < diff_min) {
> @@ -326,16 +326,7 @@ static unsigned int cpg_sd_clock_calc_div(struct sd_clock *clock,
>  		}
>  	}
>  
> -	return best_div;
> -}
> -
> -static long cpg_sd_clock_round_rate(struct clk_hw *hw, unsigned long rate,
> -				      unsigned long *parent_rate)
> -{
> -	struct sd_clock *clock = to_sd_clock(hw);
> -	unsigned int div = cpg_sd_clock_calc_div(clock, rate, *parent_rate);
> -
> -	return DIV_ROUND_CLOSEST(*parent_rate, div);
> +	return DIV_ROUND_CLOSEST(*parent_rate, best_div);
>  }
>  
>  static int cpg_sd_clock_set_rate(struct clk_hw *hw, unsigned long rate,
> -- 
> 2.17.1
> 

-- 
Regards,
Niklas Söderlund

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

* Re: [PATCH v2 5/8] clk: renesas: rcar-gen3: Loop to find best rate in cpg_sd_clock_round_rate()
  2019-08-30 13:45 ` [PATCH v2 5/8] clk: renesas: rcar-gen3: Loop to find best rate in cpg_sd_clock_round_rate() Geert Uytterhoeven
@ 2019-08-30 22:23   ` Niklas Söderlund
  0 siblings, 0 replies; 18+ messages in thread
From: Niklas Söderlund @ 2019-08-30 22:23 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Michael Turquette, Stephen Boyd, linux-clk, linux-renesas-soc

Hi Geert,

Thanks for your patch.

On 2019-08-30 15:45:12 +0200, Geert Uytterhoeven wrote:
> cpg_sd_clock_round_rate() really needs the best rate, not the best
> divider.  Hence change the iteration to find the former, and get rid of
> the final division.
> 
> Add an out-of-range rate check while at it.
> 
> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>

Reviewed-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>

> ---
> v2:
>   - Split off from "clk: renesas: rcar-gen3: Switch SD clocks to
>     .determine_rate()".
> ---
>  drivers/clk/renesas/rcar-gen3-cpg.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/clk/renesas/rcar-gen3-cpg.c b/drivers/clk/renesas/rcar-gen3-cpg.c
> index 12ea5c9a671de788..a612045cba7d97b7 100644
> --- a/drivers/clk/renesas/rcar-gen3-cpg.c
> +++ b/drivers/clk/renesas/rcar-gen3-cpg.c
> @@ -312,21 +312,25 @@ static unsigned long cpg_sd_clock_recalc_rate(struct clk_hw *hw,
>  static long cpg_sd_clock_round_rate(struct clk_hw *hw, unsigned long rate,
>  				      unsigned long *parent_rate)
>  {
> -	unsigned long calc_rate, diff, diff_min = ULONG_MAX;
> +	unsigned long best_rate = ULONG_MAX, diff_min = ULONG_MAX;
>  	struct sd_clock *clock = to_sd_clock(hw);
> -	unsigned int i, best_div = 0;
> +	unsigned long calc_rate, diff;
> +	unsigned int i;
>  
>  	for (i = 0; i < clock->div_num; i++) {
>  		calc_rate = DIV_ROUND_CLOSEST(*parent_rate,
>  					      clock->div_table[i].div);
>  		diff = calc_rate > rate ? calc_rate - rate : rate - calc_rate;
>  		if (diff < diff_min) {
> -			best_div = clock->div_table[i].div;
> +			best_rate = calc_rate;
>  			diff_min = diff;
>  		}
>  	}
>  
> -	return DIV_ROUND_CLOSEST(*parent_rate, best_div);
> +	if (best_rate > LONG_MAX)
> +		return -EINVAL;
> +
> +	return best_rate;
>  }
>  
>  static int cpg_sd_clock_set_rate(struct clk_hw *hw, unsigned long rate,
> -- 
> 2.17.1
> 

-- 
Regards,
Niklas Söderlund

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

* Re: [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate()
  2019-08-30 13:45 [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate() Geert Uytterhoeven
                   ` (7 preceding siblings ...)
  2019-08-30 13:45 ` [PATCH v2 8/8] clk: renesas: rcar-gen3: Switch SD " Geert Uytterhoeven
@ 2019-09-03 22:09 ` Stephen Boyd
  2019-09-04  6:51   ` Geert Uytterhoeven
  8 siblings, 1 reply; 18+ messages in thread
From: Stephen Boyd @ 2019-09-03 22:09 UTC (permalink / raw)
  To: Geert Uytterhoeven, Michael Turquette
  Cc: linux-clk, linux-renesas-soc, Geert Uytterhoeven

Quoting Geert Uytterhoeven (2019-08-30 06:45:07)
>         Hi Mike, Stephen,
> 
> As the .round_rate() callback returns a long clock rate, it cannot
> return clock rates that do not fit in signed long, but do fit in
> unsigned long.  The newer .determine_rate() callback does not suffer
> from this limitation.  In addition, .determine_rate() provides the
> ability to specify a rate range.
> 
> This patch series performs the customary preparatory cleanups, and
> switches the Z (CPU) and SD clocks in the R-Car Gen2 and Gen3 clock
> drivers from the .round_rate() to the .determine_rate() callback.
> Note that the "div6" clock driver hasn't been converted yet, so div6
> clocks still use .round_rate().
> 
> Changes compared to v1[1]:
>   - Add preparatory arithmetic division improvements
>   - Split off cpg_sd_clock_calc_div() absorption and SD clock best rate
>     calculation,
>   - Use div_u64() for division by unsigned long,
> 
> This has been tested on R-Car M2-W and various R-Car Gen3, and should
> have no behavioral impact.

From what I recall the rate range code is broken but I can't remember
how. Anyway, I was just curious if you ran into any issues with that
code.

> 
> To be queued in clk-renesas-for-v5.5.

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

* Re: [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate()
  2019-09-03 22:09 ` [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch " Stephen Boyd
@ 2019-09-04  6:51   ` Geert Uytterhoeven
  2019-09-11 16:24     ` Stephen Boyd
  0 siblings, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2019-09-04  6:51 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Geert Uytterhoeven, Michael Turquette, linux-clk, Linux-Renesas

Hi Stephen,

On Wed, Sep 4, 2019 at 12:09 AM Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Geert Uytterhoeven (2019-08-30 06:45:07)
> > As the .round_rate() callback returns a long clock rate, it cannot
> > return clock rates that do not fit in signed long, but do fit in
> > unsigned long.  The newer .determine_rate() callback does not suffer
> > from this limitation.  In addition, .determine_rate() provides the
> > ability to specify a rate range.
> >
> > This patch series performs the customary preparatory cleanups, and
> > switches the Z (CPU) and SD clocks in the R-Car Gen2 and Gen3 clock
> > drivers from the .round_rate() to the .determine_rate() callback.
> > Note that the "div6" clock driver hasn't been converted yet, so div6
> > clocks still use .round_rate().
> >
> > Changes compared to v1[1]:
> >   - Add preparatory arithmetic division improvements
> >   - Split off cpg_sd_clock_calc_div() absorption and SD clock best rate
> >     calculation,
> >   - Use div_u64() for division by unsigned long,
> >
> > This has been tested on R-Car M2-W and various R-Car Gen3, and should
> > have no behavioral impact.
>
> From what I recall the rate range code is broken but I can't remember
> how. Anyway, I was just curious if you ran into any issues with that
> code.

I didn't ran into any issues.  But please note that in all tested cases, the
limits were 0 and ULONG_MAX anyway, so probably it didn't trigger the
broken cases in the rate range code.

So, is it good to have .determine_rate() support in individual clock drivers
now, or do you want me to postpone the last 3 patches of my series until the
rate range code is fixed?

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] 18+ messages in thread

* Re: [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate()
  2019-09-04  6:51   ` Geert Uytterhoeven
@ 2019-09-11 16:24     ` Stephen Boyd
  2019-10-21  8:35       ` Geert Uytterhoeven
  0 siblings, 1 reply; 18+ messages in thread
From: Stephen Boyd @ 2019-09-11 16:24 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Geert Uytterhoeven, Michael Turquette, linux-clk, Linux-Renesas

Quoting Geert Uytterhoeven (2019-09-03 23:51:10)
> Hi Stephen,
> 
> On Wed, Sep 4, 2019 at 12:09 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > Quoting Geert Uytterhoeven (2019-08-30 06:45:07)
> > > As the .round_rate() callback returns a long clock rate, it cannot
> > > return clock rates that do not fit in signed long, but do fit in
> > > unsigned long.  The newer .determine_rate() callback does not suffer
> > > from this limitation.  In addition, .determine_rate() provides the
> > > ability to specify a rate range.
> > >
> > > This patch series performs the customary preparatory cleanups, and
> > > switches the Z (CPU) and SD clocks in the R-Car Gen2 and Gen3 clock
> > > drivers from the .round_rate() to the .determine_rate() callback.
> > > Note that the "div6" clock driver hasn't been converted yet, so div6
> > > clocks still use .round_rate().
> > >
> > > Changes compared to v1[1]:
> > >   - Add preparatory arithmetic division improvements
> > >   - Split off cpg_sd_clock_calc_div() absorption and SD clock best rate
> > >     calculation,
> > >   - Use div_u64() for division by unsigned long,
> > >
> > > This has been tested on R-Car M2-W and various R-Car Gen3, and should
> > > have no behavioral impact.
> >
> > From what I recall the rate range code is broken but I can't remember
> > how. Anyway, I was just curious if you ran into any issues with that
> > code.
> 
> I didn't ran into any issues.  But please note that in all tested cases, the
> limits were 0 and ULONG_MAX anyway, so probably it didn't trigger the
> broken cases in the rate range code.
> 
> So, is it good to have .determine_rate() support in individual clock drivers
> now, or do you want me to postpone the last 3 patches of my series until the
> rate range code is fixed?
> 

It's fine to use .determine_rate() because we'll fix the problems in the
clk framework. So no concern from me here. Just curious if you ran into
any problems.


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

* Re: [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate()
  2019-09-11 16:24     ` Stephen Boyd
@ 2019-10-21  8:35       ` Geert Uytterhoeven
  0 siblings, 0 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2019-10-21  8:35 UTC (permalink / raw)
  To: Stephen Boyd
  Cc: Geert Uytterhoeven, Michael Turquette, linux-clk, Linux-Renesas

Hi Stephen,

On Wed, Sep 11, 2019 at 6:24 PM Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Geert Uytterhoeven (2019-09-03 23:51:10)
> > On Wed, Sep 4, 2019 at 12:09 AM Stephen Boyd <sboyd@kernel.org> wrote:
> > > Quoting Geert Uytterhoeven (2019-08-30 06:45:07)
> > > > As the .round_rate() callback returns a long clock rate, it cannot
> > > > return clock rates that do not fit in signed long, but do fit in
> > > > unsigned long.  The newer .determine_rate() callback does not suffer
> > > > from this limitation.  In addition, .determine_rate() provides the
> > > > ability to specify a rate range.
> > > >
> > > > This patch series performs the customary preparatory cleanups, and
> > > > switches the Z (CPU) and SD clocks in the R-Car Gen2 and Gen3 clock
> > > > drivers from the .round_rate() to the .determine_rate() callback.
> > > > Note that the "div6" clock driver hasn't been converted yet, so div6
> > > > clocks still use .round_rate().
> > > >
> > > > Changes compared to v1[1]:
> > > >   - Add preparatory arithmetic division improvements
> > > >   - Split off cpg_sd_clock_calc_div() absorption and SD clock best rate
> > > >     calculation,
> > > >   - Use div_u64() for division by unsigned long,
> > > >
> > > > This has been tested on R-Car M2-W and various R-Car Gen3, and should
> > > > have no behavioral impact.
> > >
> > > From what I recall the rate range code is broken but I can't remember
> > > how. Anyway, I was just curious if you ran into any issues with that
> > > code.
> >
> > I didn't ran into any issues.  But please note that in all tested cases, the
> > limits were 0 and ULONG_MAX anyway, so probably it didn't trigger the
> > broken cases in the rate range code.
> >
> > So, is it good to have .determine_rate() support in individual clock drivers
> > now, or do you want me to postpone the last 3 patches of my series until the
> > rate range code is fixed?
> >
>
> It's fine to use .determine_rate() because we'll fix the problems in the
> clk framework. So no concern from me here. Just curious if you ran into
> any problems.

Thanks, queued in clk-renesas-for-v5.5.

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] 18+ messages in thread

end of thread, other threads:[~2019-10-21  8:35 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-30 13:45 [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch to .determine_rate() Geert Uytterhoeven
2019-08-30 13:45 ` [PATCH v2 1/8] clk: renesas: rcar-gen2: Improve arithmetic divisions Geert Uytterhoeven
2019-08-30 21:42   ` Niklas Söderlund
2019-08-30 13:45 ` [PATCH v2 2/8] clk: renesas: rcar-gen3: " Geert Uytterhoeven
2019-08-30 21:57   ` Niklas Söderlund
2019-08-30 13:45 ` [PATCH v2 3/8] clk: renesas: rcar-gen3: Avoid double table iteration in SD .set_rate() Geert Uytterhoeven
2019-08-30 22:06   ` Niklas Söderlund
2019-08-30 13:45 ` [PATCH v2 4/8] clk: renesas: rcar-gen3: Absorb cpg_sd_clock_calc_div() Geert Uytterhoeven
2019-08-30 22:14   ` Niklas Söderlund
2019-08-30 13:45 ` [PATCH v2 5/8] clk: renesas: rcar-gen3: Loop to find best rate in cpg_sd_clock_round_rate() Geert Uytterhoeven
2019-08-30 22:23   ` Niklas Söderlund
2019-08-30 13:45 ` [PATCH v2 6/8] clk: renesas: rcar-gen2: Switch Z clock to .determine_rate() Geert Uytterhoeven
2019-08-30 13:45 ` [PATCH v2 7/8] clk: renesas: rcar-gen3: Switch Z clocks " Geert Uytterhoeven
2019-08-30 13:45 ` [PATCH v2 8/8] clk: renesas: rcar-gen3: Switch SD " Geert Uytterhoeven
2019-09-03 22:09 ` [PATCH v2 0/8] clk: renesas: rcar-gen2/gen3: Switch " Stephen Boyd
2019-09-04  6:51   ` Geert Uytterhoeven
2019-09-11 16:24     ` Stephen Boyd
2019-10-21  8:35       ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).