Re: [PATCH v3 4/4] iio: cdc: add Texas Instruments FDC1004 driver

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

On Sat, Oct 31, 2026 at 10:22:34AM -0500, Victor Chernyakin wrote:

...

> > > > +     /*
> > > > +		 * Now read the two registers making up the measurement.
> > > > +		 * The datasheet says to read MSB first, LSB second.
> > > > +		 */
> > > > 		// also note in this case we don't need to have _MSB in the name
> > > > 		ret = regmap_bulk_read(map, FDC1004_REG_MEASn(0), reg_val, sizeof(reg_val));
> > > 
> > > I agree we could drop the _MSB if this was the only use
> > > of FDC1004_REG_MEASn_MSB, but we use both _MSB and _LSB macros in the
> > > volatile register table definition, and it would be weird to have one
> > > macro with _LSB but the other without _MSB.
> > 
> > You can have three!
> 
> I suppose, but I would prefer to avoid duplicating the macro, and
> I think having _MSB here helps to confirm what the comment is saying,
> that we should be reading MSB first.
> 
> > > > 		if (ret)
> > > > 			return ret;
> > > > 
> > > > #ifdef __LITTLE_ENDIAN
> > > > 		reg_val = swahw32(reg_val);
> > > > #endif
> > > > 
> > > > 		*val = sign_extend32(reg_val >> 8, 23);
> > > > 
> > > > 		// I assume the following is also possible in this case
> > > > 		// *val = (int)reg_val >> 8;
> > > > 		// but usually we use sign_extend32() in IIO even when simple
> > > > 		// casting is okay
> > > >
> > > > This also reduces stack usage.
> > > 
> > > AFAIK a good optimizer should be able to see that reg_val and reg_vals are
> > > never live at the same time and reuse the same stack slot for both (whether
> > > GCC or Clang actually do that for this function, I can't be bothered to check).
> > > 
> > > If you and Jonathan prefer that version for other reasons though, I can
> > > switch to it.
> > 
> > I gave an idea, I would wait for Jonathan to judge on it.
> 
> Since we're waiting for an answer, I've posted v4 without this change
> so that the other parts of the patch can move forward, but I can
> always add it to v5, etc.

Generally it's not recommended to post without waiting for the resolution.
If you do so, at least make sure you have mentioned the opens.

-- 
With Best Regards,
Andy Shevchenko