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

["Nuno Sá" <noname.nuno@gmail.com>]

On Tue, Jun 16, 2026 at 10:58:46AM +0300, Andy Shevchenko wrote:
> 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.
> 

...

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

Did not looked enough time but also looks like maybe mul_u64_u32_shr()
could be used? And given that it handles things nicely (even uses 128bit
when possible), might open the door for more simplifications.

Anyways, agree with Andy that in the current patch it's a bit of a pain
to the eyes :).

- Nuno Sá

> 
> 		scale_avail[0] = div_u64_rem(foo, NANO, &scale_avail[1]);
> 
> > +	}
> > +}
> 
> -- 
> With Best Regards,
> Andy Shevchenko
> 
>