Re: [PATCH v2 1/3] clk: divider: Fix the divisor for rates above 4.29 GHz

[Brian Masney]

Hi Carlo,

> DIV_ROUND_UP_ULL() takes a 32-bit divisor, so on 64-bit platforms a
> requested rate above 4.29 GHz is truncated. A divider fed by a 5.94 GHz
> PLL then computes a divisor of 7, not 1, for 5.94 GHz.
> 
> Round up with (parent_rate - 1) / rate + 1 instead, building on an
> earlier fix by Sebastian Reichel. It divides by the full rate and does
> not overflow with clk_round_rate(clk, ULONG_MAX).
> 
> Fixes: 9556f9dad8f5 ("clk: divider: handle integer overflow when dividing large clock rates")
> Suggested-by: Jerome Brunet <jbrunet@xxxxxxxxxxxx>
> Suggested-by: David Laight <david.laight.linux@xxxxxxxxx>
> Signed-off-by: Carlo Caione <ccaione@xxxxxxxxxxxx>
> ---
>  drivers/clk/clk-divider.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index f68216c65d3d..c3f46f9ee451 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -223,11 +223,21 @@ static int _round_down_table(const struct clk_div_table *table, int div)
>  	return down;
>  }
>  
> +/*
> + * DIV_ROUND_UP() without the addition, which overflows for rates close to
> + * ULONG_MAX, as with clk_round_rate(clk, ULONG_MAX)
> + */
> +static unsigned long _get_div_round_up(unsigned long parent_rate,
> +				       unsigned long rate)
> +{
> +	return parent_rate ? (parent_rate - 1) / rate + 1 : 0;
> +}
> +
>  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 = _get_div_round_up(parent_rate, rate);

_get_div_round_up() returns an unsigned long however it's downsized to
an int here.

Brian

-- 
Brian Masney <masneyb@xxxxxxxxxx>