Re: [PATCH v2] iio: frequency: adf4350: replace loop with fls_long()

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 16 Mar 2026 14:40:39 +0200
Andy Shevchenko <andriy.shevchenko@intel.com> wrote:

> On Sat, Mar 14, 2026 at 01:20:06PM -0400, Neel Bullywon wrote:
> > Address the TODO in adf4350_set_freq() by replacing the iterative
> > power-of-2 shift loop with a constant-time bitwise calculation.
> > 
> > By comparing the highest set bits of the target constant and freq
> > using fls_long(), we can calculate the required RF divider selection
> > in a single step without relying on expensive 64-bit division.
> > 
> > This ensures freq is properly shifted to meet or exceed the minimum
> > VCO frequency.  
> 
> ...
> 
> >  	st->r4_rf_div_sel = 0;  
> 
> > -	while (freq < ADF4350_MIN_VCO_FREQ) {
> > -		freq <<= 1;
> > -		st->r4_rf_div_sel++;
> > +	if (freq < ADF4350_MIN_VCO_FREQ) {
> > +		st->r4_rf_div_sel = fls_long(ADF4350_MIN_VCO_FREQ - 1) - fls_long(freq);
> > +		if ((freq << st->r4_rf_div_sel) < ADF4350_MIN_VCO_FREQ)
> > +			st->r4_rf_div_sel++;
> >  	}
> >  
> > +	freq <<= st->r4_rf_div_sel;  
> 
> Yeah, unfortunately it looks like too complicated in comparison with the
> original code. Have you checked binary output? My gut feelings that this
> gives also a lot of code in addition to.
> 
> Little optimisation can be like:
> 
> 	if (freq < ADF4350_MIN_VCO_FREQ) {
> 		st->r4_rf_div_sel = fls_long(ADF4350_MIN_VCO_FREQ - 1) - fls_long(freq);
> 		freq <<= st->r4_rf_div_sel;
> 		if (freq < ADF4350_MIN_VCO_FREQ) {
> 			st->r4_rf_div_sel++;
> 			freq <<= 1;
> 		}
> 	}
> 
> but it is still too much.
> 
> ...
> 
> Maybe we simply need to replace TODO with a NOTE explaining that if better algo
> is found we can replace.
> 
This comment update makes sense to me as will make people look for previous
attempts before sending a new one!

Neel, you ran into tricky challenge - thanks for giving it a go!

Jonathan