Re: [PATCH v5 1/3] hwmon: (ads7871) Fix endianness bug in 16-bit register reads

[Tabrez Ahmed]

On 5/1/26 14:19, David Laight wrote:

> Isn't it enough to just byteswap the result? so:
> 	return le16toh(ret);
> The whole thing can be:
> 	return le16toh(spi_w8r16(spi, reg | INST_READ_BM | INST_16BIT_BM));
> (although I suspect sparse bleats and needs an annoying (__force __le16) cast)

Hi David,

Good point, spi_w8r16() is definitely cleaner and cuts out the buffer 
allocations entirely.

I tried the one-liner approach, but le16toh caused a build error because 
it's not available in the kernel headers. I will swap it for le16_to_cpu 
in the final patch.

I'm going to split the implementation slightly in v6 to catch negative 
error codes before the conversion and to match the bitwise assignment 
style used in the rest of this driver:


static int ads7871_read_reg16(struct spi_device *spi, int reg)
{
	int ret;

	reg = reg | INST_READ_BM | INST_16BIT_BM;
	ret = spi_w8r16(spi, reg);
	if (ret < 0)
		return ret;

	return le16_to_cpu((__force __le16)ret);
}


Guenter this also pulls in your suggestion to combine the bitwise flags.

I'll get v6 out shortly.

Thanks,
Tabrez