Re: [PATCH 3/4] iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel

[Andy Shevchenko]

On Mon, Jun 15, 2026 at 05:00:01PM -0500, David Lechner (TI) wrote:
> Implement support for the programmable gain amplifier on the internal
> short SYS_MON channel. This channel is used for calibration, so it is
> useful to be able to set the PGA to the same gain as the external
> channels. The gain setting is implemented via the `_scale` attribute.
> 
> In the future, we may want to support different reference voltages for
> this channel, so the scale_available table is populated during probe
> rather than being a static table.

...

> +	switch (chan->channel) {
> +	case ADS112C14_SYS_MON_CHANNEL_SHORT: {
> +		IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> +		if (IIO_DEV_ACQUIRE_FAILED(claim))
> +			return -EBUSY;
> +
> +		for (u32 i = 0; i < ARRAY_SIZE(data->sys_mon_chan_short_scale_available); i++) {

u32 here looks confusing. the entire loop can be made readable with a temporary
for the array item I believe.


> +			if (val == data->sys_mon_chan_short_scale_available[i][0] &&
> +			    val2 == data->sys_mon_chan_short_scale_available[i][1]) {
> +				data->sys_mon_chan_short_gain_val = i;
> +				return 0;
> +			}
> +		}

		for (size_t i = 0; i < ARRAY_SIZE(data->sys_mon_chan_short_scale_available); i++) {
			const int *sa = data->sys_mon_chan_short_scale_available[i];

			if (val == sa[0] && val2 == sa[1]) {
				data->sys_mon_chan_short_gain_val = i;
				return 0;
			}
		}

> +		return -EINVAL;
> +	}
> +	default:
> +		return -EINVAL;
> +	}
> +}

...

> +static void ads112c14_populate_tables(struct ads112c14_data *data)
> +{
> +	u32 vref_uV, fsr_bits;
> +	int i;

size_t? unsigned int?

> +	/* For now, assuming we are using 2.5V reference. */
> +	vref_uV = ads112c14_internal_ref_uV[ADS112C14_REFERENCE_CFG_REF_VAL_2_5V];
> +	fsr_bits = data->chip_info->resolution_bits - 1;
> +
> +	for (i = 0; i < ARRAY_SIZE(ads112c14_pga_gains_x10); i++) {
> +		int *scale_avail = &data->sys_mon_chan_short_scale_available[i][0];
> +		u32 gain_x10 = ads112c14_pga_gains_x10[i];
> +
> +		/* NB: slightly odd arrangement to avoid overflow. */
> +		scale_avail[0] = div_u64_rem(div_u64((u64)NANO * 10 /
> +						     (MICRO / MILLI) * vref_uV /
> +						     gain_x10,
> +						     BIT(fsr_bits)),
> +					     NANO, &scale_avail[1]);

Oh, what about temporary variable for the inner division? Also note one trick
to avoid casting (and making it shorter).

		u64 foo;

		foo = div_u64(10ULL * NANO / (MICRO / MILLI) * vref_uV / gain_x10, BIT(fsr_bits));

		/* NB: slightly odd arrangement to avoid overflow. */
		scale_avail[0] = div_u64_rem(foo, NANO, &scale_avail[1]);

Now, with much more readability, it's visible that the first division is just a right shift.

		u64 foo;

		/* ...a comment to explain voodoo calculations... */
		foo = (10ULL * NANO / (MICRO / MILLI) * vref_uV / gain_x10) >> fsr_bits;

		scale_avail[0] = div_u64_rem(foo, NANO, &scale_avail[1]);

> +	}
> +}

-- 
With Best Regards,
Andy Shevchenko