Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1373975 > unrolled thread

Re: [PATCH] clocksource: use clocksource_freq2mult() helper

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH] clocksource: use clocksource_freq2mult() helper John Stultz <john.stultz@linaro.org> - 2016-04-08 07:00 +0200

#1373975 — Re: [PATCH] clocksource: use clocksource_freq2mult() helper

On Wed, Mar 16, 2016 at 3:21 AM, Alexander Kuleshov
<kuleshovmail@gmail.com> wrote:
> which is introduced in the 7aca0c072 commit to simplify calculation of
> the mult and shift in the clocks_calc_mult_shift().
>
> Signed-off-by: Alexander Kuleshov <kuleshovmail@gmail.com>
> ---
>  kernel/time/clocksource.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/kernel/time/clocksource.c b/kernel/time/clocksource.c
> index 56ece14..de57923 100644
> --- a/kernel/time/clocksource.c
> +++ b/kernel/time/clocksource.c
> @@ -80,9 +80,7 @@ clocks_calc_mult_shift(u32 *mult, u32 *shift, u32 from, u32 to, u32 maxsec)
>          * accuracy and fits the maxsec conversion range:
>          */
>         for (sft = 32; sft > 0; sft--) {
> -               tmp = (u64) to << sft;
> -               tmp += from / 2;
> -               do_div(tmp, from);
> +               tmp = clocksource_freq2mult(from, sft, to);
>                 if ((tmp >> sftacc) == 0)
>                         break;
>         }

I'm worried you never tested this, as its clearly broken, and keeps my
systems from booting.

clocksource_freq2mult returns a u32. In the code being removed, tmp is
a u64. So this may truncate the high bits.

Since sftacc is often 32, this causes it to exit prematurely on the
first pass through the loop.

Please do make sure to boot test what you send out. I spent some time
thinking I had broken my qemu testing setup before I realized it was
this simple looking patch.

thanks
-john

[toc] | [standalone]

Back to top | Article view | linux.kernel

csiph-web