Re: [PATCH v2 3/3] iio: cdc: add Texas Instruments FDC1004 driver

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

On Mon, Oct 26, 2026 at 12:06:36PM -0500, Victor Chernyakin wrote:

> > > regmap gives us the two 16-bit registers in native CPU endianness.
> > 
> > It's only if you specify endianess for the whole communication of regmap.
> > I'm too lazy now to check the original patch, but if it's not specified,
> > it means the device native endianess, whatever it is on per register basis.
> 
> We do specify it, yes (the chip is big-endian).
> 
> > > If the CPU is little endian, that means the overall 32 bits aren't in
> > > __be32 format (because the two bytes in each of the halves are swapped).
> > 
> > But yes, even if you do it in the device native endianess, it will still be
> > a pair of 16-bit values with the halves being swapped in case of treating
> > them as a single 32-bit value. This is unfortunate... So, basically this
> > needs to be elaborated in the comment above (where you refer datasheet).
> 
> Can you clarify what you want elaborated? (or are you saying, *if* we
> were working in the device's native endianness, *then* we would have to
> explain that in the comment?)

In the comment you can add something like "We may not read these
two registers in a bulk transfer because they are 16-bit wide and
the byte order in the compound 32-bit value will be wrong."

Yeah, we can address that by swapping 16-bit parts, but it might
make code worse to read (and possibly affect generation as well).
(FWIW, we have even swahw32() for that.)

...

> > What I meant is to compare decomposed val64 as well.
> 
> Hmm, if we only worked with the decomposed values, I think we would have to
> write something like this, which is probably even worse...
> 
>     if ((val < fdc1004_calibscale[0][0]) || 
>         (val == fdc1004_calibscale[0][0] && val2 < fdc1004_calibscale[0][1]) ||
>         (val > fdc1004_calibscale[2][0]) || 
>         (val == fdc1004_calibscale[2][0] && val2 > fdc1004_calibscale[2][1]))
>         return -EINVAL;

It can be avoided by using a helper to validate. Also the meaning of the outer
and inner indices is clearer. With one-dimension array it's unclear what those
indices mean. Ideally the outer indices should match something with datasheet
and in some cases can even be named.

-- 
With Best Regards,
Andy Shevchenko