[PATCH v2] drm/i915: Explicitly cast divisor and use div_u64()

Andi Shyti andi.shyti at linux.intel.com
Wed Aug 7 14:58:30 UTC 2024


Hi Thorsten,

> >  			/* This check is primarily to ensure that oa_period <=
> > -			 * UINT32_MAX (before passing to do_div which only
> > +			 * UINT32_MAX (before passing it to div_u64 which only
> >  			 * accepts a u32 denominator), but we can also skip
> >  			 * checking anything < 1Hz which implicitly can't be
> >  			 * limited via an integer oa_max_sample_rate.
> >  			 */
> >  			if (oa_period <= NSEC_PER_SEC) {
> > -				u64 tmp = NSEC_PER_SEC;
> > -				do_div(tmp, oa_period);
> > -				oa_freq_hz = tmp;
> > +				oa_freq_hz = div_u64(NSEC_PER_SEC, (u32)oa_period);
> >  			} else
> >  				oa_freq_hz = 0;
> 
> Non-blocking suggestion: this looks like it can be inlined.  And if the
> inline route is taken, it might be best to invert the conditional check
> like such:
> 
> oa_freq_hz = oa_period > NSEC_PER_SEC ? 0 :
>                                      div_u64(NSEC_PER_SEC, (u32)oa_period);
> 
> I think this is just a matter of preference, though.  The explicit if-else
> block is definitely clearer.

It's also stylistically wrong given that now the if/else don't
need the brackets anymore, triggering a checkpatch error.

Thorsten do you mind resending it either following Jonathan's
suggestion (my favourite, as well) or fix the bracket issue
following the kernel style.

Andi


More information about the dri-devel mailing list