Re: [PATCH v3 7/8] iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel

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

On Fri, Jul 10, 2026 at 05:50:40PM -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.

...

> +/*
> + * Available gains as tenths (e.g. value 5 == 0.5 gain). Indexes correspond to
> + * ADS112C14_GAIN_CFG_GAIN values.
> + */
> +static const u32 ads112c14_pga_gains_x10[] = {
> +	5, 10, 20, 40, 50, 80, 100, 160,
> +	200, 320, 500, 640, 1000, 1280, 2000, 2560,

Since the line lengths are quite different, I would add tail comments with
indices.

	5, 10, 20, 40, 50, 80, 100, 160,		/* 0 -  7 */
	200, 320, 500, 640, 1000, 1280, 2000, 2560,	/* 8 - 15 */

(or in hexadecimal, depending on the datasheet).

> +};

...

>  static int ads112c14_prepare_sys_mon_channel(struct ads112c14_data *data,
>  					     const struct iio_chan_spec *chan)
>  {
> +	u32 gain_val;
>  	int ret;
>  
> -	/* TODO: GAIN is useful for shorted PGA inputs. */
> -	/* All SYS_MON channels use GAIN of 1 to keep it simple. */
> +	/*
> +	 * All SYS_MON channels use GAIN of 1 to keep it simple. Other than
> +	 * the internal short channel, where it is useful in practice.
> +	 */
> +	gain_val = chan->channel == ADS112C14_SYS_MON_CHANNEL_SHORT ?
> +		   data->sys_mon_chan_short_gain_val : 1;

Hmm... What about

	if (chan->channel == ADS112C14_SYS_MON_CHANNEL_SHORT)
		gain_val = FIELD_PREP(ADS112C14_GAIN_CFG_GAIN, data->sys_mon_chan_short_gain_val);
	else
		gain_val = FIELD_PREP_CONST(ADS112C14_GAIN_CFG_GAIN, 1);

>  	ret = regmap_update_bits(data->regmap, ADS112C14_REG_GAIN_CFG,
>  				 ADS112C14_GAIN_CFG_SYS_MON |
>  				 ADS112C14_GAIN_CFG_GAIN,
>  				 FIELD_PREP(ADS112C14_GAIN_CFG_SYS_MON, chan->address) |
> -				 FIELD_PREP(ADS112C14_GAIN_CFG_GAIN, 1));
> +				 FIELD_PREP(ADS112C14_GAIN_CFG_GAIN, gain_val));

				 gain_val);

On the second thought this might require to have a separate mon_val to make
this consistent. Up to you.

>  	if (ret)
>  		return ret;

-- 
With Best Regards,
Andy Shevchenko