[PATCH v3 0/4] input: force feedback for trigger rumble motors

[Guillaume Casal]

Some gamepads carry rumble motors behind their triggers, in addition to
the usual two in the grips: Xbox controllers call them impulse triggers,
and handhelds such as the ASUS ROG Xbox Ally X have them too. The force
feedback API cannot reach those motors.

v1: https://lore.kernel.org/linux-input/20260730095317.767418-1-guillaume.casal.42@xxxxxxxxx/
v2: https://lore.kernel.org/linux-input/20260730134004.1118476-1-guillaume.casal.42@xxxxxxxxx/

Changes since v2
================

Patch 1: the new structure was inserted between the kernel-doc block of
struct ff_haptic_effect and the structure itself, leaving that block
orphaned and stripping ff_haptic_effect of its documentation. It now goes
after ff_haptic_effect, before the kernel-doc of struct ff_effect. No
code change.

Patch 4: the description column no longer relies on a tab that overshoots
for a name this long.

Patches 2 and 3 are unchanged.

A fifth patch ratelimiting the pr_err() in get_compatible_type() was
prepared and dropped before sending: input_ff_upload() rejects any type
absent from dev->ffbit before storing the effect, and input_ff_create()
copies dev->ffbit into ff->ffbit, so the path it was meant to protect is
not reachable for an in-tree driver.

Changes since v1
================

v1 followed the 2022 series by Daniel Bomar and added trigger_left and
trigger_right to struct ff_rumble_effect. That was wrong, and I am
grateful the problem was pointed out on the list before it went any
further.

EVIOCSFF copies the whole struct ff_effect from userspace. An
application that sets only strong_magnitude and weak_magnitude, and does
not clear the rest of the union, passes uninitialised stack content in
the bytes that v1 gave meaning to. ff-memless would then scale that
content and hand it to the driver, so a program that works today would
start buzzing the triggers at random. Keeping the size of the union
unchanged makes the layout compatible, not the meaning. This is likely
why the 2022 series never went anywhere.

v2 therefore uses a distinct effect type, FF_TRIGGER_RUMBLE, with its own
struct ff_trigger_rumble_effect, following what was done for FF_HAPTIC.
No existing application emits this type, so no uninitialised byte can be
mistaken for a magnitude. The capability bit of v1 is dropped: userspace
discovers the type in the EVIOCGBIT(EV_FF) bitmap, as it does for
FF_RUMBLE.

Two smaller points raised on v1 are fixed here as well: the missing entry
in the force[] array of hid-debug.c, and the missing line in
Documentation/input/ff.rst.

No in-tree user yet
===================

I want to be upfront about this, since it is the obvious objection: at
the tip of this series, git grep FF_TRIGGER_RUMBLE matches only the four
files it touches. No driver advertises the type, so the new arm in
ml_combine_effects() cannot run yet.

The driver that does drive it exists and is tested, on the handheld
described below, but it lives in hid-asus, which is not in mainline. Its
maintainers carry the gamepad support out of tree and upstream it in
steps. Sending its patch with this series would mean sending a patch
against a file that does not contain the code it modifies.

So the order is deliberate: settle the API here, then the driver patch
goes to its maintainers, then the SDL side. If you would rather see a
user in the same series, I can hold this until the gamepad part of
hid-asus lands upstream, but that is not under my control.

Design notes
============

ml_get_combo_effect() groups playing effects by type, so a trigger rumble
effect forms its own combo and reaches the driver in its own
play_effect() call, without disturbing FF_RUMBLE. An effect that has
finished still contributes its type with zero magnitudes, so the stop
path needs no change.

Testing
=======

checkpatch.pl reports no warnings on any of the four patches.

I booted v7.2-rc5 with this series applied under QEMU, with a small
memless driver compiled in that advertises both types and logs the
magnitudes it is handed. Userspace side, a static init issues the
EVIOCSFF ioctls. Output, kernel lines interleaved with userspace ones:

  capabilities advertised : FF_RUMBLE yes, FF_TRIGGER_RUMBLE yes

  triggers only      -> FF_TRIGGER_RUMBLE left=65535 right=65535
  grips only         -> FF_RUMBLE         strong=65535 weak=65535
  left trigger only  -> FF_TRIGGER_RUMBLE left=65535 right=    0
  right trigger only -> FF_TRIGGER_RUMBLE left=    0 right=65535

Each effect is followed by a call with zero magnitudes when it expires,
so the existing stop path needs no change.

The last case is the one that matters for the concern raised on v1. The
test fills the whole struct ff_effect with 0xAA, sets only type,
replay.length and the two rumble magnitudes, and uploads it, which is
what a careless application does today:

  old-style app, dirty union
                     -> FF_RUMBLE         strong=32768 weak=32768
                        and no FF_TRIGGER_RUMBLE call at all

The dirty bytes cannot be mistaken for trigger magnitudes, because they
never carry that meaning for an FF_RUMBLE effect. With v1 they would have
been scaled and sent to the hardware.

The same was then run against real hardware. The vendor kernel of my
handheld cannot be rebuilt, its sources are not published, so I booted
the patched kernel in a VM on the same machine and handed it the gamepad
by USB passthrough. The driver bound it as usual and the device came up
advertising both types:

  peripheral: /dev/input/event8 (ASUS ROG Ally X Gamepad)
  capabilities advertised : FF_RUMBLE yes, FF_TRIGGER_RUMBLE yes

Capturing the physical bus from the host with usbmon while the VM drove
the pad gives the reports the firmware actually received:

  triggers: L=100 R=100   grips: L=  0 R=  0    triggers only
  triggers: L=  0 R=  0   grips: L=100 R=100    grips only
  triggers: L=100 R=  0   grips: L=  0 R=  0    left trigger only
  triggers: L=  0 R=100   grips: L=  0 R=  0    right trigger only
  triggers: L=  0 R=  0   grips: L= 50 R= 50    old-style app, dirty union

The four actuators are driven independently, and the careless
application gets its 50% in the grips with nothing in the triggers,
which is the whole point of the change in shape since v1.

The driver patch that fills those two magnitudes is not part of this
series: that driver is not in mainline, and I will send it to its
maintainers once the API here settles.

Guillaume Casal (4):
  input: Add FF_TRIGGER_RUMBLE effect type
  input: ff-memless: Handle FF_TRIGGER_RUMBLE
  HID: debug: Add FF_TRIGGER_RUMBLE name
  Documentation: input: Document FF_TRIGGER_RUMBLE

 Documentation/input/ff.rst |  1 +
 drivers/hid/hid-debug.c    |  1 +
 drivers/input/ff-memless.c |  9 +++++++++
 include/uapi/linux/input.h | 18 +++++++++++++++++-
 4 files changed, 29 insertions(+), 1 deletion(-)