Re: [PATCH 1/2] iio: light: adux1020: fix interrupt clear value on SMBus-only adapters

[Andy Shevchenko]

On Tue, Nov 10, 2026 at 08:12:06AM +0000, Raffaele Tranfaglia wrote:
> On Mon, 9 Nov 2026 at 23:49, Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxx> wrote:
> > On Mon, Nov 09, 2026 at 11:39:02AM +0000, Raffaele Tranfaglia via B4 Relay wrote:

...

> > > adux1020_measure() clears the mode interrupt by writing the inverted
> > > interrupt enable mask to INT_STATUS. The mask is a u16, but integer
> > > promotion makes ~0x00f0 evaluate to 0xffffff0f, which does not fit in
> > > the 16-bit register.
> >
> > Huh?! Where did you get it from? the mentioned promotion may only happen for
> > a signed char which may not hold 0x00f0 in the first place. The above is some
> > nonsense.
> 
> The integer promotions apply to all types with a rank lower than int
> (e.g. char and short, signed or unsigned), as long as int can represent
> all their values (C11 6.3.1.1p2 [1]). For ~ the standard says "The
> integer promotions are performed on the operand" (6.5.3.3p4 [2]). So the
> u16 0x00f0 becomes the int 0x000000f0, and ~ yields 0xffffff0f, which
> regmap_write() receives as an unsigned int. sizeof(~v) is 4 for a u16 v.

Indeed, I missed that u16 --> int, which is signed. Oh, well...

> It is also what I observed: with i2c-stub (SMBus only), every read of
> in_proximity_raw fails with -EINVAL from the val > 0xffff check in
> regmap_smbus_word_write_swapped(), and succeeds with this patch.

Can you give a Kunit test case? Because regmap specifies the val_bits == 16.
See also [3].

> That said, I can reword the commit message to make this clearer.
> 
> [1] https://port70.net/~nsz/c/c11/n1570.html#6.3.1.1p2
> [2] https://port70.net/~nsz/c/c11/n1570.html#6.5.3.3p4

[3]: https://lore.kernel.org/linux-iio/avLpEA6CoZvicvDN@ashevche-desk.local/

> > >       ret = regmap_write(data->regmap, ADUX1020_REG_INT_STATUS,
> > > -                        (~adux1020_modes[mode].int_en));
> > > +                        ~adux1020_modes[mode].int_en & 0xffff);
> >
> > On top of that regmap should limit the value to 16-bit anyway. If there is a
> > problem in regmap to cut the upper bits, it should be addressed rather there.
> 
> Currently regmap is inconsistent here: the SMBus backends in
> regmap-i2c.c (and regmap-fsi.c) reject out-of-range values with -EINVAL,
> while the formatted plain I2C path silently truncates them. If you
> think regmap_write() is the right place to mask the value to val_bits,
> I can try to fix it there as a separate patch to regmap.

I believe this is due to some specific value formatters in the regmap, where
format_write is defined (!= NULL) or other similar cases.

> However, even if regmap changed, the driver would still be passing a
> 32-bit value for a 16-bit register.

Is it a problem?

> regmap_write() takes an unsigned int, but logically it accepts values of the
> register width, so I think the driver should be fixed in any case. From a
> quick search, this is also what other drivers do when writing an inverted
> value to a narrower register, e.g. ad4691 uses ~BIT(chan->channel) &
> GENMASK(15, 0) and zpa2326 uses (u8)~ZPA2326_CTRL_REG1_MASK_DATA_READY.
> Fixing the driver is also the change that can be backported cleanly through
> the Fixes: tag.

Well, I disagree based on the above analysis. But if you make a KUnit test case
(against regmap) to prove your point, it will be well appreciated!

-- 
With Best Regards,
Andy Shevchenko