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

On 4/17/26 21:11, [email protected] wrote:
> Sashiko AI review found 1 potential issue(s):
> - [High] Passing a pointer to an `int` variable (`&reg`) to `spi_write_then_read()` for a 1-byte transmission causes the wrong byte to be sent on Big-Endian architectures.
> --
> 
> commit 4f7407427267b9ca42b213449235bc095cff57b3
> Author: Tabrez Ahmed <[email protected]>
> 
> hwmon: (ads7871) Fix endianness bug in 16-bit register reads
> 
> This commit replaces spi_w8r16() with a manual spi_write_then_read() into
> a byte array to correctly handle 16-bit sensor reads. It then reconstructs
> the integer using get_unaligned_le16() to ensure correct behavior across
> all architectures.
> 
> Closes: https://sashiko.dev/#/patchset/20260329073352.270451-1-tabreztalks%40gmail.com
> 
>> diff --git a/drivers/hwmon/ads7871.c b/drivers/hwmon/ads7871.c
>> index 9bfdf9e6bcd77..9b52aa496d522 100644
>> --- a/drivers/hwmon/ads7871.c
>> +++ b/drivers/hwmon/ads7871.c
> 
> [ ... ]
> 
>> @@ -77,9 +78,14 @@ 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 buf[2];
>> +
>>   	reg = reg | INST_READ_BM | INST_16BIT_BM;
>> -	ret = spi_w8r16(spi, reg);
>> -	return ret;
>> +	ret = spi_write_then_read(spi, &reg, 1, buf, 2);
> 
> Will this transmit the wrong instruction byte on Big-Endian architectures?
> 
> The variable reg is a 32-bit int, and spi_write_then_read() reads
> sequentially from the memory address provided. On Little-Endian
> architectures, the lowest memory address correctly contains the least
> significant byte, which is the intended 8-bit command.
> 
> However, on Big-Endian architectures, the lowest memory address holds the
> most significant byte of the int, which is 0x00. Does this result in 0x00
> being transmitted over the SPI bus instead of the correct register
> instruction?
> 

This is a real problem. Please fix.

Guenter

>> +	if (ret < 0)
>> +		return ret;
>> +
>> +	return get_unaligned_le16(buf);
>>   }
>