| Message ID | 20190830134515.11925-1-geert+renesas@glider.be (mailing list archive) |
|---|---|
| Headers | show |
| Series | clk: renesas: rcar-gen2/gen3: Switch to .determine_rate() | expand |
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.
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
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.
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
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(-)