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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Tue, Mar 10, 2026 at 10:01:15PM -0400, Neel Bullywon wrote:
> Address the TODO in adf4350_set_freq() by replacing the iterative
> power-of-2 shift loop with the standard order_base_2() macro.
> 
> By utilizing DIV_ROUND_UP_ULL(), we
> can calculate the required RF divider selection in a single step.
> 
> This ensures freq is properly shifted to meet or exceed the minimum
> VCO frequency.

Thanks! My comments below.

...

> -	st->r4_rf_div_sel = 0;
> -
>  	/*
> -	 * !\TODO: The below computation is making sure we get a power of 2
> -	 * shift (st->r4_rf_div_sel) so that freq becomes higher or equal to
> -	 * ADF4350_MIN_VCO_FREQ. This might be simplified with fls()/fls_long()
> -	 * and friends.
> +	 * Calculate the required RF divider selection (power of 2 shift)
> +	 * to ensure the VCO frequency is >= ADF4350_MIN_VCO_FREQ.
>  	 */
> -	while (freq < ADF4350_MIN_VCO_FREQ) {
> -		freq <<= 1;
> -		st->r4_rf_div_sel++;
> -	}
> +	st->r4_rf_div_sel = order_base_2(DIV_ROUND_UP_ULL(ADF4350_MIN_VCO_FREQ, freq));

We don't need 64-bit division even with your approach as values are 32-bit for
this case (yeah, I know that they have 64-bit types).

What you need is to do as suggested by comment: use fls_long() against the
constant and freq to see the difference in value, if there is none the while
will become just a check and a single shift.

Or consider it as the opposite problem: "How much the given constant is now
bigger than freq?" With that it might be more obvious solution.

I.o.w. think about this a bit more.

> +	freq <<= st->r4_rf_div_sel;
>  

I would move this blank line to be before freq shift, this will associate it
with the check below.

>  	if (freq > ADF4350_MAX_FREQ_45_PRESC) {
>  		prescaler = ADF4350_REG1_PRESCALER;

-- 
With Best Regards,
Andy Shevchenko