On 11/21/2013 12:06 PM, Jonathan Cameron wrote:
Jiri Kosina <jkosina@xxxxxxx> wrote:
On Tue, 15 Oct 2013, Jonathan Cameron wrote:
In the original HID sensor specifications all Named array enums
stated to be 0-based. But the most commercial devices
implemented
as 1-based, because of the implementation by one of the major
OS
vendor. To fix this we added a quirk, which required module to
be
recompiled. Instead now added a module parameter, so that it
can
be switched at runtime. By default it will be 1-based to be
compatible with majority of devices. This is true for both
power
and report state usage id for hid sensors. Also added defines
for
power on values for D0 to D4 and using it for clarity.
Signed-off-by: Srinivas Pandruvada
<srinivas.pandruvada@xxxxxxxxxxxxxxx>
This looks fine but raises a few questions...
What other options do we have for controlling this? Are hid
sensors
chips identifiable? If so can we have a list of chips where it
is 0 in
the driver. The module parameter might still be needed to deal
with new
devices but would be nice to have most work out of the box.
We could have quirk based on vendor id/pid. But the problem is
that once they update FW to be compliant with WIN8, it
will not work.
So there is no way to distinguish. Since sensor hub is present in
most of WIN8 convertible devices, the quirk became a
new normal.
Yuck. You have my sympathies!
Got to love it when the quirk becomes the default ;)
Unfortunately they don't go back to specification update after
such change.
Just to check, is this quirk only hid-sensors related? Named
arrays are
as far as I can see a part of HID in general.
Currently I have information about HID sensors implementation only
as they are in new in the WIN8. I didn't hear any
complaint about any other HID devices.
Jiri, have you seen anything similar to this before.
I am just trying to work out if it should be a Hid quirk or should be
handled only in the hid-sensors module.
Sorry for super-late response from me, I have been drowned in other
things.
This looks indeed pretty bad. I don't have a strong objection to having
this as a generic HID quirk; the argument being, if this was made
de-facto
Win8-mandated standard, odds are that we'd start seeing a lot of
devices
with this behavior.
So if you decide to take this way, please send the patch to me, I'll
apply
it.
Thanks for the reply Jiri. That was pretty much what I was expecting.
Srinivas, please rework this as a generic his quirk.
The problem I have is identify NAry data if we implement as a common hid
quirk. If I can identify, then in hid_set_field,
I can increment the requested value by 1 if the hid_device->quirk is set
for enum base quirk.
But from the report descriptor there is no way to tell it is a named
array. For example for the following
report descriptor for setting power state, which by spec is a named
array with selectors.
From report descriptor:
HID_USAGE_SENSOR_PROPERTY_POWER_STATE, // NAry
HID_LOGICAL_MIN_8(0),
HID_LOGICAL_MAX_8(5),
HID_REPORT_SIZE(8),
HID_REPORT_COUNT(1),
HID_COLLECTION(Logical),
HID_USAGE_SENSOR_PROPERTY_POWER_STATE_UNDEFINED,
HID_USAGE_SENSOR_PROPERTY_POWER_STATE_D0_FULL_POWER,
HID_USAGE_SENSOR_PROPERTY_POWER_STATE_D1_LOW_POWER,
HID_USAGE_SENSOR_PROPERTY_POWER_STATE_D2_STANDBY_WITH_WAKE,
HID_USAGE_SENSOR_PROPERTY_POWER_STATE_D3_SLEEP_WITH_WAKE,
HID_USAGE_SENSOR_PROPERTY_POWER_STATE_D4_POWER_OFF,
HID_FEATURE(Data_Arr_Abs),
HID_END_COLLECTION,
The HID core spec allows only collection to be typed as named arrays.
But as
you can see the report descriptor still names this as COLLECTION(Logical).
If it is collection defined as HID_COLLECTION(NAMED_ARRAY), it is very
easy to implement a
quirk at HID level. Unfortunately the logical minimum and maximum also
don't really
show correct values accommodating the enum base.
One way to do this is by adding a quirk field in hid_field structure
also. This way, quirk can be
enabled on a per field basis. Then we can have a common hid quirk at a
device level and also individual field level.
Jiri, Do you have some suggestion?
Thanks,
Srinivas
Thanks,
--
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
