[PATCH v5 0/3] hwmon: (ads7871) Fix endianness and modernize driver

[PATCH v5 0/3] hwmon: (ads7871) Fix endianness and modernize driver

[Tabrez Ahmed]

This series addresses several issues in the ads7871 driver. It fixes an
architecture-dependent endianness bug in the 16-bit register read logic,
migrates the driver to the modern hwmon_device_register_with_info() API,
and moves the SPI transfer buffer into the driver's private data structure
to ensure DMA safety.

Note: I do not have access to the physical ADS7871 hardware. This
series has been compile-tested only.

Changes in v5:
- Fixed a bisectability issue where the <linux/unaligned.h> include 
  modernization was accidentally squashed into Patch 3 instead of Patch 1.
- Ensured the 1-byte SPI command write in ads7871_read_reg16() uses a 
  strictly typed 'u8' variable to prevent a Big-Endian pointer hazard.

Changes in v4:
- Patch 1: Refactored 16-bit register reads to use a dedicated 'u8' 
  transmit variable, eliminating a 32-bit pointer endianness hazard.

Changes in v3:
- Added Patch 1 to fix the pre-existing endianness bug in 16-bit reads.
- Fixed multiple formatting and alignment issues caught by checkpatch
  --strict, as requested by Guenter Roeck.
- Added "While at it, fix checkpatch violations" to Patch 2 commit message.

Tabrez Ahmed (3):
  hwmon: (ads7871) Fix endianness bug in 16-bit register reads
  hwmon: (ads7871) Convert to hwmon_device_register_with_info
  hwmon: (ads7871) Use DMA-safe buffer for SPI writes

 drivers/hwmon/ads7871.c | 122 +++++++++++++++++++++++-----------------
 1 file changed, 71 insertions(+), 51 deletions(-)

-- 
2.43.0

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

[Tabrez Ahmed]

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;
+	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)
-- 
2.43.0

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

[Guenter Roeck]

On 4/30/26 19:35, Tabrez Ahmed 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;
> +	tx_cmd = reg;

This can be combined into

	tx_cmd = reg | INST_READ_BM | INST_16BIT_BM;

Thanks,
Guenter

> +
> +	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)

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

[David Laight]

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)

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

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

[Guenter Roeck]

On 5/1/26 01:49, David Laight wrote:
> 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));

This would convert error codes to some random value.

Guenter

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

[Guenter Roeck]

On Fri, May 01, 2026 at 08:05:28AM +0530, Tabrez Ahmed 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>

Applied.

Thanks,
Guenter

[PATCH v5 2/3] hwmon: (ads7871) Convert to hwmon_device_register_with_info

[Tabrez Ahmed]

Convert the ads7871 driver from the legacy hwmon_device_register() to the
modern hwmon_device_register_with_info() API. This migration simplifies
the driver by using the structured hwmon_channel_info approach and
prepares the codebase for the transition to a shared DMA-safe buffer.
While at it, fix checkpatch violations.

Signed-off-by: Tabrez Ahmed <tabreztalks@gmail.com>
---
 drivers/hwmon/ads7871.c | 75 ++++++++++++++++++++++-------------------
 1 file changed, 40 insertions(+), 35 deletions(-)

diff --git a/drivers/hwmon/ads7871.c b/drivers/hwmon/ads7871.c
index d77eff430935b..75485a2c16b0b 100644
--- a/drivers/hwmon/ads7871.c
+++ b/drivers/hwmon/ads7871.c
@@ -56,7 +56,6 @@
 #include <linux/init.h>
 #include <linux/spi/spi.h>
 #include <linux/hwmon.h>
-#include <linux/hwmon-sysfs.h>
 #include <linux/err.h>
 #include <linux/delay.h>
 #include <linux/unaligned.h>
@@ -66,6 +65,16 @@ struct ads7871_data {
 	struct spi_device *spi;
 };
 
+static umode_t ads7871_is_visible(const void *data,
+				  enum hwmon_sensor_types type,
+				  u32 attr, int channel)
+{
+	if (type == hwmon_in && attr == hwmon_in_input)
+		return 0444;
+
+	return 0;
+}
+
 static int ads7871_read_reg8(struct spi_device *spi, int reg)
 {
 	int ret;
@@ -93,19 +102,20 @@ static int ads7871_read_reg16(struct spi_device *spi, int reg)
 static int ads7871_write_reg8(struct spi_device *spi, int reg, u8 val)
 {
 	u8 tmp[2] = {reg, val};
+
 	return spi_write(spi, tmp, sizeof(tmp));
 }
 
-static ssize_t voltage_show(struct device *dev, struct device_attribute *da,
-			    char *buf)
+static int ads7871_read(struct device *dev, enum hwmon_sensor_types type,
+			u32 attr, int channel, long *val)
 {
 	struct ads7871_data *pdata = dev_get_drvdata(dev);
 	struct spi_device *spi = pdata->spi;
-	struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
-	int ret, val, i = 0;
-	uint8_t channel, mux_cnv;
+	int ret, raw_val, i = 0;
+	u8 mux_cnv;
 
-	channel = attr->index;
+	if (type != hwmon_in || attr != hwmon_in_input)
+		return -EOPNOTSUPP;
 	/*
 	 * TODO: add support for conversions
 	 * other than single ended with a gain of 1
@@ -135,39 +145,34 @@ static ssize_t voltage_show(struct device *dev, struct device_attribute *da,
 	}
 
 	if (mux_cnv == 0) {
-		val = ads7871_read_reg16(spi, REG_LS_BYTE);
-		if (val < 0)
-			return val;
+		raw_val = ads7871_read_reg16(spi, REG_LS_BYTE);
+		if (raw_val < 0)
+			return raw_val;
+
 		/*result in volts*10000 = (val/8192)*2.5*10000*/
-		val = ((val >> 2) * 25000) / 8192;
-		return sysfs_emit(buf, "%d\n", val);
+		*val = ((raw_val >> 2) * 25000) / 8192;
+		return 0;
 	}
 
 	return -ETIMEDOUT;
 }
 
-static SENSOR_DEVICE_ATTR_RO(in0_input, voltage, 0);
-static SENSOR_DEVICE_ATTR_RO(in1_input, voltage, 1);
-static SENSOR_DEVICE_ATTR_RO(in2_input, voltage, 2);
-static SENSOR_DEVICE_ATTR_RO(in3_input, voltage, 3);
-static SENSOR_DEVICE_ATTR_RO(in4_input, voltage, 4);
-static SENSOR_DEVICE_ATTR_RO(in5_input, voltage, 5);
-static SENSOR_DEVICE_ATTR_RO(in6_input, voltage, 6);
-static SENSOR_DEVICE_ATTR_RO(in7_input, voltage, 7);
-
-static struct attribute *ads7871_attrs[] = {
-	&sensor_dev_attr_in0_input.dev_attr.attr,
-	&sensor_dev_attr_in1_input.dev_attr.attr,
-	&sensor_dev_attr_in2_input.dev_attr.attr,
-	&sensor_dev_attr_in3_input.dev_attr.attr,
-	&sensor_dev_attr_in4_input.dev_attr.attr,
-	&sensor_dev_attr_in5_input.dev_attr.attr,
-	&sensor_dev_attr_in6_input.dev_attr.attr,
-	&sensor_dev_attr_in7_input.dev_attr.attr,
+static const struct hwmon_channel_info * const ads7871_info[] = {
+	HWMON_CHANNEL_INFO(in,
+			   HWMON_I_INPUT, HWMON_I_INPUT, HWMON_I_INPUT, HWMON_I_INPUT,
+			   HWMON_I_INPUT, HWMON_I_INPUT, HWMON_I_INPUT, HWMON_I_INPUT),
 	NULL
 };
 
-ATTRIBUTE_GROUPS(ads7871);
+static const struct hwmon_ops ads7871_hwmon_ops = {
+	.is_visible = ads7871_is_visible,
+	.read = ads7871_read,
+};
+
+static const struct hwmon_chip_info ads7871_chip_info = {
+	.ops = &ads7871_hwmon_ops,
+	.info = ads7871_info,
+};
 
 static int ads7871_probe(struct spi_device *spi)
 {
@@ -202,10 +207,10 @@ static int ads7871_probe(struct spi_device *spi)
 		return -ENOMEM;
 
 	pdata->spi = spi;
-
-	hwmon_dev = devm_hwmon_device_register_with_groups(dev, spi->modalias,
-							   pdata,
-							   ads7871_groups);
+	hwmon_dev = devm_hwmon_device_register_with_info(dev, spi->modalias,
+							 pdata,
+							 &ads7871_chip_info,
+							 NULL);
 	return PTR_ERR_OR_ZERO(hwmon_dev);
 }
 
-- 
2.43.0

[PATCH v5 3/3] hwmon: (ads7871) Use DMA-safe buffer for SPI writes

[Tabrez Ahmed]

The driver currently passes a stack-allocated buffer to spi_write(),
which is incompatible with DMA on systems with CONFIG_VMAP_STACK
enabled.

Move the transfer buffer into the driver's private data structure
to ensure it is DMA-safe. Since this shared buffer now requires
serialization, this change depends on the previous commit which
migrated the driver to the hwmon 'with_info' API.

While moving the logic, also:
- Corrected the sign extension for 14-bit data by casting to s16.
- Scaled the output to millivolts (2500mV full scale
) to comply with the hwmon ABI.

Signed-off-by: Tabrez Ahmed <tabreztalks@gmail.com>
---
 drivers/hwmon/ads7871.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/hwmon/ads7871.c b/drivers/hwmon/ads7871.c
index 75485a2c16b0b..cbb9a88229c50 100644
--- a/drivers/hwmon/ads7871.c
+++ b/drivers/hwmon/ads7871.c
@@ -63,6 +63,7 @@
 #define DEVICE_NAME	"ads7871"
 struct ads7871_data {
 	struct spi_device *spi;
+	u8 tx_buf[2] ____cacheline_aligned;
 };
 
 static umode_t ads7871_is_visible(const void *data,
@@ -99,11 +100,12 @@ static int ads7871_read_reg16(struct spi_device *spi, int reg)
 	return get_unaligned_le16(rx_buf);
 }
 
-static int ads7871_write_reg8(struct spi_device *spi, int reg, u8 val)
+static int ads7871_write_reg8(struct ads7871_data *pdata, int reg, u8 val)
 {
-	u8 tmp[2] = {reg, val};
+	pdata->tx_buf[0] = reg;
+	pdata->tx_buf[1] = val;
 
-	return spi_write(spi, tmp, sizeof(tmp));
+	return spi_write(pdata->spi, pdata->tx_buf, 2);
 }
 
 static int ads7871_read(struct device *dev, enum hwmon_sensor_types type,
@@ -122,7 +124,7 @@ static int ads7871_read(struct device *dev, enum hwmon_sensor_types type,
 	 */
 	/*MUX_M3_BM forces single ended*/
 	/*This is also where the gain of the PGA would be set*/
-	ret = ads7871_write_reg8(spi, REG_GAIN_MUX,
+	ret = ads7871_write_reg8(pdata, REG_GAIN_MUX,
 				 (MUX_CNV_BM | MUX_M3_BM | channel));
 	if (ret < 0)
 		return ret;
@@ -130,6 +132,7 @@ static int ads7871_read(struct device *dev, enum hwmon_sensor_types type,
 	ret = ads7871_read_reg8(spi, REG_GAIN_MUX);
 	if (ret < 0)
 		return ret;
+
 	mux_cnv = ((ret & MUX_CNV_BM) >> MUX_CNV_BV);
 	/*
 	 * on 400MHz arm9 platform the conversion
@@ -149,8 +152,11 @@ static int ads7871_read(struct device *dev, enum hwmon_sensor_types type,
 		if (raw_val < 0)
 			return raw_val;
 
-		/*result in volts*10000 = (val/8192)*2.5*10000*/
-		*val = ((raw_val >> 2) * 25000) / 8192;
+		/*
+		 * Use (s16) to ensure the sign bit is preserved during the shift.
+		 * Report millivolts (2.5V = 2500mV).
+		 */
+		*val = ((s16)raw_val >> 2) * 2500 / 8192;
 		return 0;
 	}
 
@@ -187,11 +193,17 @@ static int ads7871_probe(struct spi_device *spi)
 	spi->bits_per_word = 8;
 	spi_setup(spi);
 
-	ads7871_write_reg8(spi, REG_SER_CONTROL, 0);
-	ads7871_write_reg8(spi, REG_AD_CONTROL, 0);
+	pdata = devm_kzalloc(dev, sizeof(*pdata), GFP_KERNEL);
+	if (!pdata)
+		return -ENOMEM;
+
+	pdata->spi = spi;
+
+	ads7871_write_reg8(pdata, REG_SER_CONTROL, 0);
+	ads7871_write_reg8(pdata, REG_AD_CONTROL, 0);
 
 	val = (OSC_OSCR_BM | OSC_OSCE_BM | OSC_REFE_BM | OSC_BUFE_BM);
-	ads7871_write_reg8(spi, REG_OSC_CONTROL, val);
+	ads7871_write_reg8(pdata, REG_OSC_CONTROL, val);
 	ret = ads7871_read_reg8(spi, REG_OSC_CONTROL);
 
 	dev_dbg(dev, "REG_OSC_CONTROL write:%x, read:%x\n", val, ret);
@@ -202,11 +214,6 @@ static int ads7871_probe(struct spi_device *spi)
 	if (val != ret)
 		return -ENODEV;
 
-	pdata = devm_kzalloc(dev, sizeof(struct ads7871_data), GFP_KERNEL);
-	if (!pdata)
-		return -ENOMEM;
-
-	pdata->spi = spi;
 	hwmon_dev = devm_hwmon_device_register_with_info(dev, spi->modalias,
 							 pdata,
 							 &ads7871_chip_info,
-- 
2.43.0