Skip to contentSkip to conversation

iio/hid-sensors: Fix IIO_CHAN_INFO_RAW returning wrong values for signed numbers

PATCH In review Hans de Goede 5 participants 8 messages Oct 31, 2018 – Nov 16, 2018

Subsystems (4)
HID IIO Real time clock (RTC) HID sensor hub drivers
lore
Share & tools
lore

Hans de Goede

author
#0 lore raw

PATCH v2 1/1 iio/hid-sensors: Fix IIO_CHAN_INFO_RAW returning wrong values for signed numbers

Before this commit sensor_hub_input_attr_get_raw_value() failed to take the signedness of 16 and 8 bit values into account, returning e.g. 65436 instead of -100 for the z-axis reading of an accelerometer.

This commit adds a new is_signed parameter to the function and makes all callers pass the appropriate value for this.

While at it, this commit also fixes up some neighboring lines where statements were needlessly split over 2 lines to improve readability.

Acked-by Srinivas Pandruvada <[email protected]>
Signed-off-by Hans de Goede <[email protected]>

---
Changes in v2:
-Adjust the kernel doc for the new is_signed parameter to
sensor_hub_input_attr_get_raw_value()
-Add Srinivas' Acked-by

---
 drivers/hid/hid-sensor-custom.c                  |  2 +-
 drivers/hid/hid-sensor-hub.c                     | 13 ++++++++++---
 drivers/iio/accel/hid-sensor-accel-3d.c          |  5 ++++-
 drivers/iio/gyro/hid-sensor-gyro-3d.c            |  5 ++++-
 drivers/iio/humidity/hid-sensor-humidity.c       |  3 ++-
 drivers/iio/light/hid-sensor-als.c               |  8 +++++---
 drivers/iio/light/hid-sensor-prox.c              |  8 +++++---
 drivers/iio/magnetometer/hid-sensor-magn-3d.c    |  8 +++++---
 drivers/iio/orientation/hid-sensor-incl-3d.c     |  8 +++++---
 drivers/iio/pressure/hid-sensor-press.c          |  8 +++++---
 drivers/iio/temperature/hid-sensor-temperature.c |  3 ++-
 drivers/rtc/rtc-hid-sensor-time.c                |  2 +-
 include/linux/hid-sensor-hub.h                   |  4 +++-
 13 files changed, 52 insertions(+), 25 deletions(-)
drivers/hid/hid-sensor-custom.c +1 -1
index e8a114157f87..bb012bc032e0 100644
@@ -358,7 +358,7 @@ static ssize_t show_value(struct device *dev, struct device_attribute *attr,
 						sensor_inst->hsdev,
 						sensor_inst->hsdev->usage,
 						usage, report_id,
-						SENSOR_HUB_SYNC);
+						SENSOR_HUB_SYNC, false);
 	} else if (!strncmp(name, "units", strlen("units")))
 		value = sensor_inst->fields[field_index].attribute.units;
 	else if (!strncmp(name, "unit-expo", strlen("unit-expo")))
drivers/hid/hid-sensor-hub.c +10 -3
index 2b63487057c2..4256fdc5cd6d 100644
@@ -299,7 +299,8 @@ EXPORT_SYMBOL_GPL(sensor_hub_get_feature);
 int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
 					u32 usage_id,
 					u32 attr_usage_id, u32 report_id,
-					enum sensor_hub_read_flags flag)
+					enum sensor_hub_read_flags flag,
+					bool is_signed)
 {
 	struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
 	unsigned long flags;

@@ -331,10 +332,16 @@ int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
 						&hsdev->pending.ready, HZ*5);
 		switch (hsdev->pending.raw_size) {
 		case 1:
-			ret_val = *(u8 *)hsdev->pending.raw_data;
+			if (is_signed)
+				ret_val = *(s8 *)hsdev->pending.raw_data;
+			else
+				ret_val = *(u8 *)hsdev->pending.raw_data;
 			break;
 		case 2:
-			ret_val = *(u16 *)hsdev->pending.raw_data;
+			if (is_signed)
+				ret_val = *(s16 *)hsdev->pending.raw_data;
+			else
+				ret_val = *(u16 *)hsdev->pending.raw_data;
 			break;
 		case 4:
 			ret_val = *(u32 *)hsdev->pending.raw_data;
drivers/iio/accel/hid-sensor-accel-3d.c +4 -1
index 41d97faf5013..38ff374a3ca4 100644
@@ -149,6 +149,7 @@ static int accel_3d_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 	struct hid_sensor_hub_device *hsdev =
 					accel_state->common_attributes.hsdev;
 

@@ -158,12 +159,14 @@ static int accel_3d_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&accel_state->common_attributes, true);
 		report_id = accel_state->accel[chan->scan_index].report_id;
+		min = accel_state->accel[chan->scan_index].logical_minimum;
 		address = accel_3d_addresses[chan->scan_index];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 					accel_state->common_attributes.hsdev,
 					hsdev->usage, address, report_id,
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
 		else {
 			*val = 0;
 			hid_sensor_power_state(&accel_state->common_attributes,
drivers/iio/gyro/hid-sensor-gyro-3d.c +4 -1
index 36941e69f959..88e857c4baf4 100644
@@ -111,6 +111,7 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;

@@ -118,13 +119,15 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&gyro_state->common_attributes, true);
 		report_id = gyro_state->gyro[chan->scan_index].report_id;
+		min = gyro_state->gyro[chan->scan_index].logical_minimum;
 		address = gyro_3d_addresses[chan->scan_index];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 					gyro_state->common_attributes.hsdev,
 					HID_USAGE_SENSOR_GYRO_3D, address,
 					report_id,
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
 		else {
 			*val = 0;
 			hid_sensor_power_state(&gyro_state->common_attributes,
drivers/iio/humidity/hid-sensor-humidity.c +2 -1
index beab6d6fd6e1..4bc95f31c730 100644
@@ -75,7 +75,8 @@ static int humidity_read_raw(struct iio_dev *indio_dev,
 				HID_USAGE_SENSOR_HUMIDITY,
 				HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY,
 				humid_st->humidity_attr.report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				humid_st->humidity_attr.logical_minimum < 0);
 		hid_sensor_power_state(&humid_st->common_attributes, false);
 
 		return IIO_VAL_INT;
drivers/iio/light/hid-sensor-als.c +5 -3
index 406caaee9a3c..94f33250ba5a 100644
@@ -93,6 +93,7 @@ static int als_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;

@@ -102,8 +103,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
 		case  CHANNEL_SCAN_INDEX_INTENSITY:
 		case  CHANNEL_SCAN_INDEX_ILLUM:
 			report_id = als_state->als_illum.report_id;
-			address =
-			HID_USAGE_SENSOR_LIGHT_ILLUM;
+			min = als_state->als_illum.logical_minimum;
+			address = HID_USAGE_SENSOR_LIGHT_ILLUM;
 			break;
 		default:
 			report_id = -1;

@@ -116,7 +117,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
 					als_state->common_attributes.hsdev,
 					HID_USAGE_SENSOR_ALS, address,
 					report_id,
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
 			hid_sensor_power_state(&als_state->common_attributes,
 						false);
 		} else {
drivers/iio/light/hid-sensor-prox.c +5 -3
index 45107f7537b5..cf5a0c242609 100644
@@ -73,6 +73,7 @@ static int prox_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;

@@ -81,8 +82,8 @@ static int prox_read_raw(struct iio_dev *indio_dev,
 		switch (chan->scan_index) {
 		case  CHANNEL_SCAN_INDEX_PRESENCE:
 			report_id = prox_state->prox_attr.report_id;
-			address =
-			HID_USAGE_SENSOR_HUMAN_PRESENCE;
+			min = prox_state->prox_attr.logical_minimum;
+			address = HID_USAGE_SENSOR_HUMAN_PRESENCE;
 			break;
 		default:
 			report_id = -1;

@@ -95,7 +96,8 @@ static int prox_read_raw(struct iio_dev *indio_dev,
 				prox_state->common_attributes.hsdev,
 				HID_USAGE_SENSOR_PROX, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 			hid_sensor_power_state(&prox_state->common_attributes,
 						false);
 		} else {
drivers/iio/magnetometer/hid-sensor-magn-3d.c +5 -3
index d55c4885211a..f3c0d41e5a8c 100644
@@ -163,21 +163,23 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&magn_state->magn_flux_attributes, true);
-		report_id =
-			magn_state->magn[chan->address].report_id;
+		report_id = magn_state->magn[chan->address].report_id;
+		min = magn_state->magn[chan->address].logical_minimum;
 		address = magn_3d_addresses[chan->address];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 				magn_state->magn_flux_attributes.hsdev,
 				HID_USAGE_SENSOR_COMPASS_3D, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 		else {
 			*val = 0;
 			hid_sensor_power_state(
drivers/iio/orientation/hid-sensor-incl-3d.c +5 -3
index 1e5451d1ff88..bdc5e4554ee4 100644
@@ -111,21 +111,23 @@ static int incl_3d_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&incl_state->common_attributes, true);
-		report_id =
-			incl_state->incl[chan->scan_index].report_id;
+		report_id = incl_state->incl[chan->scan_index].report_id;
+		min = incl_state->incl[chan->scan_index].logical_minimum;
 		address = incl_3d_addresses[chan->scan_index];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 				incl_state->common_attributes.hsdev,
 				HID_USAGE_SENSOR_INCLINOMETER_3D, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 		else {
 			hid_sensor_power_state(&incl_state->common_attributes,
 						false);
drivers/iio/pressure/hid-sensor-press.c +5 -3
index 4c437918f1d2..d7b1c00ceb4d 100644
@@ -77,6 +77,7 @@ static int press_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;

@@ -85,8 +86,8 @@ static int press_read_raw(struct iio_dev *indio_dev,
 		switch (chan->scan_index) {
 		case  CHANNEL_SCAN_INDEX_PRESSURE:
 			report_id = press_state->press_attr.report_id;
-			address =
-			HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE;
+			min = press_state->press_attr.logical_minimum;
+			address = HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE;
 			break;
 		default:
 			report_id = -1;

@@ -99,7 +100,8 @@ static int press_read_raw(struct iio_dev *indio_dev,
 				press_state->common_attributes.hsdev,
 				HID_USAGE_SENSOR_PRESSURE, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 			hid_sensor_power_state(&press_state->common_attributes,
 						false);
 		} else {
drivers/iio/temperature/hid-sensor-temperature.c +2 -1
index beaf6fd3e337..b592fc4f007e 100644
@@ -76,7 +76,8 @@ static int temperature_read_raw(struct iio_dev *indio_dev,
 			HID_USAGE_SENSOR_TEMPERATURE,
 			HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
 			temp_st->temperature_attr.report_id,
-			SENSOR_HUB_SYNC);
+			SENSOR_HUB_SYNC,
+			temp_st->temperature_attr.logical_minimum < 0);
 		hid_sensor_power_state(
 				&temp_st->common_attributes,
 				false);
drivers/rtc/rtc-hid-sensor-time.c +1 -1
index 2751dba850c6..3e1abb455472 100644
@@ -213,7 +213,7 @@ static int hid_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	/* get a report with all values through requesting one value */
 	sensor_hub_input_attr_get_raw_value(time_state->common_attributes.hsdev,
 			HID_USAGE_SENSOR_TIME, hid_time_addresses[0],
-			time_state->info[0].report_id, SENSOR_HUB_SYNC);
+			time_state->info[0].report_id, SENSOR_HUB_SYNC, false);
 	/* wait for all values (event) */
 	ret = wait_for_completion_killable_timeout(
 			&time_state->comp_last_time, HZ*6);
include/linux/hid-sensor-hub.h +3 -1
index 331dc377c275..dc12f5c4b076 100644
@@ -177,6 +177,7 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
 * @attr_usage_id:	Attribute usage id as per spec
 * @report_id:	Report id to look for
 * @flag:      Synchronous or asynchronous read
+* @is_signed:   If true then fields < 32 bits will be sign-extended
 *
 * Issues a synchronous or asynchronous read request for an input attribute.
 * Returns data upto 32 bits.

@@ -190,7 +191,8 @@ enum sensor_hub_read_flags {
 int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
  					u32 usage_id,
  					u32 attr_usage_id, u32 report_id,
- 					enum sensor_hub_read_flags flag
+					enum sensor_hub_read_flags flag,
+					bool is_signed
 );
 
 /**
Signature · 1 line
2.19.0
author
#1 lore raw

Hi,

Quoted 14 lines · Hans de Goede

Before this commit sensor_hub_input_attr_get_raw_value() failed to take the signedness of 16 and 8 bit values into account, returning e.g. 65436 instead of -100 for the z-axis reading of an accelerometer.

This commit adds a new is_signed parameter to the function and makes all callers pass the appropriate value for this.

While at it, this commit also fixes up some neighboring lines where statements were needlessly split over 2 lines to improve readability.

Acked-by Srinivas Pandruvada <[email protected]>
Signed-off-by Hans de Goede <[email protected]>

---
Changes in v2:

Review

Quoted 3 lines
(quoted diff) +0 -2
-Adjust the kernel doc for the new is_signed parameter to
  sensor_hub_input_attr_get_raw_value()
-Add Srinivas' Acked-by

This second copy of this patch is a resend with the HID maintainers added, which I accidentally forgot in the first posting of v2.

This is still v2 with no changes compared to my first submission of v2.

Regards,

Hans

Quoted 344 lines
---
  drivers/hid/hid-sensor-custom.c                  |  2 +-
  drivers/hid/hid-sensor-hub.c                     | 13 ++++++++++---
  drivers/iio/accel/hid-sensor-accel-3d.c          |  5 ++++-
  drivers/iio/gyro/hid-sensor-gyro-3d.c            |  5 ++++-
  drivers/iio/humidity/hid-sensor-humidity.c       |  3 ++-
  drivers/iio/light/hid-sensor-als.c               |  8 +++++---
  drivers/iio/light/hid-sensor-prox.c              |  8 +++++---
  drivers/iio/magnetometer/hid-sensor-magn-3d.c    |  8 +++++---
  drivers/iio/orientation/hid-sensor-incl-3d.c     |  8 +++++---
  drivers/iio/pressure/hid-sensor-press.c          |  8 +++++---
  drivers/iio/temperature/hid-sensor-temperature.c |  3 ++-
  drivers/rtc/rtc-hid-sensor-time.c                |  2 +-
  include/linux/hid-sensor-hub.h                   |  4 +++-
  13 files changed, 52 insertions(+), 25 deletions(-)
drivers/hid/hid-sensor-custom.c +1 -1
index e8a114157f87..bb012bc032e0 100644
@@ -358,7 +358,7 @@ static ssize_t show_value(struct device *dev, struct device_attribute *attr,
  						sensor_inst->hsdev,
  						sensor_inst->hsdev->usage,
  						usage, report_id,
-						SENSOR_HUB_SYNC);
+						SENSOR_HUB_SYNC, false);
  	} else if (!strncmp(name, "units", strlen("units")))
  		value = sensor_inst->fields[field_index].attribute.units;
  	else if (!strncmp(name, "unit-expo", strlen("unit-expo")))
drivers/hid/hid-sensor-hub.c +10 -3
index 2b63487057c2..4256fdc5cd6d 100644
@@ -299,7 +299,8 @@ EXPORT_SYMBOL_GPL(sensor_hub_get_feature);
  int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
  					u32 usage_id,
  					u32 attr_usage_id, u32 report_id,
-					enum sensor_hub_read_flags flag)
+					enum sensor_hub_read_flags flag,
+					bool is_signed)
  {
  	struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
  	unsigned long flags;

@@ -331,10 +332,16 @@ int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
  						&hsdev->pending.ready, HZ*5);
  		switch (hsdev->pending.raw_size) {
  		case 1:
-			ret_val = *(u8 *)hsdev->pending.raw_data;
+			if (is_signed)
+				ret_val = *(s8 *)hsdev->pending.raw_data;
+			else
+				ret_val = *(u8 *)hsdev->pending.raw_data;
  			break;
  		case 2:
-			ret_val = *(u16 *)hsdev->pending.raw_data;
+			if (is_signed)
+				ret_val = *(s16 *)hsdev->pending.raw_data;
+			else
+				ret_val = *(u16 *)hsdev->pending.raw_data;
  			break;
  		case 4:
  			ret_val = *(u32 *)hsdev->pending.raw_data;
drivers/iio/accel/hid-sensor-accel-3d.c +4 -1
index 41d97faf5013..38ff374a3ca4 100644
@@ -149,6 +149,7 @@ static int accel_3d_read_raw(struct iio_dev *indio_dev,
  	int report_id = -1;
  	u32 address;
  	int ret_type;
+	s32 min;
  	struct hid_sensor_hub_device *hsdev =
  					accel_state->common_attributes.hsdev;
 

@@ -158,12 +159,14 @@ static int accel_3d_read_raw(struct iio_dev *indio_dev,
  	case IIO_CHAN_INFO_RAW:
  		hid_sensor_power_state(&accel_state->common_attributes, true);
  		report_id = accel_state->accel[chan->scan_index].report_id;
+		min = accel_state->accel[chan->scan_index].logical_minimum;
  		address = accel_3d_addresses[chan->scan_index];
  		if (report_id >= 0)
  			*val = sensor_hub_input_attr_get_raw_value(
  					accel_state->common_attributes.hsdev,
  					hsdev->usage, address, report_id,
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
  		else {
  			*val = 0;
  			hid_sensor_power_state(&accel_state->common_attributes,
drivers/iio/gyro/hid-sensor-gyro-3d.c +4 -1
index 36941e69f959..88e857c4baf4 100644
@@ -111,6 +111,7 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
  	int report_id = -1;
  	u32 address;
  	int ret_type;
+	s32 min;
 
  	*val = 0;
  	*val2 = 0;

@@ -118,13 +119,15 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
  	case IIO_CHAN_INFO_RAW:
  		hid_sensor_power_state(&gyro_state->common_attributes, true);
  		report_id = gyro_state->gyro[chan->scan_index].report_id;
+		min = gyro_state->gyro[chan->scan_index].logical_minimum;
  		address = gyro_3d_addresses[chan->scan_index];
  		if (report_id >= 0)
  			*val = sensor_hub_input_attr_get_raw_value(
  					gyro_state->common_attributes.hsdev,
  					HID_USAGE_SENSOR_GYRO_3D, address,
  					report_id,
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
  		else {
  			*val = 0;
  			hid_sensor_power_state(&gyro_state->common_attributes,
drivers/iio/humidity/hid-sensor-humidity.c +2 -1
index beab6d6fd6e1..4bc95f31c730 100644
@@ -75,7 +75,8 @@ static int humidity_read_raw(struct iio_dev *indio_dev,
  				HID_USAGE_SENSOR_HUMIDITY,
  				HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY,
  				humid_st->humidity_attr.report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				humid_st->humidity_attr.logical_minimum < 0);
  		hid_sensor_power_state(&humid_st->common_attributes, false);
 
  		return IIO_VAL_INT;
drivers/iio/light/hid-sensor-als.c +5 -3
index 406caaee9a3c..94f33250ba5a 100644
@@ -93,6 +93,7 @@ static int als_read_raw(struct iio_dev *indio_dev,
  	int report_id = -1;
  	u32 address;
  	int ret_type;
+	s32 min;
 
  	*val = 0;
  	*val2 = 0;

@@ -102,8 +103,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
  		case  CHANNEL_SCAN_INDEX_INTENSITY:
  		case  CHANNEL_SCAN_INDEX_ILLUM:
  			report_id = als_state->als_illum.report_id;
-			address =
-			HID_USAGE_SENSOR_LIGHT_ILLUM;
+			min = als_state->als_illum.logical_minimum;
+			address = HID_USAGE_SENSOR_LIGHT_ILLUM;
  			break;
  		default:
  			report_id = -1;

@@ -116,7 +117,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
  					als_state->common_attributes.hsdev,
  					HID_USAGE_SENSOR_ALS, address,
  					report_id,
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
  			hid_sensor_power_state(&als_state->common_attributes,
  						false);
  		} else {
drivers/iio/light/hid-sensor-prox.c +5 -3
index 45107f7537b5..cf5a0c242609 100644
@@ -73,6 +73,7 @@ static int prox_read_raw(struct iio_dev *indio_dev,
  	int report_id = -1;
  	u32 address;
  	int ret_type;
+	s32 min;
 
  	*val = 0;
  	*val2 = 0;

@@ -81,8 +82,8 @@ static int prox_read_raw(struct iio_dev *indio_dev,
  		switch (chan->scan_index) {
  		case  CHANNEL_SCAN_INDEX_PRESENCE:
  			report_id = prox_state->prox_attr.report_id;
-			address =
-			HID_USAGE_SENSOR_HUMAN_PRESENCE;
+			min = prox_state->prox_attr.logical_minimum;
+			address = HID_USAGE_SENSOR_HUMAN_PRESENCE;
  			break;
  		default:
  			report_id = -1;

@@ -95,7 +96,8 @@ static int prox_read_raw(struct iio_dev *indio_dev,
  				prox_state->common_attributes.hsdev,
  				HID_USAGE_SENSOR_PROX, address,
  				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
  			hid_sensor_power_state(&prox_state->common_attributes,
  						false);
  		} else {
drivers/iio/magnetometer/hid-sensor-magn-3d.c +5 -3
index d55c4885211a..f3c0d41e5a8c 100644
@@ -163,21 +163,23 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev,
  	int report_id = -1;
  	u32 address;
  	int ret_type;
+	s32 min;
 
  	*val = 0;
  	*val2 = 0;
  	switch (mask) {
  	case IIO_CHAN_INFO_RAW:
  		hid_sensor_power_state(&magn_state->magn_flux_attributes, true);
-		report_id =
-			magn_state->magn[chan->address].report_id;
+		report_id = magn_state->magn[chan->address].report_id;
+		min = magn_state->magn[chan->address].logical_minimum;
  		address = magn_3d_addresses[chan->address];
  		if (report_id >= 0)
  			*val = sensor_hub_input_attr_get_raw_value(
  				magn_state->magn_flux_attributes.hsdev,
  				HID_USAGE_SENSOR_COMPASS_3D, address,
  				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
  		else {
  			*val = 0;
  			hid_sensor_power_state(
drivers/iio/orientation/hid-sensor-incl-3d.c +5 -3
index 1e5451d1ff88..bdc5e4554ee4 100644
@@ -111,21 +111,23 @@ static int incl_3d_read_raw(struct iio_dev *indio_dev,
  	int report_id = -1;
  	u32 address;
  	int ret_type;
+	s32 min;
 
  	*val = 0;
  	*val2 = 0;
  	switch (mask) {
  	case IIO_CHAN_INFO_RAW:
  		hid_sensor_power_state(&incl_state->common_attributes, true);
-		report_id =
-			incl_state->incl[chan->scan_index].report_id;
+		report_id = incl_state->incl[chan->scan_index].report_id;
+		min = incl_state->incl[chan->scan_index].logical_minimum;
  		address = incl_3d_addresses[chan->scan_index];
  		if (report_id >= 0)
  			*val = sensor_hub_input_attr_get_raw_value(
  				incl_state->common_attributes.hsdev,
  				HID_USAGE_SENSOR_INCLINOMETER_3D, address,
  				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
  		else {
  			hid_sensor_power_state(&incl_state->common_attributes,
  						false);
drivers/iio/pressure/hid-sensor-press.c +5 -3
index 4c437918f1d2..d7b1c00ceb4d 100644
@@ -77,6 +77,7 @@ static int press_read_raw(struct iio_dev *indio_dev,
  	int report_id = -1;
  	u32 address;
  	int ret_type;
+	s32 min;
 
  	*val = 0;
  	*val2 = 0;

@@ -85,8 +86,8 @@ static int press_read_raw(struct iio_dev *indio_dev,
  		switch (chan->scan_index) {
  		case  CHANNEL_SCAN_INDEX_PRESSURE:
  			report_id = press_state->press_attr.report_id;
-			address =
-			HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE;
+			min = press_state->press_attr.logical_minimum;
+			address = HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE;
  			break;
  		default:
  			report_id = -1;

@@ -99,7 +100,8 @@ static int press_read_raw(struct iio_dev *indio_dev,
  				press_state->common_attributes.hsdev,
  				HID_USAGE_SENSOR_PRESSURE, address,
  				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
  			hid_sensor_power_state(&press_state->common_attributes,
  						false);
  		} else {
drivers/iio/temperature/hid-sensor-temperature.c +2 -1
index beaf6fd3e337..b592fc4f007e 100644
@@ -76,7 +76,8 @@ static int temperature_read_raw(struct iio_dev *indio_dev,
  			HID_USAGE_SENSOR_TEMPERATURE,
  			HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
  			temp_st->temperature_attr.report_id,
-			SENSOR_HUB_SYNC);
+			SENSOR_HUB_SYNC,
+			temp_st->temperature_attr.logical_minimum < 0);
  		hid_sensor_power_state(
  				&temp_st->common_attributes,
  				false);
drivers/rtc/rtc-hid-sensor-time.c +1 -1
index 2751dba850c6..3e1abb455472 100644
@@ -213,7 +213,7 @@ static int hid_rtc_read_time(struct device *dev, struct rtc_time *tm)
  	/* get a report with all values through requesting one value */
  	sensor_hub_input_attr_get_raw_value(time_state->common_attributes.hsdev,
  			HID_USAGE_SENSOR_TIME, hid_time_addresses[0],
-			time_state->info[0].report_id, SENSOR_HUB_SYNC);
+			time_state->info[0].report_id, SENSOR_HUB_SYNC, false);
  	/* wait for all values (event) */
  	ret = wait_for_completion_killable_timeout(
  			&time_state->comp_last_time, HZ*6);
include/linux/hid-sensor-hub.h +3 -1
index 331dc377c275..dc12f5c4b076 100644
@@ -177,6 +177,7 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
  * @attr_usage_id:	Attribute usage id as per spec
  * @report_id:	Report id to look for
  * @flag:      Synchronous or asynchronous read
+* @is_signed:   If true then fields < 32 bits will be sign-extended
  *
  * Issues a synchronous or asynchronous read request for an input attribute.
  * Returns data upto 32 bits.

@@ -190,7 +191,8 @@ enum sensor_hub_read_flags {
  int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
   					u32 usage_id,
   					u32 attr_usage_id, u32 report_id,
- 					enum sensor_hub_read_flags flag
+					enum sensor_hub_read_flags flag,
+					bool is_signed
  );
 
  /**
maintainer
#2 lore raw
In reply to Hans de Goede
Quoted 4 lines · On Wed, 31 Oct 2018 15:20:05 +0100 Hans de Goede <[email protected]> wrote:

Before this commit sensor_hub_input_attr_get_raw_value() failed to take the signedness of 16 and 8 bit values into account, returning e.g. 65436 instead of -100 for the z-axis reading of an accelerometer.

Quoted 8 lines

This commit adds a new is_signed parameter to the function and makes all callers pass the appropriate value for this.

While at it, this commit also fixes up some neighboring lines where statements were needlessly split over 2 lines to improve readability.

Acked-by Srinivas Pandruvada <[email protected]>
Signed-off-by Hans de Goede <[email protected]>

Yikes. Not sure how this was missed for so long.

I'll want the hid maintainers ack for this but probably makes sense to take it through IIO.

Is this a for all time thing or should we have an explict Fixes tag in place?

I do wonder if we have userspace code working around this which is going to get confused, but fingers crossed that's not the case.

Definitely stable material I think.

Jonathan

Quoted 348 lines

---
Changes in v2:

(quoted diff) +0 -3
-Adjust the kernel doc for the new is_signed parameter to
 sensor_hub_input_attr_get_raw_value()
-Add Srinivas' Acked-by
---
 drivers/hid/hid-sensor-custom.c                  |  2 +-
 drivers/hid/hid-sensor-hub.c                     | 13 ++++++++++---
 drivers/iio/accel/hid-sensor-accel-3d.c          |  5 ++++-
 drivers/iio/gyro/hid-sensor-gyro-3d.c            |  5 ++++-
 drivers/iio/humidity/hid-sensor-humidity.c       |  3 ++-
 drivers/iio/light/hid-sensor-als.c               |  8 +++++---
 drivers/iio/light/hid-sensor-prox.c              |  8 +++++---
 drivers/iio/magnetometer/hid-sensor-magn-3d.c    |  8 +++++---
 drivers/iio/orientation/hid-sensor-incl-3d.c     |  8 +++++---
 drivers/iio/pressure/hid-sensor-press.c          |  8 +++++---
 drivers/iio/temperature/hid-sensor-temperature.c |  3 ++-
 drivers/rtc/rtc-hid-sensor-time.c                |  2 +-
 include/linux/hid-sensor-hub.h                   |  4 +++-
 13 files changed, 52 insertions(+), 25 deletions(-)
drivers/hid/hid-sensor-custom.c +1 -1
index e8a114157f87..bb012bc032e0 100644
@@ -358,7 +358,7 @@ static ssize_t show_value(struct device *dev, struct device_attribute *attr,
 						sensor_inst->hsdev,
 						sensor_inst->hsdev->usage,
 						usage, report_id,
-						SENSOR_HUB_SYNC);
+						SENSOR_HUB_SYNC, false);
 	} else if (!strncmp(name, "units", strlen("units")))
 		value = sensor_inst->fields[field_index].attribute.units;
 	else if (!strncmp(name, "unit-expo", strlen("unit-expo")))
drivers/hid/hid-sensor-hub.c +10 -3
index 2b63487057c2..4256fdc5cd6d 100644
@@ -299,7 +299,8 @@ EXPORT_SYMBOL_GPL(sensor_hub_get_feature);
 int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
 					u32 usage_id,
 					u32 attr_usage_id, u32 report_id,
-					enum sensor_hub_read_flags flag)
+					enum sensor_hub_read_flags flag,
+					bool is_signed)
 {
 	struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
 	unsigned long flags;

@@ -331,10 +332,16 @@ int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
 						&hsdev->pending.ready, HZ*5);
 		switch (hsdev->pending.raw_size) {
 		case 1:
-			ret_val = *(u8 *)hsdev->pending.raw_data;
+			if (is_signed)
+				ret_val = *(s8 *)hsdev->pending.raw_data;
+			else
+				ret_val = *(u8 *)hsdev->pending.raw_data;
 			break;
 		case 2:
-			ret_val = *(u16 *)hsdev->pending.raw_data;
+			if (is_signed)
+				ret_val = *(s16 *)hsdev->pending.raw_data;
+			else
+				ret_val = *(u16 *)hsdev->pending.raw_data;
 			break;
 		case 4:
 			ret_val = *(u32 *)hsdev->pending.raw_data;
drivers/iio/accel/hid-sensor-accel-3d.c +4 -1
index 41d97faf5013..38ff374a3ca4 100644
@@ -149,6 +149,7 @@ static int accel_3d_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 	struct hid_sensor_hub_device *hsdev =
 					accel_state->common_attributes.hsdev;
 

@@ -158,12 +159,14 @@ static int accel_3d_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&accel_state->common_attributes, true);
 		report_id = accel_state->accel[chan->scan_index].report_id;
+		min = accel_state->accel[chan->scan_index].logical_minimum;
 		address = accel_3d_addresses[chan->scan_index];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 					accel_state->common_attributes.hsdev,
 					hsdev->usage, address, report_id,
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
 		else {
 			*val = 0;
 			hid_sensor_power_state(&accel_state->common_attributes,
drivers/iio/gyro/hid-sensor-gyro-3d.c +4 -1
index 36941e69f959..88e857c4baf4 100644
@@ -111,6 +111,7 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;

@@ -118,13 +119,15 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&gyro_state->common_attributes, true);
 		report_id = gyro_state->gyro[chan->scan_index].report_id;
+		min = gyro_state->gyro[chan->scan_index].logical_minimum;
 		address = gyro_3d_addresses[chan->scan_index];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 					gyro_state->common_attributes.hsdev,
 					HID_USAGE_SENSOR_GYRO_3D, address,
 					report_id,
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
 		else {
 			*val = 0;
 			hid_sensor_power_state(&gyro_state->common_attributes,
drivers/iio/humidity/hid-sensor-humidity.c +2 -1
index beab6d6fd6e1..4bc95f31c730 100644
@@ -75,7 +75,8 @@ static int humidity_read_raw(struct iio_dev *indio_dev,
 				HID_USAGE_SENSOR_HUMIDITY,
 				HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY,
 				humid_st->humidity_attr.report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				humid_st->humidity_attr.logical_minimum < 0);
 		hid_sensor_power_state(&humid_st->common_attributes, false);
 
 		return IIO_VAL_INT;
drivers/iio/light/hid-sensor-als.c +5 -3
index 406caaee9a3c..94f33250ba5a 100644
@@ -93,6 +93,7 @@ static int als_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;

@@ -102,8 +103,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
 		case  CHANNEL_SCAN_INDEX_INTENSITY:
 		case  CHANNEL_SCAN_INDEX_ILLUM:
 			report_id = als_state->als_illum.report_id;
-			address =
-			HID_USAGE_SENSOR_LIGHT_ILLUM;
+			min = als_state->als_illum.logical_minimum;
+			address = HID_USAGE_SENSOR_LIGHT_ILLUM;
 			break;
 		default:
 			report_id = -1;

@@ -116,7 +117,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
 					als_state->common_attributes.hsdev,
 					HID_USAGE_SENSOR_ALS, address,
 					report_id,
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
 			hid_sensor_power_state(&als_state->common_attributes,
 						false);
 		} else {
drivers/iio/light/hid-sensor-prox.c +5 -3
index 45107f7537b5..cf5a0c242609 100644
@@ -73,6 +73,7 @@ static int prox_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;

@@ -81,8 +82,8 @@ static int prox_read_raw(struct iio_dev *indio_dev,
 		switch (chan->scan_index) {
 		case  CHANNEL_SCAN_INDEX_PRESENCE:
 			report_id = prox_state->prox_attr.report_id;
-			address =
-			HID_USAGE_SENSOR_HUMAN_PRESENCE;
+			min = prox_state->prox_attr.logical_minimum;
+			address = HID_USAGE_SENSOR_HUMAN_PRESENCE;
 			break;
 		default:
 			report_id = -1;

@@ -95,7 +96,8 @@ static int prox_read_raw(struct iio_dev *indio_dev,
 				prox_state->common_attributes.hsdev,
 				HID_USAGE_SENSOR_PROX, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 			hid_sensor_power_state(&prox_state->common_attributes,
 						false);
 		} else {
drivers/iio/magnetometer/hid-sensor-magn-3d.c +5 -3
index d55c4885211a..f3c0d41e5a8c 100644
@@ -163,21 +163,23 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&magn_state->magn_flux_attributes, true);
-		report_id =
-			magn_state->magn[chan->address].report_id;
+		report_id = magn_state->magn[chan->address].report_id;
+		min = magn_state->magn[chan->address].logical_minimum;
 		address = magn_3d_addresses[chan->address];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 				magn_state->magn_flux_attributes.hsdev,
 				HID_USAGE_SENSOR_COMPASS_3D, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 		else {
 			*val = 0;
 			hid_sensor_power_state(
drivers/iio/orientation/hid-sensor-incl-3d.c +5 -3
index 1e5451d1ff88..bdc5e4554ee4 100644
@@ -111,21 +111,23 @@ static int incl_3d_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&incl_state->common_attributes, true);
-		report_id =
-			incl_state->incl[chan->scan_index].report_id;
+		report_id = incl_state->incl[chan->scan_index].report_id;
+		min = incl_state->incl[chan->scan_index].logical_minimum;
 		address = incl_3d_addresses[chan->scan_index];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 				incl_state->common_attributes.hsdev,
 				HID_USAGE_SENSOR_INCLINOMETER_3D, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 		else {
 			hid_sensor_power_state(&incl_state->common_attributes,
 						false);
drivers/iio/pressure/hid-sensor-press.c +5 -3
index 4c437918f1d2..d7b1c00ceb4d 100644
@@ -77,6 +77,7 @@ static int press_read_raw(struct iio_dev *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;

@@ -85,8 +86,8 @@ static int press_read_raw(struct iio_dev *indio_dev,
 		switch (chan->scan_index) {
 		case  CHANNEL_SCAN_INDEX_PRESSURE:
 			report_id = press_state->press_attr.report_id;
-			address =
-			HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE;
+			min = press_state->press_attr.logical_minimum;
+			address = HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE;
 			break;
 		default:
 			report_id = -1;

@@ -99,7 +100,8 @@ static int press_read_raw(struct iio_dev *indio_dev,
 				press_state->common_attributes.hsdev,
 				HID_USAGE_SENSOR_PRESSURE, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 			hid_sensor_power_state(&press_state->common_attributes,
 						false);
 		} else {
drivers/iio/temperature/hid-sensor-temperature.c +2 -1
index beaf6fd3e337..b592fc4f007e 100644
@@ -76,7 +76,8 @@ static int temperature_read_raw(struct iio_dev *indio_dev,
 			HID_USAGE_SENSOR_TEMPERATURE,
 			HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
 			temp_st->temperature_attr.report_id,
-			SENSOR_HUB_SYNC);
+			SENSOR_HUB_SYNC,
+			temp_st->temperature_attr.logical_minimum < 0);
 		hid_sensor_power_state(
 				&temp_st->common_attributes,
 				false);
drivers/rtc/rtc-hid-sensor-time.c +1 -1
index 2751dba850c6..3e1abb455472 100644
@@ -213,7 +213,7 @@ static int hid_rtc_read_time(struct device *dev, struct rtc_time *tm)
 	/* get a report with all values through requesting one value */
 	sensor_hub_input_attr_get_raw_value(time_state->common_attributes.hsdev,
 			HID_USAGE_SENSOR_TIME, hid_time_addresses[0],
-			time_state->info[0].report_id, SENSOR_HUB_SYNC);
+			time_state->info[0].report_id, SENSOR_HUB_SYNC, false);
 	/* wait for all values (event) */
 	ret = wait_for_completion_killable_timeout(
 			&time_state->comp_last_time, HZ*6);
include/linux/hid-sensor-hub.h +3 -1
index 331dc377c275..dc12f5c4b076 100644
@@ -177,6 +177,7 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
 * @attr_usage_id:	Attribute usage id as per spec
 * @report_id:	Report id to look for
 * @flag:      Synchronous or asynchronous read
+* @is_signed:   If true then fields < 32 bits will be sign-extended
 *
 * Issues a synchronous or asynchronous read request for an input attribute.
 * Returns data upto 32 bits.

@@ -190,7 +191,8 @@ enum sensor_hub_read_flags {
 int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
  					u32 usage_id,
  					u32 attr_usage_id, u32 report_id,
- 					enum sensor_hub_read_flags flag
+					enum sensor_hub_read_flags flag,
+					bool is_signed
 );
 
 /**
maintainer
#3 lore raw
Quoted 12 lines · Jonathan Cameron
Quoted 9 lines · On Wed, 31 Oct 2018 15:20:05 +0100 Hans de Goede <[email protected]> wrote:

Before this commit sensor_hub_input_attr_get_raw_value() failed to take
the signedness of 16 and 8 bit values into account, returning e.g. 65436 instead of -100 for the z-axis reading of an accelerometer.

This commit adds a new is_signed parameter to the function and makes all
callers pass the appropriate value for this.

Quoted 8 lines
Quoted 6 lines

While at it, this commit also fixes up some neighboring lines where statements were needlessly split over 2 lines to improve readability.

Acked-by Srinivas Pandruvada <[email protected]>
Signed-off-by Hans de Goede <[email protected]>

Yikes. Not sure how this was missed for so long.

This is issue only when not 4 bytes, which is the case in most cases. The reliance on logical minimum was always risky before.

Quoted 6 lines

I'll want the hid maintainers ack for this but probably makes sense to take it through IIO.

Is this a for all time thing or should we have an explict Fixes tag in place?

I think explicit fix is fine as the user space is using buffered mode where they know size and sign.

Quoted 5 lines

I do wonder if we have userspace code working around this which is going to get confused, but fingers crossed that's not the case.

Definitely stable material I think.

Yes.

Thanks,
Srinivas

Quoted 436 lines

Jonathan

Quoted 431 lines

---
Changes in v2:

(quoted diff) +0 -4
-Adjust the kernel doc for the new is_signed parameter to
 sensor_hub_input_attr_get_raw_value()
-Add Srinivas' Acked-by
---
 drivers/hid/hid-sensor-custom.c                  |  2 +-
 drivers/hid/hid-sensor-hub.c                     | 13 ++++++++++
---
 drivers/iio/accel/hid-sensor-accel-3d.c          |  5 ++++-
 drivers/iio/gyro/hid-sensor-gyro-3d.c            |  5 ++++-
 drivers/iio/humidity/hid-sensor-humidity.c       |  3 ++-
 drivers/iio/light/hid-sensor-als.c               |  8 +++++---
 drivers/iio/light/hid-sensor-prox.c              |  8 +++++---
 drivers/iio/magnetometer/hid-sensor-magn-3d.c    |  8 +++++---
 drivers/iio/orientation/hid-sensor-incl-3d.c     |  8 +++++---
 drivers/iio/pressure/hid-sensor-press.c          |  8 +++++---
 drivers/iio/temperature/hid-sensor-temperature.c |  3 ++-
 drivers/rtc/rtc-hid-sensor-time.c                |  2 +-
 include/linux/hid-sensor-hub.h                   |  4 +++-
 13 files changed, 52 insertions(+), 25 deletions(-)
drivers/hid/hid- renamed from drivers/hid/hid-sensor-custom.c +0 -0

sensor-custom.c
index e8a114157f87..bb012bc032e0 100644

drivers/hid/hid-sensor-custom.c +1 -1
@@ -358,7 +358,7 @@ static ssize_t show_value(struct device *dev,
 struct device_attribute *attr,
 						sensor_inst->hsdev,
 						sensor_inst->hsdev-
 >usage,
 						usage, report_id,
-						SENSOR_HUB_SYNC);
+						SENSOR_HUB_SYNC,
 false);
drivers/hid/hid-sensor-custom.c +0 -0
 	} else if (!strncmp(name, "units", strlen("units")))
 		value = sensor_inst-
Quoted 1 line

fields[field_index].attribute.units;

drivers/hid/hid-sensor-custom.c +0 -0
 	else if (!strncmp(name, "unit-expo", strlen("unit-expo")))
drivers/hid/hid-sensor- renamed from drivers/hid/hid-sensor-hub.c +0 -0

hub.c
index 2b63487057c2..4256fdc5cd6d 100644

drivers/hid/hid-sensor-hub.c +1 -1
@@ -299,7 +299,8 @@ EXPORT_SYMBOL_GPL(sensor_hub_get_feature);
 int sensor_hub_input_attr_get_raw_value(struct
 hid_sensor_hub_device *hsdev,
 					u32 usage_id,
 					u32 attr_usage_id, u32
 report_id,
-					enum sensor_hub_read_flags
 flag)
+					enum sensor_hub_read_flags

flag,

drivers/hid/hid-sensor-hub.c +1 -0
+					bool is_signed)
 {
 	struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
 	unsigned long flags;

@@ -331,10 +332,16 @@ int

sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
&hsdev->pending.ready,
HZ*5);

drivers/hid/hid-sensor-hub.c +2 -1
 		switch (hsdev->pending.raw_size) {
 		case 1:
-			ret_val = *(u8 *)hsdev->pending.raw_data;
+			if (is_signed)
+				ret_val = *(s8 *)hsdev-
Quoted 1 line

pending.raw_data;

drivers/hid/hid-sensor-hub.c +2 -0
+			else
+				ret_val = *(u8 *)hsdev-
Quoted 1 line

pending.raw_data;

drivers/hid/hid-sensor-hub.c +2 -1
 			break;
 		case 2:
-			ret_val = *(u16 *)hsdev->pending.raw_data;
+			if (is_signed)
+				ret_val = *(s16 *)hsdev-
Quoted 1 line

pending.raw_data;

drivers/hid/hid-sensor-hub.c +2 -0
+			else
+				ret_val = *(u16 *)hsdev-
Quoted 1 line

pending.raw_data;

drivers/hid/hid-sensor-hub.c +0 -0
 			break;
 		case 4:
 			ret_val = *(u32 *)hsdev->pending.raw_data;
drivers/iio/accel/hid-sensor-accel-3d.c +1 -0
index 41d97faf5013..38ff374a3ca4 100644
@@ -149,6 +149,7 @@ static int accel_3d_read_raw(struct iio_dev
 *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 	struct hid_sensor_hub_device *hsdev =
 					accel_state-
Quoted 1 line

common_attributes.hsdev;

drivers/iio/accel/hid-sensor-accel-3d.c +1 -0
@@ -158,12 +159,14 @@ static int accel_3d_read_raw(struct iio_dev
 *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&accel_state->common_attributes, 
 true);
 		report_id = accel_state->accel[chan-
 >scan_index].report_id;
+		min = accel_state->accel[chan-
 >scan_index].logical_minimum;
 		address = accel_3d_addresses[chan->scan_index];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 					accel_state-
 >common_attributes.hsdev,
 					hsdev->usage, address,

report_id,

drivers/iio/accel/hid-sensor-accel-3d.c +2 -1
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
 		else {
 			*val = 0;
 			hid_sensor_power_state(&accel_state-
Quoted 1 line

common_attributes,

drivers/iio/gyro/hid-sensor-gyro-3d.c +1 -0
index 36941e69f959..88e857c4baf4 100644
@@ -111,6 +111,7 @@ static int gyro_3d_read_raw(struct iio_dev
 *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
drivers/iio/gyro/hid-sensor-gyro-3d.c +1 -0
 	*val2 = 0;

@@ -118,13 +119,15 @@ static int gyro_3d_read_raw(struct iio_dev
 *indio_dev,
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&gyro_state->common_attributes,
 true);
 		report_id = gyro_state->gyro[chan-
 >scan_index].report_id;
+		min = gyro_state->gyro[chan-
 >scan_index].logical_minimum;
 		address = gyro_3d_addresses[chan->scan_index];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 					gyro_state-
 >common_attributes.hsdev,
 					HID_USAGE_SENSOR_GYRO_3D,

address,

drivers/iio/gyro/hid-sensor-gyro-3d.c +2 -1
 					report_id,
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
 		else {
 			*val = 0;
 			hid_sensor_power_state(&gyro_state-
Quoted 1 line

common_attributes,

drivers/iio/humidity/hid-sensor-humidity.c +2 -1
index beab6d6fd6e1..4bc95f31c730 100644
@@ -75,7 +75,8 @@ static int humidity_read_raw(struct iio_dev
 *indio_dev,
 				HID_USAGE_SENSOR_HUMIDITY,
 				HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY,
 				humid_st->humidity_attr.report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				humid_st->humidity_attr.logical_minimum 
 < 0);
 		hid_sensor_power_state(&humid_st->common_attributes,

false);

drivers/iio/humidity/hid-sensor-humidity.c +0 -0
 		return IIO_VAL_INT;
drivers/iio/light/hid-sensor-als.c +1 -0
index 406caaee9a3c..94f33250ba5a 100644
@@ -93,6 +93,7 @@ static int als_read_raw(struct iio_dev
 *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
drivers/iio/light/hid-sensor-als.c +2 -2
 	*val2 = 0;

@@ -102,8 +103,8 @@ static int als_read_raw(struct iio_dev
 *indio_dev,
 		case  CHANNEL_SCAN_INDEX_INTENSITY:
 		case  CHANNEL_SCAN_INDEX_ILLUM:
 			report_id = als_state->als_illum.report_id;
-			address =
-			HID_USAGE_SENSOR_LIGHT_ILLUM;
+			min = als_state->als_illum.logical_minimum;
+			address = HID_USAGE_SENSOR_LIGHT_ILLUM;
 			break;
 		default:
drivers/iio/light/hid-sensor-als.c +2 -1
 			report_id = -1;

@@ -116,7 +117,8 @@ static int als_read_raw(struct iio_dev
 *indio_dev,
 					als_state-
 >common_attributes.hsdev,
 					HID_USAGE_SENSOR_ALS, address,
 					report_id,
-					SENSOR_HUB_SYNC);
+					SENSOR_HUB_SYNC,
+					min < 0);
 			hid_sensor_power_state(&als_state-
Quoted 1 line

common_attributes,

drivers/iio/light/hid-sensor-als.c +0 -0
 						false);
 		} else {
drivers/iio/light/hid-sensor-prox.c +1 -0
index 45107f7537b5..cf5a0c242609 100644
@@ -73,6 +73,7 @@ static int prox_read_raw(struct iio_dev
 *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
drivers/iio/light/hid-sensor-prox.c +2 -2
 	*val2 = 0;

@@ -81,8 +82,8 @@ static int prox_read_raw(struct iio_dev
 *indio_dev,
 		switch (chan->scan_index) {
 		case  CHANNEL_SCAN_INDEX_PRESENCE:
 			report_id = prox_state->prox_attr.report_id;
-			address =
-			HID_USAGE_SENSOR_HUMAN_PRESENCE;
+			min = prox_state->prox_attr.logical_minimum;
+			address = HID_USAGE_SENSOR_HUMAN_PRESENCE;
 			break;
 		default:
drivers/iio/light/hid-sensor-prox.c +2 -1
 			report_id = -1;

@@ -95,7 +96,8 @@ static int prox_read_raw(struct iio_dev
 *indio_dev,
 				prox_state->common_attributes.hsdev,
 				HID_USAGE_SENSOR_PROX, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 			hid_sensor_power_state(&prox_state-
 >common_attributes,
drivers/iio/light/hid-sensor-prox.c +0 -0
 						false);
 		} else {
drivers/iio/magnetometer/hid-sensor-magn-3d.c +5 -3
index d55c4885211a..f3c0d41e5a8c 100644
@@ -163,21 +163,23 @@ static int magn_3d_read_raw(struct iio_dev
 *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&magn_state-
 >magn_flux_attributes, true);
-		report_id =
-			magn_state->magn[chan->address].report_id;
+		report_id = magn_state->magn[chan->address].report_id;
+		min = magn_state->magn[chan->address].logical_minimum;
 		address = magn_3d_addresses[chan->address];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 				magn_state->magn_flux_attributes.hsdev,
 				HID_USAGE_SENSOR_COMPASS_3D, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 		else {
drivers/iio/magnetometer/hid-sensor-magn-3d.c +0 -0
 			*val = 0;
 			hid_sensor_power_state(
drivers/iio/orientation/hid-sensor-incl-3d.c +5 -3
index 1e5451d1ff88..bdc5e4554ee4 100644
@@ -111,21 +111,23 @@ static int incl_3d_read_raw(struct iio_dev
 *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
 	*val2 = 0;
 	switch (mask) {
 	case IIO_CHAN_INFO_RAW:
 		hid_sensor_power_state(&incl_state->common_attributes,
 true);
-		report_id =
-			incl_state->incl[chan->scan_index].report_id;
+		report_id = incl_state->incl[chan-
 >scan_index].report_id;
+		min = incl_state->incl[chan-
 >scan_index].logical_minimum;
 		address = incl_3d_addresses[chan->scan_index];
 		if (report_id >= 0)
 			*val = sensor_hub_input_attr_get_raw_value(
 				incl_state->common_attributes.hsdev,
 				HID_USAGE_SENSOR_INCLINOMETER_3D,
 address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 		else {
 			hid_sensor_power_state(&incl_state-
Quoted 1 line

common_attributes,

drivers/iio/orientation/hid-sensor-incl-3d.c +0 -0
 						false);
drivers/iio/pressure/hid-sensor-press.c +1 -0
index 4c437918f1d2..d7b1c00ceb4d 100644
@@ -77,6 +77,7 @@ static int press_read_raw(struct iio_dev
 *indio_dev,
 	int report_id = -1;
 	u32 address;
 	int ret_type;
+	s32 min;
 
 	*val = 0;
drivers/iio/pressure/hid-sensor-press.c +2 -2
 	*val2 = 0;

@@ -85,8 +86,8 @@ static int press_read_raw(struct iio_dev
 *indio_dev,
 		switch (chan->scan_index) {
 		case  CHANNEL_SCAN_INDEX_PRESSURE:
 			report_id = press_state->press_attr.report_id;
-			address =
-			HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE;
+			min = press_state->press_attr.logical_minimum;
+			address =
 HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE;
 			break;
drivers/iio/pressure/hid-sensor-press.c +2 -1
 		default:
 			report_id = -1;

@@ -99,7 +100,8 @@ static int press_read_raw(struct iio_dev
 *indio_dev,
 				press_state->common_attributes.hsdev,
 				HID_USAGE_SENSOR_PRESSURE, address,
 				report_id,
-				SENSOR_HUB_SYNC);
+				SENSOR_HUB_SYNC,
+				min < 0);
 			hid_sensor_power_state(&press_state-
 >common_attributes,
drivers/iio/pressure/hid-sensor-press.c +0 -0
 						false);
 		} else {
drivers/iio/temperature/hid-sensor-temperature.c +2 -1
index beaf6fd3e337..b592fc4f007e 100644
@@ -76,7 +76,8 @@ static int temperature_read_raw(struct iio_dev
 *indio_dev,
 			HID_USAGE_SENSOR_TEMPERATURE,
 			HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE
 ,
 			temp_st->temperature_attr.report_id,
-			SENSOR_HUB_SYNC);
+			SENSOR_HUB_SYNC,
+			temp_st->temperature_attr.logical_minimum < 0);
 		hid_sensor_power_state(
drivers/iio/temperature/hid-sensor-temperature.c +0 -0
 				&temp_st->common_attributes,
 				false);
drivers/rtc/rtc- renamed from drivers/rtc/rtc-hid-sensor-time.c +0 -0

hid-sensor-time.c
index 2751dba850c6..3e1abb455472 100644

drivers/rtc/rtc-hid-sensor-time.c +1 -1
@@ -213,7 +213,7 @@ static int hid_rtc_read_time(struct device
 *dev, struct rtc_time *tm)
 	/* get a report with all values through requesting one value */
 	sensor_hub_input_attr_get_raw_value(time_state-
 >common_attributes.hsdev,
 			HID_USAGE_SENSOR_TIME, hid_time_addresses[0],
-			time_state->info[0].report_id,
 SENSOR_HUB_SYNC);
+			time_state->info[0].report_id, SENSOR_HUB_SYNC,

false);

drivers/rtc/rtc-hid-sensor-time.c +0 -0
 	/* wait for all values (event) */
 	ret = wait_for_completion_killable_timeout(
 			&time_state->comp_last_time, HZ*6);
include/linux/hid- renamed from include/linux/hid-sensor-hub.h +0 -0

sensor-hub.h
index 331dc377c275..dc12f5c4b076 100644

include/linux/hid-sensor-hub.h +1 -0
@@ -177,6 +177,7 @@ int sensor_hub_input_get_attribute_info(struct
 hid_sensor_hub_device *hsdev,
 * @attr_usage_id:	Attribute usage id as per spec
 * @report_id:	Report id to look for
 * @flag:      Synchronous or asynchronous read
+* @is_signed:   If true then fields < 32 bits will be sign-

extended
*
* Issues a synchronous or asynchronous read request for an input
attribute.
* Returns data upto 32 bits.

include/linux/hid-sensor-hub.h +2 -1
@@ -190,7 +191,8 @@ enum sensor_hub_read_flags {
 int sensor_hub_input_attr_get_raw_value(struct
 hid_sensor_hub_device *hsdev,
  					u32 usage_id,
  					u32 attr_usage_id, u32
 report_id,
- 					enum sensor_hub_read_flags flag
+					enum sensor_hub_read_flags
 flag,
+					bool is_signed

);

/**

Acked-by
#4 lore raw
In reply to Jonathan Cameron
Quoted 14 lines · Jonathan Cameron
Quoted 10 lines · On Wed, 31 Oct 2018 15:20:05 +0100 Hans de Goede <[email protected]> wrote:

Before this commit sensor_hub_input_attr_get_raw_value() failed to take the signedness of 16 and 8 bit values into account, returning e.g. 65436 instead of -100 for the z-axis reading of an accelerometer.

This commit adds a new is_signed parameter to the function and makes all callers pass the appropriate value for this.

While at it, this commit also fixes up some neighboring lines where statements were needlessly split over 2 lines to improve readability.

Quoted 6 lines
Quoted 2 lines
Acked-by Srinivas Pandruvada <[email protected]>
Signed-off-by Hans de Goede <[email protected]>

Yikes. Not sure how this was missed for so long.

I'll want the hid maintainers ack for this but probably makes sense to take it through IIO.

In case you are still waiting for an answer here, it's an ACK from me:

Acked-by Benjamin Tissoires <[email protected]>

Feel free to take it through your tree, I'll deal with the conflicts if there are any changes to hid-sensor-custom.c or hid-sensor-hub.c in this cycle.

Cheers,
Benjamin

Quoted 360 lines

Is this a for all time thing or should we have an explict Fixes tag in place?

I do wonder if we have userspace code working around this which is going to get confused, but fingers crossed that's not the case.

Definitely stable material I think.

Jonathan

Quoted 348 lines

---
Changes in v2:

(quoted diff) +0 -3
-Adjust the kernel doc for the new is_signed parameter to
 sensor_hub_input_attr_get_raw_value()
-Add Srinivas' Acked-by
---
 drivers/hid/hid-sensor-custom.c                  |  2 +-
 drivers/hid/hid-sensor-hub.c                     | 13 ++++++++++---
 drivers/iio/accel/hid-sensor-accel-3d.c          |  5 ++++-
 drivers/iio/gyro/hid-sensor-gyro-3d.c            |  5 ++++-
 drivers/iio/humidity/hid-sensor-humidity.c       |  3 ++-
 drivers/iio/light/hid-sensor-als.c               |  8 +++++---
 drivers/iio/light/hid-sensor-prox.c              |  8 +++++---
 drivers/iio/magnetometer/hid-sensor-magn-3d.c    |  8 +++++---
 drivers/iio/orientation/hid-sensor-incl-3d.c     |  8 +++++---
 drivers/iio/pressure/hid-sensor-press.c          |  8 +++++---
 drivers/iio/temperature/hid-sensor-temperature.c |  3 ++-
 drivers/rtc/rtc-hid-sensor-time.c                |  2 +-
 include/linux/hid-sensor-hub.h                   |  4 +++-
 13 files changed, 52 insertions(+), 25 deletions(-)
drivers/hid/hid-sensor-custom.c +1 -1
index e8a114157f87..bb012bc032e0 100644
@@ -358,7 +358,7 @@ static ssize_t show_value(struct device *dev, struct device_attribute *attr,
                                              sensor_inst->hsdev,
                                              sensor_inst->hsdev->usage,
                                              usage, report_id,
-                                             SENSOR_HUB_SYNC);
+                                             SENSOR_HUB_SYNC, false);
      } else if (!strncmp(name, "units", strlen("units")))
              value = sensor_inst->fields[field_index].attribute.units;
      else if (!strncmp(name, "unit-expo", strlen("unit-expo")))
drivers/hid/hid-sensor-hub.c +10 -3
index 2b63487057c2..4256fdc5cd6d 100644
@@ -299,7 +299,8 @@ EXPORT_SYMBOL_GPL(sensor_hub_get_feature);
 int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
                                      u32 usage_id,
                                      u32 attr_usage_id, u32 report_id,
-                                     enum sensor_hub_read_flags flag)
+                                     enum sensor_hub_read_flags flag,
+                                     bool is_signed)
 {
      struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
      unsigned long flags;

@@ -331,10 +332,16 @@ int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
                                              &hsdev->pending.ready, HZ*5);
              switch (hsdev->pending.raw_size) {
              case 1:
-                     ret_val = *(u8 *)hsdev->pending.raw_data;
+                     if (is_signed)
+                             ret_val = *(s8 *)hsdev->pending.raw_data;
+                     else
+                             ret_val = *(u8 *)hsdev->pending.raw_data;
                      break;
              case 2:
-                     ret_val = *(u16 *)hsdev->pending.raw_data;
+                     if (is_signed)
+                             ret_val = *(s16 *)hsdev->pending.raw_data;
+                     else
+                             ret_val = *(u16 *)hsdev->pending.raw_data;
                      break;
              case 4:
                      ret_val = *(u32 *)hsdev->pending.raw_data;
drivers/iio/accel/hid-sensor-accel-3d.c +4 -1
index 41d97faf5013..38ff374a3ca4 100644
@@ -149,6 +149,7 @@ static int accel_3d_read_raw(struct iio_dev *indio_dev,
      int report_id = -1;
      u32 address;
      int ret_type;
+     s32 min;
      struct hid_sensor_hub_device *hsdev =
                                      accel_state->common_attributes.hsdev;
 

@@ -158,12 +159,14 @@ static int accel_3d_read_raw(struct iio_dev *indio_dev,
      case IIO_CHAN_INFO_RAW:
              hid_sensor_power_state(&accel_state->common_attributes, true);
              report_id = accel_state->accel[chan->scan_index].report_id;
+             min = accel_state->accel[chan->scan_index].logical_minimum;
              address = accel_3d_addresses[chan->scan_index];
              if (report_id >= 0)
                      *val = sensor_hub_input_attr_get_raw_value(
                                      accel_state->common_attributes.hsdev,
                                      hsdev->usage, address, report_id,
-                                     SENSOR_HUB_SYNC);
+                                     SENSOR_HUB_SYNC,
+                                     min < 0);
              else {
                      *val = 0;
                      hid_sensor_power_state(&accel_state->common_attributes,
drivers/iio/gyro/hid-sensor-gyro-3d.c +4 -1
index 36941e69f959..88e857c4baf4 100644
@@ -111,6 +111,7 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
      int report_id = -1;
      u32 address;
      int ret_type;
+     s32 min;
 
      *val = 0;
      *val2 = 0;

@@ -118,13 +119,15 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
      case IIO_CHAN_INFO_RAW:
              hid_sensor_power_state(&gyro_state->common_attributes, true);
              report_id = gyro_state->gyro[chan->scan_index].report_id;
+             min = gyro_state->gyro[chan->scan_index].logical_minimum;
              address = gyro_3d_addresses[chan->scan_index];
              if (report_id >= 0)
                      *val = sensor_hub_input_attr_get_raw_value(
                                      gyro_state->common_attributes.hsdev,
                                      HID_USAGE_SENSOR_GYRO_3D, address,
                                      report_id,
-                                     SENSOR_HUB_SYNC);
+                                     SENSOR_HUB_SYNC,
+                                     min < 0);
              else {
                      *val = 0;
                      hid_sensor_power_state(&gyro_state->common_attributes,
drivers/iio/humidity/hid-sensor-humidity.c +2 -1
index beab6d6fd6e1..4bc95f31c730 100644
@@ -75,7 +75,8 @@ static int humidity_read_raw(struct iio_dev *indio_dev,
                              HID_USAGE_SENSOR_HUMIDITY,
                              HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY,
                              humid_st->humidity_attr.report_id,
-                             SENSOR_HUB_SYNC);
+                             SENSOR_HUB_SYNC,
+                             humid_st->humidity_attr.logical_minimum < 0);
              hid_sensor_power_state(&humid_st->common_attributes, false);
 
              return IIO_VAL_INT;
drivers/iio/light/hid-sensor-als.c +5 -3
index 406caaee9a3c..94f33250ba5a 100644
@@ -93,6 +93,7 @@ static int als_read_raw(struct iio_dev *indio_dev,
      int report_id = -1;
      u32 address;
      int ret_type;
+     s32 min;
 
      *val = 0;
      *val2 = 0;

@@ -102,8 +103,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
              case  CHANNEL_SCAN_INDEX_INTENSITY:
              case  CHANNEL_SCAN_INDEX_ILLUM:
                      report_id = als_state->als_illum.report_id;
-                     address =
-                     HID_USAGE_SENSOR_LIGHT_ILLUM;
+                     min = als_state->als_illum.logical_minimum;
+                     address = HID_USAGE_SENSOR_LIGHT_ILLUM;
                      break;
              default:
                      report_id = -1;

@@ -116,7 +117,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
                                      als_state->common_attributes.hsdev,
                                      HID_USAGE_SENSOR_ALS, address,
                                      report_id,
-                                     SENSOR_HUB_SYNC);
+                                     SENSOR_HUB_SYNC,
+                                     min < 0);
                      hid_sensor_power_state(&als_state->common_attributes,
                                              false);
              } else {
drivers/iio/light/hid-sensor-prox.c +5 -3
index 45107f7537b5..cf5a0c242609 100644
@@ -73,6 +73,7 @@ static int prox_read_raw(struct iio_dev *indio_dev,
      int report_id = -1;
      u32 address;
      int ret_type;
+     s32 min;
 
      *val = 0;
      *val2 = 0;

@@ -81,8 +82,8 @@ static int prox_read_raw(struct iio_dev *indio_dev,
              switch (chan->scan_index) {
              case  CHANNEL_SCAN_INDEX_PRESENCE:
                      report_id = prox_state->prox_attr.report_id;
-                     address =
-                     HID_USAGE_SENSOR_HUMAN_PRESENCE;
+                     min = prox_state->prox_attr.logical_minimum;
+                     address = HID_USAGE_SENSOR_HUMAN_PRESENCE;
                      break;
              default:
                      report_id = -1;

@@ -95,7 +96,8 @@ static int prox_read_raw(struct iio_dev *indio_dev,
                              prox_state->common_attributes.hsdev,
                              HID_USAGE_SENSOR_PROX, address,
                              report_id,
-                             SENSOR_HUB_SYNC);
+                             SENSOR_HUB_SYNC,
+                             min < 0);
                      hid_sensor_power_state(&prox_state->common_attributes,
                                              false);
              } else {
drivers/iio/magnetometer/hid-sensor-magn-3d.c +5 -3
index d55c4885211a..f3c0d41e5a8c 100644
@@ -163,21 +163,23 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev,
      int report_id = -1;
      u32 address;
      int ret_type;
+     s32 min;
 
      *val = 0;
      *val2 = 0;
      switch (mask) {
      case IIO_CHAN_INFO_RAW:
              hid_sensor_power_state(&magn_state->magn_flux_attributes, true);
-             report_id =
-                     magn_state->magn[chan->address].report_id;
+             report_id = magn_state->magn[chan->address].report_id;
+             min = magn_state->magn[chan->address].logical_minimum;
              address = magn_3d_addresses[chan->address];
              if (report_id >= 0)
                      *val = sensor_hub_input_attr_get_raw_value(
                              magn_state->magn_flux_attributes.hsdev,
                              HID_USAGE_SENSOR_COMPASS_3D, address,
                              report_id,
-                             SENSOR_HUB_SYNC);
+                             SENSOR_HUB_SYNC,
+                             min < 0);
              else {
                      *val = 0;
                      hid_sensor_power_state(
drivers/iio/orientation/hid-sensor-incl-3d.c +5 -3
index 1e5451d1ff88..bdc5e4554ee4 100644
@@ -111,21 +111,23 @@ static int incl_3d_read_raw(struct iio_dev *indio_dev,
      int report_id = -1;
      u32 address;
      int ret_type;
+     s32 min;
 
      *val = 0;
      *val2 = 0;
      switch (mask) {
      case IIO_CHAN_INFO_RAW:
              hid_sensor_power_state(&incl_state->common_attributes, true);
-             report_id =
-                     incl_state->incl[chan->scan_index].report_id;
+             report_id = incl_state->incl[chan->scan_index].report_id;
+             min = incl_state->incl[chan->scan_index].logical_minimum;
              address = incl_3d_addresses[chan->scan_index];
              if (report_id >= 0)
                      *val = sensor_hub_input_attr_get_raw_value(
                              incl_state->common_attributes.hsdev,
                              HID_USAGE_SENSOR_INCLINOMETER_3D, address,
                              report_id,
-                             SENSOR_HUB_SYNC);
+                             SENSOR_HUB_SYNC,
+                             min < 0);
              else {
                      hid_sensor_power_state(&incl_state->common_attributes,
                                              false);
drivers/iio/pressure/hid-sensor-press.c +5 -3
index 4c437918f1d2..d7b1c00ceb4d 100644
@@ -77,6 +77,7 @@ static int press_read_raw(struct iio_dev *indio_dev,
      int report_id = -1;
      u32 address;
      int ret_type;
+     s32 min;
 
      *val = 0;
      *val2 = 0;

@@ -85,8 +86,8 @@ static int press_read_raw(struct iio_dev *indio_dev,
              switch (chan->scan_index) {
              case  CHANNEL_SCAN_INDEX_PRESSURE:
                      report_id = press_state->press_attr.report_id;
-                     address =
-                     HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE;
+                     min = press_state->press_attr.logical_minimum;
+                     address = HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE;
                      break;
              default:
                      report_id = -1;

@@ -99,7 +100,8 @@ static int press_read_raw(struct iio_dev *indio_dev,
                              press_state->common_attributes.hsdev,
                              HID_USAGE_SENSOR_PRESSURE, address,
                              report_id,
-                             SENSOR_HUB_SYNC);
+                             SENSOR_HUB_SYNC,
+                             min < 0);
                      hid_sensor_power_state(&press_state->common_attributes,
                                              false);
              } else {
drivers/iio/temperature/hid-sensor-temperature.c +2 -1
index beaf6fd3e337..b592fc4f007e 100644
@@ -76,7 +76,8 @@ static int temperature_read_raw(struct iio_dev *indio_dev,
                      HID_USAGE_SENSOR_TEMPERATURE,
                      HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
                      temp_st->temperature_attr.report_id,
-                     SENSOR_HUB_SYNC);
+                     SENSOR_HUB_SYNC,
+                     temp_st->temperature_attr.logical_minimum < 0);
              hid_sensor_power_state(
                              &temp_st->common_attributes,
                              false);
drivers/rtc/rtc-hid-sensor-time.c +1 -1
index 2751dba850c6..3e1abb455472 100644
@@ -213,7 +213,7 @@ static int hid_rtc_read_time(struct device *dev, struct rtc_time *tm)
      /* get a report with all values through requesting one value */
      sensor_hub_input_attr_get_raw_value(time_state->common_attributes.hsdev,
                      HID_USAGE_SENSOR_TIME, hid_time_addresses[0],
-                     time_state->info[0].report_id, SENSOR_HUB_SYNC);
+                     time_state->info[0].report_id, SENSOR_HUB_SYNC, false);
      /* wait for all values (event) */
      ret = wait_for_completion_killable_timeout(
                      &time_state->comp_last_time, HZ*6);
include/linux/hid-sensor-hub.h +3 -1
index 331dc377c275..dc12f5c4b076 100644
@@ -177,6 +177,7 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
 * @attr_usage_id:    Attribute usage id as per spec
 * @report_id:        Report id to look for
 * @flag:      Synchronous or asynchronous read
+* @is_signed:   If true then fields < 32 bits will be sign-extended
 *
 * Issues a synchronous or asynchronous read request for an input attribute.
 * Returns data upto 32 bits.

@@ -190,7 +191,8 @@ enum sensor_hub_read_flags {
 int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
                                      u32 usage_id,
                                      u32 attr_usage_id, u32 report_id,
-                                     enum sensor_hub_read_flags flag
+                                     enum sensor_hub_read_flags flag,
+                                     bool is_signed
 );
 
 /**
maintainer
#5 lore raw
Quoted 22 lines · On Thu, 15 Nov 2018 09:20:23 +0100 Benjamin Tissoires…
Quoted 14 lines · Jonathan Cameron
Quoted 10 lines · On Wed, 31 Oct 2018 15:20:05 +0100 Hans de Goede <[email protected]> wrote:

Before this commit sensor_hub_input_attr_get_raw_value() failed to take the signedness of 16 and 8 bit values into account, returning e.g. 65436 instead of -100 for the z-axis reading of an accelerometer.

This commit adds a new is_signed parameter to the function and makes all callers pass the appropriate value for this.

While at it, this commit also fixes up some neighboring lines where statements were needlessly split over 2 lines to improve readability.

Quoted 6 lines
Quoted 2 lines
Acked-by Srinivas Pandruvada <[email protected]>
Signed-off-by Hans de Goede <[email protected]>

Yikes. Not sure how this was missed for so long.

I'll want the hid maintainers ack for this but probably makes sense to take it through IIO.

Quoted 6 lines

In case you are still waiting for an answer here, it's an ACK from me:

Acked-by Benjamin Tissoires <[email protected]>

Feel free to take it through your tree, I'll deal with the conflicts if there are any changes to hid-sensor-custom.c or hid-sensor-hub.c in this cycle.

Cool. Applied to the fixes-togreg branch of iio.git and marked for stable.

Thanks, all,

Jonathan

Quoted 364 lines

Cheers,
Benjamin

Quoted 360 lines

Is this a for all time thing or should we have an explict Fixes tag in place?

I do wonder if we have userspace code working around this which is going to get confused, but fingers crossed that's not the case.

Definitely stable material I think.

Jonathan

Quoted 348 lines

---
Changes in v2:

(quoted diff) +0 -3
-Adjust the kernel doc for the new is_signed parameter to
 sensor_hub_input_attr_get_raw_value()
-Add Srinivas' Acked-by
---
 drivers/hid/hid-sensor-custom.c                  |  2 +-
 drivers/hid/hid-sensor-hub.c                     | 13 ++++++++++---
 drivers/iio/accel/hid-sensor-accel-3d.c          |  5 ++++-
 drivers/iio/gyro/hid-sensor-gyro-3d.c            |  5 ++++-
 drivers/iio/humidity/hid-sensor-humidity.c       |  3 ++-
 drivers/iio/light/hid-sensor-als.c               |  8 +++++---
 drivers/iio/light/hid-sensor-prox.c              |  8 +++++---
 drivers/iio/magnetometer/hid-sensor-magn-3d.c    |  8 +++++---
 drivers/iio/orientation/hid-sensor-incl-3d.c     |  8 +++++---
 drivers/iio/pressure/hid-sensor-press.c          |  8 +++++---
 drivers/iio/temperature/hid-sensor-temperature.c |  3 ++-
 drivers/rtc/rtc-hid-sensor-time.c                |  2 +-
 include/linux/hid-sensor-hub.h                   |  4 +++-
 13 files changed, 52 insertions(+), 25 deletions(-)
drivers/hid/hid-sensor-custom.c +1 -1
index e8a114157f87..bb012bc032e0 100644
@@ -358,7 +358,7 @@ static ssize_t show_value(struct device *dev, struct device_attribute *attr,
                                              sensor_inst->hsdev,
                                              sensor_inst->hsdev->usage,
                                              usage, report_id,
-                                             SENSOR_HUB_SYNC);
+                                             SENSOR_HUB_SYNC, false);
      } else if (!strncmp(name, "units", strlen("units")))
              value = sensor_inst->fields[field_index].attribute.units;
      else if (!strncmp(name, "unit-expo", strlen("unit-expo")))
drivers/hid/hid-sensor-hub.c +10 -3
index 2b63487057c2..4256fdc5cd6d 100644
@@ -299,7 +299,8 @@ EXPORT_SYMBOL_GPL(sensor_hub_get_feature);
 int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
                                      u32 usage_id,
                                      u32 attr_usage_id, u32 report_id,
-                                     enum sensor_hub_read_flags flag)
+                                     enum sensor_hub_read_flags flag,
+                                     bool is_signed)
 {
      struct sensor_hub_data *data = hid_get_drvdata(hsdev->hdev);
      unsigned long flags;

@@ -331,10 +332,16 @@ int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
                                              &hsdev->pending.ready, HZ*5);
              switch (hsdev->pending.raw_size) {
              case 1:
-                     ret_val = *(u8 *)hsdev->pending.raw_data;
+                     if (is_signed)
+                             ret_val = *(s8 *)hsdev->pending.raw_data;
+                     else
+                             ret_val = *(u8 *)hsdev->pending.raw_data;
                      break;
              case 2:
-                     ret_val = *(u16 *)hsdev->pending.raw_data;
+                     if (is_signed)
+                             ret_val = *(s16 *)hsdev->pending.raw_data;
+                     else
+                             ret_val = *(u16 *)hsdev->pending.raw_data;
                      break;
              case 4:
                      ret_val = *(u32 *)hsdev->pending.raw_data;
drivers/iio/accel/hid-sensor-accel-3d.c +4 -1
index 41d97faf5013..38ff374a3ca4 100644
@@ -149,6 +149,7 @@ static int accel_3d_read_raw(struct iio_dev *indio_dev,
      int report_id = -1;
      u32 address;
      int ret_type;
+     s32 min;
      struct hid_sensor_hub_device *hsdev =
                                      accel_state->common_attributes.hsdev;
 

@@ -158,12 +159,14 @@ static int accel_3d_read_raw(struct iio_dev *indio_dev,
      case IIO_CHAN_INFO_RAW:
              hid_sensor_power_state(&accel_state->common_attributes, true);
              report_id = accel_state->accel[chan->scan_index].report_id;
+             min = accel_state->accel[chan->scan_index].logical_minimum;
              address = accel_3d_addresses[chan->scan_index];
              if (report_id >= 0)
                      *val = sensor_hub_input_attr_get_raw_value(
                                      accel_state->common_attributes.hsdev,
                                      hsdev->usage, address, report_id,
-                                     SENSOR_HUB_SYNC);
+                                     SENSOR_HUB_SYNC,
+                                     min < 0);
              else {
                      *val = 0;
                      hid_sensor_power_state(&accel_state->common_attributes,
drivers/iio/gyro/hid-sensor-gyro-3d.c +4 -1
index 36941e69f959..88e857c4baf4 100644
@@ -111,6 +111,7 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
      int report_id = -1;
      u32 address;
      int ret_type;
+     s32 min;
 
      *val = 0;
      *val2 = 0;

@@ -118,13 +119,15 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
      case IIO_CHAN_INFO_RAW:
              hid_sensor_power_state(&gyro_state->common_attributes, true);
              report_id = gyro_state->gyro[chan->scan_index].report_id;
+             min = gyro_state->gyro[chan->scan_index].logical_minimum;
              address = gyro_3d_addresses[chan->scan_index];
              if (report_id >= 0)
                      *val = sensor_hub_input_attr_get_raw_value(
                                      gyro_state->common_attributes.hsdev,
                                      HID_USAGE_SENSOR_GYRO_3D, address,
                                      report_id,
-                                     SENSOR_HUB_SYNC);
+                                     SENSOR_HUB_SYNC,
+                                     min < 0);
              else {
                      *val = 0;
                      hid_sensor_power_state(&gyro_state->common_attributes,
drivers/iio/humidity/hid-sensor-humidity.c +2 -1
index beab6d6fd6e1..4bc95f31c730 100644
@@ -75,7 +75,8 @@ static int humidity_read_raw(struct iio_dev *indio_dev,
                              HID_USAGE_SENSOR_HUMIDITY,
                              HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY,
                              humid_st->humidity_attr.report_id,
-                             SENSOR_HUB_SYNC);
+                             SENSOR_HUB_SYNC,
+                             humid_st->humidity_attr.logical_minimum < 0);
              hid_sensor_power_state(&humid_st->common_attributes, false);
 
              return IIO_VAL_INT;
drivers/iio/light/hid-sensor-als.c +5 -3
index 406caaee9a3c..94f33250ba5a 100644
@@ -93,6 +93,7 @@ static int als_read_raw(struct iio_dev *indio_dev,
      int report_id = -1;
      u32 address;
      int ret_type;
+     s32 min;
 
      *val = 0;
      *val2 = 0;

@@ -102,8 +103,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
              case  CHANNEL_SCAN_INDEX_INTENSITY:
              case  CHANNEL_SCAN_INDEX_ILLUM:
                      report_id = als_state->als_illum.report_id;
-                     address =
-                     HID_USAGE_SENSOR_LIGHT_ILLUM;
+                     min = als_state->als_illum.logical_minimum;
+                     address = HID_USAGE_SENSOR_LIGHT_ILLUM;
                      break;
              default:
                      report_id = -1;

@@ -116,7 +117,8 @@ static int als_read_raw(struct iio_dev *indio_dev,
                                      als_state->common_attributes.hsdev,
                                      HID_USAGE_SENSOR_ALS, address,
                                      report_id,
-                                     SENSOR_HUB_SYNC);
+                                     SENSOR_HUB_SYNC,
+                                     min < 0);
                      hid_sensor_power_state(&als_state->common_attributes,
                                              false);
              } else {
drivers/iio/light/hid-sensor-prox.c +5 -3
index 45107f7537b5..cf5a0c242609 100644
@@ -73,6 +73,7 @@ static int prox_read_raw(struct iio_dev *indio_dev,
      int report_id = -1;
      u32 address;
      int ret_type;
+     s32 min;
 
      *val = 0;
      *val2 = 0;

@@ -81,8 +82,8 @@ static int prox_read_raw(struct iio_dev *indio_dev,
              switch (chan->scan_index) {
              case  CHANNEL_SCAN_INDEX_PRESENCE:
                      report_id = prox_state->prox_attr.report_id;
-                     address =
-                     HID_USAGE_SENSOR_HUMAN_PRESENCE;
+                     min = prox_state->prox_attr.logical_minimum;
+                     address = HID_USAGE_SENSOR_HUMAN_PRESENCE;
                      break;
              default:
                      report_id = -1;

@@ -95,7 +96,8 @@ static int prox_read_raw(struct iio_dev *indio_dev,
                              prox_state->common_attributes.hsdev,
                              HID_USAGE_SENSOR_PROX, address,
                              report_id,
-                             SENSOR_HUB_SYNC);
+                             SENSOR_HUB_SYNC,
+                             min < 0);
                      hid_sensor_power_state(&prox_state->common_attributes,
                                              false);
              } else {
drivers/iio/magnetometer/hid-sensor-magn-3d.c +5 -3
index d55c4885211a..f3c0d41e5a8c 100644
@@ -163,21 +163,23 @@ static int magn_3d_read_raw(struct iio_dev *indio_dev,
      int report_id = -1;
      u32 address;
      int ret_type;
+     s32 min;
 
      *val = 0;
      *val2 = 0;
      switch (mask) {
      case IIO_CHAN_INFO_RAW:
              hid_sensor_power_state(&magn_state->magn_flux_attributes, true);
-             report_id =
-                     magn_state->magn[chan->address].report_id;
+             report_id = magn_state->magn[chan->address].report_id;
+             min = magn_state->magn[chan->address].logical_minimum;
              address = magn_3d_addresses[chan->address];
              if (report_id >= 0)
                      *val = sensor_hub_input_attr_get_raw_value(
                              magn_state->magn_flux_attributes.hsdev,
                              HID_USAGE_SENSOR_COMPASS_3D, address,
                              report_id,
-                             SENSOR_HUB_SYNC);
+                             SENSOR_HUB_SYNC,
+                             min < 0);
              else {
                      *val = 0;
                      hid_sensor_power_state(
drivers/iio/orientation/hid-sensor-incl-3d.c +5 -3
index 1e5451d1ff88..bdc5e4554ee4 100644
@@ -111,21 +111,23 @@ static int incl_3d_read_raw(struct iio_dev *indio_dev,
      int report_id = -1;
      u32 address;
      int ret_type;
+     s32 min;
 
      *val = 0;
      *val2 = 0;
      switch (mask) {
      case IIO_CHAN_INFO_RAW:
              hid_sensor_power_state(&incl_state->common_attributes, true);
-             report_id =
-                     incl_state->incl[chan->scan_index].report_id;
+             report_id = incl_state->incl[chan->scan_index].report_id;
+             min = incl_state->incl[chan->scan_index].logical_minimum;
              address = incl_3d_addresses[chan->scan_index];
              if (report_id >= 0)
                      *val = sensor_hub_input_attr_get_raw_value(
                              incl_state->common_attributes.hsdev,
                              HID_USAGE_SENSOR_INCLINOMETER_3D, address,
                              report_id,
-                             SENSOR_HUB_SYNC);
+                             SENSOR_HUB_SYNC,
+                             min < 0);
              else {
                      hid_sensor_power_state(&incl_state->common_attributes,
                                              false);
drivers/iio/pressure/hid-sensor-press.c +5 -3
index 4c437918f1d2..d7b1c00ceb4d 100644
@@ -77,6 +77,7 @@ static int press_read_raw(struct iio_dev *indio_dev,
      int report_id = -1;
      u32 address;
      int ret_type;
+     s32 min;
 
      *val = 0;
      *val2 = 0;

@@ -85,8 +86,8 @@ static int press_read_raw(struct iio_dev *indio_dev,
              switch (chan->scan_index) {
              case  CHANNEL_SCAN_INDEX_PRESSURE:
                      report_id = press_state->press_attr.report_id;
-                     address =
-                     HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE;
+                     min = press_state->press_attr.logical_minimum;
+                     address = HID_USAGE_SENSOR_ATMOSPHERIC_PRESSURE;
                      break;
              default:
                      report_id = -1;

@@ -99,7 +100,8 @@ static int press_read_raw(struct iio_dev *indio_dev,
                              press_state->common_attributes.hsdev,
                              HID_USAGE_SENSOR_PRESSURE, address,
                              report_id,
-                             SENSOR_HUB_SYNC);
+                             SENSOR_HUB_SYNC,
+                             min < 0);
                      hid_sensor_power_state(&press_state->common_attributes,
                                              false);
              } else {
drivers/iio/temperature/hid-sensor-temperature.c +2 -1
index beaf6fd3e337..b592fc4f007e 100644
@@ -76,7 +76,8 @@ static int temperature_read_raw(struct iio_dev *indio_dev,
                      HID_USAGE_SENSOR_TEMPERATURE,
                      HID_USAGE_SENSOR_DATA_ENVIRONMENTAL_TEMPERATURE,
                      temp_st->temperature_attr.report_id,
-                     SENSOR_HUB_SYNC);
+                     SENSOR_HUB_SYNC,
+                     temp_st->temperature_attr.logical_minimum < 0);
              hid_sensor_power_state(
                              &temp_st->common_attributes,
                              false);
drivers/rtc/rtc-hid-sensor-time.c +1 -1
index 2751dba850c6..3e1abb455472 100644
@@ -213,7 +213,7 @@ static int hid_rtc_read_time(struct device *dev, struct rtc_time *tm)
      /* get a report with all values through requesting one value */
      sensor_hub_input_attr_get_raw_value(time_state->common_attributes.hsdev,
                      HID_USAGE_SENSOR_TIME, hid_time_addresses[0],
-                     time_state->info[0].report_id, SENSOR_HUB_SYNC);
+                     time_state->info[0].report_id, SENSOR_HUB_SYNC, false);
      /* wait for all values (event) */
      ret = wait_for_completion_killable_timeout(
                      &time_state->comp_last_time, HZ*6);
include/linux/hid-sensor-hub.h +3 -1
index 331dc377c275..dc12f5c4b076 100644
@@ -177,6 +177,7 @@ int sensor_hub_input_get_attribute_info(struct hid_sensor_hub_device *hsdev,
 * @attr_usage_id:    Attribute usage id as per spec
 * @report_id:        Report id to look for
 * @flag:      Synchronous or asynchronous read
+* @is_signed:   If true then fields < 32 bits will be sign-extended
 *
 * Issues a synchronous or asynchronous read request for an input attribute.
 * Returns data upto 32 bits.

@@ -190,7 +191,8 @@ enum sensor_hub_read_flags {
 int sensor_hub_input_attr_get_raw_value(struct hid_sensor_hub_device *hsdev,
                                      u32 usage_id,
                                      u32 attr_usage_id, u32 report_id,
-                                     enum sensor_hub_read_flags flag
+                                     enum sensor_hub_read_flags flag,
+                                     bool is_signed
 );
 
 /**
#6 lore raw
In reply to Hans de Goede
Quoted 4 lines · Hans de Goede

Before this commit sensor_hub_input_attr_get_raw_value() failed to take
the signedness of 16 and 8 bit values into account, returning e.g. 65436 instead of -100 for the z-axis reading of an accelerometer.

Anything I need to change in iio-sensor-proxy for this?

author
#7 lore raw

Hi,

Quoted 7 lines · Bastien Nocera
Quoted 4 lines · Hans de Goede

Before this commit sensor_hub_input_attr_get_raw_value() failed to take
the signedness of 16 and 8 bit values into account, returning e.g. 65436 instead of -100 for the z-axis reading of an accelerometer.

Anything I need to change in iio-sensor-proxy for this?

No, AFAICT all hid sensors offer the buffered API, so iio-sensor-proxy will prefer that. This is also likely the cause of why this was not caught earlier.

Regards,

Hans