[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-ID: <1585132.bTTmCCK9Ot@diego>
Date: Tue, 02 Sep 2014 09:33:21 +0200
From: Heiko Stübner <heiko@...ech.de>
To: Mike Turquette <mturquette@...aro.org>
Cc: linux-kernel@...r.kernel.org,
Heikki Krogerus <heikki.krogerus@...ux.intel.com>
Subject: Re: [PATCH] clk: fractional-divider: cast parent_rate to u64 before multiplying
Am Montag, 1. September 2014, 17:26:29 schrieb Mike Turquette:
> Quoting Heiko Stübner (2014-08-28 03:46:10)
>
> > On 32bit architectures, like ARM calculating the fractional rate will
> > do the multiplication before converting the value to u64 when it gets
> > assigned to ret, which can produce overflows.
> >
> > The error in question happened with a parent_rate of 386MHz, m = 3000,
> > n = 60000, which resulted in a wrong rate value of 15812Hz.
> >
> > Therefore cast parent_rate to u64 to make sure the multiplication
> > happens in a 64bit space and produces the correct 192MHz in the example.
> >
> > Signed-off-by: Heiko Stuebner <heiko@...ech.de>
>
> Looks good to me. Have you observed this issue using the vanilla
> kernel.org sources or on a downstream tree? I'm trying to decide if I
> should take this into clk-fixes or clk-next.
>
> If this doesn't happen in practice on any merged platform then I'll take
> it for 3.18.
It happens on the Rockchip platform with the newly added fractional branches
you just applied ... so it doesn't look like anybody else is affected right
now.
Heiko
>
> Regards,
> Mike
>
> > ---
> >
> > drivers/clk/clk-fractional-divider.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/clk-fractional-divider.c
> > b/drivers/clk/clk-fractional-divider.c index ede685c..82a59d0 100644
> > --- a/drivers/clk/clk-fractional-divider.c
> > +++ b/drivers/clk/clk-fractional-divider.c
> > @@ -36,7 +36,7 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw
> > *hw,>
> > m = (val & fd->mmask) >> fd->mshift;
> > n = (val & fd->nmask) >> fd->nshift;
> >
> > - ret = parent_rate * m;
> > + ret = (u64)parent_rate * m;
> >
> > do_div(ret, n);
> >
> > return ret;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists