Messages in this thread | | | From | Andy Shevchenko <> | Date | Mon, 19 Jul 2021 15:45:08 +0300 | Subject | Re: [PATCH v2 07/11] math64: New DIV_U64_ROUND_CLOSEST helper |
| |
On Fri, Jun 25, 2021 at 6:39 PM Pali Rohár <pali@kernel.org> wrote: > On Friday 25 June 2021 17:22:31 Geert Uytterhoeven wrote: > > On Fri, Jun 25, 2021 at 4:37 PM Pali Rohár <pali@kernel.org> wrote:
...
> > > +/* > > > + * DIV_U64_ROUND_CLOSEST - unsigned 64bit divide with 32bit divisor rounded to nearest integer
> > > + * @dividend: unsigned 64bit dividend
(1)
> > > + * @divisor: unsigned 32bit divisor > > > + * > > > + * Divide unsigned 64bit dividend by unsigned 32bit divisor > > > + * and round to closest integer. > > > + * > > > + * Return: dividend / divisor rounded to nearest integer > > > + */ > > > +#define DIV_U64_ROUND_CLOSEST(dividend, divisor) \ > > > + ({ u32 _tmp = (divisor); div_u64((u64)(dividend) + _tmp / 2, _tmp); }) > > > > Given "dividend" should already be an unsigned 64-bit value, I don't > > think the cast to "u64" is needed. Similar macros in this file also > > don't have the cast. > > It is just to ensure that plus operation between dividend and _tmp is > evaluated in 64-bit context to prevent overflow. Just a case when user > calls this macro with 32-bit dividend param.
This contradicts (1).
> As it is a macro (and not > inline function) type is not automatically enforced. > > DIV_S64_ROUND_CLOSEST macro assigns its argument into temporary 64-bit > variable which then ensures usage of 64-bit arithmetic operations. Same > applies for DIV64_U64_ROUND_CLOSEST and DIV64_U64_ROUND_UP macros. > > So this is reason why I added explicit cast to u64.
I don't see the reason for casting in the current code. Probably you need to rephrase documentation to explain why it's there.
-- With Best Regards, Andy Shevchenko
| |