Messages in this thread | | | Subject | Re: [PATCH v1 2/2] clk: divider: Properly handle rates exceeding UINT_MAX | From | Christopher Obbard <> | Date | Sat, 20 May 2023 08:57:38 +0100 |
| |
Hi Sebastian,
On Fri, 2023-05-19 at 21:05 +0200, Sebastian Reichel wrote: > Requesting rates exceeding UINT_MAX (so roughly 4.3 GHz) results > in very small rate being chosen instead of very high ones, since > DIV_ROUND_UP_ULL takes a 32 bit integer as second argument. > > Correct this by using DIV64_U64_ROUND_UP instead, which takes proper > 64 bit values for dividend and divisor. > > Note, that this is usually not an issue. ULONG_MAX sets the lower > 32 bits and thus effectively requests UINT_MAX. On most platforms > that is good enough. To trigger a real bug one of the following > conditions must be met: > > * A parent clock with more than 8.5 GHz is available > * Instead of ULONG_MAX a specific frequency like 4.3 GHz is > requested. That would end up becoming 5 MHz instead :) > > Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
This patch series fixes the error on Rockchip RK3588 on ROCK 5 Model B.
Tested-by: Christopher Obbard <chris.obbard@collabora.com>
> --- > drivers/clk/clk-divider.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c > index a2c2b5203b0a..dddaaf0f9d25 100644 > --- a/drivers/clk/clk-divider.c > +++ b/drivers/clk/clk-divider.c > @@ -220,7 +220,7 @@ static int _div_round_up(const struct clk_div_table *table, > unsigned long parent_rate, unsigned long rate, > unsigned long flags) > { > - int div = DIV_ROUND_UP_ULL((u64)parent_rate, rate); > + int div = DIV64_U64_ROUND_UP(parent_rate, rate); > > if (flags & CLK_DIVIDER_POWER_OF_TWO) > div = __roundup_pow_of_two(div); > @@ -237,7 +237,7 @@ static int _div_round_closest(const struct clk_div_table *table, > int up, down; > unsigned long up_rate, down_rate; > > - up = DIV_ROUND_UP_ULL((u64)parent_rate, rate); > + up = DIV64_U64_ROUND_UP(parent_rate, rate); > down = parent_rate / rate; > > if (flags & CLK_DIVIDER_POWER_OF_TWO) { > @@ -473,7 +473,7 @@ int divider_get_val(unsigned long rate, unsigned long parent_rate, > { > unsigned int div, value; > > - div = DIV_ROUND_UP_ULL((u64)parent_rate, rate); > + div = DIV64_U64_ROUND_UP(parent_rate, rate); > > if (!_is_valid_div(table, div, flags)) > return -EINVAL; > -- > 2.39.2 > >
| |