[PATCH] clk: renesas: r9a06g032: Switch to .determine_rate()

[PATCH] clk: renesas: r9a06g032: Switch to .determine_rate()

[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 divider clocks on RZ/N1 from the old
.round_rate() callback to the newer .determine_rate() callback, which
does not suffer from this limitation.

Note that range checking is not yet implemented.

Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
---
Compile-tested only due to lack of hardware.

To be queued in renesas-clk for v5.14.
---
 drivers/clk/renesas/r9a06g032-clocks.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/renesas/r9a06g032-clocks.c b/drivers/clk/renesas/r9a06g032-clocks.c
index 71b11443f6fc3801..c99942f0e4d4c751 100644
--- a/drivers/clk/renesas/r9a06g032-clocks.c
+++ b/drivers/clk/renesas/r9a06g032-clocks.c
@@ -604,20 +604,19 @@ r9a06g032_div_clamp_div(struct r9a06g032_clk_div *clk,
 	return div;
 }
 
-static long
-r9a06g032_div_round_rate(struct clk_hw *hw,
-			 unsigned long rate, unsigned long *prate)
+static int
+r9a06g032_div_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
 {
 	struct r9a06g032_clk_div *clk = to_r9a06g032_div(hw);
-	u32 div = DIV_ROUND_UP(*prate, rate);
+	u32 div = DIV_ROUND_UP(req->best_parent_rate, req->rate);
 
 	pr_devel("%s %pC %ld (prate %ld) (wanted div %u)\n", __func__,
-		 hw->clk, rate, *prate, div);
+		 hw->clk, req->rate, req->best_parent_rate, div);
 	pr_devel("   min %d (%ld) max %d (%ld)\n",
-		 clk->min, DIV_ROUND_UP(*prate, clk->min),
-		clk->max, DIV_ROUND_UP(*prate, clk->max));
+		 clk->min, DIV_ROUND_UP(req->best_parent_rate, clk->min),
+		 clk->max, DIV_ROUND_UP(req->best_parent_rate, clk->max));
 
-	div = r9a06g032_div_clamp_div(clk, rate, *prate);
+	div = r9a06g032_div_clamp_div(clk, req->rate, req->best_parent_rate);
 	/*
 	 * this is a hack. Currently the serial driver asks for a clock rate
 	 * that is 16 times the baud rate -- and that is wildly outside the
@@ -630,11 +629,13 @@ r9a06g032_div_round_rate(struct clk_hw *hw,
 	if (clk->index == R9A06G032_DIV_UART ||
 	    clk->index == R9A06G032_DIV_P2_PG) {
 		pr_devel("%s div uart hack!\n", __func__);
-		return clk_get_rate(hw->clk);
+		req->rate = clk_get_rate(hw->clk);
+		return 0;
 	}
+	req->rate = DIV_ROUND_UP(req->best_parent_rate, div);
 	pr_devel("%s %pC %ld / %u = %ld\n", __func__, hw->clk,
-		 *prate, div, DIV_ROUND_UP(*prate, div));
-	return DIV_ROUND_UP(*prate, div);
+		 req->best_parent_rate, div, req->rate);
+	return 0;
 }
 
 static int
@@ -663,7 +664,7 @@ r9a06g032_div_set_rate(struct clk_hw *hw,
 
 static const struct clk_ops r9a06g032_clk_div_ops = {
 	.recalc_rate = r9a06g032_div_recalc_rate,
-	.round_rate = r9a06g032_div_round_rate,
+	.determine_rate = r9a06g032_div_determine_rate,
 	.set_rate = r9a06g032_div_set_rate,
 };
 
-- 
2.25.1

Re: [PATCH] clk: renesas: r9a06g032: Switch to .determine_rate()

[Stephen Boyd]

Quoting Geert Uytterhoeven (2021-04-01 06:03:24)
> diff --git a/drivers/clk/renesas/r9a06g032-clocks.c b/drivers/clk/renesas/r9a06g032-clocks.c
> index 71b11443f6fc3801..c99942f0e4d4c751 100644
> --- a/drivers/clk/renesas/r9a06g032-clocks.c
> +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> @@ -630,11 +629,13 @@ r9a06g032_div_round_rate(struct clk_hw *hw,
>         if (clk->index == R9A06G032_DIV_UART ||
>             clk->index == R9A06G032_DIV_P2_PG) {
>                 pr_devel("%s div uart hack!\n", __func__);
> -               return clk_get_rate(hw->clk);
> +               req->rate = clk_get_rate(hw->clk);

Can this use clk_hw_get_rate()? Or it needs to be clk_get_rate() to make
sure the rate doesn't change while querying the framework... from the
framework? Another patch is preferred if you're interested in making the
change.

> +               return 0;
>         }
> +       req->rate = DIV_ROUND_UP(req->best_parent_rate, div);
>         pr_devel("%s %pC %ld / %u = %ld\n", __func__, hw->clk,
> -                *prate, div, DIV_ROUND_UP(*prate, div));
> -       return DIV_ROUND_UP(*prate, div);
> +                req->best_parent_rate, div, req->rate);
> +       return 0;
>  }
>  
>  static int

Re: [PATCH] clk: renesas: r9a06g032: Switch to .determine_rate()

[Geert Uytterhoeven]

Hi Stephen,

On Thu, Apr 8, 2021 at 9:09 AM Stephen Boyd <sboyd@kernel.org> wrote:
> Quoting Geert Uytterhoeven (2021-04-01 06:03:24)
> > diff --git a/drivers/clk/renesas/r9a06g032-clocks.c b/drivers/clk/renesas/r9a06g032-clocks.c
> > index 71b11443f6fc3801..c99942f0e4d4c751 100644
> > --- a/drivers/clk/renesas/r9a06g032-clocks.c
> > +++ b/drivers/clk/renesas/r9a06g032-clocks.c
> > @@ -630,11 +629,13 @@ r9a06g032_div_round_rate(struct clk_hw *hw,
> >         if (clk->index == R9A06G032_DIV_UART ||
> >             clk->index == R9A06G032_DIV_P2_PG) {
> >                 pr_devel("%s div uart hack!\n", __func__);
> > -               return clk_get_rate(hw->clk);
> > +               req->rate = clk_get_rate(hw->clk);
>
> Can this use clk_hw_get_rate()? Or it needs to be clk_get_rate() to make
> sure the rate doesn't change while querying the framework... from the
> framework? Another patch is preferred if you're interested in making the
> change.

Probably it can.  But as I don't have access to the hardware, I try to
be conservative.

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