* Re: [PATCH 3/7] ARM: dts: qcom: Add support for Samsung Galaxy Tab 4 8.0 Wi-Fi
From: Bryant Mairs @ 2023-11-07 11:26 UTC (permalink / raw)
To: Krzysztof Kozlowski, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Dmitry Torokhov,
Linus Walleij, linux-arm-msm, devicetree, linux-kernel,
linux-input
In-Reply-To: <466ffc6c-19c9-44e0-b97b-8fa4358f34ab@linaro.org>
Indeed, very sorry about that! Forgot where I left off when doing this and missed an (the?) important step.
I'll take the feedback I've already gotten, clean this up, make sure things build ;), and resubmit soon!
- B
On November 7, 2023 8:29:10 AM GMT+01:00, Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> wrote:
>On 05/11/2023 21:46, Bryant Mairs wrote:
>> Add support for this tablet based on the MSM8226 SoC, codenamed
>> "milletwifi".
>>
>> Signed-off-by: Bryant Mairs <bryant@mai.rs>
>> ---
>> arch/arm/boot/dts/qcom/Makefile | 1 +
>> .../qcom/qcom-apq8026-samsung-milletwifi.dts | 543 ++++++++++++++++++
>> 2 files changed, 544 insertions(+)
>> create mode 100644 arch/arm/boot/dts/qcom/qcom-apq8026-samsung-milletwifi.dts
>>
>
>LKP reports that patches were most likely not even built :(
>
>Best regards,
>Krzysztof
>
^ permalink raw reply
* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
From: Illia Ostapyshyn @ 2023-11-07 13:40 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: David Revoy, jkosina, jason.gerecke, jose.exposito89, linux-input,
linux-kernel, nils, peter.hutterer, ping.cheng, bagasdotme
In-Reply-To: <CAO-hwJKVwZK00yZFjuyyR9Xt4Y2-r8eLJNZfnyeopHxoZQ0eGA@mail.gmail.com>
Sending again because the mail bounced from the mailing list due to the
attachment.
Benjamin Tissoires <benjamin.tissoires@redhat.com> writes:
> And BTW, if you have a tool affected by 276e14e6c3, I'd be curious to
> get a hid-recorder sample for it so I can get regression tests for it.
I have attached [3] the recording of me:
(1) Bringing the stylus in range, touching the screen with the tip and
bringing the stylus out of range.
(2) Bringing the stylus in range, pressing the top barrel button and
bringing the stylus out of range.
(3) Bringing the stylus in range, touching the screen with the tip again
and bringing the stylus out of range.
The digitizer is the one of the two that David uses, XP-Pen Artist 24.
I don't have the other one with two erasers here, so we'd have to wait
for David's recording to investigate further.
If you revert 276e14e6c3, you can observe that after pressing the eraser
button, neither BTN_TOOL_PEN nor BTN_TOUCH events will appear in evdev
anymore for (3).
> I must confess, being the one who refactored everything, I still don't
> believe this is as simple as it may seem. I paged out all of the
> special cases, and now, without seeing the event flow I just can not
> understand why this would fix the situation.
David uses hwdb to remap Eraser (0xd0045) to BTN_STYLUS2 (0x14c) [1]:
evdev:input:b0003v28BDp092De0100-e0*
KEYBOARD_KEY_d0045=0x14c
In the end, this translates to a hidinput_setkeycode with the respective
arguments, setting usage->code to BTN_STYLUS2. In the current state,
doing so results in BTN_STYLUS2 being permanently set and never released
when pressing the top barrel switch. You can replay and observe this
with the attached [3] recording.
The if statement [2] is there to release BTN_TOOL_RUBBER if the device
has no Invert, but only after BTN_TOUCH has been released. Eraser with
value 0 releases BTN_TOUCH in the first iteration and BTN_TOOL_RUBBER in
the second (when BTN_TOUCH is not in input->key anymore).
The problem is that the condition assumes usage->code is BTN_TOUCH.
When this is not the case, (!test_bit(BTN_TOUCH, input->key)) is always
true, we release the tool and return prematurely. Therefore,
usage->code is never released.
As such, BTN_TOOL_RUBBER is not the problem and does no harm (except for
maybe showing the rubber icon in Krita). It is required, however, for a
functional eraser out of the box. I think, in the HID_QUIRK_NOINVERT
case, BTN_TOOL_RUBBER should better be omitted if Eraser is remapped to
something else, like BTN_STYLUS2. Hence the second proposal.
> So either the explanation was wrong, or it's not explaining the
> situation (I also understand that this is not a formal submission, so
> maybe that's the reason why the comment is not updated).
Right, the example was not meant as a formal submission, that's why I
didn't change the comment. Sorry for that. We should fix the comment
below it (line 1603) too after this is resolved.
Cheers,
Illia
[1]
https://www.davidrevoy.com/article842/review-xp-pen-artist-24-pro-on-linux
[2]
https://elixir.bootlin.com/linux/latest/source/drivers/hid/hid-input.c#L1594
[3] https://dl.uni-h.de/?t=dc4a5542a8e4d54964e298045a173049
^ permalink raw reply
* [PATCH AUTOSEL 6.6 16/36] HID: lenovo: Detect quirk-free fw on cptkbd and stop applying workaround
From: Sasha Levin @ 2023-11-07 15:45 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Mikhail Khvainitski, Jiri Kosina, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20231107154654.3765336-1-sashal@kernel.org>
From: Mikhail Khvainitski <me@khvoinitsky.org>
[ Upstream commit 46a0a2c96f0f47628190f122c2e3d879e590bcbe ]
Built-in firmware of cptkbd handles scrolling by itself (when middle
button is pressed) but with issues: it does not support horizontal and
hi-res scrolling and upon middle button release it sends middle button
click even if there was a scrolling event. Commit 3cb5ff0220e3 ("HID:
lenovo: Hide middle-button press until release") workarounds last
issue but it's impossible to workaround scrolling-related issues
without firmware modification.
Likely, Dennis Schneider has reverse engineered the firmware and
provided an instruction on how to patch it [1]. However,
aforementioned workaround prevents userspace (libinput) from knowing
exact moment when middle button has been pressed down and performing
"On-Button scrolling". This commit detects correctly-behaving patched
firmware if cursor movement events has been received during middle
button being pressed and stops applying workaround for this device.
Link: https://hohlerde.org/rauch/en/elektronik/projekte/tpkbd-fix/ [1]
Signed-off-by: Mikhail Khvainitski <me@khvoinitsky.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-lenovo.c | 68 ++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 44763c0da4441..9c1181313e44d 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -51,7 +51,12 @@ struct lenovo_drvdata {
int select_right;
int sensitivity;
int press_speed;
- u8 middlebutton_state; /* 0:Up, 1:Down (undecided), 2:Scrolling */
+ /* 0: Up
+ * 1: Down (undecided)
+ * 2: Scrolling
+ * 3: Patched firmware, disable workaround
+ */
+ u8 middlebutton_state;
bool fn_lock;
};
@@ -668,31 +673,48 @@ static int lenovo_event_cptkbd(struct hid_device *hdev,
{
struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
- /* "wheel" scroll events */
- if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
- usage->code == REL_HWHEEL)) {
- /* Scroll events disable middle-click event */
- cptkbd_data->middlebutton_state = 2;
- return 0;
- }
+ if (cptkbd_data->middlebutton_state != 3) {
+ /* REL_X and REL_Y events during middle button pressed
+ * are only possible on patched, bug-free firmware
+ * so set middlebutton_state to 3
+ * to never apply workaround anymore
+ */
+ if (cptkbd_data->middlebutton_state == 1 &&
+ usage->type == EV_REL &&
+ (usage->code == REL_X || usage->code == REL_Y)) {
+ cptkbd_data->middlebutton_state = 3;
+ /* send middle button press which was hold before */
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 1);
+ input_sync(field->hidinput->input);
+ }
- /* Middle click events */
- if (usage->type == EV_KEY && usage->code == BTN_MIDDLE) {
- if (value == 1) {
- cptkbd_data->middlebutton_state = 1;
- } else if (value == 0) {
- if (cptkbd_data->middlebutton_state == 1) {
- /* No scrolling inbetween, send middle-click */
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 1);
- input_sync(field->hidinput->input);
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 0);
- input_sync(field->hidinput->input);
+ /* "wheel" scroll events */
+ if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
+ usage->code == REL_HWHEEL)) {
+ /* Scroll events disable middle-click event */
+ cptkbd_data->middlebutton_state = 2;
+ return 0;
+ }
+
+ /* Middle click events */
+ if (usage->type == EV_KEY && usage->code == BTN_MIDDLE) {
+ if (value == 1) {
+ cptkbd_data->middlebutton_state = 1;
+ } else if (value == 0) {
+ if (cptkbd_data->middlebutton_state == 1) {
+ /* No scrolling inbetween, send middle-click */
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 1);
+ input_sync(field->hidinput->input);
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 0);
+ input_sync(field->hidinput->input);
+ }
+ cptkbd_data->middlebutton_state = 0;
}
- cptkbd_data->middlebutton_state = 0;
+ return 1;
}
- return 1;
}
if (usage->type == EV_KEY && usage->code == KEY_FN_ESC && value == 1) {
--
2.42.0
^ permalink raw reply related
* [PATCH AUTOSEL 6.6 34/36] HID: Add quirk for Dell Pro Wireless Keyboard and Mouse KM5221W
From: Sasha Levin @ 2023-11-07 15:46 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jiri Kosina, Robert Ayrapetyan, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20231107154654.3765336-1-sashal@kernel.org>
From: Jiri Kosina <jkosina@suse.cz>
[ Upstream commit 62cc9c3cb3ec1bf31cc116146185ed97b450836a ]
This device needs ALWAYS_POLL quirk, otherwise it keeps reconnecting
indefinitely.
Reported-by: Robert Ayrapetyan <robert.ayrapetyan@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-quirks.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index e4d2dfd5d2536..f7973ccd84a28 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -366,6 +366,7 @@
#define USB_VENDOR_ID_DELL 0x413c
#define USB_DEVICE_ID_DELL_PIXART_USB_OPTICAL_MOUSE 0x301a
+#define USB_DEVICE_ID_DELL_PRO_WIRELESS_KM5221W 0x4503
#define USB_VENDOR_ID_DELORME 0x1163
#define USB_DEVICE_ID_DELORME_EARTHMATE 0x0100
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 3983b4f282f8f..5a48fcaa32f00 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -66,6 +66,7 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_STRAFE), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_CREATIVE_SB_OMNI_SURROUND_51), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_PIXART_USB_OPTICAL_MOUSE), HID_QUIRK_ALWAYS_POLL },
+ { HID_USB_DEVICE(USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_PRO_WIRELESS_KM5221W), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_DMI, USB_DEVICE_ID_DMI_ENC), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRACAL_RAPHNET, USB_DEVICE_ID_RAPHNET_2NES2SNES), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRACAL_RAPHNET, USB_DEVICE_ID_RAPHNET_4NES4SNES), HID_QUIRK_MULTI_INPUT },
--
2.42.0
^ permalink raw reply related
* [PATCH AUTOSEL 6.5 14/34] HID: lenovo: Detect quirk-free fw on cptkbd and stop applying workaround
From: Sasha Levin @ 2023-11-07 15:47 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Mikhail Khvainitski, Jiri Kosina, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20231107154846.3766119-1-sashal@kernel.org>
From: Mikhail Khvainitski <me@khvoinitsky.org>
[ Upstream commit 46a0a2c96f0f47628190f122c2e3d879e590bcbe ]
Built-in firmware of cptkbd handles scrolling by itself (when middle
button is pressed) but with issues: it does not support horizontal and
hi-res scrolling and upon middle button release it sends middle button
click even if there was a scrolling event. Commit 3cb5ff0220e3 ("HID:
lenovo: Hide middle-button press until release") workarounds last
issue but it's impossible to workaround scrolling-related issues
without firmware modification.
Likely, Dennis Schneider has reverse engineered the firmware and
provided an instruction on how to patch it [1]. However,
aforementioned workaround prevents userspace (libinput) from knowing
exact moment when middle button has been pressed down and performing
"On-Button scrolling". This commit detects correctly-behaving patched
firmware if cursor movement events has been received during middle
button being pressed and stops applying workaround for this device.
Link: https://hohlerde.org/rauch/en/elektronik/projekte/tpkbd-fix/ [1]
Signed-off-by: Mikhail Khvainitski <me@khvoinitsky.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-lenovo.c | 68 ++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 44763c0da4441..9c1181313e44d 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -51,7 +51,12 @@ struct lenovo_drvdata {
int select_right;
int sensitivity;
int press_speed;
- u8 middlebutton_state; /* 0:Up, 1:Down (undecided), 2:Scrolling */
+ /* 0: Up
+ * 1: Down (undecided)
+ * 2: Scrolling
+ * 3: Patched firmware, disable workaround
+ */
+ u8 middlebutton_state;
bool fn_lock;
};
@@ -668,31 +673,48 @@ static int lenovo_event_cptkbd(struct hid_device *hdev,
{
struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
- /* "wheel" scroll events */
- if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
- usage->code == REL_HWHEEL)) {
- /* Scroll events disable middle-click event */
- cptkbd_data->middlebutton_state = 2;
- return 0;
- }
+ if (cptkbd_data->middlebutton_state != 3) {
+ /* REL_X and REL_Y events during middle button pressed
+ * are only possible on patched, bug-free firmware
+ * so set middlebutton_state to 3
+ * to never apply workaround anymore
+ */
+ if (cptkbd_data->middlebutton_state == 1 &&
+ usage->type == EV_REL &&
+ (usage->code == REL_X || usage->code == REL_Y)) {
+ cptkbd_data->middlebutton_state = 3;
+ /* send middle button press which was hold before */
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 1);
+ input_sync(field->hidinput->input);
+ }
- /* Middle click events */
- if (usage->type == EV_KEY && usage->code == BTN_MIDDLE) {
- if (value == 1) {
- cptkbd_data->middlebutton_state = 1;
- } else if (value == 0) {
- if (cptkbd_data->middlebutton_state == 1) {
- /* No scrolling inbetween, send middle-click */
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 1);
- input_sync(field->hidinput->input);
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 0);
- input_sync(field->hidinput->input);
+ /* "wheel" scroll events */
+ if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
+ usage->code == REL_HWHEEL)) {
+ /* Scroll events disable middle-click event */
+ cptkbd_data->middlebutton_state = 2;
+ return 0;
+ }
+
+ /* Middle click events */
+ if (usage->type == EV_KEY && usage->code == BTN_MIDDLE) {
+ if (value == 1) {
+ cptkbd_data->middlebutton_state = 1;
+ } else if (value == 0) {
+ if (cptkbd_data->middlebutton_state == 1) {
+ /* No scrolling inbetween, send middle-click */
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 1);
+ input_sync(field->hidinput->input);
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 0);
+ input_sync(field->hidinput->input);
+ }
+ cptkbd_data->middlebutton_state = 0;
}
- cptkbd_data->middlebutton_state = 0;
+ return 1;
}
- return 1;
}
if (usage->type == EV_KEY && usage->code == KEY_FN_ESC && value == 1) {
--
2.42.0
^ permalink raw reply related
* [PATCH AUTOSEL 6.1 13/30] HID: lenovo: Detect quirk-free fw on cptkbd and stop applying workaround
From: Sasha Levin @ 2023-11-07 15:49 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Mikhail Khvainitski, Jiri Kosina, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20231107155024.3766950-1-sashal@kernel.org>
From: Mikhail Khvainitski <me@khvoinitsky.org>
[ Upstream commit 46a0a2c96f0f47628190f122c2e3d879e590bcbe ]
Built-in firmware of cptkbd handles scrolling by itself (when middle
button is pressed) but with issues: it does not support horizontal and
hi-res scrolling and upon middle button release it sends middle button
click even if there was a scrolling event. Commit 3cb5ff0220e3 ("HID:
lenovo: Hide middle-button press until release") workarounds last
issue but it's impossible to workaround scrolling-related issues
without firmware modification.
Likely, Dennis Schneider has reverse engineered the firmware and
provided an instruction on how to patch it [1]. However,
aforementioned workaround prevents userspace (libinput) from knowing
exact moment when middle button has been pressed down and performing
"On-Button scrolling". This commit detects correctly-behaving patched
firmware if cursor movement events has been received during middle
button being pressed and stops applying workaround for this device.
Link: https://hohlerde.org/rauch/en/elektronik/projekte/tpkbd-fix/ [1]
Signed-off-by: Mikhail Khvainitski <me@khvoinitsky.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-lenovo.c | 68 ++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 44763c0da4441..9c1181313e44d 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -51,7 +51,12 @@ struct lenovo_drvdata {
int select_right;
int sensitivity;
int press_speed;
- u8 middlebutton_state; /* 0:Up, 1:Down (undecided), 2:Scrolling */
+ /* 0: Up
+ * 1: Down (undecided)
+ * 2: Scrolling
+ * 3: Patched firmware, disable workaround
+ */
+ u8 middlebutton_state;
bool fn_lock;
};
@@ -668,31 +673,48 @@ static int lenovo_event_cptkbd(struct hid_device *hdev,
{
struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
- /* "wheel" scroll events */
- if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
- usage->code == REL_HWHEEL)) {
- /* Scroll events disable middle-click event */
- cptkbd_data->middlebutton_state = 2;
- return 0;
- }
+ if (cptkbd_data->middlebutton_state != 3) {
+ /* REL_X and REL_Y events during middle button pressed
+ * are only possible on patched, bug-free firmware
+ * so set middlebutton_state to 3
+ * to never apply workaround anymore
+ */
+ if (cptkbd_data->middlebutton_state == 1 &&
+ usage->type == EV_REL &&
+ (usage->code == REL_X || usage->code == REL_Y)) {
+ cptkbd_data->middlebutton_state = 3;
+ /* send middle button press which was hold before */
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 1);
+ input_sync(field->hidinput->input);
+ }
- /* Middle click events */
- if (usage->type == EV_KEY && usage->code == BTN_MIDDLE) {
- if (value == 1) {
- cptkbd_data->middlebutton_state = 1;
- } else if (value == 0) {
- if (cptkbd_data->middlebutton_state == 1) {
- /* No scrolling inbetween, send middle-click */
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 1);
- input_sync(field->hidinput->input);
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 0);
- input_sync(field->hidinput->input);
+ /* "wheel" scroll events */
+ if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
+ usage->code == REL_HWHEEL)) {
+ /* Scroll events disable middle-click event */
+ cptkbd_data->middlebutton_state = 2;
+ return 0;
+ }
+
+ /* Middle click events */
+ if (usage->type == EV_KEY && usage->code == BTN_MIDDLE) {
+ if (value == 1) {
+ cptkbd_data->middlebutton_state = 1;
+ } else if (value == 0) {
+ if (cptkbd_data->middlebutton_state == 1) {
+ /* No scrolling inbetween, send middle-click */
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 1);
+ input_sync(field->hidinput->input);
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 0);
+ input_sync(field->hidinput->input);
+ }
+ cptkbd_data->middlebutton_state = 0;
}
- cptkbd_data->middlebutton_state = 0;
+ return 1;
}
- return 1;
}
if (usage->type == EV_KEY && usage->code == KEY_FN_ESC && value == 1) {
--
2.42.0
^ permalink raw reply related
* [PATCH AUTOSEL 5.15 10/22] HID: lenovo: Detect quirk-free fw on cptkbd and stop applying workaround
From: Sasha Levin @ 2023-11-07 15:51 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Mikhail Khvainitski, Jiri Kosina, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20231107155146.3767610-1-sashal@kernel.org>
From: Mikhail Khvainitski <me@khvoinitsky.org>
[ Upstream commit 46a0a2c96f0f47628190f122c2e3d879e590bcbe ]
Built-in firmware of cptkbd handles scrolling by itself (when middle
button is pressed) but with issues: it does not support horizontal and
hi-res scrolling and upon middle button release it sends middle button
click even if there was a scrolling event. Commit 3cb5ff0220e3 ("HID:
lenovo: Hide middle-button press until release") workarounds last
issue but it's impossible to workaround scrolling-related issues
without firmware modification.
Likely, Dennis Schneider has reverse engineered the firmware and
provided an instruction on how to patch it [1]. However,
aforementioned workaround prevents userspace (libinput) from knowing
exact moment when middle button has been pressed down and performing
"On-Button scrolling". This commit detects correctly-behaving patched
firmware if cursor movement events has been received during middle
button being pressed and stops applying workaround for this device.
Link: https://hohlerde.org/rauch/en/elektronik/projekte/tpkbd-fix/ [1]
Signed-off-by: Mikhail Khvainitski <me@khvoinitsky.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-lenovo.c | 68 ++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 93b1f935e526e..901c1959efed4 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -50,7 +50,12 @@ struct lenovo_drvdata {
int select_right;
int sensitivity;
int press_speed;
- u8 middlebutton_state; /* 0:Up, 1:Down (undecided), 2:Scrolling */
+ /* 0: Up
+ * 1: Down (undecided)
+ * 2: Scrolling
+ * 3: Patched firmware, disable workaround
+ */
+ u8 middlebutton_state;
bool fn_lock;
};
@@ -529,31 +534,48 @@ static int lenovo_event_cptkbd(struct hid_device *hdev,
{
struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
- /* "wheel" scroll events */
- if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
- usage->code == REL_HWHEEL)) {
- /* Scroll events disable middle-click event */
- cptkbd_data->middlebutton_state = 2;
- return 0;
- }
+ if (cptkbd_data->middlebutton_state != 3) {
+ /* REL_X and REL_Y events during middle button pressed
+ * are only possible on patched, bug-free firmware
+ * so set middlebutton_state to 3
+ * to never apply workaround anymore
+ */
+ if (cptkbd_data->middlebutton_state == 1 &&
+ usage->type == EV_REL &&
+ (usage->code == REL_X || usage->code == REL_Y)) {
+ cptkbd_data->middlebutton_state = 3;
+ /* send middle button press which was hold before */
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 1);
+ input_sync(field->hidinput->input);
+ }
- /* Middle click events */
- if (usage->type == EV_KEY && usage->code == BTN_MIDDLE) {
- if (value == 1) {
- cptkbd_data->middlebutton_state = 1;
- } else if (value == 0) {
- if (cptkbd_data->middlebutton_state == 1) {
- /* No scrolling inbetween, send middle-click */
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 1);
- input_sync(field->hidinput->input);
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 0);
- input_sync(field->hidinput->input);
+ /* "wheel" scroll events */
+ if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
+ usage->code == REL_HWHEEL)) {
+ /* Scroll events disable middle-click event */
+ cptkbd_data->middlebutton_state = 2;
+ return 0;
+ }
+
+ /* Middle click events */
+ if (usage->type == EV_KEY && usage->code == BTN_MIDDLE) {
+ if (value == 1) {
+ cptkbd_data->middlebutton_state = 1;
+ } else if (value == 0) {
+ if (cptkbd_data->middlebutton_state == 1) {
+ /* No scrolling inbetween, send middle-click */
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 1);
+ input_sync(field->hidinput->input);
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 0);
+ input_sync(field->hidinput->input);
+ }
+ cptkbd_data->middlebutton_state = 0;
}
- cptkbd_data->middlebutton_state = 0;
+ return 1;
}
- return 1;
}
return 0;
--
2.42.0
^ permalink raw reply related
* [PATCH AUTOSEL 5.15 21/22] HID: Add quirk for Dell Pro Wireless Keyboard and Mouse KM5221W
From: Sasha Levin @ 2023-11-07 15:51 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jiri Kosina, Robert Ayrapetyan, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20231107155146.3767610-1-sashal@kernel.org>
From: Jiri Kosina <jkosina@suse.cz>
[ Upstream commit 62cc9c3cb3ec1bf31cc116146185ed97b450836a ]
This device needs ALWAYS_POLL quirk, otherwise it keeps reconnecting
indefinitely.
Reported-by: Robert Ayrapetyan <robert.ayrapetyan@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-quirks.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 5fceefb3c707e..caca5d6e95d64 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -349,6 +349,7 @@
#define USB_VENDOR_ID_DELL 0x413c
#define USB_DEVICE_ID_DELL_PIXART_USB_OPTICAL_MOUSE 0x301a
+#define USB_DEVICE_ID_DELL_PRO_WIRELESS_KM5221W 0x4503
#define USB_VENDOR_ID_DELORME 0x1163
#define USB_DEVICE_ID_DELORME_EARTHMATE 0x0100
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 96ca7d981ee20..225138a39d323 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -66,6 +66,7 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_STRAFE), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_CREATIVE_SB_OMNI_SURROUND_51), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_PIXART_USB_OPTICAL_MOUSE), HID_QUIRK_ALWAYS_POLL },
+ { HID_USB_DEVICE(USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_PRO_WIRELESS_KM5221W), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_DMI, USB_DEVICE_ID_DMI_ENC), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRACAL_RAPHNET, USB_DEVICE_ID_RAPHNET_2NES2SNES), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRACAL_RAPHNET, USB_DEVICE_ID_RAPHNET_4NES4SNES), HID_QUIRK_MULTI_INPUT },
--
2.42.0
^ permalink raw reply related
* [PATCH AUTOSEL 5.10 08/16] HID: lenovo: Detect quirk-free fw on cptkbd and stop applying workaround
From: Sasha Levin @ 2023-11-07 15:52 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Mikhail Khvainitski, Jiri Kosina, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20231107155249.3768098-1-sashal@kernel.org>
From: Mikhail Khvainitski <me@khvoinitsky.org>
[ Upstream commit 46a0a2c96f0f47628190f122c2e3d879e590bcbe ]
Built-in firmware of cptkbd handles scrolling by itself (when middle
button is pressed) but with issues: it does not support horizontal and
hi-res scrolling and upon middle button release it sends middle button
click even if there was a scrolling event. Commit 3cb5ff0220e3 ("HID:
lenovo: Hide middle-button press until release") workarounds last
issue but it's impossible to workaround scrolling-related issues
without firmware modification.
Likely, Dennis Schneider has reverse engineered the firmware and
provided an instruction on how to patch it [1]. However,
aforementioned workaround prevents userspace (libinput) from knowing
exact moment when middle button has been pressed down and performing
"On-Button scrolling". This commit detects correctly-behaving patched
firmware if cursor movement events has been received during middle
button being pressed and stops applying workaround for this device.
Link: https://hohlerde.org/rauch/en/elektronik/projekte/tpkbd-fix/ [1]
Signed-off-by: Mikhail Khvainitski <me@khvoinitsky.org>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-lenovo.c | 68 ++++++++++++++++++++++++++--------------
1 file changed, 45 insertions(+), 23 deletions(-)
diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c
index 0ff03fed97709..71f7b0d539df5 100644
--- a/drivers/hid/hid-lenovo.c
+++ b/drivers/hid/hid-lenovo.c
@@ -50,7 +50,12 @@ struct lenovo_drvdata {
int select_right;
int sensitivity;
int press_speed;
- u8 middlebutton_state; /* 0:Up, 1:Down (undecided), 2:Scrolling */
+ /* 0: Up
+ * 1: Down (undecided)
+ * 2: Scrolling
+ * 3: Patched firmware, disable workaround
+ */
+ u8 middlebutton_state;
bool fn_lock;
};
@@ -478,31 +483,48 @@ static int lenovo_event_cptkbd(struct hid_device *hdev,
{
struct lenovo_drvdata *cptkbd_data = hid_get_drvdata(hdev);
- /* "wheel" scroll events */
- if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
- usage->code == REL_HWHEEL)) {
- /* Scroll events disable middle-click event */
- cptkbd_data->middlebutton_state = 2;
- return 0;
- }
+ if (cptkbd_data->middlebutton_state != 3) {
+ /* REL_X and REL_Y events during middle button pressed
+ * are only possible on patched, bug-free firmware
+ * so set middlebutton_state to 3
+ * to never apply workaround anymore
+ */
+ if (cptkbd_data->middlebutton_state == 1 &&
+ usage->type == EV_REL &&
+ (usage->code == REL_X || usage->code == REL_Y)) {
+ cptkbd_data->middlebutton_state = 3;
+ /* send middle button press which was hold before */
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 1);
+ input_sync(field->hidinput->input);
+ }
- /* Middle click events */
- if (usage->type == EV_KEY && usage->code == BTN_MIDDLE) {
- if (value == 1) {
- cptkbd_data->middlebutton_state = 1;
- } else if (value == 0) {
- if (cptkbd_data->middlebutton_state == 1) {
- /* No scrolling inbetween, send middle-click */
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 1);
- input_sync(field->hidinput->input);
- input_event(field->hidinput->input,
- EV_KEY, BTN_MIDDLE, 0);
- input_sync(field->hidinput->input);
+ /* "wheel" scroll events */
+ if (usage->type == EV_REL && (usage->code == REL_WHEEL ||
+ usage->code == REL_HWHEEL)) {
+ /* Scroll events disable middle-click event */
+ cptkbd_data->middlebutton_state = 2;
+ return 0;
+ }
+
+ /* Middle click events */
+ if (usage->type == EV_KEY && usage->code == BTN_MIDDLE) {
+ if (value == 1) {
+ cptkbd_data->middlebutton_state = 1;
+ } else if (value == 0) {
+ if (cptkbd_data->middlebutton_state == 1) {
+ /* No scrolling inbetween, send middle-click */
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 1);
+ input_sync(field->hidinput->input);
+ input_event(field->hidinput->input,
+ EV_KEY, BTN_MIDDLE, 0);
+ input_sync(field->hidinput->input);
+ }
+ cptkbd_data->middlebutton_state = 0;
}
- cptkbd_data->middlebutton_state = 0;
+ return 1;
}
- return 1;
}
return 0;
--
2.42.0
^ permalink raw reply related
* [PATCH AUTOSEL 5.10 15/16] HID: Add quirk for Dell Pro Wireless Keyboard and Mouse KM5221W
From: Sasha Levin @ 2023-11-07 15:52 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jiri Kosina, Robert Ayrapetyan, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20231107155249.3768098-1-sashal@kernel.org>
From: Jiri Kosina <jkosina@suse.cz>
[ Upstream commit 62cc9c3cb3ec1bf31cc116146185ed97b450836a ]
This device needs ALWAYS_POLL quirk, otherwise it keeps reconnecting
indefinitely.
Reported-by: Robert Ayrapetyan <robert.ayrapetyan@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-quirks.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 6712d99ad80da..7c688d7f8ccff 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -345,6 +345,7 @@
#define USB_VENDOR_ID_DELL 0x413c
#define USB_DEVICE_ID_DELL_PIXART_USB_OPTICAL_MOUSE 0x301a
+#define USB_DEVICE_ID_DELL_PRO_WIRELESS_KM5221W 0x4503
#define USB_VENDOR_ID_DELORME 0x1163
#define USB_DEVICE_ID_DELORME_EARTHMATE 0x0100
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 4229e5de06745..787349f2de01d 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -66,6 +66,7 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_STRAFE), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_CREATIVE_SB_OMNI_SURROUND_51), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_PIXART_USB_OPTICAL_MOUSE), HID_QUIRK_ALWAYS_POLL },
+ { HID_USB_DEVICE(USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_PRO_WIRELESS_KM5221W), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_DMI, USB_DEVICE_ID_DMI_ENC), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRACAL_RAPHNET, USB_DEVICE_ID_RAPHNET_2NES2SNES), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRACAL_RAPHNET, USB_DEVICE_ID_RAPHNET_4NES4SNES), HID_QUIRK_MULTI_INPUT },
--
2.42.0
^ permalink raw reply related
* [PATCH AUTOSEL 5.4 12/12] HID: Add quirk for Dell Pro Wireless Keyboard and Mouse KM5221W
From: Sasha Levin @ 2023-11-07 15:53 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jiri Kosina, Robert Ayrapetyan, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20231107155343.3768464-1-sashal@kernel.org>
From: Jiri Kosina <jkosina@suse.cz>
[ Upstream commit 62cc9c3cb3ec1bf31cc116146185ed97b450836a ]
This device needs ALWAYS_POLL quirk, otherwise it keeps reconnecting
indefinitely.
Reported-by: Robert Ayrapetyan <robert.ayrapetyan@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-quirks.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 64842926aff64..182068bf28c0a 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -350,6 +350,7 @@
#define USB_VENDOR_ID_DELL 0x413c
#define USB_DEVICE_ID_DELL_PIXART_USB_OPTICAL_MOUSE 0x301a
+#define USB_DEVICE_ID_DELL_PRO_WIRELESS_KM5221W 0x4503
#define USB_VENDOR_ID_DELORME 0x1163
#define USB_DEVICE_ID_DELORME_EARTHMATE 0x0100
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 83c3322fcf187..fae784df084d5 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -66,6 +66,7 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_STRAFE), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_CREATIVE_SB_OMNI_SURROUND_51), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_PIXART_USB_OPTICAL_MOUSE), HID_QUIRK_ALWAYS_POLL },
+ { HID_USB_DEVICE(USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_PRO_WIRELESS_KM5221W), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_DMI, USB_DEVICE_ID_DMI_ENC), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRACAL_RAPHNET, USB_DEVICE_ID_RAPHNET_2NES2SNES), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRACAL_RAPHNET, USB_DEVICE_ID_RAPHNET_4NES4SNES), HID_QUIRK_MULTI_INPUT },
--
2.42.0
^ permalink raw reply related
* [PATCH AUTOSEL 4.19 11/11] HID: Add quirk for Dell Pro Wireless Keyboard and Mouse KM5221W
From: Sasha Levin @ 2023-11-07 15:54 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jiri Kosina, Robert Ayrapetyan, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20231107155430.3768779-1-sashal@kernel.org>
From: Jiri Kosina <jkosina@suse.cz>
[ Upstream commit 62cc9c3cb3ec1bf31cc116146185ed97b450836a ]
This device needs ALWAYS_POLL quirk, otherwise it keeps reconnecting
indefinitely.
Reported-by: Robert Ayrapetyan <robert.ayrapetyan@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-quirks.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index a9d6f8acf70b5..93faf083e550b 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -343,6 +343,7 @@
#define USB_VENDOR_ID_DELL 0x413c
#define USB_DEVICE_ID_DELL_PIXART_USB_OPTICAL_MOUSE 0x301a
+#define USB_DEVICE_ID_DELL_PRO_WIRELESS_KM5221W 0x4503
#define USB_VENDOR_ID_DELORME 0x1163
#define USB_DEVICE_ID_DELORME_EARTHMATE 0x0100
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index a2ab338166e61..0b85f95810b30 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -68,6 +68,7 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_STRAFE), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_CREATIVE_SB_OMNI_SURROUND_51), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_PIXART_USB_OPTICAL_MOUSE), HID_QUIRK_ALWAYS_POLL },
+ { HID_USB_DEVICE(USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_PRO_WIRELESS_KM5221W), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_DMI, USB_DEVICE_ID_DMI_ENC), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRACAL_RAPHNET, USB_DEVICE_ID_RAPHNET_2NES2SNES), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRACAL_RAPHNET, USB_DEVICE_ID_RAPHNET_4NES4SNES), HID_QUIRK_MULTI_INPUT },
--
2.42.0
^ permalink raw reply related
* [PATCH AUTOSEL 6.1 28/30] HID: Add quirk for Dell Pro Wireless Keyboard and Mouse KM5221W
From: Sasha Levin @ 2023-11-07 15:50 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jiri Kosina, Robert Ayrapetyan, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20231107155024.3766950-1-sashal@kernel.org>
From: Jiri Kosina <jkosina@suse.cz>
[ Upstream commit 62cc9c3cb3ec1bf31cc116146185ed97b450836a ]
This device needs ALWAYS_POLL quirk, otherwise it keeps reconnecting
indefinitely.
Reported-by: Robert Ayrapetyan <robert.ayrapetyan@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-quirks.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 9a17e5cc3539b..130fc5f341422 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -365,6 +365,7 @@
#define USB_VENDOR_ID_DELL 0x413c
#define USB_DEVICE_ID_DELL_PIXART_USB_OPTICAL_MOUSE 0x301a
+#define USB_DEVICE_ID_DELL_PRO_WIRELESS_KM5221W 0x4503
#define USB_VENDOR_ID_DELORME 0x1163
#define USB_DEVICE_ID_DELORME_EARTHMATE 0x0100
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index f8f20a7c24b17..056bb32091285 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -66,6 +66,7 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_STRAFE), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_CREATIVE_SB_OMNI_SURROUND_51), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_PIXART_USB_OPTICAL_MOUSE), HID_QUIRK_ALWAYS_POLL },
+ { HID_USB_DEVICE(USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_PRO_WIRELESS_KM5221W), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_DMI, USB_DEVICE_ID_DMI_ENC), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRACAL_RAPHNET, USB_DEVICE_ID_RAPHNET_2NES2SNES), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRACAL_RAPHNET, USB_DEVICE_ID_RAPHNET_4NES4SNES), HID_QUIRK_MULTI_INPUT },
--
2.42.0
^ permalink raw reply related
* Re: [PATCH] HID: apple: add Jamesdonkey and A3R to non-apple keyboards list
From: Yihong Cao @ 2023-11-07 16:08 UTC (permalink / raw)
To: Rahul Rameshbabu
Cc: Jiri Kosina, Benjamin Tissoires, linux-input, linux-kernel
In-Reply-To: <87a5rr1sqf.fsf@protonmail.com>
On Mon, Nov 06, 2023 at 03:11:09AM +0000, Rahul Rameshbabu wrote:
> On Mon, 30 Oct, 2023 01:05:38 +0800 "Yihong Cao" <caoyihong4@outlook.com> wrote:
> > Jamesdonkey A3R keyboard is identified as "Jamesdonkey A3R" in wired
> > mode, "A3R-U" in wireless mode and "A3R" in bluetooth mode. Adding them
> > to non-apple keyboards fixes function key.
> >
> > Signed-off-by: Yihong Cao <caoyihong4@outlook.com>
> > ---
> > drivers/hid/hid-apple.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/hid/hid-apple.c b/drivers/hid/hid-apple.c
> > index 3ca45975c686..d9e9829b2200 100644
> > --- a/drivers/hid/hid-apple.c
> > +++ b/drivers/hid/hid-apple.c
> > @@ -345,6 +345,8 @@ static const struct apple_non_apple_keyboard non_apple_keyboards[] = {
> > { "AONE" },
> > { "GANSS" },
> > { "Hailuck" },
> > + { "Jamesdonkey" },
>
> Sorry, maybe I misunderstood the commit message. In wired mode, if the
> keyboard is identified as "Jamesdonkey A3R", shouldn't this value be
> "Jamesdonkey A3R" instead of "Jamesdonkey"?
>
Hi!
"Jamesdonkey" is the manufacturer and "A3R" is the model. I think adding
manufacturer to non-apple list is suggested, just like commit
c4444d8749f696384947192b602718fa310c1caf,
20afcc462579c0bd79a59ab2b87b82ffa833d118, and
a0a05054583fed17f522172e101594f1ff265463 did.
However, my keyboard's hardware is buggy, in wireless and wired mode, the
manufacturer is empty, only model name exists. For your reference, the
result of `lsusb -v` is pasted below.
In wired mode, `lsusb -v` shows:
Bus 003 Device 002: ID 05ac:024f Apple, Inc. Aluminium Keyboard (ANSI)
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 2.00
bDeviceClass 0
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 64
idVendor 0x05ac Apple, Inc.
idProduct 0x024f Aluminium Keyboard (ANSI)
bcdDevice 1.26
iManufacturer 1 Jamesdonkey
iProduct 2 A3R
iSerial 0
bNumConfigurations 1
In wireless mode:
Bus 001 Device 003: ID 05ac:024f Apple, Inc. Aluminium Keyboard (ANSI)
Device Descriptor:
bLength 18
bDescriptorType 1
bcdUSB 1.10
bDeviceClass 0
bDeviceSubClass 0
bDeviceProtocol 0
bMaxPacketSize0 8
idVendor 0x05ac Apple, Inc.
idProduct 0x024f Aluminium Keyboard (ANSI)
bcdDevice 2.00
iManufacturer 0
iProduct 1 A3R-U
And `dmesg` shows:
[ 1779.692121] input: A3R-U as /devices/pci0000:00/0000:00:08.1/0000:06:00.3/usb1/1-2/1-2:1.0/0003:05AC:024F.0008/input/input35
[ 1779.749037] apple 0003:05AC:024F.0008: input,hidraw2: USB HID v1.10 Keyboard [A3R-U] on usb-0000:06:00.3-2/input0
In bluetooth mode, the iProduct is "A3R".
Adding "A3R" to non-apple list makes keyboard to work in both wireless
and bluetooth mode.
Best wishes,
Yihong Cao
> > + { "A3R" },
> > };
> >
> > static bool apple_is_non_apple_keyboard(struct hid_device *hdev)
>
> --
> Thanks,
>
> Rahul Rameshbabu
>
^ permalink raw reply
* [PATCH AUTOSEL 6.5 32/34] HID: Add quirk for Dell Pro Wireless Keyboard and Mouse KM5221W
From: Sasha Levin @ 2023-11-07 15:48 UTC (permalink / raw)
To: linux-kernel, stable
Cc: Jiri Kosina, Robert Ayrapetyan, Sasha Levin, jikos,
benjamin.tissoires, linux-input
In-Reply-To: <20231107154846.3766119-1-sashal@kernel.org>
From: Jiri Kosina <jkosina@suse.cz>
[ Upstream commit 62cc9c3cb3ec1bf31cc116146185ed97b450836a ]
This device needs ALWAYS_POLL quirk, otherwise it keeps reconnecting
indefinitely.
Reported-by: Robert Ayrapetyan <robert.ayrapetyan@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
drivers/hid/hid-ids.h | 1 +
drivers/hid/hid-quirks.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index cc0d0186a0d95..fafc40ecfd200 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -366,6 +366,7 @@
#define USB_VENDOR_ID_DELL 0x413c
#define USB_DEVICE_ID_DELL_PIXART_USB_OPTICAL_MOUSE 0x301a
+#define USB_DEVICE_ID_DELL_PRO_WIRELESS_KM5221W 0x4503
#define USB_VENDOR_ID_DELORME 0x1163
#define USB_DEVICE_ID_DELORME_EARTHMATE 0x0100
diff --git a/drivers/hid/hid-quirks.c b/drivers/hid/hid-quirks.c
index 3983b4f282f8f..5a48fcaa32f00 100644
--- a/drivers/hid/hid-quirks.c
+++ b/drivers/hid/hid-quirks.c
@@ -66,6 +66,7 @@ static const struct hid_device_id hid_quirks[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_CORSAIR, USB_DEVICE_ID_CORSAIR_STRAFE), HID_QUIRK_NO_INIT_REPORTS | HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_CREATIVELABS, USB_DEVICE_ID_CREATIVE_SB_OMNI_SURROUND_51), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_PIXART_USB_OPTICAL_MOUSE), HID_QUIRK_ALWAYS_POLL },
+ { HID_USB_DEVICE(USB_VENDOR_ID_DELL, USB_DEVICE_ID_DELL_PRO_WIRELESS_KM5221W), HID_QUIRK_ALWAYS_POLL },
{ HID_USB_DEVICE(USB_VENDOR_ID_DMI, USB_DEVICE_ID_DMI_ENC), HID_QUIRK_NOGET },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRACAL_RAPHNET, USB_DEVICE_ID_RAPHNET_2NES2SNES), HID_QUIRK_MULTI_INPUT },
{ HID_USB_DEVICE(USB_VENDOR_ID_DRACAL_RAPHNET, USB_DEVICE_ID_RAPHNET_4NES4SNES), HID_QUIRK_MULTI_INPUT },
--
2.42.0
^ permalink raw reply related
* Re: [PATCH v7 2/2] input: joystick: driver for Adafruit Seesaw Gamepad
From: kernel test robot @ 2023-11-07 19:34 UTC (permalink / raw)
To: Anshul Dalal, linux-input, devicetree
Cc: oe-kbuild-all, Anshul Dalal, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Thomas Weißschuh,
Jeff LaBundy, Shuah Khan, linux-kernel-mentees, linux-kernel
In-Reply-To: <20231106164134.114668-2-anshulusr@gmail.com>
Hi Anshul,
kernel test robot noticed the following build errors:
[auto build test ERROR on dtor-input/next]
[also build test ERROR on dtor-input/for-linus hid/for-next linus/master v6.6 next-20231107]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Anshul-Dalal/input-joystick-driver-for-Adafruit-Seesaw-Gamepad/20231107-004813
base: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
patch link: https://lore.kernel.org/r/20231106164134.114668-2-anshulusr%40gmail.com
patch subject: [PATCH v7 2/2] input: joystick: driver for Adafruit Seesaw Gamepad
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20231108/202311080319.73n9aQfj-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231108/202311080319.73n9aQfj-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311080319.73n9aQfj-lkp@intel.com/
All errors (new ones prefixed by >>):
>> make[6]: *** No rule to make target 'drivers/input/joystick/adafruit_seesaw.o', needed by 'drivers/input/joystick/built-in.a'.
make[6]: Target 'drivers/input/joystick/' not remade because of errors.
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* [PATCH v8 1/2] dt-bindings: input: bindings for Adafruit Seesaw Gamepad
From: Anshul Dalal @ 2023-11-08 0:53 UTC (permalink / raw)
To: linux-input, devicetree
Cc: Anshul Dalal, Conor Dooley, Dmitry Torokhov,
Thomas Weißschuh, linux-kernel, Krzysztof Kozlowski,
Conor Dooley, Rob Herring, Krzysztof Kozlowski, Jeff LaBundy,
linux-kernel-mentees
Adds bindings for the Adafruit Seesaw Gamepad.
The gamepad functions as an i2c device with the default address of 0x50
and has an IRQ pin that can be enabled in the driver to allow for a rising
edge trigger on each button press or joystick movement.
Product page:
https://www.adafruit.com/product/5743
Arduino driver:
https://github.com/adafruit/Adafruit_Seesaw
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
---
Changes for v8:
- no updates
Changes for v7:
- no updates
Changes for v6:
- no updates
Changes for v5:
- Added link to the datasheet
Changes for v4:
- Fixed the URI for the id field
- Added `interrupts` property
Changes for v3:
- Updated id field to reflect updated file name from previous version
- Added `reg` property
Changes for v2:
- Renamed file to `adafruit,seesaw-gamepad.yaml`
- Removed quotes for `$id` and `$schema`
- Removed "Bindings for" from the description
- Changed node name to the generic name "joystick"
- Changed compatible to 'adafruit,seesaw-gamepad' instead of
'adafruit,seesaw_gamepad'
---
.../input/adafruit,seesaw-gamepad.yaml | 60 +++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
diff --git a/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
new file mode 100644
index 000000000000..3f0d1c5a3b9b
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
@@ -0,0 +1,60 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/adafruit,seesaw-gamepad.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Adafruit Mini I2C Gamepad with seesaw
+
+maintainers:
+ - Anshul Dalal <anshulusr@gmail.com>
+
+description: |
+ Adafruit Mini I2C Gamepad
+
+ +-----------------------------+
+ | ___ |
+ | / \ (X) |
+ | | S | __ __ (Y) (A) |
+ | \___/ |ST| |SE| (B) |
+ | |
+ +-----------------------------+
+
+ S -> 10-bit percision bidirectional analog joystick
+ ST -> Start
+ SE -> Select
+ X, A, B, Y -> Digital action buttons
+
+ Datasheet: https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf
+ Product page: https://www.adafruit.com/product/5743
+ Arduino Driver: https://github.com/adafruit/Adafruit_Seesaw
+
+properties:
+ compatible:
+ const: adafruit,seesaw-gamepad
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+ description:
+ The gamepad's IRQ pin triggers a rising edge if interrupts are enabled.
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ joystick@50 {
+ compatible = "adafruit,seesaw-gamepad";
+ reg = <0x50>;
+ };
+ };
--
2.42.0
^ permalink raw reply related
* [PATCH v8 2/2] input: joystick: driver for Adafruit Seesaw Gamepad
From: Anshul Dalal @ 2023-11-08 0:53 UTC (permalink / raw)
To: linux-input, devicetree
Cc: Anshul Dalal, Conor Dooley, Dmitry Torokhov,
Thomas Weißschuh, linux-kernel, Krzysztof Kozlowski,
Conor Dooley, Rob Herring, Krzysztof Kozlowski, Jeff LaBundy,
linux-kernel-mentees
In-Reply-To: <20231108005337.45069-1-anshulusr@gmail.com>
Adds a driver for a mini gamepad that communicates over i2c, the gamepad
has bidirectional thumb stick input and six buttons.
The gamepad chip utilizes the open framework from Adafruit called 'Seesaw'
to transmit the ADC data for the joystick and digital pin state for the
buttons. I have only implemented the functionality required to receive the
thumb stick and button state.
Steps in reading the gamepad state over i2c:
1. Reset the registers
2. Set the pin mode of the pins specified by the `BUTTON_MASK` to input
`BUTTON_MASK`: A bit-map for the six digital pins internally
connected to the joystick buttons.
3. Enable internal pullup resistors for the `BUTTON_MASK`
4. Bulk set the pin state HIGH for `BUTTON_MASK`
5. Poll the device for button and joystick state done by:
`seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)`
Product page:
https://www.adafruit.com/product/5743
Arduino driver:
https://github.com/adafruit/Adafruit_Seesaw
Driver tested on RPi Zero 2W
Signed-off-by: Anshul Dalal <anshulusr@gmail.com>
---
Changes for v8:
- Updated invalid references to `adafruit_seesaw` to `adafruit-seesaw`
Changes for v7:
adafruit-seesaw.c
- Fixed formatting for macro definitions
- Made SEESAW_BUTTON_MASK static const
- Removed __be16 type for x and y fields of seesaw_data
- Used sparse_keymap implementation instead of custom keymap
- Used i2c_msg instead of i2c_master_send and recv in
seesaw_register_read
- Use temporary variable `adc_data` to store data received from ADC
- Changed read_buf from u8[4] to __be32
- Use usleep_range instead of msleep
- Removed 'Reviewed-by: Thomas Weißschuh' due to large number of changes
since last review
Kconfig:
- Added `select INPUT_SPARSEKMAP`
Changes for v6:
- Added TODO
- Removed `clang-format` directives
- Namespaced device buttons
- Removed `char physical_path[32]` field from `struct seesaw_gamepad`
- Added `packed` attribute to `struct seesaw_data`
- Moved from having booleans per button to single `u32 button_state`
- Added `seesaw_button_description` array to directly associate device
buttons with respective keycodes
- Added wrapper functions `seesaw_register_` around `i2c_master_` API
- Ratelimited input error reporting with `dev_err_ratelimited`
- Removed `of_device_id`
Changes for v5:
- Added link to the datasheet
- Added debug log message when `seesaw_read_data` fails
Changes for v4:
- Changed `1UL << BUTTON_` to BIT(BUTTON_)
- Removed `hardware_id` field from `struct seesaw_gamepad`
- Removed redundant checks for the number of bytes written and received by
`i2c_master_send` and `i2c_master_recv`
- Used `get_unaligned_be32` to instantiate `u32 result` from `read_buf`
- Changed `result & (1UL << BUTTON_)` to
`test_bit(BUTTON_, (long *)&result)`
- Changed `KBUILD_MODNAME` in id-tables to `SEESAW_DEVICE_NAME`
- Fixed formatting issues
- Changed button reporting:
Since the gamepad had the action buttons in a non-standard layout:
(X)
(Y) (A)
(B)
Therefore moved to using generic directional action button event codes
instead of BTN_[ABXY].
Changes for v3:
- no updates
Changes for v2:
adafruit-seesaw.c:
- Renamed file from 'adafruit_seesaw.c'
- Changed device name from 'seesaw_gamepad' to 'seesaw-gamepad'
- Changed count parameter for receiving joystick x on line 118:
`2` to `sizeof(write_buf)`
- Fixed invalid buffer size on line 123 and 126:
`data->y` to `sizeof(data->y)`
- Added comment for the `mdelay(10)` on line 169
- Changed inconsistent indentation on line 271
Kconfig:
- Fixed indentation for the help text
- Updated module name
Makefile:
- Updated module object file name
MAINTAINERS:
- Updated file name for the driver and bindings
---
MAINTAINERS | 7 +
drivers/input/joystick/Kconfig | 10 +
drivers/input/joystick/Makefile | 1 +
drivers/input/joystick/adafruit-seesaw.c | 315 +++++++++++++++++++++++
4 files changed, 333 insertions(+)
create mode 100644 drivers/input/joystick/adafruit-seesaw.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 81d5fc0bba68..b3f101edc24b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -441,6 +441,13 @@ W: http://wiki.analog.com/AD7879
W: https://ez.analog.com/linux-software-drivers
F: drivers/input/touchscreen/ad7879.c
+ADAFRUIT MINI I2C GAMEPAD
+M: Anshul Dalal <anshulusr@gmail.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: Documentation/devicetree/bindings/input/adafruit,seesaw-gamepad.yaml
+F: drivers/input/joystick/adafruit-seesaw.c
+
ADDRESS SPACE LAYOUT RANDOMIZATION (ASLR)
M: Jiri Kosina <jikos@kernel.org>
S: Maintained
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index ac6925ce8366..7755e5b454d2 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -412,4 +412,14 @@ config JOYSTICK_SENSEHAT
To compile this driver as a module, choose M here: the
module will be called sensehat_joystick.
+config JOYSTICK_SEESAW
+ tristate "Adafruit Mini I2C Gamepad with Seesaw"
+ depends on I2C
+ select INPUT_SPARSEKMAP
+ help
+ Say Y here if you want to use the Adafruit Mini I2C Gamepad.
+
+ To compile this driver as a module, choose M here: the module will be
+ called adafruit-seesaw.
+
endif
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 3937535f0098..9976f596a920 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_JOYSTICK_N64) += n64joy.o
obj-$(CONFIG_JOYSTICK_PSXPAD_SPI) += psxpad-spi.o
obj-$(CONFIG_JOYSTICK_PXRC) += pxrc.o
obj-$(CONFIG_JOYSTICK_QWIIC) += qwiic-joystick.o
+obj-$(CONFIG_JOYSTICK_SEESAW) += adafruit-seesaw.o
obj-$(CONFIG_JOYSTICK_SENSEHAT) += sensehat-joystick.o
obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
diff --git a/drivers/input/joystick/adafruit-seesaw.c b/drivers/input/joystick/adafruit-seesaw.c
new file mode 100644
index 000000000000..8e8ef26a585f
--- /dev/null
+++ b/drivers/input/joystick/adafruit-seesaw.c
@@ -0,0 +1,315 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2023 Anshul Dalal <anshulusr@gmail.com>
+ *
+ * Driver for Adafruit Mini I2C Gamepad
+ *
+ * Based on the work of:
+ * Oleh Kravchenko (Sparkfun Qwiic Joystick driver)
+ *
+ * Datasheet: https://cdn-learn.adafruit.com/downloads/pdf/gamepad-qt.pdf
+ * Product page: https://www.adafruit.com/product/5743
+ * Firmware and hardware sources: https://github.com/adafruit/Adafruit_Seesaw
+ *
+ * TODO:
+ * - Add interrupt support
+ */
+
+#include <asm-generic/unaligned.h>
+#include <linux/bits.h>
+#include <linux/delay.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/input/sparse-keymap.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+#define SEESAW_DEVICE_NAME "seesaw-gamepad"
+
+#define SEESAW_STATUS_BASE 0x00
+#define SEESAW_GPIO_BASE 0x01
+#define SEESAW_ADC_BASE 0x09
+
+#define SEESAW_GPIO_DIRCLR_BULK 0x03
+#define SEESAW_GPIO_BULK 0x04
+#define SEESAW_GPIO_BULK_SET 0x05
+#define SEESAW_GPIO_PULLENSET 0x0b
+
+#define SEESAW_STATUS_HW_ID 0x01
+#define SEESAW_STATUS_SWRST 0x7f
+
+#define SEESAW_ADC_OFFSET 0x07
+
+#define SEESAW_BUTTON_A 0x05
+#define SEESAW_BUTTON_B 0x01
+#define SEESAW_BUTTON_X 0x06
+#define SEESAW_BUTTON_Y 0x02
+#define SEESAW_BUTTON_START 0x10
+#define SEESAW_BUTTON_SELECT 0x00
+
+#define SEESAW_ANALOG_X 0x0e
+#define SEESAW_ANALOG_Y 0x0f
+
+#define SEESAW_JOYSTICK_MAX_AXIS 1023
+#define SEESAW_JOYSTICK_FUZZ 2
+#define SEESAW_JOYSTICK_FLAT 4
+
+#define SEESAW_GAMEPAD_POLL_INTERVAL 16
+#define SEESAW_GAMEPAD_POLL_MIN 8
+#define SEESAW_GAMEPAD_POLL_MAX 32
+
+static const u32 SEESAW_BUTTON_MASK =
+ BIT(SEESAW_BUTTON_A) | BIT(SEESAW_BUTTON_B) | BIT(SEESAW_BUTTON_X) |
+ BIT(SEESAW_BUTTON_Y) | BIT(SEESAW_BUTTON_START) |
+ BIT(SEESAW_BUTTON_SELECT);
+
+struct seesaw_gamepad {
+ struct input_dev *input_dev;
+ struct i2c_client *i2c_client;
+};
+
+struct seesaw_data {
+ u16 x;
+ u16 y;
+ u32 button_state;
+};
+
+struct seesaw_button_description {
+ unsigned int code;
+ unsigned int bit;
+};
+
+static const struct key_entry seesaw_buttons_new[] = {
+ { KE_KEY, SEESAW_BUTTON_A, .keycode = BTN_SOUTH },
+ { KE_KEY, SEESAW_BUTTON_B, .keycode = BTN_EAST },
+ { KE_KEY, SEESAW_BUTTON_X, .keycode = BTN_NORTH },
+ { KE_KEY, SEESAW_BUTTON_Y, .keycode = BTN_WEST },
+ { KE_KEY, SEESAW_BUTTON_START, .keycode = BTN_START },
+ { KE_KEY, SEESAW_BUTTON_SELECT, .keycode = BTN_SELECT },
+ { KE_END, 0 },
+};
+
+static int seesaw_register_read(struct i2c_client *client, u8 register_high,
+ u8 register_low, char *buf, int count)
+{
+ int ret;
+ u8 register_buf[2] = { register_high, register_low };
+
+ struct i2c_msg message_buf[2] = {
+ {
+ .addr = client->addr,
+ .flags = client->flags,
+ .len = sizeof(register_buf),
+ .buf = register_buf
+ },
+ {
+ .addr = client->addr,
+ .flags = client->flags | I2C_M_RD,
+ .len = count,
+ .buf = buf
+ },
+ };
+ ret = i2c_transfer(client->adapter, message_buf,
+ ARRAY_SIZE(message_buf));
+
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int seesaw_register_write_u8(struct i2c_client *client, u8 register_high,
+ u8 register_low, u8 value)
+{
+ int ret;
+ u8 write_buf[3] = { register_high, register_low, value };
+
+ ret = i2c_master_send(client, write_buf, sizeof(write_buf));
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int seesaw_register_write_u32(struct i2c_client *client,
+ u8 register_high, u8 register_low,
+ u32 value)
+{
+ int ret;
+ u8 write_buf[6] = { register_high, register_low };
+
+ put_unaligned_be32(value, write_buf + 2);
+ ret = i2c_master_send(client, write_buf, sizeof(write_buf));
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static int seesaw_read_data(struct i2c_client *client, struct seesaw_data *data)
+{
+ int ret;
+ __be16 adc_data;
+ __be32 read_buf;
+
+ ret = seesaw_register_read(client, SEESAW_GPIO_BASE, SEESAW_GPIO_BULK,
+ (char *)&read_buf, sizeof(read_buf));
+ if (ret)
+ return ret;
+
+ data->button_state = ~be32_to_cpu(read_buf);
+
+ ret = seesaw_register_read(client, SEESAW_ADC_BASE,
+ SEESAW_ADC_OFFSET + SEESAW_ANALOG_X,
+ (char *)&adc_data, sizeof(adc_data));
+ if (ret)
+ return ret;
+ /*
+ * ADC reads left as max and right as 0, must be reversed since kernel
+ * expects reports in opposite order.
+ */
+ data->x = SEESAW_JOYSTICK_MAX_AXIS - be16_to_cpu(adc_data);
+
+ ret = seesaw_register_read(client, SEESAW_ADC_BASE,
+ SEESAW_ADC_OFFSET + SEESAW_ANALOG_Y,
+ (char *)&adc_data, sizeof(adc_data));
+ if (ret)
+ return ret;
+ data->y = be16_to_cpu(adc_data);
+
+ return 0;
+}
+
+static void seesaw_poll(struct input_dev *input)
+{
+ int err, i;
+ struct seesaw_gamepad *private = input_get_drvdata(input);
+ struct seesaw_data data;
+
+ err = seesaw_read_data(private->i2c_client, &data);
+ if (err) {
+ dev_err_ratelimited(&input->dev,
+ "failed to read joystick state: %d\n", err);
+ return;
+ }
+
+ input_report_abs(input, ABS_X, data.x);
+ input_report_abs(input, ABS_Y, data.y);
+
+ for_each_set_bit(i, (long *)&SEESAW_BUTTON_MASK,
+ BITS_PER_TYPE(SEESAW_BUTTON_MASK)) {
+ if (!sparse_keymap_report_event(
+ input, i, data.button_state & BIT(i), false)) {
+ dev_err_ratelimited(&input->dev,
+ "failed to report keymap event");
+ return;
+ };
+ }
+
+ input_sync(input);
+}
+
+static int seesaw_probe(struct i2c_client *client)
+{
+ int ret;
+ u8 hardware_id;
+ struct seesaw_gamepad *seesaw;
+
+ ret = seesaw_register_write_u8(client, SEESAW_STATUS_BASE,
+ SEESAW_STATUS_SWRST, 0xFF);
+ if (ret)
+ return ret;
+
+ /* Wait for the registers to reset before proceeding */
+ usleep_range(10000, 15000);
+
+ seesaw = devm_kzalloc(&client->dev, sizeof(*seesaw), GFP_KERNEL);
+ if (!seesaw)
+ return -ENOMEM;
+
+ ret = seesaw_register_read(client, SEESAW_STATUS_BASE,
+ SEESAW_STATUS_HW_ID, &hardware_id,
+ sizeof(hardware_id));
+ if (ret)
+ return ret;
+
+ dev_dbg(&client->dev, "Adafruit Seesaw Gamepad, Hardware ID: %02x\n",
+ hardware_id);
+
+ /* Set Pin Mode to input and enable pull-up resistors */
+ ret = seesaw_register_write_u32(client, SEESAW_GPIO_BASE,
+ SEESAW_GPIO_DIRCLR_BULK,
+ SEESAW_BUTTON_MASK);
+ if (ret)
+ return ret;
+ ret = seesaw_register_write_u32(client, SEESAW_GPIO_BASE,
+ SEESAW_GPIO_PULLENSET,
+ SEESAW_BUTTON_MASK);
+ if (ret)
+ return ret;
+ ret = seesaw_register_write_u32(client, SEESAW_GPIO_BASE,
+ SEESAW_GPIO_BULK_SET,
+ SEESAW_BUTTON_MASK);
+ if (ret)
+ return ret;
+
+ seesaw->i2c_client = client;
+ seesaw->input_dev = devm_input_allocate_device(&client->dev);
+ if (!seesaw->input_dev)
+ return -ENOMEM;
+
+ seesaw->input_dev->id.bustype = BUS_I2C;
+ seesaw->input_dev->name = "Adafruit Seesaw Gamepad";
+ seesaw->input_dev->phys = "i2c/" SEESAW_DEVICE_NAME;
+ input_set_drvdata(seesaw->input_dev, seesaw);
+ input_set_abs_params(seesaw->input_dev, ABS_X, 0,
+ SEESAW_JOYSTICK_MAX_AXIS, SEESAW_JOYSTICK_FUZZ,
+ SEESAW_JOYSTICK_FLAT);
+ input_set_abs_params(seesaw->input_dev, ABS_Y, 0,
+ SEESAW_JOYSTICK_MAX_AXIS, SEESAW_JOYSTICK_FUZZ,
+ SEESAW_JOYSTICK_FLAT);
+
+ ret = sparse_keymap_setup(seesaw->input_dev, seesaw_buttons_new, NULL);
+ if (ret) {
+ dev_err(&client->dev,
+ "failed to set up input device keymap: %d\n", ret);
+ return ret;
+ }
+
+ ret = input_setup_polling(seesaw->input_dev, seesaw_poll);
+ if (ret) {
+ dev_err(&client->dev, "failed to set up polling: %d\n", ret);
+ return ret;
+ }
+
+ input_set_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_INTERVAL);
+ input_set_max_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_MAX);
+ input_set_min_poll_interval(seesaw->input_dev, SEESAW_GAMEPAD_POLL_MIN);
+
+ ret = input_register_device(seesaw->input_dev);
+ if (ret) {
+ dev_err(&client->dev, "failed to register joystick: %d\n", ret);
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct i2c_device_id seesaw_id_table[] = {
+ { SEESAW_DEVICE_NAME, 0 },
+ { /* Sentinel */ }
+};
+MODULE_DEVICE_TABLE(i2c, seesaw_id_table);
+
+static struct i2c_driver seesaw_driver = {
+ .driver = {
+ .name = SEESAW_DEVICE_NAME,
+ },
+ .id_table = seesaw_id_table,
+ .probe = seesaw_probe,
+};
+module_i2c_driver(seesaw_driver);
+
+MODULE_AUTHOR("Anshul Dalal <anshulusr@gmail.com>");
+MODULE_DESCRIPTION("Adafruit Mini I2C Gamepad driver");
+MODULE_LICENSE("GPL");
--
2.42.0
^ permalink raw reply related
* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
From: Eric GOUYER @ 2023-11-08 5:23 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Illia Ostapyshyn, David Revoy, jkosina, jason.gerecke,
jose.exposito89, linux-input, linux-kernel
In-Reply-To: <CAO-hwJKVwZK00yZFjuyyR9Xt4Y2-r8eLJNZfnyeopHxoZQ0eGA@mail.gmail.com>
Sorry, below I re-send my email which bounced due to exceeding 100k
(hid-recorder traces included), so I re-send it without them, for some
book-keeping.
I had originally replied-to-all, so mainteners involved should have
received those traces just before (I hope).
Best Regards,
-----8<-----8<-----8<-----8<
Hello, I have the same tablet than OP (David) :
- XP-Pen Artist Pro 16 Gen 2
- on Ubuntu 23.10 linux-image-generic 6.5.0.10.12
I am not (yet ?) encountering the problem described above since I guess
that my kernel is before the suspected regression.
Here below I included much detail, but you can TL;DR + jump at the five
hid-recorder traces below.
> On Mon, Nov 6, 2023 at 9:06 PM Illia Ostapyshyn
> <ostapyshyn@sra.uni-hannover.de> wrote:
> >
> > On 11/6/23 17:59, Benjamin Tissoires wrote:
> >
> > > If the pen has 2 buttons, and an eraser side, it would be a
> > > serious design flow for XPPEN to report both as eraser.
> > >
> > > Could you please use sudo hid-recorder from hid-tools[1] on any
> > > kernel version and send us the logs here?
> > > I'll be able to replay the events locally, and understand why the
> > > kernel doesn't work properly.
> > >
> > > And if there is a design flaw that can be fixed, we might even be
> > > able to use hid-bpf to change it :)
> >
> > My wild guess is that XP-Pen 16 Artist Pro reports :
> > - (1) an Eraser usage without Invert for the upper button
> > - (2) and Eraser with Invert for the eraser tip.
I think you will agree with below traces that :
- (1) : correct (it reports invert=0 eraser=1 tipSwitch=1)
- (2) : no, for the rubber tip, it reports invert=1 eraser=0 tipSwitch=1
> > A device-specific driver could work with that, but
> > there seems to be no way to incorporate two different erasers
> > (thus, allowing userspace to map them to different actions
> > arbitrarily) in the generic driver currently.
>
> That's exactly why I want to see the exact event flow. We can not do
> "wild guesses" unfortunately (not meaning any offenses).
> And I am very suspicious about the fact that the stylus reports 2
> identical erasers. Because in the past David seemed to be able to have
> 2 distincts behaviors for the 2 "buttons" (physical button and eraser
> tail).
The Pen, hardware-wise, has the following possibilities :
- The (main) pressure tip
- The "bottom" button (nearest of the main pressure tip)
- The "top" button (farthest of the main tip
- The "back" pressure tip (it has pressure!) somewhat called rubber
All of those works I think -natively- on my kernel version, without
external 3rd-party kernel modules.
It works especially on Blender, where I can :
- click (or paint) with main pressure-tip
- middle-click (to pan viewport) with "bottom" button + move main tip
- right-click with "top" button
- erase with backside pressure-tip
I installed a .deb from the official website [1] which does not contain
kernel modules, and seems to only contains :
- a udev rule to, I think, chmod 0666 to input character devices
- a Qt app to configure some behavior, only userland-side.
I think I observed that before running the Qt app, there was only 3
xinput devices, and after launching it, there were 4 more (7 total).
Blender would not receive pen input without lanching the Qt app.
Indeed, the "first" 3 /dev/input/event* associated to the pen are
chmod'ed too restrictively (0660) for an underprivileged user.
Anyway, after running the Qt App, 4 *ANOTHER* /dev/input/event* pops,
correctly chmod'ed (thanks to the udev rules), and Blender works.
My best guess would be that the first 3 very-native /dev/input/
devices are somewhat "raw", and that the Qt app feeds those inputs to
the Qt app's configured pressure curve + remap the button to the input
as chosen by the user.
Here is the dmesg before launching the Qt App :
-----8<-----8<-----8<-----8<
usb 3-9: new full-speed USB device number 64 using xhci_hcd
usb 3-9: New USB device found, idVendor=28bd, idProduct=095b,
bcdDevice= 0.00
usb 3-9: New USB device strings: Mfr=1, Product=2, SerialNumber=3
usb 3-9: Product: Artist Pro 16 (Gen2)
usb 3-9: Manufacturer: UGTABLET
usb 3-9: SerialNumber: 00000
input: UGTABLET Artist Pro 16 (Gen2) Mouse as
/devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.0/0003:28BD:095B.009E/input/input196
input: UGTABLET Artist Pro 16 (Gen2) Keyboard as
/devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.0/0003:28BD:095B.009E/input/input197
hid-generic 0003:28BD:095B.009E: input,hidraw4: USB HID v1.00 Mouse
[UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input0
input: UGTABLET Artist Pro 16 (Gen2) as
/devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.1/0003:28BD:095B.009F/input/input198
hid-generic 0003:28BD:095B.009F: input,hidraw7: USB HID v1.00 Device
[UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input1
hid-generic 0003:28BD:095B.00A0: hiddev0,hidraw12: USB HID v1.00 Device
[UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input2
-----8<-----8<-----8<-----8<
This make "sudo xinput" reports 3 new devices :
UGTABLET Artist Pro 16 (Gen2) Mouse id=x [slave pointer (n)]
UGTABLET Artist Pro 16 (Gen2) Keyboard id=y [slave keyboard (n+1)]
UGTABLET Artist Pro 16 (Gen2) id=z [slave keyboard (n+1)]
(sorry I've masked the id to prevent confusion, I did not run "xinput"
at the same time/power cycle than "dmesg")
Running the Qt App makes "sudo xinput" report 4 new devices :
XP-Pen Mouse id=xc [slave pointer (n)]
XP-Pen Eraser id=xa [slave keyboard (n+1)]
XP-Pen Pen id=xb [slave keyboard (n+1)] << only this chmod'ed 0666
XP-Pen Mouse id=xd [slave keyboard (n+1)]
And... it suffices to make Blender works without configuring anything.
Anyway, besides this user experience / userland Qt app, here are some
five hid-recorder traces ;
I re-used the terminology appearing in the traces :
- "barrel" : the "bottom" button nearest of the main tip
- "erase" : the farther "top" button, just above "barrel"
- "back" : the "rubber" secondary pressure tip at the back of the pen
(1) hidraw7_inRange_contact_move_lift_outOfRange
I "contact" the main tip, move it, and lift it out or range.
(2) hidraw7_inRange_contact_barrelPress\
_move_barrelRelease_lift_outOfRange
I "contact" the main tip, press "barrel" (which is the button nearest
of the tip), move the pen, release the button, move it out of range.
(3) hidraw7_inRange_contact_erasePress\
_move_eraseRelease_lift_outOfRange
The same than above, except that I instead use the "erase" button,
i.e. the button just above ("vertically speaking") of the "main" button.
(4) hidraw7_inRange_contact_BOTHbarrelAnderasePress_move_\
BOTHbarrelAnderaseRelease_lift_outOfRange
That's just plain stupid, and uncomfortable to do, but it's just to
show an example of the capability of the hardware ;
In this one, I'm doing the same than above, except I stupidly press BOTH
buttons.
It means that I "contact" the main tip, press both barrel+erase, move
the pen, release both button, lift it out of range.
(5) hidraw7_inRange_contact_backPress_move_backRelease_lift_outOfRange
Back to non-stupid tests, here I am using the "rubber" (which has
pressure !), i.e. I use the back of the pen.
I contact it, press the rubber to have some pressure, move it, release
the pressure, move the pen ouf of contact and out of range.
Reading those traces, I think I observed some (to me) very logical
behavior ;
I agree that the Pen is possibly not acting in respect of the
specification, by the fact that... the spec does not differentiate the
"eraser" 2nd button, versus the "backside" rubber pressure tip.
The traces shows that :
- In range [0 - 1] is correctly reported, both for main tip + rubber tip
- Tip Pressure [0 - n] is correctly reported, for both pressure tips
- Tip Switch [0 - 1] is correctly set to ONE for all traces, when the
in-range tip goes to contact. (either main tip or rubber tip)
- Barrel Switch [0 - 1] is correctly set to ONE only when pressing
button down, and of course only when using the main tip
- Invert [0 - 1] is set to ONE when (and only) using the back rubber
tip, and the ERASE button (2nd button) does NOT set it, so "INVERT"
- Eraser [0 - 1] is set to ONE ONLY when pressing the 2nd top button,
and of :
- ONLY when using the main itp
- and if you were to instead use the "back" of the pen (rubber tip),
then you will have invert=1 + eraser=0
The main extract of those is, I think, that :
- eraser "top" button is completely independent of the back "rubber"
- eraser "top" button NEVER concerns itself to set invert=1
- backside "rubber" pressure tip will set invert=1, but leave eraser=0
So, the behavior probably breaks the specs, but sincerely I'm happy to
have the "eraser" button independent of the "rubber eraser", which
makes the stylus a somewhat 4-buttons stylus (tip, button1, button2,
rubber), and I would like to keep this.
Best Regards,
[1] https://www.xp-pen.fr/download-1027.html ; the .deb Qt app
--
Eric GOUYER
^ permalink raw reply
* [PATCH RESEND v7 0/3] Add support for vibrator in multiple PMICs
From: Fenglin Wu via B4 Relay @ 2023-11-08 7:36 UTC (permalink / raw)
To: linux-arm-msm, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski
Cc: linux-input, linux-kernel, devicetree, quic_collinsd,
quic_subbaram, quic_kamalw, jestar, Luca Weiss, Fenglin Wu,
Krzysztof Kozlowski
Add SW support for the vibrator module inside PMI632, PM7250B, PM7325B, PM7550BA.
It is very similar to the vibrator module inside PM8916 which is supported in
pm8xxx-vib driver but just the drive amplitude is controlled with 2 registers,
and the register base offset in each PMIC is different.
Changes in v7;
1. Fix a typo: SSBL_VIB_DRV_REG --> SSBI_VIB_DRV_REG
2. Move the hw_type switch case in pm8xxx_vib_set() to the refactoring
change.
Changes in v6:
1. Add "qcom,pmi632-vib" as a standalone compatible string.
Changes in v5:
1. Drop "qcom,spmi-vib-gen2" generic compatible string as requested
and use device specific compatible strings only.
Changes in v4:
1. Update to use the combination of the HW type and register offset
as the constant match data, the register base address defined in
'reg' property will be added when accessing SPMI registers using
regmap APIs.
2. Remove 'qcom,spmi-vib-gen1' generic compatible string.
Changes in v3:
1. Refactor the driver to support different type of the vibrators with
better flexibility by introducing the HW type with corresponding
register fields definitions.
2. Add 'qcom,spmi-vib-gen1' and 'qcom,spmi-vib-gen2' compatible
strings, and add PMI632, PM7250B, PM7325B, PM7550BA as compatbile as
spmi-vib-gen2.
Changes in v2:
Remove the "pm7550ba-vib" compatible string as it's compatible with pm7325b.
Fenglin Wu (3):
input: pm8xxx-vib: refactor to easily support new SPMI vibrator
dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
input: pm8xxx-vibrator: add new SPMI vibrator support
.../bindings/input/qcom,pm8xxx-vib.yaml | 16 +-
drivers/input/misc/pm8xxx-vibrator.c | 171 ++++++++++++------
2 files changed, 132 insertions(+), 55 deletions(-)
--
2.25.1
---
Fenglin Wu (3):
input: pm8xxx-vib: refactor to easily support new SPMI vibrator
dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
input: pm8xxx-vibrator: add new SPMI vibrator support
.../devicetree/bindings/input/qcom,pm8xxx-vib.yaml | 16 +-
drivers/input/misc/pm8xxx-vibrator.c | 170 ++++++++++++++-------
2 files changed, 131 insertions(+), 55 deletions(-)
---
base-commit: 650cda2ce25f08e8fae391b3ba6be27e7296c6a5
change-id: 20230925-pm8xxx-vibrator-62df3df46a6c
Best regards,
--
Fenglin Wu <quic_fenglinw@quicinc.com>
^ permalink raw reply
* [PATCH RESEND v7 3/3] input: pm8xxx-vibrator: add new SPMI vibrator support
From: Fenglin Wu via B4 Relay @ 2023-11-08 7:36 UTC (permalink / raw)
To: linux-arm-msm, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski
Cc: linux-input, linux-kernel, devicetree, quic_collinsd,
quic_subbaram, quic_kamalw, jestar, Luca Weiss, Fenglin Wu
In-Reply-To: <20231108-pm8xxx-vibrator-v7-0-632c731d25a8@quicinc.com>
From: Fenglin Wu <quic_fenglinw@quicinc.com>
Add new SPMI vibrator module which is very similar to the SPMI vibrator
module inside PM8916 but just has a finer drive voltage step (1mV vs
100mV) hence its drive level control is expanded to across 2 registers.
The vibrator module can be found in Qualcomm PMIC PMI632, then following
PM7250B, PM7325B, PM7550BA PMICs.
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
Tested-by: Luca Weiss <luca.weiss@fairphone.com> # sdm632-fairphone-fp3 (pmi632)
---
drivers/input/misc/pm8xxx-vibrator.c | 42 +++++++++++++++++++++++++++++++++---
1 file changed, 39 insertions(+), 3 deletions(-)
diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index ba9be374f892..82c788841f1f 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -21,6 +21,13 @@
#define SPMI_VIB_DRV_LEVEL_MASK GENMASK(4, 0)
#define SPMI_VIB_DRV_SHIFT 0
+#define SPMI_VIB_GEN2_DRV_REG 0x40
+#define SPMI_VIB_GEN2_DRV_MASK GENMASK(7, 0)
+#define SPMI_VIB_GEN2_DRV_SHIFT 0
+#define SPMI_VIB_GEN2_DRV2_REG 0x41
+#define SPMI_VIB_GEN2_DRV2_MASK GENMASK(3, 0)
+#define SPMI_VIB_GEN2_DRV2_SHIFT 8
+
#define SPMI_VIB_EN_REG 0x46
#define SPMI_VIB_EN_BIT BIT(7)
@@ -33,12 +40,14 @@
enum vib_hw_type {
SSBI_VIB,
SPMI_VIB,
+ SPMI_VIB_GEN2
};
struct pm8xxx_vib_data {
enum vib_hw_type hw_type;
unsigned int enable_addr;
unsigned int drv_addr;
+ unsigned int drv2_addr;
};
static const struct pm8xxx_vib_data ssbi_vib_data = {
@@ -52,6 +61,13 @@ static const struct pm8xxx_vib_data spmi_vib_data = {
.drv_addr = SPMI_VIB_DRV_REG,
};
+static const struct pm8xxx_vib_data spmi_vib_gen2_data = {
+ .hw_type = SPMI_VIB_GEN2,
+ .enable_addr = SPMI_VIB_EN_REG,
+ .drv_addr = SPMI_VIB_GEN2_DRV_REG,
+ .drv2_addr = SPMI_VIB_GEN2_DRV2_REG,
+};
+
/**
* struct pm8xxx_vib - structure to hold vibrator data
* @vib_input_dev: input device supporting force feedback
@@ -96,9 +112,12 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
mask = SPMI_VIB_DRV_LEVEL_MASK;
shift = SPMI_VIB_DRV_SHIFT;
break;
+ case SPMI_VIB_GEN2:
+ mask = SPMI_VIB_GEN2_DRV_MASK;
+ shift = SPMI_VIB_GEN2_DRV_SHIFT;
+ break;
default:
return -EINVAL;
-
}
if (on)
@@ -112,6 +131,19 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
vib->reg_vib_drv = val;
+ if (vib->data->hw_type == SPMI_VIB_GEN2) {
+ mask = SPMI_VIB_GEN2_DRV2_MASK;
+ shift = SPMI_VIB_GEN2_DRV2_SHIFT;
+ if (on)
+ val = (vib->level >> shift) & mask;
+ else
+ val = 0;
+ rc = regmap_update_bits(vib->regmap,
+ vib->reg_base + vib->data->drv2_addr, mask, val);
+ if (rc < 0)
+ return rc;
+ }
+
if (vib->data->hw_type == SSBI_VIB)
return 0;
@@ -136,10 +168,13 @@ static void pm8xxx_work_handler(struct work_struct *work)
vib->active = true;
vib->level = ((VIB_MAX_LEVELS * vib->speed) / MAX_FF_SPEED) +
VIB_MIN_LEVEL_mV;
- vib->level /= 100;
+ if (vib->data->hw_type != SPMI_VIB_GEN2)
+ vib->level /= 100;
} else {
vib->active = false;
- vib->level = VIB_MIN_LEVEL_mV / 100;
+ vib->level = VIB_MIN_LEVEL_mV;
+ if (vib->data->hw_type != SPMI_VIB_GEN2)
+ vib->level /= 100;
}
pm8xxx_vib_set(vib, vib->active);
@@ -274,6 +309,7 @@ static const struct of_device_id pm8xxx_vib_id_table[] = {
{ .compatible = "qcom,pm8058-vib", .data = &ssbi_vib_data },
{ .compatible = "qcom,pm8921-vib", .data = &ssbi_vib_data },
{ .compatible = "qcom,pm8916-vib", .data = &spmi_vib_data },
+ { .compatible = "qcom,pmi632-vib", .data = &spmi_vib_gen2_data },
{ }
};
MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
--
2.25.1
^ permalink raw reply related
* [PATCH RESEND v7 2/3] dt-bindings: input: qcom,pm8xxx-vib: add new SPMI vibrator module
From: Fenglin Wu via B4 Relay @ 2023-11-08 7:36 UTC (permalink / raw)
To: linux-arm-msm, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski
Cc: linux-input, linux-kernel, devicetree, quic_collinsd,
quic_subbaram, quic_kamalw, jestar, Luca Weiss,
Krzysztof Kozlowski, Fenglin Wu
In-Reply-To: <20231108-pm8xxx-vibrator-v7-0-632c731d25a8@quicinc.com>
From: Fenglin Wu <quic_fenglinw@quicinc.com>
Add compatible strings to support vibrator module inside PMI632,
PMI7250B, PM7325B, PM7550BA.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
.../devicetree/bindings/input/qcom,pm8xxx-vib.yaml | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
index c8832cd0d7da..2025d6a5423e 100644
--- a/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
+++ b/Documentation/devicetree/bindings/input/qcom,pm8xxx-vib.yaml
@@ -11,10 +11,18 @@ maintainers:
properties:
compatible:
- enum:
- - qcom,pm8058-vib
- - qcom,pm8916-vib
- - qcom,pm8921-vib
+ oneOf:
+ - enum:
+ - qcom,pm8058-vib
+ - qcom,pm8916-vib
+ - qcom,pm8921-vib
+ - qcom,pmi632-vib
+ - items:
+ - enum:
+ - qcom,pm7250b-vib
+ - qcom,pm7325b-vib
+ - qcom,pm7550ba-vib
+ - const: qcom,pmi632-vib
reg:
maxItems: 1
--
2.25.1
^ permalink raw reply related
* [PATCH RESEND v7 1/3] input: pm8xxx-vib: refactor to easily support new SPMI vibrator
From: Fenglin Wu via B4 Relay @ 2023-11-08 7:36 UTC (permalink / raw)
To: linux-arm-msm, Andy Gross, Bjorn Andersson, Konrad Dybcio,
Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski
Cc: linux-input, linux-kernel, devicetree, quic_collinsd,
quic_subbaram, quic_kamalw, jestar, Luca Weiss, Fenglin Wu
In-Reply-To: <20231108-pm8xxx-vibrator-v7-0-632c731d25a8@quicinc.com>
From: Fenglin Wu <quic_fenglinw@quicinc.com>
Currently, all vibrator control register addresses are hard coded,
including the base address and the offset, it's not flexible to support
new SPMI vibrator module which is usually included in different PMICs
with different base address. Refactor this by defining register offset
with HW type combination, and register base address which is defined
in 'reg' property is added for SPMI vibrators.
Signed-off-by: Fenglin Wu <quic_fenglinw@quicinc.com>
---
drivers/input/misc/pm8xxx-vibrator.c | 130 ++++++++++++++++++++++-------------
1 file changed, 81 insertions(+), 49 deletions(-)
diff --git a/drivers/input/misc/pm8xxx-vibrator.c b/drivers/input/misc/pm8xxx-vibrator.c
index 04cb87efd799..ba9be374f892 100644
--- a/drivers/input/misc/pm8xxx-vibrator.c
+++ b/drivers/input/misc/pm8xxx-vibrator.c
@@ -12,36 +12,44 @@
#include <linux/regmap.h>
#include <linux/slab.h>
+#define SSBI_VIB_DRV_REG 0x4A
+#define SSBI_VIB_DRV_EN_MANUAL_MASK GENMASK(7, 2)
+#define SSBI_VIB_DRV_LEVEL_MASK GENMASK(7, 3)
+#define SSBI_VIB_DRV_SHIFT 3
+
+#define SPMI_VIB_DRV_REG 0x41
+#define SPMI_VIB_DRV_LEVEL_MASK GENMASK(4, 0)
+#define SPMI_VIB_DRV_SHIFT 0
+
+#define SPMI_VIB_EN_REG 0x46
+#define SPMI_VIB_EN_BIT BIT(7)
+
#define VIB_MAX_LEVEL_mV (3100)
#define VIB_MIN_LEVEL_mV (1200)
#define VIB_MAX_LEVELS (VIB_MAX_LEVEL_mV - VIB_MIN_LEVEL_mV)
#define MAX_FF_SPEED 0xff
-struct pm8xxx_regs {
- unsigned int enable_addr;
- unsigned int enable_mask;
+enum vib_hw_type {
+ SSBI_VIB,
+ SPMI_VIB,
+};
- unsigned int drv_addr;
- unsigned int drv_mask;
- unsigned int drv_shift;
- unsigned int drv_en_manual_mask;
+struct pm8xxx_vib_data {
+ enum vib_hw_type hw_type;
+ unsigned int enable_addr;
+ unsigned int drv_addr;
};
-static const struct pm8xxx_regs pm8058_regs = {
- .drv_addr = 0x4A,
- .drv_mask = 0xf8,
- .drv_shift = 3,
- .drv_en_manual_mask = 0xfc,
+static const struct pm8xxx_vib_data ssbi_vib_data = {
+ .hw_type = SSBI_VIB,
+ .drv_addr = SSBI_VIB_DRV_REG,
};
-static struct pm8xxx_regs pm8916_regs = {
- .enable_addr = 0xc046,
- .enable_mask = BIT(7),
- .drv_addr = 0xc041,
- .drv_mask = 0x1F,
- .drv_shift = 0,
- .drv_en_manual_mask = 0,
+static const struct pm8xxx_vib_data spmi_vib_data = {
+ .hw_type = SPMI_VIB,
+ .enable_addr = SPMI_VIB_EN_REG,
+ .drv_addr = SPMI_VIB_DRV_REG,
};
/**
@@ -49,7 +57,8 @@ static struct pm8xxx_regs pm8916_regs = {
* @vib_input_dev: input device supporting force feedback
* @work: work structure to set the vibration parameters
* @regmap: regmap for register read/write
- * @regs: registers' info
+ * @data: vibrator HW info
+ * @reg_base: the register base of the module
* @speed: speed of vibration set from userland
* @active: state of vibrator
* @level: level of vibration to set in the chip
@@ -59,7 +68,8 @@ struct pm8xxx_vib {
struct input_dev *vib_input_dev;
struct work_struct work;
struct regmap *regmap;
- const struct pm8xxx_regs *regs;
+ const struct pm8xxx_vib_data *data;
+ unsigned int reg_base;
int speed;
int level;
bool active;
@@ -75,24 +85,39 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
{
int rc;
unsigned int val = vib->reg_vib_drv;
- const struct pm8xxx_regs *regs = vib->regs;
+ u32 mask, shift;
+
+ switch (vib->data->hw_type) {
+ case SSBI_VIB:
+ mask = SSBI_VIB_DRV_LEVEL_MASK;
+ shift = SSBI_VIB_DRV_SHIFT;
+ break;
+ case SPMI_VIB:
+ mask = SPMI_VIB_DRV_LEVEL_MASK;
+ shift = SPMI_VIB_DRV_SHIFT;
+ break;
+ default:
+ return -EINVAL;
+
+ }
if (on)
- val |= (vib->level << regs->drv_shift) & regs->drv_mask;
+ val |= (vib->level << shift) & mask;
else
- val &= ~regs->drv_mask;
+ val &= ~mask;
- rc = regmap_write(vib->regmap, regs->drv_addr, val);
+ rc = regmap_update_bits(vib->regmap, vib->reg_base + vib->data->drv_addr, mask, val);
if (rc < 0)
return rc;
vib->reg_vib_drv = val;
- if (regs->enable_mask)
- rc = regmap_update_bits(vib->regmap, regs->enable_addr,
- regs->enable_mask, on ? ~0 : 0);
+ if (vib->data->hw_type == SSBI_VIB)
+ return 0;
- return rc;
+ mask = SPMI_VIB_EN_BIT;
+ val = on ? SPMI_VIB_EN_BIT : 0;
+ return regmap_update_bits(vib->regmap, vib->reg_base + vib->data->enable_addr, mask, val);
}
/**
@@ -102,13 +127,6 @@ static int pm8xxx_vib_set(struct pm8xxx_vib *vib, bool on)
static void pm8xxx_work_handler(struct work_struct *work)
{
struct pm8xxx_vib *vib = container_of(work, struct pm8xxx_vib, work);
- const struct pm8xxx_regs *regs = vib->regs;
- int rc;
- unsigned int val;
-
- rc = regmap_read(vib->regmap, regs->drv_addr, &val);
- if (rc < 0)
- return;
/*
* pmic vibrator supports voltage ranges from 1.2 to 3.1V, so
@@ -168,9 +186,9 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
{
struct pm8xxx_vib *vib;
struct input_dev *input_dev;
+ const struct pm8xxx_vib_data *data;
int error;
- unsigned int val;
- const struct pm8xxx_regs *regs;
+ unsigned int val, reg_base;
vib = devm_kzalloc(&pdev->dev, sizeof(*vib), GFP_KERNEL);
if (!vib)
@@ -187,19 +205,33 @@ static int pm8xxx_vib_probe(struct platform_device *pdev)
INIT_WORK(&vib->work, pm8xxx_work_handler);
vib->vib_input_dev = input_dev;
- regs = of_device_get_match_data(&pdev->dev);
+ data = of_device_get_match_data(&pdev->dev);
+ if (!data)
+ return -EINVAL;
- /* operate in manual mode */
- error = regmap_read(vib->regmap, regs->drv_addr, &val);
- if (error < 0)
- return error;
+ if (data->hw_type != SSBI_VIB) {
+ error = fwnode_property_read_u32(pdev->dev.fwnode, "reg", ®_base);
+ if (error < 0) {
+ dev_err(&pdev->dev, "Failed to read reg address, rc=%d\n", error);
+ return error;
+ }
+
+ vib->reg_base += reg_base;
+ }
- val &= regs->drv_en_manual_mask;
- error = regmap_write(vib->regmap, regs->drv_addr, val);
+ error = regmap_read(vib->regmap, vib->reg_base + data->drv_addr, &val);
if (error < 0)
return error;
- vib->regs = regs;
+ /* operate in manual mode */
+ if (data->hw_type == SSBI_VIB) {
+ val &= SSBI_VIB_DRV_EN_MANUAL_MASK;
+ error = regmap_write(vib->regmap, vib->reg_base + data->drv_addr, val);
+ if (error < 0)
+ return error;
+ }
+
+ vib->data = data;
vib->reg_vib_drv = val;
input_dev->name = "pm8xxx_vib_ffmemless";
@@ -239,9 +271,9 @@ static int pm8xxx_vib_suspend(struct device *dev)
static DEFINE_SIMPLE_DEV_PM_OPS(pm8xxx_vib_pm_ops, pm8xxx_vib_suspend, NULL);
static const struct of_device_id pm8xxx_vib_id_table[] = {
- { .compatible = "qcom,pm8058-vib", .data = &pm8058_regs },
- { .compatible = "qcom,pm8921-vib", .data = &pm8058_regs },
- { .compatible = "qcom,pm8916-vib", .data = &pm8916_regs },
+ { .compatible = "qcom,pm8058-vib", .data = &ssbi_vib_data },
+ { .compatible = "qcom,pm8921-vib", .data = &ssbi_vib_data },
+ { .compatible = "qcom,pm8916-vib", .data = &spmi_vib_data },
{ }
};
MODULE_DEVICE_TABLE(of, pm8xxx_vib_id_table);
--
2.25.1
^ permalink raw reply related
* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
From: Benjamin Tissoires @ 2023-11-08 9:04 UTC (permalink / raw)
To: Eric GOUYER
Cc: Illia Ostapyshyn, David Revoy, jkosina, jason.gerecke,
jose.exposito89, linux-input, linux-kernel
In-Reply-To: <20231108062306.33f5dcd0@dryade>
Thanks Eric and Illia for the logs and the explanations.
On Wed, Nov 8, 2023 at 6:23 AM Eric GOUYER <folays@gmail.com> wrote:
>
> Sorry, below I re-send my email which bounced due to exceeding 100k
> (hid-recorder traces included), so I re-send it without them, for some
> book-keeping.
>
> I had originally replied-to-all, so mainteners involved should have
> received those traces just before (I hope).
>
> Best Regards,
>
> -----8<-----8<-----8<-----8<
>
> Hello, I have the same tablet than OP (David) :
> - XP-Pen Artist Pro 16 Gen 2
> - on Ubuntu 23.10 linux-image-generic 6.5.0.10.12
>
> I am not (yet ?) encountering the problem described above since I guess
> that my kernel is before the suspected regression.
>
> Here below I included much detail, but you can TL;DR + jump at the five
> hid-recorder traces below.
>
> > On Mon, Nov 6, 2023 at 9:06 PM Illia Ostapyshyn
> > <ostapyshyn@sra.uni-hannover.de> wrote:
> > >
> > > On 11/6/23 17:59, Benjamin Tissoires wrote:
> > >
> > > > If the pen has 2 buttons, and an eraser side, it would be a
> > > > serious design flow for XPPEN to report both as eraser.
> > > >
> > > > Could you please use sudo hid-recorder from hid-tools[1] on any
> > > > kernel version and send us the logs here?
> > > > I'll be able to replay the events locally, and understand why the
> > > > kernel doesn't work properly.
> > > >
> > > > And if there is a design flaw that can be fixed, we might even be
> > > > able to use hid-bpf to change it :)
> > >
> > > My wild guess is that XP-Pen 16 Artist Pro reports :
> > > - (1) an Eraser usage without Invert for the upper button
> > > - (2) and Eraser with Invert for the eraser tip.
>
> I think you will agree with below traces that :
> - (1) : correct (it reports invert=0 eraser=1 tipSwitch=1)
> - (2) : no, for the rubber tip, it reports invert=1 eraser=0 tipSwitch=1
>
> > > A device-specific driver could work with that, but
> > > there seems to be no way to incorporate two different erasers
> > > (thus, allowing userspace to map them to different actions
> > > arbitrarily) in the generic driver currently.
> >
> > That's exactly why I want to see the exact event flow. We can not do
> > "wild guesses" unfortunately (not meaning any offenses).
> > And I am very suspicious about the fact that the stylus reports 2
> > identical erasers. Because in the past David seemed to be able to have
> > 2 distincts behaviors for the 2 "buttons" (physical button and eraser
> > tail).
>
> The Pen, hardware-wise, has the following possibilities :
> - The (main) pressure tip
> - The "bottom" button (nearest of the main pressure tip)
> - The "top" button (farthest of the main tip
> - The "back" pressure tip (it has pressure!) somewhat called rubber
>
> All of those works I think -natively- on my kernel version, without
> external 3rd-party kernel modules.
>
> It works especially on Blender, where I can :
> - click (or paint) with main pressure-tip
> - middle-click (to pan viewport) with "bottom" button + move main tip
> - right-click with "top" button
> - erase with backside pressure-tip
>
> I installed a .deb from the official website [1] which does not contain
> kernel modules, and seems to only contains :
> - a udev rule to, I think, chmod 0666 to input character devices
> - a Qt app to configure some behavior, only userland-side.
>
> I think I observed that before running the Qt app, there was only 3
> xinput devices, and after launching it, there were 4 more (7 total).
>
> Blender would not receive pen input without lanching the Qt app.
> Indeed, the "first" 3 /dev/input/event* associated to the pen are
> chmod'ed too restrictively (0660) for an underprivileged user.
This is wrong (not your fault, of course). There is no way Blender
should directly talk to the input nodes. It should use the events from
the compositor, not randomly getting values from input nodes. sigh...
>
> Anyway, after running the Qt App, 4 *ANOTHER* /dev/input/event* pops,
> correctly chmod'ed (thanks to the udev rules), and Blender works.
I got a quick look, and those events are coming from uinput. So their
Qt App is just a horrible user space driver to fix their tablet. Ouch.
>
> My best guess would be that the first 3 very-native /dev/input/
> devices are somewhat "raw", and that the Qt app feeds those inputs to
> the Qt app's configured pressure curve + remap the button to the input
> as chosen by the user.
The "raw" nodes are giving all of the information, but not correctly.
So they are using a separate userspace driver to "fix" it.
>
> Here is the dmesg before launching the Qt App :
> -----8<-----8<-----8<-----8<
> usb 3-9: new full-speed USB device number 64 using xhci_hcd
> usb 3-9: New USB device found, idVendor=28bd, idProduct=095b,
> bcdDevice= 0.00
> usb 3-9: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> usb 3-9: Product: Artist Pro 16 (Gen2)
> usb 3-9: Manufacturer: UGTABLET
> usb 3-9: SerialNumber: 00000
> input: UGTABLET Artist Pro 16 (Gen2) Mouse as
> /devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
> 0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.0/0003:28BD:095B.009E/input/input196
> input: UGTABLET Artist Pro 16 (Gen2) Keyboard as
> /devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
> 0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.0/0003:28BD:095B.009E/input/input197
> hid-generic 0003:28BD:095B.009E: input,hidraw4: USB HID v1.00 Mouse
> [UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input0
> input: UGTABLET Artist Pro 16 (Gen2) as
> /devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
> 0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.1/0003:28BD:095B.009F/input/input198
> hid-generic 0003:28BD:095B.009F: input,hidraw7: USB HID v1.00 Device
> [UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input1
> hid-generic 0003:28BD:095B.00A0: hiddev0,hidraw12: USB HID v1.00 Device
> [UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input2
> -----8<-----8<-----8<-----8<
>
> This make "sudo xinput" reports 3 new devices :
> UGTABLET Artist Pro 16 (Gen2) Mouse id=x [slave pointer (n)]
> UGTABLET Artist Pro 16 (Gen2) Keyboard id=y [slave keyboard (n+1)]
> UGTABLET Artist Pro 16 (Gen2) id=z [slave keyboard (n+1)]
> (sorry I've masked the id to prevent confusion, I did not run "xinput"
> at the same time/power cycle than "dmesg")
>
> Running the Qt App makes "sudo xinput" report 4 new devices :
> XP-Pen Mouse id=xc [slave pointer (n)]
> XP-Pen Eraser id=xa [slave keyboard (n+1)]
> XP-Pen Pen id=xb [slave keyboard (n+1)] << only this chmod'ed 0666
> XP-Pen Mouse id=xd [slave keyboard (n+1)]
>
> And... it suffices to make Blender works without configuring anything.
>
> Anyway, besides this user experience / userland Qt app, here are some
> five hid-recorder traces ;
Many thanks for attaching them and for digging into them.
> I re-used the terminology appearing in the traces :
> - "barrel" : the "bottom" button nearest of the main tip
> - "erase" : the farther "top" button, just above "barrel"
> - "back" : the "rubber" secondary pressure tip at the back of the pen
>
> (1) hidraw7_inRange_contact_move_lift_outOfRange
> I "contact" the main tip, move it, and lift it out or range.
>
> (2) hidraw7_inRange_contact_barrelPress\
> _move_barrelRelease_lift_outOfRange
> I "contact" the main tip, press "barrel" (which is the button nearest
> of the tip), move the pen, release the button, move it out of range.
>
> (3) hidraw7_inRange_contact_erasePress\
> _move_eraseRelease_lift_outOfRange
> The same than above, except that I instead use the "erase" button,
> i.e. the button just above ("vertically speaking") of the "main" button.
>
> (4) hidraw7_inRange_contact_BOTHbarrelAnderasePress_move_\
> BOTHbarrelAnderaseRelease_lift_outOfRange
> That's just plain stupid, and uncomfortable to do, but it's just to
> show an example of the capability of the hardware ;
> In this one, I'm doing the same than above, except I stupidly press BOTH
> buttons.
> It means that I "contact" the main tip, press both barrel+erase, move
> the pen, release both button, lift it out of range.
>
> (5) hidraw7_inRange_contact_backPress_move_backRelease_lift_outOfRange
> Back to non-stupid tests, here I am using the "rubber" (which has
> pressure !), i.e. I use the back of the pen.
> I contact it, press the rubber to have some pressure, move it, release
> the pressure, move the pen ouf of contact and out of range.
>
> Reading those traces, I think I observed some (to me) very logical
> behavior ;
>
> I agree that the Pen is possibly not acting in respect of the
> specification, by the fact that... the spec does not differentiate the
> "eraser" 2nd button, versus the "backside" rubber pressure tip.
>
> The traces shows that :
> - In range [0 - 1] is correctly reported, both for main tip + rubber tip
> - Tip Pressure [0 - n] is correctly reported, for both pressure tips
Yep, that part is correct and follows the specification
>
> - Tip Switch [0 - 1] is correctly set to ONE for all traces, when the
> in-range tip goes to contact. (either main tip or rubber tip)
This is half wrong (spec-wise):
- tip switch should only be used when the actual tip is in contact.
- when using the rubber tip, we should get an eraser event :(
>
> - Barrel Switch [0 - 1] is correctly set to ONE only when pressing
> button down, and of course only when using the main tip
This is correct (as in it follows the spec)
>
> - Invert [0 - 1] is set to ONE when (and only) using the back rubber
> tip, and the ERASE button (2nd button) does NOT set it, so "INVERT"
In your case, the invert usage is correct.
In Illia's case, the invert usage is forwarded by the eraser usage, so
this is just plain wrong there.
>
> - Eraser [0 - 1] is set to ONE ONLY when pressing the 2nd top button,
> and of :
> - ONLY when using the main itp
> - and if you were to instead use the "back" of the pen (rubber tip),
> then you will have invert=1 + eraser=0
This is not correct (again, according to the specification). XP-Pen
should report SecondaryBarrelSwitch (0x5a in the HID usage table[0])
And again, in Illia's case, this button should be reported as an
invert, and eraser when the tip touches the tablet.
>
> The main extract of those is, I think, that :
> - eraser "top" button is completely independent of the back "rubber"
Yes
> - eraser "top" button NEVER concerns itself to set invert=1
yes
> - backside "rubber" pressure tip will set invert=1, but leave eraser=0
yes, but that's not correct, it should report eraser=1 when you touch
the rubber on the surface.
>
> So, the behavior probably breaks the specs, but sincerely I'm happy to
> have the "eraser" button independent of the "rubber eraser", which
> makes the stylus a somewhat 4-buttons stylus (tip, button1, button2,
> rubber), and I would like to keep this.
Yes, and I'd like to keep it that way, even if 6.6 and 6.5.8
apparently broke it.
So, to me:
- 276e14e6c3993317257e1787e93b7166fbc30905 is wrong: this is a
firmware bug (reporting invert through eraser) and should not be
tackled at the generic level, thus it should be reverted
- both of these tablets are forwarding the useful information, but not
correctly, which confuses the kernel
- I should now be able to write regression tests
- I should be able to provide HID-BPF fixes for those tablets so that
we can keep them working with or without
276e14e6c3993317257e1787e93b7166fbc30905
reverted (hopefully)
- problem is I still don't have the mechanics to integrate the HID-BPF
fixes directly in the kernel tree, so maybe I'll have to write a
driver for XP-Pen while these internals are set (it shouldn't
interfere with the HID-BPF out of the tree).
Cheers,
Benjamin
>
> Best Regards,
>
> [1] https://www.xp-pen.fr/download-1027.html ; the .deb Qt app
>
> --
> Eric GOUYER
>
[0] https://usb.org/document-library/hid-usage-tables-14
^ permalink raw reply
* Re: Requesting your attention and expertise regarding a Tablet/Kernel issue
From: José Expósito @ 2023-11-08 9:23 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Eric GOUYER, Illia Ostapyshyn, David Revoy, jkosina,
jason.gerecke, linux-input, linux-kernel
In-Reply-To: <CAO-hwJK_xp1A=dEOV-2v3KJAf0bRLDWNcrFQeBpgEuxT-qSBnw@mail.gmail.com>
Hi Benjamin,
On Wed, Nov 08, 2023 at 10:04:30AM +0100, Benjamin Tissoires wrote:
> Thanks Eric and Illia for the logs and the explanations.
>
> On Wed, Nov 8, 2023 at 6:23 AM Eric GOUYER <folays@gmail.com> wrote:
> >
> > Sorry, below I re-send my email which bounced due to exceeding 100k
> > (hid-recorder traces included), so I re-send it without them, for some
> > book-keeping.
> >
> > I had originally replied-to-all, so mainteners involved should have
> > received those traces just before (I hope).
> >
> > Best Regards,
> >
> > -----8<-----8<-----8<-----8<
> >
> > Hello, I have the same tablet than OP (David) :
> > - XP-Pen Artist Pro 16 Gen 2
> > - on Ubuntu 23.10 linux-image-generic 6.5.0.10.12
> >
> > I am not (yet ?) encountering the problem described above since I guess
> > that my kernel is before the suspected regression.
> >
> > Here below I included much detail, but you can TL;DR + jump at the five
> > hid-recorder traces below.
> >
> > > On Mon, Nov 6, 2023 at 9:06 PM Illia Ostapyshyn
> > > <ostapyshyn@sra.uni-hannover.de> wrote:
> > > >
> > > > On 11/6/23 17:59, Benjamin Tissoires wrote:
> > > >
> > > > > If the pen has 2 buttons, and an eraser side, it would be a
> > > > > serious design flow for XPPEN to report both as eraser.
> > > > >
> > > > > Could you please use sudo hid-recorder from hid-tools[1] on any
> > > > > kernel version and send us the logs here?
> > > > > I'll be able to replay the events locally, and understand why the
> > > > > kernel doesn't work properly.
> > > > >
> > > > > And if there is a design flaw that can be fixed, we might even be
> > > > > able to use hid-bpf to change it :)
> > > >
> > > > My wild guess is that XP-Pen 16 Artist Pro reports :
> > > > - (1) an Eraser usage without Invert for the upper button
> > > > - (2) and Eraser with Invert for the eraser tip.
> >
> > I think you will agree with below traces that :
> > - (1) : correct (it reports invert=0 eraser=1 tipSwitch=1)
> > - (2) : no, for the rubber tip, it reports invert=1 eraser=0 tipSwitch=1
> >
> > > > A device-specific driver could work with that, but
> > > > there seems to be no way to incorporate two different erasers
> > > > (thus, allowing userspace to map them to different actions
> > > > arbitrarily) in the generic driver currently.
> > >
> > > That's exactly why I want to see the exact event flow. We can not do
> > > "wild guesses" unfortunately (not meaning any offenses).
> > > And I am very suspicious about the fact that the stylus reports 2
> > > identical erasers. Because in the past David seemed to be able to have
> > > 2 distincts behaviors for the 2 "buttons" (physical button and eraser
> > > tail).
> >
> > The Pen, hardware-wise, has the following possibilities :
> > - The (main) pressure tip
> > - The "bottom" button (nearest of the main pressure tip)
> > - The "top" button (farthest of the main tip
> > - The "back" pressure tip (it has pressure!) somewhat called rubber
> >
> > All of those works I think -natively- on my kernel version, without
> > external 3rd-party kernel modules.
> >
> > It works especially on Blender, where I can :
> > - click (or paint) with main pressure-tip
> > - middle-click (to pan viewport) with "bottom" button + move main tip
> > - right-click with "top" button
> > - erase with backside pressure-tip
> >
> > I installed a .deb from the official website [1] which does not contain
> > kernel modules, and seems to only contains :
> > - a udev rule to, I think, chmod 0666 to input character devices
> > - a Qt app to configure some behavior, only userland-side.
> >
> > I think I observed that before running the Qt app, there was only 3
> > xinput devices, and after launching it, there were 4 more (7 total).
> >
> > Blender would not receive pen input without lanching the Qt app.
> > Indeed, the "first" 3 /dev/input/event* associated to the pen are
> > chmod'ed too restrictively (0660) for an underprivileged user.
>
> This is wrong (not your fault, of course). There is no way Blender
> should directly talk to the input nodes. It should use the events from
> the compositor, not randomly getting values from input nodes. sigh...
>
> >
> > Anyway, after running the Qt App, 4 *ANOTHER* /dev/input/event* pops,
> > correctly chmod'ed (thanks to the udev rules), and Blender works.
>
> I got a quick look, and those events are coming from uinput. So their
> Qt App is just a horrible user space driver to fix their tablet. Ouch.
>
> >
> > My best guess would be that the first 3 very-native /dev/input/
> > devices are somewhat "raw", and that the Qt app feeds those inputs to
> > the Qt app's configured pressure curve + remap the button to the input
> > as chosen by the user.
>
> The "raw" nodes are giving all of the information, but not correctly.
> So they are using a separate userspace driver to "fix" it.
>
> >
> > Here is the dmesg before launching the Qt App :
> > -----8<-----8<-----8<-----8<
> > usb 3-9: new full-speed USB device number 64 using xhci_hcd
> > usb 3-9: New USB device found, idVendor=28bd, idProduct=095b,
> > bcdDevice= 0.00
> > usb 3-9: New USB device strings: Mfr=1, Product=2, SerialNumber=3
> > usb 3-9: Product: Artist Pro 16 (Gen2)
> > usb 3-9: Manufacturer: UGTABLET
> > usb 3-9: SerialNumber: 00000
> > input: UGTABLET Artist Pro 16 (Gen2) Mouse as
> > /devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
> > 0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.0/0003:28BD:095B.009E/input/input196
> > input: UGTABLET Artist Pro 16 (Gen2) Keyboard as
> > /devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
> > 0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.0/0003:28BD:095B.009E/input/input197
> > hid-generic 0003:28BD:095B.009E: input,hidraw4: USB HID v1.00 Mouse
> > [UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input0
> > input: UGTABLET Artist Pro 16 (Gen2) as
> > /devices/pci0000:00/0000:00:02.1/0000:04:00.0/0000:05:08.0/0000:07:00.0/
> > 0000:08:0c.0/0000:69:00.0/usb3/3-9/3-9:1.1/0003:28BD:095B.009F/input/input198
> > hid-generic 0003:28BD:095B.009F: input,hidraw7: USB HID v1.00 Device
> > [UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input1
> > hid-generic 0003:28BD:095B.00A0: hiddev0,hidraw12: USB HID v1.00 Device
> > [UGTABLET Artist Pro 16 (Gen2)] on usb-0000:69:00.0-9/input2
> > -----8<-----8<-----8<-----8<
> >
> > This make "sudo xinput" reports 3 new devices :
> > UGTABLET Artist Pro 16 (Gen2) Mouse id=x [slave pointer (n)]
> > UGTABLET Artist Pro 16 (Gen2) Keyboard id=y [slave keyboard (n+1)]
> > UGTABLET Artist Pro 16 (Gen2) id=z [slave keyboard (n+1)]
> > (sorry I've masked the id to prevent confusion, I did not run "xinput"
> > at the same time/power cycle than "dmesg")
> >
> > Running the Qt App makes "sudo xinput" report 4 new devices :
> > XP-Pen Mouse id=xc [slave pointer (n)]
> > XP-Pen Eraser id=xa [slave keyboard (n+1)]
> > XP-Pen Pen id=xb [slave keyboard (n+1)] << only this chmod'ed 0666
> > XP-Pen Mouse id=xd [slave keyboard (n+1)]
> >
> > And... it suffices to make Blender works without configuring anything.
> >
> > Anyway, besides this user experience / userland Qt app, here are some
> > five hid-recorder traces ;
>
> Many thanks for attaching them and for digging into them.
>
> > I re-used the terminology appearing in the traces :
> > - "barrel" : the "bottom" button nearest of the main tip
> > - "erase" : the farther "top" button, just above "barrel"
> > - "back" : the "rubber" secondary pressure tip at the back of the pen
> >
> > (1) hidraw7_inRange_contact_move_lift_outOfRange
> > I "contact" the main tip, move it, and lift it out or range.
> >
> > (2) hidraw7_inRange_contact_barrelPress\
> > _move_barrelRelease_lift_outOfRange
> > I "contact" the main tip, press "barrel" (which is the button nearest
> > of the tip), move the pen, release the button, move it out of range.
> >
> > (3) hidraw7_inRange_contact_erasePress\
> > _move_eraseRelease_lift_outOfRange
> > The same than above, except that I instead use the "erase" button,
> > i.e. the button just above ("vertically speaking") of the "main" button.
> >
> > (4) hidraw7_inRange_contact_BOTHbarrelAnderasePress_move_\
> > BOTHbarrelAnderaseRelease_lift_outOfRange
> > That's just plain stupid, and uncomfortable to do, but it's just to
> > show an example of the capability of the hardware ;
> > In this one, I'm doing the same than above, except I stupidly press BOTH
> > buttons.
> > It means that I "contact" the main tip, press both barrel+erase, move
> > the pen, release both button, lift it out of range.
> >
> > (5) hidraw7_inRange_contact_backPress_move_backRelease_lift_outOfRange
> > Back to non-stupid tests, here I am using the "rubber" (which has
> > pressure !), i.e. I use the back of the pen.
> > I contact it, press the rubber to have some pressure, move it, release
> > the pressure, move the pen ouf of contact and out of range.
> >
> > Reading those traces, I think I observed some (to me) very logical
> > behavior ;
> >
> > I agree that the Pen is possibly not acting in respect of the
> > specification, by the fact that... the spec does not differentiate the
> > "eraser" 2nd button, versus the "backside" rubber pressure tip.
> >
> > The traces shows that :
> > - In range [0 - 1] is correctly reported, both for main tip + rubber tip
> > - Tip Pressure [0 - n] is correctly reported, for both pressure tips
>
> Yep, that part is correct and follows the specification
>
> >
> > - Tip Switch [0 - 1] is correctly set to ONE for all traces, when the
> > in-range tip goes to contact. (either main tip or rubber tip)
>
> This is half wrong (spec-wise):
> - tip switch should only be used when the actual tip is in contact.
> - when using the rubber tip, we should get an eraser event :(
>
> >
> > - Barrel Switch [0 - 1] is correctly set to ONE only when pressing
> > button down, and of course only when using the main tip
>
> This is correct (as in it follows the spec)
>
> >
> > - Invert [0 - 1] is set to ONE when (and only) using the back rubber
> > tip, and the ERASE button (2nd button) does NOT set it, so "INVERT"
>
> In your case, the invert usage is correct.
> In Illia's case, the invert usage is forwarded by the eraser usage, so
> this is just plain wrong there.
>
> >
> > - Eraser [0 - 1] is set to ONE ONLY when pressing the 2nd top button,
> > and of :
> > - ONLY when using the main itp
> > - and if you were to instead use the "back" of the pen (rubber tip),
> > then you will have invert=1 + eraser=0
>
> This is not correct (again, according to the specification). XP-Pen
> should report SecondaryBarrelSwitch (0x5a in the HID usage table[0])
> And again, in Illia's case, this button should be reported as an
> invert, and eraser when the tip touches the tablet.
>
> >
> > The main extract of those is, I think, that :
> > - eraser "top" button is completely independent of the back "rubber"
>
> Yes
>
> > - eraser "top" button NEVER concerns itself to set invert=1
>
> yes
>
> > - backside "rubber" pressure tip will set invert=1, but leave eraser=0
>
> yes, but that's not correct, it should report eraser=1 when you touch
> the rubber on the surface.
>
> >
> > So, the behavior probably breaks the specs, but sincerely I'm happy to
> > have the "eraser" button independent of the "rubber eraser", which
> > makes the stylus a somewhat 4-buttons stylus (tip, button1, button2,
> > rubber), and I would like to keep this.
>
> Yes, and I'd like to keep it that way, even if 6.6 and 6.5.8
> apparently broke it.
>
> So, to me:
> - 276e14e6c3993317257e1787e93b7166fbc30905 is wrong: this is a
> firmware bug (reporting invert through eraser) and should not be
> tackled at the generic level, thus it should be reverted
> - both of these tablets are forwarding the useful information, but not
> correctly, which confuses the kernel
> - I should now be able to write regression tests
> - I should be able to provide HID-BPF fixes for those tablets so that
> we can keep them working with or without
> 276e14e6c3993317257e1787e93b7166fbc30905
> reverted (hopefully)
> - problem is I still don't have the mechanics to integrate the HID-BPF
> fixes directly in the kernel tree, so maybe I'll have to write a
> driver for XP-Pen while these internals are set (it shouldn't
> interfere with the HID-BPF out of the tree).
I already added support for a few XP-Pen devices on the UCLogic driver
and I was planning to start working on this one during the weekend in
my DIGImend fork (to simplify testing).
Let me know if you prefer to add it yourself or if you want me to ping
you in the DIGImend discussion.
José Expósito
>
> Cheers,
> Benjamin
>
> >
> > Best Regards,
> >
> > [1] https://www.xp-pen.fr/download-1027.html ; the .deb Qt app
> >
> > --
> > Eric GOUYER
> >
>
> [0] https://usb.org/document-library/hid-usage-tables-14
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox