On Mon, Mar 06, 2017 at 10:04:50AM +0100, Benjamin Tissoires wrote: > Well, the HID spec is not very clear, that the least we can agree on :) > > But Microsoft's interpretation is rather clear in the > multitouch/touchpad/pen specification: > https://msdn.microsoft.com/en-us/library/windows/hardware/dn672278(v=vs.85).aspx > paragraph "Required HID usages for pen digitizers": > > "It should be noted that the host will recognize the values outside the > logical range as signifying the implementation of this protocol only if > the report descriptor specifically includes the bit signifying the fact > that X and Y support NULL states. Otherwise, values outside the logical > range are simply moved to the nearest boundary value." > > And the 2 report descriptors written after are correct concerning the > NULL state bit. > > For Microsoft (in the pointer delivery protocol): > - NULL state -> ignore out of range values > - No NULL State -> clamp at the nearest boundary. > > Following this would solve both issues If I understand correctly. Your > controller would be clamped to [-1..1] (No Null State), and the ones > that need to be ignored (like the ones from Denilson will be thanks to > the NULL state bit set. The clamping behaviour is the best of both worlds, it still matches how I interpret the spec and provides a concrete definition of what should happen when an out of bounds value is reported with the "No Null Position | Null State" bit unset. However, currently we just let the value pass through unchanged. So I propose another patch on top of this one (at the bottom of the email, done against for-4.12/hid-core-null-state-handling). The original change which ignores the out of range value does a dbg_hid, I'm not sure if that's necessary for the clamping scenario. I'll leave a few days for any comments and if testing goes well (I don't see why not) I'll post it on here as a patch. I'm not sure if that would be a v3 or a new patch. > Maybe we can follow this to say we are mimicking Microsoft's driver and > hope for the best? Their approach to the ambiguity takes the safest bet and compatibility with them might not be a bad idea anyway so I agree with following their interpretation. I will note a link to that website in my commit message. -- Tomasz Kramkowski | GPG: 40B037BA0A5B8680 | Web: https://the-tk.com/ --- drivers/hid/hid-input.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c index cf8256aac2bd..cf38ff79cfe9 100644 --- a/drivers/hid/hid-input.c +++ b/drivers/hid/hid-input.c @@ -1157,12 +1157,15 @@ void hidinput_hid_event(struct hid_device *hid, struct hid_field *field, struct * don't specify logical min and max. */ if ((field->flags & HID_MAIN_ITEM_VARIABLE) && - (field->flags & HID_MAIN_ITEM_NULL_STATE) && (field->logical_minimum < field->logical_maximum) && (value < field->logical_minimum || value > field->logical_maximum)) { - dbg_hid("Ignoring out-of-range value %x\n", value); - return; + if (field->flags & HID_MAIN_ITEM_NULL_STATE) { + dbg_hid("Ignoring out-of-range value %x\n", value); + return; + } + value = value < field->logical_minimum ? + field->logical_minimum : field->logical_maximum; } /* -- 2.12.0 -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
