diff mbox series

[1/2] HID: wacom: Only report rotation for art pen

Message ID 20220513215156.14914-1-ping.cheng@wacom.com (mailing list archive)
State New
Delegated to: Jiri Kosina
Headers show
Series [1/2] HID: wacom: Only report rotation for art pen | expand

Commit Message

Ping Cheng May 13, 2022, 9:51 p.m. UTC
The generic routine, wacom_wac_pen_event, turns rotation value 90
degree anti-clockwise before posting the events. This non-zero
event trggers a non-zero ABS_Z event for non art pen tools. However,
HID_DG_TWIST is only supported by art pen.

Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
--
Hi Jiri,

This is kind of a version 2 of the last one I posted two days ago.
I updated the logic so it has less changed lines: 29 vs 158! Hopefully,
the logic is easier to follow now. Please ignore the last one.

Thank you!
---
 drivers/hid/wacom_wac.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

Comments

Ping Cheng May 17, 2022, 7:10 p.m. UTC | #1
Hi Jiri,

Sorry for the noise. Both patches in this set are bug-fixing. I forgot
to add Cc: stable@vger.kernel.org in the patches. This patch (1/2) can
be backported to 4.9 or later; the other one (2/2) can be backported
to 4.14 or later.

Thanks a lot,
Ping




On Fri, May 13, 2022 at 2:52 PM Ping Cheng <pinglinux@gmail.com> wrote:
>
> The generic routine, wacom_wac_pen_event, turns rotation value 90
> degree anti-clockwise before posting the events. This non-zero
> event trggers a non-zero ABS_Z event for non art pen tools. However,
> HID_DG_TWIST is only supported by art pen.
>
> Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
> --
> Hi Jiri,
>
> This is kind of a version 2 of the last one I posted two days ago.
> I updated the logic so it has less changed lines: 29 vs 158! Hopefully,
> the logic is easier to follow now. Please ignore the last one.
>
> Thank you!
> ---
>  drivers/hid/wacom_wac.c | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index a7176fc0635d..3bee4f46003d 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -638,9 +638,26 @@ static int wacom_intuos_id_mangle(int tool_id)
>         return (tool_id & ~0xFFF) << 4 | (tool_id & 0xFFF);
>  }
>
> +static bool wacom_is_art_pen(int tool_id)
> +{
> +       bool is_art_pen = false;
> +
> +       switch (tool_id) {
> +       case 0x885:     /* Intuos3 Marker Pen */
> +       case 0x804:     /* Intuos4/5 13HD/24HD Marker Pen */
> +       case 0x10804:   /* Intuos4/5 13HD/24HD Art Pen */
> +               is_art_pen = true;
> +               break;
> +       }
> +       return is_art_pen;
> +}
> +
>  static int wacom_intuos_get_tool_type(int tool_id)
>  {
> -       int tool_type;
> +       int tool_type = BTN_TOOL_PEN;
> +
> +       if (wacom_is_art_pen(tool_id)
> +               return tool_type;
>
>         switch (tool_id) {
>         case 0x812: /* Inking pen */
> @@ -655,12 +672,9 @@ static int wacom_intuos_get_tool_type(int tool_id)
>         case 0x852:
>         case 0x823: /* Intuos3 Grip Pen */
>         case 0x813: /* Intuos3 Classic Pen */
> -       case 0x885: /* Intuos3 Marker Pen */
>         case 0x802: /* Intuos4/5 13HD/24HD General Pen */
> -       case 0x804: /* Intuos4/5 13HD/24HD Marker Pen */
>         case 0x8e2: /* IntuosHT2 pen */
>         case 0x022:
> -       case 0x10804: /* Intuos4/5 13HD/24HD Art Pen */
>         case 0x10842: /* MobileStudio Pro Pro Pen slim */
>         case 0x14802: /* Intuos4/5 13HD/24HD Classic Pen */
>         case 0x16802: /* Cintiq 13HD Pro Pen */
> @@ -718,10 +732,6 @@ static int wacom_intuos_get_tool_type(int tool_id)
>         case 0x10902: /* Intuos4/5 13HD/24HD Airbrush */
>                 tool_type = BTN_TOOL_AIRBRUSH;
>                 break;
> -
> -       default: /* Unknown tool */
> -               tool_type = BTN_TOOL_PEN;
> -               break;
>         }
>         return tool_type;
>  }
> @@ -2323,6 +2333,9 @@ static void wacom_wac_pen_event(struct hid_device *hdev, struct hid_field *field
>                 }
>                 return;
>         case HID_DG_TWIST:
> +               /* don't modify the value if the pen doesn't support the feature */
> +               if (!wacom_is_art_pen(wacom_wac->id[0])) return;
> +
>                 /*
>                  * Userspace expects pen twist to have its zero point when
>                  * the buttons/finger is on the tablet's left. HID values
> --
> 2.25.1
>
Jiri Kosina May 23, 2022, 6:48 a.m. UTC | #2
On Tue, 17 May 2022, Ping Cheng wrote:

> Hi Jiri,
> 
> Sorry for the noise. Both patches in this set are bug-fixing. I forgot
> to add Cc: stable@vger.kernel.org in the patches. This patch (1/2) can
> be backported to 4.9 or later; the other one (2/2) can be backported
> to 4.14 or later.

Ping,

thanks for the patch and the explanation. I however don't seem to have 2/2 
in my inbox; could you please resend? Or ise the 1/2 a mistake? (the 
previous version didn't indicate it's a part of a series).

Thanks,
Jiri Kosina May 27, 2022, 7:06 a.m. UTC | #3
On Fri, 13 May 2022, Ping Cheng wrote:

> The generic routine, wacom_wac_pen_event, turns rotation value 90
> degree anti-clockwise before posting the events. This non-zero
> event trggers a non-zero ABS_Z event for non art pen tools. However,
> HID_DG_TWIST is only supported by art pen.
> 
> Signed-off-by: Ping Cheng <ping.cheng@wacom.com>
> Reviewed-by: Jason Gerecke <jason.gerecke@wacom.com>
> --
> Hi Jiri,
> 
> This is kind of a version 2 of the last one I posted two days ago.
> I updated the logic so it has less changed lines: 29 vs 158! Hopefully,
> the logic is easier to follow now. Please ignore the last one.
> 
> Thank you!
> ---
>  drivers/hid/wacom_wac.c | 29 +++++++++++++++++++++--------
>  1 file changed, 21 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index a7176fc0635d..3bee4f46003d 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -638,9 +638,26 @@ static int wacom_intuos_id_mangle(int tool_id)
>  	return (tool_id & ~0xFFF) << 4 | (tool_id & 0xFFF);
>  }
>  
> +static bool wacom_is_art_pen(int tool_id)
> +{
> +	bool is_art_pen = false;
> +
> +	switch (tool_id) {
> +	case 0x885:	/* Intuos3 Marker Pen */
> +	case 0x804:	/* Intuos4/5 13HD/24HD Marker Pen */
> +	case 0x10804:	/* Intuos4/5 13HD/24HD Art Pen */
> +		is_art_pen = true;
> +		break;
> +	}
> +	return is_art_pen;
> +}
> +
>  static int wacom_intuos_get_tool_type(int tool_id)
>  {
> -	int tool_type;
> +	int tool_type = BTN_TOOL_PEN;
> +
> +	if (wacom_is_art_pen(tool_id)

I've added the missing brace here and applied both patches. Thanks,
diff mbox series

Patch

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index a7176fc0635d..3bee4f46003d 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -638,9 +638,26 @@  static int wacom_intuos_id_mangle(int tool_id)
 	return (tool_id & ~0xFFF) << 4 | (tool_id & 0xFFF);
 }
 
+static bool wacom_is_art_pen(int tool_id)
+{
+	bool is_art_pen = false;
+
+	switch (tool_id) {
+	case 0x885:	/* Intuos3 Marker Pen */
+	case 0x804:	/* Intuos4/5 13HD/24HD Marker Pen */
+	case 0x10804:	/* Intuos4/5 13HD/24HD Art Pen */
+		is_art_pen = true;
+		break;
+	}
+	return is_art_pen;
+}
+
 static int wacom_intuos_get_tool_type(int tool_id)
 {
-	int tool_type;
+	int tool_type = BTN_TOOL_PEN;
+
+	if (wacom_is_art_pen(tool_id)
+		return tool_type;
 
 	switch (tool_id) {
 	case 0x812: /* Inking pen */
@@ -655,12 +672,9 @@  static int wacom_intuos_get_tool_type(int tool_id)
 	case 0x852:
 	case 0x823: /* Intuos3 Grip Pen */
 	case 0x813: /* Intuos3 Classic Pen */
-	case 0x885: /* Intuos3 Marker Pen */
 	case 0x802: /* Intuos4/5 13HD/24HD General Pen */
-	case 0x804: /* Intuos4/5 13HD/24HD Marker Pen */
 	case 0x8e2: /* IntuosHT2 pen */
 	case 0x022:
-	case 0x10804: /* Intuos4/5 13HD/24HD Art Pen */
 	case 0x10842: /* MobileStudio Pro Pro Pen slim */
 	case 0x14802: /* Intuos4/5 13HD/24HD Classic Pen */
 	case 0x16802: /* Cintiq 13HD Pro Pen */
@@ -718,10 +732,6 @@  static int wacom_intuos_get_tool_type(int tool_id)
 	case 0x10902: /* Intuos4/5 13HD/24HD Airbrush */
 		tool_type = BTN_TOOL_AIRBRUSH;
 		break;
-
-	default: /* Unknown tool */
-		tool_type = BTN_TOOL_PEN;
-		break;
 	}
 	return tool_type;
 }
@@ -2323,6 +2333,9 @@  static void wacom_wac_pen_event(struct hid_device *hdev, struct hid_field *field
 		}
 		return;
 	case HID_DG_TWIST:
+		/* don't modify the value if the pen doesn't support the feature */
+		if (!wacom_is_art_pen(wacom_wac->id[0])) return;
+
 		/*
 		 * Userspace expects pen twist to have its zero point when
 		 * the buttons/finger is on the tablet's left. HID values