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

[David Laight <david.laight.linux@gmail.com>]

On Fri,  1 May 2026 08:05:28 +0530
Tabrez Ahmed <tabreztalks@gmail.com> wrote:

> The ads7871_read_reg16() function relies on spi_w8r16() to read the
> 16-bit sensor output. The ADS7871 device transmits the Least Significant
> Byte (LSB) first.
> 
> On Little-Endian architectures, spi_w8r16() correctly reconstructs the
> 16-bit value. However, on Big-Endian architectures, the byte swapping
> causes the first received byte (LSB) to be placed in the most significant
> byte of the u16, resulting in corrupted voltage readings.
> 
> Replace spi_w8r16() with a manual spi_write_then_read() into a byte array,
> and safely reconstruct the integer using get_unaligned_le16() to ensure
> correct behavior across all architectures. Additionally, use a u8
> variable for the command byte to ensure the correct instruction is
> transmitted on Big-Endian systems.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/20260418034601.90226-1-tabreztalks@gmail.com
> Signed-off-by: Tabrez Ahmed <tabreztalks@gmail.com>
> ---
>  drivers/hwmon/ads7871.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/ads7871.c b/drivers/hwmon/ads7871.c
> index 9bfdf9e6bcd77..d77eff430935b 100644
> --- a/drivers/hwmon/ads7871.c
> +++ b/drivers/hwmon/ads7871.c
> @@ -59,9 +59,9 @@
>  #include <linux/hwmon-sysfs.h>
>  #include <linux/err.h>
>  #include <linux/delay.h>
> +#include <linux/unaligned.h>
>  
>  #define DEVICE_NAME	"ads7871"
> -
>  struct ads7871_data {
>  	struct spi_device *spi;
>  };
> @@ -77,9 +77,17 @@ static int ads7871_read_reg8(struct spi_device *spi, int reg)
>  static int ads7871_read_reg16(struct spi_device *spi, int reg)
>  {
>  	int ret;
> +	u8 tx_cmd;
> +	u8 rx_buf[2];
> +
>  	reg = reg | INST_READ_BM | INST_16BIT_BM;
> -	ret = spi_w8r16(spi, reg);
> -	return ret;

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)

-- David



> +	tx_cmd = reg;
> +
> +	ret = spi_write_then_read(spi, &tx_cmd, 1, rx_buf, 2);
> +	if (ret < 0)
> +		return ret;
> +
> +	return get_unaligned_le16(rx_buf);
>  }
>  
>  static int ads7871_write_reg8(struct spi_device *spi, int reg, u8 val)