Re: [PATCH v3 3/3] hwmon: Driver for Texas Instruments INA238

[Guenter Roeck]

On 10/31/21 7:20 PM, Nathan Rossi wrote:
[ ... ]
> > > +
> > > +     if (attr != hwmon_in_max && attr != hwmon_in_min)
> > > +             return -EOPNOTSUPP;
> > > +
> > > +     /* convert decimal to register value */
> > > +     switch (channel) {
> > > +     case 0:
> > > +             /* signed value, clamp to max range +/-163 mV */
> > > +             regval = clamp_val(val, -163, 163);
> > > +             regval = (regval * 1000L * (4 - (int)data->gain + 1)) /
> >
> > nit: The typecast "(int)" is not needed here.
>
> Due to the unsigned type of gain, it causes promotion of regval (and
> the rest of the numerator) to unsigned long which causes issues with
> negative numbers on the divide. It makes more sense for gain to be an
> int to begin with, I will change it to int to avoid the need for type
> casting.

Are you sure ? I initially thought that as well and wrote a little test
program with that expression, but it didn't do the promotion to unsigned.

Thanks,
Guenter