On 12/16/2014 01:27 PM, Lennart Sorensen wrote: >> I see why arch_timer_freq might skip the rounding error of 39, 15 and >> 55 Vs existing logic which is possibly at a truncation error risk >> (without errata for sysclk 13, 26 and 27MHz). >> >> all you'd probably need to do is cast rate, num and den to unsigned >> long and have a common computation logic. > > If that is acceptable, then sure I can do that. I liked avoiding casts > in general though. > >> if you'd really want to handle truncation error, it must be a separate >> patch of it's own - I would not mix it with the errata fix. > > Well there is no error in the existing code because the rate / den > is always a clean integer division. The problem is introduced by the key is "there is no error in existing code for existing value" :) -> the same code for new values will fail. and introducing (rate * num) / den without cast will fail for old values. > SYSCLK1 / 610 used by the emulated clock which is not a clean division. > > So for the existing logic, the calculation was perfect. It is only for > the errata case that it is a problem. > > So I think leaving the existing calculation but moved up works well, > and then having the alternate order calculation only in the errata case > seemed cleaner and avoids casts and 64bit math which I thought was > overall desirable. In general using DIV_ROUND_UP and family of macros(in kernel.h) is the right way of doing division in similar cases in Linux kernel. And for the same functionality, we want a common equation - if it does not fit well for all values (even if we introduce new values), then we must come to a better equation that will work for all values. What we do not do is to have two equations meant for doing the same thing. -- Regards, Nishanth Menon -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
