Messages in this thread | | | Date | Mon, 14 Sep 2015 14:08:39 -0700 | From | Brian Norris <> | Subject | Re: [PATCH] clk: divider: handle integer overflow when dividing large clock rates |
| |
(New address)
Hi Mike,
On Mon, Apr 27, 2015 at 08:49:10AM -0700, Michael Turquette wrote: > Quoting Michael Turquette (2015-04-14 15:11:37) > > Quoting Brian Norris (2015-04-13 16:03:21) > > > On 32-bit architectures, 'unsigned long' (the type used to hold clock > > > rates, in Hz) is often only 32 bits wide. DIV_ROUND_UP() (as used in, > > > e.g., commit b11d282dbea2 "clk: divider: fix rate calculation for > > > fractional rates") can yield an integer overflow on clock rates that are > > > not (by themselves) too large to fit in 32 bits, because it performs > > > addition before the division. See for example: > > > > > > DIV_ROUND_UP(3000000000, 1500000000) = (3.0G + 1.5G - 1) / 1.5G > > > = OVERFLOW / 1.5G > > > > > > This patch fixes such cases by always promoting the dividend to 64-bits > > > (unsigned long long) before doing the division. While this patch does > > > not resolve the issue with large clock rates across the common clock > > > framework nor address the problems with doing full 64-bit arithmetic on > > > a 32-bit architecture, it does fix some issues seen when using clock > > > dividers on a 3GHz reference clock to produce a 1.5GHz CPU clock for an > > > ARMv7 Brahma B15 SoC. > > > > > > Signed-off-by: Brian Norris <computersforpeace@gmail.com> > > > Reference: lkml.kernel.org/g/20150413201433.GQ32500@ld-irv-0074 > > > --- > > > I'll admit I only compile-tested this particular patch. I have tested a version > > > of this patch on top of a few backports on an older kernel, and everything > > > works fine. Unforunately, some of my SoC's clock drivers still rely on > > > out-of-tree code. > > > > I smoke tested this on some hardware and it seemed fine to me. I'll give > > some time for others to comment, otherwise I'll take this for 4.2 after > > -rc1 drops. > > Applied to clk-next.
I was rebasing my old patches onto Linus' latest, and I noticed that this one never got in.
Brian
| |