[1/5] HID: wacom: Defer calculation of resolution until resolution_code is known

Commit Message

Gerecke, Jason July 30, 2024, 3:51 p.m. UTC

From: Jason Gerecke <[e-mail]>

The Wacom driver maps the HID_DG_TWIST usage to ABS_Z (rather than ABS_RZ)
for historic reasons. When the code to support twist was introduced in
commit 50066a042da5 ("HID: wacom: generic: Add support for height, tilt,
and twist usages"), we were careful to write it in such a way that it had
HID calculate the resolution of the twist axis assuming ABS_RZ instead
(so that we would get correct angular behavior). This was broken with
the introduction of commit 08a46b4190d3 ("HID: wacom: Set a default
resolution for older tablets"), which moved the resolution calculation
to occur *before* the adjustment from ABS_Z to ABS_RZ occurred.

This commit moves the calculation of resolution after the point that
we are finished setting things up for its proper use.

Signed-off-by: Jason Gerecke <[e-mail]>
Fixes: 08a46b4190d3 ("HID: wacom: Set a default resolution for older tablets")
Cc: [e-mail]
---
 drivers/hid/wacom_wac.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Comments

Jiri Kosina Aug. 1, 2024, 11:07 a.m. UTC | #1

Hi Jason,

this doesn't really look like a patch series, but rather a collection of 
independent, assorted fixes and improvements, right?

From my POV, patches 1, 2 and 3 seem like 6.11-rc material to me, while 4 
and 5 seem to be 6.12 merge window material. Do you agree?

Thanks,

Gerecke, Jason Aug. 1, 2024, 3:32 p.m. UTC | #2

On Thu, Aug 1, 2024 at 4:07 AM Jiri Kosina <[e-mail]> wrote:
>
> Hi Jason,
>
> this doesn't really look like a patch series, but rather a collection of
> independent, assorted fixes and improvements, right?
>
> From my POV, patches 1, 2 and 3 seem like 6.11-rc material to me, while 4
> and 5 seem to be 6.12 merge window material. Do you agree?
>
> Thanks,
>
> --
> Jiri Kosina
> SUSE Labs
>

Hi Jiri,

I was honestly expecting everything (except maybe the first patch) to
be queued for 6.12. If you're comfortable merging any portion of them
for 6.11 I'm okay with that.

Patch 1 fixes an issue with declared resolution of the pen rotation
axis. It's not a critical fix (most of userspace doesn't care about
that axis' resolution) but still good to address.

Patch 2 is a clean-up I noticed while fixing 1. It's not a bugfix, so
there's no reason to prioritize.

Patches 3 - 5 improve our generic touchring code. They add support for
relative, high-res relative, and multiple rings. These are new
features, so I again was expecting these to hit the next merge window.

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....

Jiri Kosina Aug. 2, 2024, 11:03 a.m. UTC | #3

On Thu, 1 Aug 2024, Jason Gerecke wrote:

> I was honestly expecting everything (except maybe the first patch) to
> be queued for 6.12. If you're comfortable merging any portion of them
> for 6.11 I'm okay with that.
> 
> Patch 1 fixes an issue with declared resolution of the pen rotation
> axis. It's not a critical fix (most of userspace doesn't care about
> that axis' resolution) but still good to address.
> 
> Patch 2 is a clean-up I noticed while fixing 1. It's not a bugfix, so
> there's no reason to prioritize.
> 
> Patches 3 - 5 improve our generic touchring code. They add support for
> relative, high-res relative, and multiple rings. These are new
> features, so I again was expecting these to hit the next merge window.

Jason,

thanks for the clarification.

I've queued #1 for 6.11, and the rest for 6.12 now.

Patch

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 1f4564982b958..2541fa2e0fa3b 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1878,12 +1878,14 @@  static void wacom_map_usage(struct input_dev *input, struct hid_usage *usage,
 	int fmax = field->logical_maximum;
 	unsigned int equivalent_usage = wacom_equivalent_usage(usage->hid);
 	int resolution_code = code;
-	int resolution = hidinput_calc_abs_res(field, resolution_code);
+	int resolution;
 
 	if (equivalent_usage == HID_DG_TWIST) {
 		resolution_code = ABS_RZ;
 	}
 
+	resolution = hidinput_calc_abs_res(field, resolution_code);
+
 	if (equivalent_usage == HID_GD_X) {
 		fmin += features->offset_left;
 		fmax -= features->offset_right;