On Thu, Aug 24, 2023 at 11:48:10AM +0100, Biju Das wrote: > Fix the below cocci warnings by replacing do_div()->div64_ul() and > bound the result with a max value of U16_MAX. > > cocci warnings: > drivers/clk/clk-versaclock3.c:404:2-8: WARNING: do_div() does a > 64-by-32 division, please consider using div64_ul instead. It's nice, but there is a room for a couple of improvements. See below. ... > /* Determine best fractional part, which is 16 bit wide */ > div_frc = rate % *parent_rate; > div_frc *= BIT(16) - 1; > - do_div(div_frc, *parent_rate); > > - vc3->div_frc = (u32)div_frc; > + vc3->div_frc = min_t(u64, div64_ul(div_frc, *parent_rate), U16_MAX); First of all, as Linus Torvalds pointed out [1] min_t() is often used as a shortcut for clamp(). Second one, the BIT(16) - 1 is specifically used as the value related to the bits in the hardware and u16 is a software type that coincidentially has the same maximum as the above mentioned bitfield. That said, here this should be clamped to the [0 .. BIT(16) - 1] range. Since the patch is applied perhaps you can cook a followup. To everyone the message is simple: try to not use typed version of min() and clamp() at all. > rate = (*parent_rate * > - (vc3->div_int * VC3_2_POW_16 + div_frc) / VC3_2_POW_16); > + (vc3->div_int * VC3_2_POW_16 + vc3->div_frc) / VC3_2_POW_16); [1]: https://lore.kernel.org/lkml/CAHk-=whwEAc22wm8h9FESPB5X+P4bLDgv0erBQMa1buTNQW7tA@xxxxxxxxxxxxxx/ -- With Best Regards, Andy Shevchenko
