Re: [PATCH v2 2/5] iio: adc: add Versal SysMon driver

[Andy Shevchenko]

On Mon, May 04, 2026 at 04:50:05PM +0100, Salih Erim wrote:

> On 5/4/2026 11:18 AM, Andy Shevchenko wrote:
> > On Sat, May 02, 2026 at 12:19:48PM +0100, Salih Erim wrote:

...

> > > +     if (format)
> > > +             mantissa = (int)(s16)mantissa;
> > 
> > Potential user of FIELD_GET_SIGNED(), but for now just use explicit call to
> > sign_extend32().
> 
> Accepted. Will use sign_extend32(mantissa, 15).
> 
> > > +     *val = (mantissa * SYSMON_MILLI) >> exponent;

The idea is to think about this (and other) user and try to come up with the
solution that will not decrease readability and at the same time give more
context. Perhaps you need an additional patch that introduces the multiplier
for millivolts (as it's done for degrees).

...

> > > +/* Signed milli scale (MILLI from linux/units.h is unsigned long) */
> > > +#define SYSMON_MILLI                 1000
> > 
> > I think you want a different approach. I already forgot how it's being used,
> > but there shouldn't be a new (re-)definition just because of this.
> 
> The issue is that MILLI from units.h is 1000UL, which causes unsigned
> promotion when multiplied with signed int values (e.g. negative
> temperatures or two's complement mantissa). There is
> MILLIDEGREE_PER_DEGREE (signed 1000) but using it for voltage
> channel conversions would be semantically wrong. Will either use
> (int)MILLI at the call sites or just use literal 1000 -- happy to
> go with whichever you prefer.

Let's wait for resolution WRT hwmon vs. IIO.


-- 
With Best Regards,
Andy Shevchenko