Re: [PATCH] clk: divider: Use DIV_ROUND_CLOSEST

[SÃren Brinkmann]

ping?

Any opinions?

	Thanks,
	SÃren

On Tue, Jan 29, 2013 at 05:25:44PM -0800, Soren Brinkmann wrote:
> Use the DIV_ROUND_CLOSEST macro to calculate divider values and minimize
> rounding errors.
> 
> Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx
> Cc: linux-kernel@xxxxxxxxxxxxxxx
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xxxxxxxxxx>
> ---
> Hi,
> 
> I just came across this behavior:
> I'm using the clk-divider as cpuclock which can be scaled through cpufreq.
> During boot I create an opp table which in this case holds the frequencies [MHz]
> 666, 333 and 222. To make sure those are actually valid frequencies I call
> clk_round_rate().
> I added a debug line in clk-divider.c:clk_divider_bestdiv() before the return
> in line 163 giving me:
> 	prate:1333333320, rate:333333330, div:4
> for adding the 333 MHz operating point.
> 
> In the running system this gives me:
> zynq:/sys/devices/system/cpu/cpu0/cpufreq # cat scaling_available_frequencies 
> 222222 333333 666666 
> zynq:/sys/devices/system/cpu/cpu0/cpufreq # cat scaling_cur_freq 
>  666666
> 
>  So far, so good. But now, let's scale the frequency:
> zynq:/sys/devices/system/cpu/cpu0/cpufreq # echo 333333 > scaling_setspeed 
> zynq:/sys/devices/system/cpu/cpu0/cpufreq # cat scaling_cur_freq
>  266666
> 
> And the corresponding debug line:
> 	prate:1333333320, rate:333333000, div:5
> 
> So, with DIV_ROUND_UP an actual divider of 4.00000396 becomes 5, resulting in a
> huge error.
> 
> 	Regards,
> 	SÃren
> 
>  drivers/clk/clk-divider.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk-divider.c b/drivers/clk/clk-divider.c
> index a9204c6..32e1b6a 100644
> --- a/drivers/clk/clk-divider.c
> +++ b/drivers/clk/clk-divider.c
> @@ -157,7 +157,7 @@ static int clk_divider_bestdiv(struct clk_hw *hw, unsigned long rate,
>  
>  	if (!(__clk_get_flags(hw->clk) & CLK_SET_RATE_PARENT)) {
>  		parent_rate = *best_parent_rate;
> -		bestdiv = DIV_ROUND_UP(parent_rate, rate);
> +		bestdiv = DIV_ROUND_CLOSEST(parent_rate, rate);
>  		bestdiv = bestdiv == 0 ? 1 : bestdiv;
>  		bestdiv = bestdiv > maxdiv ? maxdiv : bestdiv;
>  		return bestdiv;
> @@ -207,7 +207,7 @@ static int clk_divider_set_rate(struct clk_hw *hw, unsigned long rate,
>  	unsigned long flags = 0;
>  	u32 val;
>  
> -	div = parent_rate / rate;
> +	div = DIV_ROUND_CLOSEST(parent_rate, rate);
>  	value = _get_val(divider, div);
>  
>  	if (value > div_mask(divider))
> -- 
> 1.8.1.2
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/