* [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.6] HID: sony: add missing size validation for SMK-Link remotes
From: Sasha Levin @ 2026-05-20 11:19 UTC (permalink / raw)
To: patches, stable
Cc: Rosalie Wanders, Jiri Kosina, Sasha Levin, jikos, bentiss,
linux-input, linux-kernel
In-Reply-To: <20260520111944.3424570-1-sashal@kernel.org>
From: Rosalie Wanders <rosalie@mailbox.org>
[ Upstream commit a4170b63eda999d20ad6dc39ddc3ce5c1ac619e6 ]
This commit adds the missing size validation for SMK-Link remotes in
sony_raw_event(), this prevents a malicious device from allowing
hid-sony to read out of bounds of the provided buffer.
I do not own these devices so the size check only forces that the buffer
is large enough for nsg_mrxu_parse_report().
Signed-off-by: Rosalie Wanders <rosalie@mailbox.org>
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: sony`; action
`add`; intent is missing size validation for SMK-Link remotes. Tags
present: `Signed-off-by: Rosalie Wanders`, `Signed-off-by: Jiri Kosina`;
no `Fixes:`, `Reported-by:`, `Tested-by:`, `Reviewed-by:`, `Acked-by:`,
`Link:`, or `Cc: stable`. Body says a malicious device can make `hid-
sony` read beyond the provided buffer in `sony_raw_event()`. This is a
direct memory-safety bug, not a hidden cleanup.
Phase 2, diff analysis. Record: one file, `drivers/hid/hid-sony.c`, 2
insertions and 3 deletions; only `sony_raw_event()` changes. Before: any
`NSG_MRXU_REMOTE` report with `rd[0] == 0x02` called
`nsg_mrxu_parse_report()`. After: it only does so when `size >= 12`.
`nsg_mrxu_parse_report()` reads through `rd[11]`, including `rd[offset +
10]` with `offset = 1`, so the minimum size is verified by code. Bug
category: bounds check / out-of-bounds read. Fix quality: surgical and
obviously correct; the only behavioral change is rejecting malformed
short reports before parsing.
Phase 3, git history. Record: upstream commit is
`a4170b63eda999d20ad6dc39ddc3ce5c1ac619e6`, authored by Rosalie Wanders
and committed by HID maintainer Jiri Kosina. Blame shows the NSG-MRxU
parser and unchecked call were introduced by `b7289cb1cf99d` (`HID:
sony: Add touchpad support for NSG-MR5U and NSG-MR7U remotes`), first
described by `git describe` as `v4.17-rc1~118^2~2^2`. No `Fixes:` tag to
follow. Recent history shows an adjacent independent HID Sony size-
validation fix for Rock Band 3 Pro instruments, but no dependency for
this patch. Author has multiple recent `hid-sony` commits.
Phase 4, mailing list and external research. Record: `b4 dig -c
a4170b63eda99` found the exact lore submission at
`https://patch.msgid.link/20260412010806.7997-2-rosalie@mailbox.org`.
`b4 dig -a` showed only v1, no later revision. `b4 dig -w` showed Jiri
Kosina, Benjamin Tissoires, `linux-input`, and `linux-kernel` were
included. Full thread mbox shows Jiri replied: “Applied to
hid.git#for-7.1/upstream-fixes, thanks.” No NAKs, objections, explicit
stable request, or reviewer-suggested changes found. Direct WebFetch to
lore was blocked by Anubis, but `b4` fetched the thread successfully.
Phase 5, semantic analysis. Record: key functions are `sony_raw_event()`
and `nsg_mrxu_parse_report()`. `sony_raw_event()` is registered as the
HID driver `.raw_event` callback. HID core calls `.raw_event` from
`__hid_input_report()` before `hid_report_raw_event()`’s later report-
size validation, so driver-local parsing must validate `size` itself.
Lower HID layers such as USB HID call `hid_input_report()` with device-
provided lengths. `nsg_mrxu_parse_report()` is only called from
`sony_raw_event()` in this file. Similar pattern found: adjacent
upstream commit `12bd440b66ed8` adds a size check before another `rd[]`
access in the same function.
Phase 6, stable tree analysis. Record: the buggy NSG code exists in
checked stable branches `4.19.y`, `5.4.y`, `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`; `git merge-base
--is-ancestor b7289cb1cf99d4 <branch>` confirmed the introducing commit
is in each. Exact patch apply-check succeeded on `7.0.y`; older branches
failed exact context due file drift, but the required backport is a
trivial one-condition change at the same unchecked call site. No
existing stable branch log entry for this exact SMK-Link fix was found
in the checked branches.
Phase 7, subsystem context. Record: subsystem is HID input driver code,
specifically `drivers/hid/hid-sony.c`; criticality is driver-specific
but security-relevant for systems using or exposed to spoofed SMK-
Link/Sony HID devices. `MAINTAINERS` lists HID core as maintained by
Jiri Kosina and Benjamin Tissoires; Jiri committed and applied the
patch. The file has moderate recent activity, including HID Sony fixes
and device support work.
Phase 8, impact and risk. Record: affected users are `CONFIG_HID_SONY`
systems with SMK-Link NSG-MR5U/NSG-MR7U remote support, or systems that
accept a malicious/spoofed matching HID device. Trigger is a report with
ID byte `0x02` and length below 12. Failure mode is an out-of-bounds
read from the supplied report buffer; severity HIGH because it is
device-triggerable kernel memory-safety behavior. Benefit is high for
affected systems; risk is very low because valid reports of length >= 12
keep identical behavior and malformed short reports stop before unsafe
parsing.
Phase 9, synthesis. Evidence for backporting: real OOB read, malicious-
device trigger described in commit body, parser byte accesses verify the
needed bound, bug exists since v4.17-era code and is present in many
stable branches, fix is a one-line functional bounds check, maintainer
applied it to HID upstream fixes. Evidence against: no explicit `Tested-
by`, author says they do not own the hardware, and older stable trees
need a small context backport. Unresolved: no public crash report or
syzbot report found; no hardware test evidence found.
## Stable Rules
1. Obviously correct and tested: obviously correct by code inspection;
no explicit `Tested-by` and no hardware ownership, but maintainer
accepted it as an upstream fix.
2. Fixes a real bug: yes, verified unchecked parser reads up to
`rd[11]`.
3. Important issue: yes, device-triggerable kernel out-of-bounds read.
4. Small and contained: yes, one function in one driver.
5. No new features/APIs: yes.
6. Can apply to stable: yes for `7.0.y` directly; older stable branches
need trivial context adjustment.
No exception category is needed; this is a normal memory-safety fix.
## Verification
- Phase 1: parsed `git show a4170b63eda99`; confirmed tags and absence
of `Fixes:`, reports, review, stable Cc.
- Phase 2: inspected diff and `nsg_mrxu_parse_report()`; confirmed
`rd[11]` maximum access and added `size >= 12`.
- Phase 3: ran `git blame`, `git log -S`, `git show b7289cb1cf99d4`, and
author history checks.
- Phase 4: ran `b4 dig -c`, `-a`, `-w`, `b4 am`, and full `b4 mbox`;
confirmed v1-only thread and maintainer apply reply.
- Phase 5: used `rg` and `ReadFile` on HID core and `hid-sony`;
confirmed `.raw_event` call path and lower-layer `hid_input_report()`
entry.
- Phase 6: checked stable branch code, ancestry of `b7289cb1cf99d4`,
exact apply-checks, and stable logs for this subject.
- Phase 7: checked `MAINTAINERS` for HID maintainers and subsystem
ownership.
- Phase 8: mapped trigger and severity from verified parser accesses and
HID input path.
- Unverified: actual hardware testing on SMK-Link remotes; exact clean
application to older stable trees without a tiny backport adjustment.
**YES**
drivers/hid/hid-sony.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-sony.c b/drivers/hid/hid-sony.c
index a89af14e4acc6..02baaf84e9792 100644
--- a/drivers/hid/hid-sony.c
+++ b/drivers/hid/hid-sony.c
@@ -1035,10 +1035,9 @@ static int sony_raw_event(struct hid_device *hdev, struct hid_report *report,
sixaxis_parse_report(sc, rd, size);
} else if ((sc->quirks & MOTION_CONTROLLER_BT) && rd[0] == 0x01 && size == 49) {
sixaxis_parse_report(sc, rd, size);
- } else if ((sc->quirks & NAVIGATION_CONTROLLER) && rd[0] == 0x01 &&
- size == 49) {
+ } else if ((sc->quirks & NAVIGATION_CONTROLLER) && rd[0] == 0x01 && size == 49) {
sixaxis_parse_report(sc, rd, size);
- } else if ((sc->quirks & NSG_MRXU_REMOTE) && rd[0] == 0x02) {
+ } else if ((sc->quirks & NSG_MRXU_REMOTE) && rd[0] == 0x02 && size >= 12) {
nsg_mrxu_parse_report(sc, rd, size);
return 1;
} else if ((sc->quirks & RB4_GUITAR_PS4_USB) && rd[0] == 0x01 && size == 64) {
--
2.53.0
^ permalink raw reply related
* [PATCH AUTOSEL 7.0-5.15] HID: ft260: validate i2c input report length
From: Sasha Levin @ 2026-05-20 11:19 UTC (permalink / raw)
To: patches, stable
Cc: Michael Zaidman, Sebastián Josué Alba Vives,
Jiri Kosina, Sasha Levin, jikos, bentiss, linux-i2c, linux-input,
linux-kernel
In-Reply-To: <20260520111944.3424570-1-sashal@kernel.org>
From: Michael Zaidman <michael.zaidman@gmail.com>
[ Upstream commit 80c4bbb2b38513e9c3d84805fa61a0ee16d79c45 ]
Add two checks to ft260_raw_event() to prevent out-of-bounds reads
from malicious or malfunctioning devices:
First, reject reports shorter than the 2-byte header (report ID +
length fields). Without this, even accessing xfer->length on a
1-byte report is an OOB read.
Second, validate xfer->length against the actual data capacity of
the received HID report. Each I2C data report ID (0xD0 through
0xDE) defines a different report size in the HID descriptor, so the
available payload varies per report. A corrupted length field could
cause memcpy to read beyond the report buffer.
Reported-by: Sebastián Josué Alba Vives <sebasjosue84@gmail.com>
Signed-off-by: Michael Zaidman <michael.zaidman@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:
## Decision Summary
This should be backported. It is a small, single-driver HID fix for a
real out-of-bounds read in `ft260_raw_event()`, reachable from FT260 USB
HID input reports before HID core report-size validation runs. The
benefit is high because the failure mode is memory-safety/security
relevant; the regression risk is low because valid reports are unchanged
and only malformed reports are rejected.
## Phase Walkthrough
### Phase 1: Commit Message Forensics
Record: subsystem `HID: ft260`; action verb `validate`; intent is to
validate I2C input report length.
Record: tags in commit `80c4bbb2b38513e9c3d84805fa61a0ee16d79c45`:
`Reported-by: Sebastián Josué Alba Vives <sebasjosue84@gmail.com>`,
`Signed-off-by: Michael Zaidman <michael.zaidman@gmail.com>`, `Signed-
off-by: Jiri Kosina <jkosina@suse.com>`. No `Fixes:`, `Link:`, `Cc:
stable`, `Tested-by`, `Reviewed-by`, or `Acked-by` in the committed
message.
Record: bug description is explicit: malicious or malfunctioning
FT260-compatible HID devices can send too-short reports or corrupted
length fields, causing OOB reads. Symptom/failure mode is OOB heap read
before/inside `memcpy()`. No affected version listed in commit body.
Record: this is not a hidden fix; it is directly described as a memory-
safety fix.
### Phase 2: Diff Analysis
Record: one file changed, `drivers/hid/hid-ft260.c`, `14` insertions and
`2` deletions. Only `ft260_raw_event()` is modified. Scope is single-
file surgical.
Record: before, `ft260_raw_event()` cast `data` to `struct
ft260_i2c_input_report` and accessed `xfer->report`/`xfer->length`
without checking `size`; then `memcpy()` used `xfer->length` after
checking only destination capacity. After, it rejects reports shorter
than the 2-byte header and rejects `xfer->length > size - offsetof(...,
data)` before copying.
Record: bug category is memory safety: OOB read on truncated input and
OOB source read on corrupted length. No locking, API, or feature
behavior changes.
Record: fix quality is strong: minimal, obvious bounds checks
immediately before the unsafe accesses/copy. Regression risk is low;
only malformed reports are rejected. The use of actual `size` avoids the
earlier reviewed problem of rejecting valid short FT260 report IDs.
### Phase 3: Git History Investigation
Record: `git blame` shows the vulnerable header access and `memcpy()`
came from `6a82582d9fa4` (`HID: ft260: add usb hid to i2c host bridge
driver`, first contained around `v5.13-rc1`). The destination-buffer
check came from `b7121e3c04440` (`HID: ft260: skip unexpected HID input
reports`, first contained around `v6.2-rc1`).
Record: no `Fixes:` tag, so Step 3.2 is not applicable.
Record: recent file history shows related FT260 fixes and enhancements,
including `b7121e3c04440`, but no prerequisite needed for the new
source-length/header validation. For older `5.15.y`/`6.1.y`, the exact
patch context does not apply because `b7121e3c04440` is absent, but the
fix itself is still conceptually standalone.
Record: author Michael Zaidman is listed in `MAINTAINERS` as FT260
maintainer and has a long FT260 commit history. Jiri Kosina committed
the patch and is listed as HID core maintainer.
### Phase 4: Mailing List And External Research
Record: `b4 dig -c 80c4bbb2b3851` found the matching submission at `http
s://patch.msgid.link/20260411062437.279838-1-michael.zaidman@gmail.com`.
Record: `b4 dig -a` showed prior versions/discussion. The original
reporter’s v1/v2 were Cc’d to stable and had a too-broad `size < 64`
approach. Review identified that valid FT260 reports can be shorter,
leading to Michael’s final per-driver fix using the header size and
actual report size.
Record: `b4 dig -w` showed Michael Zaidman, Jiri Kosina, Benjamin
Tissoires, linux-input, linux-kernel, and the reporter on the final
submission.
Record: saved lore mbox shows Jiri applied the final patch to
`hid.git#for-7.1/upstream-fixes`. The thread also contains Michael’s
hardware test note: tested on FT260 with I2C-attached EEPROM behind
PCA9548 muxes, including small and larger reads.
Record: stable-specific history exists in the earlier reporter
submissions, including `Cc: stable@vger.kernel.org` and explicit
OOB/security rationale. No NAK remained against the final version.
### Phase 5: Code Semantic Analysis
Record: modified function is `ft260_raw_event()`.
Record: callers traced manually: `ft260_raw_event()` is registered as
`.raw_event`; HID core calls `hdrv->raw_event()` in
`__hid_input_report()` before `hid_report_raw_event()` performs report-
size checks. USB HID paths call `hid_input_report()` from
interrupt/control completion paths.
Record: callees/side effects: `ft260_raw_event()` copies HID I2C
response payload into `dev->read_buf`, advances `read_idx`, and
completes the waiting I2C read when enough data arrives.
Record: reachability: normal FT260 I2C reads go through `i2c_adapter`
operations (`ft260_i2c_xfer()` / `ft260_smbus_xfer()` ->
`ft260_i2c_read()`), while the malformed input itself is supplied by the
USB HID device. Userspace reachability depends on system I2C access
permissions; unprivileged trigger was not verified.
Record: similar pattern in this function is unique for the FT260 I2C
input report copy. Related HID OOB fixes exist in recent HID history,
supporting the general risk class but not required for this patch.
### Phase 6: Stable Tree Analysis
Record: `drivers/hid/hid-ft260.c` exists in `stable/linux-5.15.y`,
`6.1.y`, `6.6.y`, `6.12.y`, and current `v7.0.9` checkout; it does not
exist in `4.19.y`, `5.4.y`, or `5.10.y`.
Record: the buggy code pattern exists in `5.15.y`, `6.1.y`, `6.6.y`, and
`6.12.y`; none contain `80c4bbb2b3851`.
Record: patch applies cleanly to `6.6.y`, `6.12.y`, and current `v7.0.9`
using a temporary index. It does not apply unchanged to `5.15.y` or
`6.1.y` because those lack `b7121e3c04440` context, so older branches
need a small manual backport.
Record: no alternate stable fix for this exact validation was found.
### Phase 7: Subsystem And Maintainer Context
Record: subsystem is HID driver for an FTDI USB HID-to-I2C bridge.
Criticality is driver-specific but security-relevant for systems using
that hardware or exposed USB ports.
Record: subsystem/file activity is active; recent HID history includes
several OOB fixes, and FT260 has ongoing maintenance by Michael Zaidman.
### Phase 8: Impact And Risk
Record: affected population is driver/config/hardware-specific: systems
with `CONFIG_HID_FT260` and FT260 or malicious FT260-emulating USB HID
devices.
Record: trigger is a malformed HID input report from the device,
especially during/around I2C read handling. Physical or device-level
control is required; unprivileged local trigger was not verified.
Record: failure mode is OOB heap read, with reporter discussion noting
possible data exposure through the I2C read buffer. Severity: HIGH,
security-relevant memory safety.
Record: benefit is high for affected systems; risk is low because the
patch is small, contained, and only rejects malformed reports.
### Phase 9: Final Synthesis
Evidence for backporting: real OOB read; malicious/malfunctioning device
trigger; HID raw_event is called before HID core size validation;
maintainer-authored final fix; hardware test notes; one function and one
file; no new API or feature; applies cleanly to current 7.0 and newer
stable branches.
Evidence against/backport concerns: older `5.15.y` and `6.1.y` need
manual context adjustment; `4.19.y`, `5.4.y`, and `5.10.y` lack the
driver. No `Fixes:` tag, but that is not a negative signal here.
Stable rules checklist: obviously correct and tested: yes, by code
inspection and submission test notes. Fixes a real bug: yes, OOB reads.
Important issue: yes, memory safety/security. Small and contained: yes,
`14+/2-` in one function. No new features/APIs: yes. Can apply to
stable: yes for 6.6/6.12/7.0; minor backport needed for 5.15/6.1.
Exception categories: none; this is a direct memory-safety bug fix, not
a device ID/quirk/build/doc exception.
## Verification
- [Phase 1] `git show --format=fuller --no-patch 80c4bbb2b3851`:
confirmed subject, message, author/committer, and tags.
- [Phase 2] `git show --function-context 80c4bbb2b3851 --
drivers/hid/hid-ft260.c`: confirmed only `ft260_raw_event()` changes
and exact bounds checks.
- [Phase 3] `git blame -L 1065,1085 80c4bbb2b3851^ -- drivers/hid/hid-
ft260.c`: confirmed vulnerable lines originated in `6a82582d9fa4` and
related destination check in `b7121e3c04440`.
- [Phase 3] `git show` on `6a82582d9fa4` and `b7121e3c04440`: confirmed
driver introduction and related unexpected-report fix.
- [Phase 4] `b4 dig -c`, `-a`, `-w`, and `-m`: found final lore thread,
revisions, recipients, maintainer apply message, and test notes.
- [Phase 4] WebFetch/Spinics: confirmed earlier stable-cc’d reporter
patches and review discussion about avoiding a blanket 64-byte check.
- [Phase 5] Read `drivers/hid/hid-core.c`: confirmed `raw_event` runs
before `hid_report_raw_event()` size validation.
- [Phase 5] Searched FT260 call paths: confirmed registration as
`.raw_event`, USB HID input path, and I2C adapter read paths.
- [Phase 6] `git ls-tree` and `git grep` against stable branches:
confirmed affected code exists in `5.15.y+` branches checked, absent
from older listed branches.
- [Phase 6] Temporary-index `git apply --check`: clean for `6.6.y`,
`6.12.y`, and current `v7.0.9`; fails unchanged for `5.15.y`/`6.1.y`,
requiring minor backport.
- [Phase 7] `MAINTAINERS` search: confirmed Michael Zaidman maintains
FT260 and Jiri/Benjamin maintain HID core.
- UNVERIFIED: whether an unprivileged local user can trigger this on a
given distro; that depends on USB access, I2C device permissions, and
deployment policy.
**YES**
drivers/hid/hid-ft260.c | 16 ++++++++++++++--
1 file changed, 14 insertions(+), 2 deletions(-)
diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
index 333341e80b0ec..70e2eedb465af 100644
--- a/drivers/hid/hid-ft260.c
+++ b/drivers/hid/hid-ft260.c
@@ -1068,10 +1068,22 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
struct ft260_device *dev = hid_get_drvdata(hdev);
struct ft260_i2c_input_report *xfer = (void *)data;
+ if (size < offsetof(struct ft260_i2c_input_report, data)) {
+ hid_err(hdev, "short report %d\n", size);
+ return -1;
+ }
+
if (xfer->report >= FT260_I2C_REPORT_MIN &&
xfer->report <= FT260_I2C_REPORT_MAX) {
- ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report,
- xfer->length);
+ ft260_dbg("i2c resp: rep %#02x len %d size %d\n",
+ xfer->report, xfer->length, size);
+
+ if (xfer->length > size -
+ offsetof(struct ft260_i2c_input_report, data)) {
+ hid_err(hdev, "report %#02x: length %d exceeds HID report size\n",
+ xfer->report, xfer->length);
+ return -1;
+ }
if ((dev->read_buf == NULL) ||
(xfer->length > dev->read_len - dev->read_idx)) {
--
2.53.0
^ permalink raw reply related
* Re: [PATCH AUTOSEL 7.0-5.10] HID: magicmouse: Prevent out-of-bounds (OOB) read during DOUBLE_REPORT_ID
From: sashiko-bot @ 2026-05-20 11:41 UTC (permalink / raw)
To: Sasha Levin; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260520111944.3424570-9-sashal@kernel.org>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [Medium] The newly added warning for mismatched report lengths is not rate-limited, allowing an attacker to continuously spam the kernel log.
- [Low] Missing trailing newline in the newly added `hid_warn` format string.
Pre-existing issues:
- [Critical] A NULL pointer dereference can occur if a raw event is received before the device is fully initialized or if initialization fails to create an input device.
- [High] A malicious USB device can cause a kernel stack overflow by sending a crafted HID report with deeply nested DOUBLE_REPORT_ID reports.
--
commit 722425cdf7389e2fa0f87560f3507f1c80daa8b7
Author: Lee Jones <lee@kernel.org>
HID: magicmouse: Prevent out-of-bounds (OOB) read during DOUBLE_REPORT_ID
This commit adds checks to prevent out-of-bounds reads in the magicmouse
driver when processing DOUBLE_REPORT_ID reports. It verifies the report
size against the specified length and adds protection against zero-sized
recursive calls.
> 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;
[Severity: Critical]
This is a pre-existing issue, but can this lead to a NULL pointer
dereference?
If a device provides a crafted report descriptor without any input fields,
hidinput_connect() will not call .input_mapping, leaving msc->input as NULL.
During magicmouse_probe(), for some devices the probe returns 0 early without
verifying msc->input. If raw IN reports are then streamed, this function will
blindly dereference the NULL input pointer later when evaluating
input->id.product.
> int x = 0, y = 0, ii, clicks = 0, npoints;
>
> + /* Protect against zero sized recursive calls from DOUBLE_REPORT_ID */
> + if (size < 1)
> + return 0;
> +
[ ... ]
> @@ -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)",
[Severity: Low]
Does this log message need a trailing newline? It looks like the missing
newline will prevent the buffer from flushing immediately, causing subsequent
log messages to append to this line.
[Severity: Medium]
Could this warning be abused to spam the kernel log? A malicious or
misconfigured device could rapidly stream malformed reports, flooding dmesg.
Would it be safer to use hid_warn_ratelimited() here?
> + 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]);
[Severity: High]
This is a pre-existing issue, but does this recursion safely handle a payload
entirely filled with DOUBLE_REPORT_ID markers and data[1] set to 0?
Because size is only decremented by 2 in each recursive step:
magicmouse_raw_event(..., size - 2 - data[1]);
A large report of 4096 bytes could trigger up to 2048 nested calls. Since
tail-call optimization is not guaranteed in the kernel, couldn't this exhaust
the kernel stack and trigger a panic?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520111944.3424570-9-sashal@kernel.org?part=1
^ permalink raw reply
* Re: [PATCH AUTOSEL 7.0-5.10] HID: mcp2221: fix OOB write in mcp2221_raw_event()
From: sashiko-bot @ 2026-05-20 11:56 UTC (permalink / raw)
To: Sasha Levin; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260520111944.3424570-25-sashal@kernel.org>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
New issues:
- [High] The patch introduces `mcp->rxbuf_size` to prevent Out-of-Bounds (OOB) writes but fails to initialize it in `mcp_smbus_xfer()` for block operations.
Pre-existing issues:
- [High] The `mcp->rxbuf` pointer is never cleared after an I2C/SMBus transfer completes or times out, leaving a dangling pointer that can be exploited.
- [High] The `mcp2221_raw_event()` handler ignores the `size` parameter provided by the HID subsystem, leading to out-of-bounds reads on the kernel heap.
- [High] A data race exists between the I2C transfer context and the HID raw event context, which accesses `rxbuf` state variables without synchronization.
--
commit 161d8d13687380796daedd1f35b98269619cd589
Author: Florian Pradines <florian.pradines@gmail.com>
HID: mcp2221: fix OOB write in mcp2221_raw_event()
mcp2221_raw_event() copies device-supplied data into mcp->rxbuf at offset
rxbuf_idx without verifying that the copy fits within the destination buffer.
A device responding with up to 60 bytes to a small I2C/SMBus read can overflow
this buffer. This patch adds an rxbuf_size field to struct mcp2221 and checks
rxbuf_idx + data[3] <= rxbuf_size before performing the copy.
> 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
[ ... ]
> @@ -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;
[Severity: High]
Does this overlook updating mcp->rxbuf_size for block operations in
mcp_smbus_xfer()?
In mcp_smbus_xfer(), mcp->rxbuf is manually assigned for block reads without
updating mcp->rxbuf_size:
drivers/hid/hid-mcp2221.c:mcp_smbus_xfer() {
...
case I2C_SMBUS_BLOCK_DATA:
if (read_write == I2C_SMBUS_READ) {
...
mcp->rxbuf_idx = 0;
mcp->rxbuf = data->block;
...
}
If rxbuf_size is 0, the new bounds check in mcp2221_raw_event() will always
fail, breaking legitimate SMBus block reads. If rxbuf_size holds a large stale
value from a prior transfer, the check might pass and still allow an overflow
on the data->block stack buffer.
> }
>
> 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;
> + }
[Severity: High]
This is a pre-existing issue, but does mcp2221_raw_event() ignore the size
parameter provided by the HID subsystem?
The driver unconditionally accesses data bytes like data[3], and later copies
data[3] bytes starting from &data[4], without verifying the actual length of
the received HID report buffer. If a malicious device provides a small report
size, could reading or copying these bytes result in out-of-bounds reads on
the kernel heap?
[Severity: High]
This isn't a bug introduced by this patch, but is there a data race between
the I2C transfer context and the HID raw event context?
While mcp_i2c_smbus_read() and mcp_smbus_xfer() update mcp->rxbuf,
mcp->rxbuf_size, and mcp->rxbuf_idx while holding mcp->lock,
mcp2221_raw_event() runs asynchronously in the HID input report context and
accesses these variables without acquiring the lock.
If a stray or malicious USB report arrives concurrently with a new I2C read
request, could the state become desynchronized and lead to an out-of-bounds
write?
> buf = mcp->rxbuf;
[Severity: High]
This is a pre-existing issue, but is the mcp->rxbuf pointer left dangling
after a transfer completes or times out?
During transfers, mcp->rxbuf points to a caller-provided buffer which is often
allocated on the kernel stack. When a transfer times out, the stack frame is
popped, but mcp->rxbuf still points to the now-invalid stack address.
If a delayed or spurious USB report arrives later, could mcp2221_raw_event()
copy device-controlled data into this stale pointer and corrupt the kernel
stack?
> memcpy(&buf[mcp->rxbuf_idx], &data[4], data[3]);
> mcp->rxbuf_idx = mcp->rxbuf_idx + data[3];
> mcp->status = 0;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520111944.3424570-25-sashal@kernel.org?part=1
^ permalink raw reply
* Re: [PATCH AUTOSEL 7.0-5.15] HID: ft260: validate i2c input report length
From: sashiko-bot @ 2026-05-20 11:57 UTC (permalink / raw)
To: Sasha Levin; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260520111944.3424570-54-sashal@kernel.org>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [High] memcpy reads beyond the bounds of the statically sized array 'data[2]', which will trigger a FORTIFY_SOURCE panic for payloads > 2 bytes.
- [High] A race condition in the probe function leads to a NULL pointer dereference in `ft260_raw_event` if an input report is received early.
- [High] A race condition exists between the HID raw event handler and the I2C read timeout path, leading to a Use-After-Free or kernel stack corruption.
- [High] The probe function incorrectly returns 0 (success) when intentionally rejecting the UART interface, rendering it unusable.
--
commit 4ff190a6df13fab19cc5a93ba01ca6de278d20bd
Author: Michael Zaidman <michael.zaidman@gmail.com>
HID: ft260: validate i2c input report length
This commit adds checks to ft260_raw_event to prevent out-of-bounds reads
from malicious or malfunctioning devices. It rejects reports shorter than the
2-byte header and validates the length field against the actual HID report
capacity.
> diff --git a/drivers/hid/hid-ft260.c b/drivers/hid/hid-ft260.c
> index 333341e80b0ec..70e2eedb465af 100644
> --- a/drivers/hid/hid-ft260.c
> +++ b/drivers/hid/hid-ft260.c
> @@ -1068,10 +1068,22 @@ static int ft260_raw_event(struct hid_device *hdev, struct hid_report *report,
> struct ft260_device *dev = hid_get_drvdata(hdev);
[Severity: High]
This is a pre-existing issue, but can this result in a NULL pointer
dereference during device initialization?
If hid_hw_start() is called in ft260_probe() before hid_set_drvdata() is
initialized, the device might send an input report immediately (e.g., from
a pending interrupt). This would trigger ft260_raw_event(), leaving dev as
NULL and causing a panic when dev->read_buf is accessed below.
> struct ft260_i2c_input_report *xfer = (void *)data;
>
> + if (size < offsetof(struct ft260_i2c_input_report, data)) {
> + hid_err(hdev, "short report %d\n", size);
> + return -1;
> + }
> +
> if (xfer->report >= FT260_I2C_REPORT_MIN &&
> xfer->report <= FT260_I2C_REPORT_MAX) {
> - ft260_dbg("i2c resp: rep %#02x len %d\n", xfer->report,
> - xfer->length);
> + ft260_dbg("i2c resp: rep %#02x len %d size %d\n",
> + xfer->report, xfer->length, size);
> +
> + if (xfer->length > size -
> + offsetof(struct ft260_i2c_input_report, data)) {
> + hid_err(hdev, "report %#02x: length %d exceeds HID report size\n",
> + xfer->report, xfer->length);
> + return -1;
> + }
>
> if ((dev->read_buf == NULL) ||
[Severity: High]
This is a pre-existing issue, but does this check safely prevent a
use-after-free or stack corruption?
In ft260_i2c_read(), the driver blocks on wait_for_completion_timeout(). If a
timeout occurs, it sets dev->read_buf = NULL and returns, destroying the
caller-provided buffer which may be a stack frame.
If ft260_raw_event() checks dev->read_buf != NULL and is preempted while the
timeout path returns, wouldn't the subsequent memcpy() write hardware payload
data into freed memory or a popped stack frame?
> (xfer->length > dev->read_len - dev->read_idx)) {
There is another pre-existing issue right below the diff in ft260_raw_event():
drivers/hid/hid-ft260.c:ft260_raw_event() {
[ ... ]
if ((dev->read_buf == NULL) ||
(xfer->length > dev->read_len - dev->read_idx)) {
hid_err(hdev, "unexpected report %#02x, length %d\n",
xfer->report, xfer->length);
return -1;
}
memcpy(&dev->read_buf[dev->read_idx], &xfer->data,
xfer->length);
[ ... ]
}
[Severity: High]
This isn't a bug introduced by this patch, but will this memcpy() trigger a
FORTIFY_SOURCE panic for payloads larger than 2 bytes?
The ft260_i2c_input_report structure defines the data payload as a fixed-size
array:
struct ft260_i2c_input_report {
u8 report;
u8 length;
u8 data[2];
} __packed;
If xfer->length is greater than 2, the compiler statically knows the source
object size is exactly 2 bytes. Under CONFIG_FORTIFY_SOURCE=y, wouldn't this
trigger a mandatory kernel panic at runtime for a buffer overread? Could the
data field be changed to a flexible array member?
There is also a pre-existing issue in the probe function:
drivers/hid/hid-ft260.c:ft260_probe() {
[ ... ]
ret = ft260_is_interface_enabled(hdev);
if (ret <= 0)
goto err_hid_close;
[ ... ]
err_hid_close:
hid_hw_close(hdev);
err_hid_stop:
hid_hw_stop(hdev);
return ret;
}
[Severity: High]
This isn't a bug introduced by this patch, but does the probe function
incorrectly handle a rejected UART interface?
When ft260_is_interface_enabled() encounters the UART interface, it leaves
the return code as 0. The driver jumps to the error path, stops the hardware,
and returns 0 instead of an error code like -ENODEV.
Returning 0 informs the kernel driver core that hid-ft260 successfully bound
to the device interface. Doesn't this hijack the UART interface and leave it
in a disabled state, permanently preventing generic HID drivers (like
hid-generic) from claiming it?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520111944.3424570-54-sashal@kernel.org?part=1
^ permalink raw reply
* Re: [PATCH AUTOSEL 7.0-5.10] HID: elan: Add support for ELAN SB974D touchpad
From: sashiko-bot @ 2026-05-20 12:24 UTC (permalink / raw)
To: Sasha Levin; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260520111944.3424570-28-sashal@kernel.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] The driver fails to restore the device to multitouch mode after a system suspend/resume cycle.
- [High] In USB mode, if more than two fingers are active, the driver assigns identical coordinates to all fingers after the first one.
--
commit d5ae02b47586e24db0bca9c6a7b552b9fc7ef6e4
Author: Damien Dejean <damiendejean@google.com>
HID: elan: Add support for ELAN SB974D touchpad
This commit adds support for the ELAN SB974D touchpad by registering its
device ID in the supported devices list so the driver can parse its
vendor-specific formats.
> 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) },
[Severity: High]
This is a pre-existing issue, but does the driver fail to restore the device
to multitouch mode after a system suspend/resume cycle?
The ELAN touchpads require a vendor-specific initialization command
(elan_start_multitouch) to disable mouse emulation and enable multitouch
mode. This sequence is currently only executed during the initial elan_probe.
The hid_driver struct for elan_driver does not provide resume or reset_resume
callbacks. When the system suspends and the device loses power (both in USB
and I2C modes), it reverts to its default mouse emulation mode.
Upon resume, the core HID drivers call hid_driver_reset_resume(), which acts
as a no-op since the callback is missing and returns success without rebinding.
Does this leave the touchpad permanently in a degraded state after a
suspend/resume cycle?
[Severity: High]
This is a pre-existing issue, but in USB mode, if more than two fingers are
active, does the driver assign identical coordinates to all fingers after the
first one?
Looking at elan_usb_report_input(), when parsing the ELAN_MT_SECOND_FINGER
packet, the driver iterates over ELAN_MAX_FINGERS to dispatch slot updates
based on the active touch bits in prev_report[2].
For the first active bit found, it correctly uses coordinates from
prev_report + 3. For all subsequent active bits, the else branch executes:
elan_report_mt_slot(drvdata, data + 1, i);
Because data + 1 is passed directly each time without incrementing any offset,
are the exact same coordinates and pressure values assigned to all fingers
after the first?
> { }
> };
> MODULE_DEVICE_TABLE(hid, elan_devices);
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520111944.3424570-28-sashal@kernel.org?part=1
^ permalink raw reply
* Re: [PATCH v3 0/4] HID: Proper fix for OOM in hid-core
From: Lee Jones @ 2026-05-20 12:46 UTC (permalink / raw)
To: Benjamin Tissoires
Cc: Greg Kroah-Hartman, Jiri Kosina, Filipe Laíns,
Bastien Nocera, Ping Cheng, Jason Gerecke, Viresh Kumar,
Johan Hovold, Alex Elder, Icenowy Zheng, linux-input,
linux-kernel, greybus-dev, linux-staging, linux-usb, stable
In-Reply-To: <agxswzzCNMcxRN1n@beelink>
On Tue, 19 May 2026, Benjamin Tissoires wrote:
> On May 19 2026, Greg Kroah-Hartman wrote:
> > On Tue, May 19, 2026 at 02:46:13PM +0200, Benjamin Tissoires wrote:
> > > On May 19 2026, Lee Jones wrote:
> > > > On Tue, 12 May 2026, Lee Jones wrote:
> > > >
> > > > > On Wed, 06 May 2026, Lee Jones wrote:
> > > > >
> > > > > > On Mon, 04 May 2026, Benjamin Tissoires wrote:
> > > > > >
> > > > > > > Commit 0a3fe972a7cb ("HID: core: Mitigate potential OOB by removing
> > > > > > > bogus memset()") enforced the provided data to be at least the size of
> > > > > > > the declared buffer in the report descriptor to prevent a buffer
> > > > > > > overflow.
> > > > > > >
> > > > > > > We only had corner cases of malicious devices exposing the OOM because
> > > > > > > in most cases, the buffer provided by the transport layer needs to be
> > > > > > > allocated at probe time and is large enough to handle all the possible
> > > > > > > reports.
> > > > > > >
> > > > > > > However, the patch from above, which enforces the spec a little bit more
> > > > > > > introduced both regressions for devices not following the spec (not
> > > > > > > necesserally malicious), but also a stream of errors for those devices.
> > > > > > >
> > > > > > > Let's revert to the old behavior by giving more information to HID core
> > > > > > > to be able to decide whether it can or not memset the rest of the buffer
> > > > > > > to 0 and continue the processing.
> > > > > > >
> > > > > > > Note that the first commit makes an API change, but the callers are
> > > > > > > relatively limited, so it should be fine on its own. The second patch
> > > > > > > can't really make the same kind of API change because we have too many
> > > > > > > callers in various subsystems. We can switch them one by one to the safe
> > > > > > > approach when needed.
> > > > > > >
> > > > > > > The last 2 patches are small cleanups I initially put together with the
> > > > > > > 2 first patches, but they can be applied on their own and don't need to
> > > > > > > be pulled in stable like the first 2.
> > > > > > >
> > > > > > > Cheers,
> > > > > > > Benjamin
> > > > > > >
> > > > > > > Signed-off-by: Benjamin Tissoires <bentiss@kernel.org>
> > > > > > > ---
> > > > > > > Changes in v3:
> > > > > > > - fixed ghib -> ghid in greybus
> > > > > > > - fixed i386 size_t debug size reported by kernel-bot
> > > > > > > - Link to v2: https://lore.kernel.org/r/20260416-wip-fix-core-v2-0-be92570e5627@kernel.org
> > > > > > >
> > > > > > > Changes in v2:
> > > > > > > - added a small blurb explaining the difference between the safe and the
> > > > > > > non safe version of hid_safe_input_report
> > > > > > > - Link to v1: https://lore.kernel.org/r/20260415-wip-fix-core-v1-0-ed3c4c823175@kernel.org
> > > > > > >
> > > > > > > ---
> > > > > > > Benjamin Tissoires (4):
> > > > > > > HID: pass the buffer size to hid_report_raw_event
> > > > > > > HID: core: introduce hid_safe_input_report()
> > > > > > > HID: multitouch: use __free(kfree) to clean up temporary buffers
> > > > > > > HID: wacom: use __free(kfree) to clean up temporary buffers
> > > > > > >
> > > > > > > drivers/hid/bpf/hid_bpf_dispatch.c | 6 ++--
> > > > > > > drivers/hid/hid-core.c | 67 ++++++++++++++++++++++++++++++--------
> > > > > > > drivers/hid/hid-gfrm.c | 4 +--
> > > > > > > drivers/hid/hid-logitech-hidpp.c | 2 +-
> > > > > > > drivers/hid/hid-multitouch.c | 18 ++++------
> > > > > > > drivers/hid/hid-primax.c | 2 +-
> > > > > > > drivers/hid/hid-vivaldi-common.c | 2 +-
> > > > > > > drivers/hid/i2c-hid/i2c-hid-core.c | 7 ++--
> > > > > > > drivers/hid/usbhid/hid-core.c | 11 ++++---
> > > > > > > drivers/hid/wacom_sys.c | 46 +++++++++-----------------
> > > > > > > drivers/staging/greybus/hid.c | 2 +-
> > > > > > > include/linux/hid.h | 6 ++--
> > > > > > > include/linux/hid_bpf.h | 14 +++++---
> > > > > > > 13 files changed, 109 insertions(+), 78 deletions(-)
> > > > > >
> > > > > > What's the plan for this set Benjamin? -rcs or -next?
> > > > >
> > > > > Are there any updates on this set please?
> > > > >
> > > > > FYI, this set is still important to us.
> > > > >
> > > > > Ideally, if all is well, it would go into the -rcs for v7.1.
> > > >
> > > > I'm still actively tracking these.
> > > >
> > > > It looks like Mark has been reverting them from -next and I'm getting
> > > > complaints from the Stable folks that they are causing build errors.
> > > >
> > > > drivers/hid/hid-core.c: In function 'hid_safe_input_report':
> > > > drivers/hid/hid-core.c:2195:16: error: too many arguments to function '__hid_input_report'
> > > > 2195 | return __hid_input_report(hid, type, data, bufsize, size, interrupt, 0,
> > > >
> > > > Are you folks still working on this set?
> > >
> > > Well, everything is in Linus' tree:
> > >
> > > not yet in a released rc (taken yesterday by Linus directly):
> > >
> > > 4d3a2a466b8d HID: core: Fix size_t specifier in hid_report_raw_event()
> > >
> > > Already in 7.1-rc4:
> > >
> > > 206342541fc8 HID: core: introduce hid_safe_input_report()
> > > 2c85c61d1332 HID: pass the buffer size to hid_report_raw_event
> > >
> > > Not sure why the patches don't apply to stable, but from an upstream
> > > subsystem point of view, everything is in order.
> >
> > We dropped them from stable because of the build breakage :(
> >
>
> If that was just the i386 size_t issue, then it has been fixed in Linus'
> tree. Could you try retaking them? Please?
Thanks for following-up on this, I really appreciate it.
It looks as though these are no longer reverted from today's -next. \o/
--
Lee Jones
^ permalink raw reply
* Re: (subset) [PATCH 1/9] dt-bindings: mfd: mediatek: mt6397: Add rtc for MT6359
From: Lee Jones @ 2026-05-20 14:40 UTC (permalink / raw)
To: Sen Chu, Sean Wang, Macpaul Lin, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Dmitry Torokhov, Chen Zhong,
Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Louis-Alexis Eyraud
Cc: kernel, linux-pm, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, linux-input, linux-iio
In-Reply-To: <20260429-mediatek-genio-mt6365-cleanup-v1-1-6f43838be92f@collabora.com>
On Wed, 29 Apr 2026 11:44:14 +0200, Louis-Alexis Eyraud wrote:
> The rtc block of MT6359 PMIC is compatible with the one found in MT6358
> but this compatibility was never expressed in the dt-bindings, so add
> the missing compatible string for the rtc subnode.
Applied, thanks!
[1/9] dt-bindings: mfd: mediatek: mt6397: Add rtc for MT6359
commit: 4edf75f835bad0b7e7ac022594b129f9dbf8698c
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: (subset) [PATCH 2/9] dt-bindings: mfd: mediatek: mt6397: Add MT6365 PMIC support
From: Lee Jones @ 2026-05-20 14:41 UTC (permalink / raw)
To: Sen Chu, Sean Wang, Macpaul Lin, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
AngeloGioacchino Del Regno, Dmitry Torokhov, Chen Zhong,
Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Louis-Alexis Eyraud
Cc: kernel, linux-pm, devicetree, linux-kernel, linux-arm-kernel,
linux-mediatek, linux-input, linux-iio
In-Reply-To: <20260429-mediatek-genio-mt6365-cleanup-v1-2-6f43838be92f@collabora.com>
On Wed, 29 Apr 2026 11:44:15 +0200, Louis-Alexis Eyraud wrote:
> MT6365 PMIC is compatible with MT6359, so add the compatible strings
> for the main and sub devices (regulator, rtc, audio codec).
Applied, thanks!
[2/9] dt-bindings: mfd: mediatek: mt6397: Add MT6365 PMIC support
commit: 5a7c6466133eee4370995ad5ccf4c377011a381e
--
Lee Jones [李琼斯]
^ permalink raw reply
* Re: [PATCH v2] input: zforce_ts: Remove error print for devm_add_action_or_reset()
From: Waqar Hameed @ 2026-05-20 14:55 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: kernel, linux-input, linux-kernel
In-Reply-To: <pndfrbqj2u6.a.out@axis.com>
On Fri, Oct 10, 2025 at 15:36 +0200 Waqar Hameed <waqar.hameed@axis.com> wrote:
> On Tue, Aug 05, 2025 at 11:33 +0200 Waqar Hameed <waqar.hameed@axis.com> wrote:
[...]
> Friendly ping incoming!
Friendly ping 2 incoming!
This will be the last ping for the original patch [1]. I'll interpret
the silence as a NACK (though it would preferable to get an explicit
one...). All other patches in the previous patch series has been merged
[2].
[1] https://lore.kernel.org/lkml/pndbjoum7td.a.out@axis.com/
[2] https://lore.kernel.org/all/pnd7c0s6ji2.fsf@axis.com/
^ permalink raw reply
* Re: [PATCH v5 5/9] mfd: mt6397: Add support for MT6392 PMIC
From: Lee Jones @ 2026-05-20 15:00 UTC (permalink / raw)
To: Luca Leonardo Scorcia
Cc: linux-mediatek, Fabien Parent, Val Packett,
AngeloGioacchino Del Regno, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Sen Chu, Sean Wang,
Macpaul Lin, Matthias Brugger, Linus Walleij, Liam Girdwood,
Mark Brown, Gary Bisson, Louis-Alexis Eyraud, Julien Massot,
Akari Tsuyukusa, Chen Zhong, linux-input, devicetree,
linux-kernel, linux-pm, linux-arm-kernel, linux-gpio
In-Reply-To: <20260420213529.1645560-6-l.scorcia@gmail.com>
On Mon, 20 Apr 2026, Luca Leonardo Scorcia wrote:
> From: Fabien Parent <parent.f@gmail.com>
>
> Align the MT6397 PMIC driver to other MFD drivers by passing only an
> identifier through mt6397_of_match[*].data and add support for the MT6392
> PMIC.
>
> Signed-off-by: Fabien Parent <parent.f@gmail.com>
> Signed-off-by: Val Packett <val@packett.cool>
> Signed-off-by: Luca Leonardo Scorcia <l.scorcia@gmail.com>
> Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
> ---
> drivers/mfd/mt6397-core.c | 118 +++++--
> drivers/mfd/mt6397-irq.c | 8 +
> include/linux/mfd/mt6392/core.h | 42 +++
> include/linux/mfd/mt6392/registers.h | 487 +++++++++++++++++++++++++++
> include/linux/mfd/mt6397/core.h | 1 +
> 5 files changed, 630 insertions(+), 26 deletions(-)
> create mode 100644 include/linux/mfd/mt6392/core.h
> create mode 100644 include/linux/mfd/mt6392/registers.h
>
> diff --git "a/drivers/mfd/mt6397-core.c" "b/drivers/mfd/mt6397-core.c"
> index 1bdacda9a933..5c5c24517c00 100644
> --- "a/drivers/mfd/mt6397-core.c"
> +++ "b/drivers/mfd/mt6397-core.c"
> @@ -18,6 +18,7 @@
> #include <linux/mfd/mt6357/core.h>
> #include <linux/mfd/mt6358/core.h>
> #include <linux/mfd/mt6359/core.h>
> +#include <linux/mfd/mt6392/core.h>
> #include <linux/mfd/mt6397/core.h>
> #include <linux/mfd/mt6323/registers.h>
> #include <linux/mfd/mt6328/registers.h>
> @@ -25,8 +26,20 @@
> #include <linux/mfd/mt6357/registers.h>
> #include <linux/mfd/mt6358/registers.h>
> #include <linux/mfd/mt6359/registers.h>
> +#include <linux/mfd/mt6392/registers.h>
> #include <linux/mfd/mt6397/registers.h>
>
> +enum mfd_match_data {
> + MATCH_DATA_MT6323 = 23,
> + MATCH_DATA_MT6328 = 28,
> + MATCH_DATA_MT6331 = 31,
> + MATCH_DATA_MT6357 = 57,
> + MATCH_DATA_MT6358 = 58,
> + MATCH_DATA_MT6359 = 59,
> + MATCH_DATA_MT6392 = 92,
> + MATCH_DATA_MT6397 = 97,
> +};
> +
> #define MT6323_RTC_BASE 0x8000
> #define MT6323_RTC_SIZE 0x40
>
> @@ -39,6 +52,9 @@
> #define MT6358_RTC_BASE 0x0588
> #define MT6358_RTC_SIZE 0x3c
>
> +#define MT6392_RTC_BASE 0x8000
> +#define MT6392_RTC_SIZE 0x3e
> +
> #define MT6397_RTC_BASE 0xe000
> #define MT6397_RTC_SIZE 0x3e
>
> @@ -65,6 +81,11 @@ static const struct resource mt6358_rtc_resources[] = {
> DEFINE_RES_IRQ(MT6358_IRQ_RTC),
> };
>
> +static const struct resource mt6392_rtc_resources[] = {
> + DEFINE_RES_MEM(MT6392_RTC_BASE, MT6392_RTC_SIZE),
> + DEFINE_RES_IRQ(MT6392_IRQ_RTC),
> +};
> +
> static const struct resource mt6397_rtc_resources[] = {
> DEFINE_RES_MEM(MT6397_RTC_BASE, MT6397_RTC_SIZE),
> DEFINE_RES_IRQ(MT6397_IRQ_RTC),
> @@ -114,6 +135,11 @@ static const struct resource mt6331_keys_resources[] = {
> DEFINE_RES_IRQ_NAMED(MT6331_IRQ_STATUS_HOMEKEY, "homekey"),
> };
>
> +static const struct resource mt6392_keys_resources[] = {
> + DEFINE_RES_IRQ_NAMED(MT6392_IRQ_PWRKEY, "powerkey"),
> + DEFINE_RES_IRQ_NAMED(MT6392_IRQ_FCHRKEY, "homekey"),
What does FCHRKEY mean? Is that a datasheet name?
> +};
> +
> static const struct resource mt6397_keys_resources[] = {
> DEFINE_RES_IRQ_NAMED(MT6397_IRQ_PWRKEY, "powerkey"),
> DEFINE_RES_IRQ_NAMED(MT6397_IRQ_HOMEKEY, "homekey"),
> @@ -253,6 +279,25 @@ static const struct mfd_cell mt6359_devs[] = {
> },
> };
>
> +static const struct mfd_cell mt6392_devs[] = {
> + {
> + .name = "mt6392-rtc",
> + .num_resources = ARRAY_SIZE(mt6392_rtc_resources),
> + .resources = mt6392_rtc_resources,
> + .of_compatible = "mediatek,mt6392-rtc",
> + }, {
> + .name = "mt6392-regulator",
> + }, {
> + .name = "mt6392-pinctrl",
> + .of_compatible = "mediatek,mt6392-pinctrl",
> + }, {
> + .name = "mt6392-keys",
> + .num_resources = ARRAY_SIZE(mt6392_keys_resources),
> + .resources = mt6392_keys_resources,
> + .of_compatible = "mediatek,mt6392-keys"
> + },
MFD_CELL_*() for each.
> +};
> +
> static const struct mfd_cell mt6397_devs[] = {
> {
> .name = "mt6397-rtc",
> @@ -335,6 +380,14 @@ static const struct chip_data mt6359_core = {
> .irq_init = mt6358_irq_init,
> };
>
> +static const struct chip_data mt6392_core = {
> + .cid_addr = MT6392_CID,
> + .cid_shift = 0,
> + .cells = mt6392_devs,
> + .cell_size = ARRAY_SIZE(mt6392_devs),
> + .irq_init = mt6397_irq_init,
> +};
> +
> static const struct chip_data mt6397_core = {
> .cid_addr = MT6397_CID,
> .cid_shift = 0,
> @@ -349,6 +402,7 @@ static int mt6397_probe(struct platform_device *pdev)
> unsigned int id = 0;
> struct mt6397_chip *pmic;
> const struct chip_data *pmic_core;
> + enum mfd_match_data device_data;
>
> pmic = devm_kzalloc(&pdev->dev, sizeof(*pmic), GFP_KERNEL);
> if (!pmic)
> @@ -364,9 +418,36 @@ static int mt6397_probe(struct platform_device *pdev)
> if (!pmic->regmap)
> return -ENODEV;
>
> - pmic_core = of_device_get_match_data(&pdev->dev);
> - if (!pmic_core)
> + device_data = (unsigned int)(uintptr_t)of_device_get_match_data(&pdev->dev);
We should be using generic match data variants now like device_get_match_data().
device_data should be chip_id or something.
> + switch (device_data) {
> + case MATCH_DATA_MT6323:
Pick some better nomenclature.
Ignore the fact that we're using these values in match data.
Do they have real differences, ideally ones which can be read from h/w?
> + pmic_core = &mt6323_core;
> + break;
> + case MATCH_DATA_MT6328:
> + pmic_core = &mt6328_core;
> + break;
> + case MATCH_DATA_MT6331:
> + pmic_core = &mt6331_mt6332_core;
> + break;
> + case MATCH_DATA_MT6357:
> + pmic_core = &mt6357_core;
> + break;
> + case MATCH_DATA_MT6358:
> + pmic_core = &mt6358_core;
> + break;
> + case MATCH_DATA_MT6359:
> + pmic_core = &mt6359_core;
> + break;
> + case MATCH_DATA_MT6392:
> + pmic_core = &mt6392_core;
> + break;
> + case MATCH_DATA_MT6397:
> + pmic_core = &mt6397_core;
> + break;
> + default:
> + dev_err(&pdev->dev, "Unknown device match data %u\n", device_data);
The user doesn't care about your match data.
"Device not supported"
> return -ENODEV;
> + }
>
> ret = regmap_read(pmic->regmap, pmic_core->cid_addr, &id);
> if (ret) {
> @@ -398,30 +479,15 @@ static int mt6397_probe(struct platform_device *pdev)
> }
>
> static const struct of_device_id mt6397_of_match[] = {
> - {
> - .compatible = "mediatek,mt6323",
> - .data = &mt6323_core,
> - }, {
> - .compatible = "mediatek,mt6328",
> - .data = &mt6328_core,
> - }, {
> - .compatible = "mediatek,mt6331",
> - .data = &mt6331_mt6332_core,
> - }, {
> - .compatible = "mediatek,mt6357",
> - .data = &mt6357_core,
> - }, {
> - .compatible = "mediatek,mt6358",
> - .data = &mt6358_core,
> - }, {
> - .compatible = "mediatek,mt6359",
> - .data = &mt6359_core,
> - }, {
> - .compatible = "mediatek,mt6397",
> - .data = &mt6397_core,
> - }, {
> - /* sentinel */
> - }
> + { .compatible = "mediatek,mt6323", .data = (void *)MATCH_DATA_MT6323, },
> + { .compatible = "mediatek,mt6328", .data = (void *)MATCH_DATA_MT6328, },
> + { .compatible = "mediatek,mt6331", .data = (void *)MATCH_DATA_MT6331, },
> + { .compatible = "mediatek,mt6357", .data = (void *)MATCH_DATA_MT6357, },
> + { .compatible = "mediatek,mt6358", .data = (void *)MATCH_DATA_MT6358, },
> + { .compatible = "mediatek,mt6359", .data = (void *)MATCH_DATA_MT6359, },
> + { .compatible = "mediatek,mt6392", .data = (void *)MATCH_DATA_MT6392, },
> + { .compatible = "mediatek,mt6397", .data = (void *)MATCH_DATA_MT6397, },
This is much better.
Maybe we can get rid of the bespoke 'struct chip_data' now?
> + { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, mt6397_of_match);
>
> diff --git "a/drivers/mfd/mt6397-irq.c" "b/drivers/mfd/mt6397-irq.c"
> index 5d2e5459f744..80ea5b92d232 100644
> --- "a/drivers/mfd/mt6397-irq.c"
> +++ "b/drivers/mfd/mt6397-irq.c"
> @@ -15,6 +15,8 @@
> #include <linux/mfd/mt6328/registers.h>
> #include <linux/mfd/mt6331/core.h>
> #include <linux/mfd/mt6331/registers.h>
> +#include <linux/mfd/mt6392/core.h>
> +#include <linux/mfd/mt6392/registers.h>
> #include <linux/mfd/mt6397/core.h>
> #include <linux/mfd/mt6397/registers.h>
>
> @@ -203,6 +205,12 @@ int mt6397_irq_init(struct mt6397_chip *chip)
> chip->int_status[0] = MT6397_INT_STATUS0;
> chip->int_status[1] = MT6397_INT_STATUS1;
> break;
> + case MT6392_CHIP_ID:
> + chip->int_con[0] = MT6392_INT_CON0;
> + chip->int_con[1] = MT6392_INT_CON1;
> + chip->int_status[0] = MT6392_INT_STATUS0;
> + chip->int_status[1] = MT6392_INT_STATUS1;
> + break;
>
> default:
> dev_err(chip->dev, "unsupported chip: 0x%x\n", chip->chip_id);
> diff --git "a/include/linux/mfd/mt6392/core.h" "b/include/linux/mfd/mt6392/core.h"
> new file mode 100644
> index 000000000000..4780dab4da92
> --- /dev/null
> +++ "b/include/linux/mfd/mt6392/core.h"
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2020 MediaTek Inc.
This needs an update.
> + * Author: Chen Zhong <chen.zhong@mediatek.com>
> + */
> +
> +#ifndef __MFD_MT6392_CORE_H__
> +#define __MFD_MT6392_CORE_H__
> +
> +enum mt6392_irq_numbers {
> + MT6392_IRQ_SPKL_AB = 0,
> + MT6392_IRQ_SPKL,
> + MT6392_IRQ_BAT_L,
> + MT6392_IRQ_BAT_H,
> + MT6392_IRQ_WATCHDOG,
> + MT6392_IRQ_PWRKEY,
> + MT6392_IRQ_THR_L,
> + MT6392_IRQ_THR_H,
> + MT6392_IRQ_VBATON_UNDET,
> + MT6392_IRQ_BVALID_DET,
> + MT6392_IRQ_CHRDET,
> + MT6392_IRQ_OV,
> + MT6392_IRQ_LDO = 16,
> + MT6392_IRQ_FCHRKEY,
> + MT6392_IRQ_RELEASE_PWRKEY,
> + MT6392_IRQ_RELEASE_FCHRKEY,
> + MT6392_IRQ_RTC,
> + MT6392_IRQ_VPROC,
> + MT6392_IRQ_VSYS,
> + MT6392_IRQ_VCORE,
> + MT6392_IRQ_TYPE_C_CC,
> + MT6392_IRQ_TYPEC_H_MAX,
> + MT6392_IRQ_TYPEC_H_MIN,
> + MT6392_IRQ_TYPEC_L_MAX,
> + MT6392_IRQ_TYPEC_L_MIN,
> + MT6392_IRQ_THR_MAX,
> + MT6392_IRQ_THR_MIN,
> + MT6392_IRQ_NAG_C_DLTV,
> + MT6392_IRQ_NR,
> +};
> +
> +#endif /* __MFD_MT6392_CORE_H__ */
> diff --git "a/include/linux/mfd/mt6392/registers.h" "b/include/linux/mfd/mt6392/registers.h"
> new file mode 100644
> index 000000000000..4f3a6db830d1
> --- /dev/null
> +++ "b/include/linux/mfd/mt6392/registers.h"
> @@ -0,0 +1,487 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Copyright (c) 2020 MediaTek Inc.
Same and throughout.
> + * Author: Chen Zhong <chen.zhong@mediatek.com>
> + */
> +
> +#ifndef __MFD_MT6392_REGISTERS_H__
> +#define __MFD_MT6392_REGISTERS_H__
> +
> +/* PMIC Registers */
> +#define MT6392_CHR_CON0 0x0000
> +#define MT6392_CHR_CON1 0x0002
> +#define MT6392_CHR_CON2 0x0004
> +#define MT6392_CHR_CON3 0x0006
> +#define MT6392_CHR_CON4 0x0008
> +#define MT6392_CHR_CON5 0x000A
> +#define MT6392_CHR_CON6 0x000C
> +#define MT6392_CHR_CON7 0x000E
> +#define MT6392_CHR_CON8 0x0010
> +#define MT6392_CHR_CON9 0x0012
> +#define MT6392_CHR_CON10 0x0014
> +#define MT6392_CHR_CON11 0x0016
> +#define MT6392_CHR_CON12 0x0018
> +#define MT6392_CHR_CON13 0x001A
> +#define MT6392_CHR_CON14 0x001C
> +#define MT6392_CHR_CON15 0x001E
> +#define MT6392_CHR_CON16 0x0020
> +#define MT6392_CHR_CON17 0x0022
> +#define MT6392_CHR_CON18 0x0024
> +#define MT6392_CHR_CON19 0x0026
> +#define MT6392_CHR_CON20 0x0028
> +#define MT6392_CHR_CON21 0x002A
> +#define MT6392_CHR_CON22 0x002C
> +#define MT6392_CHR_CON23 0x002E
> +#define MT6392_CHR_CON24 0x0030
> +#define MT6392_CHR_CON25 0x0032
> +#define MT6392_CHR_CON26 0x0034
> +#define MT6392_CHR_CON27 0x0036
> +#define MT6392_CHR_CON28 0x0038
> +#define MT6392_CHR_CON29 0x003A
> +#define MT6392_STRUP_CON0 0x003C
> +#define MT6392_STRUP_CON2 0x003E
> +#define MT6392_STRUP_CON3 0x0040
> +#define MT6392_STRUP_CON4 0x0042
> +#define MT6392_STRUP_CON5 0x0044
> +#define MT6392_STRUP_CON6 0x0046
> +#define MT6392_STRUP_CON7 0x0048
> +#define MT6392_STRUP_CON8 0x004A
> +#define MT6392_STRUP_CON9 0x004C
> +#define MT6392_STRUP_CON10 0x004E
> +#define MT6392_STRUP_CON11 0x0050
> +#define MT6392_SPK_CON0 0x0052
> +#define MT6392_SPK_CON1 0x0054
> +#define MT6392_SPK_CON2 0x0056
> +#define MT6392_SPK_CON6 0x005E
> +#define MT6392_SPK_CON7 0x0060
> +#define MT6392_SPK_CON8 0x0062
> +#define MT6392_SPK_CON9 0x0064
> +#define MT6392_SPK_CON10 0x0066
> +#define MT6392_SPK_CON11 0x0068
> +#define MT6392_SPK_CON12 0x006A
> +#define MT6392_STRUP_CON12 0x006E
> +#define MT6392_STRUP_CON13 0x0070
> +#define MT6392_STRUP_CON14 0x0072
> +#define MT6392_STRUP_CON15 0x0074
> +#define MT6392_STRUP_CON16 0x0076
> +#define MT6392_STRUP_CON17 0x0078
> +#define MT6392_STRUP_CON18 0x007A
> +#define MT6392_STRUP_CON19 0x007C
> +#define MT6392_STRUP_CON20 0x007E
> +#define MT6392_CID 0x0100
> +#define MT6392_TOP_CKPDN0 0x0102
> +#define MT6392_TOP_CKPDN0_SET 0x0104
> +#define MT6392_TOP_CKPDN0_CLR 0x0106
> +#define MT6392_TOP_CKPDN1 0x0108
> +#define MT6392_TOP_CKPDN1_SET 0x010A
> +#define MT6392_TOP_CKPDN1_CLR 0x010C
> +#define MT6392_TOP_CKPDN2 0x010E
> +#define MT6392_TOP_CKPDN2_SET 0x0110
> +#define MT6392_TOP_CKPDN2_CLR 0x0112
> +#define MT6392_TOP_RST_CON 0x0114
> +#define MT6392_TOP_RST_CON_SET 0x0116
> +#define MT6392_TOP_RST_CON_CLR 0x0118
> +#define MT6392_TOP_RST_MISC 0x011A
> +#define MT6392_TOP_RST_MISC_SET 0x011C
> +#define MT6392_TOP_RST_MISC_CLR 0x011E
> +#define MT6392_TOP_CKCON0 0x0120
> +#define MT6392_TOP_CKCON0_SET 0x0122
> +#define MT6392_TOP_CKCON0_CLR 0x0124
> +#define MT6392_TOP_CKCON1 0x0126
> +#define MT6392_TOP_CKCON1_SET 0x0128
> +#define MT6392_TOP_CKCON1_CLR 0x012A
> +#define MT6392_TOP_CKTST0 0x012C
> +#define MT6392_TOP_CKTST1 0x012E
> +#define MT6392_TOP_CKTST2 0x0130
> +#define MT6392_TEST_OUT 0x0132
> +#define MT6392_TEST_CON0 0x0134
> +#define MT6392_TEST_CON1 0x0136
> +#define MT6392_EN_STATUS0 0x0138
> +#define MT6392_EN_STATUS1 0x013A
> +#define MT6392_OCSTATUS0 0x013C
> +#define MT6392_OCSTATUS1 0x013E
> +#define MT6392_PGSTATUS 0x0140
> +#define MT6392_CHRSTATUS 0x0142
> +#define MT6392_TDSEL_CON 0x0144
> +#define MT6392_RDSEL_CON 0x0146
> +#define MT6392_SMT_CON0 0x0148
> +#define MT6392_SMT_CON1 0x014A
> +#define MT6392_DRV_CON0 0x0152
> +#define MT6392_DRV_CON1 0x0154
> +#define MT6392_INT_CON0 0x0160
> +#define MT6392_INT_CON0_SET 0x0162
> +#define MT6392_INT_CON0_CLR 0x0164
> +#define MT6392_INT_CON1 0x0166
> +#define MT6392_INT_CON1_SET 0x0168
> +#define MT6392_INT_CON1_CLR 0x016A
> +#define MT6392_INT_MISC_CON 0x016C
> +#define MT6392_INT_MISC_CON_SET 0x016E
> +#define MT6392_INT_MISC_CON_CLR 0x0170
> +#define MT6392_INT_STATUS0 0x0172
> +#define MT6392_INT_STATUS1 0x0174
> +#define MT6392_OC_GEAR_0 0x0176
> +#define MT6392_OC_GEAR_1 0x0178
> +#define MT6392_OC_GEAR_2 0x017A
> +#define MT6392_OC_CTL_VPROC 0x017C
> +#define MT6392_OC_CTL_VSYS 0x017E
> +#define MT6392_OC_CTL_VCORE 0x0180
> +#define MT6392_FQMTR_CON0 0x0182
> +#define MT6392_FQMTR_CON1 0x0184
> +#define MT6392_FQMTR_CON2 0x0186
> +#define MT6392_RG_SPI_CON 0x0188
> +#define MT6392_DEW_DIO_EN 0x018A
> +#define MT6392_DEW_READ_TEST 0x018C
> +#define MT6392_DEW_WRITE_TEST 0x018E
> +#define MT6392_DEW_CRC_SWRST 0x0190
> +#define MT6392_DEW_CRC_EN 0x0192
> +#define MT6392_DEW_CRC_VAL 0x0194
> +#define MT6392_DEW_DBG_MON_SEL 0x0196
> +#define MT6392_DEW_CIPHER_KEY_SEL 0x0198
> +#define MT6392_DEW_CIPHER_IV_SEL 0x019A
> +#define MT6392_DEW_CIPHER_EN 0x019C
> +#define MT6392_DEW_CIPHER_RDY 0x019E
> +#define MT6392_DEW_CIPHER_MODE 0x01A0
> +#define MT6392_DEW_CIPHER_SWRST 0x01A2
> +#define MT6392_DEW_RDDMY_NO 0x01A4
> +#define MT6392_DEW_RDATA_DLY_SEL 0x01A6
> +#define MT6392_CLK_TRIM_CON0 0x01A8
> +#define MT6392_BUCK_CON0 0x0200
> +#define MT6392_BUCK_CON1 0x0202
> +#define MT6392_BUCK_CON2 0x0204
> +#define MT6392_BUCK_CON3 0x0206
> +#define MT6392_BUCK_CON4 0x0208
> +#define MT6392_BUCK_CON5 0x020A
> +#define MT6392_VPROC_CON0 0x020C
> +#define MT6392_VPROC_CON1 0x020E
> +#define MT6392_VPROC_CON2 0x0210
> +#define MT6392_VPROC_CON3 0x0212
> +#define MT6392_VPROC_CON4 0x0214
> +#define MT6392_VPROC_CON5 0x0216
> +#define MT6392_VPROC_CON7 0x021A
> +#define MT6392_VPROC_CON8 0x021C
> +#define MT6392_VPROC_CON9 0x021E
> +#define MT6392_VPROC_CON10 0x0220
> +#define MT6392_VPROC_CON11 0x0222
> +#define MT6392_VPROC_CON12 0x0224
> +#define MT6392_VPROC_CON13 0x0226
> +#define MT6392_VPROC_CON14 0x0228
> +#define MT6392_VPROC_CON15 0x022A
> +#define MT6392_VPROC_CON18 0x0230
> +#define MT6392_VSYS_CON0 0x0232
> +#define MT6392_VSYS_CON1 0x0234
> +#define MT6392_VSYS_CON2 0x0236
> +#define MT6392_VSYS_CON3 0x0238
> +#define MT6392_VSYS_CON4 0x023A
> +#define MT6392_VSYS_CON5 0x023C
> +#define MT6392_VSYS_CON7 0x0240
> +#define MT6392_VSYS_CON8 0x0242
> +#define MT6392_VSYS_CON9 0x0244
> +#define MT6392_VSYS_CON10 0x0246
> +#define MT6392_VSYS_CON11 0x0248
> +#define MT6392_VSYS_CON12 0x024A
> +#define MT6392_VSYS_CON13 0x024C
> +#define MT6392_VSYS_CON14 0x024E
> +#define MT6392_VSYS_CON15 0x0250
> +#define MT6392_VSYS_CON18 0x0256
> +#define MT6392_BUCK_OC_CON0 0x0258
> +#define MT6392_BUCK_OC_CON1 0x025A
> +#define MT6392_BUCK_OC_CON2 0x025C
> +#define MT6392_BUCK_OC_CON3 0x025E
> +#define MT6392_BUCK_OC_CON4 0x0260
> +#define MT6392_BUCK_OC_VPROC_CON0 0x0262
> +#define MT6392_BUCK_OC_VCORE_CON0 0x0264
> +#define MT6392_BUCK_OC_VSYS_CON0 0x0266
> +#define MT6392_BUCK_ANA_MON_CON0 0x0268
> +#define MT6392_BUCK_EFUSE_OC_CON0 0x026A
> +#define MT6392_VCORE_CON0 0x0300
> +#define MT6392_VCORE_CON1 0x0302
> +#define MT6392_VCORE_CON2 0x0304
> +#define MT6392_VCORE_CON3 0x0306
> +#define MT6392_VCORE_CON4 0x0308
> +#define MT6392_VCORE_CON5 0x030A
> +#define MT6392_VCORE_CON7 0x030E
> +#define MT6392_VCORE_CON8 0x0310
> +#define MT6392_VCORE_CON9 0x0312
> +#define MT6392_VCORE_CON10 0x0314
> +#define MT6392_VCORE_CON11 0x0316
> +#define MT6392_VCORE_CON12 0x0318
> +#define MT6392_VCORE_CON13 0x031A
> +#define MT6392_VCORE_CON14 0x031C
> +#define MT6392_VCORE_CON15 0x031E
> +#define MT6392_VCORE_CON18 0x0324
> +#define MT6392_BUCK_K_CON0 0x032A
> +#define MT6392_BUCK_K_CON1 0x032C
> +#define MT6392_BUCK_K_CON2 0x032E
> +#define MT6392_ANALDO_CON0 0x0400
> +#define MT6392_ANALDO_CON1 0x0402
> +#define MT6392_ANALDO_CON2 0x0404
> +#define MT6392_ANALDO_CON3 0x0406
> +#define MT6392_ANALDO_CON4 0x0408
> +#define MT6392_ANALDO_CON6 0x040C
> +#define MT6392_ANALDO_CON7 0x040E
> +#define MT6392_ANALDO_CON8 0x0410
> +#define MT6392_ANALDO_CON10 0x0412
> +#define MT6392_ANALDO_CON15 0x0414
> +#define MT6392_ANALDO_CON16 0x0416
> +#define MT6392_ANALDO_CON17 0x0418
> +#define MT6392_ANALDO_CON21 0x0420
> +#define MT6392_ANALDO_CON22 0x0422
> +#define MT6392_ANALDO_CON23 0x0424
> +#define MT6392_ANALDO_CON24 0x0426
> +#define MT6392_ANALDO_CON25 0x0428
> +#define MT6392_ANALDO_CON26 0x042A
> +#define MT6392_ANALDO_CON27 0x042C
> +#define MT6392_ANALDO_CON28 0x042E
> +#define MT6392_ANALDO_CON29 0x0430
> +#define MT6392_DIGLDO_CON0 0x0500
> +#define MT6392_DIGLDO_CON2 0x0502
> +#define MT6392_DIGLDO_CON3 0x0504
> +#define MT6392_DIGLDO_CON5 0x0506
> +#define MT6392_DIGLDO_CON6 0x0508
> +#define MT6392_DIGLDO_CON7 0x050A
> +#define MT6392_DIGLDO_CON8 0x050C
> +#define MT6392_DIGLDO_CON10 0x0510
> +#define MT6392_DIGLDO_CON11 0x0512
> +#define MT6392_DIGLDO_CON12 0x0514
> +#define MT6392_DIGLDO_CON15 0x051A
> +#define MT6392_DIGLDO_CON20 0x0524
> +#define MT6392_DIGLDO_CON21 0x0526
> +#define MT6392_DIGLDO_CON23 0x0528
> +#define MT6392_DIGLDO_CON24 0x052A
> +#define MT6392_DIGLDO_CON26 0x052C
> +#define MT6392_DIGLDO_CON27 0x052E
> +#define MT6392_DIGLDO_CON28 0x0530
> +#define MT6392_DIGLDO_CON29 0x0532
> +#define MT6392_DIGLDO_CON30 0x0534
> +#define MT6392_DIGLDO_CON31 0x0536
> +#define MT6392_DIGLDO_CON32 0x0538
> +#define MT6392_DIGLDO_CON33 0x053A
> +#define MT6392_DIGLDO_CON36 0x0540
> +#define MT6392_DIGLDO_CON41 0x0546
> +#define MT6392_DIGLDO_CON44 0x054C
> +#define MT6392_DIGLDO_CON47 0x0552
> +#define MT6392_DIGLDO_CON48 0x0554
> +#define MT6392_DIGLDO_CON49 0x0556
> +#define MT6392_DIGLDO_CON50 0x0558
> +#define MT6392_DIGLDO_CON51 0x055A
> +#define MT6392_DIGLDO_CON52 0x055C
> +#define MT6392_DIGLDO_CON53 0x055E
> +#define MT6392_DIGLDO_CON54 0x0560
> +#define MT6392_DIGLDO_CON55 0x0562
> +#define MT6392_DIGLDO_CON56 0x0564
> +#define MT6392_DIGLDO_CON57 0x0566
> +#define MT6392_DIGLDO_CON58 0x0568
> +#define MT6392_DIGLDO_CON59 0x056A
> +#define MT6392_DIGLDO_CON60 0x056C
> +#define MT6392_DIGLDO_CON61 0x056E
> +#define MT6392_DIGLDO_CON62 0x0570
> +#define MT6392_DIGLDO_CON63 0x0572
> +#define MT6392_EFUSE_CON0 0x0600
> +#define MT6392_EFUSE_CON1 0x0602
> +#define MT6392_EFUSE_CON2 0x0604
> +#define MT6392_EFUSE_CON3 0x0606
> +#define MT6392_EFUSE_CON4 0x0608
> +#define MT6392_EFUSE_CON5 0x060A
> +#define MT6392_EFUSE_CON6 0x060C
> +#define MT6392_EFUSE_VAL_0_15 0x060E
> +#define MT6392_EFUSE_VAL_16_31 0x0610
> +#define MT6392_EFUSE_VAL_32_47 0x0612
> +#define MT6392_EFUSE_VAL_48_63 0x0614
> +#define MT6392_EFUSE_VAL_64_79 0x0616
> +#define MT6392_EFUSE_VAL_80_95 0x0618
> +#define MT6392_EFUSE_VAL_96_111 0x061A
> +#define MT6392_EFUSE_VAL_112_127 0x061C
> +#define MT6392_EFUSE_VAL_128_143 0x061E
> +#define MT6392_EFUSE_VAL_144_159 0x0620
> +#define MT6392_EFUSE_VAL_160_175 0x0622
> +#define MT6392_EFUSE_VAL_176_191 0x0624
> +#define MT6392_EFUSE_VAL_192_207 0x0626
> +#define MT6392_EFUSE_VAL_208_223 0x0628
> +#define MT6392_EFUSE_VAL_224_239 0x062A
> +#define MT6392_EFUSE_VAL_240_255 0x062C
> +#define MT6392_EFUSE_VAL_256_271 0x062E
> +#define MT6392_EFUSE_VAL_272_287 0x0630
> +#define MT6392_EFUSE_VAL_288_303 0x0632
> +#define MT6392_EFUSE_VAL_304_319 0x0634
> +#define MT6392_EFUSE_VAL_320_335 0x0636
> +#define MT6392_EFUSE_VAL_336_351 0x0638
> +#define MT6392_EFUSE_VAL_352_367 0x063A
> +#define MT6392_EFUSE_VAL_368_383 0x063C
> +#define MT6392_EFUSE_VAL_384_399 0x063E
> +#define MT6392_EFUSE_VAL_400_415 0x0640
> +#define MT6392_EFUSE_VAL_416_431 0x0642
> +#define MT6392_RTC_MIX_CON0 0x0644
> +#define MT6392_RTC_MIX_CON1 0x0646
> +#define MT6392_EFUSE_VAL_432_447 0x0648
> +#define MT6392_EFUSE_VAL_448_463 0x064A
> +#define MT6392_EFUSE_VAL_464_479 0x064C
> +#define MT6392_EFUSE_VAL_480_495 0x064E
> +#define MT6392_EFUSE_VAL_496_511 0x0650
> +#define MT6392_EFUSE_DOUT_0_15 0x0652
> +#define MT6392_EFUSE_DOUT_16_31 0x0654
> +#define MT6392_EFUSE_DOUT_32_47 0x0656
> +#define MT6392_EFUSE_DOUT_48_63 0x0658
> +#define MT6392_EFUSE_DOUT_64_79 0x065A
> +#define MT6392_EFUSE_DOUT_80_95 0x065C
> +#define MT6392_EFUSE_DOUT_96_111 0x065E
> +#define MT6392_EFUSE_DOUT_112_127 0x0660
> +#define MT6392_EFUSE_DOUT_128_143 0x0662
> +#define MT6392_EFUSE_DOUT_144_159 0x0664
> +#define MT6392_EFUSE_DOUT_160_175 0x0666
> +#define MT6392_EFUSE_DOUT_176_191 0x0668
> +#define MT6392_EFUSE_DOUT_192_207 0x066A
> +#define MT6392_EFUSE_DOUT_208_223 0x066C
> +#define MT6392_EFUSE_DOUT_224_239 0x066E
> +#define MT6392_EFUSE_DOUT_240_255 0x0670
> +#define MT6392_EFUSE_DOUT_256_271 0x0672
> +#define MT6392_EFUSE_DOUT_272_287 0x0674
> +#define MT6392_EFUSE_DOUT_288_303 0x0676
> +#define MT6392_EFUSE_DOUT_304_319 0x0678
> +#define MT6392_EFUSE_DOUT_320_335 0x067A
> +#define MT6392_EFUSE_DOUT_336_351 0x067C
> +#define MT6392_EFUSE_DOUT_352_367 0x067E
> +#define MT6392_EFUSE_DOUT_368_383 0x0680
> +#define MT6392_EFUSE_DOUT_384_399 0x0682
> +#define MT6392_EFUSE_DOUT_400_415 0x0684
> +#define MT6392_EFUSE_DOUT_416_431 0x0686
> +#define MT6392_EFUSE_DOUT_432_447 0x0688
> +#define MT6392_EFUSE_DOUT_448_463 0x068A
> +#define MT6392_EFUSE_DOUT_464_479 0x068C
> +#define MT6392_EFUSE_DOUT_480_495 0x068E
> +#define MT6392_EFUSE_DOUT_496_511 0x0690
> +#define MT6392_EFUSE_CON7 0x0692
> +#define MT6392_EFUSE_CON8 0x0694
> +#define MT6392_EFUSE_CON9 0x0696
> +#define MT6392_AUXADC_ADC0 0x0700
> +#define MT6392_AUXADC_ADC1 0x0702
> +#define MT6392_AUXADC_ADC2 0x0704
> +#define MT6392_AUXADC_ADC3 0x0706
> +#define MT6392_AUXADC_ADC4 0x0708
> +#define MT6392_AUXADC_ADC5 0x070A
> +#define MT6392_AUXADC_ADC6 0x070C
> +#define MT6392_AUXADC_ADC7 0x070E
> +#define MT6392_AUXADC_ADC8 0x0710
> +#define MT6392_AUXADC_ADC9 0x0712
> +#define MT6392_AUXADC_ADC10 0x0714
> +#define MT6392_AUXADC_ADC11 0x0716
> +#define MT6392_AUXADC_ADC12 0x0718
> +#define MT6392_AUXADC_ADC13 0x071A
> +#define MT6392_AUXADC_ADC14 0x071C
> +#define MT6392_AUXADC_ADC15 0x071E
> +#define MT6392_AUXADC_ADC16 0x0720
> +#define MT6392_AUXADC_ADC17 0x0722
> +#define MT6392_AUXADC_ADC18 0x0724
> +#define MT6392_AUXADC_ADC19 0x0726
> +#define MT6392_AUXADC_ADC20 0x0728
> +#define MT6392_AUXADC_ADC21 0x072A
> +#define MT6392_AUXADC_ADC22 0x072C
> +#define MT6392_AUXADC_STA0 0x072E
> +#define MT6392_AUXADC_STA1 0x0730
> +#define MT6392_AUXADC_RQST0 0x0732
> +#define MT6392_AUXADC_RQST0_SET 0x0734
> +#define MT6392_AUXADC_RQST0_CLR 0x0736
> +#define MT6392_AUXADC_CON0 0x0738
> +#define MT6392_AUXADC_CON0_SET 0x073A
> +#define MT6392_AUXADC_CON0_CLR 0x073C
> +#define MT6392_AUXADC_CON1 0x073E
> +#define MT6392_AUXADC_CON2 0x0740
> +#define MT6392_AUXADC_CON3 0x0742
> +#define MT6392_AUXADC_CON4 0x0744
> +#define MT6392_AUXADC_CON5 0x0746
> +#define MT6392_AUXADC_CON6 0x0748
> +#define MT6392_AUXADC_CON7 0x074A
> +#define MT6392_AUXADC_CON8 0x074C
> +#define MT6392_AUXADC_CON9 0x074E
> +#define MT6392_AUXADC_CON10 0x0750
> +#define MT6392_AUXADC_CON11 0x0752
> +#define MT6392_AUXADC_CON12 0x0754
> +#define MT6392_AUXADC_CON13 0x0756
> +#define MT6392_AUXADC_CON14 0x0758
> +#define MT6392_AUXADC_CON15 0x075A
> +#define MT6392_AUXADC_CON16 0x075C
> +#define MT6392_AUXADC_AUTORPT0 0x075E
> +#define MT6392_AUXADC_LBAT0 0x0760
> +#define MT6392_AUXADC_LBAT1 0x0762
> +#define MT6392_AUXADC_LBAT2 0x0764
> +#define MT6392_AUXADC_LBAT3 0x0766
> +#define MT6392_AUXADC_LBAT4 0x0768
> +#define MT6392_AUXADC_LBAT5 0x076A
> +#define MT6392_AUXADC_LBAT6 0x076C
> +#define MT6392_AUXADC_THR0 0x076E
> +#define MT6392_AUXADC_THR1 0x0770
> +#define MT6392_AUXADC_THR2 0x0772
> +#define MT6392_AUXADC_THR3 0x0774
> +#define MT6392_AUXADC_THR4 0x0776
> +#define MT6392_AUXADC_THR5 0x0778
> +#define MT6392_AUXADC_THR6 0x077A
> +#define MT6392_AUXADC_EFUSE0 0x077C
> +#define MT6392_AUXADC_EFUSE1 0x077E
> +#define MT6392_AUXADC_EFUSE2 0x0780
> +#define MT6392_AUXADC_EFUSE3 0x0782
> +#define MT6392_AUXADC_EFUSE4 0x0784
> +#define MT6392_AUXADC_EFUSE5 0x0786
> +#define MT6392_AUXADC_NAG_0 0x0788
> +#define MT6392_AUXADC_NAG_1 0x078A
> +#define MT6392_AUXADC_NAG_2 0x078C
> +#define MT6392_AUXADC_NAG_3 0x078E
> +#define MT6392_AUXADC_NAG_4 0x0790
> +#define MT6392_AUXADC_NAG_5 0x0792
> +#define MT6392_AUXADC_NAG_6 0x0794
> +#define MT6392_AUXADC_NAG_7 0x0796
> +#define MT6392_AUXADC_NAG_8 0x0798
> +#define MT6392_AUXADC_TYPEC_H_1 0x079A
> +#define MT6392_AUXADC_TYPEC_H_2 0x079C
> +#define MT6392_AUXADC_TYPEC_H_3 0x079E
> +#define MT6392_AUXADC_TYPEC_H_4 0x07A0
> +#define MT6392_AUXADC_TYPEC_H_5 0x07A2
> +#define MT6392_AUXADC_TYPEC_H_6 0x07A4
> +#define MT6392_AUXADC_TYPEC_H_7 0x07A6
> +#define MT6392_AUXADC_TYPEC_L_1 0x07A8
> +#define MT6392_AUXADC_TYPEC_L_2 0x07AA
> +#define MT6392_AUXADC_TYPEC_L_3 0x07AC
> +#define MT6392_AUXADC_TYPEC_L_4 0x07AE
> +#define MT6392_AUXADC_TYPEC_L_5 0x07B0
> +#define MT6392_AUXADC_TYPEC_L_6 0x07B2
> +#define MT6392_AUXADC_TYPEC_L_7 0x07B4
> +#define MT6392_AUXADC_NAG_9 0x07B6
> +#define MT6392_TYPE_C_PHY_RG_0 0x0800
> +#define MT6392_TYPE_C_PHY_RG_CC_RESERVE_CSR 0x0802
> +#define MT6392_TYPE_C_VCMP_CTRL 0x0804
> +#define MT6392_TYPE_C_CTRL 0x0806
> +#define MT6392_TYPE_C_CC_SW_CTRL 0x080a
> +#define MT6392_TYPE_C_CC_VOL_PERIODIC_MEAS_VAL 0x080c
> +#define MT6392_TYPE_C_CC_VOL_DEBOUNCE_CNT_VAL 0x080e
> +#define MT6392_TYPE_C_DRP_SRC_CNT_VAL_0 0x0810
> +#define MT6392_TYPE_C_DRP_SNK_CNT_VAL_0 0x0814
> +#define MT6392_TYPE_C_DRP_TRY_CNT_VAL_0 0x0818
> +#define MT6392_TYPE_C_CC_SRC_DEFAULT_DAC_VAL 0x0820
> +#define MT6392_TYPE_C_CC_SRC_15_DAC_VAL 0x0822
> +#define MT6392_TYPE_C_CC_SRC_30_DAC_VAL 0x0824
> +#define MT6392_TYPE_C_CC_SNK_DAC_VAL_0 0x0828
> +#define MT6392_TYPE_C_CC_SNK_DAC_VAL_1 0x082a
> +#define MT6392_TYPE_C_INTR_EN_0 0x0830
> +#define MT6392_TYPE_C_INTR_EN_2 0x0834
> +#define MT6392_TYPE_C_INTR_0 0x0838
> +#define MT6392_TYPE_C_INTR_2 0x083C
> +#define MT6392_TYPE_C_CC_STATUS 0x0840
> +#define MT6392_TYPE_C_PWR_STATUS 0x0842
> +#define MT6392_TYPE_C_PHY_RG_CC1_RESISTENCE_0 0x0844
> +#define MT6392_TYPE_C_PHY_RG_CC1_RESISTENCE_1 0x0846
> +#define MT6392_TYPE_C_PHY_RG_CC2_RESISTENCE_0 0x0848
> +#define MT6392_TYPE_C_PHY_RG_CC2_RESISTENCE_1 0x084a
> +#define MT6392_TYPE_C_CC_SW_FORCE_MODE_ENABLE_0 0x0860
> +#define MT6392_TYPE_C_CC_SW_FORCE_MODE_VAL_0 0x0864
> +#define MT6392_TYPE_C_CC_SW_FORCE_MODE_VAL_1 0x0866
> +#define MT6392_TYPE_C_CC_SW_FORCE_MODE_ENABLE_1 0x0868
> +#define MT6392_TYPE_C_CC_SW_FORCE_MODE_VAL_2 0x086c
> +#define MT6392_TYPE_C_CC_DAC_CALI_CTRL 0x0870
> +#define MT6392_TYPE_C_CC_DAC_CALI_RESULT 0x0872
> +#define MT6392_TYPE_C_DEBUG_PORT_SELECT_0 0x0880
> +#define MT6392_TYPE_C_DEBUG_PORT_SELECT_1 0x0882
> +#define MT6392_TYPE_C_DEBUG_MODE_SELECT 0x0884
> +#define MT6392_TYPE_C_DEBUG_OUT_READ_0 0x0888
> +#define MT6392_TYPE_C_DEBUG_OUT_READ_1 0x088a
> +#define MT6392_TYPE_C_SW_DEBUG_PORT_0 0x088c
> +#define MT6392_TYPE_C_SW_DEBUG_PORT_1 0x088e
> +
> +#endif /* __MFD_MT6392_REGISTERS_H__ */
> diff --git "a/include/linux/mfd/mt6397/core.h" "b/include/linux/mfd/mt6397/core.h"
> index 340fc72e22aa..3729a6856c13 100644
> --- "a/include/linux/mfd/mt6397/core.h"
> +++ "b/include/linux/mfd/mt6397/core.h"
> @@ -20,6 +20,7 @@ enum chip_id {
> MT6359_CHIP_ID = 0x59,
> MT6366_CHIP_ID = 0x66,
> MT6391_CHIP_ID = 0x91,
> + MT6392_CHIP_ID = 0x92,
This looks like a duplicated struct.
Can't we use this instead of the made-up 'mfd_match_data'?
> MT6397_CHIP_ID = 0x97,
> };
>
> --
> 2.43.0
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v5 5/6] mfd: motorola-cpcap: diverge configuration per-board
From: Lee Jones @ 2026-05-20 15:07 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pavel Machek, David Lechner, Tony Lindgren, linux-input,
devicetree, linux-kernel, linux-leds
In-Reply-To: <20260510110804.33045-6-clamor95@gmail.com>
On Sun, 10 May 2026, Svyatoslav Ryhel wrote:
> MFD have rigid subdevice structure which does not allow flexible dynamic
> subdevice linking. Address this by diverging CPCAP subdevice composition
> to take into account board specific configuration.
>
> Create a common default subdevice composition, rename existing subdevice
> composition into cpcap_mapphone_mfd_devices since it targets mainly
> Mapphone board.
>
> Removed st,6556002 as it is no longer applicable to all cases and
> duplicates motorola,cpcap, which is used as the default composition.
>
> Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> ---
> drivers/mfd/motorola-cpcap.c | 142 ++++++++++++++++-------------
> include/linux/mfd/motorola-cpcap.h | 6 ++
> 2 files changed, 87 insertions(+), 61 deletions(-)
Looking much better, thanks.
Nit: A patch-level changelog really is much more helpful to reviewers.
> diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> index d8243b956f87..f5a7fdd89dd5 100644
> --- a/drivers/mfd/motorola-cpcap.c
> +++ b/drivers/mfd/motorola-cpcap.c
> @@ -12,6 +12,7 @@
> #include <linux/kernel.h>
> #include <linux/module.h>
> #include <linux/mod_devicetable.h>
> +#include <linux/property.h>
> #include <linux/regmap.h>
> #include <linux/sysfs.h>
>
> @@ -30,6 +31,7 @@ struct cpcap_ddata {
> struct regmap_irq_chip_data *irqdata[CPCAP_NR_IRQ_CHIPS];
> const struct regmap_config *regmap_conf;
> struct regmap *regmap;
> + enum cpcap_variant variant;
> };
>
> static int cpcap_sense_irq(struct regmap *regmap, int irq)
> @@ -195,20 +197,6 @@ static int cpcap_init_irq(struct cpcap_ddata *cpcap)
> return 0;
> }
>
> -static const struct of_device_id cpcap_of_match[] = {
> - { .compatible = "motorola,cpcap", },
> - { .compatible = "st,6556002", },
> - {},
> -};
> -MODULE_DEVICE_TABLE(of, cpcap_of_match);
> -
> -static const struct spi_device_id cpcap_spi_ids[] = {
> - { .name = "cpcap", },
> - { .name = "6556002", },
> - {},
> -};
> -MODULE_DEVICE_TABLE(spi, cpcap_spi_ids);
> -
> static const struct regmap_config cpcap_regmap_config = {
> .reg_bits = 16,
> .reg_stride = 4,
> @@ -241,62 +229,76 @@ static int cpcap_resume(struct device *dev)
>
> static DEFINE_SIMPLE_DEV_PM_OPS(cpcap_pm, cpcap_suspend, cpcap_resume);
>
> -static const struct mfd_cell cpcap_mfd_devices[] = {
> - {
> - .name = "cpcap_adc",
> - .of_compatible = "motorola,mapphone-cpcap-adc",
> - }, {
> - .name = "cpcap_battery",
> - .of_compatible = "motorola,cpcap-battery",
> - }, {
> - .name = "cpcap-charger",
> - .of_compatible = "motorola,mapphone-cpcap-charger",
> - }, {
> - .name = "cpcap-regulator",
> - .of_compatible = "motorola,mapphone-cpcap-regulator",
> - }, {
> - .name = "cpcap-rtc",
> - .of_compatible = "motorola,cpcap-rtc",
> - }, {
> - .name = "cpcap-pwrbutton",
> - .of_compatible = "motorola,cpcap-pwrbutton",
> - }, {
> - .name = "cpcap-usb-phy",
> - .of_compatible = "motorola,mapphone-cpcap-usb-phy",
> - }, {
> - .name = "cpcap-led",
> - .id = 0,
> - .of_compatible = "motorola,cpcap-led-red",
> - }, {
> - .name = "cpcap-led",
> - .id = 1,
> - .of_compatible = "motorola,cpcap-led-green",
> - }, {
> - .name = "cpcap-led",
> - .id = 2,
> - .of_compatible = "motorola,cpcap-led-blue",
> - }, {
> - .name = "cpcap-led",
> - .id = 3,
> - .of_compatible = "motorola,cpcap-led-adl",
> - }, {
> - .name = "cpcap-led",
> - .id = 4,
> - .of_compatible = "motorola,cpcap-led-cp",
> - }, {
> - .name = "cpcap-codec",
> - }
> +static const struct mfd_cell cpcap_default_mfd_devices[] = {
> + MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,cpcap-adc"),
> + MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0,
> + "motorola,cpcap-battery"),
> + MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0,
> + "motorola,cpcap-regulator"),
> + MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> + MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0,
> + "motorola,cpcap-pwrbutton"),
> + MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0,
> + "motorola,cpcap-usb-phy"),
> + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> + MFD_CELL_NAME("cpcap-codec"),
> +};
Nit: I wouldn't complain if you wanted to have all of these on a single
line for neatness.
> +static const struct mfd_cell cpcap_mapphone_mfd_devices[] = {
> + MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0,
> + "motorola,mapphone-cpcap-adc"),
> + MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0,
> + "motorola,cpcap-battery"),
> + MFD_CELL_OF("cpcap-charger", NULL, NULL, 0, 0,
> + "motorola,mapphone-cpcap-charger"),
> + MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0,
> + "motorola,mapphone-cpcap-regulator"),
> + MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> + MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0,
> + "motorola,cpcap-pwrbutton"),
> + MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0,
> + "motorola,mapphone-cpcap-usb-phy"),
> + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> + MFD_CELL_NAME("cpcap-codec"),
> };
A lot of these are duplicated, right?
I would have a comment set, then the differences in separate containers.
> static int cpcap_probe(struct spi_device *spi)
> {
> struct cpcap_ddata *cpcap;
> + const struct mfd_cell *cells;
> + unsigned int num_cells;
> int ret;
>
> cpcap = devm_kzalloc(&spi->dev, sizeof(*cpcap), GFP_KERNEL);
> if (!cpcap)
> return -ENOMEM;
>
> + cpcap->variant = (enum cpcap_variant)spi_get_device_match_data(spi);
> + if (!cpcap->variant)
> + return -ENODEV;
Isn't this covered in the 'default' below?
> + switch (cpcap->variant) {
> + case CPCAP_DEFAULT:
> + cells = cpcap_default_mfd_devices;
> + num_cells = ARRAY_SIZE(cpcap_default_mfd_devices);
> + break;
> + case CPCAP_MAPPHONE:
> + cells = cpcap_mapphone_mfd_devices;
> + num_cells = ARRAY_SIZE(cpcap_mapphone_mfd_devices);
> + break;
> + default:
> + return dev_err_probe(&spi->dev, -EINVAL,
> + "Unknown device %d\n", cpcap->variant);
This should be -ENODEV.
> + }
> +
> cpcap->spi = spi;
> spi_set_drvdata(spi, cpcap);
>
> @@ -331,10 +333,28 @@ static int cpcap_probe(struct spi_device *spi)
> spi->dev.coherent_dma_mask = 0;
> spi->dev.dma_mask = &spi->dev.coherent_dma_mask;
>
> - return devm_mfd_add_devices(&spi->dev, 0, cpcap_mfd_devices,
> - ARRAY_SIZE(cpcap_mfd_devices), NULL, 0, NULL);
> + return devm_mfd_add_devices(&spi->dev, 0, cells, num_cells, NULL, 0, NULL);
> }
>
> +static const struct of_device_id cpcap_of_match[] = {
> + {
> + .compatible = "motorola,cpcap",
> + .data = (void *)CPCAP_DEFAULT
Single line should be fine.
> + }, {
> + .compatible = "motorola,mapphone-cpcap",
> + .data = (void *)CPCAP_MAPPHONE
> + },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(of, cpcap_of_match);
> +
> +static const struct spi_device_id cpcap_spi_ids[] = {
> + { "cpcap", CPCAP_DEFAULT },
> + { "mapphone-cpcap", CPCAP_MAPPHONE },
> + { /* sentinel */ }
> +};
> +MODULE_DEVICE_TABLE(spi, cpcap_spi_ids);
> +
> static struct spi_driver cpcap_driver = {
> .driver = {
> .name = "cpcap-core",
> diff --git a/include/linux/mfd/motorola-cpcap.h b/include/linux/mfd/motorola-cpcap.h
> index 981e5777deb7..1a85b06272c8 100644
> --- a/include/linux/mfd/motorola-cpcap.h
> +++ b/include/linux/mfd/motorola-cpcap.h
> @@ -25,6 +25,12 @@
> #define CPCAP_REVISION_2_0 0x10
> #define CPCAP_REVISION_2_1 0x11
>
> +enum cpcap_variant {
> + CPCAP_DEFAULT = 1,
> + CPCAP_MAPPHONE,
> + CPCAP_MAX
> +};
> +
> /* CPCAP registers */
> #define CPCAP_REG_INT1 0x0000 /* Interrupt 1 */
> #define CPCAP_REG_INT2 0x0004 /* Interrupt 2 */
> --
> 2.51.0
>
--
Lee Jones
^ permalink raw reply
* Re: [PATCH v5 5/6] mfd: motorola-cpcap: diverge configuration per-board
From: Svyatoslav Ryhel @ 2026-05-20 15:29 UTC (permalink / raw)
To: Lee Jones
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pavel Machek, David Lechner, Tony Lindgren, linux-input,
devicetree, linux-kernel, linux-leds
In-Reply-To: <20260520150758.GG2767592@google.com>
ср, 20 трав. 2026 р. о 18:08 Lee Jones <lee@kernel.org> пише:
>
> On Sun, 10 May 2026, Svyatoslav Ryhel wrote:
>
> > MFD have rigid subdevice structure which does not allow flexible dynamic
> > subdevice linking. Address this by diverging CPCAP subdevice composition
> > to take into account board specific configuration.
> >
> > Create a common default subdevice composition, rename existing subdevice
> > composition into cpcap_mapphone_mfd_devices since it targets mainly
> > Mapphone board.
> >
> > Removed st,6556002 as it is no longer applicable to all cases and
> > duplicates motorola,cpcap, which is used as the default composition.
> >
> > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > ---
> > drivers/mfd/motorola-cpcap.c | 142 ++++++++++++++++-------------
> > include/linux/mfd/motorola-cpcap.h | 6 ++
> > 2 files changed, 87 insertions(+), 61 deletions(-)
>
> Looking much better, thanks.
>
> Nit: A patch-level changelog really is much more helpful to reviewers.
>
Noted, but I will not guarantee that I will do patch-level changelogs, sorry.
> > diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
> > index d8243b956f87..f5a7fdd89dd5 100644
> > --- a/drivers/mfd/motorola-cpcap.c
> > +++ b/drivers/mfd/motorola-cpcap.c
> > @@ -12,6 +12,7 @@
> > #include <linux/kernel.h>
> > #include <linux/module.h>
> > #include <linux/mod_devicetable.h>
> > +#include <linux/property.h>
> > #include <linux/regmap.h>
> > #include <linux/sysfs.h>
> >
> > @@ -30,6 +31,7 @@ struct cpcap_ddata {
> > struct regmap_irq_chip_data *irqdata[CPCAP_NR_IRQ_CHIPS];
> > const struct regmap_config *regmap_conf;
> > struct regmap *regmap;
> > + enum cpcap_variant variant;
> > };
> >
> > static int cpcap_sense_irq(struct regmap *regmap, int irq)
> > @@ -195,20 +197,6 @@ static int cpcap_init_irq(struct cpcap_ddata *cpcap)
> > return 0;
> > }
> >
> > -static const struct of_device_id cpcap_of_match[] = {
> > - { .compatible = "motorola,cpcap", },
> > - { .compatible = "st,6556002", },
> > - {},
> > -};
> > -MODULE_DEVICE_TABLE(of, cpcap_of_match);
> > -
> > -static const struct spi_device_id cpcap_spi_ids[] = {
> > - { .name = "cpcap", },
> > - { .name = "6556002", },
> > - {},
> > -};
> > -MODULE_DEVICE_TABLE(spi, cpcap_spi_ids);
> > -
> > static const struct regmap_config cpcap_regmap_config = {
> > .reg_bits = 16,
> > .reg_stride = 4,
> > @@ -241,62 +229,76 @@ static int cpcap_resume(struct device *dev)
> >
> > static DEFINE_SIMPLE_DEV_PM_OPS(cpcap_pm, cpcap_suspend, cpcap_resume);
> >
> > -static const struct mfd_cell cpcap_mfd_devices[] = {
> > - {
> > - .name = "cpcap_adc",
> > - .of_compatible = "motorola,mapphone-cpcap-adc",
> > - }, {
> > - .name = "cpcap_battery",
> > - .of_compatible = "motorola,cpcap-battery",
> > - }, {
> > - .name = "cpcap-charger",
> > - .of_compatible = "motorola,mapphone-cpcap-charger",
> > - }, {
> > - .name = "cpcap-regulator",
> > - .of_compatible = "motorola,mapphone-cpcap-regulator",
> > - }, {
> > - .name = "cpcap-rtc",
> > - .of_compatible = "motorola,cpcap-rtc",
> > - }, {
> > - .name = "cpcap-pwrbutton",
> > - .of_compatible = "motorola,cpcap-pwrbutton",
> > - }, {
> > - .name = "cpcap-usb-phy",
> > - .of_compatible = "motorola,mapphone-cpcap-usb-phy",
> > - }, {
> > - .name = "cpcap-led",
> > - .id = 0,
> > - .of_compatible = "motorola,cpcap-led-red",
> > - }, {
> > - .name = "cpcap-led",
> > - .id = 1,
> > - .of_compatible = "motorola,cpcap-led-green",
> > - }, {
> > - .name = "cpcap-led",
> > - .id = 2,
> > - .of_compatible = "motorola,cpcap-led-blue",
> > - }, {
> > - .name = "cpcap-led",
> > - .id = 3,
> > - .of_compatible = "motorola,cpcap-led-adl",
> > - }, {
> > - .name = "cpcap-led",
> > - .id = 4,
> > - .of_compatible = "motorola,cpcap-led-cp",
> > - }, {
> > - .name = "cpcap-codec",
> > - }
> > +static const struct mfd_cell cpcap_default_mfd_devices[] = {
> > + MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,cpcap-adc"),
> > + MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0,
> > + "motorola,cpcap-battery"),
> > + MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0,
> > + "motorola,cpcap-regulator"),
> > + MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> > + MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0,
> > + "motorola,cpcap-pwrbutton"),
> > + MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0,
> > + "motorola,cpcap-usb-phy"),
> > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> > + MFD_CELL_NAME("cpcap-codec"),
> > +};
>
> Nit: I wouldn't complain if you wanted to have all of these on a single
> line for neatness.
>
Noted
> > +static const struct mfd_cell cpcap_mapphone_mfd_devices[] = {
> > + MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0,
> > + "motorola,mapphone-cpcap-adc"),
> > + MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0,
> > + "motorola,cpcap-battery"),
> > + MFD_CELL_OF("cpcap-charger", NULL, NULL, 0, 0,
> > + "motorola,mapphone-cpcap-charger"),
> > + MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0,
> > + "motorola,mapphone-cpcap-regulator"),
> > + MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> > + MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0,
> > + "motorola,cpcap-pwrbutton"),
> > + MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0,
> > + "motorola,mapphone-cpcap-usb-phy"),
> > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> > + MFD_CELL_NAME("cpcap-codec"),
> > };
>
> A lot of these are duplicated, right?
>
> I would have a comment set, then the differences in separate containers.
It may be impossible to predict a generic setup since some devices may
require unique compatibles, other may not have LEDs, third may be
partially incompatible with existing cells. In other mfd cases
creating a generic bundle might be good, but in this case I would
suggest better to keep these separate entirely per-device. They will
not take much space, nor add confusion with these macros.
>
> > static int cpcap_probe(struct spi_device *spi)
> > {
> > struct cpcap_ddata *cpcap;
> > + const struct mfd_cell *cells;
> > + unsigned int num_cells;
> > int ret;
> >
> > cpcap = devm_kzalloc(&spi->dev, sizeof(*cpcap), GFP_KERNEL);
> > if (!cpcap)
> > return -ENOMEM;
> >
> > + cpcap->variant = (enum cpcap_variant)spi_get_device_match_data(spi);
> > + if (!cpcap->variant)
> > + return -ENODEV;
>
> Isn't this covered in the 'default' below?
>
This is for case cpcap->variant = 0, it should never happen, but check
will not cause harm
>
> > + switch (cpcap->variant) {
> > + case CPCAP_DEFAULT:
> > + cells = cpcap_default_mfd_devices;
> > + num_cells = ARRAY_SIZE(cpcap_default_mfd_devices);
> > + break;
> > + case CPCAP_MAPPHONE:
> > + cells = cpcap_mapphone_mfd_devices;
> > + num_cells = ARRAY_SIZE(cpcap_mapphone_mfd_devices);
> > + break;
> > + default:
> > + return dev_err_probe(&spi->dev, -EINVAL,
> > + "Unknown device %d\n", cpcap->variant);
>
> This should be -ENODEV.
>
hm, match is ENODEV cause it looks for device, here driver checks
id/variant, so shouldn't it be EINVAL? I assume error message should
be "Unknown device version" or "Unknown device ID"
> > + }
> > +
> > cpcap->spi = spi;
> > spi_set_drvdata(spi, cpcap);
> >
> > @@ -331,10 +333,28 @@ static int cpcap_probe(struct spi_device *spi)
> > spi->dev.coherent_dma_mask = 0;
> > spi->dev.dma_mask = &spi->dev.coherent_dma_mask;
> >
> > - return devm_mfd_add_devices(&spi->dev, 0, cpcap_mfd_devices,
> > - ARRAY_SIZE(cpcap_mfd_devices), NULL, 0, NULL);
> > + return devm_mfd_add_devices(&spi->dev, 0, cells, num_cells, NULL, 0, NULL);
> > }
> >
> > +static const struct of_device_id cpcap_of_match[] = {
> > + {
> > + .compatible = "motorola,cpcap",
> > + .data = (void *)CPCAP_DEFAULT
>
> Single line should be fine.
>
Noted
> > + }, {
> > + .compatible = "motorola,mapphone-cpcap",
> > + .data = (void *)CPCAP_MAPPHONE
> > + },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, cpcap_of_match);
> > +
> > +static const struct spi_device_id cpcap_spi_ids[] = {
> > + { "cpcap", CPCAP_DEFAULT },
> > + { "mapphone-cpcap", CPCAP_MAPPHONE },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(spi, cpcap_spi_ids);
> > +
> > static struct spi_driver cpcap_driver = {
> > .driver = {
> > .name = "cpcap-core",
> > diff --git a/include/linux/mfd/motorola-cpcap.h b/include/linux/mfd/motorola-cpcap.h
> > index 981e5777deb7..1a85b06272c8 100644
> > --- a/include/linux/mfd/motorola-cpcap.h
> > +++ b/include/linux/mfd/motorola-cpcap.h
> > @@ -25,6 +25,12 @@
> > #define CPCAP_REVISION_2_0 0x10
> > #define CPCAP_REVISION_2_1 0x11
> >
> > +enum cpcap_variant {
> > + CPCAP_DEFAULT = 1,
> > + CPCAP_MAPPHONE,
> > + CPCAP_MAX
> > +};
> > +
> > /* CPCAP registers */
> > #define CPCAP_REG_INT1 0x0000 /* Interrupt 1 */
> > #define CPCAP_REG_INT2 0x0004 /* Interrupt 2 */
> > --
> > 2.51.0
> >
>
> --
> Lee Jones
^ permalink raw reply
* Re: [PATCH v5 5/6] mfd: motorola-cpcap: diverge configuration per-board
From: Lee Jones @ 2026-05-20 16:05 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pavel Machek, David Lechner, Tony Lindgren, linux-input,
devicetree, linux-kernel, linux-leds
In-Reply-To: <CAPVz0n0t4PXfmgWYQ1vSTFwfg=+g4oGU+-dwgnBVKxUoUwHGqw@mail.gmail.com>
On Wed, 20 May 2026, Svyatoslav Ryhel wrote:
> ср, 20 трав. 2026 р. о 18:08 Lee Jones <lee@kernel.org> пише:
> >
> > On Sun, 10 May 2026, Svyatoslav Ryhel wrote:
> >
> > > MFD have rigid subdevice structure which does not allow flexible dynamic
> > > subdevice linking. Address this by diverging CPCAP subdevice composition
> > > to take into account board specific configuration.
> > >
> > > Create a common default subdevice composition, rename existing subdevice
> > > composition into cpcap_mapphone_mfd_devices since it targets mainly
> > > Mapphone board.
> > >
> > > Removed st,6556002 as it is no longer applicable to all cases and
> > > duplicates motorola,cpcap, which is used as the default composition.
> > >
> > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > ---
> > > drivers/mfd/motorola-cpcap.c | 142 ++++++++++++++++-------------
> > > include/linux/mfd/motorola-cpcap.h | 6 ++
> > > 2 files changed, 87 insertions(+), 61 deletions(-)
> >
> > Looking much better, thanks.
> >
> > Nit: A patch-level changelog really is much more helpful to reviewers.
> >
>
> Noted, but I will not guarantee that I will do patch-level changelogs, sorry.
That's fine. All I can do is ask.
Note that, helping out reviewers usually ends up helping you too.
[...]
> > > +static const struct mfd_cell cpcap_default_mfd_devices[] = {
> > > + MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,cpcap-adc"),
> > > + MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0,
> > > + "motorola,cpcap-battery"),
> > > + MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0,
> > > + "motorola,cpcap-regulator"),
> > > + MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> > > + MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0,
> > > + "motorola,cpcap-pwrbutton"),
> > > + MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0,
> > > + "motorola,cpcap-usb-phy"),
> > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> > > + MFD_CELL_NAME("cpcap-codec"),
> > > +};
> >
> > Nit: I wouldn't complain if you wanted to have all of these on a single
> > line for neatness.
> >
>
> Noted
>
> > > +static const struct mfd_cell cpcap_mapphone_mfd_devices[] = {
> > > + MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0,
> > > + "motorola,mapphone-cpcap-adc"),
> > > + MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0,
> > > + "motorola,cpcap-battery"),
> > > + MFD_CELL_OF("cpcap-charger", NULL, NULL, 0, 0,
> > > + "motorola,mapphone-cpcap-charger"),
> > > + MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0,
> > > + "motorola,mapphone-cpcap-regulator"),
> > > + MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> > > + MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0,
> > > + "motorola,cpcap-pwrbutton"),
> > > + MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0,
> > > + "motorola,mapphone-cpcap-usb-phy"),
> > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> > > + MFD_CELL_NAME("cpcap-codec"),
> > > };
> >
> > A lot of these are duplicated, right?
> >
> > I would have a comment set, then the differences in separate containers.
>
> It may be impossible to predict a generic setup since some devices may
> require unique compatibles, other may not have LEDs, third may be
> partially incompatible with existing cells. In other mfd cases
> creating a generic bundle might be good, but in this case I would
> suggest better to keep these separate entirely per-device. They will
> not take much space, nor add confusion with these macros.
I'm not sure we're understanding each other. Let me give you an example:
static const struct mfd_cell cpcap_common_devices[] = {
MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0, "motorola,cpcap-battery"),
MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0, "motorola,cpcap-pwrbutton"),
MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
MFD_CELL_NAME("cpcap-codec"),
};
static const struct mfd_cell cpcap_default_devices[] = {
MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,cpcap-adc"),
MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0, "motorola,cpcap-regulator"),
MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0, "motorola,cpcap-usb-phy"),
};
static const struct mfd_cell cpcap_mapphone_devices[] = {
MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-adc"),
MFD_CELL_OF("cpcap-charger", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-charger"),
MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-regulator"),
MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-usb-phy"),
};
This way, it's super easy to read / maintain the common and unique devices.
The only potential drawback would be 2 calls to mfd_add_devices() but
that's common practice.
Also notice that I droped the "_mfd" parts, which you should too.
> > > static int cpcap_probe(struct spi_device *spi)
> > > {
> > > struct cpcap_ddata *cpcap;
> > > + const struct mfd_cell *cells;
> > > + unsigned int num_cells;
> > > int ret;
> > >
> > > cpcap = devm_kzalloc(&spi->dev, sizeof(*cpcap), GFP_KERNEL);
> > > if (!cpcap)
> > > return -ENOMEM;
> > >
> > > + cpcap->variant = (enum cpcap_variant)spi_get_device_match_data(spi);
> > > + if (!cpcap->variant)
> > > + return -ENODEV;
> >
> > Isn't this covered in the 'default' below?
> >
>
> This is for case cpcap->variant = 0, it should never happen, but check
> will not cause harm
The 'default' branch in the switch below will pick that up too. This
check is superfluous.
> > > + switch (cpcap->variant) {
> > > + case CPCAP_DEFAULT:
> > > + cells = cpcap_default_mfd_devices;
> > > + num_cells = ARRAY_SIZE(cpcap_default_mfd_devices);
> > > + break;
> > > + case CPCAP_MAPPHONE:
> > > + cells = cpcap_mapphone_mfd_devices;
> > > + num_cells = ARRAY_SIZE(cpcap_mapphone_mfd_devices);
> > > + break;
> > > + default:
> > > + return dev_err_probe(&spi->dev, -EINVAL,
> > > + "Unknown device %d\n", cpcap->variant);
> >
> > This should be -ENODEV.
> >
>
> hm, match is ENODEV cause it looks for device, here driver checks
> id/variant, so shouldn't it be EINVAL? I assume error message should
> be "Unknown device version" or "Unknown device ID"
All of your supported devices are represented in this switch statement.
Any other request, regardless of the reason should results in
"Unsupported device" and a -ENODEV - same as your check for
!cpcap->variant above.
--
Lee Jones
^ permalink raw reply
* [PATCH v14 0/2] Firmware Support for USB-HID Devices and CP2112
From: Danny Kaehn @ 2026-05-20 16:13 UTC (permalink / raw)
To: Benjamin Tissoires, Andy Shevchenko, Danny Kaehn, Andi Shyti,
Conor Dooley
Cc: Jiri Kosina, linux-input, Dmitry Torokhov, Ethan Twardy,
linux-i2c, linux-kernel, Leo Huang, Arun D Patil, Willie Thai,
Ting-Kai Chen, Bartosz Golaszewski
This patchset allows USB-HID devices to have Firmware bindings through sharing
the USB fwnode with the HID driver, and adds such a binding and driver
implementation for the CP2112 USB to SMBus Bridge (which necessitated the
USB-HID change). This change allows a CP2112 permanently attached in hardware to
be described in DT and ACPI and interoperate with other drivers.
Changes in v14:
- Drop DT binding patch (merged upstream) (remove DT mailing lists and
maintainers from email chain)
- Refactor logic in cp2112_probe() to reduce extra duplicate
dev_fwnode() calls by retrieving and storing the CP2112's fwnode once
- Update comment on cp2112_child_acpi_cell_addrs enum to explain why
values are explicitly defined
- Edit commit message for 1/2 per Andy's comments (lowercase letters in
description; _ADR "equal to" Zero, etc..
- Edit commit message for 2/2 to use lowercase letters
- Link to v13: https://lore.kernel.org/r/20260127-cp2112-dt-v13-0-6448ddd4bf22@plexus.com
Changes in v13:
- dt-binding changes:
- drop scl-gpios and sda-gpios from binding, since they are included
from the i2c-controller schena.
- Set indentation to 4 spaces consistently for the DTS example
- Fix alignment for gpio-line-names in the example
- Use dev_fwnode in hid-cp2112.c instead of directly accessing fwnode
- Immediately release the fwnode_handle from
device_get_named_child_node() in hid-cp2112.c
- Link to v12: https://lore.kernel.org/r/20251126-cp2112-dt-v12-0-2cdba6481db3@plexus.com
Changes in v12:
- dt-binding changes:
- Drop "on the host controller" from top-level description based on
comment from Rob H.
- Correct "Properties must precede subnodes" dt_binding_check error by
moving gpio_chip-related properties above the i2c subnode in the
binding and in the example.
- Include `interrupt-controller` property in the example
- Modify hid-cp2112.c to support separate schemas for DT vs. ACPI - DT
combines gpio subnode with the CP2112's node, but will have an I2C
subnode; while ACPI will maintain separate child nodes for the GPIO
I2C devices
Changes in v11:
- Eliminate 'gpio' subnode for DT and ACPI for the CP2112 per comment
from Rob H.
- Edit hid-cp2112.c to match for ACPI index and fall back to matching by
name (instead of the other way around)
- Separate CP2112 I2C bus speed configuration into a separate patch
Changes in v10:
- Define an enumeration and mapping for CP2112 ACPI _ADRs and devicetree
child node names, and use these in the scanning of child nodes
- Address other miscellaneous
Changes in v9:
- Add _ADR-based ACPI binding of child nodes (I2C is _ADR Zero, GPIO is _ADR One)
- Use a loop-based approach for assigning child nodes within probe().
As a consequence, hid-cp2112.c no longer maintains references to the
child fwnodes during the lifetime of the device. (plese correct if this
is actually needed for this use-case)
Changes in v8:
- Apply Review tags retroactively to patches previously reviewed
Changes in v7:
- Use dev_fwnode when calling fwnod_handle_put in i2c_adapter in hid-cp2112.c
- Capitalize I2C and GPIO in commit message for patch 0003
Changes in v6:
- Fix fwnode_handle reference leaks in hid-cp21112.c
- Simplify hog node pattern in silabs,cp2112.yaml
Changes in v5:
- Use fwnode API instead of of_node api in hid-core.c and hid-cp2112.c
- Include sda-gpios and scl-gpios in silabs,cp2112.yaml
- Additional fixups to silabs,cp2112.yaml to address comments
- Remove ngpios property
- Constrain the hog pattern to a single naming scheme
- Remove unneeded properties from the gpio hog which are provided by
the parent schema
- Submit threaded interrupt bugfix separately from this patchset, as requested
Changes in v4:
- Moved silabs,cp2112.yaml to /Documentation/devicetree/bindings/i2c
Changes in v3:
- Additional fixups to silabs,cp2112.yaml to address comments
Changes in v2:
- Added more detail to silabs,cp2112.yaml dt-binding
- Moved silabs,cp2112.yaml to /Documentation/devicetree/bindings/input
- Added support for setting smbus clock-frequency from DT in hid-cp2112.c
- Added freeing of of_nodes on error paths of _probe in hid-cp2112.c
Danny Kaehn (3):
dt-bindings: i2c: Add CP2112 HID USB to SMBus Bridge
HID: usbhid: Share USB device firmware node with child HID device
HID: cp2112: Fwnode Support
.../bindings/i2c/silabs,cp2112.yaml | 113 ++++++++++++++++++
drivers/hid/hid-cp2112.c | 50 ++++++++
drivers/hid/usbhid/hid-core.c | 2 +
3 files changed, 165 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/silabs,cp2112.yaml
--
2.25.1
---
Danny Kaehn (2):
HID: cp2112: Add fwnode support
HID: cp2112: Configure I2C bus speed from firmware
drivers/hid/hid-cp2112.c | 41 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 41 insertions(+)
---
base-commit: 70eda68668d1476b459b64e69b8f36659fa9dfa8
change-id: 20240605-cp2112-dt-7cdc95448e8a
Best regards,
--
Danny Kaehn <danny.kaehn@plexus.com>
^ permalink raw reply
* [PATCH v14 1/2] HID: cp2112: Add fwnode support
From: Danny Kaehn @ 2026-05-20 16:13 UTC (permalink / raw)
To: Benjamin Tissoires, Andy Shevchenko, Danny Kaehn, Andi Shyti,
Conor Dooley
Cc: Jiri Kosina, linux-input, Dmitry Torokhov, Ethan Twardy,
linux-i2c, linux-kernel, Leo Huang, Arun D Patil, Willie Thai,
Ting-Kai Chen, Bartosz Golaszewski
In-Reply-To: <20260520-cp2112-dt-v14-0-b1b4b6734b6f@plexus.com>
Support describing the CP2112's I2C and GPIO interfaces in firmware.
Bindings between the firmware nodes and the functions of the device
are distinct between ACPI and DeviceTree.
For ACPI, the i2c_adapter will use the child with _ADR equal to Zero
and the gpio_chip will use the child with _ADR equal to One.
For DeviceTree, the i2c_adapter will use the child with name "i2c",
but the gpio_chip will share a firmware node with the CP2112.
Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
---
drivers/hid/hid-cp2112.c | 37 +++++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index 803b883ae875..e960fc988058 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -29,6 +29,18 @@
#include <linux/usb/ch9.h>
#include "hid-ids.h"
+/**
+ * enum cp2112_child_acpi_cell_addrs - Child ACPI addresses for CP2112 sub-functions
+ * Note that the enum values are explicitly defined, as this defines the interface
+ * between ACPI and Linux
+ * @CP2112_I2C_ADR: Address for I2C node
+ * @CP2112_GPIO_ADR: Address for GPIO node
+ */
+enum cp2112_child_acpi_cell_addrs {
+ CP2112_I2C_ADR = 0,
+ CP2112_GPIO_ADR = 1,
+};
+
#define CP2112_REPORT_MAX_LENGTH 64
#define CP2112_GPIO_CONFIG_LENGTH 5
#define CP2112_GPIO_GET_LENGTH 2
@@ -1208,7 +1220,10 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
struct cp2112_device *dev;
u8 buf[3];
struct cp2112_smbus_config_report config;
+ struct fwnode_handle *cp2112_fwnode;
+ struct fwnode_handle *child;
struct gpio_irq_chip *girq;
+ u32 addr;
int ret;
dev = devm_kzalloc(&hdev->dev, sizeof(*dev), GFP_KERNEL);
@@ -1226,6 +1241,28 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
+ cp2112_fwnode = dev_fwnode(&hdev->dev);
+ if (is_acpi_device_node(cp2112_fwnode)) {
+ fwnode_for_each_child_node(cp2112_fwnode, child) {
+ ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
+ if (ret)
+ continue;
+
+ switch (addr) {
+ case CP2112_I2C_ADR:
+ device_set_node(&dev->adap.dev, child);
+ break;
+ case CP2112_GPIO_ADR:
+ dev->gc.fwnode = child;
+ break;
+ }
+ }
+ } else if (is_of_node(cp2112_fwnode)) {
+ child = fwnode_get_named_child_node(cp2112_fwnode, "i2c");
+ device_set_node(&dev->adap.dev, child);
+ fwnode_handle_put(child);
+ }
+
ret = hid_parse(hdev);
if (ret) {
hid_err(hdev, "parse failed\n");
--
2.25.1
^ permalink raw reply related
* [PATCH v14 2/2] HID: cp2112: Configure I2C bus speed from firmware
From: Danny Kaehn @ 2026-05-20 16:13 UTC (permalink / raw)
To: Benjamin Tissoires, Andy Shevchenko, Danny Kaehn, Andi Shyti,
Conor Dooley
Cc: Jiri Kosina, linux-input, Dmitry Torokhov, Ethan Twardy,
linux-i2c, linux-kernel, Leo Huang, Arun D Patil, Willie Thai,
Ting-Kai Chen, Bartosz Golaszewski
In-Reply-To: <20260520-cp2112-dt-v14-0-b1b4b6734b6f@plexus.com>
Now that the I2C adapter on the CP2112 can have an associated firmware
node, set the bus speed based on firmware configuration
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Danny Kaehn <danny.kaehn@plexus.com>
---
drivers/hid/hid-cp2112.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
index e960fc988058..04379db93571 100644
--- a/drivers/hid/hid-cp2112.c
+++ b/drivers/hid/hid-cp2112.c
@@ -1223,6 +1223,7 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
struct fwnode_handle *cp2112_fwnode;
struct fwnode_handle *child;
struct gpio_irq_chip *girq;
+ struct i2c_timings timings;
u32 addr;
int ret;
@@ -1308,6 +1309,9 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
goto err_power_normal;
}
+ i2c_parse_fw_timings(&dev->adap.dev, &timings, true);
+
+ config.clock_speed = cpu_to_be32(timings.bus_freq_hz);
config.retry_time = cpu_to_be16(1);
ret = cp2112_hid_output(hdev, (u8 *)&config, sizeof(config),
--
2.25.1
^ permalink raw reply related
* Re: [PATCH v5 5/6] mfd: motorola-cpcap: diverge configuration per-board
From: Svyatoslav Ryhel @ 2026-05-20 16:30 UTC (permalink / raw)
To: Lee Jones
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pavel Machek, David Lechner, Tony Lindgren, linux-input,
devicetree, linux-kernel, linux-leds
In-Reply-To: <20260520160548.GK2767592@google.com>
ср, 20 трав. 2026 р. о 19:05 Lee Jones <lee@kernel.org> пише:
>
> On Wed, 20 May 2026, Svyatoslav Ryhel wrote:
>
> > ср, 20 трав. 2026 р. о 18:08 Lee Jones <lee@kernel.org> пише:
> > >
> > > On Sun, 10 May 2026, Svyatoslav Ryhel wrote:
> > >
> > > > MFD have rigid subdevice structure which does not allow flexible dynamic
> > > > subdevice linking. Address this by diverging CPCAP subdevice composition
> > > > to take into account board specific configuration.
> > > >
> > > > Create a common default subdevice composition, rename existing subdevice
> > > > composition into cpcap_mapphone_mfd_devices since it targets mainly
> > > > Mapphone board.
> > > >
> > > > Removed st,6556002 as it is no longer applicable to all cases and
> > > > duplicates motorola,cpcap, which is used as the default composition.
> > > >
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > ---
> > > > drivers/mfd/motorola-cpcap.c | 142 ++++++++++++++++-------------
> > > > include/linux/mfd/motorola-cpcap.h | 6 ++
> > > > 2 files changed, 87 insertions(+), 61 deletions(-)
> > >
> > > Looking much better, thanks.
> > >
> > > Nit: A patch-level changelog really is much more helpful to reviewers.
> > >
> >
> > Noted, but I will not guarantee that I will do patch-level changelogs, sorry.
>
> That's fine. All I can do is ask.
>
> Note that, helping out reviewers usually ends up helping you too.
>
> [...]
>
> > > > +static const struct mfd_cell cpcap_default_mfd_devices[] = {
> > > > + MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,cpcap-adc"),
> > > > + MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0,
> > > > + "motorola,cpcap-battery"),
> > > > + MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0,
> > > > + "motorola,cpcap-regulator"),
> > > > + MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> > > > + MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0,
> > > > + "motorola,cpcap-pwrbutton"),
> > > > + MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0,
> > > > + "motorola,cpcap-usb-phy"),
> > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> > > > + MFD_CELL_NAME("cpcap-codec"),
> > > > +};
> > >
> > > Nit: I wouldn't complain if you wanted to have all of these on a single
> > > line for neatness.
> > >
> >
> > Noted
> >
> > > > +static const struct mfd_cell cpcap_mapphone_mfd_devices[] = {
> > > > + MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0,
> > > > + "motorola,mapphone-cpcap-adc"),
> > > > + MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0,
> > > > + "motorola,cpcap-battery"),
> > > > + MFD_CELL_OF("cpcap-charger", NULL, NULL, 0, 0,
> > > > + "motorola,mapphone-cpcap-charger"),
> > > > + MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0,
> > > > + "motorola,mapphone-cpcap-regulator"),
> > > > + MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> > > > + MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0,
> > > > + "motorola,cpcap-pwrbutton"),
> > > > + MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0,
> > > > + "motorola,mapphone-cpcap-usb-phy"),
> > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> > > > + MFD_CELL_NAME("cpcap-codec"),
> > > > };
> > >
> > > A lot of these are duplicated, right?
> > >
> > > I would have a comment set, then the differences in separate containers.
> >
> > It may be impossible to predict a generic setup since some devices may
> > require unique compatibles, other may not have LEDs, third may be
> > partially incompatible with existing cells. In other mfd cases
> > creating a generic bundle might be good, but in this case I would
> > suggest better to keep these separate entirely per-device. They will
> > not take much space, nor add confusion with these macros.
>
> I'm not sure we're understanding each other. Let me give you an example:
>
> static const struct mfd_cell cpcap_common_devices[] = {
> MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0, "motorola,cpcap-battery"),
> MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0, "motorola,cpcap-pwrbutton"),
> MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> MFD_CELL_NAME("cpcap-codec"),
> };
>
> static const struct mfd_cell cpcap_default_devices[] = {
> MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,cpcap-adc"),
> MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0, "motorola,cpcap-regulator"),
> MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0, "motorola,cpcap-usb-phy"),
> };
>
> static const struct mfd_cell cpcap_mapphone_devices[] = {
> MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-adc"),
> MFD_CELL_OF("cpcap-charger", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-charger"),
> MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-regulator"),
> MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-usb-phy"),
> };
>
> This way, it's super easy to read / maintain the common and unique devices.
>
> The only potential drawback would be 2 calls to mfd_add_devices() but
> that's common practice.
>
> Also notice that I droped the "_mfd" parts, which you should too.
>
What if next added device does not have cpcap-battery, cpcap-led-adl,
cpcap-pwrbutton, uses external codec and would require cpcap-watchdog
which is not yet present. This will cause issues. Even Mot introduces
in the last patch of this patchset has slightly different composition.
> > > > static int cpcap_probe(struct spi_device *spi)
> > > > {
> > > > struct cpcap_ddata *cpcap;
> > > > + const struct mfd_cell *cells;
> > > > + unsigned int num_cells;
> > > > int ret;
> > > >
> > > > cpcap = devm_kzalloc(&spi->dev, sizeof(*cpcap), GFP_KERNEL);
> > > > if (!cpcap)
> > > > return -ENOMEM;
> > > >
> > > > + cpcap->variant = (enum cpcap_variant)spi_get_device_match_data(spi);
> > > > + if (!cpcap->variant)
> > > > + return -ENODEV;
> > >
> > > Isn't this covered in the 'default' below?
> > >
> >
> > This is for case cpcap->variant = 0, it should never happen, but check
> > will not cause harm
>
> The 'default' branch in the switch below will pick that up too. This
> check is superfluous.
>
Noted
> > > > + switch (cpcap->variant) {
> > > > + case CPCAP_DEFAULT:
> > > > + cells = cpcap_default_mfd_devices;
> > > > + num_cells = ARRAY_SIZE(cpcap_default_mfd_devices);
> > > > + break;
> > > > + case CPCAP_MAPPHONE:
> > > > + cells = cpcap_mapphone_mfd_devices;
> > > > + num_cells = ARRAY_SIZE(cpcap_mapphone_mfd_devices);
> > > > + break;
> > > > + default:
> > > > + return dev_err_probe(&spi->dev, -EINVAL,
> > > > + "Unknown device %d\n", cpcap->variant);
> > >
> > > This should be -ENODEV.
> > >
> >
> > hm, match is ENODEV cause it looks for device, here driver checks
> > id/variant, so shouldn't it be EINVAL? I assume error message should
> > be "Unknown device version" or "Unknown device ID"
>
> All of your supported devices are represented in this switch statement.
> Any other request, regardless of the reason should results in
> "Unsupported device" and a -ENODEV - same as your check for
> !cpcap->variant above.
>
Noted
> --
> Lee Jones
^ permalink raw reply
* Re: [PATCH v14 1/2] HID: cp2112: Add fwnode support
From: sashiko-bot @ 2026-05-20 17:02 UTC (permalink / raw)
To: Danny Kaehn; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260520-cp2112-dt-v14-1-b1b4b6734b6f@plexus.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Premature release of firmware node references results in use-after-free vulnerabilities when assigning the I2C adapter and GPIO chip firmware nodes.
--
commit 3dfaf368e855835bf23009205c4b93a2a070aae1
Author: Danny Kaehn <danny.kaehn@plexus.com>
HID: cp2112: Add fwnode support
This commit adds firmware node support to describe the CP2112's I2C and
GPIO interfaces. It differentiates between ACPI and DeviceTree bindings,
mapping the child nodes to the respective adapter and gpio_chip devices.
> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> index 803b883ae8750..e960fc988058f 100644
> --- a/drivers/hid/hid-cp2112.c
> +++ b/drivers/hid/hid-cp2112.c
[ ... ]
> @@ -1226,6 +1241,28 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> return ret;
> }
>
> + cp2112_fwnode = dev_fwnode(&hdev->dev);
> + if (is_acpi_device_node(cp2112_fwnode)) {
> + fwnode_for_each_child_node(cp2112_fwnode, child) {
> + ret = acpi_get_local_address(ACPI_HANDLE_FWNODE(child), &addr);
> + if (ret)
> + continue;
> +
> + switch (addr) {
> + case CP2112_I2C_ADR:
> + device_set_node(&dev->adap.dev, child);
> + break;
> + case CP2112_GPIO_ADR:
> + dev->gc.fwnode = child;
> + break;
> + }
[Severity: High]
Does this code leave dangling pointers in the device structures?
The fwnode_for_each_child_node() macro automatically drops the reference to
the child node when advancing to the next node or exiting the loop.
Because device_set_node() and direct pointer assignments do not increment
the reference count, the nodes assigned here could be prematurely freed
resulting in a use-after-free.
> + }
> + } else if (is_of_node(cp2112_fwnode)) {
> + child = fwnode_get_named_child_node(cp2112_fwnode, "i2c");
> + device_set_node(&dev->adap.dev, child);
> + fwnode_handle_put(child);
[Severity: High]
Are we releasing the firmware node prematurely here?
fwnode_get_named_child_node() returns a node with an incremented reference
count. However, since device_set_node() does not take its own reference on
the child node, dropping the reference immediately after could leave
dev->adap.dev with a dangling pointer if the node is later removed.
> + }
> +
> ret = hid_parse(hdev);
> if (ret) {
> hid_err(hdev, "parse failed\n");
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520-cp2112-dt-v14-0-b1b4b6734b6f@plexus.com?part=1
^ permalink raw reply
* Re: [PATCH v5 5/6] mfd: motorola-cpcap: diverge configuration per-board
From: Lee Jones @ 2026-05-20 17:02 UTC (permalink / raw)
To: Svyatoslav Ryhel
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pavel Machek, David Lechner, Tony Lindgren, linux-input,
devicetree, linux-kernel, linux-leds
In-Reply-To: <CAPVz0n2s32uYP76R9VNtWYnHrDQJKK+qC1uRRdkaX1ekF15Pww@mail.gmail.com>
On Wed, 20 May 2026, Svyatoslav Ryhel wrote:
> ср, 20 трав. 2026 р. о 19:05 Lee Jones <lee@kernel.org> пише:
> >
> > On Wed, 20 May 2026, Svyatoslav Ryhel wrote:
> >
> > > ср, 20 трав. 2026 р. о 18:08 Lee Jones <lee@kernel.org> пише:
> > > >
> > > > On Sun, 10 May 2026, Svyatoslav Ryhel wrote:
> > > >
> > > > > MFD have rigid subdevice structure which does not allow flexible dynamic
> > > > > subdevice linking. Address this by diverging CPCAP subdevice composition
> > > > > to take into account board specific configuration.
> > > > >
> > > > > Create a common default subdevice composition, rename existing subdevice
> > > > > composition into cpcap_mapphone_mfd_devices since it targets mainly
> > > > > Mapphone board.
> > > > >
> > > > > Removed st,6556002 as it is no longer applicable to all cases and
> > > > > duplicates motorola,cpcap, which is used as the default composition.
> > > > >
> > > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > > ---
> > > > > drivers/mfd/motorola-cpcap.c | 142 ++++++++++++++++-------------
> > > > > include/linux/mfd/motorola-cpcap.h | 6 ++
> > > > > 2 files changed, 87 insertions(+), 61 deletions(-)
> > > >
> > > > Looking much better, thanks.
> > > >
> > > > Nit: A patch-level changelog really is much more helpful to reviewers.
> > > >
> > >
> > > Noted, but I will not guarantee that I will do patch-level changelogs, sorry.
> >
> > That's fine. All I can do is ask.
> >
> > Note that, helping out reviewers usually ends up helping you too.
> >
> > [...]
> >
> > > > > +static const struct mfd_cell cpcap_default_mfd_devices[] = {
> > > > > + MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,cpcap-adc"),
> > > > > + MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0,
> > > > > + "motorola,cpcap-battery"),
> > > > > + MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0,
> > > > > + "motorola,cpcap-regulator"),
> > > > > + MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> > > > > + MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0,
> > > > > + "motorola,cpcap-pwrbutton"),
> > > > > + MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0,
> > > > > + "motorola,cpcap-usb-phy"),
> > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> > > > > + MFD_CELL_NAME("cpcap-codec"),
> > > > > +};
> > > >
> > > > Nit: I wouldn't complain if you wanted to have all of these on a single
> > > > line for neatness.
> > > >
> > >
> > > Noted
> > >
> > > > > +static const struct mfd_cell cpcap_mapphone_mfd_devices[] = {
> > > > > + MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0,
> > > > > + "motorola,mapphone-cpcap-adc"),
> > > > > + MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0,
> > > > > + "motorola,cpcap-battery"),
> > > > > + MFD_CELL_OF("cpcap-charger", NULL, NULL, 0, 0,
> > > > > + "motorola,mapphone-cpcap-charger"),
> > > > > + MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0,
> > > > > + "motorola,mapphone-cpcap-regulator"),
> > > > > + MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> > > > > + MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0,
> > > > > + "motorola,cpcap-pwrbutton"),
> > > > > + MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0,
> > > > > + "motorola,mapphone-cpcap-usb-phy"),
> > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> > > > > + MFD_CELL_NAME("cpcap-codec"),
> > > > > };
> > > >
> > > > A lot of these are duplicated, right?
> > > >
> > > > I would have a comment set, then the differences in separate containers.
> > >
> > > It may be impossible to predict a generic setup since some devices may
> > > require unique compatibles, other may not have LEDs, third may be
> > > partially incompatible with existing cells. In other mfd cases
> > > creating a generic bundle might be good, but in this case I would
> > > suggest better to keep these separate entirely per-device. They will
> > > not take much space, nor add confusion with these macros.
> >
> > I'm not sure we're understanding each other. Let me give you an example:
> >
> > static const struct mfd_cell cpcap_common_devices[] = {
> > MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0, "motorola,cpcap-battery"),
> > MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> > MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0, "motorola,cpcap-pwrbutton"),
> > MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> > MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> > MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> > MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> > MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> > MFD_CELL_NAME("cpcap-codec"),
> > };
> >
> > static const struct mfd_cell cpcap_default_devices[] = {
> > MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,cpcap-adc"),
> > MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0, "motorola,cpcap-regulator"),
> > MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0, "motorola,cpcap-usb-phy"),
> > };
> >
> > static const struct mfd_cell cpcap_mapphone_devices[] = {
> > MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-adc"),
> > MFD_CELL_OF("cpcap-charger", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-charger"),
> > MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-regulator"),
> > MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-usb-phy"),
> > };
> >
> > This way, it's super easy to read / maintain the common and unique devices.
> >
> > The only potential drawback would be 2 calls to mfd_add_devices() but
> > that's common practice.
> >
> > Also notice that I droped the "_mfd" parts, which you should too.
> >
>
> What if next added device does not have cpcap-battery, cpcap-led-adl,
> cpcap-pwrbutton, uses external codec and would require cpcap-watchdog
> which is not yet present. This will cause issues. Even Mot introduces
> in the last patch of this patchset has slightly different composition.
We usually just code-up what we have _today_.
If new devices come in with different requirements, it takes 5 mins to rejig.
--
Lee Jones
^ permalink raw reply
* [syzbot] [input?] possible deadlock in tasklet_action_common (2)
From: syzbot @ 2026-05-20 17:05 UTC (permalink / raw)
To: dmitry.torokhov, linux-input, linux-kernel, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: e98d21c170b0 Add linux-next specific files for 20260508
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=1354bfce580000
kernel config: https://syzkaller.appspot.com/x/.config?x=59b98218d9b2edf4
dashboard link: https://syzkaller.appspot.com/bug?extid=b5d7ab56d43de3fd5aac
compiler: Debian clang version 21.1.8 (++20251221033036+2078da43e25a-1~exp1~20251221153213.50), Debian LLD 21.1.8
Unfortunately, I don't have any reproducer for this issue yet.
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/66f2a00ee290/disk-e98d21c1.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/6b982257ce9e/vmlinux-e98d21c1.xz
kernel image: https://storage.googleapis.com/syzbot-assets/a73fbea43e1a/bzImage-e98d21c1.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+b5d7ab56d43de3fd5aac@syzkaller.appspotmail.com
======================================================
WARNING: possible circular locking dependency detected
syzkaller #0 Tainted: G L
------------------------------------------------------
syz.2.4328/32663 is trying to acquire lock:
ffff8880b8724168 (tasklet_sync_callback.cb_lock){+...}-{3:3}, at: spin_lock include/linux/spinlock_rt.h:45 [inline]
ffff8880b8724168 (tasklet_sync_callback.cb_lock){+...}-{3:3}, at: tasklet_lock_callback kernel/softirq.c:881 [inline]
ffff8880b8724168 (tasklet_sync_callback.cb_lock){+...}-{3:3}, at: tasklet_action_common+0xc5/0x610 kernel/softirq.c:931
but task is already holding lock:
ffff888058598270 (&dev->event_lock#2){+.+.}-{3:3}, at: spin_lock include/linux/spinlock_rt.h:45 [inline]
ffff888058598270 (&dev->event_lock#2){+.+.}-{3:3}, at: class_spinlock_irqsave_constructor include/linux/spinlock.h:619 [inline]
ffff888058598270 (&dev->event_lock#2){+.+.}-{3:3}, at: input_inject_event+0xa4/0x320 drivers/input/input.c:419
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&dev->event_lock#2){+.+.}-{3:3}:
rt_spin_lock+0x83/0x400 kernel/locking/spinlock_rt.c:56
spin_lock include/linux/spinlock_rt.h:45 [inline]
class_spinlock_irqsave_constructor include/linux/spinlock.h:619 [inline]
input_inject_event+0xa4/0x320 drivers/input/input.c:419
led_trigger_event+0x13b/0x220 drivers/leds/led-triggers.c:420
kbd_propagate_led_state drivers/tty/vt/keyboard.c:1118 [inline]
kbd_bh+0x1b4/0x2c0 drivers/tty/vt/keyboard.c:1297
tasklet_action_common+0x31c/0x610 kernel/softirq.c:942
handle_softirqs+0x1de/0x6d0 kernel/softirq.c:626
__do_softirq kernel/softirq.c:660 [inline]
run_ktimerd+0x69/0x100 kernel/softirq.c:1155
smpboot_thread_fn+0x541/0xa50 kernel/smpboot.c:160
kthread+0x388/0x470 kernel/kthread.c:436
ret_from_fork+0x514/0xb70 arch/x86/kernel/process.c:158
ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:245
-> #0 (tasklet_sync_callback.cb_lock){+...}-{3:3}:
check_prev_add kernel/locking/lockdep.c:3167 [inline]
check_prevs_add kernel/locking/lockdep.c:3286 [inline]
validate_chain kernel/locking/lockdep.c:3910 [inline]
__lock_acquire+0x15a5/0x2d10 kernel/locking/lockdep.c:5239
lock_acquire+0x106/0x350 kernel/locking/lockdep.c:5870
rt_spin_lock+0x83/0x400 kernel/locking/spinlock_rt.c:56
spin_lock include/linux/spinlock_rt.h:45 [inline]
tasklet_lock_callback kernel/softirq.c:881 [inline]
tasklet_action_common+0xc5/0x610 kernel/softirq.c:931
handle_softirqs+0x1de/0x6d0 kernel/softirq.c:626
__do_softirq kernel/softirq.c:660 [inline]
__local_bh_enable_ip+0x170/0x2b0 kernel/softirq.c:302
local_bh_enable include/linux/bottom_half.h:33 [inline]
__alloc_skb+0x1aa/0x7d0 net/core/skbuff.c:697
alloc_skb include/linux/skbuff.h:1381 [inline]
hidp_send_message+0xb5/0x230 net/bluetooth/hidp/core.c:111
hidp_send_intr_message net/bluetooth/hidp/core.c:143 [inline]
hidp_input_event+0x2a4/0x380 net/bluetooth/hidp/core.c:175
input_event_dispose+0x80/0x6b0 drivers/input/input.c:322
input_inject_event+0x1d7/0x320 drivers/input/input.c:424
kbd_led_trigger_activate+0xbc/0x100 drivers/tty/vt/keyboard.c:1074
led_trigger_set+0x53b/0x960 drivers/leds/led-triggers.c:220
led_match_default_trigger drivers/leds/led-triggers.c:277 [inline]
led_trigger_set_default+0x266/0x2a0 drivers/leds/led-triggers.c:300
led_classdev_register_ext+0x787/0x9c0 drivers/leds/led-class.c:581
led_classdev_register include/linux/leds.h:274 [inline]
input_leds_connect+0x517/0x790 drivers/input/input-leds.c:145
input_attach_handler drivers/input/input.c:1011 [inline]
input_register_device+0xce0/0x1140 drivers/input/input.c:2395
hidp_session_dev_add net/bluetooth/hidp/core.c:861 [inline]
hidp_session_probe+0x1a5/0x8a0 net/bluetooth/hidp/core.c:1139
l2cap_register_user+0xc2/0x1d0 net/bluetooth/l2cap_core.c:1725
hidp_connection_add+0x158b/0x1a20 net/bluetooth/hidp/core.c:1411
do_hidp_sock_ioctl net/bluetooth/hidp/sock.c:81 [inline]
hidp_sock_ioctl+0x403/0x650 net/bluetooth/hidp/sock.c:128
sock_do_ioctl+0x101/0x320 net/socket.c:1328
sock_ioctl+0x5c9/0x7f0 net/socket.c:1449
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:597 [inline]
__se_sys_ioctl+0xff/0x170 fs/ioctl.c:583
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x15f/0xf80 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&dev->event_lock#2);
lock(tasklet_sync_callback.cb_lock);
lock(&dev->event_lock#2);
lock(tasklet_sync_callback.cb_lock);
*** DEADLOCK ***
10 locks held by syz.2.4328/32663:
#0: ffff88805ebb3370 (&conn->lock#2){+.+.}-{4:4}, at: l2cap_register_user+0x35/0x1d0 net/bluetooth/l2cap_core.c:1712
#1: ffffffff8f73d960 (hidp_session_sem){++++}-{4:4}, at: hidp_session_probe+0x98/0x8a0 net/bluetooth/hidp/core.c:1129
#2: ffffffff8f006e58 (input_mutex){+.+.}-{4:4}, at: class_mutex_intr_constructor include/linux/mutex.h:255 [inline]
#2: ffffffff8f006e58 (input_mutex){+.+.}-{4:4}, at: input_register_device+0xa56/0x1140 drivers/input/input.c:2391
#3: ffff8880125ea860 (&led_cdev->led_access){+.+.}-{4:4}, at: led_classdev_register_ext+0x484/0x9c0 drivers/leds/led-class.c:539
#4: ffffffff8e9c4400 (triggers_list_lock){++++}-{4:4}, at: led_trigger_set_default+0x77/0x2a0 drivers/leds/led-triggers.c:297
#5: ffff8880125ea788 (&led_cdev->trigger_lock){+.+.}-{4:4}, at: led_trigger_set_default+0x87/0x2a0 drivers/leds/led-triggers.c:298
#6: ffff888058598270 (&dev->event_lock#2){+.+.}-{3:3}, at: spin_lock include/linux/spinlock_rt.h:45 [inline]
#6: ffff888058598270 (&dev->event_lock#2){+.+.}-{3:3}, at: class_spinlock_irqsave_constructor include/linux/spinlock.h:619 [inline]
#6: ffff888058598270 (&dev->event_lock#2){+.+.}-{3:3}, at: input_inject_event+0xa4/0x320 drivers/input/input.c:419
#7: ffffffff8e1c8240 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire include/linux/rcupdate.h:300 [inline]
#7: ffffffff8e1c8240 (rcu_read_lock){....}-{1:3}, at: rcu_read_lock include/linux/rcupdate.h:838 [inline]
#7: ffffffff8e1c8240 (rcu_read_lock){....}-{1:3}, at: __rt_spin_lock kernel/locking/spinlock_rt.c:50 [inline]
#7: ffffffff8e1c8240 (rcu_read_lock){....}-{1:3}, at: rt_spin_lock+0x1e0/0x400 kernel/locking/spinlock_rt.c:57
#8: ffffffff8e1c8240 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire include/linux/rcupdate.h:300 [inline]
#8: ffffffff8e1c8240 (rcu_read_lock){....}-{1:3}, at: rcu_read_lock include/linux/rcupdate.h:838 [inline]
#8: ffffffff8e1c8240 (rcu_read_lock){....}-{1:3}, at: class_rcu_constructor include/linux/rcupdate.h:1181 [inline]
#8: ffffffff8e1c8240 (rcu_read_lock){....}-{1:3}, at: input_inject_event+0xb0/0x320 drivers/input/input.c:420
#9: ffffffff8e1c8240 (rcu_read_lock){....}-{1:3}, at: __local_bh_disable_ip+0x3c/0x420 kernel/softirq.c:163
stack backtrace:
CPU: 1 UID: 0 PID: 32663 Comm: syz.2.4328 Tainted: G L syzkaller #0 PREEMPT_{RT,(full)}
Tainted: [L]=SOFTLOCKUP
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/18/2026
Call Trace:
<TASK>
dump_stack_lvl+0xe8/0x150 lib/dump_stack.c:120
print_circular_bug+0x2e1/0x300 kernel/locking/lockdep.c:2045
check_noncircular+0x12e/0x150 kernel/locking/lockdep.c:2177
check_prev_add kernel/locking/lockdep.c:3167 [inline]
check_prevs_add kernel/locking/lockdep.c:3286 [inline]
validate_chain kernel/locking/lockdep.c:3910 [inline]
__lock_acquire+0x15a5/0x2d10 kernel/locking/lockdep.c:5239
lock_acquire+0x106/0x350 kernel/locking/lockdep.c:5870
rt_spin_lock+0x83/0x400 kernel/locking/spinlock_rt.c:56
spin_lock include/linux/spinlock_rt.h:45 [inline]
tasklet_lock_callback kernel/softirq.c:881 [inline]
tasklet_action_common+0xc5/0x610 kernel/softirq.c:931
handle_softirqs+0x1de/0x6d0 kernel/softirq.c:626
__do_softirq kernel/softirq.c:660 [inline]
__local_bh_enable_ip+0x170/0x2b0 kernel/softirq.c:302
local_bh_enable include/linux/bottom_half.h:33 [inline]
__alloc_skb+0x1aa/0x7d0 net/core/skbuff.c:697
alloc_skb include/linux/skbuff.h:1381 [inline]
hidp_send_message+0xb5/0x230 net/bluetooth/hidp/core.c:111
hidp_send_intr_message net/bluetooth/hidp/core.c:143 [inline]
hidp_input_event+0x2a4/0x380 net/bluetooth/hidp/core.c:175
input_event_dispose+0x80/0x6b0 drivers/input/input.c:322
input_inject_event+0x1d7/0x320 drivers/input/input.c:424
kbd_led_trigger_activate+0xbc/0x100 drivers/tty/vt/keyboard.c:1074
led_trigger_set+0x53b/0x960 drivers/leds/led-triggers.c:220
led_match_default_trigger drivers/leds/led-triggers.c:277 [inline]
led_trigger_set_default+0x266/0x2a0 drivers/leds/led-triggers.c:300
led_classdev_register_ext+0x787/0x9c0 drivers/leds/led-class.c:581
led_classdev_register include/linux/leds.h:274 [inline]
input_leds_connect+0x517/0x790 drivers/input/input-leds.c:145
input_attach_handler drivers/input/input.c:1011 [inline]
input_register_device+0xce0/0x1140 drivers/input/input.c:2395
hidp_session_dev_add net/bluetooth/hidp/core.c:861 [inline]
hidp_session_probe+0x1a5/0x8a0 net/bluetooth/hidp/core.c:1139
l2cap_register_user+0xc2/0x1d0 net/bluetooth/l2cap_core.c:1725
hidp_connection_add+0x158b/0x1a20 net/bluetooth/hidp/core.c:1411
do_hidp_sock_ioctl net/bluetooth/hidp/sock.c:81 [inline]
hidp_sock_ioctl+0x403/0x650 net/bluetooth/hidp/sock.c:128
sock_do_ioctl+0x101/0x320 net/socket.c:1328
sock_ioctl+0x5c9/0x7f0 net/socket.c:1449
vfs_ioctl fs/ioctl.c:51 [inline]
__do_sys_ioctl fs/ioctl.c:597 [inline]
__se_sys_ioctl+0xff/0x170 fs/ioctl.c:583
do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
do_syscall_64+0x15f/0xf80 arch/x86/entry/syscall_64.c:94
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fcb5e6cce59
Code: ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 e8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fcb5c905028 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00007fcb5e946090 RCX: 00007fcb5e6cce59
RDX: 00002000000000c0 RSI: 00000000400448c8 RDI: 0000000000000009
RBP: 00007fcb5e762d6f R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 00007fcb5e946128 R14: 00007fcb5e946090 R15: 00007ffddb8adae8
</TASK>
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
^ permalink raw reply
* Re: [PATCH v5 5/6] mfd: motorola-cpcap: diverge configuration per-board
From: Svyatoslav Ryhel @ 2026-05-20 17:25 UTC (permalink / raw)
To: Lee Jones
Cc: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Pavel Machek, David Lechner, Tony Lindgren, linux-input,
devicetree, linux-kernel, linux-leds
In-Reply-To: <20260520170234.GN2767592@google.com>
ср, 20 трав. 2026 р. о 20:02 Lee Jones <lee@kernel.org> пише:
>
> On Wed, 20 May 2026, Svyatoslav Ryhel wrote:
>
> > ср, 20 трав. 2026 р. о 19:05 Lee Jones <lee@kernel.org> пише:
> > >
> > > On Wed, 20 May 2026, Svyatoslav Ryhel wrote:
> > >
> > > > ср, 20 трав. 2026 р. о 18:08 Lee Jones <lee@kernel.org> пише:
> > > > >
> > > > > On Sun, 10 May 2026, Svyatoslav Ryhel wrote:
> > > > >
> > > > > > MFD have rigid subdevice structure which does not allow flexible dynamic
> > > > > > subdevice linking. Address this by diverging CPCAP subdevice composition
> > > > > > to take into account board specific configuration.
> > > > > >
> > > > > > Create a common default subdevice composition, rename existing subdevice
> > > > > > composition into cpcap_mapphone_mfd_devices since it targets mainly
> > > > > > Mapphone board.
> > > > > >
> > > > > > Removed st,6556002 as it is no longer applicable to all cases and
> > > > > > duplicates motorola,cpcap, which is used as the default composition.
> > > > > >
> > > > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > > > ---
> > > > > > drivers/mfd/motorola-cpcap.c | 142 ++++++++++++++++-------------
> > > > > > include/linux/mfd/motorola-cpcap.h | 6 ++
> > > > > > 2 files changed, 87 insertions(+), 61 deletions(-)
> > > > >
> > > > > Looking much better, thanks.
> > > > >
> > > > > Nit: A patch-level changelog really is much more helpful to reviewers.
> > > > >
> > > >
> > > > Noted, but I will not guarantee that I will do patch-level changelogs, sorry.
> > >
> > > That's fine. All I can do is ask.
> > >
> > > Note that, helping out reviewers usually ends up helping you too.
> > >
> > > [...]
> > >
> > > > > > +static const struct mfd_cell cpcap_default_mfd_devices[] = {
> > > > > > + MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,cpcap-adc"),
> > > > > > + MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0,
> > > > > > + "motorola,cpcap-battery"),
> > > > > > + MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0,
> > > > > > + "motorola,cpcap-regulator"),
> > > > > > + MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> > > > > > + MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0,
> > > > > > + "motorola,cpcap-pwrbutton"),
> > > > > > + MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0,
> > > > > > + "motorola,cpcap-usb-phy"),
> > > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> > > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> > > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> > > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> > > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> > > > > > + MFD_CELL_NAME("cpcap-codec"),
> > > > > > +};
> > > > >
> > > > > Nit: I wouldn't complain if you wanted to have all of these on a single
> > > > > line for neatness.
> > > > >
> > > >
> > > > Noted
> > > >
> > > > > > +static const struct mfd_cell cpcap_mapphone_mfd_devices[] = {
> > > > > > + MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0,
> > > > > > + "motorola,mapphone-cpcap-adc"),
> > > > > > + MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0,
> > > > > > + "motorola,cpcap-battery"),
> > > > > > + MFD_CELL_OF("cpcap-charger", NULL, NULL, 0, 0,
> > > > > > + "motorola,mapphone-cpcap-charger"),
> > > > > > + MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0,
> > > > > > + "motorola,mapphone-cpcap-regulator"),
> > > > > > + MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> > > > > > + MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0,
> > > > > > + "motorola,cpcap-pwrbutton"),
> > > > > > + MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0,
> > > > > > + "motorola,mapphone-cpcap-usb-phy"),
> > > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> > > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> > > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> > > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> > > > > > + MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> > > > > > + MFD_CELL_NAME("cpcap-codec"),
> > > > > > };
> > > > >
> > > > > A lot of these are duplicated, right?
> > > > >
> > > > > I would have a comment set, then the differences in separate containers.
> > > >
> > > > It may be impossible to predict a generic setup since some devices may
> > > > require unique compatibles, other may not have LEDs, third may be
> > > > partially incompatible with existing cells. In other mfd cases
> > > > creating a generic bundle might be good, but in this case I would
> > > > suggest better to keep these separate entirely per-device. They will
> > > > not take much space, nor add confusion with these macros.
> > >
> > > I'm not sure we're understanding each other. Let me give you an example:
> > >
> > > static const struct mfd_cell cpcap_common_devices[] = {
> > > MFD_CELL_OF("cpcap_battery", NULL, NULL, 0, 0, "motorola,cpcap-battery"),
> > > MFD_CELL_OF("cpcap-rtc", NULL, NULL, 0, 0, "motorola,cpcap-rtc"),
> > > MFD_CELL_OF("cpcap-pwrbutton", NULL, NULL, 0, 0, "motorola,cpcap-pwrbutton"),
> > > MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 0, "motorola,cpcap-led-red"),
> > > MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 1, "motorola,cpcap-led-green"),
> > > MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 2, "motorola,cpcap-led-blue"),
> > > MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 3, "motorola,cpcap-led-adl"),
> > > MFD_CELL_OF("cpcap-led", NULL, NULL, 0, 4, "motorola,cpcap-led-cp"),
> > > MFD_CELL_NAME("cpcap-codec"),
> > > };
> > >
> > > static const struct mfd_cell cpcap_default_devices[] = {
> > > MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,cpcap-adc"),
> > > MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0, "motorola,cpcap-regulator"),
> > > MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0, "motorola,cpcap-usb-phy"),
> > > };
> > >
> > > static const struct mfd_cell cpcap_mapphone_devices[] = {
> > > MFD_CELL_OF("cpcap_adc", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-adc"),
> > > MFD_CELL_OF("cpcap-charger", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-charger"),
> > > MFD_CELL_OF("cpcap-regulator", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-regulator"),
> > > MFD_CELL_OF("cpcap-usb-phy", NULL, NULL, 0, 0, "motorola,mapphone-cpcap-usb-phy"),
> > > };
> > >
> > > This way, it's super easy to read / maintain the common and unique devices.
> > >
> > > The only potential drawback would be 2 calls to mfd_add_devices() but
> > > that's common practice.
> > >
> > > Also notice that I droped the "_mfd" parts, which you should too.
> > >
> >
> > What if next added device does not have cpcap-battery, cpcap-led-adl,
> > cpcap-pwrbutton, uses external codec and would require cpcap-watchdog
> > which is not yet present. This will cause issues. Even Mot introduces
> > in the last patch of this patchset has slightly different composition.
>
> We usually just code-up what we have _today_.
>
> If new devices come in with different requirements, it takes 5 mins to rejig.
>
Ok, but I will take into account Mot commit when factoring out common.
> --
> Lee Jones
^ permalink raw reply
* Re: [PATCH v14 2/2] HID: cp2112: Configure I2C bus speed from firmware
From: sashiko-bot @ 2026-05-20 17:44 UTC (permalink / raw)
To: Danny Kaehn; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260520-cp2112-dt-v14-2-b1b4b6734b6f@plexus.com>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [Medium] The driver silently overwrites custom hardware I2C clock speeds with a 100kHz default for devices without firmware nodes.
Pre-existing issues:
- [High] Firmware nodes are attached to device structures without holding a reference, leading to a dangling pointer and use-after-free.
- [High] The delayed work `gpio_poll_worker` is not canceled in the probe error path, leading to a use-after-free if the probe fails.
--
commit 07ff467e4d8df706a7a27476591cd82570fb8287
Author: Danny Kaehn <danny.kaehn@plexus.com>
HID: cp2112: Configure I2C bus speed from firmware
Now that the I2C adapter on the CP2112 can have an associated firmware
node, set the bus speed based on firmware configuration.
> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c
> index e960fc988058f..04379db93571b 100644
> --- a/drivers/hid/hid-cp2112.c
> +++ b/drivers/hid/hid-cp2112.c
[ ... ]
> @@ -1308,6 +1309,9 @@ static int cp2112_probe(struct hid_device *hdev, const struct hid_device_id *id)
> goto err_power_normal;
> }
>
> + i2c_parse_fw_timings(&dev->adap.dev, &timings, true);
> +
[Severity: High]
This isn't a bug introduced by this patch, but does this new call dereference
a dangling pointer?
Earlier in cp2112_probe(), the reference to the child firmware node is
dropped either explicitly via fwnode_handle_put(child) in the OF path, or
implicitly when the fwnode_for_each_child_node loop iterates or completes
in the ACPI path:
} else if (is_of_node(cp2112_fwnode)) {
child = fwnode_get_named_child_node(cp2112_fwnode, "i2c");
device_set_node(&dev->adap.dev, child);
fwnode_handle_put(child);
}
device_set_node(&dev->adap.dev, child) assigns the pointer without taking
its own reference, leaving dev->adap.dev.fwnode as a dangling pointer.
This newly added i2c_parse_fw_timings() call dereferences this released
fwnode via device_property_read_u32().
Could this lead to a use-after-free?
> + config.clock_speed = cpu_to_be32(timings.bus_freq_hz);
[Severity: Medium]
Since use_defaults is set to true, does this overwrite custom hardware I2C
clock speeds with a 100kHz default for devices without firmware nodes?
Before this patch, it looks like cp2112_probe() read the current SMBus
configuration from the device, only modified retry_time, and wrote it back.
This preserved the device's original configured clock speed (which might be
set to 400kHz in the EEPROM).
By calling i2c_parse_fw_timings() with an uninitialized local timings
struct and use_defaults = true, timings.bus_freq_hz will be
unconditionally set to 100kHz if the device lacks a firmware property.
Is it intended to silently downgrade the speed and override user-configured
EEPROM settings?
[Severity: High]
This is a pre-existing issue, but is there a missing cancellation of the
delayed work in the probe error path?
If sysfs_create_group() fails during probe, the code jumps to the
err_gpiochip_remove label, which calls gpiochip_remove(&dev->gc) and
returns:
ret = sysfs_create_group(&hdev->dev.kobj, &cp2112_attr_group);
if (ret < 0) {
...
goto err_gpiochip_remove;
}
...
err_gpiochip_remove:
gpiochip_remove(&dev->gc);
err_free_i2c:
Because gpiochip_add_data() was already called, an IRQ consumer could have
concurrently requested a GPIO line, triggering cp2112_gpio_irq_startup()
and queuing dev->gpio_poll_worker.
Unlike cp2112_remove(), the error path lacks a call to
cancel_delayed_work_sync(&dev->gpio_poll_worker).
Could this mean the queued work will execute on freed memory?
> config.retry_time = cpu_to_be16(1);
>
> ret = cp2112_hid_output(hdev, (u8 *)&config, sizeof(config),
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260520-cp2112-dt-v14-0-b1b4b6734b6f@plexus.com?part=2
^ permalink raw reply
* [PATCH v6 0/6] mfd: cpcap: convert documentation to schema and add Mot board support
From: Svyatoslav Ryhel @ 2026-05-20 18:21 UTC (permalink / raw)
To: Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Lee Jones, Pavel Machek, Svyatoslav Ryhel
Cc: linux-input, devicetree, linux-kernel, linux-leds
The initial goal was only to add support for the CPCAP used in the Mot
Tegra20 board; however, since the documentation was already partially
converted, I decided to complete the conversion to schema too.
The CPCAP regulator, leds, rtc, pwrbutton and core files were converted
from TXT to YAML while preserving the original structure. Mot board
compatibility was added to the regulator and core schema. Since these
were one-line patches, they were not separated into dedicated commits;
however, the commit message notes this for both cases.
Finally, the CPCAP MFD was slightly refactored to improve support for
multiple subcell compositions.
---
Changes in v2:
- fixed code style
- rtc conversion was picked, so patch dropped
- added audio ports description into mfd schema
- splitted schema conversion and compatible addition
- minor style improvements and typo fixes
Changes in v3:
- added regulator node names list into pattern
- filled spi_device_id with driver data
- ADC patches were picked, so changes dropped
Changes in v4:
- dropped regulator patches (applied)
Changes in v5:
- switched to MFD_CELL_* macros
- switched to use determinator of model
- switched to spi_get_device_match_data
Changes in v6:
- removed address-cells and size-cells from main node
- changed macros formatting and OF matches
- factored out common devices and made device addition staged
- dropped cpcap->variant check for 0
- EINVAL > ENODEV in variant checking switch
---
Svyatoslav Ryhel (6):
dt-bindings: leds: leds-cpcap: convert to DT schema
dt-bindings: input: cpcap-pwrbutton: convert to DT schema
dt-bindings: mfd: motorola-cpcap: convert to DT schema
dt-bindings: mfd: motorola-cpcap: document Mapphone and Mot CPCAP
mfd: motorola-cpcap: diverge configuration per-board
mfd: motorola-cpcap: add support for Mot CPCAP composition
.../bindings/input/cpcap-pwrbutton.txt | 20 -
.../input/motorola,cpcap-pwrbutton.yaml | 32 ++
.../devicetree/bindings/leds/leds-cpcap.txt | 29 --
.../bindings/leds/motorola,cpcap-leds.yaml | 42 ++
.../bindings/mfd/motorola,cpcap.yaml | 408 ++++++++++++++++++
.../bindings/mfd/motorola-cpcap.txt | 78 ----
drivers/mfd/motorola-cpcap.c | 143 +++---
include/linux/mfd/motorola-cpcap.h | 7 +
8 files changed, 571 insertions(+), 188 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/input/cpcap-pwrbutton.txt
create mode 100644 Documentation/devicetree/bindings/input/motorola,cpcap-pwrbutton.yaml
delete mode 100644 Documentation/devicetree/bindings/leds/leds-cpcap.txt
create mode 100644 Documentation/devicetree/bindings/leds/motorola,cpcap-leds.yaml
create mode 100644 Documentation/devicetree/bindings/mfd/motorola,cpcap.yaml
delete mode 100644 Documentation/devicetree/bindings/mfd/motorola-cpcap.txt
--
2.51.0
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox