On 11/27/13 22:19, Srinivas Pandruvada wrote: > In the original HID sensor hub firmwares all Named array enums were > to 0-based. But the most recent hub implemented as 1-based, > because of the implementation by one of the major OS vendor. > Using logical minimum for the field as the base of enum. So we add > logical minimum to the selector values before setting those fields. > Some sensor hub FWs already changed logical minimum from 0 to 1 > to reflect this and hope every other vendor will follow. > There is no easy way to add a common HID quirk for NAry elements, > even if the standard specifies these field as NAry, the collection > used to describe selectors is still just "logical". > > Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> Looks like a good solution to me. This one is a little interesting. Technically I 'believe' we don't have a bug as it is possible to make these devices work via the kconfig option and it definitely isn't a regression. As such I have applied this to the branch intended for the next kernel cycled (togreg) rather than to the fixes branch. If people have a very strong feeling about this then shout reasonably quickly - I'll probably hold off sending that branch to Greg for a few days anyway. I've cc'd GregKH to see if he has any input on the path this should take. Jonathan > --- > drivers/iio/common/hid-sensors/Kconfig | 9 --------- > drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 20 +++++++++++++++----- > include/linux/hid-sensor-ids.h | 12 ++++++++++++ > 3 files changed, 27 insertions(+), 14 deletions(-) > > diff --git a/drivers/iio/common/hid-sensors/Kconfig b/drivers/iio/common/hid-sensors/Kconfig > index 1178121..39188b7 100644 > --- a/drivers/iio/common/hid-sensors/Kconfig > +++ b/drivers/iio/common/hid-sensors/Kconfig > @@ -25,13 +25,4 @@ config HID_SENSOR_IIO_TRIGGER > If this driver is compiled as a module, it will be named > hid-sensor-trigger. > > -config HID_SENSOR_ENUM_BASE_QUIRKS > - bool "ENUM base quirks for HID Sensor IIO drivers" > - depends on HID_SENSOR_IIO_COMMON > - help > - Say yes here to build support for sensor hub FW using > - enumeration, which is using 1 as base instead of 0. > - Since logical minimum is still set 0 instead of 1, > - there is no easy way to differentiate. > - > endmenu > diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c > index b6e77e0..781d3fa 100644 > --- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c > +++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c > @@ -33,24 +33,34 @@ static int hid_sensor_data_rdy_trigger_set_state(struct iio_trigger *trig, > { > struct hid_sensor_common *st = iio_trigger_get_drvdata(trig); > int state_val; > + int report_val; > > if (state) { > if (sensor_hub_device_open(st->hsdev)) > return -EIO; > - } else > + state_val = > + HID_USAGE_SENSOR_PROP_POWER_STATE_D0_FULL_POWER_ENUM; > + report_val = > + HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EVENTS_ENUM; > + > + } else { > sensor_hub_device_close(st->hsdev); > + state_val = > + HID_USAGE_SENSOR_PROP_POWER_STATE_D4_POWER_OFF_ENUM; > + report_val = > + HID_USAGE_SENSOR_PROP_REPORTING_STATE_NO_EVENTS_ENUM; > + } > > - state_val = state ? 1 : 0; > - if (IS_ENABLED(CONFIG_HID_SENSOR_ENUM_BASE_QUIRKS)) > - ++state_val; > st->data_ready = state; > + state_val += st->power_state.logical_minimum; > + report_val += st->report_state.logical_minimum; > sensor_hub_set_feature(st->hsdev, st->power_state.report_id, > st->power_state.index, > (s32)state_val); > > sensor_hub_set_feature(st->hsdev, st->report_state.report_id, > st->report_state.index, > - (s32)state_val); > + (s32)report_val); > > return 0; > } > diff --git a/include/linux/hid-sensor-ids.h b/include/linux/hid-sensor-ids.h > index 4f945d3..8323775 100644 > --- a/include/linux/hid-sensor-ids.h > +++ b/include/linux/hid-sensor-ids.h > @@ -117,4 +117,16 @@ > #define HID_USAGE_SENSOR_PROP_REPORT_STATE 0x200316 > #define HID_USAGE_SENSOR_PROY_POWER_STATE 0x200319 > > +/* Power state enumerations */ > +#define HID_USAGE_SENSOR_PROP_POWER_STATE_UNDEFINED_ENUM 0x00 > +#define HID_USAGE_SENSOR_PROP_POWER_STATE_D0_FULL_POWER_ENUM 0x01 > +#define HID_USAGE_SENSOR_PROP_POWER_STATE_D1_LOW_POWER_ENUM 0x02 > +#define HID_USAGE_SENSOR_PROP_POWER_STATE_D2_STANDBY_WITH_WAKE_ENUM 0x03 > +#define HID_USAGE_SENSOR_PROP_POWER_STATE_D3_SLEEP_WITH_WAKE_ENUM 0x04 > +#define HID_USAGE_SENSOR_PROP_POWER_STATE_D4_POWER_OFF_ENUM 0x05 > + > +/* Report State enumerations */ > +#define HID_USAGE_SENSOR_PROP_REPORTING_STATE_NO_EVENTS_ENUM 0x00 > +#define HID_USAGE_SENSOR_PROP_REPORTING_STATE_ALL_EVENTS_ENUM 0x01 > + > #endif > -- To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html