* [PATCH AUTOSEL 6.17-6.12] HID: pidff: Use direction fix only for conditional effects
[not found] <20251025160905.3857885-1-sashal@kernel.org>
@ 2025-10-25 15:56 ` Sasha Levin
2025-10-25 16:01 ` [PATCH AUTOSEL 6.17-6.6] HID: asus: add Z13 folio to generic group for multitouch to work Sasha Levin
1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2025-10-25 15:56 UTC (permalink / raw)
To: patches, stable
Cc: Tomasz Pakuła, Oleg Makarenko, Jiri Kosina, Sasha Levin,
jikos, bentiss, linux-input, linux-usb
From: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
[ Upstream commit f345a4798dab800159b09d088e7bdae0f16076c3 ]
The already fixed bug in SDL only affected conditional effects. This
should fix FFB in Forza Horizion 4/5 on Moza Devices as Forza Horizon
flips the constant force direction instead of using negative magnitude
values.
Changing the direction in the effect directly in pidff_upload_effect()
would affect it's value in further operations like comparing to the old
effect and/or just reading the effect values in the user application.
This, in turn, would lead to constant PID_SET_EFFECT spam as the effect
direction would constantly not match the value that's set by the
application.
This way, it's still transparent to any software/API.
Only affects conditional effects now so it's better for it to explicitly
state that in the name. If any HW ever needs fixed direction for other
effects, we'll add more quirks.
Signed-off-by: Tomasz Pakuła <tomasz.pakula.oficjalny@gmail.com>
Reviewed-by: Oleg Makarenko <oleg@makarenk.ooo>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
YES
- What it fixes
- The earlier quirk forced a fixed direction (0x4000) for all effect
types on specific wheelbases to work around mis-set PID directions
in some user stacks (dinput → wine → SDL). That broad forcing breaks
games that legitimately use the direction field for non-conditional
effects (e.g., Forza Horizon 4/5 uses direction flips for constant
force instead of negative magnitudes). This patch narrows the quirk
to conditional effects only (spring, damper, inertia, friction),
matching where the SDL-side bug actually applied.
- How it changes behavior
- Adds a helper to detect conditional effects and a wrapper to set the
direction only when appropriate:
- New logic: pidff_is_effect_conditional() and
pidff_set_effect_direction() in drivers/hid/usbhid/hid-pidff.c
(replaces unconditional forcing in Set Effect).
- Set Effect now calls the helper instead of unconditionally forcing
direction for all effects.
- Renames the quirk to reflect scope:
HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION →
HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION, still using the same bit
(BIT(3)).
- Updates device ID table entries to use the new quirk name for Moza
devices in drivers/hid/hid-universal-pidff.c.
- Why it’s appropriate for stable
- User-visible bug/regression: Fixes incorrect or missing FFB in Forza
Horizon 4/5 on Moza devices when the kernel overrode constant-force
directions. The message explicitly states this resolves that case.
- Small and contained: Limited to HID PID force-feedback code and
quirk tables; no architectural changes.
- Minimal risk: Only affects devices already marked with the quirk
bit. Behavior is narrowed (less intrusive) by applying the fixed
direction only to conditional effects; other effect types now honor
the application-provided direction as intended.
- No side effects on unrelated subsystems: Touches only HID FFB code
paths.
- Clear lineage: It logically corrects the earlier
“FIX_WHEEL_DIRECTION” quirk (drivers/hid/usbhid/hid-pidff.c:397)
that forced direction for all effect types.
- Specific code references
- Current unconditional forcing (to be replaced by helper call):
drivers/hid/usbhid/hid-pidff.c:397
- Quirk bit definition that’s renamed but remains BIT(3):
drivers/hid/usbhid/hid-pidff.h:20
- Fixed direction constant (still used, but now applied
conditionally): drivers/hid/usbhid/hid-pidff.c:151
- Device entries using the quirk (updated to new name):
drivers/hid/hid-universal-pidff.c:6
- Additional considerations
- The commit avoids mutating effect->direction in
pidff_upload_effect(), preventing mismatches with the application’s
state and avoiding needless PID_SET_EFFECT churn. It keeps behavior
transparent to user space.
- If any future hardware requires fixed direction for non-conditional
effects, the commit message notes that more targeted quirks will be
added, which further limits regression risk.
Given it’s a targeted regression fix for a real-world breakage, small in
scope, and reduces the quirk’s blast radius, it’s a strong candidate for
stable backport (especially on branches that already carry the earlier
FIX_WHEEL_DIRECTION quirk).
drivers/hid/hid-universal-pidff.c | 20 ++++++++++----------
drivers/hid/usbhid/hid-pidff.c | 28 +++++++++++++++++++++++-----
drivers/hid/usbhid/hid-pidff.h | 2 +-
3 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/hid-universal-pidff.c b/drivers/hid/hid-universal-pidff.c
index 554a6559aeb73..70fce0f88e825 100644
--- a/drivers/hid/hid-universal-pidff.c
+++ b/drivers/hid/hid-universal-pidff.c
@@ -144,25 +144,25 @@ static int universal_pidff_input_configured(struct hid_device *hdev,
static const struct hid_device_id universal_pidff_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R3),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R3_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R5),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R5_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R9),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R9_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R12),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R12_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R16_R21),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_MOZA, USB_DEVICE_ID_MOZA_R16_R21_2),
- .driver_data = HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION },
+ .driver_data = HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION },
{ HID_USB_DEVICE(USB_VENDOR_ID_CAMMUS, USB_DEVICE_ID_CAMMUS_C5) },
{ HID_USB_DEVICE(USB_VENDOR_ID_CAMMUS, USB_DEVICE_ID_CAMMUS_C12) },
{ HID_USB_DEVICE(USB_VENDOR_ID_VRS, USB_DEVICE_ID_VRS_DFP),
diff --git a/drivers/hid/usbhid/hid-pidff.c b/drivers/hid/usbhid/hid-pidff.c
index 614a20b620231..c6b4f61e535d5 100644
--- a/drivers/hid/usbhid/hid-pidff.c
+++ b/drivers/hid/usbhid/hid-pidff.c
@@ -205,6 +205,14 @@ struct pidff_device {
u8 effect_count;
};
+static int pidff_is_effect_conditional(struct ff_effect *effect)
+{
+ return effect->type == FF_SPRING ||
+ effect->type == FF_DAMPER ||
+ effect->type == FF_INERTIA ||
+ effect->type == FF_FRICTION;
+}
+
/*
* Clamp value for a given field
*/
@@ -294,6 +302,20 @@ static void pidff_set_duration(struct pidff_usage *usage, u16 duration)
pidff_set_time(usage, duration);
}
+static void pidff_set_effect_direction(struct pidff_device *pidff,
+ struct ff_effect *effect)
+{
+ u16 direction = effect->direction;
+
+ /* Use fixed direction if needed */
+ if (pidff->quirks & HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION &&
+ pidff_is_effect_conditional(effect))
+ direction = PIDFF_FIXED_WHEEL_DIRECTION;
+
+ pidff->effect_direction->value[0] =
+ pidff_rescale(direction, U16_MAX, pidff->effect_direction);
+}
+
/*
* Send envelope report to the device
*/
@@ -395,11 +417,7 @@ static void pidff_set_effect_report(struct pidff_device *pidff,
pidff->set_effect[PID_GAIN].field->logical_maximum;
pidff->set_effect[PID_DIRECTION_ENABLE].value[0] = 1;
- /* Use fixed direction if needed */
- pidff->effect_direction->value[0] = pidff_rescale(
- pidff->quirks & HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION ?
- PIDFF_FIXED_WHEEL_DIRECTION : effect->direction,
- U16_MAX, pidff->effect_direction);
+ pidff_set_effect_direction(pidff, effect);
/* Omit setting delay field if it's missing */
if (!(pidff->quirks & HID_PIDFF_QUIRK_MISSING_DELAY))
diff --git a/drivers/hid/usbhid/hid-pidff.h b/drivers/hid/usbhid/hid-pidff.h
index a53a8b436baa6..f321f675e1318 100644
--- a/drivers/hid/usbhid/hid-pidff.h
+++ b/drivers/hid/usbhid/hid-pidff.h
@@ -16,7 +16,7 @@
#define HID_PIDFF_QUIRK_PERMISSIVE_CONTROL BIT(2)
/* Use fixed 0x4000 direction during SET_EFFECT report upload */
-#define HID_PIDFF_QUIRK_FIX_WHEEL_DIRECTION BIT(3)
+#define HID_PIDFF_QUIRK_FIX_CONDITIONAL_DIRECTION BIT(3)
/* Force all periodic effects to be uploaded as SINE */
#define HID_PIDFF_QUIRK_PERIODIC_SINE_ONLY BIT(4)
--
2.51.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* [PATCH AUTOSEL 6.17-6.6] HID: asus: add Z13 folio to generic group for multitouch to work
[not found] <20251025160905.3857885-1-sashal@kernel.org>
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17-6.12] HID: pidff: Use direction fix only for conditional effects Sasha Levin
@ 2025-10-25 16:01 ` Sasha Levin
1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2025-10-25 16:01 UTC (permalink / raw)
To: patches, stable
Cc: Antheas Kapenekakis, Luke D. Jones, Jiri Kosina, Sasha Levin,
jikos, bentiss, linux-input
From: Antheas Kapenekakis <lkml@antheas.dev>
[ Upstream commit b595974b4afe0e171dd707da570964ff642742e3 ]
The Asus Z13 folio has a multitouch touchpad that needs to bind
to the hid-multitouch driver in order to work properly. So bind
it to the HID_GROUP_GENERIC group to release the touchpad and
move it to the bottom so that the comment applies to it.
While at it, change the generic KEYBOARD3 name to Z13_FOLIO.
Reviewed-by: Luke D. Jones <luke@ljones.dev>
Signed-off-by: Antheas Kapenekakis <lkml@antheas.dev>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
Based on my comprehensive analysis, here is my recommendation:
## Backport Status: **YES**
### Summary of Analysis
This commit fixes a hardware functionality bug where the multitouch
touchpad on the ASUS ROG Z13 folio does not work. The fix is a textbook
example of a stable tree backport candidate.
### Technical Understanding
**What Changed:**
1. **drivers/hid/hid-asus.c (lines 1387-1421)**: The device entry for
Z13 folio (device ID 0x1a30) was moved from the middle of the device
table to the bottom, and crucially changed from:
```c
HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3)
```
to:
```c
HID_DEVICE(BUS_USB, HID_GROUP_GENERIC, USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO)
```
2. **drivers/hid/hid-ids.h (line 226)**: Renamed the constant from
`USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3` to the more descriptive
`USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO`
**Why This Matters:**
- The Z13 folio is a composite USB device with both a keyboard and
multitouch touchpad
- `HID_USB_DEVICE` expands to `.bus = BUS_USB` without specifying a
group, so it matches **all HID groups**
- `HID_DEVICE(BUS_USB, HID_GROUP_GENERIC, ...)` explicitly restricts
binding to only `HID_GROUP_GENERIC`
- The multitouch touchpad presents itself as `HID_GROUP_MULTITOUCH` or
`HID_GROUP_MULTITOUCH_WIN_8`
- When hid-asus binds to all groups, it prevents hid-multitouch from
binding to the touchpad
- By restricting hid-asus to `HID_GROUP_GENERIC` (keyboard), hid-
multitouch can properly handle the touchpad
### Evidence of Correctness
1. **Established Pattern**: This exact fix was applied to the T101HA
keyboard in 2021 (commit a94f66aecdaa4). The commit message for that
fix explicitly states: *"The hid-multitouch hiddev has a group of
HID_GROUP_MULTITOUCH_WIN_8, so the same result can be achieved by
making the hid_device_id entry for the dock in the asus_devices[]
table only match on HID_GROUP_GENERIC"*
2. **Wide Usage**: Verified that `HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
...)` pattern is used extensively in:
- hid-asus.c (T101HA at drivers/hid/hid-asus.c:1422)
- hid-google-hammer.c (9+ instances)
- hid-ite.c (4+ instances)
- hid-lenovo.c (5+ instances)
3. **Device History**: The Z13 keyboard was originally added in v6.5
(commit 74e47b2c52ed4) using HID_USB_DEVICE, which inadvertently
prevented the touchpad from working. This commit corrects that
oversight.
4. **Author Credibility**: Antheas Kapenekakis has 15+ kernel
contributions focused on platform/x86 and HID subsystems, including
other Z13 fixes. The patch was reviewed by Luke D. Jones, the
original author of Z13 support.
5. **Comment Alignment**: The commit moves the entry below an existing
comment (lines 1415-1418) that explicitly explains: *"Note bind to
the HID_GROUP_GENERIC group, so that we only bind to the keyboard
part, while letting hid-multitouch.c handle the touchpad."* This
comment now applies to both T101HA and Z13 folio.
### Risk Analysis
**Minimal Risk:**
- **Isolated Scope**: Only affects ASUS device ID 0x1a30 (Z13 folio)
- **No Dependencies**: Standalone fix with no prerequisites
- **No Architectural Changes**: Uses existing, proven pattern
- **Verified Pattern**: Multiple similar devices use this approach since
2021 without issues
- **Clean Rename**: Old constant name completely removed, no lingering
references found
**Regression Potential:**
- Extremely low - the change makes a non-working touchpad work
- Users without this fix have a broken touchpad; users with it have
working touchpad
- The keyboard functionality is unaffected (same quirks applied)
### Stable Tree Criteria Assessment
✅ **Fixes important bug**: Multitouch touchpad completely non-functional
✅ **Small and contained**: 2 files changed, device ID rename + entry
repositioning
✅ **No architectural changes**: Uses established HID group filtering
pattern
✅ **Minimal regression risk**: Proven pattern, isolated to one device
✅ **Confined to subsystem**: HID subsystem only
✅ **Applies to stable**: Any stable kernel >= 6.5 (where Z13 support
exists)
**Note**: While the commit lacks a `Cc: stable@vger.kernel.org` tag or
`Fixes:` tag, it clearly meets stable tree criteria and should be
backported to all stable kernels that contain the original Z13 support
(6.5+).
### Conclusion
This is an **unambiguous YES** for backporting. The commit:
- Fixes real hardware (touchpad doesn't work without it)
- Uses a well-tested, 4+ year old pattern
- Has minimal risk and isolated impact
- Comes from a credible contributor
- Was reviewed by the original device maintainer
This should be backported to all stable trees from 6.5 onwards where the
Z13 folio device support exists.
drivers/hid/hid-asus.c | 6 +++---
drivers/hid/hid-ids.h | 2 +-
2 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-asus.c b/drivers/hid/hid-asus.c
index 8db9d4e7c3b0b..a444d41e53b6c 100644
--- a/drivers/hid/hid-asus.c
+++ b/drivers/hid/hid-asus.c
@@ -1387,9 +1387,6 @@ static const struct hid_device_id asus_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2),
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
- { HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
- USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3),
- QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
{ HID_USB_DEVICE(USB_VENDOR_ID_ASUSTEK,
USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR),
QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
@@ -1419,6 +1416,9 @@ static const struct hid_device_id asus_devices[] = {
* Note bind to the HID_GROUP_GENERIC group, so that we only bind to the keyboard
* part, while letting hid-multitouch.c handle the touchpad.
*/
+ { HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
+ USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO),
+ QUIRK_USE_KBD_BACKLIGHT | QUIRK_ROG_NKEY_KEYBOARD },
{ HID_DEVICE(BUS_USB, HID_GROUP_GENERIC,
USB_VENDOR_ID_ASUSTEK, USB_DEVICE_ID_ASUSTEK_T101HA_KEYBOARD) },
{ }
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index ded5348d190c5..5721b8414bbdf 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -223,7 +223,7 @@
#define USB_DEVICE_ID_ASUSTEK_ROG_KEYBOARD3 0x1822
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD 0x1866
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD2 0x19b6
-#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_KEYBOARD3 0x1a30
+#define USB_DEVICE_ID_ASUSTEK_ROG_Z13_FOLIO 0x1a30
#define USB_DEVICE_ID_ASUSTEK_ROG_Z13_LIGHTBAR 0x18c6
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY 0x1abe
#define USB_DEVICE_ID_ASUSTEK_ROG_NKEY_ALLY_X 0x1b4c
--
2.51.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
end of thread, other threads:[~2025-10-25 16:29 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20251025160905.3857885-1-sashal@kernel.org>
2025-10-25 15:56 ` [PATCH AUTOSEL 6.17-6.12] HID: pidff: Use direction fix only for conditional effects Sasha Levin
2025-10-25 16:01 ` [PATCH AUTOSEL 6.17-6.6] HID: asus: add Z13 folio to generic group for multitouch to work Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).