Re: [PATCH v4 08/14] iio: adc: ad7768: Add per-channel conversion delay

On Fri, Aug 21, 2026 at 04:07:01PM +0200, Janani Sunil wrote:
> Expose the per-channel synchronization phase offset through the IIO
> conversion-delay attribute.
> 
> Derive the delay resolution from MCLK, power mode and decimation rate.
> Validate the requested delay and program the corresponding phase
> register when applying the active channel configuration.

...

> +static int ad7768_get_convdelay_params(struct ad7768_state *st, unsigned int ch,
> +				       struct ad7768_convdelay_params *params)
> +{
> +	struct ad7768_freq_config f_cfg;
> +	unsigned int dec_rate;
> +	unsigned int mclk_div;
> +	unsigned int mult;
> +	u64 mclk;
> +	int ret;
> +
> +	ret = ad7768_get_freq_cfg(st, st->ch_freq[ch], &f_cfg);
> +	if (ret)
> +		return ret;
> +
> +	dec_rate = ad7768_dec_rate[f_cfg.dec_rate];
> +	switch (dec_rate) {
> +	case 32:
> +		params->shift = 3;
> +		params->max_raw = 31;
> +		mult = 1;
> +		break;
> +	case 64:
> +		params->shift = 2;
> +		params->max_raw = 63;
> +		mult = 1;
> +		break;
> +	case 128:
> +		params->shift = 1;
> +		params->max_raw = 127;
> +		mult = 1;
> +		break;
> +	case 256:
> +		params->shift = 0;
> +		params->max_raw = 255;
> +		mult = 1;
> +		break;
> +	case 512:
> +		params->shift = 0;
> +		params->max_raw = 255;
> +		mult = 2;
> +		break;
> +	case 1024:
> +		params->shift = 0;
> +		params->max_raw = 255;
> +		mult = 4;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}

So, this is just a bit twiddling of the dec_rate.

Can be written like

	/* Optional, perhaps shouldn't appear */
	if (!dec_rate)
		return -EINVAL;

	/* Same? What does table allow to have? */
	if (!is_power_of_two(dec_rate))
		return -EINVAL;

	mult = ilog2(dec_rate);
	if (mult > 10) {
		return -EINVAL;
	} else if (mult > 8) {
		params->shift = 0;
		params->max_raw = 255;
		mult = BIT(mult - 8);
	} else if (mult > 5) {
		params->shift = 8 - mult;
		params->max_raw = dec_rate - 1;
		mult = 1;
	} else {
		return -EINVAL;
	}

TBH, I don't know which looks easier to read. It all depends on what datasheet
says about these parameters and multiplier and what the table allows to have.

> +	mclk = clk_get_rate(st->mclk);
> +	if (!mclk)
> +		return -EINVAL;
> +
> +	mclk_div = ad7768_power_modes[st->power_mode_idx].mclk_div;
> +	params->step_ps = DIV_ROUND_CLOSEST_ULL((u64)mult * PSEC_PER_SEC *
> +						mclk_div, mclk);
> +
> +	return 0;
> +}

-- 
With Best Regards,
Andy Shevchenko