HID: wacom: Move HID fix for AES serial number into wacom_hid_usage_quirk
diff mbox series

Message ID 20190124190944.27305-1-killertofu@gmail.com
State New
Delegated to: Jiri Kosina
Headers show
Series
  • HID: wacom: Move HID fix for AES serial number into wacom_hid_usage_quirk
Related show

Commit Message

Jason Gerecke Jan. 24, 2019, 7:09 p.m. UTC
The 'wacom_hid_usage_quirk' function is the intended home for fixing
up descriptors that are buggy or that don't quite fit the mold. Commit
578325120e was supposed to move all of these quirks but it missed the
code to handle fixup the serial number usages for AES pens. Lets move
this code out of 'wacom_wac_pen_usage_mapping' where it was previously
lurking and put it into the same place as the others.

Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
---
 drivers/hid/wacom_sys.c | 32 ++++++++++++++++++++++++++++++++
 drivers/hid/wacom_wac.c | 21 ---------------------
 2 files changed, 32 insertions(+), 21 deletions(-)

Comments

Benjamin Tissoires Jan. 29, 2019, 11:02 a.m. UTC | #1
On Thu, Jan 24, 2019 at 8:10 PM Jason Gerecke <killertofu@gmail.com> wrote:
>
> The 'wacom_hid_usage_quirk' function is the intended home for fixing
> up descriptors that are buggy or that don't quite fit the mold. Commit
> 578325120e was supposed to move all of these quirks but it missed the
> code to handle fixup the serial number usages for AES pens. Lets move
> this code out of 'wacom_wac_pen_usage_mapping' where it was previously
> lurking and put it into the same place as the others.
>
> Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> ---

Thanks, applied to for-5.1/wacom.

I realized one thing today: your "From" address (gmail) doesn't match
your SoB address (wacom).
I still applied the patch because we used to not care about that for
your patches, but this raises a useless checkpatch.pl warning (and I
am scrutinizing any incoming patches with checkpatch.pl starting from
now).

Can you fix your pipeline?

Cheers,
Benjamin

>  drivers/hid/wacom_sys.c | 32 ++++++++++++++++++++++++++++++++
>  drivers/hid/wacom_wac.c | 21 ---------------------
>  2 files changed, 32 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 0bdd85d486fe..a8633b1437b2 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -252,6 +252,38 @@ static void wacom_hid_usage_quirk(struct hid_device *hdev,
>                 }
>         }
>
> +       /*
> +        * Wacom's AES devices use different vendor-defined usages to
> +        * report serial number information compared to their branded
> +        * hardware. The usages are also sometimes ill-defined and do
> +        * not have the correct logical min/max values set. Lets patch
> +        * the descriptor to use the branded usage convention and fix
> +        * the errors.
> +        */
> +       if (usage->hid == WACOM_HID_WT_SERIALNUMBER &&
> +           field->report_size == 16 &&
> +           field->index + 2 < field->report->maxfield) {
> +               struct hid_field *a = field->report->field[field->index + 1];
> +               struct hid_field *b = field->report->field[field->index + 2];
> +
> +               if (a->maxusage > 0 &&
> +                   a->usage[0].hid == HID_DG_TOOLSERIALNUMBER &&
> +                   a->report_size == 32 &&
> +                   b->maxusage > 0 &&
> +                   b->usage[0].hid == 0xFF000000 &&
> +                   b->report_size == 8) {
> +                       features->quirks |= WACOM_QUIRK_AESPEN;
> +                       usage->hid = WACOM_HID_WD_TOOLTYPE;
> +                       field->logical_minimum = S16_MIN;
> +                       field->logical_maximum = S16_MAX;
> +                       a->logical_minimum = S32_MIN;
> +                       a->logical_maximum = S32_MAX;
> +                       b->usage[0].hid = WACOM_HID_WD_SERIALHI;
> +                       b->logical_minimum = 0;
> +                       b->logical_maximum = U8_MAX;
> +               }
> +       }
> +
>         /* 2nd-generation Intuos Pro Large has incorrect Y maximum */
>         if (hdev->vendor == USB_VENDOR_ID_WACOM &&
>             hdev->product == 0x0358 &&
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index 5dd3a8245f0f..72477e872324 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -2159,27 +2159,6 @@ static void wacom_wac_pen_usage_mapping(struct hid_device *hdev,
>         case HID_DG_TOOLSERIALNUMBER:
>                 features->quirks |= WACOM_QUIRK_TOOLSERIAL;
>                 wacom_map_usage(input, usage, field, EV_MSC, MSC_SERIAL, 0);
> -
> -               /* Adjust AES usages to match modern convention */
> -               if (usage->hid == WACOM_HID_WT_SERIALNUMBER && field->report_size == 16) {
> -                       if (field->index + 2 < field->report->maxfield) {
> -                               struct hid_field *a = field->report->field[field->index + 1];
> -                               struct hid_field *b = field->report->field[field->index + 2];
> -
> -                               if (a->maxusage > 0 && a->usage[0].hid == HID_DG_TOOLSERIALNUMBER && a->report_size == 32 &&
> -                                   b->maxusage > 0 && b->usage[0].hid == 0xFF000000 && b->report_size == 8) {
> -                                       features->quirks |= WACOM_QUIRK_AESPEN;
> -                                       usage->hid = WACOM_HID_WD_TOOLTYPE;
> -                                       field->logical_minimum = S16_MIN;
> -                                       field->logical_maximum = S16_MAX;
> -                                       a->logical_minimum = S32_MIN;
> -                                       a->logical_maximum = S32_MAX;
> -                                       b->usage[0].hid = WACOM_HID_WD_SERIALHI;
> -                                       b->logical_minimum = 0;
> -                                       b->logical_maximum = U8_MAX;
> -                               }
> -                       }
> -               }
>                 break;
>         case WACOM_HID_WD_SENSE:
>                 features->quirks |= WACOM_QUIRK_SENSE;
> --
> 2.20.1
>
Jason Gerecke Jan. 29, 2019, 3:10 p.m. UTC | #2
On Tue, Jan 29, 2019 at 3:02 AM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Thu, Jan 24, 2019 at 8:10 PM Jason Gerecke <killertofu@gmail.com> wrote:
> >
> > The 'wacom_hid_usage_quirk' function is the intended home for fixing
> > up descriptors that are buggy or that don't quite fit the mold. Commit
> > 578325120e was supposed to move all of these quirks but it missed the
> > code to handle fixup the serial number usages for AES pens. Lets move
> > this code out of 'wacom_wac_pen_usage_mapping' where it was previously
> > lurking and put it into the same place as the others.
> >
> > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com>
> > ---
>
> Thanks, applied to for-5.1/wacom.
>
> I realized one thing today: your "From" address (gmail) doesn't match
> your SoB address (wacom).
> I still applied the patch because we used to not care about that for
> your patches, but this raises a useless checkpatch.pl warning (and I
> am scrutinizing any incoming patches with checkpatch.pl starting from
> now).
>
> Can you fix your pipeline?
>
> Cheers,
> Benjamin
>

Thanks, and I'll see what I can do :)

Jason
---
Now instead of four in the eights place /
you’ve got three, ‘Cause you added one  /
(That is to say, eight) to the two,     /
But you can’t take seven from three,    /
So you look at the sixty-fours....

> >  drivers/hid/wacom_sys.c | 32 ++++++++++++++++++++++++++++++++
> >  drivers/hid/wacom_wac.c | 21 ---------------------
> >  2 files changed, 32 insertions(+), 21 deletions(-)
> >
> > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> > index 0bdd85d486fe..a8633b1437b2 100644
> > --- a/drivers/hid/wacom_sys.c
> > +++ b/drivers/hid/wacom_sys.c
> > @@ -252,6 +252,38 @@ static void wacom_hid_usage_quirk(struct hid_device *hdev,
> >                 }
> >         }
> >
> > +       /*
> > +        * Wacom's AES devices use different vendor-defined usages to
> > +        * report serial number information compared to their branded
> > +        * hardware. The usages are also sometimes ill-defined and do
> > +        * not have the correct logical min/max values set. Lets patch
> > +        * the descriptor to use the branded usage convention and fix
> > +        * the errors.
> > +        */
> > +       if (usage->hid == WACOM_HID_WT_SERIALNUMBER &&
> > +           field->report_size == 16 &&
> > +           field->index + 2 < field->report->maxfield) {
> > +               struct hid_field *a = field->report->field[field->index + 1];
> > +               struct hid_field *b = field->report->field[field->index + 2];
> > +
> > +               if (a->maxusage > 0 &&
> > +                   a->usage[0].hid == HID_DG_TOOLSERIALNUMBER &&
> > +                   a->report_size == 32 &&
> > +                   b->maxusage > 0 &&
> > +                   b->usage[0].hid == 0xFF000000 &&
> > +                   b->report_size == 8) {
> > +                       features->quirks |= WACOM_QUIRK_AESPEN;
> > +                       usage->hid = WACOM_HID_WD_TOOLTYPE;
> > +                       field->logical_minimum = S16_MIN;
> > +                       field->logical_maximum = S16_MAX;
> > +                       a->logical_minimum = S32_MIN;
> > +                       a->logical_maximum = S32_MAX;
> > +                       b->usage[0].hid = WACOM_HID_WD_SERIALHI;
> > +                       b->logical_minimum = 0;
> > +                       b->logical_maximum = U8_MAX;
> > +               }
> > +       }
> > +
> >         /* 2nd-generation Intuos Pro Large has incorrect Y maximum */
> >         if (hdev->vendor == USB_VENDOR_ID_WACOM &&
> >             hdev->product == 0x0358 &&
> > diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> > index 5dd3a8245f0f..72477e872324 100644
> > --- a/drivers/hid/wacom_wac.c
> > +++ b/drivers/hid/wacom_wac.c
> > @@ -2159,27 +2159,6 @@ static void wacom_wac_pen_usage_mapping(struct hid_device *hdev,
> >         case HID_DG_TOOLSERIALNUMBER:
> >                 features->quirks |= WACOM_QUIRK_TOOLSERIAL;
> >                 wacom_map_usage(input, usage, field, EV_MSC, MSC_SERIAL, 0);
> > -
> > -               /* Adjust AES usages to match modern convention */
> > -               if (usage->hid == WACOM_HID_WT_SERIALNUMBER && field->report_size == 16) {
> > -                       if (field->index + 2 < field->report->maxfield) {
> > -                               struct hid_field *a = field->report->field[field->index + 1];
> > -                               struct hid_field *b = field->report->field[field->index + 2];
> > -
> > -                               if (a->maxusage > 0 && a->usage[0].hid == HID_DG_TOOLSERIALNUMBER && a->report_size == 32 &&
> > -                                   b->maxusage > 0 && b->usage[0].hid == 0xFF000000 && b->report_size == 8) {
> > -                                       features->quirks |= WACOM_QUIRK_AESPEN;
> > -                                       usage->hid = WACOM_HID_WD_TOOLTYPE;
> > -                                       field->logical_minimum = S16_MIN;
> > -                                       field->logical_maximum = S16_MAX;
> > -                                       a->logical_minimum = S32_MIN;
> > -                                       a->logical_maximum = S32_MAX;
> > -                                       b->usage[0].hid = WACOM_HID_WD_SERIALHI;
> > -                                       b->logical_minimum = 0;
> > -                                       b->logical_maximum = U8_MAX;
> > -                               }
> > -                       }
> > -               }
> >                 break;
> >         case WACOM_HID_WD_SENSE:
> >                 features->quirks |= WACOM_QUIRK_SENSE;
> > --
> > 2.20.1
> >

Patch
diff mbox series

diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 0bdd85d486fe..a8633b1437b2 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -252,6 +252,38 @@  static void wacom_hid_usage_quirk(struct hid_device *hdev,
 		}
 	}
 
+	/*
+	 * Wacom's AES devices use different vendor-defined usages to
+	 * report serial number information compared to their branded
+	 * hardware. The usages are also sometimes ill-defined and do
+	 * not have the correct logical min/max values set. Lets patch
+	 * the descriptor to use the branded usage convention and fix
+	 * the errors.
+	 */
+	if (usage->hid == WACOM_HID_WT_SERIALNUMBER &&
+	    field->report_size == 16 &&
+	    field->index + 2 < field->report->maxfield) {
+		struct hid_field *a = field->report->field[field->index + 1];
+		struct hid_field *b = field->report->field[field->index + 2];
+
+		if (a->maxusage > 0 &&
+		    a->usage[0].hid == HID_DG_TOOLSERIALNUMBER &&
+		    a->report_size == 32 &&
+		    b->maxusage > 0 &&
+		    b->usage[0].hid == 0xFF000000 &&
+		    b->report_size == 8) {
+			features->quirks |= WACOM_QUIRK_AESPEN;
+			usage->hid = WACOM_HID_WD_TOOLTYPE;
+			field->logical_minimum = S16_MIN;
+			field->logical_maximum = S16_MAX;
+			a->logical_minimum = S32_MIN;
+			a->logical_maximum = S32_MAX;
+			b->usage[0].hid = WACOM_HID_WD_SERIALHI;
+			b->logical_minimum = 0;
+			b->logical_maximum = U8_MAX;
+		}
+	}
+
 	/* 2nd-generation Intuos Pro Large has incorrect Y maximum */
 	if (hdev->vendor == USB_VENDOR_ID_WACOM &&
 	    hdev->product == 0x0358 &&
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 5dd3a8245f0f..72477e872324 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -2159,27 +2159,6 @@  static void wacom_wac_pen_usage_mapping(struct hid_device *hdev,
 	case HID_DG_TOOLSERIALNUMBER:
 		features->quirks |= WACOM_QUIRK_TOOLSERIAL;
 		wacom_map_usage(input, usage, field, EV_MSC, MSC_SERIAL, 0);
-
-		/* Adjust AES usages to match modern convention */
-		if (usage->hid == WACOM_HID_WT_SERIALNUMBER && field->report_size == 16) {
-			if (field->index + 2 < field->report->maxfield) {
-				struct hid_field *a = field->report->field[field->index + 1];
-				struct hid_field *b = field->report->field[field->index + 2];
-
-				if (a->maxusage > 0 && a->usage[0].hid == HID_DG_TOOLSERIALNUMBER && a->report_size == 32 &&
-				    b->maxusage > 0 && b->usage[0].hid == 0xFF000000 && b->report_size == 8) {
-					features->quirks |= WACOM_QUIRK_AESPEN;
-					usage->hid = WACOM_HID_WD_TOOLTYPE;
-					field->logical_minimum = S16_MIN;
-					field->logical_maximum = S16_MAX;
-					a->logical_minimum = S32_MIN;
-					a->logical_maximum = S32_MAX;
-					b->usage[0].hid = WACOM_HID_WD_SERIALHI;
-					b->logical_minimum = 0;
-					b->logical_maximum = U8_MAX;
-				}
-			}
-		}
 		break;
 	case WACOM_HID_WD_SENSE:
 		features->quirks |= WACOM_QUIRK_SENSE;