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

On 8/24/26 09:10, Andy Shevchenko wrote:

On Fri, Aug 21, 2026 at 04:07:01PM +0200, Janani Sunil wrote:

quoted

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.

...

quoted

+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.

I kept the explicit switch case because it the device supports only the 6 discrete decimation ratios listed in the table 32.
The switch directly mirrors the phase resolution, number of steps and register bit mappings from that table, which I find easier to verify against the datasheet.

The arithmetic version is more compact, but the proposed conditions would reject the valid x32 case because ilog2(32) is 5. A corrected arithmetic version would be functionally equivalent, but I do not think it improves the readability here.

Regards,
Jan