Replace the manual sign manipulation with sign_extend32() and change the
affected variable from u16 to s32 to properly handle negative values.

Resolve a logic error where the sign bit was being checked at bit 10
instead of bit 9 for a 10-bit value.

Convert the data variable to be in centidegrees celsius (0.25 C per bit)
so we can use simple division and modulo for sysfs_emit() instead of the
convoluted bit shifting and masking.

Signed-off-by: Michael Harris <michaelharriscode@gmail.com>
---
 drivers/staging/iio/addac/adt7316.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/iio/addac/adt7316.c b/drivers/staging/iio/addac/adt7316.c
index 8a9a8262c2bec34f3c3e79d8174f492b9a23fb70..1412808c50c76a68b5771a25c46dd3308c5cbcdb 100644
--- a/drivers/staging/iio/addac/adt7316.c
+++ b/drivers/staging/iio/addac/adt7316.c
@@ -155,9 +155,12 @@
  * ADT7316 value masks
  */
 #define ADT7316_VALUE_MASK		0xfff
-#define ADT7316_T_VALUE_SIGN		0x400
 #define ADT7316_T_VALUE_FLOAT_OFFSET	2
-#define ADT7316_T_VALUE_FLOAT_MASK	0x2
+
+/*
+ * ADT7316 hardware constants
+ */
+#define ADT7316_TEMP_CENTIDEG_PER_BIT	25
 
 /*
  * Chip ID
@@ -1090,9 +1093,8 @@ static IIO_DEVICE_ATTR(DAC_internal_Vref, 0644,
 static ssize_t adt7316_show_ad(struct adt7316_chip_info *chip,
 			       int channel, char *buf)
 {
-	u16 data;
+	s32 data;
 	u8 msb, lsb;
-	char sign = ' ';
 	int ret;
 
 	if ((chip->config2 & ADT7316_AD_SINGLE_CH_MODE) &&
@@ -1151,15 +1153,13 @@ static ssize_t adt7316_show_ad(struct adt7316_chip_info *chip,
 		break;
 	}
 
-	if (data & ADT7316_T_VALUE_SIGN) {
-		/* convert supplement to positive value */
-		data = (ADT7316_T_VALUE_SIGN << 1) - data;
-		sign = '-';
-	}
+	data = sign_extend32(data, 9);
+	data *= ADT7316_TEMP_CENTIDEG_PER_BIT;
 
-	return sysfs_emit(buf, "%c%d.%.2d\n", sign,
-		(data >> ADT7316_T_VALUE_FLOAT_OFFSET),
-		(data & ADT7316_T_VALUE_FLOAT_MASK) * 25);
+	return sysfs_emit(buf, "%s%d.%02u\n",
+			  (data < 0 ? "-" : ""),
+			  abs(data / 100),
+			  abs(data % 100));
 }
 
 static ssize_t adt7316_show_VDD(struct device *dev,

-- 
2.53.0

On Thu, 05 Mar 2026 23:16:58 -0800
Michael Harris <michaelharriscode@gmail.com> wrote:

> Replace the manual sign manipulation with sign_extend32() and change the
> affected variable from u16 to s32 to properly handle negative values.
> 
> Resolve a logic error where the sign bit was being checked at bit 10
> instead of bit 9 for a 10-bit value.
> 
> Convert the data variable to be in centidegrees celsius (0.25 C per bit)
> so we can use simple division and modulo for sysfs_emit() instead of the
> convoluted bit shifting and masking.
> 
> Signed-off-by: Michael Harris <michaelharriscode@gmail.com>

As Andy noted needs a fixes tag.

> ---
>  drivers/staging/iio/addac/adt7316.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/iio/addac/adt7316.c b/drivers/staging/iio/addac/adt7316.c
> index 8a9a8262c2bec34f3c3e79d8174f492b9a23fb70..1412808c50c76a68b5771a25c46dd3308c5cbcdb 100644
> --- a/drivers/staging/iio/addac/adt7316.c
> +++ b/drivers/staging/iio/addac/adt7316.c
> @@ -155,9 +155,12 @@
>   * ADT7316 value masks
>   */
>  #define ADT7316_VALUE_MASK		0xfff
> -#define ADT7316_T_VALUE_SIGN		0x400
>  #define ADT7316_T_VALUE_FLOAT_OFFSET	2
> -#define ADT7316_T_VALUE_FLOAT_MASK	0x2
> +
> +/*
> + * ADT7316 hardware constants
> + */
> +#define ADT7316_TEMP_CENTIDEG_PER_BIT	25

As it's only used in one place, I'd think just use the numeric value down there
and if you feel it needs more background, add a comment there.

On Thu, Mar 05, 2026 at 11:16:58PM -0800, Michael Harris wrote:
> Replace the manual sign manipulation with sign_extend32() and change the
> affected variable from u16 to s32 to properly handle negative values.
> 
> Resolve a logic error where the sign bit was being checked at bit 10
> instead of bit 9 for a 10-bit value.
> 
> Convert the data variable to be in centidegrees celsius (0.25 C per bit)
> so we can use simple division and modulo for sysfs_emit() instead of the
> convoluted bit shifting and masking.

Sounds like a candidate for Fixes tag.

...

> -	return sysfs_emit(buf, "%c%d.%.2d\n", sign,
> -		(data >> ADT7316_T_VALUE_FLOAT_OFFSET),
> -		(data & ADT7316_T_VALUE_FLOAT_MASK) * 25);
> +	return sysfs_emit(buf, "%s%d.%02u\n",
> +			  (data < 0 ? "-" : ""),
> +			  abs(data / 100),
> +			  abs(data % 100));

Hmm... If you create a temporary variable for abs(data), this might give
slightly different code generation, presumably better on (some) architectures.
/ % combined maybe a single assembly instruction.

Or keep sign in a char as it was in the original code and just move data to be
abs(data).

-- 
With Best Regards,
Andy Shevchenko