* [PATCH AUTOSEL 7.0-6.1] HID: google: hammer: stop hardware on devres action failure
From: Sasha Levin @ 2026-05-20 11:19 UTC (permalink / raw)
To: patches, stable
Cc: Myeonghun Pak, Jiri Kosina, Sasha Levin, jikos, bentiss,
linux-input, linux-kernel
In-Reply-To: <20260520111944.3424570-1-sashal@kernel.org>
From: Myeonghun Pak <mhun512@gmail.com>
[ Upstream commit b08665fe80fab0956e64741c07d9bbcec635c34d ]
hammer_probe() starts the HID hardware before registering the devres
action that stops it. If devm_add_action() fails, probe returns an
error with the hardware still started because the cleanup action was
never registered and the driver's remove callback is not called after a
failed probe.
Use devm_add_action_or_reset() so the stop action runs immediately on
registration failure while preserving the existing devres-managed cleanup
path for later probe failures and remove.
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase Walkthrough
### Phase 1: Commit Message Forensics
- Step 1.1 Record: subsystem `HID: google: hammer`; action verb `stop`;
intent is to stop HID hardware if devres action registration fails.
- Step 1.2 Record: tags in committed
`b08665fe80fab0956e64741c07d9bbcec635c34d`: `Signed-off-by: Myeonghun
Pak <mhun512@gmail.com>` and `Signed-off-by: Jiri Kosina
<jkosina@suse.com>`. No `Fixes:`, `Reported-by:`, `Tested-by:`,
`Reviewed-by:`, `Acked-by:`, `Link:`, or `Cc: stable`.
- Step 1.3 Record: bug is an error-path cleanup failure.
`hammer_probe()` calls `hid_hw_start()` before registering
`hammer_stop` with devres; if `devm_add_action()` returns `-ENOMEM`,
probe fails and no registered cleanup action calls `hid_hw_stop()`.
- Step 1.4 Record: yes, this is a hidden bug fix: a one-line
devres/error-path cleanup fix preventing started HID hardware from
being left active after failed probe.
### Phase 2: Diff Analysis
- Step 2.1 Record: one file, `drivers/hid/hid-google-hammer.c`; 1
insertion, 1 deletion; modified function `hammer_probe()`; single-file
surgical fix.
- Step 2.2 Record: before, failure of `devm_add_action()` returned
directly after `hid_hw_start()` succeeded. After,
`devm_add_action_or_reset()` runs `hammer_stop(hdev)` immediately if
action registration fails.
- Step 2.3 Record: bug category is error-path resource cleanup. The
broken state is missing `hid_hw_stop()` after successful
`hid_hw_start()` when devres action allocation fails.
- Step 2.4 Record: fix quality is high: it uses the kernel helper whose
inline definition calls the action on registration failure. No
unrelated changes, no public API change, and no normal-path behavior
change when registration succeeds.
### Phase 3: Git History Investigation
- Step 3.1 Record: blame shows `devm_add_action(&hdev->dev, hammer_stop,
hdev)` came from `d950db3f80a801` (`HID: google: switch to devm when
registering keyboard backlight LED`), first contained by `v5.18-rc1`.
- Step 3.2 Record: no `Fixes:` tag. I followed the blamed introducing
commit instead; `d950db3f80a801` added `hammer_stop()` and moved stop
cleanup into devres.
- Step 3.3 Record: recent file history shows related Google HID
cleanup/device-ID/vivaldi work, but no intermediate fix for this exact
missed cleanup.
- Step 3.4 Record: candidate author has only this HID commit in
`origin/master` in the checked history. Jiri Kosina is listed in
`MAINTAINERS` for HID core and applied/signed the commit.
- Step 3.5 Record: no dependency found. The replacement helper exists in
affected stable branches checked.
### Phase 4: Mailing List And External Research
- Step 4.1 Record: `b4 dig -c b08665fe80fab` found the original patch at
`https://patch.msgid.link/20260424125043.52639-1-
pakmyeonghun@bagmyeonghun-ui-MacBookPro.local`. `b4 dig -a` found only
v1.
- Step 4.2 Record: `b4 dig -w` showed the original recipients were
Myeonghun Pak, Jiri Kosina, Benjamin Tissoires, `linux-
input@vger.kernel.org`, and `linux-kernel@vger.kernel.org`.
- Step 4.3 Record: no separate bug report or `Reported-by` tag was
present.
- Step 4.4 Record: thread had 3 messages; Jiri replied, “Makes sense,
thanks for catching it. Applied.” No NAKs or objections found in the
fetched mbox.
- Step 4.5 Record: direct lore stable web search was blocked by Anubis;
local stable branch log search did not find this commit already
present.
### Phase 5: Code Semantic Analysis
- Step 5.1 Record: modified function is `hammer_probe()`.
- Step 5.2 Record: `hammer_probe()` is registered as `.probe` in
`hammer_driver`; HID core reaches it via `hid_device_probe()` ->
`__hid_device_probe()` -> `hdrv->probe(hdev, id)`.
- Step 5.3 Record: key callees are `hid_parse()`, `hid_hw_start()`,
`devm_add_action_or_reset()`, `hid_hw_open()`, and
`hammer_register_leds()`. `hid_hw_start()` documentation says
`hid_hw_stop()` must be called after success.
- Step 5.4 Record: path is reachable during HID device probe for Google
Hammer-family USB IDs in the driver table. Trigger for the bug is
allocation failure inside `devm_add_action()` after hardware start.
- Step 5.5 Record: no other `hammer_stop` pattern found. Nearby HID
drivers commonly call `hid_hw_stop()` on probe-error paths, supporting
that this cleanup is expected.
### Phase 6: Stable Tree Analysis
- Step 6.1 Record: buggy code exists in `stable/linux-6.1.y`, `6.6.y`,
`6.12.y`, `6.15.y`, `6.16.y`, `6.17.y`, `6.18.y`, and `6.19.y`; not in
`stable/linux-5.15.y`.
- Step 6.2 Record: patch applied cleanly with `git apply --check` on
temporary worktrees for `6.1.y`, `6.6.y`, `6.12.y`, and `6.19.y`.
- Step 6.3 Record: no related stable fix with the same subject was found
in checked stable branch logs.
### Phase 7: Subsystem Context
- Step 7.1 Record: subsystem is HID driver code, specifically Google
Hammer HID devices; criticality is driver-specific/peripheral.
- Step 7.2 Record: HID is actively maintained; recent `drivers/hid`
history includes multiple fixes and quirks, and the candidate was
merged through the HID tree.
### Phase 8: Impact And Risk
- Step 8.1 Record: affected users are users of Google Hammer-family HID
hardware on stable kernels containing `d950db3f80a801`.
- Step 8.2 Record: trigger is uncommon but real: memory/devres
allocation failure during device probe after `hid_hw_start()`
succeeds. I did not verify unprivileged syscall reachability.
- Step 8.3 Record: failure mode is leaked/active HID hardware state
after failed probe, not a verified crash or data corruption. Severity
is medium, with higher concern under memory pressure because cleanup
is explicitly required by HID core semantics.
- Step 8.4 Record: benefit is moderate for affected hardware and error
recovery correctness; risk is very low because the successful path is
unchanged and the failure path calls the already intended cleanup
function.
### Phase 9: Final Synthesis
- Step 9.1 Record: evidence for backporting: real cleanup bug, affected
maintained stable trees, one-line obviously correct fix, uses existing
helper, applies cleanly, maintainer accepted it. Evidence against: no
reported user crash, rare trigger, driver-specific impact. Unresolved:
no runtime test report or concrete field failure report found.
- Step 9.2 Record: stable rules: obviously correct yes; fixes real bug
yes; important enough as resource/hardware-state leak on failed probe
yes, though not crash-class; small and contained yes; no new
feature/API yes; applies to stable yes for checked branches.
- Step 9.3 Record: no automatic exception category; this is not a device
ID, quirk, DT, build, or documentation patch.
- Step 9.4 Record: risk-benefit favors backporting because the fix is a
minimal correction to a verified cleanup bug in stable-present code.
## Verification
- Phase 1: inspected `git show --stat --no-ext-diff b08665fe80fab`;
confirmed subject, body, tags, and 1-line diff.
- Phase 2: inspected full candidate diff; confirmed only
`devm_add_action()` -> `devm_add_action_or_reset()` in
`hammer_probe()`.
- Phase 3: ran blame on changed area; confirmed introducing commit
`d950db3f80a801`, first contained by `v5.18-rc1`.
- Phase 3: inspected `d950db3f80a801`; confirmed it added devres-managed
`hammer_stop()`.
- Phase 4: ran `b4 dig -c`, `-a`, and `-w`; confirmed original patch
URL, v1 only, maintainers/lists included.
- Phase 4: fetched full thread with `b4 mbox`; confirmed Jiri Kosina
replied that the patch made sense and was applied.
- Phase 5: read HID core and devres code; confirmed `hid_hw_start()`
requires `hid_hw_stop()`, and `devm_add_action_or_reset()` invokes the
action on failure.
- Phase 6: checked stable branches with `git grep`; confirmed buggy line
in `6.1.y` through `6.19.y`, not `5.15.y`.
- Phase 6: ran `git apply --check` in temporary worktrees for `6.1.y`,
`6.6.y`, `6.12.y`, and `6.19.y`; all passed.
- UNVERIFIED: no separate user bug report, crash report, or runtime test
result was found.
**YES**
drivers/hid/hid-google-hammer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hid/hid-google-hammer.c b/drivers/hid/hid-google-hammer.c
index 1af477e58480b..c99c3c0d442e1 100644
--- a/drivers/hid/hid-google-hammer.c
+++ b/drivers/hid/hid-google-hammer.c
@@ -496,7 +496,7 @@ static int hammer_probe(struct hid_device *hdev,
if (error)
return error;
- error = devm_add_action(&hdev->dev, hammer_stop, hdev);
+ error = devm_add_action_or_reset(&hdev->dev, hammer_stop, hdev);
if (error)
return error;
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-6.12] HID: i2c-hid: add reset quirk for BLTP7853 touchpad
From: Sasha Levin @ 2026-05-20 11:19 UTC (permalink / raw)
To: patches, stable
Cc: Xu Rao, Jiri Kosina, Sasha Levin, jikos, bentiss, linux-input,
linux-kernel
In-Reply-To: <20260520111944.3424570-1-sashal@kernel.org>
From: Xu Rao <raoxu@uniontech.com>
[ Upstream commit a991aa5e89365ba1959fae6847fd288125b209e5 ]
The BLTP7853 I2C HID touchpad may fail to probe after reboot or
reprobe because reset completion is not signalled to the host. The
driver then waits for the reset-complete interrupt until it times out
and the device probe fails:
i2c_hid i2c-BLTP7853:00: failed to reset device.
i2c_hid i2c-BLTP7853:00: can't add hid device: -61
i2c_hid: probe of i2c-BLTP7853:00 failed with error -61
Add I2C_HID_QUIRK_NO_IRQ_AFTER_RESET for the device so i2c-hid does
not wait for a reset interrupt that may never arrive.
Signed-off-by: Xu Rao <raoxu@uniontech.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase Walkthrough
### Phase 1: Commit Message Forensics
Record: subsystem `HID: i2c-hid`; action verb `add`; intent is to add a
reset-handling quirk for the BLTP7853 touchpad.
Record: tags present are `Signed-off-by: Xu Rao <raoxu@uniontech.com>`
and `Signed-off-by: Jiri Kosina <jkosina@suse.com>`. No `Fixes:`,
`Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-by:`, `Link:`, or
`Cc: stable@vger.kernel.org` tags are present in the supplied message.
Record: the commit message describes a real probe failure: the device
may not signal reset completion after reboot or reprobe, the driver
waits for reset completion, and probe fails with `-61` / `-ENODATA`. The
root cause given is missing reset-complete IRQ from the device.
Record: this is not hidden as cleanup; it is explicitly a hardware
workaround for a probe failure.
### Phase 2: Diff Analysis
Record: files changed are `drivers/hid/hid-ids.h` with 3 additions and
`drivers/hid/i2c-hid/i2c-hid-core.c` with 2 additions. Total scope: 5
added lines, no removals. No function body is changed; the modified
object is the private `i2c_hid_quirks[]` table.
Record: before the patch, vendor/product `0x36b6/0xc001` had no private
i2c-hid quirk entry, so the reset path used normal reset-ack waiting.
After the patch, `i2c_hid_lookup_quirk()` can match the BLTP7853 IDs and
set `I2C_HID_QUIRK_NO_IRQ_AFTER_RESET`.
Record: in current code, `I2C_HID_QUIRK_NO_IRQ_AFTER_RESET` makes
`i2c_hid_finish_hwreset()` sleep 100 ms and clear
`I2C_HID_RESET_PENDING` instead of waiting for a reset IRQ. In older
stable code, the same quirk skips the 5 second wait path that returns
`-ENODATA`.
Record: bug category is hardware workaround / quirk. Specific mechanism:
a device that does not raise the expected reset-complete interrupt is
handled through an existing no-IRQ-after-reset path.
Record: fix quality is high: tiny table-only change, uses existing
infrastructure, no new API, no locking change, no cross-subsystem
behavior. Regression risk is very low and limited to devices reporting
exactly the new vendor/product pair.
### Phase 3: Git History Investigation
Record: exact candidate subject was not found in local `git log` on
`master`, `all-next`, `fixes-next`, `input-next`, `pending-7.0`, or
`for-greg/7.0-200`; no candidate commit hash was available to blame the
newly added lines.
Record: existing no-IRQ reset quirk support was introduced by
`402946a8ef71e` (`HID: i2c-hid: Add no-irq-after-reset quirk for
0911:5288 device`), contained since `v4.15-rc1`. Related Voyo quirk
`fc6a31b007393` shows the same failure pattern: repeated `failed to
reset device` and final `can't add hid device: -61`.
Record: no `Fixes:` tag is present, so Step 3.2 is not applicable.
Record: recent history contains normal HID/i2c-hid quirk and fix
activity. Relevant related changes include `7bcf9ebb50f2a` (`Turn
missing reset ack into a warning`) and older device-specific no-IRQ
quirks. No prerequisite was identified beyond existing
`I2C_HID_QUIRK_NO_IRQ_AFTER_RESET` support.
Record: local `git log --author="Xu Rao" -- drivers/hid` found no HID
commits on `master`. `MAINTAINERS` confirms Jiri Kosina is an HID core
maintainer, and he signed off the supplied commit.
### Phase 4: Mailing List And External Research
Record: candidate `b4 dig` could not be performed because the exact
candidate commit hash is unavailable and the subject is not present in
local reachable history.
Record: `b4 dig -c 402946a8ef71e` found the original related no-IRQ-
after-reset quirk submission at
`https://patch.msgid.link/20171107122800.23196-1-hdegoede@redhat.com`;
`b4 dig -a` showed v1 and v2; `b4 dig -w` showed HID maintainers and
`linux-input@vger.kernel.org` were included.
Record: `b4 dig -c 7bcf9ebb50f2a` found the reset-ack warning patch as
`[PATCH v3 5/7]`.
Record: web searches for the exact BLTP7853 reset-quirk subject and
stable-list discussion found no direct candidate thread. Searches did
find related BLTP7853 touchpad reports, but not independent confirmation
of this exact `0x36b6/0xc001` reset failure.
### Phase 5: Code Semantic Analysis
Record: modified key object is `i2c_hid_quirks[]`; relevant functions
are `i2c_hid_lookup_quirk()`, `__i2c_hid_core_probe()`,
`i2c_hid_finish_hwreset()`, and older-branch `i2c_hid_command()`
behavior.
Record: callers verified: `i2c_hid_core_probe()` is called from ACPI,
OF, ELAN, and Goodix i2c-hid probe drivers. `__i2c_hid_core_probe()`
reads HID descriptor vendor/product IDs and calls
`i2c_hid_lookup_quirk()`.
Record: call chain is device enumeration/probe: platform driver probe ->
`i2c_hid_core_probe()` -> `__i2c_hid_core_probe()` ->
`i2c_hid_lookup_quirk()` -> HID registration -> reset during HID parse.
This is reachable during boot, reboot, and reprobe for affected
hardware.
Record: similar patterns exist for HANTICK, ITE/Voyo, and Raydium
devices already using `I2C_HID_QUIRK_NO_IRQ_AFTER_RESET`.
### Phase 6: Stable Tree Analysis
Record: `I2C_HID_QUIRK_NO_IRQ_AFTER_RESET` exists in active checked
stable branches `5.10.y`, `5.15.y`, `6.1.y`, `6.6.y`, `6.12.y`,
`6.18.y`, `6.19.y`, and `7.0.y`.
Record: BLTP-specific IDs from this patch are not present in those
checked branches or `master`.
Record: `5.10.y`, `5.15.y`, `6.1.y`, and `6.6.y` still have the reset-
ack timeout returning `-ENODATA`, matching the commit-message failure.
`6.12.y` and newer checked branches contain `7bcf9ebb50f2a`, where
missing reset ack is downgraded to a warning, so the benefit there is
mostly avoiding a delay/warning rather than fixing probe failure.
Record: patch applies cleanly to current `stable/linux-7.0.y` when
checked with `git apply --check`. Older stable trees may need minor
context adjustment due to file layout differences, but the required
quirk mechanism exists.
### Phase 7: Subsystem And Maintainer Context
Record: subsystem is HID/i2c-hid under `drivers/hid/`, criticality
`IMPORTANT` for users of affected laptop touchpad hardware.
Record: subsystem is actively maintained; `MAINTAINERS` lists HID core
as maintained by Jiri Kosina and Benjamin Tissoires.
### Phase 8: Impact And Risk
Record: affected population is hardware-specific: systems with an I2C
HID device reporting the BLTP7853 IDs added by this patch.
Record: trigger condition is probe/reprobe after reboot or reprobe, as
described by the commit message and matched to the older stable reset
path.
Record: failure severity is HIGH for older stable branches because the
touchpad can fail to bind at all. For newer branches with
`7bcf9ebb50f2a`, severity is LOW-MEDIUM because generic handling avoids
probe failure but still waits/logs.
Record: benefit is high for older stable users with this hardware; risk
is very low because the change is a narrow hardware quirk table entry.
### Phase 9: Final Synthesis
Evidence for backporting: real device probe failure, existing quirk
mechanism, tiny hardware-specific change, no new API, maintainer sign-
off, and older stable branches demonstrably still have the failing reset
timeout behavior.
Evidence against backporting: no direct candidate lore thread or
external bug report could be verified; exact hardware ID `0x36b6/0xc001`
was only verified from the supplied patch, not an external hardware
database. Newer stable branches already have generic missing-reset-ack
handling, reducing urgency there.
Stable rules: obviously correct by inspection, fixes a real hardware
probe failure on older stable branches, small and contained, no new
feature/API, and falls under the accepted hardware quirk exception.
Backport difficulty should be clean or minor.
## Verification
- [Phase 1] Parsed supplied message and tags: only Xu Rao and Jiri
Kosina Signed-off-by tags present.
- [Phase 2] Read `drivers/hid/i2c-hid/i2c-hid-core.c`: verified quirk
table and reset handling.
- [Phase 3] Searched local histories without `--all`: exact candidate
not found; related no-IRQ quirk commits found.
- [Phase 3] Inspected `402946a8ef71e`, `fc6a31b007393`, and
`7bcf9ebb50f2a`: confirmed same reset-ack quirk mechanism and later
generic warning behavior.
- [Phase 4] Ran `b4 dig` for related commits; candidate `b4 dig` was not
possible without a candidate hash.
- [Phase 5] Traced callers with `rg` and read ACPI/OF probe files:
confirmed probe-time reachability.
- [Phase 6] Checked active stable branches: quirk support exists in all
checked branches; old reset failure path exists through `6.6.y`;
`6.12.y+` has generic warning handling.
- [Phase 6] Ran `git apply --check` against current `7.0.y`: patch shape
applies cleanly.
- UNVERIFIED: original candidate lore discussion, independent bug
report, and exact external hardware database confirmation for
`0x36b6/0xc001`.
**YES**
drivers/hid/hid-ids.h | 3 +++
drivers/hid/i2c-hid/i2c-hid-core.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index c371721826dcc..559040e47f3c7 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -277,6 +277,9 @@
#define USB_VENDOR_ID_BIGBEN 0x146b
#define USB_DEVICE_ID_BIGBEN_PS3OFMINIPAD 0x0902
+#define I2C_VENDOR_ID_BLTP 0x36b6
+#define I2C_PRODUCT_ID_BLTP7853 0xc001
+
#define USB_VENDOR_ID_BTC 0x046e
#define USB_DEVICE_ID_BTC_EMPREX_REMOTE 0x5578
#define USB_DEVICE_ID_BTC_EMPREX_REMOTE_2 0x5577
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c
index 5a183af3d5c6a..baff2728603ec 100644
--- a/drivers/hid/i2c-hid/i2c-hid-core.c
+++ b/drivers/hid/i2c-hid/i2c-hid-core.c
@@ -149,6 +149,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_BOGUS_IRQ },
{ I2C_VENDOR_ID_GOODIX, I2C_DEVICE_ID_GOODIX_0D42,
I2C_HID_QUIRK_DELAY_WAKEUP_AFTER_RESUME },
+ { I2C_VENDOR_ID_BLTP, I2C_PRODUCT_ID_BLTP7853,
+ I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
{ 0, 0 }
};
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.10] HID: elan: Add support for ELAN SB974D touchpad
From: Sasha Levin @ 2026-05-20 11:19 UTC (permalink / raw)
To: patches, stable
Cc: Damien Dejean, Kornel Dulęba, Jiri Kosina, Sasha Levin,
jikos, bentiss, linux-input, linux-kernel
In-Reply-To: <20260520111944.3424570-1-sashal@kernel.org>
From: Damien Dejean <damiendejean@google.com>
[ Upstream commit 55ce1858848132ed074fe907f00b5ce1ccab0ce1 ]
Elan SB974D touchpad uses ELAN_MT_I2C format to send HID reports. Add an
entry to match for the device and parse its vendor specific format.
Signed-off-by: Damien Dejean <damiendejean@google.com>
Signed-off-by: Kornel Dulęba <korneld@google.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit Message Forensics
Step 1.1 Record: subsystem `HID: elan`; action verb `Add support`;
intent is to add ELAN SB974D touchpad matching so the existing ELAN HID
driver parses its vendor-specific I2C multitouch reports.
Step 1.2 Record: tags are:
- `Signed-off-by: Damien Dejean <damiendejean@google.com>`
- `Signed-off-by: Kornel Dulęba <korneld@google.com>`
- `Signed-off-by: Jiri Kosina <jkosina@suse.com>`
No `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-by:`,
`Cc: stable`, or `Link:` tags are present.
Step 1.3 Record: the body says SB974D uses `ELAN_MT_I2C` report format
and needs an ID table entry so `hid-elan` handles and parses that
format. Symptom is unsupported or generic handling of this touchpad, not
a crash. No affected-version statement.
Step 1.4 Record: this is not a hidden memory/race/crash fix. It is a
hardware enablement/device-ID addition to an existing driver, which is a
stable exception category.
## Phase 2: Diff Analysis
Step 2.1 Record: two files changed, 2 insertions total:
- `drivers/hid/hid-elan.c`: adds one `HID_I2C_DEVICE()` entry to
`elan_devices`.
- `drivers/hid/hid-ids.h`: adds `USB_DEVICE_ID_SB974D 0x0400`.
Modified data structure: `elan_devices[]`. Scope: single-driver,
surgical ID addition.
Step 2.2 Record: before, ELAN I2C product `0x0400` did not match `hid-
elan`; after, BUS_I2C vendor `0x04f3` product `0x0400` matches `hid-
elan` and uses existing probe/raw-event paths.
Step 2.3 Record: bug category is hardware support/device ID addition. No
new parser, locking, memory management, or API change. The fix works by
routing this specific I2C HID device to already-existing `ELAN_MT_I2C`
handling.
Step 2.4 Record: fix quality is high: two-line addition, localized,
mirrors the existing Toshiba I2C ELAN entry, and has very low regression
risk. Main possible risk is binding product `0x0400` to `hid-elan`, but
the commit message and external fwupd discussion verify `04f3:0400` is
an ELAN touchpad PID.
## Phase 3: Git History Investigation
Step 3.1 Record: `git blame` shows `elan_devices[]` was introduced by
`9a6a4193d65b` in v4.17-rc1 range; the existing I2C Toshiba entry was
introduced by `e7ad3dc9f4a2` in v4.19-rc1 range. This means the required
driver and I2C-format support are old and present in long-term stable
lines.
Step 3.2 Record: no `Fixes:` tag, so no introducing bug commit to
follow.
Step 3.3 Record: recent history shows this commit is standalone; only
later related file change found was unrelated `HID: i2c-hid: add reset
quirk for BLTP7853 touchpad`.
Step 3.4 Record: author Damien Dejean has only this HID commit in the
checked `origin/master` HID history. Jiri Kosina committed/applied it;
`MAINTAINERS` lists Jiri Kosina and Benjamin Tissoires as HID core
maintainers.
Step 3.5 Record: no prerequisite commits found beyond the existing `hid-
elan` driver and I2C parser, both already present in checked stable
branches.
## Phase 4: Mailing List And External Research
Step 4.1 Record: `b4 dig -c 55ce185884813` found the original patch at `
https://patch.msgid.link/20260414133858.3992799-1-
damiendejean@google.com`. `b4 dig -a` showed only v1, no later
revisions.
Step 4.2 Record: `b4 dig -w` showed recipients included Jiri Kosina,
Benjamin Tissoires, `linux-input@vger.kernel.org`, `linux-
kernel@vger.kernel.org`, Damien Dejean, and Kornel Dulęba.
Step 4.3 Record: no `Reported-by` or bug-report `Link:`. The lore thread
mbox contains Jiri Kosina’s “Applied to hid.git#for-7.1/upstream-fixes,
thanks.” No NAKs or concerns in the saved thread.
Step 4.4 Record: no multi-patch series; standalone patch.
Step 4.5 Record: lore WebFetch hit Anubis, but `b4` successfully fetched
the mbox. Stable branch grep found no existing `SB974D` backport in
`stable/linux-7.0.y` or `stable/linux-6.19.y`.
## Phase 5: Code Semantic Analysis
Step 5.1 Record: modified table is `elan_devices[]`; functional paths
affected are existing callbacks `elan_probe`, `elan_input_mapping`,
`elan_input_configured`, and `elan_raw_event`.
Step 5.2 Record: `hid_match_device()` matches `hdrv->id_table`;
`hid_device_probe()` calls the matched driver’s `.probe`;
`hid_input_report()` calls `.raw_event`. Thus the new ID makes this
hardware reach existing `hid-elan` probe and report parsing.
Step 5.3 Record: relevant callees include `hid_parse`, `hid_hw_start`,
`elan_start_multitouch`, and for I2C reports `elan_i2c_report_input`.
Step 5.4 Record: reachability is hardware enumeration and HID input
report delivery. Users of the SB974D touchpad hit this during device
probe and normal touch input.
Step 5.5 Record: similar pattern exists in the same driver for
`USB_DEVICE_ID_TOSHIBA_CLICK_L9W`, also using `HID_I2C_DEVICE()` and the
existing `ELAN_MT_I2C` path.
## Phase 6: Stable Tree Analysis
Step 6.1 Record: checked stable branches `5.10.y`, `5.15.y`, `6.1.y`,
`6.6.y`, `6.12.y`, `6.18.y`, `6.19.y`, and `7.0.y`; all contain
`CONFIG_HID_ELAN`, `drivers/hid/hid-elan.c`, and the existing
`TOSHIBA_CLICK_L9W` I2C ELAN entry.
Step 6.2 Record: `git apply --check` succeeded on current `7.0.y`,
`6.19.y`, and representative older stable worktrees `5.10.y`, `5.15.y`,
`6.1.y`, `6.6.y`, `6.12.y`, `6.18.y`.
Step 6.3 Record: no related `SB974D` fix already present in checked
stable branches.
## Phase 7: Subsystem Context
Step 7.1 Record: subsystem is HID input driver support. Criticality is
driver-specific/peripheral, affecting users of this ELAN SB974D touchpad
hardware.
Step 7.2 Record: HID is actively maintained; this patch was applied
through `hid.git#for-7.1/upstream-fixes` by HID maintainer Jiri Kosina.
## Phase 8: Impact And Risk
Step 8.1 Record: affected population is hardware-specific: systems with
ELAN SB974D I2C touchpad and `CONFIG_HID_ELAN`.
Step 8.2 Record: trigger is device enumeration and normal touchpad input
on that hardware. No evidence found that unprivileged users can trigger
a security issue; this is hardware functionality.
Step 8.3 Record: failure mode without the patch is lack of proper `hid-
elan` handling for this touchpad’s vendor-specific `ELAN_MT_I2C` report
format. Severity is medium for affected hardware because touchpad
functionality is user-visible, but not a crash/corruption/security
issue.
Step 8.4 Record: benefit is high for affected hardware and low globally.
Risk is very low: two lines, no behavior change for unrelated device
IDs, no API change, and clean apply to checked stable branches.
## Phase 9: Final Synthesis
Evidence for backporting: this is exactly the stable exception for
adding a new device ID to an existing driver; the driver and parser
already exist in stable; the patch is tiny, cleanly applies, and was
accepted by the HID maintainer. Evidence against: it is hardware
enablement rather than a conventional critical bug fix, and there is no
user bug report or `Tested-by` tag in the commit.
Stable rules:
1. Obviously correct and tested? Code is obviously correct by
inspection; explicit testing tag absent.
2. Fixes a real user-visible issue? Yes for affected SB974D systems:
proper touchpad driver binding/report parsing.
3. Important issue? Medium hardware functionality issue, not
crash/corruption/security.
4. Small and contained? Yes, 2 insertions in HID.
5. No new APIs/features? No API; hardware ID addition only.
6. Can apply to stable? Yes, verified across representative active
stable branches.
Exception category: new device ID added to an existing driver. This is
stable-suitable.
## Verification
- Phase 1: `git show --format=fuller 55ce185884813` verified subject,
body, author, committer, and tags.
- Phase 2: `git show --stat` and full diff verified exactly 2 insertions
in `hid-elan.c` and `hid-ids.h`.
- Phase 3: `git blame` verified table history; `git describe --contains`
placed original driver support around v4.17-rc1 and I2C support around
v4.19-rc1.
- Phase 3: `git log --oneline` checks found no dependent SB974D series
and only this author’s HID commit in the checked range.
- Phase 4: `b4 dig`, `b4 dig -a`, and `b4 dig -w` verified original lore
submission, single v1, recipients, and maintainer application.
- Phase 4: WebFetch to lore/stable was blocked by Anubis; the saved `b4`
mbox was used instead.
- Phase 5: `ReadFile` and `rg` verified `elan_probe`, `elan_raw_event`,
`elan_i2c_report_input`, `hid_match_device`, and `hid_input_report`
call paths.
- Phase 6: `git grep` verified relevant stable branches contain
`HID_ELAN` and the existing ELAN I2C entry.
- Phase 6: `git apply --check` verified clean application to `7.0.y`,
`6.19.y`, `6.18.y`, `6.12.y`, `6.6.y`, `6.1.y`, `5.15.y`, and
`5.10.y`.
- Phase 7: `MAINTAINERS` verified Jiri Kosina and Benjamin Tissoires as
HID maintainers.
- Phase 8: no sanitizer report, crash report, or security impact was
found; decision is driven by the verified device-ID stable exception.
**YES**
drivers/hid/hid-elan.c | 1 +
drivers/hid/hid-ids.h | 1 +
2 files changed, 2 insertions(+)
diff --git a/drivers/hid/hid-elan.c b/drivers/hid/hid-elan.c
index 76d93fc48f6a2..0190ad567ce4d 100644
--- a/drivers/hid/hid-elan.c
+++ b/drivers/hid/hid-elan.c
@@ -513,6 +513,7 @@ static const struct hid_device_id elan_devices[] = {
{ HID_USB_DEVICE(USB_VENDOR_ID_ELAN, USB_DEVICE_ID_HP_X2_10_COVER),
.driver_data = ELAN_HAS_LED },
{ HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, USB_DEVICE_ID_TOSHIBA_CLICK_L9W) },
+ { HID_I2C_DEVICE(USB_VENDOR_ID_ELAN, USB_DEVICE_ID_SB974D) },
{ }
};
MODULE_DEVICE_TABLE(hid, elan_devices);
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index c1e4a6ce96317..c371721826dcc 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -455,6 +455,7 @@
#define USB_DEVICE_ID_EDIFIER_QR30 0xa101 /* EDIFIER Hal0 2.0 SE */
#define USB_VENDOR_ID_ELAN 0x04f3
+#define USB_DEVICE_ID_SB974D 0x0400
#define USB_DEVICE_ID_TOSHIBA_CLICK_L9W 0x0401
#define USB_DEVICE_ID_HP_X2 0x074d
#define USB_DEVICE_ID_HP_X2_10_COVER 0x0755
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.10] HID: mcp2221: fix OOB write in mcp2221_raw_event()
From: Sasha Levin @ 2026-05-20 11:18 UTC (permalink / raw)
To: patches, stable
Cc: Florian Pradines, Benoît Sevens, Jiri Kosina, Sasha Levin,
gupt21, jikos, bentiss, linux-i2c, linux-input, linux-kernel
In-Reply-To: <20260520111944.3424570-1-sashal@kernel.org>
From: Florian Pradines <florian.pradines@gmail.com>
[ Upstream commit f097d246677b03db814c5862f368cea341b76a00 ]
mcp2221_raw_event() copies device-supplied data into mcp->rxbuf at
offset rxbuf_idx without checking that the copy fits within the
destination buffer. A device responding with up to 60 bytes to a
small I2C/SMBus read can overflow the buffer.
Add a rxbuf_size field to struct mcp2221, set it alongside rxbuf in
mcp_i2c_smbus_read(), and check rxbuf_idx + data[3] <= rxbuf_size
before the memcpy.
Reported-by: Benoît Sevens <bsevens@google.com>
Signed-off-by: Florian Pradines <florian.pradines@gmail.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit Message Forensics
Step 1.1 Record: subsystem `HID: mcp2221`; action verb `fix`; claimed
intent: prevent an out-of-bounds write in `mcp2221_raw_event()` when
copying device-supplied I2C read data into `mcp->rxbuf`.
Step 1.2 Record: tags found in upstream commit
`f097d246677b03db814c5862f368cea341b76a00`: `Reported-by: Benoît Sevens
<bsevens@google.com>`, `Signed-off-by: Florian Pradines
<florian.pradines@gmail.com>`, `Signed-off-by: Jiri Kosina
<jkosina@suse.com>`. No `Fixes:`, no `Cc: stable`, no `Link:` in the
commit message.
Step 1.3 Record: bug description is explicit: `mcp2221_raw_event()`
copies `data[3]` bytes into `mcp->rxbuf + rxbuf_idx` without checking
the destination buffer size. Failure mode is kernel memory corruption
via OOB write when a device returns up to 60 bytes for a smaller
requested read.
Step 1.4 Record: not hidden; this is a direct memory-safety fix.
## Phase 2: Diff Analysis
Step 2.1 Record: one file changed, `drivers/hid/hid-mcp2221.c`, 7
insertions. Modified areas: `struct mcp2221`, `mcp_i2c_smbus_read()`,
`mcp2221_raw_event()`. Scope: single-file surgical fix.
Step 2.2 Record: before, the driver tracked `rxbuf` and `rxbuf_idx` but
not the destination size. After, it stores `rxbuf_size` when setting
`rxbuf`, and rejects `rxbuf_idx + data[3] > rxbuf_size` before
`memcpy()`.
Step 2.3 Record: bug category is memory safety, specifically OOB write
from device-controlled length and cumulative receive offset. The earlier
`data[3] > 60` check only capped a single MCP2221 report, not the
destination buffer size.
Step 2.4 Record: fix is minimal and locally correct for the upstream
parent, but direct stable backports need care because older stable
branches still have two manual `mcp_smbus_xfer()` paths that set
`mcp->rxbuf` directly.
## Phase 3: Git History
Step 3.1 Record: `git blame` showed `rxbuf`, `rxbuf_idx`,
`mcp_i2c_smbus_read()`, and the `memcpy()` path originated in
`67a95c21463d0`, first contained in `v5.7-rc1`. The partial-read
handling came from `2682468671aa0`, first contained in `v6.8-rc1`. The
prior per-report `data[3] > 60` guard came from `b56cc41a3ae73`, first
contained in `v6.17-rc4` and backported to stable equivalents.
Step 3.2 Record: no `Fixes:` tag, so no tagged introducing commit to
follow.
Step 3.3 Record: recent related commits include `b56cc41a3ae73` for the
60-byte report cap, `e31b556c0ba21` for read-error cancellation, and
`ab05515757fcb` for using `mcp_i2c_smbus_read()` for block reads.
`ab05515757fcb` is an important context dependency for the exact
upstream diff.
Step 3.4 Record: author Florian Pradines had no other local commits
found in `drivers/hid`; Jiri Kosina, the HID maintainer,
committed/applied the patch.
Step 3.5 Record: upstream parent contains `ab05515757fcb`; stable
branches checked from `5.10.y` through `7.0.y` do not. A stable backport
should either adapt the patch to set `rxbuf_size` in the two remaining
manual `mcp_smbus_xfer()` paths or include an appropriate
prerequisite/backport.
## Phase 4: Mailing List / External Research
Step 4.1 Record: `b4 dig -c f097d246677b03db814c5862f368cea341b76a00`
found the lore thread: `https://patch.msgid.link/20260509094517.2691246-
1-florian.pradines@gmail.com`. `b4 dig -a` showed only v1.
Step 4.2 Record: `b4 dig -w` showed recipients included Florian
Pradines, Rishi Gupta, Jiri Kosina, Benjamin Tissoires, `linux-i2c`, and
`linux-input`.
Step 4.3 Record: reporter Benoît Sevens previously submitted a similar
heap-buffer-overflow patch. Jiri replied that Benoît’s patch was mangled
and that Florian’s patch would be taken with Benoît added as `Reported-
by`. Benoît then noted the missing `mcp_smbus_xfer()` assignments, which
is relevant for older stable bases.
Step 4.4 Record: not part of a multi-patch series. Related patches exist
for report-size validation, but Jiri questioned the “MCP2221 always
sends 64-byte reports” assumption in that separate thread.
Step 4.5 Record: direct lore/stable WebFetch was blocked by Anubis, but
web search found related stable discussion for prior mcp2221 fixes, not
an exact stable request for `f097d246677b`.
## Phase 5: Semantic Analysis
Step 5.1 Record: changed function is `mcp_i2c_smbus_read()`; changed
callback is `mcp2221_raw_event()`.
Step 5.2 Record: callers of `mcp_i2c_smbus_read()` are `mcp_i2c_xfer()`
and `mcp_smbus_xfer()`, wired into `mcp_i2c_algo`. `mcp2221_raw_event()`
is called from HID core before normal raw report processing, via
`hid_input_report()` / `__hid_input_report()`.
Step 5.3 Record: affected callees include `mcp_send_data_req_status()`,
`mcp_send_report()`, `wait_for_completion_timeout()`, and the vulnerable
`memcpy()`.
Step 5.4 Record: reachability is real: USB HID interrupt input reaches
`mcp2221_raw_event()`, and I2C/SMBus transfers reach
`mcp_i2c_smbus_read()` through the registered I2C adapter.
Triggerability by an unprivileged local user was not verified; trigger
by a malicious or buggy MCP2221-side device response is verified by the
code path.
Step 5.5 Record: similar pattern found in stable branches: two manual
`mcp_smbus_xfer()` receive-buffer assignments remain in stable branches
and must be handled in backports.
## Phase 6: Stable Tree Analysis
Step 6.1 Record: the driver was introduced in `v5.7-rc1`; local stable
refs `5.10.y`, `5.15.y`, `6.1.y`, `6.6.y`, `6.12.y`, `6.18.y`, `6.19.y`,
and `7.0.y` contain the vulnerable `rxbuf`/`memcpy()` pattern. `5.4.y`
does not contain the driver.
Step 6.2 Record: `git apply --check` against current
`stable/linux-7.0.y` succeeds syntactically, but semantic backport work
is needed because stable lacks `ab05515757fcb`.
Step 6.3 Record: prior stable branches already contain the earlier
60-byte cap fix or its backported equivalent, but that does not fix
cumulative overflow into smaller destination buffers.
## Phase 7: Subsystem Context
Step 7.1 Record: subsystem is HID driver for the Microchip MCP2221 USB-
to-I2C/SMBus bridge. Criticality: driver-specific, but memory corruption
from USB/HID device input is high severity for affected systems.
Step 7.2 Record: file history shows active recent maintenance and
multiple security/correctness fixes in this driver.
## Phase 8: Impact And Risk
Step 8.1 Record: affected population is users of MCP2221 hardware or
systems exposed to such USB HID devices.
Step 8.2 Record: trigger is an I2C/SMBus read where the MCP2221-side
response reports more bytes than the destination buffer can hold.
Physical/malicious-device trigger is verified; unprivileged local
trigger depends on device-node permissions and was not verified.
Step 8.3 Record: failure mode is OOB write into kernel memory, severity
HIGH to CRITICAL due to memory corruption/crash/security risk.
Step 8.4 Record: benefit is high. Risk is low for the upstream parent
and for correctly adapted stable backports. Risk is medium for blind
cherry-picks into older stable branches because `rxbuf_size` would not
be set in all stable receive-buffer paths.
## Phase 9: Final Synthesis
Evidence for backporting: real OOB write, explicit reporter, small
patch, maintainer-applied, affects stable-supported code since `5.10.y`,
fixes a memory-corruption class bug.
Evidence against or concerns: no `Fixes:`/`Cc: stable`; exact upstream
diff assumes `ab05515757fcb`, absent from checked stable branches; blind
cherry-pick is not sufficient for older stable trees.
Stable rules checklist: obviously correct on its upstream base: yes.
Fixes a real user-affecting bug: yes. Important issue: yes, kernel
memory corruption. Small and contained: yes, 7 insertions. No new
feature/API: yes. Applies to stable: syntactically yes on `7.0.y`, but
older stable needs a branch-specific backport adjustment.
Exception category: none; this is a memory-safety fix, not a device
ID/quirk/doc/build exception.
## Verification
- Phase 1: fetched and inspected upstream commit
`f097d246677b03db814c5862f368cea341b76a00`.
- Phase 2: verified exact diff is 7 insertions in `drivers/hid/hid-
mcp2221.c`.
- Phase 3: ran `git blame`, `git describe --contains`, and targeted `git
log` without `--all`; identified `67a95c21463d0`, `2682468671aa0`,
`b56cc41a3ae73`, and `ab05515757fcb`.
- Phase 4: ran `b4 dig -c`, `b4 dig -a`, `b4 dig -w`, and fetched mboxes
with `b4 mbox`; verified Jiri’s “Applied, thanks” and Benoît’s earlier
report/backport-relevant concern.
- Phase 5: used code search and file reads to verify HID and I2C/SMBus
call paths.
- Phase 6: used `git grep` against stable refs to verify vulnerable
patterns in `5.10.y` through `7.0.y`, and absence from `5.4.y`.
- Phase 6: verified exact patch syntactically applies to current
`stable/linux-7.0.y`, but `ab05515757fcb` is not an ancestor of
checked stable refs.
- Phase 8: severity is based on verified device-controlled `memcpy()`
into kernel buffer without destination-size check.
- UNVERIFIED: no runtime reproducer was executed; no build test was run;
unprivileged local triggerability was not verified.
This should be backported, but stable maintainers should not blindly
cherry-pick it into pre-`ab05515757fcb` trees without adding
`rxbuf_size` initialization to every remaining `mcp->rxbuf` assignment.
**YES**
drivers/hid/hid-mcp2221.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/hid/hid-mcp2221.c b/drivers/hid/hid-mcp2221.c
index ef3b5c77c38e3..badd4958bc12c 100644
--- a/drivers/hid/hid-mcp2221.c
+++ b/drivers/hid/hid-mcp2221.c
@@ -121,6 +121,7 @@ struct mcp2221 {
u8 *rxbuf;
u8 txbuf[64];
int rxbuf_idx;
+ int rxbuf_size;
int status;
u8 cur_i2c_clk_div;
struct gpio_chip *gc;
@@ -323,12 +324,14 @@ static int mcp_i2c_smbus_read(struct mcp2221 *mcp,
mcp->txbuf[3] = (u8)(msg->addr << 1);
total_len = msg->len;
mcp->rxbuf = msg->buf;
+ mcp->rxbuf_size = msg->len;
} else {
mcp->txbuf[1] = smbus_len;
mcp->txbuf[2] = 0;
mcp->txbuf[3] = (u8)(smbus_addr << 1);
total_len = smbus_len;
mcp->rxbuf = smbus_buf;
+ mcp->rxbuf_size = smbus_len;
}
ret = mcp_send_data_req_status(mcp, mcp->txbuf, 4);
@@ -912,6 +915,10 @@ static int mcp2221_raw_event(struct hid_device *hdev,
mcp->status = -EINVAL;
break;
}
+ if (mcp->rxbuf_idx + data[3] > mcp->rxbuf_size) {
+ mcp->status = -EINVAL;
+ break;
+ }
buf = mcp->rxbuf;
memcpy(&buf[mcp->rxbuf_idx], &data[4], data[3]);
mcp->rxbuf_idx = mcp->rxbuf_idx + data[3];
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.10] HID: magicmouse: Prevent out-of-bounds (OOB) read during DOUBLE_REPORT_ID
From: Sasha Levin @ 2026-05-20 11:18 UTC (permalink / raw)
To: patches, stable
Cc: Lee Jones, Günther Noack, Jiri Kosina, Sasha Levin, jikos,
bentiss, linux-input, linux-kernel
In-Reply-To: <20260520111944.3424570-1-sashal@kernel.org>
From: Lee Jones <lee@kernel.org>
[ Upstream commit d93ba918a185aca2594da63e92fdc5495b559c0f ]
It is currently possible for a malicious or misconfigured USB device to
cause an out-of-bounds (OOB) read when submitting reports using
DOUBLE_REPORT_ID by specifying a large report length and providing a
smaller one.
Let's prevent that by comparing the specified report length with the
actual size of the data read in from userspace. If the actual data
length ends up being smaller than specified, we'll politely warn the
user and prevent any further processing.
Signed-off-by: Lee Jones <lee@kernel.org>
Reviewed-by: Günther Noack <gnoack@google.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase 1: Commit Message Forensics
Step 1.1 Record: Subsystem `HID: magicmouse`; action verb `Prevent`;
claimed intent is preventing an out-of-bounds read while parsing
`DOUBLE_REPORT_ID`.
Step 1.2 Record: Tags present: `Signed-off-by: Lee Jones
<lee@kernel.org>`, `Reviewed-by: Günther Noack <gnoack@google.com>`,
`Signed-off-by: Jiri Kosina <jkosina@suse.com>`. No `Fixes:`, `Reported-
by:`, `Tested-by:`, `Link:`, or `Cc: stable@vger.kernel.org` tag was
present in the supplied commit text.
Step 1.3 Record: The body describes a malformed HID report where
`data[1]` advertises a larger embedded report length than the actual
received buffer. Symptom/failure mode: OOB read during recursive
parsing. No affected kernel versions are named. Root cause:
`magicmouse_raw_event()` trusted the embedded length before checking
that the buffer actually contained that many bytes.
Step 1.4 Record: This is not a hidden cleanup; it is an explicit memory-
safety bug fix.
## Phase 2: Diff Analysis
Step 2.1 Record: One file changed: `drivers/hid/hid-magicmouse.c`, 16
insertions, no removals. Modified function: `magicmouse_raw_event()`.
Scope: single-file surgical fix.
Step 2.2 Record: First hunk adds `if (size < 1) return 0;` before
`switch (data[0])`, preventing zero-sized recursive calls from
dereferencing `data[0]`. Second hunk adds `size < 2` before reading
`data[1]`, then validates `size >= data[1] + 2` before splitting the
double report into recursive calls.
Step 2.3 Record: Bug category is memory safety / bounds validation.
Broken mechanism: the `DOUBLE_REPORT_ID` case used `data[1]` to choose
recursive buffer pointers and sizes without proving the first embedded
report existed in the received buffer.
Step 2.4 Record: The fix is obviously correct by inspection: it checks
the exact fields before use and follows existing invalid-report behavior
of returning `0`. Regression risk is very low; only malformed undersized
reports are rejected. Minor concern: the new `hid_warn()` string lacks a
trailing newline, but that is logging style, not functional risk.
## Phase 3: Git History Investigation
Step 3.1 Record: `git blame` shows `magicmouse_raw_event()` dates back
to `128537cea464` from 2010, while the `DOUBLE_REPORT_ID` recursive
parsing lines were introduced by `a462230e16acc` (`HID: magicmouse:
enable Magic Trackpad support`). `git describe --contains` places that
introduction at `v2.6.37-rc1~144^2~3^5~2`.
Step 3.2 Record: No `Fixes:` tag is present, so there was no tagged
introducing commit to follow. Blame independently identifies the
relevant old code.
Step 3.3 Record: Recent `drivers/hid/hid-magicmouse.c` history contains
other fixes such as missing input crash prevention and report-fixup leak
fixes, but no prerequisite for this bounds check was identified. This
patch is standalone.
Step 3.4 Record: `git log --author='Lee Jones' -- drivers/hid` shows Lee
Jones has multiple recent HID fixes, including OOB/UAF-related HID work.
The patch was reviewed by Günther Noack and applied by HID maintainer
Jiri Kosina in the mailing-list thread.
Step 3.5 Record: No dependency was found. The patch only uses existing
`hid_warn()` and local variables already present in
`magicmouse_raw_event()`.
## Phase 4: Mailing List And External Research
Step 4.1 Record: No commit hash was supplied and the commit object was
not found in local `git log`, so `b4 dig -c <commit>` was not
applicable. I used `b4 mbox` with the v2 Message-ID instead. Lore URL:
`https://lore.kernel.org/all/20260416131655.2279756-1-lee@kernel.org/`.
Series revisions found: v1 and v2.
Step 4.2 Record: The v2 headers show Lee sent it to Jiri Kosina,
Benjamin Tissoires, `linux-input@vger.kernel.org`, `linux-
kernel@vger.kernel.org`, and Cc’d Günther Noack. Jiri later replied
“Applied”.
Step 4.3 Record: No `Reported-by` or external bug-report link was
present. The thread itself contains the bug explanation.
Step 4.4 Record: v1 added only the `size < data[1] + 2` check. Günther
Noack objected that `size == data[1] + 2` still caused a zero-sized
second recursive call which then dereferenced `data[0]`. v2 added the
top-level `size < 1` and `DOUBLE_REPORT_ID` `size < 2` checks; Günther
replied “This looks correct now” and gave `Reviewed-by`.
Step 4.5 Record: Web search for stable-specific discussion did not find
a stable nomination or a reason not to backport.
## Phase 5: Code Semantic Analysis
Step 5.1 Record: Modified function: `magicmouse_raw_event()`.
Step 5.2 Record: Callers are HID core through the driver `.raw_event`
callback and recursive self-calls in the `DOUBLE_REPORT_ID` case. `hid-
core.c` calls `hdrv->raw_event(hid, report, data, size)` before generic
report processing.
Step 5.3 Record: Key callees in the affected path are recursive
`magicmouse_raw_event()` calls, `magicmouse_emit_touch()`, input event
reporting helpers, and `hid_warn()`. The new checks run before recursive
pointer arithmetic.
Step 5.4 Record: Reachability is verified through `hid_input_report()`,
including `usbhid` interrupt/control paths and `uhid` input paths. A
matched or emulated HID device can feed raw reports to this parser.
Unprivileged triggering was not verified and is not needed for the
decision.
Step 5.5 Record: Search in `drivers/hid` found this `DOUBLE_REPORT_ID`
split pattern only in `hid-magicmouse.c`.
## Phase 6: Stable Tree Analysis
Step 6.1 Record: The vulnerable pattern exists in `v6.6`, `v6.1`,
`v5.15`, `v5.10`, `v5.4`, and `v4.19` by `git grep`. The relevant code
was introduced around `v2.6.37-rc1`.
Step 6.2 Record: Expected backport difficulty is low: the same block and
function exist across checked stable baselines, though line offsets
differ. Exact clean application to every stable branch was not tested.
Step 6.3 Record: Local history searches for `DOUBLE_REPORT_ID` and `out-
of-bounds.*magicmouse` found no prior equivalent fix in this checkout.
## Phase 7: Subsystem Context
Step 7.1 Record: Subsystem is HID input driver support for Apple Magic
Mouse/Trackpad, under `drivers/hid`. Criticality: peripheral/driver-
specific, but memory-safety relevant.
Step 7.2 Record: HID is actively maintained; recent `drivers/hid`
history shows multiple 2026 fixes including OOB, UAF, and NULL-deref
fixes.
## Phase 8: Impact And Risk
Step 8.1 Record: Affected users are systems with `CONFIG_HID_MAGICMOUSE`
and matching Apple Magic Trackpad/Magic Mouse style HID devices or
emulated/spoofed devices reaching this driver.
Step 8.2 Record: Trigger requires a malformed `DOUBLE_REPORT_ID` report
where the embedded length does not fit the actual received size, or
where the second recursive call would be zero-sized. This is not a
normal-path trigger, but it is reachable via raw HID reports.
Step 8.3 Record: Failure mode is OOB read in kernel HID parsing.
Severity: HIGH because it is memory safety in a device-input path and
can be triggered by malformed device input; exact crash or disclosure
behavior was not independently reproduced.
Step 8.4 Record: Benefit is high for affected systems because it blocks
a concrete OOB read. Risk is very low: 16 inserted validation lines,
single function, no ABI/API change, no locking change.
## Phase 9: Final Synthesis
Step 9.1 Record: Evidence for backporting: real OOB read, old code
present in many stable baselines, small surgical validation, reviewed
after v1 caught an incomplete fix, applied by HID maintainer. Evidence
against: no `Tested-by`, no explicit stable nomination, exact clean
apply to every tree not tested. Unresolved: exact upstream commit hash
unavailable locally; exact unprivileged triggerability unverified; exact
USB-vs-Bluetooth trigger wording not independently verified.
Step 9.2 Record: Stable rules: obviously correct: yes, by code
inspection and review; tested: no explicit `Tested-by`. Fixes real bug:
yes, OOB read. Important issue: yes, memory safety. Small and contained:
yes, one function, 16 insertions. No new features/APIs: yes. Can apply
to stable: likely low difficulty; exact apply unverified.
Step 9.3 Record: No exception category applies; this is a normal memory-
safety bug fix, not a device ID, quirk, DT, build, or documentation
patch.
Step 9.4 Record: The technical merit strongly favors backporting. The
fix is narrow, old affected code is present across stable trees, and the
mailing-list review specifically corrected the subtle recursive zero-
size case.
## Verification
- Phase 1: Parsed supplied commit message and tags.
- Phase 2: Compared supplied diff with local `drivers/hid/hid-
magicmouse.c` context.
- Phase 3: Ran `git blame -L 386,496 -- drivers/hid/hid-magicmouse.c`;
identified `a462230e16acc` as the `DOUBLE_REPORT_ID` recursive parsing
introduction.
- Phase 3: Ran `git describe --contains --match 'v[0-9]*'
a462230e16acc`; found `v2.6.37-rc1~144^2~3^5~2`.
- Phase 4: Used `b4 mbox 20260416131655.2279756-1-lee@kernel.org`; found
v2 thread, reviewer approval, and Jiri’s “Applied” reply.
- Phase 4: Used `b4 mbox ad-S8Riah-f6mEWm@google.com`; found v1 review
pointing out the remaining zero-sized recursive-call bug.
- Phase 5: Used `rg` and read `hid-core.c`; verified `.raw_event`
dispatch and `hid_input_report()` reachability.
- Phase 6: Used `git grep` on `v6.6`, `v6.1`, `v5.15`, `v5.10`, `v5.4`,
and `v4.19`; confirmed vulnerable code exists.
- Phase 7: Checked `drivers/hid/Kconfig`; confirmed
`CONFIG_HID_MAGICMOUSE`.
- Phase 8: Severity assessment is based on verified OOB read mechanics;
exact crash/leak manifestation is unverified.
- UNVERIFIED: Exact commit hash in local repository; exact clean apply
to every stable branch; unprivileged triggerability.
**YES**
drivers/hid/hid-magicmouse.c | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/drivers/hid/hid-magicmouse.c b/drivers/hid/hid-magicmouse.c
index 9eadf3252d0dc..f53839484c861 100644
--- a/drivers/hid/hid-magicmouse.c
+++ b/drivers/hid/hid-magicmouse.c
@@ -390,6 +390,10 @@ static int magicmouse_raw_event(struct hid_device *hdev,
struct input_dev *input = msc->input;
int x = 0, y = 0, ii, clicks = 0, npoints;
+ /* Protect against zero sized recursive calls from DOUBLE_REPORT_ID */
+ if (size < 1)
+ return 0;
+
switch (data[0]) {
case TRACKPAD_REPORT_ID:
case TRACKPAD2_BT_REPORT_ID:
@@ -490,6 +494,18 @@ static int magicmouse_raw_event(struct hid_device *hdev,
/* Sometimes the trackpad sends two touch reports in one
* packet.
*/
+
+ /* Ensure that we have at least 2 elements (report type and size) */
+ if (size < 2)
+ return 0;
+
+ if (size < data[1] + 2) {
+ hid_warn(hdev,
+ "received report length (%d) was smaller than specified (%d)",
+ size, data[1] + 2);
+ return 0;
+ }
+
magicmouse_raw_event(hdev, report, data + 2, data[1]);
magicmouse_raw_event(hdev, report, data + 2 + data[1],
size - 2 - data[1]);
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-6.12] HID: logitech-hidpp: Add support for newer Bluetooth keyboards
From: Sasha Levin @ 2026-05-20 11:18 UTC (permalink / raw)
To: patches, stable
Cc: Alain Michaud, Olivier Gay, Jiri Kosina, Sasha Levin, jikos,
bentiss, linux-input, linux-kernel
From: Alain Michaud <alainmichaud@google.com>
[ Upstream commit 64ffa2e5e02ff54b23221d0282155f37283fabea ]
Add product IDs (PIDs) for several newer Logitech Bluetooth keyboards
to the hidpp_devices matching table, enabling full HID++ support for
them.
The added keyboards are:
- Logitech Signature K650 & B2B
- Logitech Pebble Keys 2 K380S
- Logitech Casa Pop-Up Desk & B2B
- Logitech Wave Keys & B2B
- Logitech Signature Slim K950 & B2B
- Logitech MX Keys S & B2B
- Logitech Keys-To-Go 2
- Logitech Pop Icon Keys
- Logitech MX Keys Mini & B2B
- Logitech Signature Slim Solar+ K980 B2B
- Logitech Bluetooth Keyboard K250/K251
- Logitech Signature Comfort K880 & B2B
Signed-off-by: Alain Michaud <alainmichaud@google.com>
Reviewed-by: Olivier Gay <ogay@logitech.com>
Signed-off-by: Jiri Kosina <jkosina@suse.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
## Phase Walkthrough
**Phase 1 Record:** Subsystem `HID: logitech-hidpp`; action `Add`;
intent is to add Bluetooth PIDs so newer Logitech keyboards bind to
`hid-logitech-hidpp`. Tags: `Signed-off-by: Alain Michaud`, `Reviewed-
by: Olivier Gay <ogay@logitech.com>`, `Signed-off-by: Jiri Kosina`; no
`Fixes`, `Reported-by`, `Tested-by`, `Link`, or stable Cc. Body
describes hardware enablement, not crash/security. Hidden bug fix: no,
this is explicit device-ID enablement.
**Phase 2 Record:** One file changed: `drivers/hid/hid-logitech-
hidpp.c`, 38 insertions, all in `hidpp_devices[]`. Before: listed
Logitech Bluetooth devices stopped at existing IDs such as
`0xb391`/`0xb042`. After: adds 19
`HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, pid)` entries. Bug
category: hardware enablement/device IDs. Fix quality: mechanically
simple, no code flow, locking, allocation, ABI, or API changes.
**Phase 3 Record:** Upstream commit is
`64ffa2e5e02ff54b23221d0282155f37283fabea`, contained by
`v7.1-rc4~28^2`. Blame shows the new entries are from this commit;
surrounding table entries are established prior HID++ Bluetooth IDs. No
`Fixes:` tag to follow. Related history shows repeated prior Logitech
HID++ Bluetooth ID additions, plus a prior broad “match all Logitech
Bluetooth” approach that was reverted because it could bind unsupported
mice and leave them dead. This supports the narrow-ID approach. Author
has only this HID/HIDPP commit locally; reviewer/maintainer path is
through HID maintainers.
**Phase 4 Record:** `b4 dig -c 64ffa2e5e02f` found the original
submission at `https://patch.msgid.link/20260512132244.2194556-1-
alainmichaud@google.com`. `b4 dig -a` showed only v1. `b4 dig -w` showed
Jiri Kosina, Benjamin Tissoires, HID++/Logitech reviewers, `linux-
input`, and `linux-kernel` were included. Saved mbox shows Jiri replied
“Applied, thanks.” No objections or stable nomination found in that
mbox. WebFetch to lore/stable was blocked by Anubis, so stable-list
search could not be verified externally.
**Phase 5 Record:** Key changed object is `hidpp_devices[]`. It is
assigned to `hidpp_driver.id_table`, exported via `MODULE_DEVICE_TABLE`,
and registered by `module_hid_driver(hidpp_driver)`. HID core uses
`hid_match_device()` -> `hid_match_id()` against `hdrv->id_table`,
matching bus/vendor/product. `HID_BLUETOOTH_DEVICE` expands to
`BUS_BLUETOOTH` plus vendor/product. Reachability: Bluetooth HID device
enumeration/driver matching. Similar pattern: many existing Logitech
Bluetooth HID++ IDs in the same table.
**Phase 6 Record:** Checked tags `v5.10`, `v5.15`, `v6.1`, `v6.6`,
`v6.12`, `v6.18`, `v6.19`, and `v7.0`: all have `hidpp_devices[]`,
`MODULE_DEVICE_TABLE(hid, hidpp_devices)`, and `HID_BLUETOOTH_DEVICE`;
all lack the candidate PIDs. Patch applies cleanly to current
`stable/linux-7.0.y`; older trees likely need trivial context placement
because some newer adjacent IDs are absent.
**Phase 7 Record:** Subsystem is HID Logitech HID++ driver, under
maintained HID/Input. Criticality: driver-specific, but for affected
Logitech keyboard users it controls full HID++ support. Subsystem is
active; recent history includes HIDPP fixes and other device-ID
additions.
**Phase 8 Record:** Affected users are owners of the listed Logitech
Bluetooth keyboards on stable kernels. Trigger is pairing/using those
devices over Bluetooth. Failure mode is missing enhanced HID++ driver
support, not a crash; severity medium for affected hardware
functionality. Benefit is high for those users and low system-wide risk.
Risk is very low: data-only ID table entries, no behavioral change for
non-matching devices.
**Phase 9 Record:** Evidence for backporting: classic stable exception
for adding device IDs to an existing driver; small and contained;
reviewed by Logitech contact; applied by HID maintainer; code exists in
active stable trees; prior history favors explicit IDs over broad
matching. Evidence against: not a crash/security/data-corruption fix;
product ID correctness is verified only by commit review, not an
external public spec. Stable checklist: obviously correct yes; real
user-visible hardware support issue yes; important under device-ID
exception yes; small/contained yes; no new API yes; applies cleanly to
7.0 and should be trivial elsewhere.
## Verification
- [Phase 1] Parsed commit metadata with `git show --format=fuller`:
confirmed tags and no `Fixes`/`Reported-by`.
- [Phase 2] Inspected full diff: 38 insertions only in
`hidpp_devices[]`.
- [Phase 3] Used `git blame` around the table and `git log` on the file:
confirmed surrounding HID++ Bluetooth ID history and related revert of
broad Bluetooth matching.
- [Phase 4] Used `b4 dig -c/-a/-w` and saved/read mbox: confirmed v1
thread, recipients, and maintainer “Applied, thanks.”
- [Phase 5] Used code search/read of `hid-core.c`,
`include/linux/hid.h`, and `hid-logitech-hidpp.c`: confirmed driver
table matching path.
- [Phase 6] Checked stable tags for table/macro/PID presence and ran
`git apply --check` on current 7.0 tree.
- [Phase 7] Checked `MAINTAINERS` and recent HID/HIDPP logs.
- [Phase 8] Impact/risk derived from verified diff and HID matching
path.
- UNVERIFIED: lore stable-search results, because WebFetch was blocked
by Anubis.
- UNVERIFIED: independent public confirmation of every Logitech PID
beyond the patch author/reviewer/maintainer record.
This should be backported under the stable device-ID exception. It
enables existing HID++ driver support for specific hardware with minimal
regression risk.
**YES**
drivers/hid/hid-logitech-hidpp.c | 38 ++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c
index d1dea7297712d..68f055ef1f444 100644
--- a/drivers/hid/hid-logitech-hidpp.c
+++ b/drivers/hid/hid-logitech-hidpp.c
@@ -4672,6 +4672,44 @@ static const struct hid_device_id hidpp_devices[] = {
HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb391) },
{ /* MX Master 4 mouse over Bluetooth */
HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb042) },
+ { /* Logitech Signature K650 over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb36f) },
+ { /* Logitech Signature K650 B2B over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb370) },
+ { /* Logitech Pebble Keys 2 K380S over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb377) },
+ { /* Logitech Casa Pop-Up Desk over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb371) },
+ { /* Logitech Casa Pop-Up Desk B2B over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb374) },
+ { /* Logitech Wave Keys over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb383) },
+ { /* Logitech Wave Keys B2B over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb384) },
+ { /* Logitech Signature Slim K950 over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb386) },
+ { /* Logitech Signature Slim K950 B2B over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb388) },
+ { /* Logitech MX Keys S over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb378) },
+ { /* Logitech MX Keys S B2B over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb380) },
+ { /* Logitech Keys-To-Go 2 over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb38c) },
+ { /* Logitech Pop Icon Keys over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb38f) },
+ { /* Logitech MX Keys Mini over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb369) },
+ { /* Logitech MX Keys Mini B2B over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb36e) },
+ { /* Logitech Signature Slim Solar+ K980 B2B over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb394) },
+ { /* Logitech Bluetooth Keyboard K250/K251 over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb397) },
+ { /* Logitech Signature Comfort K880 over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb39c) },
+ { /* Logitech Signature Comfort K880 B2B over Bluetooth */
+ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_LOGITECH, 0xb39d) },
{}
};
--
2.53.0
^ permalink raw reply related
* Re: [PATCH v7 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: sashiko-bot @ 2026-05-20 3:58 UTC (permalink / raw)
To: Derek J. Clark; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260520013158.3633277-5-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Concurrent execution of sysfs attributes can lead to premature command completion and stale reads because untracked generic ACKs from zero-timeout commands hijack pending wait completions.
- [High] A race condition during module removal can lead to a Use-After-Free of `drvdata` due to improper teardown ordering of the `rgb_queue` delayed work.
--
commit efccc9e9df435fd90fd3e39aab88962809dcbfde
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add Rumble Intensity Attributes
Adds intensity adjustment for the left and right rumble motors.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 61917902e38d3..4adc5588618df 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -919,6 +951,154 @@ static ssize_t button_mapping_options_show(struct device *dev,
> }
> static DEVICE_ATTR_RO(button_mapping_options);
>
> +static ssize_t rumble_intensity_left_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
[ ... ]
> + ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
> + (u8 *)&report, sizeof(report), 25);
> + if (ret)
> + return ret;
> +
> + /* MCU will not send ACK until the USB transaction completes. ACK is sent
> + * immediately after and will hit the stale state machine, before the next
> + * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
> + * for ACK that ill never come.
> + */
> + ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
[Severity: High]
This is a pre-existing issue, but using timeout 0 here appears to leave an
untracked generic ACK in the hardware pipeline.
If another thread concurrently triggers a hardware command (like a sysfs
show/store) that waits for an ACK, could the claw_raw_event() handler
unconditionally consume this delayed generic ACK to complete the new
command's wait?
Since the generic ACK handler doesn't verify drvdata->waiting_cmd, this
might cause the new command to return prematurely before its actual data
arrives, leading to stale sysfs reads or dropped configuration writes.
> + if (ret)
> + return ret;
> +
> + return count;
> +}
[ ... ]
> @@ -1557,6 +1748,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
> spin_lock_init(&drvdata->mode_lock);
> spin_lock_init(&drvdata->profile_lock);
> spin_lock_init(&drvdata->frame_lock);
> + spin_lock_init(&drvdata->rumble_lock);
> init_completion(&drvdata->send_cmd_complete);
> INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
> INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
[Severity: High]
This is a pre-existing issue not introduced by this patch, but while
reviewing the driver's workqueue lifecycle, I noticed a potential race
condition during module removal.
In claw_remove(), if cancel_delayed_work_sync(&drvdata->rgb_queue) is called
before setting drvdata->rgb_registered to false, could a concurrent sysfs
write to the LED brightness file cause a use-after-free?
Since the LED class device is managed via devres, it remains accessible. A
write could invoke claw_led_brightness_set(), check that rgb_registered is
still true, and re-arm the delayed work timer after it was cancelled. When
devres later frees drvdata, the timer would fire and access freed memory.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520013158.3633277-1-derekjohn.clark@gmail.com?part=4
^ permalink raw reply
* Re: [PATCH v7 3/4] HID: hid-msi: Add RGB control interface
From: sashiko-bot @ 2026-05-20 3:23 UTC (permalink / raw)
To: Derek J. Clark; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260520013158.3633277-4-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] A race condition in `claw_remove()` allows the `rgb_queue` delayed work to be re-armed after it is cancelled, leading to a Use-After-Free.
- [High] Failing to unregister the LED class device if `device_add_group()` fails leaves it registered but sets `rgb_registered = false`, causing list corruption upon retry.
--
commit 35b79b95f2f2fd70b7faaa568ae55b37643edb83
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add RGB control interface
Adds RGB control interface for MSI Claw devices. The MSI Claw uses a
fairly unique RGB interface. It has 9 total zones (4 per joystick ring
and 1 for the ABXY buttons), and supports up to 8 sequential frames of
RGB zone data.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index 193435ec9386c..61917902e38d3 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -871,31 +960,521 @@ static const struct attribute_group claw_gamepad_attr_group = {
[ ... ]
> +try_rgb:
> + /* Add and enable RGB interface once we have the device state */
> + /* Pairs with smp_store_release from below */
> + if (!smp_load_acquire(&drvdata->rgb_registered) && rgb_ready) {
> + ret = devm_led_classdev_multicolor_register(&drvdata->hdev->dev, &drvdata->led_mc);
> + if (ret) {
> + dev_err(&drvdata->hdev->dev,
> + "Failed to create led device: %d\n", ret);
> + goto update_kobjects;
> + }
> +
> + ret = device_add_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
> + if (ret) {
> + dev_err(&drvdata->hdev->dev,
> + "Failed to create RGB attrs: %d\n", ret);
> + goto update_kobjects;
> + }
[Severity: High]
If device_add_group() fails here, we jump to update_kobjects without rolling
back the devm_led_classdev_multicolor_register() call, and rgb_registered
remains false.
On a subsequent system resume, cfg_resume_fn() will see rgb_registered is
false and schedule cfg_setup_fn() again:
if (!smp_load_acquire(&drvdata->rgb_registered))
schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
Will the second call to devm_led_classdev_multicolor_register() with the exact
same drvdata->led_mc structure corrupt the leds_list, since led_cdev->node
is already in the list from the first call?
> +
> + /* Pairs with smp_load_acquire in attribute show/store functions */
> + smp_store_release(&drvdata->rgb_registered, true);
> + }
[ ... ]
> @@ -1021,11 +1622,15 @@ static void claw_remove(struct hid_device *hdev)
> return;
> }
>
> + /* Block writes to brightness/multi_intensity during teardown */
> cancel_delayed_work_sync(&drvdata->cfg_resume);
> cancel_delayed_work_sync(&drvdata->cfg_setup);
> + cancel_delayed_work_sync(&drvdata->rgb_queue);
>
> /* Pairs with smp_load_acquire in attribute show/store functions */
> smp_store_release(&drvdata->gamepad_registered, false);
> + /* Pairs with smp_load_acquire in attribute show/store functions */
> + smp_store_release(&drvdata->rgb_registered, false);
[Severity: High]
Could setting rgb_registered to false after cancelling the delayed work
cause a use-after-free here?
Because sysfs attributes are managed by devres, they remain active until
claw_remove() returns. A concurrent sysfs write could check rgb_registered
and then get preempted:
/* in effect_store() */
if (!smp_load_acquire(&drvdata->rgb_registered))
return -ENODEV;
/* Preempted here */
Meanwhile, claw_remove() runs:
cancel_delayed_work_sync(&drvdata->rgb_queue);
smp_store_release(&drvdata->rgb_registered, false);
Then the sysfs writer resumes and queues the work again:
mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
After claw_remove() returns, devres will free drvdata. When the queued work
eventually executes claw_rgb_queue_fn(), will it access the freed drvdata?
Would clearing rgb_registered before calling cancel_delayed_work_sync()
prevent the work from being re-queued?
>
> hid_hw_close(hdev);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520013158.3633277-1-derekjohn.clark@gmail.com?part=3
^ permalink raw reply
* Re: [PATCH v7 2/4] HID: hid-msi: Add M-key mapping attributes
From: sashiko-bot @ 2026-05-20 2:51 UTC (permalink / raw)
To: Derek J. Clark; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260520013158.3633277-3-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] A stale or late `READ_PROFILE_ACK` packet will abort an active, legitimate `READ_PROFILE` command.
- [High] A Time-Of-Check to Time-Of-Use (TOCTOU) race in `claw_profile_event()` can clobber a newly set `profile_pending` state.
- [High] The driver sends the `SYNC_TO_ROM` command without waiting for its ACK, intentionally leaving an unconsumed ACK in the pipeline.
- [Low] Early error returns leave `drvdata->profile_pending` in a dangling state.
Pre-existing issues:
- [High] Generic ACKs (`CLAW_COMMAND_TYPE_ACK`) unconditionally complete any active wait, undermining synchronization guarantees.
--
commit ea147cb0c22726417b3bcecf39e1edbaa79122af
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add M-key mapping attributes
Adds sysfs attributes to allow remapping the M-keys with up to 5 values when in
macro mode. The driver maps gamepad buttons, keyboard keys, and mouse events,
using device endpoints or event devices depending on the mapping type.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> index d95483907a5e5..193435ec9386c 100644
> --- a/drivers/hid/hid-msi.c
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> @@ -136,6 +320,39 @@ static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
> return 0;
> }
>
> +static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
> +{
> + enum claw_profile_ack_pending profile;
> + struct claw_mkey_report *mkeys;
> + u8 *codes, key;
> + int i;
> +
> + scoped_guard(spinlock, &drvdata->profile_lock)
> + profile = drvdata->profile_pending;
> +
> + switch (profile) {
> + case CLAW_M1_PENDING:
> + case CLAW_M2_PENDING:
> + key = (profile == CLAW_M1_PENDING) ? CLAW_KEY_M1 : CLAW_KEY_M2;
> + mkeys = (struct claw_mkey_report *)cmd_rep->data;
> + if (be16_to_cpu(mkeys->read_addr) != drvdata->bmap_addr[key])
> + return -EINVAL;
[Severity: Low]
If this returns early because of an address mismatch, will profile_pending be
left in a dangling state instead of being cleared to CLAW_NO_PENDING?
[ ... ]
> + default:
> + dev_dbg(&drvdata->hdev->dev,
> + "Got profile event without changes pending from command: %x\n",
> + cmd_rep->cmd);
> + return -EINVAL;
> + }
> + scoped_guard(spinlock, &drvdata->profile_lock)
> + drvdata->profile_pending = CLAW_NO_PENDING;
[Severity: High]
Can a concurrent sysfs read overwrite the profile_pending state during the
window between releasing and re-acquiring the lock?
If another CPU acquires the lock and sets a new state, such as CLAW_M2_PENDING,
won't this unconditionally overwrite that new state with CLAW_NO_PENDING and
break the subsequent command's wait?
> +
> + return 0;
> +}
[ ... ]
> @@ -166,6 +383,19 @@ static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *repor
> }
> }
>
> + break;
> + case CLAW_COMMAND_TYPE_READ_PROFILE_ACK:
> + ret = claw_profile_event(drvdata, cmd_rep);
> +
> + scoped_guard(spinlock, &drvdata->cmd_lock) {
> + if (drvdata->waiting_for_ack &&
> + drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_PROFILE) {
> + drvdata->cmd_status = ret;
> + drvdata->waiting_for_ack = false;
> + complete(&drvdata->send_cmd_complete);
> + }
> + }
> +
> + break;
[Severity: High]
If a late or stale READ_PROFILE_ACK arrives during an active READ_PROFILE
command wait, it will cause claw_profile_event() to return -EINVAL.
Will this unconditionally assign the -EINVAL to cmd_status and complete
the wait prematurely, aborting the active valid command instead of ignoring
the mismatched ACK?
> case CLAW_COMMAND_TYPE_ACK:
> scoped_guard(spinlock, &drvdata->cmd_lock) {
> if (drvdata->waiting_for_ack) {
> drvdata->cmd_status = 0;
> drvdata->waiting_for_ack = false;
> complete(&drvdata->send_cmd_complete);
> }
[Severity: High]
This is a pre-existing issue, but does this generic ACK handler blindly
complete any active wait without checking drvdata->waiting_cmd?
If a generic ACK arrives while a command like READ_PROFILE is actively
waiting for a specific data payload, the generic ACK will fulfill the wait
early with a success status, causing the caller to falsely assume the read
succeeded and consume stale data.
[ ... ]
> @@ -433,6 +663,173 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
> }
> static DEVICE_ATTR_WO(reset);
>
> +static int mkey_mapping_name_to_code(const char *name)
...
> +static int claw_mkey_store(struct device *dev, const char *buf, u8 mkey)
> +{
...
> +set_buttons:
> + scoped_guard(mutex, &drvdata->rom_mutex) {
> + ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
> + (u8 *)&report, sizeof(report), 25);
> + if (ret)
> + return ret;
> + /* MCU will not send ACK until the USB transaction completes. ACK is sent
> + * immediately after and will hit the stale state machine, before the next
> + * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
> + * for ACK that ill never come.
> + */
> + ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
[Severity: High]
Since this sends SYNC_TO_ROM with a timeout of 0, doesn't it intentionally
leave an unconsumed ACK in the pipeline?
Because the generic ACK handler doesn't check waiting_cmd, won't this
delayed ACK unconditionally complete the next command issued to the device
that waits for a response, such as claw_mkey_show(), breaking the host-device
synchronization?
> + }
> +
> + return ret;
> +}
> +
> +static int claw_mkey_show(struct device *dev, char *buf, enum claw_key_index m_key)
> +{
> + struct hid_device *hdev = to_hid_device(dev);
> + struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
> + struct claw_mkey_report report = { {0x01, cpu_to_be16(drvdata->bmap_addr[m_key])}, 0x07 };
> + int i, ret, count = 0;
> + const char *name;
> + u8 *codes;
> +
> + /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
> + if (!smp_load_acquire(&drvdata->gamepad_registered))
> + return -ENODEV;
> +
> + codes = (m_key == CLAW_KEY_M1) ? drvdata->m1_codes : drvdata->m2_codes;
> +
> + guard(mutex)(&drvdata->profile_mutex);
> + scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
> + drvdata->profile_pending = (m_key == CLAW_KEY_M1) ? CLAW_M1_PENDING
> + : CLAW_M2_PENDING;
> +
> + ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE,
> + (u8 *)&report, sizeof(report), 25);
> + if (ret)
> + return ret;
[Severity: Low]
If claw_hw_output_report() fails, such as due to a timeout, does this early
return leave profile_pending in a dangling state instead of clearing it
back to CLAW_NO_PENDING?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520013158.3633277-1-derekjohn.clark@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v7 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: sashiko-bot @ 2026-05-20 2:08 UTC (permalink / raw)
To: Derek J. Clark; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260520013158.3633277-2-derekjohn.clark@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] A Time-of-Check to Time-of-Use (TOCTOU) race condition in the sysfs attribute handlers allows hardware access after device teardown.
- [Medium] State machine is not reset if hid_hw_output_report() returns a negative error code.
--
commit 4222880100fe04d05b88fc8c07f6d4c9f2dcb27c
Author: Derek J. Clark <derekjohn.clark@gmail.com>
HID: hid-msi: Add MSI Claw configuration driver
Adds a configuration HID driver for the MSI Claw series of handheld PCs
to support gamepad mode switching, M-key behavior adjustments, and device
reset functions.
> diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
> new file mode 100644
> index 0000000000000..d95483907a5e5
> --- /dev/null
> +++ b/drivers/hid/hid-msi.c
[ ... ]
> +static int claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
> + size_t len, unsigned int timeout)
> +{
[ ... ]
> + guard(mutex)(&drvdata->cfg_mutex);
> + if (timeout) {
> + reinit_completion(&drvdata->send_cmd_complete);
> + scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
> + drvdata->waiting_cmd = index;
> + drvdata->waiting_for_ack = true;
> + drvdata->cmd_status = -ETIMEDOUT;
> + }
> + }
> +
> + dev_dbg(&hdev->dev, "Send data as raw output report: [%*ph]\n",
> + CLAW_PACKET_SIZE, dmabuf);
> +
> + ret = hid_hw_output_report(hdev, dmabuf, CLAW_PACKET_SIZE);
> + if (ret < 0)
> + return ret;
[Severity: Medium]
Does this leave the state machine armed if hid_hw_output_report() returns a
negative error code (like -EPIPE)?
If the early return is taken, waiting_for_ack and waiting_cmd remain set.
If a subsequent command is sent with timeout == 0, and an unexpected input
report arrives that matches the stale waiting_cmd, could it improperly
trigger a completion and corrupt the internal state?
Would it be safer to clear waiting_cmd and waiting_for_ack on all error paths
before returning?
> +
> + ret = ret == CLAW_PACKET_SIZE ? 0 : -EIO;
> + if (ret) {
> + scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
> + drvdata->waiting_cmd = CLAW_COMMAND_TYPE_NONE;
> + drvdata->waiting_for_ack = false;
> + }
> + return ret;
> + }
[ ... ]
> +static ssize_t gamepad_mode_store(struct device *dev, struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct hid_device *hdev = to_hid_device(dev);
> + struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
> + int i, ret = -EINVAL;
> + u8 data[2];
> +
> + /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
> + if (!smp_load_acquire(&drvdata->gamepad_registered))
> + return -ENODEV;
[Severity: High]
Can a sysfs thread race with device removal here and cause a use-after-free?
The lockless check of gamepad_registered does not appear to prevent
msi_remove() from running concurrently once the check passes. If the USB
device is disconnected, a sysfs thread could evaluate the flag as true
and then get preempted.
Concurrently, msi_remove() could run, set the flag to false, and call
hid_hw_stop(), which tears down the lower-level USB HID resources.
When the sysfs thread resumes, it would eventually call:
claw_hw_output_report() -> hid_hw_output_report() -> usbhid_output_report()
on the torn-down hdev structure.
Should a mutex or other synchronization mechanism be used to safely serialize
the sysfs execution against hardware teardown?
> +
> + for (i = 0; i < ARRAY_SIZE(claw_gamepad_mode_text); i++) {
> + if (claw_gamepad_mode_text[i] && sysfs_streq(buf, claw_gamepad_mode_text[i])) {
> + ret = i;
> + break;
> + }
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520013158.3633277-1-derekjohn.clark@gmail.com?part=1
^ permalink raw reply
* [PATCH v7 3/4] HID: hid-msi: Add RGB control interface
From: Derek J. Clark @ 2026-05-20 1:31 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260520013158.3633277-1-derekjohn.clark@gmail.com>
Adds RGB control interface for MSI Claw devices. The MSI Claw uses a
fairly unique RGB interface. It has 9 total zones (4 per joystick ring
and 1 for the ABXY buttons), and supports up to 8 sequential frames of
RGB zone data. Each frame is written to a specific area of MCU memory by
the profile command, the value of which changes based on the firmware of
the device. Unlike other devices (such as the Legion Go or the OneXPlayer
devices), there are no hard coded effects built into the MCU. Instead,
the basic effects are provided as a series of frame data. I have
mirrored the effects available in Windows in this driver, while keeping
the effect names consistent with the Lenovo drivers for the effects that
are similar.
Initial reverse-engineering and implementation of this feature was done
by Zhouwang Huang. I refactored the overall format to conform to kernel
driver best practices and style guides. Claude was used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v7:
- Use smp_[store_release|load_acquire] pattern for checking
rgb_registered to avoid possible races during teardown.
- Add gating to cfg_setup_fn, allowing either gamepad settings or rgb
settings to populate if the other fails for any reason.
- Use spinlock when writing profile_pending.
v6:
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Gate all attribute show/store functions with rgb_registered,
enabling use of devm_device_add_group.
v5:
- Move adding the RGB device into cfg_setup to prevent led core
attributes from being written to prior to setup completing.
- Ensure frame_lock is properly init.
- Change variable names in RGB functions from frame and zone to f and
z respectively to fit all scoped_guard actions in 100 columns.
v4:
- Fix frame_calc validity check to use >=.
- USe spinlock instead of mutex in raw_event and related attribute
_store function.
- Ensure delayed work is canceled in suspend & canceled before sysfs
attribute removal.
v3:
- Add mutex for read/write of rgb frame data.
- Remove setting rgb_frame_count when reading rgb profiles as it always
returns garbage data.
- Ensure rgb_speed is getting drvdata from a valid lookup (not hdev).
v2:
- Use pending_profile mutex
---
drivers/hid/hid-msi.c | 637 ++++++++++++++++++++++++++++++++++++++++--
1 file changed, 621 insertions(+), 16 deletions(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 193435ec9386c..61917902e38d3 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -21,6 +21,7 @@
#include <linux/device.h>
#include <linux/hid.h>
#include <linux/kobject.h>
+#include <linux/led-class-multicolor.h>
#include <linux/leds.h>
#include <linux/module.h>
#include <linux/mutex.h>
@@ -44,6 +45,10 @@
#define CLAW_KEYS_MAX 5
+#define CLAW_RGB_ZONES 9
+#define CLAW_RGB_MAX_FRAMES 8
+#define CLAW_RGB_FRAME_OFFSET 0x24
+
enum claw_command_index {
CLAW_COMMAND_TYPE_NONE = 0x00,
CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
@@ -73,6 +78,7 @@ enum claw_profile_ack_pending {
CLAW_NO_PENDING,
CLAW_M1_PENDING,
CLAW_M2_PENDING,
+ CLAW_RGB_PENDING,
};
enum claw_key_index {
@@ -231,6 +237,22 @@ static const struct {
{ 0xff, "DISABLED" },
};
+enum claw_rgb_effect_index {
+ CLAW_RGB_EFFECT_MONOCOLOR,
+ CLAW_RGB_EFFECT_BREATHE,
+ CLAW_RGB_EFFECT_CHROMA,
+ CLAW_RGB_EFFECT_RAINBOW,
+ CLAW_RGB_EFFECT_FROSTFIRE,
+};
+
+static const char * const claw_rgb_effect_text[] = {
+ [CLAW_RGB_EFFECT_MONOCOLOR] = "monocolor",
+ [CLAW_RGB_EFFECT_BREATHE] = "breathe",
+ [CLAW_RGB_EFFECT_CHROMA] = "chroma",
+ [CLAW_RGB_EFFECT_RAINBOW] = "rainbow",
+ [CLAW_RGB_EFFECT_FROSTFIRE] = "frostfire",
+};
+
static const u16 button_mapping_addr_old[] = {
0x007a, /* M1 */
0x011f, /* M2 */
@@ -241,6 +263,9 @@ static const u16 button_mapping_addr_new[] = {
0x0164, /* M2 */
};
+static const u16 rgb_addr_old = 0x01fa;
+static const u16 rgb_addr_new = 0x024a;
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -262,6 +287,27 @@ struct claw_mkey_report {
u8 codes[5];
} __packed;
+struct rgb_zone {
+ u8 red;
+ u8 green;
+ u8 blue;
+};
+
+struct rgb_frame {
+ struct rgb_zone zone[CLAW_RGB_ZONES];
+};
+
+struct claw_rgb_report {
+ struct claw_profile_report;
+ u8 frame_bytes;
+ u8 padding;
+ u8 frame_count;
+ u8 state; /* Always 0x09 */
+ u8 speed;
+ u8 brightness;
+ struct rgb_frame zone_data;
+} __packed;
+
struct claw_drvdata {
/* MCU General Variables */
enum claw_profile_ack_pending profile_pending;
@@ -289,6 +335,18 @@ struct claw_drvdata {
spinlock_t mode_lock; /* Lock for mode data read/write */
const u16 *bmap_addr;
bool bmap_support;
+
+ /* RGB Variables */
+ struct rgb_frame rgb_frames[CLAW_RGB_MAX_FRAMES];
+ enum claw_rgb_effect_index rgb_effect;
+ struct led_classdev_mc led_mc;
+ struct delayed_work rgb_queue;
+ spinlock_t frame_lock; /* lock for rgb_frames read/write */
+ bool rgb_registered;
+ u8 rgb_frame_count;
+ bool rgb_enabled;
+ u8 rgb_speed;
+ u16 rgb_addr;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -324,8 +382,11 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
{
enum claw_profile_ack_pending profile;
struct claw_mkey_report *mkeys;
- u8 *codes, key;
- int i;
+ struct claw_rgb_report *frame;
+ u16 rgb_addr, read_addr;
+ u8 *codes, key, f_idx;
+ u16 frame_calc;
+ int i, ret = 0;
scoped_guard(spinlock, &drvdata->profile_lock)
profile = drvdata->profile_pending;
@@ -341,6 +402,34 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
for (i = 0; i < CLAW_KEYS_MAX; i++)
codes[i] = (mkeys->codes[i]);
break;
+ case CLAW_RGB_PENDING:
+ frame = (struct claw_rgb_report *)cmd_rep->data;
+ rgb_addr = drvdata->rgb_addr;
+ read_addr = be16_to_cpu(frame->read_addr);
+ frame_calc = (read_addr - rgb_addr) / CLAW_RGB_FRAME_OFFSET;
+ if (frame_calc >= CLAW_RGB_MAX_FRAMES) {
+ dev_err(&drvdata->hdev->dev, "Got unsupported frame index: %x\n",
+ frame_calc);
+ return -EINVAL;
+ }
+ f_idx = frame_calc;
+
+ scoped_guard(spinlock, &drvdata->frame_lock) {
+ memcpy(&drvdata->rgb_frames[f_idx], &frame->zone_data,
+ sizeof(struct rgb_frame));
+
+ /* Only use frame 0 for remaining variable assignment */
+ if (f_idx != 0)
+ break;
+
+ drvdata->rgb_speed = frame->speed;
+ drvdata->led_mc.led_cdev.brightness = frame->brightness;
+ drvdata->led_mc.subled_info[0].intensity = frame->zone_data.zone[0].red;
+ drvdata->led_mc.subled_info[1].intensity = frame->zone_data.zone[0].green;
+ drvdata->led_mc.subled_info[2].intensity = frame->zone_data.zone[0].blue;
+ }
+
+ break;
default:
dev_dbg(&drvdata->hdev->dev,
"Got profile event without changes pending from command: %x\n",
@@ -350,7 +439,7 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
scoped_guard(spinlock, &drvdata->profile_lock)
drvdata->profile_pending = CLAW_NO_PENDING;
- return 0;
+ return ret;
}
static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
@@ -871,31 +960,521 @@ static const struct attribute_group claw_gamepad_attr_group = {
.is_visible = claw_gamepad_attr_is_visible,
};
+/* Read RGB config from device */
+static int claw_read_rgb_config(struct hid_device *hdev)
+{
+ u8 data[4] = { 0x01, 0x00, 0x00, CLAW_RGB_FRAME_OFFSET };
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u16 read_addr = drvdata->rgb_addr;
+ size_t len = ARRAY_SIZE(data);
+ int ret, i;
+
+ if (!drvdata->rgb_addr)
+ return -ENODEV;
+
+ /* Loop through all 8 pages of RGB data */
+ guard(mutex)(&drvdata->profile_mutex);
+ for (i = 0; i < CLAW_RGB_MAX_FRAMES; i++) {
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_RGB_PENDING;
+ data[1] = (read_addr >> 8) & 0xff;
+ data[2] = read_addr & 0x00ff;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE, data, len, 25);
+ if (ret)
+ return ret;
+
+ read_addr += CLAW_RGB_FRAME_OFFSET;
+ }
+
+ return 0;
+}
+
+/* Send RGB configuration to device */
+static int claw_write_rgb_state(struct claw_drvdata *drvdata)
+{
+ struct claw_rgb_report report = { {0x01, 0}, CLAW_RGB_FRAME_OFFSET, 0x00,
+ drvdata->rgb_frame_count, 0x09, drvdata->rgb_speed,
+ drvdata->led_mc.led_cdev.brightness };
+ u16 write_addr = drvdata->rgb_addr;
+ int f, ret;
+
+ if (!drvdata->rgb_addr)
+ return -ENODEV;
+
+ if (!drvdata->rgb_frame_count)
+ return -EINVAL;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ /* Loop through (up to) 8 pages of RGB data */
+ for (f = 0; f < drvdata->rgb_frame_count; f++) {
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock)
+ report.zone_data = drvdata->rgb_frames[f];
+
+ /* Set the MCU address to write the frame data to */
+ report.read_addr = cpu_to_be16(write_addr);
+
+ /* Serialize the rgb_report and write it to MCU */
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ /* Increment the write addr by the offset for the next frame */
+ write_addr += CLAW_RGB_FRAME_OFFSET;
+ }
+
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+
+ return ret;
+}
+
+/* Fill all zones with the same color */
+static void claw_frame_fill_solid(struct rgb_frame *frame, struct rgb_zone zone)
+{
+ int z;
+
+ for (z = 0; z < CLAW_RGB_ZONES; z++)
+ frame->zone[z] = zone;
+}
+
+/* Apply solid effect (1 frame, no color) */
+static int claw_apply_disabled(struct claw_drvdata *drvdata)
+{
+ struct rgb_zone off = { 0x00, 0x00, 0x00};
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = 1;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], off);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply solid effect (1 frame, all zones same color) */
+static int claw_apply_monocolor(struct claw_drvdata *drvdata)
+{
+ struct mc_subled *subleds = drvdata->led_mc.subled_info;
+ struct rgb_zone zone = { subleds[0].intensity, subleds[1].intensity,
+ subleds[2].intensity };
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = 1;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], zone);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply breathe effect (2 frames: color -> off) */
+static int claw_apply_breathe(struct claw_drvdata *drvdata)
+{
+ struct mc_subled *subleds = drvdata->led_mc.subled_info;
+ struct rgb_zone zone = { subleds[0].intensity, subleds[1].intensity,
+ subleds[2].intensity };
+ static const struct rgb_zone off = { 0, 0, 0 };
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = 2;
+ claw_frame_fill_solid(&drvdata->rgb_frames[0], zone);
+ claw_frame_fill_solid(&drvdata->rgb_frames[1], off);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply chroma effect (6 frames: rainbow cycle, all zones sync) */
+static int claw_apply_chroma(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* red */
+ {255, 255, 0}, /* yellow */
+ { 0, 255, 0}, /* green */
+ { 0, 255, 255}, /* cyan */
+ { 0, 0, 255}, /* blue */
+ {255, 0, 255}, /* magenta */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int f;
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = frame_count;
+
+ for (f = 0; f < frame_count; f++)
+ claw_frame_fill_solid(&drvdata->rgb_frames[f], colors[f]);
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply rainbow effect (4 frames: rotating colors around joysticks) */
+static int claw_apply_rainbow(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* red */
+ { 0, 255, 0}, /* green */
+ { 0, 255, 255}, /* cyan */
+ { 0, 0, 255}, /* blue */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int f, z;
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = frame_count;
+
+ for (f = 0; f < frame_count; f++) {
+ for (z = 0; z < 4; z++) {
+ drvdata->rgb_frames[f].zone[z] = colors[(z + f) % 4];
+ drvdata->rgb_frames[f].zone[z + 4] = colors[(z + f) % 4];
+ }
+ drvdata->rgb_frames[f].zone[8] = colors[f];
+ }
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/*
+ * Apply frostfire effect (4 frames: fire vs ice rotating)
+ * Right joystick: fire red -> dark -> ice blue -> dark (clockwise)
+ * Left joystick: ice blue -> dark -> fire red -> dark (counter-clockwise)
+ * ABXY: fire red -> dark -> ice blue -> dark
+ */
+static int claw_apply_frostfire(struct claw_drvdata *drvdata)
+{
+ static const struct rgb_zone colors[] = {
+ {255, 0, 0}, /* fire red */
+ { 0, 0, 0}, /* dark */
+ { 0, 0, 255}, /* ice blue */
+ { 0, 0, 0}, /* dark */
+ };
+ u8 frame_count = ARRAY_SIZE(colors);
+ int f, z;
+
+ scoped_guard(spinlock_irqsave, &drvdata->frame_lock) {
+ drvdata->rgb_frame_count = frame_count;
+
+ for (f = 0; f < frame_count; f++) {
+ for (z = 0; z < 4; z++) {
+ drvdata->rgb_frames[f].zone[z] = colors[(z + f) % 4];
+ drvdata->rgb_frames[f].zone[z + 4] = colors[(z - f + 6) % 4];
+ }
+ drvdata->rgb_frames[f].zone[8] = colors[f];
+ }
+ }
+
+ return claw_write_rgb_state(drvdata);
+}
+
+/* Apply current state to device */
+static int claw_apply_rgb_state(struct claw_drvdata *drvdata)
+{
+ if (!drvdata->rgb_enabled)
+ return claw_apply_disabled(drvdata);
+
+ switch (drvdata->rgb_effect) {
+ case CLAW_RGB_EFFECT_MONOCOLOR:
+ return claw_apply_monocolor(drvdata);
+ case CLAW_RGB_EFFECT_BREATHE:
+ return claw_apply_breathe(drvdata);
+ case CLAW_RGB_EFFECT_CHROMA:
+ return claw_apply_chroma(drvdata);
+ case CLAW_RGB_EFFECT_RAINBOW:
+ return claw_apply_rainbow(drvdata);
+ case CLAW_RGB_EFFECT_FROSTFIRE:
+ return claw_apply_frostfire(drvdata);
+ default:
+ dev_err(drvdata->led_mc.led_cdev.dev,
+ "No supported rgb_effect selected\n");
+ return -EINVAL;
+ }
+}
+
+static void claw_rgb_queue_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, rgb_queue);
+ int ret;
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return;
+
+ ret = claw_apply_rgb_state(drvdata);
+ if (ret)
+ dev_err(drvdata->led_mc.led_cdev.dev,
+ "Failed to apply RGB state: %d\n", ret);
+}
+
+static ssize_t effect_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ int ret;
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+
+ ret = sysfs_match_string(claw_rgb_effect_text, buf);
+ if (ret < 0)
+ return ret;
+
+ drvdata->rgb_effect = ret;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t effect_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+
+ if (drvdata->rgb_effect >= ARRAY_SIZE(claw_rgb_effect_text))
+ return -EINVAL;
+
+ return sysfs_emit(buf, "%s\n", claw_rgb_effect_text[drvdata->rgb_effect]);
+}
+
+static DEVICE_ATTR_RW(effect);
+
+static ssize_t effect_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_rgb_effect_text); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_rgb_effect_text[i]);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(effect_index);
+
+static ssize_t enabled_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ bool val;
+ int ret;
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+
+ ret = kstrtobool(buf, &val);
+ if (ret)
+ return ret;
+
+ drvdata->rgb_enabled = val;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t enabled_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+
+ return sysfs_emit(buf, "%s\n", drvdata->rgb_enabled ? "true" : "false");
+}
+static DEVICE_ATTR_RW(enabled);
+
+static ssize_t enabled_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "true false\n");
+}
+static DEVICE_ATTR_RO(enabled_index);
+
+static ssize_t speed_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ unsigned int val, speed;
+ int ret;
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+
+ ret = kstrtouint(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 20)
+ return -EINVAL;
+
+ /* 0 is fastest, invert value for intuitive userspace speed */
+ speed = 20 - val;
+
+ drvdata->rgb_speed = speed;
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+
+ return count;
+}
+
+static ssize_t speed_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct led_classdev *led_cdev = dev_get_drvdata(dev);
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+ u8 speed = 20 - drvdata->rgb_speed;
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return -ENODEV;
+
+ return sysfs_emit(buf, "%u\n", speed);
+}
+static DEVICE_ATTR_RW(speed);
+
+static ssize_t speed_range_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ return sysfs_emit(buf, "0-20\n");
+}
+static DEVICE_ATTR_RO(speed_range);
+
+static void claw_led_brightness_set(struct led_classdev *led_cdev,
+ enum led_brightness _brightness)
+{
+ struct led_classdev_mc *led_mc = container_of(led_cdev, struct led_classdev_mc, led_cdev);
+ struct claw_drvdata *drvdata = container_of(led_mc, struct claw_drvdata, led_mc);
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->rgb_registered))
+ return;
+
+ mod_delayed_work(system_wq, &drvdata->rgb_queue, msecs_to_jiffies(50));
+}
+
+static struct attribute *claw_rgb_attrs[] = {
+ &dev_attr_effect.attr,
+ &dev_attr_effect_index.attr,
+ &dev_attr_enabled.attr,
+ &dev_attr_enabled_index.attr,
+ &dev_attr_speed.attr,
+ &dev_attr_speed_range.attr,
+ NULL,
+};
+
+static const struct attribute_group claw_rgb_attr_group = {
+ .attrs = claw_rgb_attrs,
+};
+
+static struct mc_subled claw_rgb_subled_info[] = {
+ {
+ .color_index = LED_COLOR_ID_RED,
+ .channel = 0x1,
+ },
+ {
+ .color_index = LED_COLOR_ID_GREEN,
+ .channel = 0x2,
+ },
+ {
+ .color_index = LED_COLOR_ID_BLUE,
+ .channel = 0x3,
+ },
+};
+
static void cfg_setup_fn(struct work_struct *work)
{
struct delayed_work *dwork = container_of(work, struct delayed_work, work);
struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_setup);
+ bool gamepad_ready = false, rgb_ready = false;
int ret;
ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE,
NULL, 0, 25);
if (ret) {
dev_err(&drvdata->hdev->dev,
- "Failed to setup device, can't read gamepad mode: %d\n", ret);
- return;
+ "Failed to read gamepad mode: %d\n", ret);
+ goto prep_rgb;
}
+ gamepad_ready = true;
- /* Add sysfs attributes after we get the device state */
- ret = devm_device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+prep_rgb:
+ ret = claw_read_rgb_config(drvdata->hdev);
if (ret) {
dev_err(&drvdata->hdev->dev,
- "Failed to setup device, can't create gamepad attrs: %d\n", ret);
- return;
+ "Failed to read RGB config: %d\n", ret);
+ goto try_gamepad;
}
- /* Pairs with smp_load_acquire in attribute show/store functions */
- smp_store_release(&drvdata->gamepad_registered, true);
+ rgb_ready = true;
- kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+try_gamepad:
+ /* Add sysfs attributes after we get the device state */
+ /* Pairs with smp_store_release from below */
+ if (!smp_load_acquire(&drvdata->gamepad_registered) && gamepad_ready) {
+ ret = devm_device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to create gamepad attrs: %d\n", ret);
+ goto try_rgb;
+ }
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->gamepad_registered, true);
+ }
+
+try_rgb:
+ /* Add and enable RGB interface once we have the device state */
+ /* Pairs with smp_store_release from below */
+ if (!smp_load_acquire(&drvdata->rgb_registered) && rgb_ready) {
+ ret = devm_led_classdev_multicolor_register(&drvdata->hdev->dev, &drvdata->led_mc);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to create led device: %d\n", ret);
+ goto update_kobjects;
+ }
+
+ ret = device_add_group(drvdata->led_mc.led_cdev.dev, &claw_rgb_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to create RGB attrs: %d\n", ret);
+ goto update_kobjects;
+ }
+
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->rgb_registered, true);
+ }
+
+update_kobjects:
+ /* Pairs with smp_store_release from above */
+ if (smp_load_acquire(&drvdata->gamepad_registered))
+ kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+ /* Pairs with smp_store_release from above */
+ if (smp_load_acquire(&drvdata->rgb_registered))
+ kobject_uevent(&drvdata->led_mc.led_cdev.dev->kobj, KOBJ_CHANGE);
}
static void cfg_resume_fn(struct work_struct *work)
@@ -903,8 +1482,10 @@ static void cfg_resume_fn(struct work_struct *work)
struct delayed_work *dwork = container_of(work, struct delayed_work, work);
struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_resume);
- /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
- if (!smp_load_acquire(&drvdata->gamepad_registered))
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered) ||
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ !smp_load_acquire(&drvdata->rgb_registered))
schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
}
@@ -915,18 +1496,24 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
if (major == 0x01) {
drvdata->bmap_support = true;
- if (minor >= 0x66)
+ if (minor >= 0x66) {
drvdata->bmap_addr = button_mapping_addr_new;
- else
+ drvdata->rgb_addr = rgb_addr_new;
+ } else {
drvdata->bmap_addr = button_mapping_addr_old;
+ drvdata->rgb_addr = rgb_addr_old;
+ }
return;
}
if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
drvdata->bmap_support = true;
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rgb_addr = rgb_addr_new;
return;
}
+
+ drvdata->rgb_addr = rgb_addr_old;
}
static int claw_probe(struct hid_device *hdev, u8 ep)
@@ -941,6 +1528,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
return -ENOMEM;
drvdata->gamepad_mode = CLAW_GAMEPAD_MODE_XINPUT;
+ drvdata->rgb_enabled = true;
drvdata->hdev = hdev;
drvdata->ep = ep;
@@ -951,15 +1539,28 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
if (!drvdata->bmap_support)
dev_dbg(&hdev->dev, "M-Key mapping is not supported. Update firmware to enable.\n");
+ drvdata->led_mc.led_cdev.name = "msi_claw:rgb:joystick_rings";
+ drvdata->led_mc.led_cdev.brightness = 0x50;
+ drvdata->led_mc.led_cdev.max_brightness = 0x64;
+ drvdata->led_mc.led_cdev.color = LED_COLOR_ID_RGB;
+ drvdata->led_mc.led_cdev.brightness_set = claw_led_brightness_set;
+ drvdata->led_mc.num_colors = 3;
+ drvdata->led_mc.subled_info = devm_kmemdup(&hdev->dev, claw_rgb_subled_info,
+ sizeof(claw_rgb_subled_info), GFP_KERNEL);
+ if (!drvdata->led_mc.subled_info)
+ return -ENOMEM;
+
mutex_init(&drvdata->cfg_mutex);
mutex_init(&drvdata->profile_mutex);
mutex_init(&drvdata->rom_mutex);
spin_lock_init(&drvdata->cmd_lock);
spin_lock_init(&drvdata->mode_lock);
spin_lock_init(&drvdata->profile_lock);
+ spin_lock_init(&drvdata->frame_lock);
init_completion(&drvdata->send_cmd_complete);
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
+ INIT_DELAYED_WORK(&drvdata->rgb_queue, &claw_rgb_queue_fn);
/* For control interface: open the HID transport for sending commands. */
ret = hid_hw_open(hdev);
@@ -1021,11 +1622,15 @@ static void claw_remove(struct hid_device *hdev)
return;
}
+ /* Block writes to brightness/multi_intensity during teardown */
cancel_delayed_work_sync(&drvdata->cfg_resume);
cancel_delayed_work_sync(&drvdata->cfg_setup);
+ cancel_delayed_work_sync(&drvdata->rgb_queue);
/* Pairs with smp_load_acquire in attribute show/store functions */
smp_store_release(&drvdata->gamepad_registered, false);
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->rgb_registered, false);
hid_hw_close(hdev);
}
--
2.53.0
^ permalink raw reply related
* [PATCH v7 4/4] HID: hid-msi: Add Rumble Intensity Attributes
From: Derek J. Clark @ 2026-05-20 1:31 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260520013158.3633277-1-derekjohn.clark@gmail.com>
Adds intensity adjustment for the left and right rumble motors.
Claude was used during the reverse-engineering data gathering for this
feature done by Zhouwang Huang. As the code had already been affected,
I used Claude to create the initial framing for the feature, then did
manual cleanup of the _show and _store functions afterwards to fix bugs
and keep the coding style consistent. Claude was also used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v7:
- Match on write address for rumble reports to prevent late ACK
from causing synchronization errors.
- Use spinlock for read/write profile_pending.
- Use smp_[store_release|load_acquire] pattern for checking
gamepad_registered to avoid possible races during teardown.
- Use struct for rumble reports.
v6:
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Add spinlock_irqsave for read/write access on rumble_intensity
variables.
- Gate all attribute show/store functions with gamepad_registered.
v5:
- Remove mkey related changes.
v2:
- Use pending_profile and sync to rom mutexes.
---
drivers/hid/hid-msi.c | 192 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 192 insertions(+)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index 61917902e38d3..4adc5588618df 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -79,6 +79,8 @@ enum claw_profile_ack_pending {
CLAW_M1_PENDING,
CLAW_M2_PENDING,
CLAW_RGB_PENDING,
+ CLAW_RUMBLE_LEFT_PENDING,
+ CLAW_RUMBLE_RIGHT_PENDING,
};
enum claw_key_index {
@@ -266,6 +268,11 @@ static const u16 button_mapping_addr_new[] = {
static const u16 rgb_addr_old = 0x01fa;
static const u16 rgb_addr_new = 0x024a;
+static const u16 rumble_addr[] = {
+ 0x0022, /* left */
+ 0x0023, /* right */
+};
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -308,6 +315,12 @@ struct claw_rgb_report {
struct rgb_frame zone_data;
} __packed;
+struct claw_rumble_report {
+ struct claw_profile_report;
+ u8 padding;
+ u8 intensity;
+} __packed;
+
struct claw_drvdata {
/* MCU General Variables */
enum claw_profile_ack_pending profile_pending;
@@ -331,9 +344,13 @@ struct claw_drvdata {
enum claw_gamepad_mode_index gamepad_mode;
u8 m1_codes[CLAW_KEYS_MAX];
u8 m2_codes[CLAW_KEYS_MAX];
+ u8 rumble_intensity_right;
+ u8 rumble_intensity_left;
bool gamepad_registered;
+ spinlock_t rumble_lock; /* lock for rumble_intensity read/write */
spinlock_t mode_lock; /* Lock for mode data read/write */
const u16 *bmap_addr;
+ bool rumble_support;
bool bmap_support;
/* RGB Variables */
@@ -381,6 +398,7 @@ static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
{
enum claw_profile_ack_pending profile;
+ struct claw_rumble_report *rumble;
struct claw_mkey_report *mkeys;
struct claw_rgb_report *frame;
u16 rgb_addr, read_addr;
@@ -430,6 +448,20 @@ static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_
}
break;
+ case CLAW_RUMBLE_LEFT_PENDING:
+ rumble = (struct claw_rumble_report *)cmd_rep->data;
+ if (be16_to_cpu(rumble->read_addr) != rumble_addr[0])
+ return -EINVAL;
+ scoped_guard(spinlock, &drvdata->rumble_lock)
+ drvdata->rumble_intensity_left = rumble->intensity;
+ break;
+ case CLAW_RUMBLE_RIGHT_PENDING:
+ rumble = (struct claw_rumble_report *)cmd_rep->data;
+ if (be16_to_cpu(rumble->read_addr) != rumble_addr[1])
+ return -EINVAL;
+ scoped_guard(spinlock, &drvdata->rumble_lock)
+ drvdata->rumble_intensity_right = rumble->intensity;
+ break;
default:
dev_dbg(&drvdata->hdev->dev,
"Got profile event without changes pending from command: %x\n",
@@ -919,6 +951,154 @@ static ssize_t button_mapping_options_show(struct device *dev,
}
static DEVICE_ATTR_RO(button_mapping_options);
+static ssize_t rumble_intensity_left_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[0])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ report.intensity = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_left_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[0])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+ u8 val;
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_RUMBLE_LEFT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->rumble_lock)
+ val = drvdata->rumble_intensity_left;
+
+ return sysfs_emit(buf, "%u\n", val);
+}
+static DEVICE_ATTR_RW(rumble_intensity_left);
+
+static ssize_t rumble_intensity_right_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[1])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ u8 val;
+ int ret;
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+
+ ret = kstrtou8(buf, 10, &val);
+ if (ret)
+ return ret;
+
+ if (val > 100)
+ return -EINVAL;
+
+ report.intensity = val;
+
+ guard(mutex)(&drvdata->rom_mutex);
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t rumble_intensity_right_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct claw_rumble_report report = { {0x01, cpu_to_be16(rumble_addr[1])}, 0x01 };
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret;
+ u8 val;
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_RUMBLE_RIGHT_PENDING;
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->rumble_lock)
+ val = drvdata->rumble_intensity_right;
+
+ return sysfs_emit(buf, "%u\n", val);
+}
+static DEVICE_ATTR_RW(rumble_intensity_right);
+
+static ssize_t rumble_intensity_range_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ return sysfs_emit(buf, "0-100\n");
+}
+static DEVICE_ATTR_RO(rumble_intensity_range);
+
static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
int n)
{
@@ -939,6 +1119,12 @@ static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribu
attr == &dev_attr_reset.attr)
return attr->mode;
+ /* Hide rumble attrs if not supported */
+ if (attr == &dev_attr_rumble_intensity_left.attr ||
+ attr == &dev_attr_rumble_intensity_right.attr ||
+ attr == &dev_attr_rumble_intensity_range.attr)
+ return drvdata->rumble_support ? attr->mode : 0;
+
/* Hide button mapping attrs if it isn't supported */
return drvdata->bmap_support ? attr->mode : 0;
}
@@ -952,6 +1138,9 @@ static struct attribute *claw_gamepad_attrs[] = {
&dev_attr_mkeys_function.attr,
&dev_attr_mkeys_function_index.attr,
&dev_attr_reset.attr,
+ &dev_attr_rumble_intensity_left.attr,
+ &dev_attr_rumble_intensity_right.attr,
+ &dev_attr_rumble_intensity_range.attr,
NULL,
};
@@ -1498,6 +1687,7 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
drvdata->bmap_support = true;
if (minor >= 0x66) {
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rumble_support = true;
drvdata->rgb_addr = rgb_addr_new;
} else {
drvdata->bmap_addr = button_mapping_addr_old;
@@ -1509,6 +1699,7 @@ static void claw_features_supported(struct claw_drvdata *drvdata)
if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
drvdata->bmap_support = true;
drvdata->bmap_addr = button_mapping_addr_new;
+ drvdata->rumble_support = true;
drvdata->rgb_addr = rgb_addr_new;
return;
}
@@ -1557,6 +1748,7 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
spin_lock_init(&drvdata->mode_lock);
spin_lock_init(&drvdata->profile_lock);
spin_lock_init(&drvdata->frame_lock);
+ spin_lock_init(&drvdata->rumble_lock);
init_completion(&drvdata->send_cmd_complete);
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
--
2.53.0
^ permalink raw reply related
* [PATCH v7 2/4] HID: hid-msi: Add M-key mapping attributes
From: Derek J. Clark @ 2026-05-20 1:31 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260520013158.3633277-1-derekjohn.clark@gmail.com>
Adds attributes that allow for remapping the M-keys with up to 5 values
when in macro mode. There are 2 mappable buttons on the rear of the
device, M1 on the right and M2 on the left. When mapped, the events will
fire from one of three event devices: gamepad buttons will fire from the
device handled by xpad, while keyboard and mouse events will fire from
respectively typed evdevs provided by the input core. Names of each
mapping have been kept as close to the event that will fire from the evdev
as possible, with context added to the ABS_ events on the direction of the
movement.
Initial reverse-engineering and implementation of this feature was done
by Zhouwang Huang. I refactored the overall format to conform to kernel
driver best practices and style guides. Claude was used as an initial
reviewer of this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v7:
- Use smp_[store_release|load_acquire] pattern for checking
gamepad_registered to avoid possible races during teardown.
- Add profile_lock for read/write profile_pending.
- Match on write address for mkey reports to prevent late ACK
from causing synchronization errors.
- Use struct for mkey reports.
v6:
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Gate all attribute show/store functions with gamepad_registered.
- Remove duplicated argv_free macro.
v5:
- Ensure adding "DISABLED" key to valid entries is done in the correct
patch.
- Re-enable sending an empty string to clear button mappings in
addition to setting DISABLED.
v4:
- Change dev_warn to dev_dbg in claw_profile_event.
- use __free with DEFINE_FREE macro for argv instead of manually
running argv_free, cleaining up scoped_guard goto.
v3:
- Use scoped_guard where necessary.
v2:
- Add mutex for SYNC_TO_ROM commands to ensure every SYNC is completed
before more data is written to the MCU volatile memory.
- Add mutex for profile_pending to ensure every profile action
response is serialized to the generating command.
---
drivers/hid/hid-msi.c | 444 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 443 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
index d95483907a5e5..193435ec9386c 100644
--- a/drivers/hid/hid-msi.c
+++ b/drivers/hid/hid-msi.c
@@ -42,6 +42,8 @@
#define CLAW_DINPUT_CFG_INTF_IN 0x82
#define CLAW_XINPUT_CFG_INTF_IN 0x83
+#define CLAW_KEYS_MAX 5
+
enum claw_command_index {
CLAW_COMMAND_TYPE_NONE = 0x00,
CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
@@ -67,6 +69,17 @@ static const char * const claw_gamepad_mode_text[] = {
[CLAW_GAMEPAD_MODE_DESKTOP] = "desktop",
};
+enum claw_profile_ack_pending {
+ CLAW_NO_PENDING,
+ CLAW_M1_PENDING,
+ CLAW_M2_PENDING,
+};
+
+enum claw_key_index {
+ CLAW_KEY_M1,
+ CLAW_KEY_M2,
+};
+
enum claw_mkeys_function_index {
CLAW_MKEY_FUNCTION_MACRO,
CLAW_MKEY_FUNCTION_DISABLED,
@@ -79,6 +92,155 @@ static const char * const claw_mkeys_function_text[] = {
[CLAW_MKEY_FUNCTION_COMBO] = "combination",
};
+static const struct {
+ u8 code;
+ const char *name;
+} claw_button_mapping_key_map[] = {
+ /* Gamepad buttons */
+ { 0x01, "ABS_HAT0Y_UP" },
+ { 0x02, "ABS_HAT0Y_DOWN" },
+ { 0x03, "ABS_HAT0X_LEFT" },
+ { 0x04, "ABS_HAT0X_RIGHT" },
+ { 0x05, "BTN_TL" },
+ { 0x06, "BTN_TR" },
+ { 0x07, "BTN_THUMBL" },
+ { 0x08, "BTN_THUMBR" },
+ { 0x09, "BTN_SOUTH" },
+ { 0x0a, "BTN_EAST" },
+ { 0x0b, "BTN_NORTH" },
+ { 0x0c, "BTN_WEST" },
+ { 0x0d, "BTN_MODE" },
+ { 0x0e, "BTN_SELECT" },
+ { 0x0f, "BTN_START" },
+ { 0x13, "BTN_TL2"},
+ { 0x14, "BTN_TR2"},
+ { 0x15, "ABS_Y_UP"},
+ { 0x16, "ABS_Y_DOWN"},
+ { 0x17, "ABS_X_LEFT"},
+ { 0x18, "ABS_X_RIGHT"},
+ { 0x19, "ABS_RY_UP"},
+ { 0x1a, "ABS_RY_DOWN"},
+ { 0x1b, "ABS_RX_LEFT"},
+ { 0x1c, "ABS_RX_RIGHT"},
+ /* Keyboard keys */
+ { 0x32, "KEY_ESC" },
+ { 0x33, "KEY_F1" },
+ { 0x34, "KEY_F2" },
+ { 0x35, "KEY_F3" },
+ { 0x36, "KEY_F4" },
+ { 0x37, "KEY_F5" },
+ { 0x38, "KEY_F6" },
+ { 0x39, "KEY_F7" },
+ { 0x3a, "KEY_F8" },
+ { 0x3b, "KEY_F9" },
+ { 0x3c, "KEY_F10" },
+ { 0x3d, "KEY_F11" },
+ { 0x3e, "KEY_F12" },
+ { 0x3f, "KEY_GRAVE" },
+ { 0x40, "KEY_1" },
+ { 0x41, "KEY_2" },
+ { 0x42, "KEY_3" },
+ { 0x43, "KEY_4" },
+ { 0x44, "KEY_5" },
+ { 0x45, "KEY_6" },
+ { 0x46, "KEY_7" },
+ { 0x47, "KEY_8" },
+ { 0x48, "KEY_9" },
+ { 0x49, "KEY_0" },
+ { 0x4a, "KEY_MINUS" },
+ { 0x4b, "KEY_EQUAL" },
+ { 0x4c, "KEY_BACKSPACE" },
+ { 0x4d, "KEY_TAB" },
+ { 0x4e, "KEY_Q" },
+ { 0x4f, "KEY_W" },
+ { 0x50, "KEY_E" },
+ { 0x51, "KEY_R" },
+ { 0x52, "KEY_T" },
+ { 0x53, "KEY_Y" },
+ { 0x54, "KEY_U" },
+ { 0x55, "KEY_I" },
+ { 0x56, "KEY_O" },
+ { 0x57, "KEY_P" },
+ { 0x58, "KEY_LEFTBRACE" },
+ { 0x59, "KEY_RIGHTBRACE" },
+ { 0x5a, "KEY_BACKSLASH" },
+ { 0x5b, "KEY_CAPSLOCK" },
+ { 0x5c, "KEY_A" },
+ { 0x5d, "KEY_S" },
+ { 0x5e, "KEY_D" },
+ { 0x5f, "KEY_F" },
+ { 0x60, "KEY_G" },
+ { 0x61, "KEY_H" },
+ { 0x62, "KEY_J" },
+ { 0x63, "KEY_K" },
+ { 0x64, "KEY_L" },
+ { 0x65, "KEY_SEMICOLON" },
+ { 0x66, "KEY_APOSTROPHE" },
+ { 0x67, "KEY_ENTER" },
+ { 0x68, "KEY_LEFTSHIFT" },
+ { 0x69, "KEY_Z" },
+ { 0x6a, "KEY_X" },
+ { 0x6b, "KEY_C" },
+ { 0x6c, "KEY_V" },
+ { 0x6d, "KEY_B" },
+ { 0x6e, "KEY_N" },
+ { 0x6f, "KEY_M" },
+ { 0x70, "KEY_COMMA" },
+ { 0x71, "KEY_DOT" },
+ { 0x72, "KEY_SLASH" },
+ { 0x73, "KEY_RIGHTSHIFT" },
+ { 0x74, "KEY_LEFTCTRL" },
+ { 0x75, "KEY_LEFTMETA" },
+ { 0x76, "KEY_LEFTALT" },
+ { 0x77, "KEY_SPACE" },
+ { 0x78, "KEY_RIGHTALT" },
+ { 0x79, "KEY_RIGHTCTRL" },
+ { 0x7a, "KEY_INSERT" },
+ { 0x7b, "KEY_HOME" },
+ { 0x7c, "KEY_PAGEUP" },
+ { 0x7d, "KEY_DELETE" },
+ { 0x7e, "KEY_END" },
+ { 0x7f, "KEY_PAGEDOWN" },
+ { 0x8a, "KEY_KPENTER" },
+ { 0x8b, "KEY_KP0" },
+ { 0x8c, "KEY_KP1" },
+ { 0x8d, "KEY_KP2" },
+ { 0x8e, "KEY_KP3" },
+ { 0x8f, "KEY_KP4" },
+ { 0x90, "KEY_KP5" },
+ { 0x91, "KEY_KP6" },
+ { 0x92, "KEY_KP7" },
+ { 0x93, "KEY_KP8" },
+ { 0x94, "KEY_KP9" },
+ { 0x95, "MD_PLAY" },
+ { 0x96, "MD_STOP" },
+ { 0x97, "MD_NEXT" },
+ { 0x98, "MD_PREV" },
+ { 0x99, "MD_VOL_UP" },
+ { 0x9a, "MD_VOL_DOWN" },
+ { 0x9b, "MD_VOL_MUTE" },
+ { 0x9c, "KEY_F23" },
+ /* Mouse events */
+ { 0xc8, "BTN_LEFT" },
+ { 0xc9, "BTN_MIDDLE" },
+ { 0xca, "BTN_RIGHT" },
+ { 0xcb, "BTN_SIDE" },
+ { 0xcc, "BTN_EXTRA" },
+ { 0xcd, "REL_WHEEL_UP" },
+ { 0xce, "REL_WHEEL_DOWN" },
+ { 0xff, "DISABLED" },
+};
+
+static const u16 button_mapping_addr_old[] = {
+ 0x007a, /* M1 */
+ 0x011f, /* M2 */
+};
+
+static const u16 button_mapping_addr_new[] = {
+ 0x00bb, /* M1 */
+ 0x0164, /* M2 */
+};
+
struct claw_command_report {
u8 report_id;
u8 padding[2];
@@ -87,24 +249,46 @@ struct claw_command_report {
u8 data[59];
} __packed;
+struct claw_profile_report {
+ u8 profile;
+ __be16 read_addr;
+} __packed;
+
+struct claw_mkey_report {
+ struct claw_profile_report;
+ u8 padding_0;
+ u8 padding_1;
+ u8 padding_2;
+ u8 codes[5];
+} __packed;
+
struct claw_drvdata {
/* MCU General Variables */
+ enum claw_profile_ack_pending profile_pending;
struct completion send_cmd_complete;
struct delayed_work cfg_resume;
struct delayed_work cfg_setup;
+ struct mutex profile_mutex; /* mutex for profile_pending calls */
+ spinlock_t profile_lock; /* Lock for profile_pending read/write */
struct hid_device *hdev;
struct mutex cfg_mutex; /* mutex for synchronous data */
+ struct mutex rom_mutex; /* mutex for SYNC_TO_ROM calls */
bool waiting_for_ack;
spinlock_t cmd_lock; /* Lock for cmd data read/write */
u8 waiting_cmd;
int cmd_status;
+ u16 bcd_device;
u8 ep;
/* Gamepad Variables */
enum claw_mkeys_function_index mkeys_function;
enum claw_gamepad_mode_index gamepad_mode;
+ u8 m1_codes[CLAW_KEYS_MAX];
+ u8 m2_codes[CLAW_KEYS_MAX];
bool gamepad_registered;
spinlock_t mode_lock; /* Lock for mode data read/write */
+ const u16 *bmap_addr;
+ bool bmap_support;
};
static int get_endpoint_address(struct hid_device *hdev)
@@ -136,6 +320,39 @@ static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
return 0;
}
+static int claw_profile_event(struct claw_drvdata *drvdata, struct claw_command_report *cmd_rep)
+{
+ enum claw_profile_ack_pending profile;
+ struct claw_mkey_report *mkeys;
+ u8 *codes, key;
+ int i;
+
+ scoped_guard(spinlock, &drvdata->profile_lock)
+ profile = drvdata->profile_pending;
+
+ switch (profile) {
+ case CLAW_M1_PENDING:
+ case CLAW_M2_PENDING:
+ key = (profile == CLAW_M1_PENDING) ? CLAW_KEY_M1 : CLAW_KEY_M2;
+ mkeys = (struct claw_mkey_report *)cmd_rep->data;
+ if (be16_to_cpu(mkeys->read_addr) != drvdata->bmap_addr[key])
+ return -EINVAL;
+ codes = (profile == CLAW_M1_PENDING) ? drvdata->m1_codes : drvdata->m2_codes;
+ for (i = 0; i < CLAW_KEYS_MAX; i++)
+ codes[i] = (mkeys->codes[i]);
+ break;
+ default:
+ dev_dbg(&drvdata->hdev->dev,
+ "Got profile event without changes pending from command: %x\n",
+ cmd_rep->cmd);
+ return -EINVAL;
+ }
+ scoped_guard(spinlock, &drvdata->profile_lock)
+ drvdata->profile_pending = CLAW_NO_PENDING;
+
+ return 0;
+}
+
static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
u8 *data, int size)
{
@@ -166,6 +383,19 @@ static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *repor
}
}
+ break;
+ case CLAW_COMMAND_TYPE_READ_PROFILE_ACK:
+ ret = claw_profile_event(drvdata, cmd_rep);
+
+ scoped_guard(spinlock, &drvdata->cmd_lock) {
+ if (drvdata->waiting_for_ack &&
+ drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_PROFILE) {
+ drvdata->cmd_status = ret;
+ drvdata->waiting_for_ack = false;
+ complete(&drvdata->send_cmd_complete);
+ }
+ }
+
break;
case CLAW_COMMAND_TYPE_ACK:
scoped_guard(spinlock, &drvdata->cmd_lock) {
@@ -433,6 +663,173 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
}
static DEVICE_ATTR_WO(reset);
+static int mkey_mapping_name_to_code(const char *name)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++) {
+ if (!strcmp(name, claw_button_mapping_key_map[i].name))
+ return claw_button_mapping_key_map[i].code;
+ }
+
+ return -EINVAL;
+}
+
+static const char *mkey_mapping_code_to_name(u8 code)
+{
+ int i;
+
+ if (code == 0xff)
+ return NULL;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++) {
+ if (claw_button_mapping_key_map[i].code == code)
+ return claw_button_mapping_key_map[i].name;
+ }
+
+ return NULL;
+}
+
+static int claw_mkey_store(struct device *dev, const char *buf, u8 mkey)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct claw_mkey_report report = { {0x01, cpu_to_be16(drvdata->bmap_addr[mkey])},
+ 0x07, 0x04, 0x00, {0xff, 0xff, 0xff, 0xff, 0xff} };
+ char **raw_keys __free(argv_free) = NULL;
+ int ret, key_count, i;
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+
+ raw_keys = argv_split(GFP_KERNEL, buf, &key_count);
+ if (!raw_keys)
+ return -ENOMEM;
+
+ if (key_count > CLAW_KEYS_MAX)
+ return -EINVAL;
+
+ if (key_count == 0)
+ goto set_buttons;
+
+ for (i = 0; i < key_count; i++) {
+ ret = mkey_mapping_name_to_code(raw_keys[i]);
+ if (ret < 0)
+ return ret;
+
+ report.codes[i] = ret;
+ }
+
+set_buttons:
+ scoped_guard(mutex, &drvdata->rom_mutex) {
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+ /* MCU will not send ACK until the USB transaction completes. ACK is sent
+ * immediately after and will hit the stale state machine, before the next
+ * command re-arms the state machine. Timeout 0 ensures no deadlock waiting
+ * for ACK that ill never come.
+ */
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SYNC_TO_ROM, NULL, 0, 0);
+ }
+
+ return ret;
+}
+
+static int claw_mkey_show(struct device *dev, char *buf, enum claw_key_index m_key)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ struct claw_mkey_report report = { {0x01, cpu_to_be16(drvdata->bmap_addr[m_key])}, 0x07 };
+ int i, ret, count = 0;
+ const char *name;
+ u8 *codes;
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+
+ codes = (m_key == CLAW_KEY_M1) ? drvdata->m1_codes : drvdata->m2_codes;
+
+ guard(mutex)(&drvdata->profile_mutex);
+ scoped_guard(spinlock_irqsave, &drvdata->profile_lock)
+ drvdata->profile_pending = (m_key == CLAW_KEY_M1) ? CLAW_M1_PENDING
+ : CLAW_M2_PENDING;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_PROFILE,
+ (u8 *)&report, sizeof(report), 25);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < CLAW_KEYS_MAX; i++) {
+ name = mkey_mapping_code_to_name(codes[i]);
+ if (name)
+ count += sysfs_emit_at(buf, count, "%s ", name);
+ }
+
+ if (!count)
+ return sysfs_emit(buf, "(not set)\n");
+
+ buf[count - 1] = '\n';
+
+ return count;
+}
+
+static ssize_t button_m1_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+
+ ret = claw_mkey_store(dev, buf, CLAW_KEY_M1);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t button_m1_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return claw_mkey_show(dev, buf, CLAW_KEY_M1);
+}
+static DEVICE_ATTR_RW(button_m1);
+
+static ssize_t button_m2_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ int ret;
+
+ ret = claw_mkey_store(dev, buf, CLAW_KEY_M2);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t button_m2_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ return claw_mkey_show(dev, buf, CLAW_KEY_M2);
+}
+static DEVICE_ATTR_RW(button_m2);
+
+static ssize_t button_mapping_options_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_button_mapping_key_map); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_button_mapping_key_map[i].name);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(button_mapping_options);
+
static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
int n)
{
@@ -445,10 +842,22 @@ static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribu
return 0;
}
- return attr->mode;
+ /* Always show attrs available on all firmware */
+ if (attr == &dev_attr_gamepad_mode.attr ||
+ attr == &dev_attr_gamepad_mode_index.attr ||
+ attr == &dev_attr_mkeys_function.attr ||
+ attr == &dev_attr_mkeys_function_index.attr ||
+ attr == &dev_attr_reset.attr)
+ return attr->mode;
+
+ /* Hide button mapping attrs if it isn't supported */
+ return drvdata->bmap_support ? attr->mode : 0;
}
static struct attribute *claw_gamepad_attrs[] = {
+ &dev_attr_button_m1.attr,
+ &dev_attr_button_m2.attr,
+ &dev_attr_button_mapping_options.attr,
&dev_attr_gamepad_mode.attr,
&dev_attr_gamepad_mode_index.attr,
&dev_attr_mkeys_function.attr,
@@ -499,8 +908,31 @@ static void cfg_resume_fn(struct work_struct *work)
schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
}
+static void claw_features_supported(struct claw_drvdata *drvdata)
+{
+ u8 major = (drvdata->bcd_device >> 8) & 0xff;
+ u8 minor = drvdata->bcd_device & 0xff;
+
+ if (major == 0x01) {
+ drvdata->bmap_support = true;
+ if (minor >= 0x66)
+ drvdata->bmap_addr = button_mapping_addr_new;
+ else
+ drvdata->bmap_addr = button_mapping_addr_old;
+ return;
+ }
+
+ if ((major == 0x02 && minor >= 0x17) || major >= 0x03) {
+ drvdata->bmap_support = true;
+ drvdata->bmap_addr = button_mapping_addr_new;
+ return;
+ }
+}
+
static int claw_probe(struct hid_device *hdev, u8 ep)
{
+ struct usb_interface *intf = to_usb_interface(hdev->dev.parent);
+ struct usb_device *udev = interface_to_usbdev(intf);
struct claw_drvdata *drvdata;
int ret;
@@ -512,9 +944,19 @@ static int claw_probe(struct hid_device *hdev, u8 ep)
drvdata->hdev = hdev;
drvdata->ep = ep;
+ /* Determine feature level from firmware version */
+ drvdata->bcd_device = le16_to_cpu(udev->descriptor.bcdDevice);
+ claw_features_supported(drvdata);
+
+ if (!drvdata->bmap_support)
+ dev_dbg(&hdev->dev, "M-Key mapping is not supported. Update firmware to enable.\n");
+
mutex_init(&drvdata->cfg_mutex);
+ mutex_init(&drvdata->profile_mutex);
+ mutex_init(&drvdata->rom_mutex);
spin_lock_init(&drvdata->cmd_lock);
spin_lock_init(&drvdata->mode_lock);
+ spin_lock_init(&drvdata->profile_lock);
init_completion(&drvdata->send_cmd_complete);
INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
--
2.53.0
^ permalink raw reply related
* [PATCH v7 1/4] HID: hid-msi: Add MSI Claw configuration driver
From: Derek J. Clark @ 2026-05-20 1:31 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
In-Reply-To: <20260520013158.3633277-1-derekjohn.clark@gmail.com>
Adds configuration HID driver for the MSI Claw series of handheld PC's.
In this initial patch add the initial driver outline and attributes for
changing the gamepad mode, M-key behavior, and add a WO reset function.
Sending the SWITCH_MODE and RESET commands causes a USB disconnect in
the device. The completion will therefore never get hit and would trigger
an -EIO. To avoid showing the user an error for every write to these
attrs a bypass for the completion handling is introduced when timeout ==
0.
The initial version of this patch was written by Denis Benato, which
contained the initial reverse-engineering and implementation for the
gamepad mode switching. This work was later expanded by Zhouwang Huang
to include more gamepad modes. Finally, I refactored the drivers data
in/out flow and overall format to conform to kernel driver best
practices and style guides. Claude was used as an initial reviewer of
this patch.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v7:
- Use smp_[store_release|load_acquire] pattern for checking
gamepad_registered to avoid possible races during teardown.
- Reorder reinit_completion in claw_hw_output_report to avoid race
with possible incoming ACKs.
- Reorder cancel_delayed_work_sync to ensure setup can't be re-armed
after cancel.
- Reset command state machine if hw_output_report has an error.
- Add comments to (hopefully) silence sashinko-bot warnings about the
use of endpoint matching and the impossible scenario of switching to
the alternate endpoint from userspace while the driver is bound.
- Don't use spinlock_irqsave when already in irq context.
v6:
- Add send/ack pattern to ensure synchronous acks.
- Use spinlock_irqsave instead of mutex for read/write MODE event
data.
- add select NEW_LEDS to kconfig.
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Gate all attribute show/store functions with gamepad_registered,
enabling use of devm_device_add_group.
- Re-arm cfg_setup in resume if it was canceled in an early suspend.
- Don't set gamepad_mode on resume, MCU preserves state.
- Ensure all count variables are checked for > 0 characters before
setting buf - 1 to \n.
v5:
- Swap disabled & combination mkeys_function enum values.
- Ensure mode_mutex is properly init.
- Ensure claw_remove is calling hid_hw_close and not hid_hw_stop for
all paths.
v4:
- Add msi_suspend/claw_suspend.
- Reorder claw_remove to cancel all work before removing sysfs.
- Add mutex lock for removing sysfs attributes.
- Add mutex lock for MODE command data read/write.
v3:
- Ensure claw_hw_output_report is properly guarded.
- Reoder claw_probe to ensure all mutex, completion, and variable
assignments are in place prior to setting drvdata.
- Ensure gamepad_mode is set to a valid enum value in claw_probe.
v2:
- Rename driver to hid-msi from hid-msi-claw.
- Rename reusable/generic functions to msi_* from claw_*, retaining
claw specific functions.
- Add generic entrypoints for probe, remove, and raw event that route
to claw specific functions.
---
MAINTAINERS | 6 +
drivers/hid/Kconfig | 13 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 5 +
drivers/hid/hid-msi.c | 692 ++++++++++++++++++++++++++++++++++++++++++
5 files changed, 717 insertions(+)
create mode 100644 drivers/hid/hid-msi.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 6f6517bf4f970..8e2de98b768f7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17965,6 +17965,12 @@ S: Odd Fixes
F: Documentation/devicetree/bindings/net/ieee802154/mrf24j40.txt
F: drivers/net/ieee802154/mrf24j40.c
+MSI HID DRIVER
+M: Derek J. Clark <derekjohn.clark@gmail.com>
+L: linux-input@vger.kernel.org
+S: Maintained
+F: drivers/hid/hid-msi.c
+
MSI EC DRIVER
M: Nikita Kravets <teackot@gmail.com>
L: platform-driver-x86@vger.kernel.org
diff --git a/drivers/hid/Kconfig b/drivers/hid/Kconfig
index 10c12d8e65579..7766676051a52 100644
--- a/drivers/hid/Kconfig
+++ b/drivers/hid/Kconfig
@@ -492,6 +492,19 @@ config HID_GT683R
Currently the following devices are know to be supported:
- MSI GT683R
+config HID_MSI
+ tristate "MSI Claw Gamepad Support"
+ depends on USB_HID
+ select NEW_LEDS
+ select LEDS_CLASS
+ select LEDS_CLASS_MULTICOLOR
+ help
+ Support for the MSI Claw RGB and controller configuration
+
+ Say Y here to include configuration interface support for the MSI Claw Line
+ of Handheld Console Controllers. Say M here to compile this driver as a
+ module. The module will be called hid-msi.
+
config HID_KEYTOUCH
tristate "Keytouch HID devices"
help
diff --git a/drivers/hid/Makefile b/drivers/hid/Makefile
index 07dfdb6a49c59..80925a17b059c 100644
--- a/drivers/hid/Makefile
+++ b/drivers/hid/Makefile
@@ -92,6 +92,7 @@ obj-$(CONFIG_HID_MAYFLASH) += hid-mf.o
obj-$(CONFIG_HID_MEGAWORLD_FF) += hid-megaworld.o
obj-$(CONFIG_HID_MICROSOFT) += hid-microsoft.o
obj-$(CONFIG_HID_MONTEREY) += hid-monterey.o
+obj-$(CONFIG_HID_MSI) += hid-msi.o
obj-$(CONFIG_HID_MULTITOUCH) += hid-multitouch.o
obj-$(CONFIG_HID_NINTENDO) += hid-nintendo.o
obj-$(CONFIG_HID_NTI) += hid-nti.o
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 933b7943bdb50..94a9b89dc240a 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -1047,7 +1047,12 @@
#define USB_DEVICE_ID_MOZA_R16_R21_2 0x0010
#define USB_VENDOR_ID_MSI 0x1770
+#define USB_VENDOR_ID_MSI_2 0x0db0
#define USB_DEVICE_ID_MSI_GT683R_LED_PANEL 0xff00
+#define USB_DEVICE_ID_MSI_CLAW_XINPUT 0x1901
+#define USB_DEVICE_ID_MSI_CLAW_DINPUT 0x1902
+#define USB_DEVICE_ID_MSI_CLAW_DESKTOP 0x1903
+#define USB_DEVICE_ID_MSI_CLAW_BIOS 0x1904
#define USB_VENDOR_ID_NATIONAL_SEMICONDUCTOR 0x0400
#define USB_DEVICE_ID_N_S_HARMONY 0xc359
diff --git a/drivers/hid/hid-msi.c b/drivers/hid/hid-msi.c
new file mode 100644
index 0000000000000..d95483907a5e5
--- /dev/null
+++ b/drivers/hid/hid-msi.c
@@ -0,0 +1,692 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * HID driver for MSI Claw Handheld PC gamepads.
+ *
+ * Provides configuration support for the MSI Claw series of handheld PC
+ * gamepads. Multiple iterations of the device firmware has led to some
+ * quirks for how certain attributes are handled. The original firmware
+ * did not support remapping of the M1 (right) and M2 (left) rear paddles.
+ * Additionally, the MCU RAM address for writing configuration data has
+ * changed twice. Checks are done during probe to enumerate these variances.
+ *
+ * Copyright (c) 2026 Zhouwang Huang <honjow311@gmail.com>
+ * Copyright (c) 2026 Denis Benato <denis.benato@linux.dev>
+ * Copyright (c) 2026 Valve Corporation
+ */
+
+#include <linux/array_size.h>
+#include <linux/cleanup.h>
+#include <linux/completion.h>
+#include <linux/container_of.h>
+#include <linux/device.h>
+#include <linux/hid.h>
+#include <linux/kobject.h>
+#include <linux/leds.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/pm.h>
+#include <linux/spinlock.h>
+#include <linux/sysfs.h>
+#include <linux/types.h>
+#include <linux/unaligned.h>
+#include <linux/usb.h>
+#include <linux/workqueue.h>
+
+#include "hid-ids.h"
+
+#define CLAW_OUTPUT_REPORT_ID 0x0f
+#define CLAW_INPUT_REPORT_ID 0x10
+
+#define CLAW_PACKET_SIZE 64
+
+#define CLAW_DINPUT_CFG_INTF_IN 0x82
+#define CLAW_XINPUT_CFG_INTF_IN 0x83
+
+enum claw_command_index {
+ CLAW_COMMAND_TYPE_NONE = 0x00,
+ CLAW_COMMAND_TYPE_READ_PROFILE = 0x04,
+ CLAW_COMMAND_TYPE_READ_PROFILE_ACK = 0x05,
+ CLAW_COMMAND_TYPE_ACK = 0x06,
+ CLAW_COMMAND_TYPE_WRITE_PROFILE_DATA = 0x21,
+ CLAW_COMMAND_TYPE_SYNC_TO_ROM = 0x22,
+ CLAW_COMMAND_TYPE_SWITCH_MODE = 0x24,
+ CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE = 0x26,
+ CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK = 0x27,
+ CLAW_COMMAND_TYPE_RESET_DEVICE = 0x28,
+};
+
+enum claw_gamepad_mode_index {
+ CLAW_GAMEPAD_MODE_XINPUT = 0x01,
+ CLAW_GAMEPAD_MODE_DINPUT = 0x02,
+ CLAW_GAMEPAD_MODE_DESKTOP = 0x04,
+};
+
+static const char * const claw_gamepad_mode_text[] = {
+ [CLAW_GAMEPAD_MODE_XINPUT] = "xinput",
+ [CLAW_GAMEPAD_MODE_DINPUT] = "dinput",
+ [CLAW_GAMEPAD_MODE_DESKTOP] = "desktop",
+};
+
+enum claw_mkeys_function_index {
+ CLAW_MKEY_FUNCTION_MACRO,
+ CLAW_MKEY_FUNCTION_DISABLED,
+ CLAW_MKEY_FUNCTION_COMBO,
+};
+
+static const char * const claw_mkeys_function_text[] = {
+ [CLAW_MKEY_FUNCTION_MACRO] = "macro",
+ [CLAW_MKEY_FUNCTION_DISABLED] = "disabled",
+ [CLAW_MKEY_FUNCTION_COMBO] = "combination",
+};
+
+struct claw_command_report {
+ u8 report_id;
+ u8 padding[2];
+ u8 header_tail;
+ u8 cmd;
+ u8 data[59];
+} __packed;
+
+struct claw_drvdata {
+ /* MCU General Variables */
+ struct completion send_cmd_complete;
+ struct delayed_work cfg_resume;
+ struct delayed_work cfg_setup;
+ struct hid_device *hdev;
+ struct mutex cfg_mutex; /* mutex for synchronous data */
+ bool waiting_for_ack;
+ spinlock_t cmd_lock; /* Lock for cmd data read/write */
+ u8 waiting_cmd;
+ int cmd_status;
+ u8 ep;
+
+ /* Gamepad Variables */
+ enum claw_mkeys_function_index mkeys_function;
+ enum claw_gamepad_mode_index gamepad_mode;
+ bool gamepad_registered;
+ spinlock_t mode_lock; /* Lock for mode data read/write */
+};
+
+static int get_endpoint_address(struct hid_device *hdev)
+{
+ struct usb_host_endpoint *ep;
+ struct usb_interface *intf;
+
+ intf = to_usb_interface(hdev->dev.parent);
+ ep = intf->cur_altsetting->endpoint;
+ if (ep)
+ return ep->desc.bEndpointAddress;
+
+ return -ENODEV;
+}
+
+static int claw_gamepad_mode_event(struct claw_drvdata *drvdata,
+ struct claw_command_report *cmd_rep)
+{
+ if (cmd_rep->data[0] >= ARRAY_SIZE(claw_gamepad_mode_text) ||
+ !claw_gamepad_mode_text[cmd_rep->data[0]] ||
+ cmd_rep->data[1] >= ARRAY_SIZE(claw_mkeys_function_text))
+ return -EINVAL;
+
+ scoped_guard(spinlock, &drvdata->mode_lock) {
+ drvdata->gamepad_mode = cmd_rep->data[0];
+ drvdata->mkeys_function = cmd_rep->data[1];
+ }
+
+ return 0;
+}
+
+static int claw_raw_event(struct claw_drvdata *drvdata, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct claw_command_report *cmd_rep;
+ int ret = 0;
+
+ if (size != CLAW_PACKET_SIZE)
+ return 0;
+
+ cmd_rep = (struct claw_command_report *)data;
+
+ if (cmd_rep->report_id != CLAW_INPUT_REPORT_ID || cmd_rep->header_tail != 0x3c)
+ return 0;
+
+ dev_dbg(&drvdata->hdev->dev, "Rx data as raw input report: [%*ph]\n",
+ CLAW_PACKET_SIZE, data);
+
+ switch (cmd_rep->cmd) {
+ case CLAW_COMMAND_TYPE_GAMEPAD_MODE_ACK:
+ ret = claw_gamepad_mode_event(drvdata, cmd_rep);
+
+ scoped_guard(spinlock, &drvdata->cmd_lock) {
+ if (drvdata->waiting_for_ack &&
+ drvdata->waiting_cmd == CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE) {
+ drvdata->cmd_status = ret;
+ drvdata->waiting_for_ack = false;
+ complete(&drvdata->send_cmd_complete);
+ }
+ }
+
+ break;
+ case CLAW_COMMAND_TYPE_ACK:
+ scoped_guard(spinlock, &drvdata->cmd_lock) {
+ if (drvdata->waiting_for_ack) {
+ drvdata->cmd_status = 0;
+ drvdata->waiting_for_ack = false;
+ complete(&drvdata->send_cmd_complete);
+ }
+ dev_dbg(&drvdata->hdev->dev, "Waiting CMD: %x\n", drvdata->waiting_cmd);
+ }
+
+ break;
+ default:
+ dev_dbg(&drvdata->hdev->dev, "Unknown command: %x\n", cmd_rep->cmd);
+ return 0;
+ }
+
+ return ret;
+}
+
+static int msi_raw_event(struct hid_device *hdev, struct hid_report *report,
+ u8 *data, int size)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata || (drvdata->ep != CLAW_XINPUT_CFG_INTF_IN &&
+ drvdata->ep != CLAW_DINPUT_CFG_INTF_IN))
+ return 0;
+
+ return claw_raw_event(drvdata, report, data, size);
+}
+
+static int claw_hw_output_report(struct hid_device *hdev, u8 index, u8 *data,
+ size_t len, unsigned int timeout)
+{
+ unsigned char *dmabuf __free(kfree) = NULL;
+ u8 header[] = { CLAW_OUTPUT_REPORT_ID, 0, 0, 0x3c, index };
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ size_t header_size = ARRAY_SIZE(header);
+ int ret;
+
+ if (header_size + len > CLAW_PACKET_SIZE)
+ return -EINVAL;
+
+ /* We can't use a devm_alloc reusable buffer without side effects during suspend */
+ dmabuf = kzalloc(CLAW_PACKET_SIZE, GFP_KERNEL);
+ if (!dmabuf)
+ return -ENOMEM;
+
+ memcpy(dmabuf, header, header_size);
+ if (data && len)
+ memcpy(dmabuf + header_size, data, len);
+
+ guard(mutex)(&drvdata->cfg_mutex);
+ if (timeout) {
+ reinit_completion(&drvdata->send_cmd_complete);
+ scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
+ drvdata->waiting_cmd = index;
+ drvdata->waiting_for_ack = true;
+ drvdata->cmd_status = -ETIMEDOUT;
+ }
+ }
+
+ dev_dbg(&hdev->dev, "Send data as raw output report: [%*ph]\n",
+ CLAW_PACKET_SIZE, dmabuf);
+
+ ret = hid_hw_output_report(hdev, dmabuf, CLAW_PACKET_SIZE);
+ if (ret < 0)
+ return ret;
+
+ ret = ret == CLAW_PACKET_SIZE ? 0 : -EIO;
+ if (ret) {
+ scoped_guard(spinlock_irqsave, &drvdata->cmd_lock) {
+ drvdata->waiting_cmd = CLAW_COMMAND_TYPE_NONE;
+ drvdata->waiting_for_ack = false;
+ }
+ return ret;
+ }
+
+ if (timeout) {
+ ret = wait_for_completion_interruptible_timeout(&drvdata->send_cmd_complete,
+ msecs_to_jiffies(timeout));
+
+ dev_dbg(&hdev->dev, "Remaining timeout: %u\n", ret);
+ ret = ret > 0 ? drvdata->cmd_status : ret ?: -EBUSY;
+ scoped_guard(spinlock_irqsave, &drvdata->cmd_lock)
+ drvdata->waiting_for_ack = false;
+ }
+
+ return ret;
+}
+
+static ssize_t gamepad_mode_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int i, ret = -EINVAL;
+ u8 data[2];
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+
+ for (i = 0; i < ARRAY_SIZE(claw_gamepad_mode_text); i++) {
+ if (claw_gamepad_mode_text[i] && sysfs_streq(buf, claw_gamepad_mode_text[i])) {
+ ret = i;
+ break;
+ }
+ }
+ if (ret < 0)
+ return ret;
+
+ data[0] = ret;
+ scoped_guard(spinlock_irqsave, &drvdata->mode_lock)
+ data[1] = drvdata->mkeys_function;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data, ARRAY_SIZE(data), 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t gamepad_mode_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret, i;
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 25);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->mode_lock)
+ i = drvdata->gamepad_mode;
+
+ if (!claw_gamepad_mode_text[i] || claw_gamepad_mode_text[i][0] == '\0')
+ return sysfs_emit(buf, "unsupported\n");
+
+ return sysfs_emit(buf, "%s\n", claw_gamepad_mode_text[i]);
+}
+static DEVICE_ATTR_RW(gamepad_mode);
+
+static ssize_t gamepad_mode_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ ssize_t count = 0;
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(claw_gamepad_mode_text); i++) {
+ if (!claw_gamepad_mode_text[i] || claw_gamepad_mode_text[i][0] == '\0')
+ continue;
+ count += sysfs_emit_at(buf, count, "%s ", claw_gamepad_mode_text[i]);
+ }
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(gamepad_mode_index);
+
+static ssize_t mkeys_function_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int i, ret = -EINVAL;
+ u8 data[2];
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+
+ for (i = 0; i < ARRAY_SIZE(claw_mkeys_function_text); i++) {
+ if (claw_mkeys_function_text[i] && sysfs_streq(buf, claw_mkeys_function_text[i])) {
+ ret = i;
+ break;
+ }
+ }
+ if (ret < 0)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->mode_lock)
+ data[0] = drvdata->gamepad_mode;
+ data[1] = ret;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_SWITCH_MODE, data, ARRAY_SIZE(data), 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+
+static ssize_t mkeys_function_show(struct device *dev, struct device_attribute *attr,
+ char *buf)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ int ret, i;
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE, NULL, 0, 25);
+ if (ret)
+ return ret;
+
+ scoped_guard(spinlock_irqsave, &drvdata->mode_lock)
+ i = drvdata->mkeys_function;
+
+ if (i >= ARRAY_SIZE(claw_mkeys_function_text))
+ return sysfs_emit(buf, "unsupported\n");
+
+ return sysfs_emit(buf, "%s\n", claw_mkeys_function_text[i]);
+}
+static DEVICE_ATTR_RW(mkeys_function);
+
+static ssize_t mkeys_function_index_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ int i, count = 0;
+
+ for (i = 0; i < ARRAY_SIZE(claw_mkeys_function_text); i++)
+ count += sysfs_emit_at(buf, count, "%s ", claw_mkeys_function_text[i]);
+
+ if (count)
+ buf[count - 1] = '\n';
+
+ return count;
+}
+static DEVICE_ATTR_RO(mkeys_function_index);
+
+static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct hid_device *hdev = to_hid_device(dev);
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+ bool val;
+ int ret;
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ return -ENODEV;
+
+ ret = kstrtobool(buf, &val);
+ if (ret)
+ return ret;
+
+ if (!val)
+ return -EINVAL;
+
+ ret = claw_hw_output_report(hdev, CLAW_COMMAND_TYPE_RESET_DEVICE, NULL, 0, 0);
+ if (ret)
+ return ret;
+
+ return count;
+}
+static DEVICE_ATTR_WO(reset);
+
+static umode_t claw_gamepad_attr_is_visible(struct kobject *kobj, struct attribute *attr,
+ int n)
+{
+ struct hid_device *hdev = to_hid_device(kobj_to_dev(kobj));
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata) {
+ dev_warn(&hdev->dev,
+ "Failed to get drvdata from kobj. Gamepad attributes are not available.\n");
+ return 0;
+ }
+
+ return attr->mode;
+}
+
+static struct attribute *claw_gamepad_attrs[] = {
+ &dev_attr_gamepad_mode.attr,
+ &dev_attr_gamepad_mode_index.attr,
+ &dev_attr_mkeys_function.attr,
+ &dev_attr_mkeys_function_index.attr,
+ &dev_attr_reset.attr,
+ NULL,
+};
+
+static const struct attribute_group claw_gamepad_attr_group = {
+ .attrs = claw_gamepad_attrs,
+ .is_visible = claw_gamepad_attr_is_visible,
+};
+
+static void cfg_setup_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_setup);
+ int ret;
+
+ ret = claw_hw_output_report(drvdata->hdev, CLAW_COMMAND_TYPE_READ_GAMEPAD_MODE,
+ NULL, 0, 25);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't read gamepad mode: %d\n", ret);
+ return;
+ }
+
+ /* Add sysfs attributes after we get the device state */
+ ret = devm_device_add_group(&drvdata->hdev->dev, &claw_gamepad_attr_group);
+ if (ret) {
+ dev_err(&drvdata->hdev->dev,
+ "Failed to setup device, can't create gamepad attrs: %d\n", ret);
+ return;
+ }
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->gamepad_registered, true);
+
+ kobject_uevent(&drvdata->hdev->dev.kobj, KOBJ_CHANGE);
+}
+
+static void cfg_resume_fn(struct work_struct *work)
+{
+ struct delayed_work *dwork = container_of(work, struct delayed_work, work);
+ struct claw_drvdata *drvdata = container_of(dwork, struct claw_drvdata, cfg_resume);
+
+ /* Pairs with smp_store_release from cfg_setup_fn in system_wq context */
+ if (!smp_load_acquire(&drvdata->gamepad_registered))
+ schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
+}
+
+static int claw_probe(struct hid_device *hdev, u8 ep)
+{
+ struct claw_drvdata *drvdata;
+ int ret;
+
+ drvdata = devm_kzalloc(&hdev->dev, sizeof(*drvdata), GFP_KERNEL);
+ if (!drvdata)
+ return -ENOMEM;
+
+ drvdata->gamepad_mode = CLAW_GAMEPAD_MODE_XINPUT;
+ drvdata->hdev = hdev;
+ drvdata->ep = ep;
+
+ mutex_init(&drvdata->cfg_mutex);
+ spin_lock_init(&drvdata->cmd_lock);
+ spin_lock_init(&drvdata->mode_lock);
+ init_completion(&drvdata->send_cmd_complete);
+ INIT_DELAYED_WORK(&drvdata->cfg_resume, &cfg_resume_fn);
+ INIT_DELAYED_WORK(&drvdata->cfg_setup, &cfg_setup_fn);
+
+ /* For control interface: open the HID transport for sending commands. */
+ ret = hid_hw_open(hdev);
+ if (ret)
+ return ret;
+
+ hid_set_drvdata(hdev, drvdata);
+ schedule_delayed_work(&drvdata->cfg_setup, msecs_to_jiffies(500));
+
+ return 0;
+}
+
+static int msi_probe(struct hid_device *hdev, const struct hid_device_id *id)
+{
+ int ret;
+ u8 ep;
+
+ if (!hid_is_usb(hdev)) {
+ ret = -ENODEV;
+ goto err_probe;
+ }
+
+ ret = hid_parse(hdev);
+ if (ret)
+ goto err_probe;
+
+ /* Set quirk to create separate input devices per HID application */
+ hdev->quirks |= HID_QUIRK_INPUT_PER_APP | HID_QUIRK_MULTI_INPUT;
+ ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (ret)
+ goto err_probe;
+
+ /* For non-control interfaces (keyboard/mouse), allow userspace to grab the devices. */
+ ret = get_endpoint_address(hdev);
+ if (ret < 0)
+ goto err_stop_hw;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN) {
+ ret = claw_probe(hdev, ep);
+ if (ret)
+ goto err_stop_hw;
+ }
+
+ return 0;
+
+err_stop_hw:
+ hid_hw_stop(hdev);
+err_probe:
+ return dev_err_probe(&hdev->dev, ret, "Failed to init device\n");
+}
+
+static void claw_remove(struct hid_device *hdev)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata) {
+ hid_hw_close(hdev);
+ return;
+ }
+
+ cancel_delayed_work_sync(&drvdata->cfg_resume);
+ cancel_delayed_work_sync(&drvdata->cfg_setup);
+
+ /* Pairs with smp_load_acquire in attribute show/store functions */
+ smp_store_release(&drvdata->gamepad_registered, false);
+
+ hid_hw_close(hdev);
+}
+
+static void msi_remove(struct hid_device *hdev)
+{
+ int ret;
+ u8 ep;
+
+ /* Safe assumption. SET_INTERFACE ioctl can't be used while driver is bound */
+ ret = get_endpoint_address(hdev);
+ if (ret <= 0)
+ goto hw_stop;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN)
+ claw_remove(hdev);
+
+hw_stop:
+ hid_hw_stop(hdev);
+}
+
+static int claw_resume(struct hid_device *hdev)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata)
+ return -ENODEV;
+
+ /* MCU can take up to 500ms to be ready after resume */
+ schedule_delayed_work(&drvdata->cfg_resume, msecs_to_jiffies(500));
+ return 0;
+}
+
+static int msi_resume(struct hid_device *hdev)
+{
+ int ret;
+ u8 ep;
+
+ /* Safe assumption. SET_INTERFACE ioctl can't be used while driver is bound */
+ ret = get_endpoint_address(hdev);
+ if (ret <= 0)
+ return 0;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN)
+ return claw_resume(hdev);
+
+ return 0;
+}
+
+static int claw_suspend(struct hid_device *hdev)
+{
+ struct claw_drvdata *drvdata = hid_get_drvdata(hdev);
+
+ if (!drvdata)
+ return -ENODEV;
+
+ cancel_delayed_work_sync(&drvdata->cfg_resume);
+ cancel_delayed_work_sync(&drvdata->cfg_setup);
+
+ return 0;
+}
+
+static int msi_suspend(struct hid_device *hdev, pm_message_t msg)
+{
+ int ret;
+ u8 ep;
+
+ /* Safe assumption. SET_INTERFACE ioctl can't be used while driver is bound */
+ ret = get_endpoint_address(hdev);
+ if (ret <= 0)
+ return 0;
+
+ ep = ret;
+ if (ep == CLAW_XINPUT_CFG_INTF_IN || ep == CLAW_DINPUT_CFG_INTF_IN)
+ return claw_suspend(hdev);
+
+ return 0;
+}
+
+static const struct hid_device_id msi_devices[] = {
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_XINPUT) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_DINPUT) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_DESKTOP) },
+ { HID_USB_DEVICE(USB_VENDOR_ID_MSI_2, USB_DEVICE_ID_MSI_CLAW_BIOS) },
+ { }
+};
+MODULE_DEVICE_TABLE(hid, msi_devices);
+
+static struct hid_driver msi_driver = {
+ .name = "hid-msi",
+ .id_table = msi_devices,
+ .raw_event = msi_raw_event,
+ .probe = msi_probe,
+ .remove = msi_remove,
+ .resume = msi_resume,
+ .suspend = pm_ptr(msi_suspend),
+};
+module_hid_driver(msi_driver);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Denis Benato <denis.benato@linux.dev>");
+MODULE_AUTHOR("Zhouwang Huang <honjow311@gmail.com>");
+MODULE_AUTHOR("Derek J. Clark <derekjohn.clark@gmail.com>");
+MODULE_DESCRIPTION("HID driver for MSI Claw Handheld PC gamepads");
--
2.53.0
^ permalink raw reply related
* [PATCH v7 0/4] Add MSI Claw HID Configuration Driver
From: Derek J. Clark @ 2026-05-20 1:31 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Pierre-Loup A . Griffais, Denis Benato, Zhouwang Huang,
Derek J . Clark, linux-input, linux-doc, linux-kernel
This series adds an HID Configuration driver for the MSI Claw line of
Handheld Gaming PC's. The MSI Claw HID interface provides multiple
features, such as the ability to switch between xinput, dinput, and a
desktop mode, RGB control, rumble intensity, and mapping of the rear "M"
keys. There are additional gamepad modes that are not included in this
driver as they appear to be used in assembly line testing or are
incomplete in the firmware. During my testing I found them to be unstable.
The initial version of this driver was written by Denis Benato, which
contained the initial reverse-engineering and implementation for the
gamepad mode switching. This work was later expanded by Zhouwang Huang
to include more gamepad modes and additional features. Finally, I
refactored the entire driver, fixed multiple bugs, and refined the overall
format to conform to kernel driver best practices and style guide.
Claude was used initially by Zhouwang Huang to quickly parse HID captures
during the reverse-engineering of some of the features. Since Claude had
already been used, as a test of its capabilities I had it implement the
rumble intensity attribute after I had already rewritten most of the
driver, which I then manually edited to fix some mistakes. I also used
Claude to review the driver and these patches for any mistakes and bugs.
Assisted-by: Claude:claude-sonnet-4-6
Co-developed-by: Denis Benato <denis.benato@linux.dev>
Signed-off-by: Denis Benato <denis.benato@linux.dev>
Co-developed-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Zhouwang Huang <honjow311@gmail.com>
Signed-off-by: Derek J. Clark <derekjohn.clark@gmail.com>
---
v7:
- Use smp_[store_release|load_acquire] pattern for checking
gamepad_registered and rgb_registered to avoid possible races during
teardown.
- Reorder reinit_completion in claw_hw_output_report to avoid race
with possible incoming ACKs.
- Reorder cancel_delayed_work_sync to ensure setup can't be re-armed
after cancel.
- Reset command state machine if hw_output_report has an error.
- Add comments to (hopefully) silence sashinko-bot warnings about the
use of endpoint matching and the impossible scenario of switching to
the alternate endpoint from userspace while the driver is bound.
- Don't use spinlock_irqsave when already in irq context.
- Add profile_lock for read/write profile_pending.
- Use struct for mkey reports and rumble reports, following the
pattern established by rgb reports previously.
- Add gating to cfg_setup_fn, allowing either gamepad settings or rgb
settings to populate if the other fails for any reason.
- Match on write address for rumble and mkey reports to prevent late
ACK from causing synchronization errors.
v6: https://lore.kernel.org/linux-input/20260518222935.1802071-1-derekjohn.clark@gmail.com/
- Add send/ack pattern to ensure synchronous acks.
- Use spinlock_irqsave instead of mutex for read/write MODE event
data.
- add select NEW_LEDS to kconfig.
- Make all timeouts 25ms to ensure at least 2 jiffies in a 100Hz
config.
- Gate all attribute show/store functions with gamepad_registered or
rgb_registered, enabling use of devm_device_add_group and ending
the need to hold a mutex during remove.
- Don't set gamepad_mode on resume, MCU preserves state.
- Ensure all count variables are checked for > 0 characters before
setting buf - 1 to \n.
- Re-arm cfg_setup in resume if it was canceled in an early suspend.
- Remove duplicated argv_free macro.
- Add spinlock_irqsave vice mutex for read/write access on attribute
variables.
v5: https://lore.kernel.org/linux-input/20260517013925.3120314-1-derekjohn.clark@gmail.com/
- Swap disabled & combination mkeys_function enum values.
- Fix bug introduced in v5 where claw_buttons_store would return
-EINVAL on all valid key entries.
- Ensure mode_mutex is properly init.
- Ensure claw_remove is calling hid_hw_close and not hid_hw_stop for
all paths.
- Ensure adding "DISABLED" key to valid entries is done in the correct
patch.
- Re-enable sending an empty string to clear button mappings in
addition to setting DISABLED.
- Move adding the RGB device into cfg_setup to prevent led core
attributes from being written to prior to setup completing.
- Ensure frame_lock is properly init.
- Change variable names in RGB functions from frame and zone to f and
z respectively to fit all scoped_guard actions in 100 columns.
v4: https://lore.kernel.org/linux-input/20260516042841.500299-1-derekjohn.clark@gmail.com/
- Add msi_suspend/claw_suspend.
- Reorder claw_remove to cancel all work before removing sysfs.
- Add mutex lock for removing sysfs attributes.
- Add mutex lock for MODE command data read/write.
- Change dev_warn to dev_dbg in claw_profile_event.
- use __free with DEFINE_FREE macro for argv instead of manually
running argv_free, cleaining up scoped_guard goto.
- Fix frame_calc validity check to use >=.
- Use spinlock instead of mutex in raw_event and related attribute
_store function.
- Ensure delayed work is canceled in suspend & canceled before sysfs
attribute removal.
v3: https://lore.kernel.org/linux-input/20260515033622.2095277-1-derekjohn.clark@gmail.com/
- Add mutex for read/write if rgb frame data.
- Ensure claw_hw_output_report is properly guarded.
- Remove setting rgb_frame_count when reading rgb profiles as it always
returns garbage data.
- Ensure rgb_speed is getting drvdata from a valid lookup (not hdev).
- Use scoped_guard where necessary.
- Reoder claw_probe to ensure all mutex, completion, and variable
assignments are in place prior to setting drvdata.
- Ensure gamepad_mode is set to a valid enum value in claw_probe.
v2: https://lore.kernel.org/linux-input/20260513231445.3213501-1-derekjohn.clark@gmail.com/
- Use mutexes to guard SYNC_TO_ROM calls and pending_profile calls.
- Rename driver to hid-msi and add generic entrypoints for
probe/resume/remove that call claw specific functions in order to
future proof the driver for other MSI HID interfaces.
- Fix various bugs and formatting issues.
v1: https://lore.kernel.org/linux-input/20260510043510.442807-1-derekjohn.clark@gmail.com/
Derek J. Clark (4):
HID: hid-msi: Add MSI Claw configuration driver
HID: hid-msi: Add M-key mapping attributes
HID: hid-msi: Add RGB control interface
HID: hid-msi: Add Rumble Intensity Attributes
MAINTAINERS | 6 +
drivers/hid/Kconfig | 13 +
drivers/hid/Makefile | 1 +
drivers/hid/hid-ids.h | 5 +
drivers/hid/hid-msi.c | 1911 +++++++++++++++++++++++++++++++++++++++++
5 files changed, 1936 insertions(+)
create mode 100644 drivers/hid/hid-msi.c
--
2.53.0
^ permalink raw reply
* Re: [PATCH v4 6/6] iio: pressure: HID: hid-sensor-press: Refactor channel initialization
From: sashiko-bot @ 2026-05-20 1:14 UTC (permalink / raw)
To: Natália Salvino André; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260519234752.23911-7-natalia.andre@ime.usp.br>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The commit introduces a compilation error by using the incorrect field name `.format` instead of `.sign` when initializing `struct iio_scan_type`.
--
commit 8d524ba8c95e026dee3af23dc0139fc49731c194
Author: Natália Salvino André <natalia.andre@ime.usp.br>
iio: pressure: HID: hid-sensor-press: Refactor channel initialization
This commit replaces the local press_adjust_channel_bit_mask() function with a
compound literal for scan_type initialization to improve code readability.
> diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
> index 5f1d6abda3e40..a039b99d9851d 100644
> --- a/drivers/iio/pressure/hid-sensor-press.c
> +++ b/drivers/iio/pressure/hid-sensor-press.c
[ ... ]
> @@ -225,8 +215,11 @@ static int press_parse_report(struct platform_device *pdev,
> &st->press_attr);
> if (ret < 0)
> return ret;
> - press_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_PRESSURE,
> - st->press_attr.size);
> + channels[CHANNEL_SCAN_INDEX_PRESSURE].scan_type = (struct iio_scan_type) {
> + .format = 's',
[Severity: Low]
Does this introduce a build failure?
Looking at struct iio_scan_type defined in include/linux/iio/iio.h, the
member used to indicate the sign of the sample is named sign, rather than
format. The original code being removed correctly assigned to scan_type.sign.
> + .realbits = BYTES_TO_BITS(st->press_attr.size),
> + .storagebits = BITS_PER_TYPE(u32),
> + };
>
> dev_dbg(&pdev->dev, "press %x:%x\n", st->press_attr.index,
> st->press_attr.report_id);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519234752.23911-1-natalia.andre@ime.usp.br?part=6
^ permalink raw reply
* Re: [PATCH v4 5/6] iio: magnetometer: HID: hid-sensor-magn-3d: Refactor channel initialization
From: sashiko-bot @ 2026-05-20 1:01 UTC (permalink / raw)
To: Natália Salvino André; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260519234752.23911-6-natalia.andre@ime.usp.br>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Invalid struct member name '.format' used in compound literal for struct iio_scan_type.
--
commit a348ae7b1670bf238471e69df7f224b48282264e
Author: Natália Salvino André <natalia.andre@ime.usp.br>
iio: magnetometer: HID: hid-sensor-magn-3d: Refactor channel initialization
This commit replaces the local magn_3d_adjust_channel_bit_mask() function
with a compound literal for scan_type initialization to improve code
readability.
> diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> index c673f9323e476..3b26dc41dd4cf 100644
> --- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
> +++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
[ ... ]
> @@ -418,9 +408,11 @@ static int magn_3d_parse_report(struct platform_device *pdev,
> if (i != CHANNEL_SCAN_INDEX_TIMESTAMP) {
> /* Set magn_val_addr to iio value address */
> st->magn_val_addr[i] = &st->iio_vals[*chan_count];
> - magn_3d_adjust_channel_bit_mask(_channels,
> - *chan_count,
> - st->magn[i].size);
> + _channels[*chan_count].scan_type = (struct iio_scan_type) {
> + .format = 's',
[Severity: Low]
Will this cause a build failure? Looking at struct iio_scan_type, there
is no .format member. The correct field for the sign indicator appears to
be .sign, which was used in the original magn_3d_adjust_channel_bit_mask()
function.
> + .realbits = BYTES_TO_BITS(st->magn[i].size),
> + .storagebits = BITS_PER_TYPE(u32),
> + };
> }
> (*chan_count)++;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519234752.23911-1-natalia.andre@ime.usp.br?part=5
^ permalink raw reply
* Re: [PATCH v4 4/6] iio: light: HID: hid-sensor-prox: Refactor channel initialization
From: sashiko-bot @ 2026-05-20 0:49 UTC (permalink / raw)
To: Natália Salvino André; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260519234752.23911-5-natalia.andre@ime.usp.br>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch introduces a compiler error by using an invalid member name `.format` in a compound literal for `struct iio_scan_type`.
--
commit 79e0a9b7358ae7d1e9e80ed657afdcf93d6d8469
Author: Natália Salvino André <natalia.andre@ime.usp.br>
iio: light: HID: hid-sensor-prox: Refactor channel initialization
This commit replaces the local prox_adjust_channel_bit_mask() function
with a compound literal for scan_type initialization. It aims to improve
code readability by directly assigning the struct fields.
> diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
> index efa904a70d0ef..edc9274a2c076 100644
> --- a/drivers/iio/light/hid-sensor-prox.c
> +++ b/drivers/iio/light/hid-sensor-prox.c
[ ... ]
> @@ -250,8 +240,11 @@ static int prox_parse_report(struct platform_device *pdev,
> st->scan_mask[0] |= BIT(index);
> channels[index] = prox_channels[i];
> channels[index].scan_index = index;
> - prox_adjust_channel_bit_mask(channels, index,
> - st->prox_attr[index].size);
> + channels[index].scan_type = (struct iio_scan_type) {
> + .format = 's',
[Severity: Low]
Will this cause a compiler error? struct iio_scan_type does not appear to
have a member named format. The previous code set the sign field to 's',
so perhaps this should be .sign instead of .format?
> + .realbits = BYTES_TO_BITS(st->prox_attr[index].size),
> + .storagebits = BITS_PER_TYPE(u32),
> + };
> dev_dbg(&pdev->dev, "prox %x:%x\n", st->prox_attr[index].index,
> st->prox_attr[index].report_id);
> st->scale_precision[index] =
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519234752.23911-1-natalia.andre@ime.usp.br?part=4
^ permalink raw reply
* Re: [PATCH v4 3/6] iio: light: HID: hid-sensor-als: Refactor channel initialization
From: sashiko-bot @ 2026-05-20 0:38 UTC (permalink / raw)
To: Natália Salvino André; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260519234752.23911-4-natalia.andre@ime.usp.br>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Incorrect member name `.format` used in `struct iio_scan_type` initialization.
--
commit 6e85d51443d3e127d310915780d0c6f113e03de0
Author: Natália Salvino André <natalia.andre@ime.usp.br>
iio: light: HID: hid-sensor-als: Refactor channel initialization
This patch refactors the channel initialization by replacing the local
function als_adjust_channel_bit_mask() with a compound literal to
initialize scan_type.
> diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
> index 830e5ae7f34a7..e5b4c5e80f1cf 100644
> --- a/drivers/iio/light/hid-sensor-als.c
> +++ b/drivers/iio/light/hid-sensor-als.c
[ ... ]
> @@ -334,7 +324,11 @@ static int als_parse_report(struct platform_device *pdev,
>
> channels[index] = als_channels[i];
> st->als_scan_mask[0] |= BIT(i);
> - als_adjust_channel_bit_mask(channels, index, st->als[i].size);
> + channels[index].scan_type = (struct iio_scan_type) {
> + .format = 's',
[Severity: Low]
Does struct iio_scan_type have a .format member? Looking at
include/linux/iio/iio.h, the member for indicating signedness is .sign, which
was used in the original als_adjust_channel_bit_mask() code.
Will this cause a compilation failure due to an unknown field?
> + .realbits = BYTES_TO_BITS(st->als[i].size),
> + .storagebits = BITS_PER_TYPE(u32),
> + };
> ++index;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519234752.23911-1-natalia.andre@ime.usp.br?part=3
^ permalink raw reply
* Re: [PATCH v4 2/6] iio: gyro: HID: hid-sensor-gyro-3d: Refactor channel initialization
From: sashiko-bot @ 2026-05-20 0:31 UTC (permalink / raw)
To: Natália Salvino André; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260519234752.23911-3-natalia.andre@ime.usp.br>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Low] The `struct iio_scan_type` is incorrectly initialized with a non-existent `.format` member, causing a build failure.
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but a teardown race condition exists when the driver is unbound, leading to a potential Use-After-Free of `gyro_state` inside the HID raw event handler.
--
commit fa4886d3bbba316658546fb6162186593b204b9c
Author: Natália Salvino André <natalia.andre@ime.usp.br>
iio: gyro: HID: hid-sensor-gyro-3d: Refactor channel initialization
This commit replaces the local gyro_3d_adjust_channel_bit_mask() function
with a compound literal for scan_type initialization to improve code
readability. It also cleans up the channel initialization loop by iterating
directly over the channel scan indices.
> diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
> index c43990c518f77..ee7bde8620b41 100644
> --- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
> +++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
[ ... ]
> @@ -248,19 +238,20 @@ static int gyro_3d_parse_report(struct platform_device *pdev,
> struct gyro_3d_state *st)
> {
> int ret;
> - int i;
>
> - for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) {
> + for (unsigned int ch = CHANNEL_SCAN_INDEX_X; ch <= CHANNEL_SCAN_INDEX_Z; ch++) {
> ret = sensor_hub_input_get_attribute_info(hsdev,
> HID_INPUT_REPORT,
> usage_id,
> - HID_USAGE_SENSOR_ANGL_VELOCITY_X_AXIS + i,
> - &st->gyro[CHANNEL_SCAN_INDEX_X + i]);
> + HID_USAGE_SENSOR_ANGL_VELOCITY_X_AXIS + ch,
> + &st->gyro[ch]);
> if (ret < 0)
> break;
> - gyro_3d_adjust_channel_bit_mask(channels,
> - CHANNEL_SCAN_INDEX_X + i,
> - st->gyro[CHANNEL_SCAN_INDEX_X + i].size);
> + channels[ch].scan_type = (struct iio_scan_type) {
> + .format = 's',
[Severity: Low]
Does this initialization cause a build failure?
Looking at struct iio_scan_type, it designates the field for signedness as sign,
not format. The original code correctly assigned 's' to
channels[channel].scan_type.sign. Using format here will likely cause a compiler
error about having no member named format.
> + .realbits = BYTES_TO_BITS(st->gyro[ch].size),
> + .storagebits = BITS_PER_TYPE(u32),
> + };
> }
[Severity: High]
This isn't a bug introduced by this patch, but could a teardown race condition
exist when the driver is unbound, leading to a potential use-after-free of
gyro_state inside the HID raw event handler?
When the driver is manually unbound, hid_gyro_3d_remove() calls
sensor_hub_remove_callback(). This core function unregisters the callback list
entry but does not flush or synchronize with concurrently running
sensor_hub_raw_event() asynchronous dispatches.
Consequently, remove completes and devres proceeds to free indio_dev, which
embeds gyro_state and the callback structure. A concurrent
sensor_hub_raw_event() that fetched the callback pointer prior to removal (after
dropping dyn_callback_lock) could then execute callback->capture_sample() and
callback->send_event() on freed memory.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519234752.23911-1-natalia.andre@ime.usp.br?part=2
^ permalink raw reply
* Re: [PATCH v4 1/6] iio: accel: HID: hid-sensor-accel-3d: Refactor channel initialization
From: sashiko-bot @ 2026-05-20 0:04 UTC (permalink / raw)
To: Natália Salvino André; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260519234752.23911-2-natalia.andre@ime.usp.br>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] The patch introduces a compilation error by using a non-existent field `.format` instead of `.sign` in the initialization of `struct iio_scan_type`.
--
commit 5d0645bc673099b585d567599624c53910242efc
Author: Natália Salvino André <natalia.andre@ime.usp.br>
iio: accel: HID: hid-sensor-accel-3d: Refactor channel initialization
Clean up the channel initialization loop and replace the local
accel_3d_adjust_channel_bit_mask() function with a compound literal
to improve code readability.
> diff --git a/drivers/iio/accel/hid-sensor-accel-3d.c b/drivers/iio/accel/hid-sensor-accel-3d.c
> index 2ff591b3458f7..2bf05ab5235ef 100644
> --- a/drivers/iio/accel/hid-sensor-accel-3d.c
> +++ b/drivers/iio/accel/hid-sensor-accel-3d.c
[ ... ]
> @@ -297,19 +287,20 @@ static int accel_3d_parse_report(struct platform_device *pdev,
> struct accel_3d_state *st)
> {
> int ret;
> - int i;
>
> - for (i = 0; i <= CHANNEL_SCAN_INDEX_Z; ++i) {
> + for (unsigned int ch = CHANNEL_SCAN_INDEX_X; ch <= CHANNEL_SCAN_INDEX_Z; ch++) {
> ret = sensor_hub_input_get_attribute_info(hsdev,
> HID_INPUT_REPORT,
> usage_id,
> - HID_USAGE_SENSOR_ACCEL_X_AXIS + i,
> - &st->accel[CHANNEL_SCAN_INDEX_X + i]);
> + HID_USAGE_SENSOR_ACCEL_X_AXIS + ch,
> + &st->accel[ch]);
> if (ret < 0)
> break;
> - accel_3d_adjust_channel_bit_mask(channels,
> - CHANNEL_SCAN_INDEX_X + i,
> - st->accel[CHANNEL_SCAN_INDEX_X + i].size);
> + channels[ch].scan_type = (struct iio_scan_type) {
> + .format = 's',
[Severity: Low]
Does this code result in a compilation error? Looking at struct iio_scan_type
defined in include/linux/iio/iio.h, it contains a sign member but no format
member. Should this be .sign = 's' instead?
> + .realbits = BYTES_TO_BITS(st->accel[ch].size),
> + .storagebits = BITS_PER_TYPE(u32),
> + };
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260519234752.23911-1-natalia.andre@ime.usp.br?part=1
^ permalink raw reply
* [PATCH v4 6/6] iio: pressure: HID: hid-sensor-press: Refactor channel initialization
From: Natália Salvino André @ 2026-05-19 23:40 UTC (permalink / raw)
To: andy, dlechner, jic23, jikos, nuno.sa, srinivas.pandruvada
Cc: Natália Salvino André, Pietro Di Consolo Gregorio,
linux-iio, linux-input
In-Reply-To: <20260519234752.23911-1-natalia.andre@ime.usp.br>
Replace the local press_adjust_channel_bit_mask() function with a
compound literal for scan_type initialization to improve code
readability.
Signed-off-by: Natália Salvino André <natalia.andre@ime.usp.br>
Co-developed-by: Pietro Di Consolo Gregorio <pietro.gregorio@usp.br>
Signed-off-by: Pietro Di Consolo Gregorio <pietro.gregorio@usp.br>
---
drivers/iio/pressure/hid-sensor-press.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/pressure/hid-sensor-press.c b/drivers/iio/pressure/hid-sensor-press.c
index 5f1d6abda3e4..a039b99d9851 100644
--- a/drivers/iio/pressure/hid-sensor-press.c
+++ b/drivers/iio/pressure/hid-sensor-press.c
@@ -3,6 +3,7 @@
* HID Sensors Driver
* Copyright (c) 2014, Intel Corporation.
*/
+#include <linux/bitops.h>
#include <linux/device.h>
#include <linux/platform_device.h>
#include <linux/module.h>
@@ -53,17 +54,6 @@ static const struct iio_chan_spec press_channels[] = {
};
-/* Adjust channel real bits based on report descriptor */
-static void press_adjust_channel_bit_mask(struct iio_chan_spec *channels,
- int channel, int size)
-{
- channels[channel].scan_type.sign = 's';
- /* Real storage bits will change based on the report desc. */
- channels[channel].scan_type.realbits = size * 8;
- /* Maximum size of a sample to capture is u32 */
- channels[channel].scan_type.storagebits = sizeof(u32) * 8;
-}
-
/* Channel read_raw handler */
static int press_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
@@ -225,8 +215,11 @@ static int press_parse_report(struct platform_device *pdev,
&st->press_attr);
if (ret < 0)
return ret;
- press_adjust_channel_bit_mask(channels, CHANNEL_SCAN_INDEX_PRESSURE,
- st->press_attr.size);
+ channels[CHANNEL_SCAN_INDEX_PRESSURE].scan_type = (struct iio_scan_type) {
+ .format = 's',
+ .realbits = BYTES_TO_BITS(st->press_attr.size),
+ .storagebits = BITS_PER_TYPE(u32),
+ };
dev_dbg(&pdev->dev, "press %x:%x\n", st->press_attr.index,
st->press_attr.report_id);
--
2.51.0
^ permalink raw reply related
* [PATCH v4 5/6] iio: magnetometer: HID: hid-sensor-magn-3d: Refactor channel initialization
From: Natália Salvino André @ 2026-05-19 23:40 UTC (permalink / raw)
To: andy, dlechner, jic23, jikos, nuno.sa, srinivas.pandruvada
Cc: Natália Salvino André, Pietro Di Consolo Gregorio,
linux-iio, linux-input
In-Reply-To: <20260519234752.23911-1-natalia.andre@ime.usp.br>
Replace the local magn_3d_adjust_channel_bit_mask() function with a
compound literal for scan_type initialization to improve code
readability.
Signed-off-by: Natália Salvino André <natalia.andre@ime.usp.br>
Co-developed-by: Pietro Di Consolo Gregorio <pietro.gregorio@usp.br>
Signed-off-by: Pietro Di Consolo Gregorio <pietro.gregorio@usp.br>
---
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 20 ++++++-------------
1 file changed, 6 insertions(+), 14 deletions(-)
diff --git a/drivers/iio/magnetometer/hid-sensor-magn-3d.c b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
index c673f9323e47..3b26dc41dd4c 100644
--- a/drivers/iio/magnetometer/hid-sensor-magn-3d.c
+++ b/drivers/iio/magnetometer/hid-sensor-magn-3d.c
@@ -3,6 +3,7 @@
* HID Sensors Driver
* Copyright (c) 2012, Intel Corporation.
*/
+#include <linux/bitops.h>
#include <linux/device.h>
#include <linux/platform_device.h>
#include <linux/module.h>
@@ -132,17 +133,6 @@ static const struct iio_chan_spec magn_3d_channels[] = {
IIO_CHAN_SOFT_TIMESTAMP(7)
};
-/* Adjust channel real bits based on report descriptor */
-static void magn_3d_adjust_channel_bit_mask(struct iio_chan_spec *channels,
- int channel, int size)
-{
- channels[channel].scan_type.sign = 's';
- /* Real storage bits will change based on the report desc. */
- channels[channel].scan_type.realbits = size * 8;
- /* Maximum size of a sample to capture is u32 */
- channels[channel].scan_type.storagebits = sizeof(u32) * 8;
-}
-
/* Channel read_raw handler */
static int magn_3d_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
@@ -418,9 +408,11 @@ static int magn_3d_parse_report(struct platform_device *pdev,
if (i != CHANNEL_SCAN_INDEX_TIMESTAMP) {
/* Set magn_val_addr to iio value address */
st->magn_val_addr[i] = &st->iio_vals[*chan_count];
- magn_3d_adjust_channel_bit_mask(_channels,
- *chan_count,
- st->magn[i].size);
+ _channels[*chan_count].scan_type = (struct iio_scan_type) {
+ .format = 's',
+ .realbits = BYTES_TO_BITS(st->magn[i].size),
+ .storagebits = BITS_PER_TYPE(u32),
+ };
}
(*chan_count)++;
}
--
2.51.0
^ permalink raw reply related
* [PATCH v4 4/6] iio: light: HID: hid-sensor-prox: Refactor channel initialization
From: Natália Salvino André @ 2026-05-19 23:40 UTC (permalink / raw)
To: andy, dlechner, jic23, jikos, nuno.sa, srinivas.pandruvada
Cc: Natália Salvino André, Pietro Di Consolo Gregorio,
linux-iio, linux-input
In-Reply-To: <20260519234752.23911-1-natalia.andre@ime.usp.br>
Replace the local prox_adjust_channel_bit_mask() function with a
compound literal for scan_type initialization to improve code
readability.
Signed-off-by: Natália Salvino André <natalia.andre@ime.usp.br>
Co-developed-by: Pietro Di Consolo Gregorio <pietro.gregorio@usp.br>
Signed-off-by: Pietro Di Consolo Gregorio <pietro.gregorio@usp.br>
---
drivers/iio/light/hid-sensor-prox.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-prox.c b/drivers/iio/light/hid-sensor-prox.c
index efa904a70d0e..edc9274a2c07 100644
--- a/drivers/iio/light/hid-sensor-prox.c
+++ b/drivers/iio/light/hid-sensor-prox.c
@@ -3,6 +3,7 @@
* HID Sensors Driver
* Copyright (c) 2014, Intel Corporation.
*/
+#include <linux/bitops.h>
#include <linux/device.h>
#include <linux/platform_device.h>
#include <linux/module.h>
@@ -67,17 +68,6 @@ static const struct iio_chan_spec prox_channels[] = {
PROX_CHANNEL(false, 0),
};
-/* Adjust channel real bits based on report descriptor */
-static void prox_adjust_channel_bit_mask(struct iio_chan_spec *channels,
- int channel, int size)
-{
- channels[channel].scan_type.sign = 's';
- /* Real storage bits will change based on the report desc. */
- channels[channel].scan_type.realbits = size * 8;
- /* Maximum size of a sample to capture is u32 */
- channels[channel].scan_type.storagebits = sizeof(u32) * 8;
-}
-
/* Channel read_raw handler */
static int prox_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
@@ -250,8 +240,11 @@ static int prox_parse_report(struct platform_device *pdev,
st->scan_mask[0] |= BIT(index);
channels[index] = prox_channels[i];
channels[index].scan_index = index;
- prox_adjust_channel_bit_mask(channels, index,
- st->prox_attr[index].size);
+ channels[index].scan_type = (struct iio_scan_type) {
+ .format = 's',
+ .realbits = BYTES_TO_BITS(st->prox_attr[index].size),
+ .storagebits = BITS_PER_TYPE(u32),
+ };
dev_dbg(&pdev->dev, "prox %x:%x\n", st->prox_attr[index].index,
st->prox_attr[index].report_id);
st->scale_precision[index] =
--
2.51.0
^ permalink raw reply related
* [PATCH v4 3/6] iio: light: HID: hid-sensor-als: Refactor channel initialization
From: Natália Salvino André @ 2026-05-19 23:40 UTC (permalink / raw)
To: andy, dlechner, jic23, jikos, nuno.sa, srinivas.pandruvada
Cc: Natália Salvino André, Pietro Di Consolo Gregorio,
linux-iio, linux-input
In-Reply-To: <20260519234752.23911-1-natalia.andre@ime.usp.br>
Replace the local als_adjust_channel_bit_mask() function with a
compound literal for scan_type initialization to improve code
readability.
Signed-off-by: Natália Salvino André <natalia.andre@ime.usp.br>
Co-developed-by: Pietro Di Consolo Gregorio <pietro.gregorio@usp.br>
Signed-off-by: Pietro Di Consolo Gregorio <pietro.gregorio@usp.br>
---
drivers/iio/light/hid-sensor-als.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/iio/light/hid-sensor-als.c b/drivers/iio/light/hid-sensor-als.c
index 384572844162..d72e260b8266 100644
--- a/drivers/iio/light/hid-sensor-als.c
+++ b/drivers/iio/light/hid-sensor-als.c
@@ -3,6 +3,7 @@
* HID Sensors Driver
* Copyright (c) 2012, Intel Corporation.
*/
+#include <linux/bitops.h>
#include <linux/device.h>
#include <linux/platform_device.h>
#include <linux/module.h>
@@ -117,17 +118,6 @@ static const struct iio_chan_spec als_channels[] = {
IIO_CHAN_SOFT_TIMESTAMP(CHANNEL_SCAN_INDEX_TIMESTAMP)
};
-/* Adjust channel real bits based on report descriptor */
-static void als_adjust_channel_bit_mask(struct iio_chan_spec *channels,
- int channel, int size)
-{
- channels[channel].scan_type.sign = 's';
- /* Real storage bits will change based on the report desc. */
- channels[channel].scan_type.realbits = size * 8;
- /* Maximum size of a sample to capture is u32 */
- channels[channel].scan_type.storagebits = sizeof(u32) * 8;
-}
-
/* Channel read_raw handler */
static int als_read_raw(struct iio_dev *indio_dev,
struct iio_chan_spec const *chan,
@@ -335,7 +325,11 @@ static int als_parse_report(struct platform_device *pdev,
channels[index] = als_channels[i];
st->als_scan_mask[0] |= BIT(i);
- als_adjust_channel_bit_mask(channels, index, st->als[i].size);
+ channels[index].scan_type = (struct iio_scan_type) {
+ .format = 's',
+ .realbits = BYTES_TO_BITS(st->als[i].size),
+ .storagebits = BITS_PER_TYPE(u32),
+ };
++index;
dev_dbg(&pdev->dev, "als %x:%x\n", st->als[i].index,
--
2.51.0
^ permalink raw reply related
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