Linux Input/HID development
 help / color / mirror / Atom feed
* [PATCH v2] HID: wacom: validate report length in wacom_intuos_pro2_bt_irq
From: Ibrahim Hashimov @ 2026-07-13  9:34 UTC (permalink / raw)
  To: ping.cheng, jason.gerecke, jikos, bentiss
  Cc: dmitry.torokhov, linux-input, linux-kernel, stable
In-Reply-To: <20260713075110.92469-1-security@auditcode.ai>

wacom_intuos_pro2_bt_irq() receives the wire report length in `len`
but never consults it before parsing. After the report-id gate it
unconditionally calls wacom_intuos_pro2_bt_pen() and then, selected by
features.type, a fixed chain of sub-parsers, none of which receive
`len`:

	wacom_intuos_pro2_bt_pen(wacom);
	if (type == INTUOSP2_BT || type == INTUOSP2S_BT) {
		wacom_intuos_pro2_bt_touch(wacom);
		wacom_intuos_pro2_bt_pad(wacom);
		wacom_intuos_pro2_bt_battery(wacom);
	} else {
		wacom_intuos_gen3_bt_pad(wacom);
		wacom_intuos_gen3_bt_battery(wacom);
	}

Each sub-parser dereferences wacom->data at fixed offsets. The furthest
byte touched on each branch is:

  INTUOSP2_BT / INTUOSP2S_BT: wacom_intuos_pro2_bt_pad() reads data[285]
	(the touchring byte), so the report must be at least 286 bytes;
  INTUOSHT3_BT ("gen3"): wacom_intuos_gen3_bt_battery() reads data[45],
	so the report must be at least 46 bytes.

features.type is selected from the VID/PID id_table entry and
wacom_setup_device_quirks() force-registers the pen/pad/touch inputs
for that type independent of the report descriptor, so a malicious or
malfunctioning paired/spoofed Bluetooth peripheral can advertise that
VID/PID and send an undersized report that still satisfies the
data[0] == 0x80/0x81 gate. The driver then reads past the received
report and forwards the bytes to userspace via evdev (MSC_SERIAL /
ABS_MISC / ABS_WHEEL on the pen and pad input nodes), an out-of-bounds
read with a concrete userspace read-back channel, and a true
out-of-bounds read on transports whose backing buffer is sized to the
(small) report descriptor rather than a fixed-size staging buffer.

This is the same class of bug commit 2f1763f62909 ("HID: wacom: fix
out-of-bounds read in wacom_intuos_bt_irq") already hardened in the
sibling wacom_intuos_bt_irq(), which guards each report id against its
minimum length before parsing.

Guard wacom_intuos_pro2_bt_irq() the same way: before parsing, reject
reports shorter than the furthest offset the selected branch actually
dereferences, warn, and bail out. Because the whole pen/touch/pad/
battery chain runs unconditionally per branch, a single up-front check
against the maximum offset (286 bytes for INTUOSP2_BT/INTUOSP2S_BT,
46 bytes for the gen3 branch) bounds every sub-parser. Returning 0 on
a short report also skips those calls for the same malformed report,
which is the safe, conservative behavior.

Fixes: 4922cd26f03c ("HID: wacom: Support 2nd-gen Intuos Pro's Bluetooth classic interface")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
v2: the v1 check (len < 109 / len < 43) only bounded
    wacom_intuos_pro2_bt_pen(). As sashiko-bot pointed out, the
    unconditional touch/pad/battery calls read much further --
    wacom_intuos_pro2_bt_pad() up to data[285] and
    wacom_intuos_gen3_bt_battery() up to data[45] -- so a report in the
    gap (109..285 / 43..45) passed the guard yet still over-read. Raise
    the minimums to the furthest offset each branch dereferences
    (286 / 46) so the guard actually bounds the whole call chain.

 drivers/hid/wacom_wac.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625..89a191cf1924 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1548,6 +1548,19 @@ static int wacom_intuos_pro2_bt_irq(struct wacom_wac *wacom, size_t len)
 		return 0;
 	}
 
+	if (wacom->features.type == INTUOSP2_BT ||
+	    wacom->features.type == INTUOSP2S_BT) {
+		if (len < 286) {
+			dev_warn(wacom->pen_input->dev.parent,
+				 "Pro2 BT report too short: %zu bytes\n", len);
+			return 0;
+		}
+	} else if (len < 46) {
+		dev_warn(wacom->pen_input->dev.parent,
+			 "Pro2 BT report too short: %zu bytes\n", len);
+		return 0;
+	}
+
 	wacom_intuos_pro2_bt_pen(wacom);
 	if (wacom->features.type == INTUOSP2_BT ||
 	    wacom->features.type == INTUOSP2S_BT) {
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related

* Re: [PATCH 0/3] HID: wiimote: new LED behavior on connect, and scoped guards
From: David Rheinsberg @ 2026-07-13  9:28 UTC (permalink / raw)
  To: Rafael Passos, Benjamin Tissoires, Jiri Kosina
  Cc: Shuah Khan, Brigham Campbell, Jori Koolstra, linux-input
In-Reply-To: <DJV56QRLBO6S.3H33MNASQ866@rcpassos.me>

Hi

On Fri, Jul 10, 2026, at 9:35 PM, Rafael Passos wrote:
> I checked sashiko reviews, and there are a few needed fixes.
> I will delay sending the v2 by a few days, in case there are any other reviews.
>
> Specially regarding these two questions:
>
> On Fri Jul 10, 2026 at 12:32 PM -03, Rafael Passos wrote:
>> [...]
>> The hid_info log message prints out the controller id.
>> I was considering adding this number to the controller name too,
>> but was unsure if this could impact applications using it.
> - device naming

What device name are you referring to exactly?

>> The wiimote_hid_probe call can also be changed in this way, but I
>> left it because it uses the wiimote_destroy in one of the err cases (w/ kfree inside).
>> But I can apply the change there too, to make this more complete.
> - wiimote_hid_probe scope cleanup

Ideally, you'd be able to call wiimote_destroy() on partially initialized devices. Not sure this is doable, but I would love to see it. I don't think using __free is worth it, if you retain the other goto labels in that function.

I mean, this is an old driver, lots of the cleanup-handlers were not around back then. But if I were to chose, I would either keep it as it is, or ensure that the destructors work with any partially initialized device just fine, and correctly checks that it only touches stuff that was initialized (or ensures the functions it calls do so).

Anyway, I like the series. I will review your v2 properly, so please go ahead!

Thanks
David

^ permalink raw reply

* Re: [PATCH] HID: wacom: validate report length before Intuos Pro2 BT pen parse
From: sashiko-bot @ 2026-07-13  8:01 UTC (permalink / raw)
  To: Ibrahim Hashimov; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260713075110.92469-1-security@auditcode.ai>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The length checks introduced by the patch are insufficient. While they correctly bound the reads in `wacom_intuos_pro2_bt_pen()`, they fail to prevent out-of-bounds reads in the subsequent unconditional calls to the touch, pad, and battery parsing functions if the report length falls between the checked bounds and the maximum read offset.
--

--- Patch [1]: [PATCH] HID: wacom: validate report length before Intuos Pro2 BT pen parse ---
commit 1ba7672ecbf2a3b1f2ac28c705ac40735f4dcbcf
Author: Ibrahim Hashimov <security@auditcode.ai>

HID: wacom: validate report length before Intuos Pro2 BT pen parse

This commit adds length validation in wacom_intuos_pro2_bt_irq() to
prevent out-of-bounds reads before parsing pen reports for Intuos Pro2
and Gen3 Bluetooth devices.

> diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
> index da1f0ea85625d..89a191cf1924c 100644
> --- a/drivers/hid/wacom_wac.c
> +++ b/drivers/hid/wacom_wac.c
> @@ -1548,6 +1548,19 @@ static int wacom_intuos_pro2_bt_irq(struct wacom_wac *wacom, size_t len)
>  		return 0;
>  	}
>  
> +	if (wacom->features.type == INTUOSP2_BT ||
> +	    wacom->features.type == INTUOSP2S_BT) {
> +		if (len < 109) {
> +			dev_warn(wacom->pen_input->dev.parent,
> +				 "Pro2 BT report too short: %zu bytes\n", len);
> +			return 0;
> +		}
> +	} else if (len < 43) {
> +		dev_warn(wacom->pen_input->dev.parent,
> +			 "Pro2 BT report too short: %zu bytes\n", len);
> +		return 0;
> +	}
> +
>  	wacom_intuos_pro2_bt_pen(wacom);
>  	if (wacom->features.type == INTUOSP2_BT ||
>  	    wacom->features.type == INTUOSP2S_BT) {

[Severity: High]
This is a pre-existing issue, but are these length checks sufficient to prevent
out-of-bounds reads in the subsequent parsing functions?

While this successfully protects wacom_intuos_pro2_bt_pen(), the code goes on
to unconditionally call other parsing functions:

drivers/hid/wacom_wac.c:wacom_intuos_pro2_bt_irq() {
    ...
	wacom_intuos_pro2_bt_pen(wacom);
	if (wacom->features.type == INTUOSP2_BT ||
	    wacom->features.type == INTUOSP2S_BT) {
		wacom_intuos_pro2_bt_touch(wacom);
		wacom_intuos_pro2_bt_pad(wacom);
    ...
}

In wacom_intuos_pro2_bt_pad(), it reads up to offset 285:

drivers/hid/wacom_wac.c:wacom_intuos_pro2_bt_pad() {
    ...
	int ring = data[285] & 0x7F;
	bool ringstatus = data[285] & 0x80;
    ...
}

And for Gen3 devices, after the len < 43 check, it calls
wacom_intuos_gen3_bt_battery() which reads up to offset 45:

drivers/hid/wacom_wac.c:wacom_intuos_gen3_bt_battery() {
    ...
	bool chg = data[45] & 0x80;
	int battery_status = data[45] & 0x7F;
    ...
}

If a spoofed device sends a report that passes the initial minimum bounds
check (e.g., exactly 109 bytes for Pro2 or 43 bytes for Gen3), could this
still result in out-of-bounds reads when parsing the pad, touch, or battery
data?

>  		wacom_intuos_pro2_bt_touch(wacom);
>  		wacom_intuos_pro2_bt_pad(wacom);
>  		wacom_intuos_pro2_bt_battery(wacom);
>  	} else {
>  		wacom_intuos_gen3_bt_pad(wacom);
>  		wacom_intuos_gen3_bt_battery(wacom);
>  	}
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713075110.92469-1-security@auditcode.ai?part=1

^ permalink raw reply

* [PATCH] HID: wacom: validate report length before Intuos Pro2 BT pen parse
From: Ibrahim Hashimov @ 2026-07-13  7:51 UTC (permalink / raw)
  To: Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires
  Cc: linux-input, linux-kernel, stable

wacom_intuos_pro2_bt_irq() receives the wire report length in `len`
but never consults it before calling wacom_intuos_pro2_bt_pen(). The
only admission check on this path is the report-id byte:

	if (data[0] != 0x80 && data[0] != 0x81) {
		...
		return 0;
	}
	wacom_intuos_pro2_bt_pen(wacom);

wacom_intuos_pro2_bt_pen() itself does not even receive `len` (it
takes only `struct wacom_wac *wacom`), so it cannot bound its own
reads. For INTUOSP2_BT / INTUOSP2S_BT devices it unconditionally does:

	wacom->serial[0] = get_unaligned_le64(&data[99]);
	wacom->id[0]     = get_unaligned_le16(&data[107]);

i.e. it reads up to offset 108 regardless of how many bytes the
peripheral actually sent. features.type is selected from the VID/PID
id_table entry (BT_DEVICE_WACOM(0x361) -> INTUOSP2_BT) and
wacom_setup_device_quirks() force-registers the pen/pad/touch inputs
for that type independent of the report descriptor, so a malicious or
malfunctioning paired/spoofed Bluetooth peripheral can advertise that
VID/PID and send an undersized report (e.g. 10 bytes) that still
satisfies the data[0] == 0x80/0x81 gate. The driver then reads past
the received report and forwards the bytes to userspace via evdev
(MSC_SERIAL / ABS_MISC on the pen input node), an out-of-bounds read
with a concrete userspace read-back channel, and a true
out-of-bounds read on transports where the backing buffer is sized to
the (small) report descriptor rather than a fixed-size staging
buffer.

The non-Pro2 branch of the same function (INTUOSHT3_BT, reading
&data[33]/&data[41]) has the identical defect at a smaller offset.

This is the same class of bug commit 2f1763f62909 ("HID: wacom: fix
out-of-bounds read in wacom_intuos_bt_irq") already hardened in the
sibling function wacom_intuos_bt_irq(), which added explicit
short-report guards before parsing:

	case 0x04:
		if (len < 32) {
			dev_warn(..., "Report 0x04 too short: %zu bytes\n", len);
			break;
		}
		wacom_intuos_bt_process_data(wacom, data + i);
		...
	case 0x03:
		if (i == 1 && len < 22) {
			dev_warn(..., "Report 0x03 too short: %zu bytes\n", len);
			break;
		}

wacom_intuos_pro2_bt_irq() never received the analogous guard.

Fix it the same way: before calling wacom_intuos_pro2_bt_pen(), check
`len` against the minimum size each branch of that function actually
dereferences (109 bytes for INTUOSP2_BT/INTUOSP2S_BT, whose furthest
read is &data[107] as a le16 -> byte index 108; 43 bytes for the
INTUOSHT3_BT ("gen3") branch, whose furthest read is &data[41] as a
le16 -> byte index 42). On a short report, warn and bail out before
touching wacom->data past what was actually received, exactly as
wacom_intuos_bt_irq() now does for its own report ids. Returning 0
here also skips the subsequent pro2 touch/pad/battery calls for the
same malformed report, which is the safe, conservative behavior.

Runtime-verified on a v6.19 stand: injecting the truncated report
via a spoofed 056a:0361 BT peripheral leaks bytes past the declared
report length to the pen evdev node (MSC_SERIAL / ABS_MISC) before
this fix, and the same input is rejected by the new length check
with no leaked event afterward. KASAN itself stays quiet on the
fixed-size uhid staging buffer used to inject the report (the
over-read there lands in-object); on a descriptor-sized USB/BT
buffer the identical read crosses the slab boundary and is a
genuine KASAN-visible out-of-bounds read.

Fixes: 4922cd26f03c ("HID: wacom: Support 2nd-gen Intuos Pro's Bluetooth classic interface")
Cc: stable@vger.kernel.org
Signed-off-by: Ibrahim Hashimov <security@auditcode.ai>
Assisted-by: AuditCode-AI:2026.07
---
 drivers/hid/wacom_wac.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625..89a191cf1924 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -1548,6 +1548,19 @@ static int wacom_intuos_pro2_bt_irq(struct wacom_wac *wacom, size_t len)
 		return 0;
 	}
 
+	if (wacom->features.type == INTUOSP2_BT ||
+	    wacom->features.type == INTUOSP2S_BT) {
+		if (len < 109) {
+			dev_warn(wacom->pen_input->dev.parent,
+				 "Pro2 BT report too short: %zu bytes\n", len);
+			return 0;
+		}
+	} else if (len < 43) {
+		dev_warn(wacom->pen_input->dev.parent,
+			 "Pro2 BT report too short: %zu bytes\n", len);
+		return 0;
+	}
+
 	wacom_intuos_pro2_bt_pen(wacom);
 	if (wacom->features.type == INTUOSP2_BT ||
 	    wacom->features.type == INTUOSP2S_BT) {
-- 
2.50.1 (Apple Git-155)


^ permalink raw reply related

* [dtor-input:next] BUILD SUCCESS 79d7a453c79e818b8059b1aa6d6a7ad53d9d4f72
From: kernel test robot @ 2026-07-13  7:01 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git next
branch HEAD: 79d7a453c79e818b8059b1aa6d6a7ad53d9d4f72  Input: i8042 - replace strlcat() with seq_buf and scnprintf()

elapsed time: 729m

configs tested: 193
configs skipped: 2

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha                             allnoconfig    gcc-16.1.0
alpha                            allyesconfig    gcc-16.1.0
alpha                               defconfig    gcc-16.1.0
arc                              allmodconfig    clang-23
arc                               allnoconfig    gcc-16.1.0
arc                              allyesconfig    clang-23
arc                                 defconfig    gcc-16.1.0
arc                   randconfig-001-20260713    gcc-8.5.0
arc                   randconfig-002-20260713    gcc-8.5.0
arm                               allnoconfig    gcc-16.1.0
arm                              allyesconfig    clang-23
arm                                 defconfig    gcc-16.1.0
arm                   randconfig-001-20260713    gcc-8.5.0
arm                   randconfig-002-20260713    gcc-8.5.0
arm                   randconfig-003-20260713    gcc-8.5.0
arm                   randconfig-004-20260713    gcc-8.5.0
arm64                            allmodconfig    clang-23
arm64                             allnoconfig    gcc-16.1.0
arm64                               defconfig    gcc-16.1.0
arm64                          randconfig-001    clang-23
arm64                 randconfig-001-20260713    clang-23
arm64                 randconfig-001-20260713    gcc-13.4.0
arm64                          randconfig-002    clang-23
arm64                 randconfig-002-20260713    clang-23
arm64                 randconfig-002-20260713    gcc-13.4.0
arm64                          randconfig-003    clang-23
arm64                 randconfig-003-20260713    clang-23
arm64                 randconfig-003-20260713    gcc-13.4.0
arm64                          randconfig-004    clang-23
arm64                 randconfig-004-20260713    clang-23
arm64                 randconfig-004-20260713    gcc-13.4.0
csky                             allmodconfig    gcc-16.1.0
csky                              allnoconfig    gcc-16.1.0
csky                                defconfig    gcc-16.1.0
csky                           randconfig-001    clang-23
csky                  randconfig-001-20260713    clang-23
csky                  randconfig-001-20260713    gcc-13.4.0
csky                           randconfig-002    clang-23
csky                  randconfig-002-20260713    clang-23
csky                  randconfig-002-20260713    gcc-13.4.0
hexagon                          allmodconfig    gcc-16.1.0
hexagon                           allnoconfig    gcc-16.1.0
hexagon                             defconfig    gcc-16.1.0
hexagon               randconfig-001-20260713    clang-23
hexagon               randconfig-002-20260713    clang-23
i386                             allmodconfig    clang-22
i386                              allnoconfig    gcc-16.1.0
i386                             allyesconfig    clang-22
i386        buildonly-randconfig-001-20260713    gcc-14
i386        buildonly-randconfig-002-20260713    gcc-14
i386        buildonly-randconfig-003-20260713    gcc-14
i386        buildonly-randconfig-004-20260713    gcc-14
i386        buildonly-randconfig-005-20260713    gcc-14
i386        buildonly-randconfig-006-20260713    gcc-14
i386                                defconfig    gcc-16.1.0
i386                  randconfig-001-20260713    clang-22
i386                  randconfig-002-20260713    clang-22
i386                  randconfig-003-20260713    clang-22
i386                  randconfig-004-20260713    clang-22
i386                  randconfig-005-20260713    clang-22
i386                  randconfig-006-20260713    clang-22
i386                  randconfig-007-20260713    clang-22
i386                  randconfig-011-20260713    clang-22
i386                  randconfig-012-20260713    clang-22
i386                  randconfig-013-20260713    clang-22
i386                  randconfig-014-20260713    clang-22
i386                  randconfig-015-20260713    clang-22
i386                  randconfig-016-20260713    clang-22
i386                  randconfig-017-20260713    clang-22
loongarch                        allmodconfig    clang-23
loongarch                         allnoconfig    gcc-16.1.0
loongarch                           defconfig    clang-23
loongarch             randconfig-001-20260713    clang-23
loongarch             randconfig-002-20260713    clang-23
m68k                             allmodconfig    gcc-16.1.0
m68k                              allnoconfig    gcc-16.1.0
m68k                             allyesconfig    clang-23
m68k                                defconfig    clang-23
microblaze                        allnoconfig    gcc-16.1.0
microblaze                       allyesconfig    gcc-16.1.0
microblaze                          defconfig    clang-23
mips                             allmodconfig    gcc-16.1.0
mips                              allnoconfig    gcc-16.1.0
mips                             allyesconfig    gcc-16.1.0
mips                  maltasmvp_eva_defconfig    gcc-16.1.0
mips                        omega2p_defconfig    clang-17
mips                          rm200_defconfig    gcc-16.1.0
nios2                            allmodconfig    clang-20
nios2                             allnoconfig    clang-23
nios2                               defconfig    clang-23
nios2                 randconfig-001-20260713    clang-23
nios2                 randconfig-002-20260713    clang-23
openrisc                         allmodconfig    clang-20
openrisc                          allnoconfig    clang-23
openrisc                            defconfig    gcc-16.1.0
parisc                           allmodconfig    gcc-16.1.0
parisc                            allnoconfig    clang-23
parisc                           allyesconfig    clang-17
parisc                           allyesconfig    gcc-16.1.0
parisc                              defconfig    gcc-16.1.0
parisc                randconfig-001-20260713    clang-23
parisc                randconfig-002-20260713    clang-23
parisc64                            defconfig    clang-23
powerpc                          allmodconfig    gcc-16.1.0
powerpc                           allnoconfig    clang-23
powerpc               randconfig-001-20260713    clang-23
powerpc               randconfig-002-20260713    clang-23
powerpc64             randconfig-001-20260713    clang-23
powerpc64             randconfig-002-20260713    clang-23
riscv                            allmodconfig    clang-23
riscv                             allnoconfig    clang-23
riscv                            allyesconfig    clang-23
riscv                               defconfig    gcc-16.1.0
riscv                          randconfig-001    clang-19
riscv                 randconfig-001-20260713    clang-19
riscv                          randconfig-002    clang-19
riscv                 randconfig-002-20260713    clang-19
s390                             allmodconfig    clang-17
s390                             allmodconfig    clang-23
s390                              allnoconfig    clang-23
s390                             allyesconfig    gcc-16.1.0
s390                                defconfig    clang-18
s390                                defconfig    gcc-16.1.0
s390                           randconfig-001    clang-19
s390                  randconfig-001-20260713    clang-19
s390                           randconfig-002    clang-19
s390                  randconfig-002-20260713    clang-19
sh                               allmodconfig    gcc-16.1.0
sh                                allnoconfig    clang-23
sh                               allyesconfig    clang-17
sh                               allyesconfig    gcc-16.1.0
sh                                  defconfig    gcc-14
sh                             randconfig-001    clang-19
sh                    randconfig-001-20260713    clang-19
sh                             randconfig-002    clang-19
sh                    randconfig-002-20260713    clang-19
sparc                             allnoconfig    clang-23
sparc                               defconfig    gcc-16.1.0
sparc                 randconfig-001-20260713    gcc-12.5.0
sparc                 randconfig-002-20260713    gcc-12.5.0
sparc64                          allmodconfig    clang-20
sparc64                             defconfig    gcc-14
sparc64               randconfig-001-20260713    gcc-12.5.0
sparc64               randconfig-002-20260713    gcc-12.5.0
um                               allmodconfig    clang-17
um                               allmodconfig    clang-23
um                                allnoconfig    clang-23
um                               allyesconfig    gcc-16.1.0
um                                  defconfig    gcc-14
um                             i386_defconfig    gcc-14
um                    randconfig-001-20260713    gcc-12.5.0
um                    randconfig-002-20260713    gcc-12.5.0
um                           x86_64_defconfig    gcc-14
x86_64                           allmodconfig    clang-22
x86_64                            allnoconfig    clang-23
x86_64                           allyesconfig    clang-22
x86_64      buildonly-randconfig-001-20260713    clang-22
x86_64      buildonly-randconfig-002-20260713    clang-22
x86_64      buildonly-randconfig-003-20260713    clang-22
x86_64      buildonly-randconfig-004-20260713    clang-22
x86_64      buildonly-randconfig-005-20260713    clang-22
x86_64      buildonly-randconfig-006-20260713    clang-22
x86_64                              defconfig    gcc-14
x86_64                                  kexec    clang-22
x86_64                randconfig-001-20260713    gcc-13
x86_64                randconfig-002-20260713    gcc-13
x86_64                randconfig-003-20260713    gcc-13
x86_64                randconfig-004-20260713    gcc-13
x86_64                randconfig-005-20260713    gcc-13
x86_64                randconfig-006-20260713    gcc-13
x86_64                randconfig-011-20260713    gcc-14
x86_64                randconfig-012-20260713    gcc-14
x86_64                randconfig-013-20260713    gcc-14
x86_64                randconfig-014-20260713    gcc-14
x86_64                randconfig-015-20260713    gcc-14
x86_64                randconfig-016-20260713    gcc-14
x86_64                randconfig-071-20260713    gcc-14
x86_64                randconfig-072-20260713    gcc-14
x86_64                randconfig-073-20260713    gcc-14
x86_64                randconfig-074-20260713    gcc-14
x86_64                randconfig-075-20260713    gcc-14
x86_64                randconfig-076-20260713    gcc-14
x86_64                               rhel-9.4    clang-22
x86_64                           rhel-9.4-bpf    gcc-14
x86_64                          rhel-9.4-func    clang-22
x86_64                    rhel-9.4-kselftests    clang-22
x86_64                         rhel-9.4-kunit    gcc-14
x86_64                           rhel-9.4-ltp    gcc-14
x86_64                          rhel-9.4-rust    clang-22
xtensa                            allnoconfig    clang-23
xtensa                           allyesconfig    clang-20
xtensa                randconfig-001-20260713    gcc-12.5.0
xtensa                randconfig-002-20260713    gcc-12.5.0

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* [dtor-input:for-linus] BUILD SUCCESS adea84ee6cdea611146c4251d3c1616f5a09feca
From: kernel test robot @ 2026-07-13  6:49 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input

tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git for-linus
branch HEAD: adea84ee6cdea611146c4251d3c1616f5a09feca  Input: mms114 - fix multi-touch slot corruption

elapsed time: 722m

configs tested: 189
configs skipped: 2

The following configs have been built successfully.
More configs may be tested in the coming days.

tested configs:
alpha                             allnoconfig    gcc-16.1.0
alpha                            allyesconfig    gcc-16.1.0
alpha                               defconfig    gcc-16.1.0
arc                              allmodconfig    clang-23
arc                               allnoconfig    gcc-16.1.0
arc                              allyesconfig    clang-23
arc                                 defconfig    gcc-16.1.0
arc                   randconfig-001-20260713    gcc-8.5.0
arc                   randconfig-002-20260713    gcc-8.5.0
arm                               allnoconfig    gcc-16.1.0
arm                              allyesconfig    clang-23
arm                                 defconfig    gcc-16.1.0
arm                   randconfig-001-20260713    gcc-8.5.0
arm                   randconfig-002-20260713    gcc-8.5.0
arm                   randconfig-003-20260713    gcc-8.5.0
arm                   randconfig-004-20260713    gcc-8.5.0
arm64                            allmodconfig    clang-23
arm64                             allnoconfig    gcc-16.1.0
arm64                               defconfig    gcc-16.1.0
arm64                          randconfig-001    clang-23
arm64                 randconfig-001-20260713    clang-23
arm64                 randconfig-001-20260713    gcc-13.4.0
arm64                          randconfig-002    clang-23
arm64                 randconfig-002-20260713    clang-23
arm64                 randconfig-002-20260713    gcc-13.4.0
arm64                          randconfig-003    clang-23
arm64                 randconfig-003-20260713    clang-23
arm64                 randconfig-003-20260713    gcc-13.4.0
arm64                          randconfig-004    clang-23
arm64                 randconfig-004-20260713    clang-23
arm64                 randconfig-004-20260713    gcc-13.4.0
csky                             allmodconfig    gcc-16.1.0
csky                              allnoconfig    gcc-16.1.0
csky                                defconfig    gcc-16.1.0
csky                           randconfig-001    clang-23
csky                  randconfig-001-20260713    clang-23
csky                  randconfig-001-20260713    gcc-13.4.0
csky                           randconfig-002    clang-23
csky                  randconfig-002-20260713    clang-23
csky                  randconfig-002-20260713    gcc-13.4.0
hexagon                          allmodconfig    gcc-16.1.0
hexagon                           allnoconfig    gcc-16.1.0
hexagon                             defconfig    gcc-16.1.0
hexagon               randconfig-001-20260713    clang-23
hexagon               randconfig-002-20260713    clang-23
i386                             allmodconfig    clang-22
i386                              allnoconfig    gcc-16.1.0
i386                             allyesconfig    clang-22
i386        buildonly-randconfig-001-20260713    gcc-14
i386        buildonly-randconfig-002-20260713    gcc-14
i386        buildonly-randconfig-003-20260713    gcc-14
i386        buildonly-randconfig-004-20260713    gcc-14
i386        buildonly-randconfig-005-20260713    gcc-14
i386        buildonly-randconfig-006-20260713    gcc-14
i386                                defconfig    gcc-16.1.0
i386                  randconfig-001-20260713    clang-22
i386                  randconfig-002-20260713    clang-22
i386                  randconfig-003-20260713    clang-22
i386                  randconfig-004-20260713    clang-22
i386                  randconfig-005-20260713    clang-22
i386                  randconfig-006-20260713    clang-22
i386                  randconfig-007-20260713    clang-22
i386                  randconfig-011-20260713    clang-22
i386                  randconfig-012-20260713    clang-22
i386                  randconfig-013-20260713    clang-22
i386                  randconfig-014-20260713    clang-22
i386                  randconfig-015-20260713    clang-22
i386                  randconfig-016-20260713    clang-22
i386                  randconfig-017-20260713    clang-22
loongarch                        allmodconfig    clang-23
loongarch                         allnoconfig    gcc-16.1.0
loongarch                           defconfig    clang-23
loongarch             randconfig-001-20260713    clang-23
loongarch             randconfig-002-20260713    clang-23
m68k                             allmodconfig    gcc-16.1.0
m68k                              allnoconfig    gcc-16.1.0
m68k                             allyesconfig    clang-23
m68k                                defconfig    clang-23
microblaze                        allnoconfig    gcc-16.1.0
microblaze                       allyesconfig    gcc-16.1.0
microblaze                          defconfig    clang-23
mips                             allmodconfig    gcc-16.1.0
mips                              allnoconfig    gcc-16.1.0
mips                             allyesconfig    gcc-16.1.0
mips                  maltasmvp_eva_defconfig    gcc-16.1.0
mips                        omega2p_defconfig    clang-17
mips                          rm200_defconfig    gcc-16.1.0
nios2                            allmodconfig    clang-20
nios2                             allnoconfig    clang-23
nios2                               defconfig    clang-23
nios2                 randconfig-001-20260713    clang-23
nios2                 randconfig-002-20260713    clang-23
openrisc                         allmodconfig    clang-20
openrisc                          allnoconfig    clang-23
openrisc                            defconfig    gcc-16.1.0
parisc                           allmodconfig    gcc-16.1.0
parisc                            allnoconfig    clang-23
parisc                           allyesconfig    clang-17
parisc                              defconfig    gcc-16.1.0
parisc                randconfig-001-20260713    clang-23
parisc                randconfig-002-20260713    clang-23
parisc64                            defconfig    clang-23
powerpc                          allmodconfig    gcc-16.1.0
powerpc                           allnoconfig    clang-23
powerpc               randconfig-001-20260713    clang-23
powerpc               randconfig-002-20260713    clang-23
powerpc64             randconfig-001-20260713    clang-23
powerpc64             randconfig-002-20260713    clang-23
riscv                            allmodconfig    clang-23
riscv                             allnoconfig    clang-23
riscv                            allyesconfig    clang-23
riscv                               defconfig    gcc-16.1.0
riscv                          randconfig-001    clang-19
riscv                 randconfig-001-20260713    clang-19
riscv                          randconfig-002    clang-19
riscv                 randconfig-002-20260713    clang-19
s390                             allmodconfig    clang-17
s390                              allnoconfig    clang-23
s390                             allyesconfig    gcc-16.1.0
s390                                defconfig    clang-18
s390                                defconfig    gcc-16.1.0
s390                           randconfig-001    clang-19
s390                  randconfig-001-20260713    clang-19
s390                           randconfig-002    clang-19
s390                  randconfig-002-20260713    clang-19
sh                               allmodconfig    gcc-16.1.0
sh                                allnoconfig    clang-23
sh                               allyesconfig    clang-17
sh                                  defconfig    gcc-14
sh                             randconfig-001    clang-19
sh                    randconfig-001-20260713    clang-19
sh                             randconfig-002    clang-19
sh                    randconfig-002-20260713    clang-19
sparc                             allnoconfig    clang-23
sparc                               defconfig    gcc-16.1.0
sparc                 randconfig-001-20260713    gcc-12.5.0
sparc                 randconfig-002-20260713    gcc-12.5.0
sparc64                          allmodconfig    clang-20
sparc64                             defconfig    gcc-14
sparc64               randconfig-001-20260713    gcc-12.5.0
sparc64               randconfig-002-20260713    gcc-12.5.0
um                               allmodconfig    clang-17
um                                allnoconfig    clang-23
um                               allyesconfig    gcc-16.1.0
um                                  defconfig    gcc-14
um                             i386_defconfig    gcc-14
um                    randconfig-001-20260713    gcc-12.5.0
um                    randconfig-002-20260713    gcc-12.5.0
um                           x86_64_defconfig    gcc-14
x86_64                           allmodconfig    clang-22
x86_64                            allnoconfig    clang-23
x86_64                           allyesconfig    clang-22
x86_64      buildonly-randconfig-001-20260713    clang-22
x86_64      buildonly-randconfig-002-20260713    clang-22
x86_64      buildonly-randconfig-003-20260713    clang-22
x86_64      buildonly-randconfig-004-20260713    clang-22
x86_64      buildonly-randconfig-005-20260713    clang-22
x86_64      buildonly-randconfig-006-20260713    clang-22
x86_64                              defconfig    gcc-14
x86_64                                  kexec    clang-22
x86_64                randconfig-001-20260713    gcc-13
x86_64                randconfig-002-20260713    gcc-13
x86_64                randconfig-003-20260713    gcc-13
x86_64                randconfig-004-20260713    gcc-13
x86_64                randconfig-005-20260713    gcc-13
x86_64                randconfig-006-20260713    gcc-13
x86_64                randconfig-011-20260713    gcc-14
x86_64                randconfig-012-20260713    gcc-14
x86_64                randconfig-013-20260713    gcc-14
x86_64                randconfig-014-20260713    gcc-14
x86_64                randconfig-015-20260713    gcc-14
x86_64                randconfig-016-20260713    gcc-14
x86_64                randconfig-071-20260713    gcc-14
x86_64                randconfig-072-20260713    gcc-14
x86_64                randconfig-073-20260713    gcc-14
x86_64                randconfig-074-20260713    gcc-14
x86_64                randconfig-075-20260713    gcc-14
x86_64                randconfig-076-20260713    gcc-14
x86_64                               rhel-9.4    clang-22
x86_64                           rhel-9.4-bpf    gcc-14
x86_64                          rhel-9.4-func    clang-22
x86_64                    rhel-9.4-kselftests    clang-22
x86_64                         rhel-9.4-kunit    gcc-14
x86_64                           rhel-9.4-ltp    gcc-14
x86_64                          rhel-9.4-rust    clang-22
xtensa                            allnoconfig    clang-23
xtensa                           allyesconfig    clang-20
xtensa                randconfig-001-20260713    gcc-12.5.0
xtensa                randconfig-002-20260713    gcc-12.5.0

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply

* Re: [PATCH] Input: zinitix - support the 6-byte/count touch report used by bt532
From: Nikita Travkin @ 2026-07-13  5:47 UTC (permalink / raw)
  To: Kavan Smith; +Cc: dmitry.torokhov, michael.srba, linux-input, linux-kernel
In-Reply-To: <20260706180826.408989-1-kavansmith82@gmail.com>

Kavan Smith писал(а) 06.07.2026 23:08:
> The driver assumes an 8-byte per-contact report stride with a per-finger
> bitmask in report byte[2]. bt532 firmware (Samsung Galaxy Tab A 8.0,
> msm8916-samsung-gt58) instead packs contacts at a 6-byte stride and reports
> a finger COUNT in byte[2]. Under the 8-byte/bitmask assumption fingers 1+
> are read misaligned, and a two-finger report (count 0x02) makes
> for_each_set_bit() visit only slot 1, collapsing multitouch to a single
> contact.
> 
> Add a per-compatible struct zinitix_chip_data that selects the report stride
> and how byte[2] is interpreted, and attach it to the bt532 compatible (the
> only in-tree user of it). All other compatibles keep the existing 8-byte,
> bitmask behaviour unchanged. Contacts are parsed at the chip's stride, and
> for count-style firmware every slot the chip marks with SUB_BIT_EXIST is
> reported.
> 

Hi!

I don't think this approach is correct here. Since the layout is most
likely fw defined, I don't think it should be attached to the chip
compatible but rather to each specific device - oem driver has a
dedicated fw update just for gt58 for example.

Notably, the driver already has facilities to define which touch mode a
device uses, just that only one mode was implemented so far.

Please refer to the OEM driver [1], perhaps you will be able to notice
something simpler there that'd make this firmware accept mode=2...

See few more minor comments below

Nikita

[1] https://github.com/msm8916-mainline/linux-downstream/blob/SM-T355Y/drivers/input/touchscreen/zinitix_bt532/zinitix_bt532_ts.c

> Signed-off-by: Kavan Smith <kavansmith82@gmail.com>
> ---
>  drivers/input/touchscreen/zinitix.c | 71 +++++++++++++++++++++++++++--
>  1 file changed, 58 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/input/touchscreen/zinitix.c b/drivers/input/touchscreen/zinitix.c
> --- a/drivers/input/touchscreen/zinitix.c
> +++ b/drivers/input/touchscreen/zinitix.c
> @@ -135,26 +135,47 @@
>  #define CHIP_ON_DELAY				15 // ms
>  #define FIRMWARE_ON_DELAY			40 // ms
>  
> +/*
> + * Two Zinitix touch-report layouts exist in the wild. The default is an 8-byte
> + * per-contact stride with a per-finger bitmask in report byte[2]. Some firmware
> + * (e.g. bt532 on the Samsung Galaxy Tab A 8.0) packs contacts at a 6-byte stride
> + * and reports a finger COUNT in byte[2] instead of a bitmask. Selected per
> + * compatible via the of_device_id .data below.
> + */
> +struct zinitix_chip_data {
> +	unsigned int coord_stride;	/* per-contact byte stride in the report */
> +	bool finger_count;		/* byte[2] is a finger count, not a bitmask */
> +};
> +
> +static const struct zinitix_chip_data zinitix_bt532_data = {
> +	.coord_stride = 6,
> +	.finger_count = true,
> +};
> +
>  struct point_coord {
>  	__le16	x;
>  	__le16	y;
>  	u8	width;
>  	u8	sub_status;
> -	// currently unused, but needed as padding:
> -	u8	minor_width;
> -	u8	angle;
> +	/*
> +	 * 8-byte-stride firmware appends two unused bytes here (minor_width,
> +	 * angle); the report is parsed at the chip's coord_stride, so on that
> +	 * firmware they are simply skipped rather than being part of this struct.
> +	 */
>  };
>  
>  struct touch_event {
>  	__le16	status;
>  	u8	finger_mask;
>  	u8	time_stamp;
> -	struct point_coord point_coord[MAX_SUPPORTED_FINGER_NUM];
> +	/* raw per-contact bytes, parsed at the chip's coord_stride (max 8) */
> +	u8	coords[MAX_SUPPORTED_FINGER_NUM * 8];
>  };
>  
>  struct bt541_ts_data {
>  	struct i2c_client *client;
>  	struct input_dev *input_dev;
> +	const struct zinitix_chip_data *chip;
>  	struct touchscreen_properties prop;
>  	struct regulator_bulk_data supplies[2];
>  	u32 zinitix_mode;
> @@ -444,6 +465,8 @@
>  {
>  	struct bt541_ts_data *bt541 = bt541_handler;
>  	struct i2c_client *client = bt541->client;
> +	unsigned int stride = bt541->chip ? bt541->chip->coord_stride : 8;
> +	bool finger_count = bt541->chip ? bt541->chip->finger_count : false;
>  	struct touch_event touch_event;
>  	unsigned long finger_mask;
>  	__le16 icon_events;
> @@ -452,8 +475,11 @@
>  
>  	memset(&touch_event, 0, sizeof(struct touch_event));
>  
> +	/* header (status, finger_mask, time_stamp) + one stride per contact */
>  	error = zinitix_read_data(bt541->client, ZINITIX_POINT_STATUS_REG,
> -				  &touch_event, sizeof(struct touch_event));
> +				  &touch_event,
> +				  offsetof(struct touch_event, coords) +
> +					  MAX_SUPPORTED_FINGER_NUM * stride);
>  	if (error) {
>  		dev_err(&client->dev, "Failed to read in touchpoint struct\n");
>  		goto out;
> @@ -470,13 +496,31 @@
>  		zinitix_report_keys(bt541, le16_to_cpu(icon_events));
>  	}
>  
> -	finger_mask = touch_event.finger_mask;
> -	for_each_set_bit(i, &finger_mask, MAX_SUPPORTED_FINGER_NUM) {
> -		const struct point_coord *p = &touch_event.point_coord[i];
> -
> -		/* Only process contacts that are actually reported */
> -		if (p->sub_status & SUB_BIT_EXIST)
> -			zinitix_report_finger(bt541, i, p);
> +	if (finger_count) {
> +		/*
> +		 * On this firmware byte[2] is a finger COUNT, not a bitmask: a

Which firmware?

> +		 * 2-finger report carries 0x02, so for_each_set_bit() would look
> +		 * only at slot 1 and silently drop slot 0. The chip marks every
> +		 * populated contact with SUB_BIT_EXIST, so iterate all slots and
> +		 * report the present ones.
> +		 */

Maybe worth checking how OEM driver works for different devices
that use zinitix chips.

> +		for (i = 0; i < MAX_SUPPORTED_FINGER_NUM; i++) {
> +			const struct point_coord *p =
> +				(const struct point_coord *)&touch_event.coords[i * stride];
> +
> +			if (p->sub_status & SUB_BIT_EXIST)
> +				zinitix_report_finger(bt541, i, p);
> +		}
> +	} else {
> +		/* default: byte[2] is a per-finger bitmask */
> +		finger_mask = touch_event.finger_mask;
> +		for_each_set_bit(i, &finger_mask, MAX_SUPPORTED_FINGER_NUM) {
> +			const struct point_coord *p =
> +				(const struct point_coord *)&touch_event.coords[i * stride];
> +
> +			if (p->sub_status & SUB_BIT_EXIST)
> +				zinitix_report_finger(bt541, i, p);
> +		}
>  	}
>  
>  	input_mt_sync_frame(bt541->input_dev);
> @@ -627,6 +671,7 @@
>  		return -ENOMEM;
>  
>  	bt541->client = client;
> +	bt541->chip = device_get_match_data(&client->dev);
>  	i2c_set_clientdata(client, bt541);
>  
>  	error = zinitix_init_regulators(bt541);
> @@ -741,7 +786,7 @@
>  	{ .compatible = "zinitix,bt431" },
>  	{ .compatible = "zinitix,bt432" },
>  	{ .compatible = "zinitix,bt531" },
> -	{ .compatible = "zinitix,bt532" },
> +	{ .compatible = "zinitix,bt532", .data = &zinitix_bt532_data },
>  	{ .compatible = "zinitix,bt538" },
>  	{ .compatible = "zinitix,bt541" },
>  	{ .compatible = "zinitix,bt548" },

^ permalink raw reply

* RE: [PATCH v3 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA
From: Xu, Even @ 2026-07-13  3:25 UTC (permalink / raw)
  To: sashiko-reviews@lists.linux.dev
  Cc: linux-input@vger.kernel.org, dmitry.torokhov@gmail.com
In-Reply-To: <20260713031749.2DC211F000E9@smtp.kernel.org>



> -----Original Message-----
> From: sashiko-bot@kernel.org <sashiko-bot@kernel.org>
> Sent: Monday, July 13, 2026 11:18 AM
> To: Xu, Even <even.xu@intel.com>
> Cc: linux-input@vger.kernel.org; dmitry.torokhov@gmail.com
> Subject: Re: [PATCH v3 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read
> DMA
> 
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to
> consider:
> - [Critical] `thc_rxdma_reset` improperly clears descriptor addresses for all DMA
> channels without pausing them first, potentially causing active TX/SW DMA to
> access physical address 0.
> - [High] Error paths in `thc_rxdma_reset` fail to restore the interrupt quiesce
> state, permanently disabling device interrupts.
> --
> 
> --- Patch [1]: [PATCH v3 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read
> DMA --- commit 64b22c0d882751a67b709351d2c2e3daf211bb52
> Author: Even Xu <even.xu@intel.com>
> 
> HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA
> 
> This commit adds a new helper function thc_rxdma_reset() to perform a read
> DMA reset. It is intended to be called for RxDMA recovery when fatal DMA errors
> occur.
> 
> > diff --git a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
> > b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
> > index 6ee675e0a7384..7ceb8aeeccd37 100644
> > --- a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
> > +++ b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
> [ ... ]
> > +int thc_rxdma_reset(struct thc_device *dev) {
> > +	int ret;
> > +
> > +	if (mutex_lock_interruptible(&dev->thc_bus_lock))
> > +		return -EINTR;
> > +
> > +	ret = thc_interrupt_quiesce(dev, true);
> > +	if (ret) {
> > +		dev_err(dev->dev, "Quiesce interrupt failed during RxDMA
> reset\n");
> > +		goto end;
> > +	}
> > +
> > +	ret = thc_wait_for_dma_pause(dev, THC_RXDMA1);
> > +	if (ret) {
> > +		dev_err(dev->dev, "Wait for RxDMA1 pause failed during RxDMA
> reset\n");
> > +		goto end;
> 
> [Severity: High]
> Does this skip unquiescing the interrupts?  If thc_wait_for_dma_pause() fails, we
> jump to the end label and return without calling thc_interrupt_quiesce(dev, false).
> Will this leave device interrupts permanently disabled?
> 
> > +	}
> > +
> > +	ret = thc_wait_for_dma_pause(dev, THC_RXDMA2);
> > +	if (ret) {
> > +		dev_err(dev->dev, "Wait for RxDMA2 pause failed during RxDMA
> reset\n");
> > +		goto end;
> > +	}
> > +
> > +	thc_dma_unconfigure(dev);
> 

This is expected, thc_rxdma_reset() is designed only be called in fatal DMA error. In this case, if pause RxDMA failed, it means DMA engine cannot be recovered.

> [Severity: Critical]
> Can this cause active TX or SW DMA engines to access physical address 0?
> 
> thc_rxdma_reset() pauses THC_RXDMA1 and THC_RXDMA2, but
> thc_dma_unconfigure() unconditionally iterates over MAX_THC_DMA_CHANNEL
> and sets the PRD base address to 0 for all channels, including THC_TXDMA and
> THC_SWDMA:
> 
> void thc_dma_unconfigure(struct thc_device *dev) {
> 	int chan;
> 
> 	for (chan = 0; chan < MAX_THC_DMA_CHANNEL; chan++) {
> 		dma_set_prd_base_addr(dev, 0, &dev->dma_ctx-
> >dma_config[chan]);
> 		dma_clear_prd_control(dev, &dev->dma_ctx-
> >dma_config[chan]);
> 	}
>         ...
> }
> 
> If a previous thc_dma_write() or thc_swdma_read() timed out, it unlocks
> thc_bus_lock but leaves the TX/SW DMA engine running in hardware. Overwriting
> the PRD base address to 0 while the engine is active could result in memory
> corruption or an IOMMU fault.

This is not a problem, All DMA operations were protected by THC bus mutex lock.
There is no Tx/SW DMA operation when calls into this thc_rxdma_reset().

> 
> > +
> > +	ret = thc_dma_configure(dev);
> > +	if (ret) {
> > +		dev_err(dev->dev, "Re-config DMA failed during RxDMA
> reset\n");
> > +		goto end;
> > +	}
> > +
> > +	thc_interrupt_quiesce(dev, false);
> > +
> > +end:
> > +	mutex_unlock(&dev->thc_bus_lock);
> > +	return ret;
> > +}
> 
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260713030412.2531933-1-
> even.xu@intel.com?part=1

^ permalink raw reply

* Re: [PATCH v3 3/3] HID: Intel-thc-hid: Intel-quickspi: Refine recover callback
From: sashiko-bot @ 2026-07-13  3:17 UTC (permalink / raw)
  To: Even Xu; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260713030412.2531933-4-even.xu@intel.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Teardown race in `quickspi_remove()` and `quickspi_shutdown()` against `try_recover()` allows hardware interrupts to be incorrectly re-enabled or work to be scheduled after cancellation, leading to Use-After-Free.
- [High] Missing `cancel_work_sync` in the probe error path leads to a Use-After-Free if recovery work is scheduled during a failed probe.
- [High] The recovery work is not cancelled during system suspend, potentially corrupting hardware state.
--

--- Patch [3]: [PATCH v3 3/3] HID: Intel-thc-hid: Intel-quickspi: Refine recover callback ---
commit 944320624d1a0211e0b09ee8578713e28378329b
Author: Even Xu <even.xu@intel.com>

HID: Intel-thc-hid: Intel-quickspi: Refine recover callback

This commit refines the quickspi device recovery flow. It moves the
recovery process from the IRQ handler to a workqueue, uses the
thc_rxdma_reset() API to simplify operations, and manages interrupt
states throughout the recovery sequence.

> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> index 4ae2e1718b30c..83ed5516b8097 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c

[ ... ]

> @@ -385,6 +378,7 @@ static struct quickspi_device *quickspi_dev_init(struct pci_dev *pdev, void __io
>  	init_waitqueue_head(&qsdev->report_desc_got_wq);
>  	init_waitqueue_head(&qsdev->get_report_cmpl_wq);
>  	init_waitqueue_head(&qsdev->set_report_cmpl_wq);
> +	INIT_WORK(&qsdev->recover_work, try_recover);

[Severity: High]
Since recover_work is being initialized here, what happens if
quickspi_probe() fails after the IRQ is registered?

If a hardware error interrupt occurs and schedules recover_work, and a
subsequent step in the probe process fails, the error path jumps to
the dev_deinit label which cleans up the device but never calls
cancel_work_sync().

Could this result in the devres framework freeing the IRQ and qsdev
while the recovery work is still pending, leading to a use-after-free
crash when the work runs?

[ ... ]

> @@ -711,6 +705,12 @@ static void quickspi_remove(struct pci_dev *pdev)
>  		return;
>  
>  	quickspi_hid_remove(qsdev);
> +
> +	thc_interrupt_quiesce(qsdev->thc_hw, true);
> +	thc_interrupt_enable(qsdev->thc_hw, false);
> +
> +	cancel_work_sync(&qsdev->recover_work);
> +

[Severity: High]
Is there a race condition here against the threaded IRQ handler during
device teardown?

Since qsdev->state is not set to QUICKSPI_DISABLED and the OS-level
IRQ is not disabled before cancel_work_sync() is called, could a
concurrent execution of quickspi_irq_thread_handler() schedule the
work again immediately after cancel_work_sync() returns?

Also, if try_recover() is running concurrently during this teardown, it
unconditionally calls thc_interrupt_enable(qsdev->thc_hw, true) on exit,
which could re-enable hardware interrupts after they were just disabled
here. Could this lead to a use-after-free when the devres framework
later frees the qsdev structure?

[Severity: High]
Does the quickspi_suspend() callback need to cancel this work as well?

Currently, quickspi_suspend() transitions the device to a suspended
state but lacks a call to cancel_work_sync(&qsdev->recover_work).
Because schedule_work() queues onto the non-freezable system_wq, the
try_recover() work could execute concurrently with or after the
suspend process.

Since try_recover() calls pm_runtime_resume_and_get(), resets the
RxDMA, and re-enables hardware interrupts, could this leave the device
in an inconsistent active state across system suspend, potentially
causing resume failures or power leaks?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713030412.2531933-1-even.xu@intel.com?part=3

^ permalink raw reply

* Re: [PATCH v3 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA
From: sashiko-bot @ 2026-07-13  3:17 UTC (permalink / raw)
  To: Even Xu; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260713030412.2531933-2-even.xu@intel.com>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] `thc_rxdma_reset` improperly clears descriptor addresses for all DMA channels without pausing them first, potentially causing active TX/SW DMA to access physical address 0.
- [High] Error paths in `thc_rxdma_reset` fail to restore the interrupt quiesce state, permanently disabling device interrupts.
--

--- Patch [1]: [PATCH v3 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA ---
commit 64b22c0d882751a67b709351d2c2e3daf211bb52
Author: Even Xu <even.xu@intel.com>

HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA

This commit adds a new helper function thc_rxdma_reset() to perform a read DMA
reset. It is intended to be called for RxDMA recovery when fatal DMA errors
occur.

> diff --git a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
> index 6ee675e0a7384..7ceb8aeeccd37 100644
> --- a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
> +++ b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
[ ... ]
> +int thc_rxdma_reset(struct thc_device *dev)
> +{
> +	int ret;
> +
> +	if (mutex_lock_interruptible(&dev->thc_bus_lock))
> +		return -EINTR;
> +
> +	ret = thc_interrupt_quiesce(dev, true);
> +	if (ret) {
> +		dev_err(dev->dev, "Quiesce interrupt failed during RxDMA reset\n");
> +		goto end;
> +	}
> +
> +	ret = thc_wait_for_dma_pause(dev, THC_RXDMA1);
> +	if (ret) {
> +		dev_err(dev->dev, "Wait for RxDMA1 pause failed during RxDMA reset\n");
> +		goto end;

[Severity: High]
Does this skip unquiescing the interrupts?  If thc_wait_for_dma_pause() fails,
we jump to the end label and return without calling 
thc_interrupt_quiesce(dev, false).  Will this leave device interrupts 
permanently disabled?

> +	}
> +
> +	ret = thc_wait_for_dma_pause(dev, THC_RXDMA2);
> +	if (ret) {
> +		dev_err(dev->dev, "Wait for RxDMA2 pause failed during RxDMA reset\n");
> +		goto end;
> +	}
> +
> +	thc_dma_unconfigure(dev);

[Severity: Critical]
Can this cause active TX or SW DMA engines to access physical address 0?

thc_rxdma_reset() pauses THC_RXDMA1 and THC_RXDMA2, but thc_dma_unconfigure() 
unconditionally iterates over MAX_THC_DMA_CHANNEL and sets the PRD base
address to 0 for all channels, including THC_TXDMA and THC_SWDMA:

void thc_dma_unconfigure(struct thc_device *dev)
{
	int chan;

	for (chan = 0; chan < MAX_THC_DMA_CHANNEL; chan++) {
		dma_set_prd_base_addr(dev, 0, &dev->dma_ctx->dma_config[chan]);
		dma_clear_prd_control(dev, &dev->dma_ctx->dma_config[chan]);
	}
        ...
}

If a previous thc_dma_write() or thc_swdma_read() timed out, it unlocks 
thc_bus_lock but leaves the TX/SW DMA engine running in hardware. Overwriting 
the PRD base address to 0 while the engine is active could result in memory 
corruption or an IOMMU fault.

> +
> +	ret = thc_dma_configure(dev);
> +	if (ret) {
> +		dev_err(dev->dev, "Re-config DMA failed during RxDMA reset\n");
> +		goto end;
> +	}
> +
> +	thc_interrupt_quiesce(dev, false);
> +
> +end:
> +	mutex_unlock(&dev->thc_bus_lock);
> +	return ret;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713030412.2531933-1-even.xu@intel.com?part=1

^ permalink raw reply

* Re: [PATCH v3 2/3] HID: Intel-thc-hid: Intel-quicki2c: Refine recover callback
From: sashiko-bot @ 2026-07-13  3:16 UTC (permalink / raw)
  To: Even Xu; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260713030412.2531933-3-even.xu@intel.com>

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The recovery workqueue can be scheduled by the threaded IRQ handler after `cancel_work_sync()` returns, leading to a Use-After-Free during device removal or shutdown.
- [High] Missing cancellation of `recover_work` in the probe error path leads to a Use-After-Free if initialization fails.
- [High] Missing cancellation of `recover_work` during system suspend leaves the hardware in a corrupted state.
--

--- Patch [2]: [PATCH v3 2/3] HID: Intel-thc-hid: Intel-quicki2c: Refine recover callback ---
commit 81b9a7d4f94249cad95844adc988f68429457956
Author: Even Xu <even.xu@intel.com>

HID: Intel-thc-hid: Intel-quicki2c: Refine recover callback

This commit modifies the recovery flow for Intel-quicki2c by using a
workqueue to process device resets instead of handling them directly
within the IRQ handler. It introduces the recover_work work_struct and
manages hardware interrupt states during the recovery process.

> diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> --- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
[ ... ]
> @@ -386,6 +385,7 @@ static struct quicki2c_device *quicki2c_dev_init(struct pci_dev *pdev, void __io
>  	qcdev->ddata = ddata;
>  
>  	init_waitqueue_head(&qcdev->reset_ack_wq);
> +	INIT_WORK(&qcdev->recover_work, try_recover);
>  
>  	/* THC hardware init */

[Severity: High]
Does this work need to be cancelled during the probe error paths and
system suspend transitions?

If a failure occurs during quicki2c_probe() after the hardware interrupts are
enabled, the devres framework will free qcdev. If an interrupt fires and
schedules this work before the error handler returns, could the background
workqueue dereference the freed qcdev?

Similarly, quicki2c_suspend() and quicki2c_freeze() do not appear to cancel
this work. If an interrupt schedules recover_work just before the system
suspends, could try_recover() run concurrently and re-enable hardware
interrupts, overriding the suspended hardware state?

[ ... ]
> @@ -772,6 +772,12 @@ static void quicki2c_remove(struct pci_dev *pdev)
>  		return;
>  
>  	quicki2c_hid_remove(qcdev);
> +
> +	thc_interrupt_quiesce(qcdev->thc_hw, true);
> +	thc_interrupt_enable(qcdev->thc_hw, false);
> +
> +	cancel_work_sync(&qcdev->recover_work);
> +
>  	quicki2c_dma_deinit(qcdev);

[Severity: High]
Can the threaded IRQ handler race with this removal and re-queue the work?

The IRQ handler is managed by devm_request_threaded_irq() and remains active
until devres cleanup occurs after quicki2c_remove() returns.

If an interrupt fires right as cancel_work_sync() completes, couldn't the
IRQ thread call schedule_work() again?

If try_recover() then executes concurrently, it overrides the hardware
teardown by calling thc_interrupt_enable(..., true). When devres subsequently
frees the IRQ and qcdev, will this result in a use-after-free when the
re-queued work executes?

[ ... ]
> @@ -796,6 +802,11 @@ static void quicki2c_shutdown(struct pci_dev *pdev)
>  	if (!qcdev)
>  		return;
>  
> +	thc_interrupt_quiesce(qcdev->thc_hw, true);
> +	thc_interrupt_enable(qcdev->thc_hw, false);
> +
> +	cancel_work_sync(&qcdev->recover_work);
> +
>  	/* Must stop DMA before reboot to avoid DMA entering into unknown state */

[Severity: High]
Does this cancellation share the same race condition with the threaded IRQ
handler as quicki2c_remove()?

Without disabling the Linux IRQ line before cancelling the work, could a
concurrent hardware interrupt allow quicki2c_irq_thread_handler() to
re-queue recover_work during system shutdown?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260713030412.2531933-1-even.xu@intel.com?part=2

^ permalink raw reply

* [PATCH v3 3/3] HID: Intel-thc-hid: Intel-quickspi: Refine recover callback
From: Even Xu @ 2026-07-13  3:04 UTC (permalink / raw)
  To: bentiss, jikos; +Cc: srinivas.pandruvada, linux-input, linux-kernel, Even Xu
In-Reply-To: <20260713030412.2531933-1-even.xu@intel.com>

Refine recover flow:
1. Use workqueue to handle recover flow instead of processing in irq
   handler.
2. Call thc_rxdma_reset() API to simplify the recover operation.
3. Disable interrupt during whole recover flow.
4. If recover fails, disable interrupt to avoid interrupt storm.

Signed-off-by: Even Xu <even.xu@intel.com>
---
 .../intel-quickspi/pci-quickspi.c             | 53 ++++++++++---------
 .../intel-quickspi/quickspi-dev.h             |  3 ++
 2 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
index f669235f1883..3690ddd061e1 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
@@ -252,34 +252,28 @@ static irqreturn_t quickspi_irq_quick_handler(int irq, void *dev_id)
 }
 
 /**
- * try_recover - Try to recovery THC and Device
- * @qsdev: pointer to quickspi device
+ * try_recover - Recover callback to recover THC
+ * @work: pointer to work_struct
  *
- * This function is a error handler, called when fatal error happens.
- * It try to reset Touch Device and re-configure THC to recovery
+ * This function is an error handler, called when fatal error happens.
+ * It try to reset Touch Device and re-configure THC to recover
  * transferring between Device and THC.
- *
- * Return: 0 if successful or error code on failed.
  */
-static int try_recover(struct quickspi_device *qsdev)
+static void try_recover(struct work_struct *work)
 {
-	int ret;
+	struct quickspi_device *qsdev = container_of(work, struct quickspi_device, recover_work);
 
-	ret = reset_tic(qsdev);
-	if (ret) {
-		dev_err(qsdev->dev, "Reset touch device failed, ret = %d\n", ret);
-		return ret;
-	}
+	if (pm_runtime_resume_and_get(qsdev->dev))
+		return;
 
-	thc_dma_unconfigure(qsdev->thc_hw);
+	thc_interrupt_enable(qsdev->thc_hw, false);
 
-	ret = thc_dma_configure(qsdev->thc_hw);
-	if (ret) {
-		dev_err(qsdev->dev, "Re-configure THC DMA failed, ret = %d\n", ret);
-		return ret;
-	}
+	if (thc_rxdma_reset(qsdev->thc_hw))
+		qsdev->state = QUICKSPI_DISABLED;
+	else
+		thc_interrupt_enable(qsdev->thc_hw, true);
 
-	return 0;
+	pm_runtime_put_autosuspend(qsdev->dev);
 }
 
 /**
@@ -337,11 +331,10 @@ static irqreturn_t quickspi_irq_thread_handler(int irq, void *dev_id)
 	}
 
 end:
-	thc_interrupt_enable(qsdev->thc_hw, true);
-
 	if (err_recover)
-		if (try_recover(qsdev))
-			qsdev->state = QUICKSPI_DISABLED;
+		schedule_work(&qsdev->recover_work);
+	else
+		thc_interrupt_enable(qsdev->thc_hw, true);
 
 	pm_runtime_put_autosuspend(qsdev->dev);
 
@@ -385,6 +378,7 @@ static struct quickspi_device *quickspi_dev_init(struct pci_dev *pdev, void __io
 	init_waitqueue_head(&qsdev->report_desc_got_wq);
 	init_waitqueue_head(&qsdev->get_report_cmpl_wq);
 	init_waitqueue_head(&qsdev->set_report_cmpl_wq);
+	INIT_WORK(&qsdev->recover_work, try_recover);
 
 	/* thc hw init */
 	qsdev->thc_hw = thc_dev_init(qsdev->dev, qsdev->mem_addr);
@@ -711,6 +705,12 @@ static void quickspi_remove(struct pci_dev *pdev)
 		return;
 
 	quickspi_hid_remove(qsdev);
+
+	thc_interrupt_quiesce(qsdev->thc_hw, true);
+	thc_interrupt_enable(qsdev->thc_hw, false);
+
+	cancel_work_sync(&qsdev->recover_work);
+
 	quickspi_dma_deinit(qsdev);
 
 	pm_runtime_get_noresume(qsdev->dev);
@@ -737,6 +737,11 @@ static void quickspi_shutdown(struct pci_dev *pdev)
 	if (!qsdev)
 		return;
 
+	thc_interrupt_quiesce(qsdev->thc_hw, true);
+	thc_interrupt_enable(qsdev->thc_hw, false);
+
+	cancel_work_sync(&qsdev->recover_work);
+
 	/* Must stop DMA before reboot to avoid DMA entering into unknown state */
 	quickspi_dma_deinit(qsdev);
 
diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
index bf5e18f5a5f4..363e589b0bde 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
@@ -8,6 +8,7 @@
 #include <linux/hid-over-spi.h>
 #include <linux/sizes.h>
 #include <linux/wait.h>
+#include <linux/workqueue.h>
 
 #include "quickspi-protocol.h"
 
@@ -173,6 +174,8 @@ struct quickspi_device {
 
 	wait_queue_head_t set_report_cmpl_wq;
 	bool set_report_cmpl;
+
+	struct work_struct recover_work;
 };
 
 #endif /* _QUICKSPI_DEV_H_ */
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 2/3] HID: Intel-thc-hid: Intel-quicki2c: Refine recover callback
From: Even Xu @ 2026-07-13  3:04 UTC (permalink / raw)
  To: bentiss, jikos; +Cc: srinivas.pandruvada, linux-input, linux-kernel, Even Xu
In-Reply-To: <20260713030412.2531933-1-even.xu@intel.com>

Refine recover flow:
1. Use workqueue to handle recover flow instead of processing in irq
   handler.
2. Call thc_rxdma_reset() API to simplify the recover operation.
3. Disable interrupt during whole recover flow.
4. If recover fails, disable interrupt to avoid interrupt storm.

Signed-off-by: Even Xu <even.xu@intel.com>
---
 .../intel-quicki2c/pci-quicki2c.c             | 49 ++++++++++++-------
 .../intel-quicki2c/quicki2c-dev.h             |  2 +
 2 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
index 46d3e9a01999..2b586d0635b6 100644
--- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
+++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
@@ -245,28 +245,28 @@ static irqreturn_t quicki2c_irq_quick_handler(int irq, void *dev_id)
 }
 
 /**
- * try_recover - Try to recovery THC and Device
- * @qcdev: Pointer to quicki2c_device structure
+ * try_recover - Recover callback to recover THC
+ * @work: pointer to work_struct
  *
  * This function is an error handler, called when fatal error happens.
- * It try to reset touch device and re-configure THC to recovery
- * communication between touch device and THC.
- *
- * Return: 0 if successful or error code on failure
+ * It try to reset Touch Device and re-configure THC to recover
+ * transferring between Device and THC.
  */
-static int try_recover(struct quicki2c_device *qcdev)
+static void try_recover(struct work_struct *work)
 {
-	int ret;
+	struct quicki2c_device *qcdev = container_of(work, struct quicki2c_device, recover_work);
 
-	thc_dma_unconfigure(qcdev->thc_hw);
+	if (pm_runtime_resume_and_get(qcdev->dev))
+		return;
 
-	ret = thc_dma_configure(qcdev->thc_hw);
-	if (ret) {
-		dev_err(qcdev->dev, "Reconfig DMA failed\n");
-		return ret;
-	}
+	thc_interrupt_enable(qcdev->thc_hw, false);
 
-	return 0;
+	if (thc_rxdma_reset(qcdev->thc_hw))
+		qcdev->state = QUICKI2C_DISABLED;
+	else
+		thc_interrupt_enable(qcdev->thc_hw, true);
+
+	pm_runtime_put_autosuspend(qcdev->dev);
 }
 
 static int handle_input_report(struct quicki2c_device *qcdev)
@@ -343,11 +343,10 @@ static irqreturn_t quicki2c_irq_thread_handler(int irq, void *dev_id)
 	}
 
 exit:
-	thc_interrupt_enable(qcdev->thc_hw, true);
-
 	if (err_recover)
-		if (try_recover(qcdev))
-			qcdev->state = QUICKI2C_DISABLED;
+		schedule_work(&qcdev->recover_work);
+	else
+		thc_interrupt_enable(qcdev->thc_hw, true);
 
 	pm_runtime_put_autosuspend(qcdev->dev);
 
@@ -386,6 +385,7 @@ static struct quicki2c_device *quicki2c_dev_init(struct pci_dev *pdev, void __io
 	qcdev->ddata = ddata;
 
 	init_waitqueue_head(&qcdev->reset_ack_wq);
+	INIT_WORK(&qcdev->recover_work, try_recover);
 
 	/* THC hardware init */
 	qcdev->thc_hw = thc_dev_init(qcdev->dev, qcdev->mem_addr);
@@ -772,6 +772,12 @@ static void quicki2c_remove(struct pci_dev *pdev)
 		return;
 
 	quicki2c_hid_remove(qcdev);
+
+	thc_interrupt_quiesce(qcdev->thc_hw, true);
+	thc_interrupt_enable(qcdev->thc_hw, false);
+
+	cancel_work_sync(&qcdev->recover_work);
+
 	quicki2c_dma_deinit(qcdev);
 
 	pm_runtime_get_noresume(qcdev->dev);
@@ -796,6 +802,11 @@ static void quicki2c_shutdown(struct pci_dev *pdev)
 	if (!qcdev)
 		return;
 
+	thc_interrupt_quiesce(qcdev->thc_hw, true);
+	thc_interrupt_enable(qcdev->thc_hw, false);
+
+	cancel_work_sync(&qcdev->recover_work);
+
 	/* Must stop DMA before reboot to avoid DMA entering into unknown state */
 	quicki2c_dma_deinit(qcdev);
 
diff --git a/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h b/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
index 61dbdece59a1..aedf85291e60 100644
--- a/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
+++ b/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
@@ -222,6 +222,8 @@ struct quicki2c_device {
 	wait_queue_head_t reset_ack_wq;
 	bool reset_ack;
 
+	struct work_struct recover_work;
+
 	u32 i2c_max_frame_size_enable;
 	u32 i2c_max_frame_size;
 	u32 i2c_int_delay_enable;
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA
From: Even Xu @ 2026-07-13  3:04 UTC (permalink / raw)
  To: bentiss, jikos; +Cc: srinivas.pandruvada, linux-input, linux-kernel, Even Xu
In-Reply-To: <20260713030412.2531933-1-even.xu@intel.com>

Add a helper function thc_rxdma_reset() to do read DMA reset, it can be
called when fatal DMA error happens.

Signed-off-by: Even Xu <even.xu@intel.com>
---
 .../intel-thc-hid/intel-thc/intel-thc-dma.c   | 51 +++++++++++++++++++
 .../intel-thc-hid/intel-thc/intel-thc-dma.h   |  1 +
 2 files changed, 52 insertions(+)

diff --git a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
index 6ee675e0a738..7ceb8aeeccd3 100644
--- a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
+++ b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.c
@@ -561,6 +561,57 @@ static int thc_wait_for_dma_pause(struct thc_device *dev, enum thc_dma_channel c
 	return 0;
 }
 
+/**
+ * thc_rxdma_reset - Reset all read DMA engines
+ *
+ * @dev: The pointer of THC private device context
+ *
+ * This is a helper function to reset RxDMA configure. It's typically used
+ * for RxDMA recovery when fatal error happens.
+ *
+ * Return: 0 if successful or error code on failure.
+ */
+int thc_rxdma_reset(struct thc_device *dev)
+{
+	int ret;
+
+	if (mutex_lock_interruptible(&dev->thc_bus_lock))
+		return -EINTR;
+
+	ret = thc_interrupt_quiesce(dev, true);
+	if (ret) {
+		dev_err(dev->dev, "Quiesce interrupt failed during RxDMA reset\n");
+		goto end;
+	}
+
+	ret = thc_wait_for_dma_pause(dev, THC_RXDMA1);
+	if (ret) {
+		dev_err(dev->dev, "Wait for RxDMA1 pause failed during RxDMA reset\n");
+		goto end;
+	}
+
+	ret = thc_wait_for_dma_pause(dev, THC_RXDMA2);
+	if (ret) {
+		dev_err(dev->dev, "Wait for RxDMA2 pause failed during RxDMA reset\n");
+		goto end;
+	}
+
+	thc_dma_unconfigure(dev);
+
+	ret = thc_dma_configure(dev);
+	if (ret) {
+		dev_err(dev->dev, "Re-config DMA failed during RxDMA reset\n");
+		goto end;
+	}
+
+	thc_interrupt_quiesce(dev, false);
+
+end:
+	mutex_unlock(&dev->thc_bus_lock);
+	return ret;
+}
+EXPORT_SYMBOL_NS_GPL(thc_rxdma_reset, "INTEL_THC");
+
 static int read_dma_buffer(struct thc_device *dev,
 			   struct thc_dma_configuration *read_config,
 			   u8 prd_table_index, void *read_buff)
diff --git a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.h b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.h
index 541d33995baf..715423453a9d 100644
--- a/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.h
+++ b/drivers/hid/intel-thc-hid/intel-thc/intel-thc-dma.h
@@ -145,6 +145,7 @@ int  thc_dma_allocate(struct thc_device *dev);
 int  thc_dma_configure(struct thc_device *dev);
 void thc_dma_unconfigure(struct thc_device *dev);
 void thc_dma_release(struct thc_device *dev);
+int  thc_rxdma_reset(struct thc_device *dev);
 int  thc_rxdma_read(struct thc_device *dev, enum thc_dma_channel dma_channel,
 		    void *read_buff, size_t *read_len, int *read_finished);
 int  thc_swdma_read(struct thc_device *dev, void *write_buff, size_t write_len,
-- 
2.43.0


^ permalink raw reply related

* [PATCH v3 0/3] HID: Intel-thc-hid: Refine error recovery flow
From: Even Xu @ 2026-07-13  3:04 UTC (permalink / raw)
  To: bentiss, jikos; +Cc: srinivas.pandruvada, linux-input, linux-kernel, Even Xu

This series refines the fatal error recovery flow for the Intel THC
(Touch Host Controller) subsystem, covering both the QuickI2C and
QuickSPI drivers.

Currently, when a fatal DMA error is detected in the IRQ thread handler,
the recovery is performed inline: the interrupt handler calls
try_recover() directly, which unconfigures and reconfigures the DMA
engine.

This approach has several problems:
1. Recovery runs in the IRQ thread context, which is not ideal for
   potentially slow reset operations.
2. The interrupt is re-enabled before recovery completes, risking an
   interrupt storm if DMA errors persist.
3. The DMA reset logic is open-coded in each protocol driver, leading
   to duplication and divergence over time.

This patch series addresses all of the above:

By adding a new thc_rxdma_reset() API to the THC core layer, QuickI2C
and QuickSPI drivers can call it respectively to refine the recovery
callback.

The synchronous try_recover() call in the IRQ thread is replaced with
schedule_work(), deferring recovery to a workqueue.  Within the work
function:
- The interrupt line is disabled before any DMA manipulation.
- thc_rxdma_reset() is used instead of the open-coded sequence.
- On failure the device is marked DISABLED and the interrupt remains
  off, preventing an interrupt storm.

Change log:
v3:
 - Quiesce external interrupt and disable THC internal interrupt in
   remove() and shutdown() callback before cancel_work_sync().

v2:
 - Use dev_err() instead of dev_err_once() so repeated failures during
   recurring recovery are not silently suppressed.
 - Pause both RxDMA channels via thc_wait_for_dma_pause() before calling
   thc_dma_unconfigure() to ensure the DMA engines are inactive before
   clearing PRD base addresses, preventing potential IOMMU faults or
   memory corruption.
 - Hold a runtime PM reference inside try_recover() to prevent the
   device from suspending while the work accesses hardware registers.
 - Add cancel_work_sync() in quicki2c_remove() and quicki2c_shutdown()
   to prevent use-after-free if recovery work is still queued at teardown.
 - Only re-enable the interrupt in the IRQ thread handler when no recovery
   is needed; the work function handles re-enabling after successful reset,
   avoiding an interrupt storm from the uncleared hardware error state.



Even Xu (3):
  HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA
  HID: Intel-thc-hid: Intel-quicki2c: Refine recover callback
  HID: Intel-thc-hid: Intel-quickspi: Refine recover callback

 .../intel-quicki2c/pci-quicki2c.c             | 49 ++++++++++-------
 .../intel-quicki2c/quicki2c-dev.h             |  2 +
 .../intel-quickspi/pci-quickspi.c             | 53 ++++++++++---------
 .../intel-quickspi/quickspi-dev.h             |  3 ++
 .../intel-thc-hid/intel-thc/intel-thc-dma.c   | 51 ++++++++++++++++++
 .../intel-thc-hid/intel-thc/intel-thc-dma.h   |  1 +
 6 files changed, 116 insertions(+), 43 deletions(-)

-- 
2.43.0


^ permalink raw reply

* Re: [PATCH 1/1] HID: logitech: add Bolt receiver support for Logitech HID++ devices
From: Kateřina Medvědová @ 2026-07-12 18:03 UTC (permalink / raw)
  To: erikhakan; +Cc: bentiss, hadess, jikos, lains, linux-input, linux-kernel
In-Reply-To: <20260712003051.338194-2-erikhakan@gmail.com>

Hello Erik,
I tested your patch on my hardware and found two issues:

1. Both keyboard and mouse inputs stop working correctly. The mouse 
becomes very sensitive and creates events that shouldn't happen 
(sometimes scrolls on its own) and the keyboard's keys are scrambled. I 
verified on my hardware that if you pass the first two of Bolt's 
interfaces (0 boot keyboard and 1 boot mouse) to the generic HID driver 
and only handle interface 2 in the DJ driver, you can keep the extra 
functionality (like battery reporting) without affecting inputs. The 
existing bInterfaceNumber >= no_dj_interfaces check never triggers for 
Bolt. no_dj_interfaces = 3, but the receiver's highest interface number 
is 2, so interfaces 0 and 1 fall through to the DJ path. Here's what 
fixed it:


if (id->driver_data == recvr_type_bolt && intf &&
     intf->altsetting->desc.bInterfaceNumber !=
				LOGITECH_DJ_INTERFACE_NUMBER) {
	hdev->quirks |= HID_QUIRK_INPUT_PER_APP;
	return hid_hw_start(hdev, HID_CONNECT_DEFAULT);
}


2. Bolt devices stay behind after unpairing in userspace (using Solaar). 
The existing driver code doesn't handle Bolt's unpair events so devices 
stay visible after unpairing. Here's how I handled it:

if (djrcv_dev->type == recvr_type_bolt &&
     hidpp_report->report_id == REPORT_ID_HIDPP_SHORT &&
     hidpp_report->sub_id == REPORT_TYPE_NOTIF_DEVICE_UNPAIRED) {
	struct dj_workitem workitem = {
		.device_index = device_index,
		.type = WORKITEM_TYPE_UNPAIRED,
	};

	kfifo_in(&djrcv_dev->notif_fifo, &workitem, sizeof(workitem));
	schedule_work(&djrcv_dev->work);
	spin_unlock_irqrestore(&djrcv_dev->lock, flags);
	return false;
}

Hardware tested:
- Logi Bolt receiver (046d:c548)
- Logi POP Icon Keys
- Logi Lift Vertical
- Logi K650

Happy to do more testing.

Regards,
Kateřina Medvědová

^ permalink raw reply

* Re: [PATCH 1/2] Input: applespi - register touchpad device synchronously in probe
From: sashiko-bot @ 2026-07-12 16:30 UTC (permalink / raw)
  To: Shih-Yuan Lee; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260712161527.7350-2-fourdollars@debian.org>

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] Re-entrancy in async SPI completion leads to callback slot overwrite and NULL pointer dereference.
- [High] Missing timeouts in probe error path and remove path cause indefinite system hang if the SPI controller wedges.
- [High] Synchronous touchpad detection failure forces a complete driver teardown, disabling the functional keyboard.
- [Medium] Lockless copy and evaluation of touchpad info struct causes a data race.
- [Medium] Timeout condition fails if the touchpad's model number is 0.
- [Low] Redundant and contradictory debugfs NULL pointer check.
--

--- Patch [1]: [PATCH 1/2] Input: applespi - register touchpad device synchronously in probe ---
commit 4961cde9c37a41e959c60a2aa6d8bde99a5e96b5
Author: Shih-Yuan Lee <fourdollars@debian.org>

Input: applespi - register touchpad device synchronously in probe

This patch simplifies the applespi driver by making touchpad detection and
registration synchronous during probe rather than using an asynchronous worker.
It also introduces timeouts for wait events and changes the probe type to prefer
asynchronous execution by the driver core.

> diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
> index b5ff71cd5a701..c1065a6ba96c8 100644
> --- a/drivers/input/keyboard/applespi.c
> +++ b/drivers/input/keyboard/applespi.c
[ ... ]
> @@ -607,13 +612,61 @@ static void applespi_setup_write_txfrs(struct applespi_data *applespi)
>  	spi_message_add_tail(st_t, msg);
>  }
>  
> +static bool applespi_async_outstanding(struct applespi_data *applespi)
> +{
> +	return applespi->spi_complete[0].complete ||
> +	       applespi->spi_complete[1].complete;
> +}
> +
> +static void applespi_async_complete(void *context)
> +{
> +	struct applespi_complete_info *info = context;
> +	struct applespi_data *applespi = info->applespi;
> +	unsigned long flags;
> +
> +	info->complete(applespi);
> +
> +	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
> +
> +	info->complete = NULL;

[Severity: Critical]
Can this execution order cause a nested applespi_async() call to mistakenly
view the slot as occupied?

Because info->complete(applespi) executes before info->complete = NULL, if the
callback queues a new SPI message, it might see this slot as still in use.

> +
> +	if (applespi->cancel_spi && !applespi_async_outstanding(applespi))
> +		wake_up_all(&applespi->wait_queue);
> +
> +	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
> +}
> +
>  static int applespi_async(struct applespi_data *applespi,
>  			  struct spi_message *message, void (*complete)(void *))
>  {
> -	message->complete = complete;
> -	message->context = applespi;
> +	struct applespi_complete_info *info;
> +	int sts;
>  
> -	return spi_async(applespi->spi, message);
> +	if (applespi->cancel_spi) {
> +		if (!applespi_async_outstanding(applespi))
> +			wake_up_all(&applespi->wait_queue);
> +		return -ESHUTDOWN;
> +	}
> +
> +	/*
> +	 * There can only be at most 2 spi requests in flight, one for "reads"
> +	 * and one for "writes".
> +	 */
> +	if (!applespi->spi_complete[0].complete)
> +		info = &applespi->spi_complete[0];
> +	else
> +		info = &applespi->spi_complete[1];
> +	info->complete = complete;

[Severity: Critical]
Does this blindly overwrite the second slot if the first is occupied, even if
the second slot is also in use?

If a re-entrant call happens before info->complete is cleared in
applespi_async_complete(), this might overwrite the callback of a concurrent
in-flight transfer. This could lead to a NULL pointer dereference or
use-after-free later.

> +	info->applespi = applespi;
> +
> +	message->complete = applespi_async_complete;
> +	message->context = info;
> +
> +	sts = spi_async(applespi->spi, message);
[ ... ]
> @@ -963,12 +1016,18 @@ static void applespi_debug_update_dimensions(struct applespi_data *applespi,
>  static int applespi_tp_dim_open(struct inode *inode, struct file *file)
>  {
>  	struct applespi_data *applespi = inode->i_private;
> +	struct input_dev *touchpad;
>  
>  	file->private_data = applespi;
>  
> +	/* Pairs with smp_store_release in applespi_register_touchpad_device() */
> +	touchpad = smp_load_acquire(&applespi->touchpad_input_dev);
> +	if (!touchpad)
> +		return -ENODEV;
> +

[Severity: Low]
Is this NULL pointer check redundant?

The commit message states that the patch "obsoletes the need for async work
cancellation on driver remove and debugfs NULL pointer checks." However, this
adds a new NULL pointer check.

Since the probe is now synchronous, the debugfs file isn't created until the
touchpad is fully registered, making it impossible for touchpad_input_dev to be
NULL here.

>  	snprintf(applespi->tp_dim_val, sizeof(applespi->tp_dim_val),
>  		 "0x%.4x %dx%d+%u+%u\n",
[ ... ]
> @@ -1324,26 +1383,14 @@ applespi_register_touchpad_device(struct applespi_data *applespi,
>  	return 0;
>  }
>  
> -static void applespi_worker(struct work_struct *work)
> -{
[ ... ]
> -}
> -
>  static void applespi_handle_cmd_response(struct applespi_data *applespi,
>  					 struct spi_packet *packet,
>  					 struct message *message)
>  {
>  	if (packet->device == PACKET_DEV_INFO &&
>  	    le16_to_cpu(message->type) == 0x1020) {
> -		/*
> -		 * We're not allowed to sleep here, but registering an input
> -		 * device can sleep.
> -		 */
>  		applespi->rcvd_tp_info = message->tp_info;
> -		schedule_work(&applespi->work);
> +		wake_up_all(&applespi->wait_queue);

[Severity: Medium]
Is there a data race here since the 112-byte message->tp_info struct is copied
locklessly before waking the wait queue?

>  		return;
>  	}
[ ... ]
> @@ -1757,6 +1803,22 @@ static int applespi_probe(struct spi_device *spi)
>  	/* trigger touchpad setup */
>  	applespi_init(applespi, false);
>  
> +	/* set up the touchpad as a separate input device */
> +	sts = wait_event_timeout(applespi->wait_queue,
> +				 applespi->rcvd_tp_info.model_no,
> +				 msecs_to_jiffies(3000));

[Severity: Medium]
Because applespi->rcvd_tp_info.model_no is evaluated locklessly without an
acquire memory barrier, could the waiting thread observe a non-zero model_no
and wake up before preceding fields are fully committed to memory?

[Severity: Medium]
What happens if a legitimate touchpad reports a model_no of 0?

Since applespi->rcvd_tp_info.model_no is used as a boolean flag, a value of 0
would cause the wait condition to fail and trigger the 3-second timeout, even
though the hardware responded successfully.

> +	if (!sts) {
> +		dev_err(&applespi->spi->dev,
> +			"Timed out waiting for device info\n");
> +		sts = -ETIMEDOUT;
> +		goto cancel_spi;
> +	}

[Severity: High]
Does a touchpad detection timeout here completely abort the driver probe and
disable a fully functional keyboard?

In the previous asynchronous design, a missing info packet safely skipped
touchpad registration while leaving the keyboard intact.

> +
> +	sts = applespi_register_touchpad_device(applespi,
> +						&applespi->rcvd_tp_info);
> +	if (sts)
> +		goto cancel_spi;
> +
>  	/*
>  	 * By default this device is not enabled for wakeup; but USB keyboards
>  	 * generally are, so the expectation is that by default the keyboard
> @@ -1789,25 +1851,50 @@ static int applespi_probe(struct spi_device *spi)
>  			    &applespi_tp_dim_fops);
>  
>  	return 0;
> +
> +cancel_spi:
> +	acpi_disable_gpe(NULL, applespi->gpe);
> +	acpi_remove_gpe_handler(NULL, applespi->gpe, applespi_notify);
> +
> +	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
> +	applespi->cancel_spi = true;
> +	wait_event_lock_irq(applespi->wait_queue,
> +			    !applespi_async_outstanding(applespi),
> +			    applespi->cmd_msg_lock);

[Severity: High]
Does this probe error path lack a timeout?

The commit message mentions adding 3-second timeouts to prevent suspend/remove
hangs, but if the SPI controller wedges, this wait_event_lock_irq() could hang
indefinitely. This same issue appears to be present in applespi_remove().

> +	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
> +
> +	return sts;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260712161527.7350-1-fourdollars@debian.org?part=1

^ permalink raw reply

* [PATCH 2/2] Input: applespi - fix use-after-free in applespi_remove()
From: Shih-Yuan Lee @ 2026-07-12 16:15 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260712161527.7350-1-fourdollars@debian.org>

applespi_remove() called applespi_drain_writes() to wait for in-flight
write transfers, then immediately called acpi_disable_gpe() and
acpi_remove_gpe_handler().  However it then called applespi_drain_reads()
*after* the GPE handler was removed, which races with any read SPI
completion callback that could still reference the applespi struct
already being torn down.

Moreover, the two drain helpers use separate wait paths that can miss
each other: a read completion arriving just after drain_writes() returns
but before drain_reads() is called will set read_active, and the
subsequent drain_reads() will then wait on a wait_queue that nobody will
ever wake because the GPE is already gone.

Fix by replacing the two separate drain calls with a single barrier
using the existing cancel_spi + wait_event_lock_irq mechanism:

  - Set cancel_spi = true under the spinlock so that applespi_async()
    immediately rejects new SPI submissions and wakes the wait queue
    once all outstanding operations have drained.
  - Wait for !applespi_async_outstanding() before proceeding with
    teardown.
  - Disable the GPE and remove its handler only after all in-flight SPI
    transfers have completed, eliminating the use-after-free window.

Fixes: 0b7a8ac72fc1 ("Input: applespi - add driver for Apple SPI keyboard and touchpad")
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
 drivers/input/keyboard/applespi.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index c1065a6ba96c..1f5cc2b4dc30 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -1900,15 +1900,21 @@ static void applespi_drain_reads(struct applespi_data *applespi)
 static void applespi_remove(struct spi_device *spi)
 {
 	struct applespi_data *applespi = spi_get_drvdata(spi);
+	unsigned long flags;
 
-	applespi_drain_writes(applespi);
+	/* Prevent any new SPI transfers and wait for outstanding ones */
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+	applespi->cancel_spi = true;
+	wait_event_lock_irq(applespi->wait_queue,
+			    !applespi_async_outstanding(applespi),
+			    applespi->cmd_msg_lock);
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
 
+	/* Disable GPE and remove handler */
 	acpi_disable_gpe(NULL, applespi->gpe);
 	acpi_remove_gpe_handler(NULL, applespi->gpe, applespi_notify);
 	device_wakeup_disable(&spi->dev);
 
-	applespi_drain_reads(applespi);
-
 	debugfs_remove_recursive(applespi->debugfs_root);
 }
 
-- 
2.39.5


^ permalink raw reply related

* [PATCH 1/2] Input: applespi - register touchpad device synchronously in probe
From: Shih-Yuan Lee @ 2026-07-12 16:15 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: linux-input, linux-kernel, Shih-Yuan Lee, Ronald Tschalär
In-Reply-To: <20260712161527.7350-1-fourdollars@debian.org>

Touchpad registration is currently done asynchronously from a worker
thread because it can sleep. This leads to high-severity unbind/remove
use-after-free vulnerabilities and debugfs race conditions (userspace
opening tp_dim debugfs file before touchpad_input_dev is initialized).

Simplify the driver and make it robust by making the touchpad detection and
registration synchronous in the probe function. Introduce a timeout of
3 seconds when waiting for the touchpad info response packet, and add
3 second timeouts to the drain functions to prevent suspend/remove hangs.
Define probe_type as PROBE_PREFER_ASYNCHRONOUS so that the probe is run
asynchronously by the driver core, preventing boot delays.

This obsoletes the need for async work cancellation on driver remove
and debugfs NULL pointer checks.

Suggested-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Based-on-patch-by: Ronald Tschalär <ronald@innovation.ch>
Signed-off-by: Shih-Yuan Lee <fourdollars@debian.org>
---
 drivers/input/keyboard/applespi.c | 148 ++++++++++++++++++++++++------
 1 file changed, 118 insertions(+), 30 deletions(-)

diff --git a/drivers/input/keyboard/applespi.c b/drivers/input/keyboard/applespi.c
index b5ff71cd5a70..c1065a6ba96c 100644
--- a/drivers/input/keyboard/applespi.c
+++ b/drivers/input/keyboard/applespi.c
@@ -417,11 +417,16 @@ struct applespi_data {
 
 	bool				suspended;
 	bool				drain;
-	wait_queue_head_t		drain_complete;
+	wait_queue_head_t		wait_queue;
 	bool				read_active;
 	bool				write_active;
 
-	struct work_struct		work;
+	struct applespi_complete_info {
+		void				(*complete)(void *context);
+		struct applespi_data		*applespi;
+	}				spi_complete[2];
+	bool				cancel_spi;
+
 	struct touchpad_info_protocol	rcvd_tp_info;
 
 	struct dentry			*debugfs_root;
@@ -607,13 +612,61 @@ static void applespi_setup_write_txfrs(struct applespi_data *applespi)
 	spi_message_add_tail(st_t, msg);
 }
 
+static bool applespi_async_outstanding(struct applespi_data *applespi)
+{
+	return applespi->spi_complete[0].complete ||
+	       applespi->spi_complete[1].complete;
+}
+
+static void applespi_async_complete(void *context)
+{
+	struct applespi_complete_info *info = context;
+	struct applespi_data *applespi = info->applespi;
+	unsigned long flags;
+
+	info->complete(applespi);
+
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+
+	info->complete = NULL;
+
+	if (applespi->cancel_spi && !applespi_async_outstanding(applespi))
+		wake_up_all(&applespi->wait_queue);
+
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+}
+
 static int applespi_async(struct applespi_data *applespi,
 			  struct spi_message *message, void (*complete)(void *))
 {
-	message->complete = complete;
-	message->context = applespi;
+	struct applespi_complete_info *info;
+	int sts;
 
-	return spi_async(applespi->spi, message);
+	if (applespi->cancel_spi) {
+		if (!applespi_async_outstanding(applespi))
+			wake_up_all(&applespi->wait_queue);
+		return -ESHUTDOWN;
+	}
+
+	/*
+	 * There can only be at most 2 spi requests in flight, one for "reads"
+	 * and one for "writes".
+	 */
+	if (!applespi->spi_complete[0].complete)
+		info = &applespi->spi_complete[0];
+	else
+		info = &applespi->spi_complete[1];
+	info->complete = complete;
+	info->applespi = applespi;
+
+	message->complete = applespi_async_complete;
+	message->context = info;
+
+	sts = spi_async(applespi->spi, message);
+	if (sts)
+		info->complete = NULL;
+
+	return sts;
 }
 
 static inline bool applespi_check_write_status(struct applespi_data *applespi,
@@ -677,7 +730,7 @@ static int applespi_setup_spi(struct applespi_data *applespi)
 		return sts;
 
 	spin_lock_init(&applespi->cmd_msg_lock);
-	init_waitqueue_head(&applespi->drain_complete);
+	init_waitqueue_head(&applespi->wait_queue);
 
 	return 0;
 }
@@ -725,7 +778,7 @@ static void applespi_msg_complete(struct applespi_data *applespi,
 		applespi->write_active = false;
 
 	if (applespi->drain && !applespi->write_active)
-		wake_up_all(&applespi->drain_complete);
+		wake_up_all(&applespi->wait_queue);
 
 	if (is_write_msg) {
 		applespi->cmd_msg_queued = 0;
@@ -963,12 +1016,18 @@ static void applespi_debug_update_dimensions(struct applespi_data *applespi,
 static int applespi_tp_dim_open(struct inode *inode, struct file *file)
 {
 	struct applespi_data *applespi = inode->i_private;
+	struct input_dev *touchpad;
 
 	file->private_data = applespi;
 
+	/* Pairs with smp_store_release in applespi_register_touchpad_device() */
+	touchpad = smp_load_acquire(&applespi->touchpad_input_dev);
+	if (!touchpad)
+		return -ENODEV;
+
 	snprintf(applespi->tp_dim_val, sizeof(applespi->tp_dim_val),
 		 "0x%.4x %dx%d+%u+%u\n",
-		 applespi->touchpad_input_dev->id.product,
+		 touchpad->id.product,
 		 applespi->tp_dim_min_x, applespi->tp_dim_min_y,
 		 applespi->tp_dim_max_x - applespi->tp_dim_min_x,
 		 applespi->tp_dim_max_y - applespi->tp_dim_min_y);
@@ -1324,26 +1383,14 @@ applespi_register_touchpad_device(struct applespi_data *applespi,
 	return 0;
 }
 
-static void applespi_worker(struct work_struct *work)
-{
-	struct applespi_data *applespi =
-		container_of(work, struct applespi_data, work);
-
-	applespi_register_touchpad_device(applespi, &applespi->rcvd_tp_info);
-}
-
 static void applespi_handle_cmd_response(struct applespi_data *applespi,
 					 struct spi_packet *packet,
 					 struct message *message)
 {
 	if (packet->device == PACKET_DEV_INFO &&
 	    le16_to_cpu(message->type) == 0x1020) {
-		/*
-		 * We're not allowed to sleep here, but registering an input
-		 * device can sleep.
-		 */
 		applespi->rcvd_tp_info = message->tp_info;
-		schedule_work(&applespi->work);
+		wake_up_all(&applespi->wait_queue);
 		return;
 	}
 
@@ -1415,7 +1462,7 @@ static void applespi_got_data(struct applespi_data *applespi)
 			applespi->read_active = false;
 			applespi->write_active = false;
 
-			wake_up_all(&applespi->drain_complete);
+			wake_up_all(&applespi->wait_queue);
 		}
 
 		return;
@@ -1610,6 +1657,7 @@ static int applespi_probe(struct spi_device *spi)
 	struct applespi_data *applespi;
 	acpi_handle spi_handle = ACPI_HANDLE(&spi->dev);
 	acpi_status acpi_sts;
+	unsigned long flags;
 	int sts, i;
 	unsigned long long gpe, usb_status;
 
@@ -1628,8 +1676,6 @@ static int applespi_probe(struct spi_device *spi)
 
 	applespi->spi = spi;
 
-	INIT_WORK(&applespi->work, applespi_worker);
-
 	/* store the driver data */
 	spi_set_drvdata(spi, applespi);
 
@@ -1757,6 +1803,22 @@ static int applespi_probe(struct spi_device *spi)
 	/* trigger touchpad setup */
 	applespi_init(applespi, false);
 
+	/* set up the touchpad as a separate input device */
+	sts = wait_event_timeout(applespi->wait_queue,
+				 applespi->rcvd_tp_info.model_no,
+				 msecs_to_jiffies(3000));
+	if (!sts) {
+		dev_err(&applespi->spi->dev,
+			"Timed out waiting for device info\n");
+		sts = -ETIMEDOUT;
+		goto cancel_spi;
+	}
+
+	sts = applespi_register_touchpad_device(applespi,
+						&applespi->rcvd_tp_info);
+	if (sts)
+		goto cancel_spi;
+
 	/*
 	 * By default this device is not enabled for wakeup; but USB keyboards
 	 * generally are, so the expectation is that by default the keyboard
@@ -1789,25 +1851,50 @@ static int applespi_probe(struct spi_device *spi)
 			    &applespi_tp_dim_fops);
 
 	return 0;
+
+cancel_spi:
+	acpi_disable_gpe(NULL, applespi->gpe);
+	acpi_remove_gpe_handler(NULL, applespi->gpe, applespi_notify);
+
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+	applespi->cancel_spi = true;
+	wait_event_lock_irq(applespi->wait_queue,
+			    !applespi_async_outstanding(applespi),
+			    applespi->cmd_msg_lock);
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
+
+	return sts;
 }
 
 static void applespi_drain_writes(struct applespi_data *applespi)
 {
-	guard(spinlock_irqsave)(&applespi->cmd_msg_lock);
+	unsigned long flags;
+
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
 
 	applespi->drain = true;
-	wait_event_lock_irq(applespi->drain_complete, !applespi->write_active,
-			    applespi->cmd_msg_lock);
+	wait_event_lock_irq_timeout(applespi->wait_queue,
+				    !applespi->write_active,
+				    applespi->cmd_msg_lock,
+				    msecs_to_jiffies(3000));
+
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
 }
 
 static void applespi_drain_reads(struct applespi_data *applespi)
 {
-	guard(spinlock_irqsave)(&applespi->cmd_msg_lock);
+	unsigned long flags;
 
-	wait_event_lock_irq(applespi->drain_complete, !applespi->read_active,
-			    applespi->cmd_msg_lock);
+	spin_lock_irqsave(&applespi->cmd_msg_lock, flags);
+
+	wait_event_lock_irq_timeout(applespi->wait_queue,
+				    !applespi->read_active,
+				    applespi->cmd_msg_lock,
+				    msecs_to_jiffies(3000));
 
 	applespi->suspended = true;
+
+	spin_unlock_irqrestore(&applespi->cmd_msg_lock, flags);
 }
 
 static void applespi_remove(struct spi_device *spi)
@@ -1919,6 +2006,7 @@ static struct spi_driver applespi_driver = {
 		.name			= "applespi",
 		.acpi_match_table	= applespi_acpi_match,
 		.pm			= pm_sleep_ptr(&applespi_pm_ops),
+		.probe_type		= PROBE_PREFER_ASYNCHRONOUS,
 	},
 	.probe		= applespi_probe,
 	.remove		= applespi_remove,
-- 
2.39.5


^ permalink raw reply related

* [PATCH 0/2] Input: applespi - probe and remove fixes
From: Shih-Yuan Lee @ 2026-07-12 16:15 UTC (permalink / raw)
  To: Dmitry Torokhov; +Cc: linux-input, linux-kernel, Shih-Yuan Lee

This series fixes two bugs in the applespi driver found during work to
restore MacBook8,1 SPI keyboard/touchpad functionality across S3
suspend/resume cycles.

Patch 1 fixes a probe-time regression introduced when the touchpad
registration was made asynchronous: a deferred workqueue task could
complete after the spi_device was already removed, causing a NULL pointer
dereference.  The fix registers the touchpad input device synchronously
in probe and removes the deferred path.

Patch 2 fixes a use-after-free race in applespi_remove().  The original
code called applespi_drain_writes(), then removed the GPE handler, then
called applespi_drain_reads() -- but any read completion arriving after
GPE removal would reference the already-torn-down applespi struct.  The
fix uses the existing cancel_spi + wait_event_lock_irq mechanism to
drain all in-flight SPI transfers atomically before proceeding with
teardown.

These patches are independent of the companion spi-pxa2xx series also
submitted today (MacBook8,1 DMA quirk and LPSS S3 resume fix).

Shih-Yuan Lee (2):
  Input: applespi - register touchpad device synchronously in probe
  Input: applespi - fix use-after-free in applespi_remove()

 drivers/input/keyboard/applespi.c | 160 ++++++++++++++++++++++++------
 1 file changed, 127 insertions(+), 33 deletions(-)

-- 
2.39.5


^ permalink raw reply

* Re: [PATCH v5 0/3] Input/SPI: fixes for MacBook8,1 DMA timeout, UAF, and NULL pointer dereference
From: Shih-Yuan Lee @ 2026-07-12 16:05 UTC (permalink / raw)
  To: Dmitry Torokhov, Mark Brown
  Cc: linux-input, linux-spi, linux-kernel, Shih-Yuan Lee
In-Reply-To: <20260711055247.5412-1-fourdollars@debian.org>

Hi Dmitry, Mark, and the linux-input/linux-spi community,

Thank you for all the feedback across v1 through v5 of this series.

During further testing on the MacBook8,1 hardware, I discovered two
additional issues that change the scope of the fix:

1. The LPSS SPI controller (00:15.4) loses all private register state
   across S3 suspend/resume.  Restoring only the SSP clock on resume,
   without de-asserting LPSS_PRIV_RESETS (0x204) first, causes a PCIe
   Completion Timeout and a watchdog-triggered system reset.  Additionally,
   the IDMA block (which shares the SPI interrupt line) needs its registers
   restored to prevent spurious interrupts from masking real SPI transfers.
   This is a generic LPSS fix in spi-pxa2xx.c, not specific to MacBook8,1.

2. The applespi_remove() teardown has a use-after-free race between the
   drain_writes/drain_reads calls and the GPE handler removal.

Given these changes, the patch series has grown and now spans two
subsystems more cleanly than before:

  [input] Input: applespi - register touchpad device synchronously in probe
  [input] Input: applespi - fix use-after-free in applespi_remove()
  [spi]   spi: pxa2xx: disable DMA and fix runtime PM for Apple MacBook8,1
  [spi]   spi: pxa2xx: restore LPSS private and IDMA registers on S3 resume

I will submit these as two separate series to linux-input and linux-spi
respectively rather than continuing this thread, to keep subsystem
boundaries clean.

Thanks again for your patience and review.

Shih-Yuan Lee

-- 
2.39.5


^ permalink raw reply

* Re: [PATCH v3 00/10] Remove support for platform data from samsung keypad
From: Krzysztof Kozlowski @ 2026-07-12 12:30 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
	Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
	Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski,
	linux-input, devicetree, linux-kernel, linux-arm-kernel,
	linux-samsung-soc, patches, Conor Dooley, Bartosz Golaszewski
In-Reply-To: <20260711-samsung-kp-v3-0-b2fcaba77aff@gmail.com>

On Sat, Jul 11, 2026 at 07:12:41PM -0700, Dmitry Torokhov wrote:
> Hi,
> 
> This series converts the Samsung keypad driver and the Cragganmore 6410
> board to generic device properties, allowing the removal of legacy platform
> data and GPIO lookup tables.
> 
> We rework the Samsung keypad driver to stop using platform data, utilizing
> a new compact matrix keypad binding. To support this on the Cragganmore
> board, we add infrastructure to associate software nodes with Samsung
> gpio_chips, and add software node propagation support to the wm831x MFD
> core to route DVS GPIO to the PMIC regulator.
> 
> Finally, we convert all peripherals on the Cragganmore board (including
> keypad, GPIO keys, PMIC, WM1250, SPI0, basic-mmio-gpio, and LEDs) to
> software properties. This allows us to eliminate all legacy GPIO lookup
> tables and fixes incorrect GPIO chip names from previous conversions.
> 
> Mark, it would be great if you could give this a spin on Cragganmore.
> 
> The majority of changes are in arch/board code, with a couple of input
> matches and one for MFD. If everyone agrees maybe we can merge it all
> through the soc tree?

Yes, I could take it via samsung-soc. I assume I have your acks for
unput parts.

Best regards,
Krzysztof


^ permalink raw reply

* Re: [PATCH v2 02/10] m68k/coldfire: replace linux/gpio.h inclusions
From: Greg Ungerer @ 2026-07-12 12:27 UTC (permalink / raw)
  To: Arnd Bergmann, linux-gpio, Linus Walleij, Bartosz Golaszewski
  Cc: Arnd Bergmann, Andrew Lunn, Sebastian Hesselbarth,
	Gregory Clement, Frank Li, Robert Jarzmik, Krzysztof Kozlowski,
	Thomas Bogendoerfer, Hauke Mehrtens, Rafał Miłecki,
	Yoshinori Sato, John Paul Adrian Glaubitz, Dmitry Torokhov,
	Dominik Brodowski, linux-kernel, linux-arm-kernel,
	linux-samsung-soc, patches, linux-m68k, linux-mips, linux-sh,
	linux-input, linux-sunxi, linux-phy, linux-rockchip, linux-sound,
	Bartosz Golaszewski
In-Reply-To: <20260710211954.1373336-3-arnd@kernel.org>

Hi Arnd,

On 11/7/26 07:19, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> linux/gpio.h should no longer be used, convert these instead to
> linux/gpio/legacy.h for coldfire.
> 
> Acked-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I have applied this one to the for-next branch of the m68knommu git tree.

Thanks
Greg


> ---
>   arch/m68k/coldfire/device.c     | 2 +-
>   arch/m68k/include/asm/mcfgpio.h | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/m68k/coldfire/device.c b/arch/m68k/coldfire/device.c
> index 1420bae0964f..9a0258acd998 100644
> --- a/arch/m68k/coldfire/device.c
> +++ b/arch/m68k/coldfire/device.c
> @@ -12,7 +12,7 @@
>   #include <linux/init.h>
>   #include <linux/io.h>
>   #include <linux/spi/spi.h>
> -#include <linux/gpio.h>
> +#include <linux/gpio/legacy.h>
>   #include <linux/fec.h>
>   #include <linux/dmaengine.h>
>   #include <asm/traps.h>
> diff --git a/arch/m68k/include/asm/mcfgpio.h b/arch/m68k/include/asm/mcfgpio.h
> index 7103cfa4edb6..29726aa40eb6 100644
> --- a/arch/m68k/include/asm/mcfgpio.h
> +++ b/arch/m68k/include/asm/mcfgpio.h
> @@ -16,7 +16,7 @@ int __mcfgpio_request(unsigned gpio);
>   void __mcfgpio_free(unsigned gpio);
>   
>   #ifdef CONFIG_GPIOLIB
> -#include <linux/gpio.h>
> +#include <linux/gpio/legacy.h>
>   #else
>   
>   /* our alternate 'gpiolib' functions */


^ permalink raw reply

* [PATCH 2/2] selftests/hid: test battery queries with nonzero field offsets
From: Mason Camara @ 2026-07-12  4:47 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: Shuah Khan, John Chen, linux-input, linux-kselftest, linux-kernel
In-Reply-To: <20260712044702.893825-1-ping@masoncamara.com>

Add a UHID mouse whose input report 0x90 contains a status byte before
the capacity field. Answer GET_REPORT with 90 04 5f and read the power
supply capacity before sending any input report.

On an unpatched kernel, the capacity read returns the status byte (4)
instead of the capacity value (95), so the test fails. Enable
CONFIG_HID_BATTERY_STRENGTH in the HID selftest configuration.

Assisted-by: Codex:GPT-5
Signed-off-by: Mason Camara <ping@masoncamara.com>
---
 tools/testing/selftests/hid/config            |  1 +
 .../testing/selftests/hid/tests/test_mouse.py | 54 +++++++++++++++++++
 2 files changed, 55 insertions(+)

diff --git a/tools/testing/selftests/hid/config b/tools/testing/selftests/hid/config
index 1758b055f295..da52335b865a 100644
--- a/tools/testing/selftests/hid/config
+++ b/tools/testing/selftests/hid/config
@@ -16,6 +16,7 @@ CONFIG_FTRACE_SYSCALLS=y
 CONFIG_FUNCTION_TRACER=y
 CONFIG_HIDRAW=y
 CONFIG_HID=y
+CONFIG_HID_BATTERY_STRENGTH=y
 CONFIG_HID_BPF=y
 CONFIG_INPUT_EVDEV=y
 CONFIG_UHID=y
diff --git a/tools/testing/selftests/hid/tests/test_mouse.py b/tools/testing/selftests/hid/tests/test_mouse.py
index eb4e15a0e53b..141c1f06905b 100644
--- a/tools/testing/selftests/hid/tests/test_mouse.py
+++ b/tools/testing/selftests/hid/tests/test_mouse.py
@@ -11,6 +11,7 @@ import hidtools.hid
 from hidtools.util import BusType
 import libevdev
 import logging
+import threading
 import pytest
 
 logger = logging.getLogger("hidtools.test.mouse")
@@ -598,6 +599,35 @@ class ResolutionMultiplierHWheelMouse(TwoWheelMouse):
         return 0
 
 
+class BatteryOffsetMouse(BaseMouse):
+    report_descriptor = [
+        # Mouse report
+        0x05, 0x01, 0x09, 0x02, 0xa1, 0x01, 0x85, 0x12,
+        0x05, 0x09, 0x19, 0x01, 0x29, 0x02, 0x15, 0x00,
+        0x25, 0x01, 0x95, 0x02, 0x75, 0x01, 0x81, 0x02,
+        0x95, 0x01, 0x75, 0x06, 0x81, 0x01, 0x05, 0x01,
+        0x09, 0x01, 0xa1, 0x00, 0x09, 0x30, 0x09, 0x31,
+        0x15, 0x81, 0x25, 0x7f, 0x75, 0x08, 0x95, 0x02,
+        0x81, 0x06, 0xc0, 0xc0,
+        # Battery report: one status byte followed by capacity
+        0x06, 0x00, 0xff, 0x09, 0x14, 0xa1, 0x01, 0x85,
+        0x90, 0x05, 0x84, 0x75, 0x01, 0x95, 0x03, 0x15,
+        0x00, 0x25, 0x01, 0x09, 0x61, 0x05, 0x85, 0x09,
+        0x44, 0x09, 0x46, 0x81, 0x02, 0x95, 0x05, 0x81,
+        0x01, 0x75, 0x08, 0x95, 0x01, 0x15, 0x00, 0x26,
+        0xff, 0x00, 0x09, 0x65, 0x81, 0x02, 0xc0,
+    ]
+
+    def __init__(self, rdesc=report_descriptor, name=None, input_info=None):
+        super().__init__(rdesc, name, input_info)
+
+    def get_report(self, req, rnum, rtype):
+        if rtype != self.UHID_INPUT_REPORT or rnum != 0x90:
+            return (1, [])
+
+        return (0, [0x90, 0x04, 0x5F])
+
+
 class BaseTest:
     class TestMouse(base.BaseTestCase.TestUhid):
         def test_buttons(self):
@@ -1045,3 +1075,27 @@ class TestBadReportDescriptorMouse(base.BaseTestCase.TestUhid):
 
     def assertName(self, uhdev):
         pass
+
+
+class TestBatteryOffsetMouse(base.BaseTestCase.TestUhid):
+    def create_device(self):
+        return BatteryOffsetMouse()
+
+    def test_queried_battery_field_offset(self):
+        uhdev = self.uhdev
+        power_supply = uhdev.power_supply_class
+        assert power_supply is not None
+
+        done = False
+
+        def dispatch():
+            while not done:
+                uhdev.dispatch(1)
+
+        thread = threading.Thread(target=dispatch)
+        thread.start()
+        try:
+            assert power_supply.capacity == 95
+        finally:
+            done = True
+            thread.join()
-- 
2.55.0


^ permalink raw reply related

* [PATCH 1/2] HID: input: honor report field offsets in battery queries
From: Mason Camara @ 2026-07-12  4:47 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires
  Cc: Shuah Khan, John Chen, linux-input, linux-kselftest, linux-kernel
In-Reply-To: <20260712044702.893825-1-ping@masoncamara.com>

hidinput_query_battery_capacity() always reads buf[1] from a GET_REPORT
reply. This assumes capacity usage starts at bit 0 of the report payload
and that capacity occupies the first byte.

The Magic Mouse 2 puts a status byte before AbsoluteStateOfCharge in
input report 0x90. Its GET_REPORT response is:

  90 04 5f

Here 0x04 is the status byte and 0x5f is the capacity. Reading buf[1]
reports 4% in sysfs.

This query runs when userspace reads capacity before an input report
updates the battery cache. The event path already extracts the usage
from the descriptor, so moving the mouse before the sysfs read can hide
the bug.

Record the report containing the capacity field, bit offset, and size when
setting up the battery. Allocate a buffer large enough for the complete
report, skip the report ID byte, and extract the value at the recorded bit
offset. Reject replies that do not contain the complete field.

Fixes: 9de07a4e8d4c ("HID: input: map battery capacity (00850065)")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221263
Assisted-by: Codex:GPT-5
Signed-off-by: Mason Camara <ping@masoncamara.com>
---
 drivers/hid/hid-input.c | 39 +++++++++++++++++++++++++++++----------
 include/linux/hid.h     |  6 ++++++
 2 files changed, 35 insertions(+), 10 deletions(-)

diff --git a/drivers/hid/hid-input.c b/drivers/hid/hid-input.c
index 3487600cadb4..01ce9e176299 100644
--- a/drivers/hid/hid-input.c
+++ b/drivers/hid/hid-input.c
@@ -431,18 +431,28 @@ static int hidinput_scale_battery_capacity(struct hid_battery *bat,
 
 static int hidinput_query_battery_capacity(struct hid_battery *bat)
 {
+	u8 *buf __free(kfree) = NULL;
+	unsigned int field_end;
+	unsigned int report_len;
+	u32 value;
 	int ret;
 
-	u8 *buf __free(kfree) = kmalloc(4, GFP_KERNEL);
+	report_len = hid_report_len(bat->report) + !bat->report->id;
+	buf = hid_alloc_report_buf(bat->report, GFP_KERNEL);
 	if (!buf)
 		return -ENOMEM;
 
-	ret = hid_hw_raw_request(bat->dev, bat->report_id, buf, 4,
+	ret = hid_hw_raw_request(bat->dev, bat->report_id, buf, report_len,
 				 bat->report_type, HID_REQ_GET_REPORT);
-	if (ret < 2)
+	field_end = DIV_ROUND_UP(bat->report_offset + bat->report_size, 8) +
+		    1;
+	if (ret < 0 || (unsigned int)ret < field_end)
 		return -ENODATA;
 
-	return hidinput_scale_battery_capacity(bat, buf[1]);
+	value = hid_field_extract(bat->dev, buf + 1, bat->report_offset,
+				  bat->report_size);
+
+	return hidinput_scale_battery_capacity(bat, value);
 }
 
 static int hidinput_get_battery_property(struct power_supply *psy,
@@ -529,7 +539,8 @@ static void hidinput_cleanup_battery(void *res)
 }
 
 static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
-				  struct hid_field *field, bool is_percentage)
+				  struct hid_field *field, unsigned int usage_index,
+				  bool is_percentage)
 {
 	struct hid_battery *bat;
 	struct power_supply_desc *psy_desc;
@@ -593,6 +604,10 @@ static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
 	bat->max = max;
 	bat->report_type = report_type;
 	bat->report_id = field->report->id;
+	bat->report = field->report;
+	bat->report_offset = field->report_offset +
+			     usage_index * field->report_size;
+	bat->report_size = field->report_size;
 	bat->charge_status = POWER_SUPPLY_STATUS_DISCHARGING;
 	bat->status = HID_BATTERY_UNKNOWN;
 
@@ -687,7 +702,8 @@ static void hidinput_update_battery(struct hid_device *dev, int report_id,
 }
 #else  /* !CONFIG_HID_BATTERY_STRENGTH */
 static int hidinput_setup_battery(struct hid_device *dev, unsigned report_type,
-				  struct hid_field *field, bool is_percentage)
+				  struct hid_field *field, unsigned int usage_index,
+				  bool is_percentage)
 {
 	return 0;
 }
@@ -1035,7 +1051,8 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 			break;
 
 		case 0x3b: /* Battery Strength */
-			hidinput_setup_battery(device, HID_INPUT_REPORT, field, false);
+			hidinput_setup_battery(device, HID_INPUT_REPORT, field,
+					       usage_index, false);
 			usage->type = EV_PWR;
 			return;
 
@@ -1313,7 +1330,8 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 	case HID_UP_GENDEVCTRLS:
 		switch (usage->hid) {
 		case HID_DC_BATTERYSTRENGTH:
-			hidinput_setup_battery(device, HID_INPUT_REPORT, field, false);
+			hidinput_setup_battery(device, HID_INPUT_REPORT, field,
+					       usage_index, false);
 			usage->type = EV_PWR;
 			return;
 		}
@@ -1322,7 +1340,8 @@ static void hidinput_configure_usage(struct hid_input *hidinput, struct hid_fiel
 	case HID_UP_BATTERY:
 		switch (usage->hid) {
 		case HID_BAT_ABSOLUTESTATEOFCHARGE:
-			hidinput_setup_battery(device, HID_INPUT_REPORT, field, true);
+			hidinput_setup_battery(device, HID_INPUT_REPORT, field,
+					       usage_index, true);
 			usage->type = EV_PWR;
 			return;
 		case HID_BAT_CHARGING:
@@ -2040,7 +2059,7 @@ static void report_features(struct hid_device *hid)
 				/* Verify if Battery Strength feature is available */
 				if (usage->hid == HID_DC_BATTERYSTRENGTH)
 					hidinput_setup_battery(hid, HID_FEATURE_REPORT,
-							       rep->field[i], false);
+							       rep->field[i], j, false);
 
 				if (drv->feature_mapping)
 					drv->feature_mapping(hid, rep->field[i], usage);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index b240baa95ab5..396de1080fdc 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -642,6 +642,9 @@ enum hid_battery_status {
  * @max: maximum battery value from HID descriptor
  * @report_type: HID report type (input/feature)
  * @report_id: HID report ID for this battery
+ * @report: HID report containing the capacity field
+ * @report_offset: bit offset of the capacity field in the report
+ * @report_size: size of the capacity field in bits
  * @charge_status: current charging status
  * @status: battery reporting status
  * @capacity: current battery capacity (0-100)
@@ -657,6 +660,9 @@ struct hid_battery {
 	__s32 max;
 	__s32 report_type;
 	__s32 report_id;
+	struct hid_report *report;
+	__u32 report_offset;
+	__u32 report_size;
 	__s32 charge_status;
 	enum hid_battery_status status;
 	__s32 capacity;
-- 
2.55.0


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox