diff mbox

[v2,07/11] HID: hid-multitouch: move ALWAYS_VALID quirk check

Message ID 1351241067-9521-8-git-send-email-benjamin.tissoires@gmail.com (mailing list archive)
State New, archived
Delegated to: Jiri Kosina
Headers show

Commit Message

Benjamin Tissoires Oct. 26, 2012, 8:44 a.m. UTC
Win 8 device specification changed the requirements for the hid usages
of the multitouch devices. Now InRange is optional and must be only
used when the device supports hovering.

This ensures that the quirk ALWAYS_VALID is taken into account and
also ensures its precedence over the other VALID* quirks.

Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
---
 drivers/hid/hid-multitouch.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

Comments

Henrik Rydberg Oct. 29, 2012, 10:16 p.m. UTC | #1
Hi Benjamin,

> Win 8 device specification changed the requirements for the hid usages
> of the multitouch devices. Now InRange is optional and must be only
> used when the device supports hovering.
> 
> This ensures that the quirk ALWAYS_VALID is taken into account and
> also ensures its precedence over the other VALID* quirks.
> 
> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
> ---
>  drivers/hid/hid-multitouch.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

Since we seem to never actually reset td, this seems equivalent,
unless some device added ALWAYS VALID by mistake, even if INRANGE is
not part of the report descriptor.

> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
> index 000c979..43bd8e8 100644
> --- a/drivers/hid/hid-multitouch.c
> +++ b/drivers/hid/hid-multitouch.c
> @@ -506,7 +506,7 @@ static int mt_compute_slot(struct mt_device *td, struct input_dev *input)
>   */
>  static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>  {
> -	if (td->curvalid) {
> +	if (td->curvalid || td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID) {

I found at least one presence of this construct in the kernel, but I
think the overwhelming majority use parenthesis.

>  		int slotnum = mt_compute_slot(td, input);
>  		struct mt_slot *s = &td->curdata;
>  
> @@ -561,9 +561,7 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
>  	if (hid->claimed & HID_CLAIMED_INPUT) {
>  		switch (usage->hid) {
>  		case HID_DG_INRANGE:
> -			if (quirks & MT_QUIRK_ALWAYS_VALID)
> -				td->curvalid = true;
> -			else if (quirks & MT_QUIRK_VALID_IS_INRANGE)
> +			if (quirks & MT_QUIRK_VALID_IS_INRANGE)
>  				td->curvalid = value;
>  			break;
>  		case HID_DG_TIPSWITCH:
> -- 
> 1.7.11.7
> 

Thanks,
Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Benjamin Tissoires Oct. 30, 2012, 10:19 a.m. UTC | #2
On Mon, Oct 29, 2012 at 11:16 PM, Henrik Rydberg <rydberg@euromail.se> wrote:
> Hi Benjamin,
>
>> Win 8 device specification changed the requirements for the hid usages
>> of the multitouch devices. Now InRange is optional and must be only
>> used when the device supports hovering.
>>
>> This ensures that the quirk ALWAYS_VALID is taken into account and
>> also ensures its precedence over the other VALID* quirks.
>>
>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
>> ---
>>  drivers/hid/hid-multitouch.c | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> Since we seem to never actually reset td, this seems equivalent,
> unless some device added ALWAYS VALID by mistake, even if INRANGE is
> not part of the report descriptor.

Yes, it is equivalent for Win7 devices. The field InRange was
mandatory, and currently, nobody mixed ALWAYS_VALID with other quirks.
But now, InRange is optional, and device not supporting hovering
should not use it, so it's mandatory to move it at a place we can be
sure it will be taken into account.

>
>> diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
>> index 000c979..43bd8e8 100644
>> --- a/drivers/hid/hid-multitouch.c
>> +++ b/drivers/hid/hid-multitouch.c
>> @@ -506,7 +506,7 @@ static int mt_compute_slot(struct mt_device *td, struct input_dev *input)
>>   */
>>  static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
>>  {
>> -     if (td->curvalid) {
>> +     if (td->curvalid || td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID) {
>
> I found at least one presence of this construct in the kernel, but I
> think the overwhelming majority use parenthesis.

oops, I was really angry against parenthesis in this patch series :)

Cheers,
Benjamin

>
>>               int slotnum = mt_compute_slot(td, input);
>>               struct mt_slot *s = &td->curdata;
>>
>> @@ -561,9 +561,7 @@ static int mt_event(struct hid_device *hid, struct hid_field *field,
>>       if (hid->claimed & HID_CLAIMED_INPUT) {
>>               switch (usage->hid) {
>>               case HID_DG_INRANGE:
>> -                     if (quirks & MT_QUIRK_ALWAYS_VALID)
>> -                             td->curvalid = true;
>> -                     else if (quirks & MT_QUIRK_VALID_IS_INRANGE)
>> +                     if (quirks & MT_QUIRK_VALID_IS_INRANGE)
>>                               td->curvalid = value;
>>                       break;
>>               case HID_DG_TIPSWITCH:
>> --
>> 1.7.11.7
>>
>
> Thanks,
> Henrik
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/hid/hid-multitouch.c b/drivers/hid/hid-multitouch.c
index 000c979..43bd8e8 100644
--- a/drivers/hid/hid-multitouch.c
+++ b/drivers/hid/hid-multitouch.c
@@ -506,7 +506,7 @@  static int mt_compute_slot(struct mt_device *td, struct input_dev *input)
  */
 static void mt_complete_slot(struct mt_device *td, struct input_dev *input)
 {
-	if (td->curvalid) {
+	if (td->curvalid || td->mtclass.quirks & MT_QUIRK_ALWAYS_VALID) {
 		int slotnum = mt_compute_slot(td, input);
 		struct mt_slot *s = &td->curdata;
 
@@ -561,9 +561,7 @@  static int mt_event(struct hid_device *hid, struct hid_field *field,
 	if (hid->claimed & HID_CLAIMED_INPUT) {
 		switch (usage->hid) {
 		case HID_DG_INRANGE:
-			if (quirks & MT_QUIRK_ALWAYS_VALID)
-				td->curvalid = true;
-			else if (quirks & MT_QUIRK_VALID_IS_INRANGE)
+			if (quirks & MT_QUIRK_VALID_IS_INRANGE)
 				td->curvalid = value;
 			break;
 		case HID_DG_TIPSWITCH: