| Message ID | 20181031142005.15802-1-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] iio/hid-sensors: Fix IIO_CHAN_INFO_RAW returning wrong values for signed numbers | expand |
Hi, On 31-10-18 15:20, Hans de Goede 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. > > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > 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 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 > --- > 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(-) > > diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c > index e8a114157f87..bb012bc032e0 100644 > --- a/drivers/hid/hid-sensor-custom.c > +++ b/drivers/hid/hid-sensor-custom.c > @@ -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"))) > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c > index 2b63487057c2..4256fdc5cd6d 100644 > --- a/drivers/hid/hid-sensor-hub.c > +++ b/drivers/hid/hid-sensor-hub.c > @@ -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; > diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c > index 41d97faf5013..38ff374a3ca4 100644 > --- a/drivers/iio/accel/hid-sensor-accel-3d.c > +++ b/drivers/iio/accel/hid-sensor-accel-3d.c > @@ -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, > diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c > index 36941e69f959..88e857c4baf4 100644 > --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c > +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c > @@ -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, > diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c > index beab6d6fd6e1..4bc95f31c730 100644 > --- a/drivers/iio/humidity/hid-sensor-humidity.c > +++ b/drivers/iio/humidity/hid-sensor-humidity.c > @@ -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; > diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c > index 406caaee9a3c..94f33250ba5a 100644 > --- a/drivers/iio/light/hid-sensor-als.c > +++ b/drivers/iio/light/hid-sensor-als.c > @@ -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 { > diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c > index 45107f7537b5..cf5a0c242609 100644 > --- a/drivers/iio/light/hid-sensor-prox.c > +++ b/drivers/iio/light/hid-sensor-prox.c > @@ -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 { > diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > index d55c4885211a..f3c0d41e5a8c 100644 > --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c > +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > @@ -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( > diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c > index 1e5451d1ff88..bdc5e4554ee4 100644 > --- a/drivers/iio/orientation/hid-sensor-incl-3d.c > +++ b/drivers/iio/orientation/hid-sensor-incl-3d.c > @@ -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); > diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c > index 4c437918f1d2..d7b1c00ceb4d 100644 > --- a/drivers/iio/pressure/hid-sensor-press.c > +++ b/drivers/iio/pressure/hid-sensor-press.c > @@ -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 { > diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c > index beaf6fd3e337..b592fc4f007e 100644 > --- a/drivers/iio/temperature/hid-sensor-temperature.c > +++ b/drivers/iio/temperature/hid-sensor-temperature.c > @@ -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); > diff --git a/drivers/rtc/rtc-hid-sensor-time.c b/drivers/rtc/rtc-hid-sensor-time.c > index 2751dba850c6..3e1abb455472 100644 > --- a/drivers/rtc/rtc-hid-sensor-time.c > +++ b/drivers/rtc/rtc-hid-sensor-time.c > @@ -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); > diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h > index 331dc377c275..dc12f5c4b076 100644 > --- a/include/linux/hid-sensor-hub.h > +++ b/include/linux/hid-sensor-hub.h > @@ -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 > ); > > /** >
On Wed, 31 Oct 2018 15:20:05 +0100 Hans de Goede <hdegoede@redhat.com> 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. > > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> 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 > --- > 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(-) > > diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c > index e8a114157f87..bb012bc032e0 100644 > --- a/drivers/hid/hid-sensor-custom.c > +++ b/drivers/hid/hid-sensor-custom.c > @@ -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"))) > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c > index 2b63487057c2..4256fdc5cd6d 100644 > --- a/drivers/hid/hid-sensor-hub.c > +++ b/drivers/hid/hid-sensor-hub.c > @@ -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; > diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c > index 41d97faf5013..38ff374a3ca4 100644 > --- a/drivers/iio/accel/hid-sensor-accel-3d.c > +++ b/drivers/iio/accel/hid-sensor-accel-3d.c > @@ -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, > diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c > index 36941e69f959..88e857c4baf4 100644 > --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c > +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c > @@ -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, > diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c > index beab6d6fd6e1..4bc95f31c730 100644 > --- a/drivers/iio/humidity/hid-sensor-humidity.c > +++ b/drivers/iio/humidity/hid-sensor-humidity.c > @@ -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; > diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c > index 406caaee9a3c..94f33250ba5a 100644 > --- a/drivers/iio/light/hid-sensor-als.c > +++ b/drivers/iio/light/hid-sensor-als.c > @@ -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 { > diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c > index 45107f7537b5..cf5a0c242609 100644 > --- a/drivers/iio/light/hid-sensor-prox.c > +++ b/drivers/iio/light/hid-sensor-prox.c > @@ -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 { > diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > index d55c4885211a..f3c0d41e5a8c 100644 > --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c > +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > @@ -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( > diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c > index 1e5451d1ff88..bdc5e4554ee4 100644 > --- a/drivers/iio/orientation/hid-sensor-incl-3d.c > +++ b/drivers/iio/orientation/hid-sensor-incl-3d.c > @@ -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); > diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c > index 4c437918f1d2..d7b1c00ceb4d 100644 > --- a/drivers/iio/pressure/hid-sensor-press.c > +++ b/drivers/iio/pressure/hid-sensor-press.c > @@ -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 { > diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c > index beaf6fd3e337..b592fc4f007e 100644 > --- a/drivers/iio/temperature/hid-sensor-temperature.c > +++ b/drivers/iio/temperature/hid-sensor-temperature.c > @@ -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); > diff --git a/drivers/rtc/rtc-hid-sensor-time.c b/drivers/rtc/rtc-hid-sensor-time.c > index 2751dba850c6..3e1abb455472 100644 > --- a/drivers/rtc/rtc-hid-sensor-time.c > +++ b/drivers/rtc/rtc-hid-sensor-time.c > @@ -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); > diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h > index 331dc377c275..dc12f5c4b076 100644 > --- a/include/linux/hid-sensor-hub.h > +++ b/include/linux/hid-sensor-hub.h > @@ -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 > ); > > /**
On Sat, 2018-11-03 at 12:26 +0000, Jonathan Cameron wrote: > On Wed, 31 Oct 2018 15:20:05 +0100 > Hans de Goede <hdegoede@redhat.com> 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. > > > > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > 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. > > 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. > > 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 > > Jonathan > > > --- > > 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(-) > > > > diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid- > > sensor-custom.c > > index e8a114157f87..bb012bc032e0 100644 > > --- a/drivers/hid/hid-sensor-custom.c > > +++ b/drivers/hid/hid-sensor-custom.c > > @@ -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"))) > > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor- > > hub.c > > index 2b63487057c2..4256fdc5cd6d 100644 > > --- a/drivers/hid/hid-sensor-hub.c > > +++ b/drivers/hid/hid-sensor-hub.c > > @@ -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; > > diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c > > b/drivers/iio/accel/hid-sensor-accel-3d.c > > index 41d97faf5013..38ff374a3ca4 100644 > > --- a/drivers/iio/accel/hid-sensor-accel-3d.c > > +++ b/drivers/iio/accel/hid-sensor-accel-3d.c > > @@ -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, > > diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c > > b/drivers/iio/gyro/hid-sensor-gyro-3d.c > > index 36941e69f959..88e857c4baf4 100644 > > --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c > > +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c > > @@ -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, > > diff --git a/drivers/iio/humidity/hid-sensor-humidity.c > > b/drivers/iio/humidity/hid-sensor-humidity.c > > index beab6d6fd6e1..4bc95f31c730 100644 > > --- a/drivers/iio/humidity/hid-sensor-humidity.c > > +++ b/drivers/iio/humidity/hid-sensor-humidity.c > > @@ -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; > > diff --git a/drivers/iio/light/hid-sensor-als.c > > b/drivers/iio/light/hid-sensor-als.c > > index 406caaee9a3c..94f33250ba5a 100644 > > --- a/drivers/iio/light/hid-sensor-als.c > > +++ b/drivers/iio/light/hid-sensor-als.c > > @@ -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 { > > diff --git a/drivers/iio/light/hid-sensor-prox.c > > b/drivers/iio/light/hid-sensor-prox.c > > index 45107f7537b5..cf5a0c242609 100644 > > --- a/drivers/iio/light/hid-sensor-prox.c > > +++ b/drivers/iio/light/hid-sensor-prox.c > > @@ -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 { > > diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c > > b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > > index d55c4885211a..f3c0d41e5a8c 100644 > > --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c > > +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > > @@ -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( > > diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c > > b/drivers/iio/orientation/hid-sensor-incl-3d.c > > index 1e5451d1ff88..bdc5e4554ee4 100644 > > --- a/drivers/iio/orientation/hid-sensor-incl-3d.c > > +++ b/drivers/iio/orientation/hid-sensor-incl-3d.c > > @@ -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); > > diff --git a/drivers/iio/pressure/hid-sensor-press.c > > b/drivers/iio/pressure/hid-sensor-press.c > > index 4c437918f1d2..d7b1c00ceb4d 100644 > > --- a/drivers/iio/pressure/hid-sensor-press.c > > +++ b/drivers/iio/pressure/hid-sensor-press.c > > @@ -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 { > > diff --git a/drivers/iio/temperature/hid-sensor-temperature.c > > b/drivers/iio/temperature/hid-sensor-temperature.c > > index beaf6fd3e337..b592fc4f007e 100644 > > --- a/drivers/iio/temperature/hid-sensor-temperature.c > > +++ b/drivers/iio/temperature/hid-sensor-temperature.c > > @@ -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); > > diff --git a/drivers/rtc/rtc-hid-sensor-time.c b/drivers/rtc/rtc- > > hid-sensor-time.c > > index 2751dba850c6..3e1abb455472 100644 > > --- a/drivers/rtc/rtc-hid-sensor-time.c > > +++ b/drivers/rtc/rtc-hid-sensor-time.c > > @@ -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); > > diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid- > > sensor-hub.h > > index 331dc377c275..dc12f5c4b076 100644 > > --- a/include/linux/hid-sensor-hub.h > > +++ b/include/linux/hid-sensor-hub.h > > @@ -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 > > ); > > > > /** > >
On Wed, 2018-10-31 at 15:20 +0100, Hans de Goede 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. Anything I need to change in iio-sensor-proxy for this?
Hi, On 13-11-18 14:09, Bastien Nocera wrote: > On Wed, 2018-10-31 at 15:20 +0100, Hans de Goede 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. > > 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
On Sat, Nov 3, 2018 at 1:26 PM Jonathan Cameron <jic23@kernel.org> wrote: > > On Wed, 31 Oct 2018 15:20:05 +0100 > Hans de Goede <hdegoede@redhat.com> 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. > > > > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > 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 <benjamin.tissoires@redhat.com> 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 > > 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 > > > --- > > 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(-) > > > > diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c > > index e8a114157f87..bb012bc032e0 100644 > > --- a/drivers/hid/hid-sensor-custom.c > > +++ b/drivers/hid/hid-sensor-custom.c > > @@ -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"))) > > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c > > index 2b63487057c2..4256fdc5cd6d 100644 > > --- a/drivers/hid/hid-sensor-hub.c > > +++ b/drivers/hid/hid-sensor-hub.c > > @@ -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; > > diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c > > index 41d97faf5013..38ff374a3ca4 100644 > > --- a/drivers/iio/accel/hid-sensor-accel-3d.c > > +++ b/drivers/iio/accel/hid-sensor-accel-3d.c > > @@ -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, > > diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c > > index 36941e69f959..88e857c4baf4 100644 > > --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c > > +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c > > @@ -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, > > diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c > > index beab6d6fd6e1..4bc95f31c730 100644 > > --- a/drivers/iio/humidity/hid-sensor-humidity.c > > +++ b/drivers/iio/humidity/hid-sensor-humidity.c > > @@ -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; > > diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c > > index 406caaee9a3c..94f33250ba5a 100644 > > --- a/drivers/iio/light/hid-sensor-als.c > > +++ b/drivers/iio/light/hid-sensor-als.c > > @@ -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 { > > diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c > > index 45107f7537b5..cf5a0c242609 100644 > > --- a/drivers/iio/light/hid-sensor-prox.c > > +++ b/drivers/iio/light/hid-sensor-prox.c > > @@ -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 { > > diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > > index d55c4885211a..f3c0d41e5a8c 100644 > > --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c > > +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > > @@ -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( > > diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c > > index 1e5451d1ff88..bdc5e4554ee4 100644 > > --- a/drivers/iio/orientation/hid-sensor-incl-3d.c > > +++ b/drivers/iio/orientation/hid-sensor-incl-3d.c > > @@ -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); > > diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c > > index 4c437918f1d2..d7b1c00ceb4d 100644 > > --- a/drivers/iio/pressure/hid-sensor-press.c > > +++ b/drivers/iio/pressure/hid-sensor-press.c > > @@ -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 { > > diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c > > index beaf6fd3e337..b592fc4f007e 100644 > > --- a/drivers/iio/temperature/hid-sensor-temperature.c > > +++ b/drivers/iio/temperature/hid-sensor-temperature.c > > @@ -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); > > diff --git a/drivers/rtc/rtc-hid-sensor-time.c b/drivers/rtc/rtc-hid-sensor-time.c > > index 2751dba850c6..3e1abb455472 100644 > > --- a/drivers/rtc/rtc-hid-sensor-time.c > > +++ b/drivers/rtc/rtc-hid-sensor-time.c > > @@ -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); > > diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h > > index 331dc377c275..dc12f5c4b076 100644 > > --- a/include/linux/hid-sensor-hub.h > > +++ b/include/linux/hid-sensor-hub.h > > @@ -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 > > ); > > > > /** >
On Thu, 15 Nov 2018 09:20:23 +0100 Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > On Sat, Nov 3, 2018 at 1:26 PM Jonathan Cameron <jic23@kernel.org> wrote: > > > > On Wed, 31 Oct 2018 15:20:05 +0100 > > Hans de Goede <hdegoede@redhat.com> 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. > > > > > > Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > > 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 <benjamin.tissoires@redhat.com> > > 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 > > Cheers, > Benjamin > > > > > 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 > > > > > --- > > > 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(-) > > > > > > diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c > > > index e8a114157f87..bb012bc032e0 100644 > > > --- a/drivers/hid/hid-sensor-custom.c > > > +++ b/drivers/hid/hid-sensor-custom.c > > > @@ -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"))) > > > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c > > > index 2b63487057c2..4256fdc5cd6d 100644 > > > --- a/drivers/hid/hid-sensor-hub.c > > > +++ b/drivers/hid/hid-sensor-hub.c > > > @@ -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; > > > diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c > > > index 41d97faf5013..38ff374a3ca4 100644 > > > --- a/drivers/iio/accel/hid-sensor-accel-3d.c > > > +++ b/drivers/iio/accel/hid-sensor-accel-3d.c > > > @@ -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, > > > diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c > > > index 36941e69f959..88e857c4baf4 100644 > > > --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c > > > +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c > > > @@ -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, > > > diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c > > > index beab6d6fd6e1..4bc95f31c730 100644 > > > --- a/drivers/iio/humidity/hid-sensor-humidity.c > > > +++ b/drivers/iio/humidity/hid-sensor-humidity.c > > > @@ -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; > > > diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c > > > index 406caaee9a3c..94f33250ba5a 100644 > > > --- a/drivers/iio/light/hid-sensor-als.c > > > +++ b/drivers/iio/light/hid-sensor-als.c > > > @@ -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 { > > > diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c > > > index 45107f7537b5..cf5a0c242609 100644 > > > --- a/drivers/iio/light/hid-sensor-prox.c > > > +++ b/drivers/iio/light/hid-sensor-prox.c > > > @@ -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 { > > > diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > > > index d55c4885211a..f3c0d41e5a8c 100644 > > > --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c > > > +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c > > > @@ -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( > > > diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c > > > index 1e5451d1ff88..bdc5e4554ee4 100644 > > > --- a/drivers/iio/orientation/hid-sensor-incl-3d.c > > > +++ b/drivers/iio/orientation/hid-sensor-incl-3d.c > > > @@ -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); > > > diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c > > > index 4c437918f1d2..d7b1c00ceb4d 100644 > > > --- a/drivers/iio/pressure/hid-sensor-press.c > > > +++ b/drivers/iio/pressure/hid-sensor-press.c > > > @@ -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 { > > > diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c > > > index beaf6fd3e337..b592fc4f007e 100644 > > > --- a/drivers/iio/temperature/hid-sensor-temperature.c > > > +++ b/drivers/iio/temperature/hid-sensor-temperature.c > > > @@ -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); > > > diff --git a/drivers/rtc/rtc-hid-sensor-time.c b/drivers/rtc/rtc-hid-sensor-time.c > > > index 2751dba850c6..3e1abb455472 100644 > > > --- a/drivers/rtc/rtc-hid-sensor-time.c > > > +++ b/drivers/rtc/rtc-hid-sensor-time.c > > > @@ -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); > > > diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h > > > index 331dc377c275..dc12f5c4b076 100644 > > > --- a/include/linux/hid-sensor-hub.h > > > +++ b/include/linux/hid-sensor-hub.h > > > @@ -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 > > > ); > > > > > > /** > >
diff --git a/drivers/hid/hid-sensor-custom.c b/drivers/hid/hid-sensor-custom.c index e8a114157f87..bb012bc032e0 100644 --- a/drivers/hid/hid-sensor-custom.c +++ b/drivers/hid/hid-sensor-custom.c @@ -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"))) diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c index 2b63487057c2..4256fdc5cd6d 100644 --- a/drivers/hid/hid-sensor-hub.c +++ b/drivers/hid/hid-sensor-hub.c @@ -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; diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c index 41d97faf5013..38ff374a3ca4 100644 --- a/drivers/iio/accel/hid-sensor-accel-3d.c +++ b/drivers/iio/accel/hid-sensor-accel-3d.c @@ -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, diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c index 36941e69f959..88e857c4baf4 100644 --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c @@ -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, diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c index beab6d6fd6e1..4bc95f31c730 100644 --- a/drivers/iio/humidity/hid-sensor-humidity.c +++ b/drivers/iio/humidity/hid-sensor-humidity.c @@ -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; diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c index 406caaee9a3c..94f33250ba5a 100644 --- a/drivers/iio/light/hid-sensor-als.c +++ b/drivers/iio/light/hid-sensor-als.c @@ -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 { diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c index 45107f7537b5..cf5a0c242609 100644 --- a/drivers/iio/light/hid-sensor-prox.c +++ b/drivers/iio/light/hid-sensor-prox.c @@ -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 { diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c index d55c4885211a..f3c0d41e5a8c 100644 --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c @@ -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( diff --git a/drivers/iio/orientation/hid-sensor-incl-3d.c b/drivers/iio/orientation/hid-sensor-incl-3d.c index 1e5451d1ff88..bdc5e4554ee4 100644 --- a/drivers/iio/orientation/hid-sensor-incl-3d.c +++ b/drivers/iio/orientation/hid-sensor-incl-3d.c @@ -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); diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c index 4c437918f1d2..d7b1c00ceb4d 100644 --- a/drivers/iio/pressure/hid-sensor-press.c +++ b/drivers/iio/pressure/hid-sensor-press.c @@ -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 { diff --git a/drivers/iio/temperature/hid-sensor-temperature.c b/drivers/iio/temperature/hid-sensor-temperature.c index beaf6fd3e337..b592fc4f007e 100644 --- a/drivers/iio/temperature/hid-sensor-temperature.c +++ b/drivers/iio/temperature/hid-sensor-temperature.c @@ -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); diff --git a/drivers/rtc/rtc-hid-sensor-time.c b/drivers/rtc/rtc-hid-sensor-time.c index 2751dba850c6..3e1abb455472 100644 --- a/drivers/rtc/rtc-hid-sensor-time.c +++ b/drivers/rtc/rtc-hid-sensor-time.c @@ -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); diff --git a/include/linux/hid-sensor-hub.h b/include/linux/hid-sensor-hub.h index 331dc377c275..dc12f5c4b076 100644 --- a/include/linux/hid-sensor-hub.h +++ b/include/linux/hid-sensor-hub.h @@ -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 ); /**