* [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).