* Re: [PATCH v2] HID: intel-thc-hid: intel-quickspi: validate report size before copy
From: sashiko-bot @ 2026-07-17 9:37 UTC (permalink / raw)
To: HyeongJun An; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260717091622.1744196-1-sammiee5311@gmail.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Unbounded memcpy in `quickspi_get_report()` causes a heap buffer overflow if the device responds with a report larger than the caller-allocated buffer.
- [High] The receive buffer `qsdev->input_buf` can be overflowed during an RXDMA interrupt because the message size is bounds-checked against a 4K-aligned limit instead of the actual buffer capacity.
- [High] Unbounded memcpy in `quickspi_handle_input_data()` causes a heap buffer overflow of `qsdev->report_buf` on the receive path.
- [High] Concurrent ioctl requests lead to data corruption in the shared DMA buffer `qsdev->report_buf`.
--
--- Patch [1]: [PATCH v2] HID: intel-thc-hid: intel-quickspi: validate report size before copy ---
commit 13f5051992cd9276077cab1966153d3866079f49
Author: HyeongJun An <sammiee5311@gmail.com>
HID: intel-thc-hid: intel-quickspi: validate report size before copy
write_cmd_to_txdma() builds an output report in qsdev->report_buf, a heap
buffer allocated in quickspi_alloc_report_buf() to the device-descriptor
derived max_report_len (a few hundred bytes for a touch controller). It
copies the caller-supplied report into that buffer:
[ ... ]
> 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..da5ecfcd0fbf8 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> @@ -555,7 +555,14 @@ static int quickspi_alloc_report_buf(struct quickspi_device *qsdev)
> max_report_len = max(le16_to_cpu(qsdev->dev_desc.max_output_len),
> le16_to_cpu(qsdev->dev_desc.max_input_len));
>
> - qsdev->report_buf = devm_kzalloc(qsdev->dev, max_report_len, GFP_KERNEL);
> + /*
> + * write_cmd_to_txdma() writes the output report header ahead of the
> + * content in this buffer, so it has to hold both.
> + */
> + qsdev->report_buf_size = HIDSPI_OUTPUT_REPORT_SIZE(max_report_len);
> +
> + qsdev->report_buf = devm_kzalloc(qsdev->dev, qsdev->report_buf_size,
> + GFP_KERNEL);
[Severity: High]
This is a pre-existing issue, but does the allocation for qsdev->input_buf
earlier in quickspi_alloc_report_buf() need to be 4K-aligned to prevent a heap
overflow during an RX DMA interrupt?
In thc_rxdma_read(), read_dma_buffer() bounds-checks the message length
against read_config->max_packet_size, which is 4K-aligned via
thc_dma_set_max_packet_sizes().
If a hardware device sends a DMA packet larger than max_input_len but
smaller than or equal to ALIGN(max_input_len, SZ_4K), it would pass the
bounds check in read_dma_buffer():
intel-thc/intel-thc-dma.c:read_dma_buffer() {
...
if (mes_len > read_config->max_packet_size) {
...
}
and copy up to 4096 bytes into qsdev->input_buf, which was allocated strictly
to the unaligned max_input_len.
> if (!qsdev->report_buf)
> return -ENOMEM;
>
[ ... ]
> diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
> index cb19057f1191b..db6054843e777 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
> @@ -30,6 +30,9 @@ static int write_cmd_to_txdma(struct quickspi_device *qsdev,
>
> write_buf = (struct output_report *)qsdev->report_buf;
>
> + if (HIDSPI_OUTPUT_REPORT_SIZE(report_buf_len) > qsdev->report_buf_size)
> + return -EINVAL;
> +
> write_buf->output_hdr.report_type = report_type;
> write_buf->output_hdr.content_len = cpu_to_le16(report_buf_len);
> write_buf->output_hdr.content_id = report_id;
>
> if (report_buf && report_buf_len > 0)
> memcpy(write_buf->content, report_buf, report_buf_len);
[Severity: High]
This is a pre-existing issue, but can concurrent ioctl requests cause data
corruption in the shared DMA buffer qsdev->report_buf?
Multiple userspace processes can concurrently call HIDIOCSFEATURE or
HIDIOCGFEATURE. The HID core routines hold a read-lock, allowing parallel
execution in the driver callbacks.
Does write_cmd_to_txdma() modify qsdev->report_buf and trigger DMA writes
without mutual exclusion against concurrent TX or RX requests like
quickspi_handle_input_data()?
quickspi-protocol.c:quickspi_handle_input_data() {
...
memcpy(qsdev->report_buf, input_report, qsdev->report_len);
...
}
[Severity: High]
This is a pre-existing issue, but can the receive path also overflow
qsdev->report_buf?
While this patch secures the output path, quickspi_handle_input_data()
parses input_len from the payload and executes an unbounded memcpy:
quickspi-protocol.c:quickspi_handle_input_data() {
...
memcpy(qsdev->report_buf, input_report, qsdev->report_len);
...
}
If a compromised device sends a GET_FEATURE_RESPONSE with a length exceeding
qsdev->report_buf_size, will this cause a heap buffer overflow?
[Severity: Critical]
This isn't a bug introduced by this patch, but is there an unbounded memcpy
in quickspi_get_report() that could cause a heap buffer overflow?
When a userspace process calls ioctl(HIDIOCGFEATURE) with a length parameter
smaller than the actual hardware report size, quickspi_hid_raw_request()
discards the bounds-checking len parameter:
quickspi-hid.c:quickspi_hid_raw_request() {
...
ret = quickspi_get_report(qsdev, rtype, reportnum, buf);
...
}
Then quickspi_get_report() copies qsdev->report_len bytes into the
caller's buffer without validating against the buffer's true capacity:
quickspi-protocol.c:quickspi_get_report() {
...
memcpy(buf, qsdev->report_buf, qsdev->report_len);
...
}
Could this lead to memory corruption if the device responds with a report
larger than the caller-allocated buffer?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717091622.1744196-1-sammiee5311@gmail.com?part=1
^ permalink raw reply
* Re: [PATCH v4 0/3] input: misc: Add an initial driver for haptics inside Qcom PMIH010x PMIC
From: Krzysztof Kozlowski @ 2026-07-17 9:25 UTC (permalink / raw)
To: Fenglin Wu, linux-arm-msm, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Stephen Boyd,
Bjorn Andersson, Konrad Dybcio
Cc: David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260717-qcom-spmi-haptics-v4-0-b0fe0ed30849@oss.qualcomm.com>
On 17/07/2026 09:28, Fenglin Wu wrote:
> Dependencies:
> - [patch 2/3] depends on [patch 1/3] and they should be applied together
>
Why are you cc-ing internal kernel@oss.qualcomm.com?
I already asked you TWICE. Both times you ignored that, so that's it.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v2] HID: intel-thc-hid: intel-quickspi: validate report size before copy
From: HyeongJun An @ 2026-07-17 9:16 UTC (permalink / raw)
To: Even Xu, Xinpeng Sun, Jiri Kosina, Benjamin Tissoires
Cc: linux-input, linux-kernel, stable, HyeongJun An
In-Reply-To: <20260628133717.941389-1-sammiee5311@gmail.com>
write_cmd_to_txdma() builds an output report in qsdev->report_buf, a heap
buffer allocated in quickspi_alloc_report_buf() to the device-descriptor
derived max_report_len (a few hundred bytes for a touch controller). It
copies the caller-supplied report into that buffer:
memcpy(write_buf->content, report_buf, report_buf_len);
The HID core caps a report at HID_MAX_BUFFER_SIZE (16384) by default, and
quickspi_hid_ll_driver does not set max_buffer_size, so the length reaches
the driver unbounded. A hidraw SET_REPORT/SET_FEATURE ioctl carrying a
report larger than max_report_len therefore overflows report_buf with
attacker-controlled length and content.
Record the report_buf allocation size and reject reports that do not fit
before copying, matching the equivalent guard in the intel-quicki2c
sibling (quicki2c_init_write_buf()) and the hid-goodix-spi fix.
write_cmd_to_txdma() writes the output report header ahead of the content
in the same buffer, so size the allocation to cover the header as well.
That keeps the added bound from rejecting a maximum-sized report.
Fixes: 9d8d51735a3a ("HID: intel-thc-hid: intel-quickspi: Add HIDSPI protocol implementation")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
---
v2: Size report_buf to cover the output report header as well, so the added
bound cannot reject a valid maximum-sized report (raised by the Sashiko
AI review of v1). No other change.
v1: https://lore.kernel.org/all/20260628133717.941389-1-sammiee5311@gmail.com/
drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c | 9 ++++++++-
drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h | 1 +
.../hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c | 3 +++
3 files changed, 12 insertions(+), 1 deletion(-)
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 4ae2e1718b30..da5ecfcd0fbf 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
@@ -555,7 +555,14 @@ static int quickspi_alloc_report_buf(struct quickspi_device *qsdev)
max_report_len = max(le16_to_cpu(qsdev->dev_desc.max_output_len),
le16_to_cpu(qsdev->dev_desc.max_input_len));
- qsdev->report_buf = devm_kzalloc(qsdev->dev, max_report_len, GFP_KERNEL);
+ /*
+ * write_cmd_to_txdma() writes the output report header ahead of the
+ * content in this buffer, so it has to hold both.
+ */
+ qsdev->report_buf_size = HIDSPI_OUTPUT_REPORT_SIZE(max_report_len);
+
+ qsdev->report_buf = devm_kzalloc(qsdev->dev, qsdev->report_buf_size,
+ GFP_KERNEL);
if (!qsdev->report_buf)
return -ENOMEM;
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..0ed964bfe3dd 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-dev.h
@@ -157,6 +157,7 @@ struct quickspi_device {
u8 *report_descriptor;
u8 *input_buf;
u8 *report_buf;
+ u32 report_buf_size;
u32 report_len;
wait_queue_head_t reset_ack_wq;
diff --git a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
index cb19057f1191..db6054843e77 100644
--- a/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
+++ b/drivers/hid/intel-thc-hid/intel-quickspi/quickspi-protocol.c
@@ -30,6 +30,9 @@ static int write_cmd_to_txdma(struct quickspi_device *qsdev,
write_buf = (struct output_report *)qsdev->report_buf;
+ if (HIDSPI_OUTPUT_REPORT_SIZE(report_buf_len) > qsdev->report_buf_size)
+ return -EINVAL;
+
write_buf->output_hdr.report_type = report_type;
write_buf->output_hdr.content_len = cpu_to_le16(report_buf_len);
write_buf->output_hdr.content_id = report_id;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v4 3/3] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: sashiko-bot @ 2026-07-17 7:42 UTC (permalink / raw)
To: Fenglin Wu; +Cc: conor+dt, devicetree, robh, linux-input, dmitry.torokhov
In-Reply-To: <20260717-qcom-spmi-haptics-v4-3-b0fe0ed30849@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Missing stop logic in the haptics worker causes indefinite playback and PM reference leak when effect duration expires.
- [Medium] Explicit input_unregister_device() on a devm-managed device violates input subsystem rules.
- [Medium] Data race on `irq_enabled` causes permanently unbalanced IRQ disable depth on bus errors.
- [Low] Use of undefined macro `kmalloc_obj` causes compilation failure.
--
--- Patch [3]: [PATCH v4 3/3] input: misc: Add Qualcomm SPMI PMIC haptics driver ---
commit e63f14da39f2a372d48e418f9fac8b20cd9d0698
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
input: misc: Add Qualcomm SPMI PMIC haptics driver
Adds an initial driver for the Qualcomm PMIH0108 PMIC haptics module.
Supports DIRECT_PLAY and FIFO modes via the input force-feedback framework.
> diff --git a/drivers/input/misc/qcom-spmi-haptics.c b/drivers/input/misc/qcom-spmi-haptics.c
> new file mode 100644
> index 0000000000000..235f77e0229c5
> --- /dev/null
> +++ b/drivers/input/misc/qcom-spmi-haptics.c
[ ... ]
> +static int haptics_enqueue_play_req(struct qcom_haptics *h, int effect_id, bool play)
> +{
> + struct haptics_play_req *req;
> +
> + guard(spinlock_irqsave)(&h->play_queue_lock);
> +
> + /*
> + * Coalesce with an already queued request for
> + * the same effect instead of appending.
> + */
> + list_for_each_entry(req, &h->play_queue, node) {
> + if (req->effect_id == effect_id) {
> + req->play = play;
> + return 0;
> + }
> + }
> +
> + req = kmalloc_obj(*req, GFP_ATOMIC);
> + if (!req)
> + return -ENOMEM;
[Severity: Low]
Does this code compile successfully? I couldn't find kmalloc_obj() defined in
the standard kernel headers. Should this use kmalloc() or kzalloc() instead?
> +
> + req->effect_id = effect_id;
> + req->play = play;
[ ... ]
> +static irqreturn_t haptics_fifo_empty_irq(int irq, void *dev_id)
> +{
[ ... ]
> + /* Refill: write the next chunk */
> + to_write = min_t(u32, h->data_len - h->data_written,
> + h->fifo_len - FIFO_EMPTY_THRESH);
> + ret = haptics_write_fifo_chunk(h, &h->fifo_data[h->data_written], to_write);
> + if (ret) {
> + dev_err(h->dev, "refill FIFO samples failed, ret=%d\n", ret);
> + /*
> + * If data refilling is failed,stop the HW play and disable the
> + * IRQ to prevent the FIFO empty IRQ being fired continuously.
> + */
> + cfg_write(h, HAP_CFG_SPMI_PLAY_REG, 0);
> + disable_irq_nosync(h->fifo_empty_irq);
> + h->irq_enabled = false;
[Severity: Medium]
Can a data race occur on h->irq_enabled here?
If userspace stops playback concurrently with a bus write failure in
haptics_fifo_empty_irq(), both paths could check and modify h->irq_enabled
without mutual exclusion.
For example, this path:
haptics_fifo_empty_irq()
disable_irq_nosync(h->fifo_empty_irq);
h->irq_enabled = false;
Could race with:
haptics_stop_locked()
haptics_fifo_irq_enable(h, false);
This might result in disable_irq() being called twice while enable_irq() is
only called once, leaving the IRQ permanently disabled and breaking future
FIFO playback.
> + return IRQ_HANDLED;
> + }
> +
> + h->data_written += to_write;
[ ... ]
> +static void haptics_play_work(struct work_struct *work)
> +{
> + struct qcom_haptics *h = container_of(to_delayed_work(work),
> + struct qcom_haptics, play_work);
> + struct haptics_play_req *req = haptics_dequeue_play_req(h);
> + bool rearmed = false;
> + u32 length_us;
> + int ret;
> +
> + guard(mutex)(&h->play_lock);
> +
> + /* No valid request available */
> + if (req == NULL)
> + return;
[Severity: High]
When a finite-length effect expires, schedule_delayed_work() wakes up this
worker, but haptics_dequeue_play_req() returns NULL since the queue is
empty.
Since we return early here when req is NULL, how does the driver stop the
active effect and release the runtime PM reference? This appears to leave
the device vibrating indefinitely.
> +
> + if (req->play) {
> + ret = haptics_start_locked(h, req->effect_id);
[ ... ]
> +static void qcom_haptics_remove(struct platform_device *pdev)
> +{
> + struct qcom_haptics *h = platform_get_drvdata(pdev);
> +
> + /*
> + * Unregister the input device explicitly at the beginning
> + * to avoid the input device being used after the resources
> + * are freed.
> + */
> + input_unregister_device(h->input);
[Severity: Medium]
Is it safe to explicitly unregister this input device here?
The input device is allocated with devm_input_allocate_device() during
probe, which means the input core will automatically handle unregistration
when the provider device is unbound.
Manually calling input_unregister_device() on a devm-managed device can
lead to double-unregistration.
> + disable_delayed_work_sync(&h->play_work);
> + scoped_guard(mutex, &h->play_lock) {
> + haptics_queue_flush(h);
> + haptics_stop_locked(h, true);
> + }
> +
> + haptics_enable_module(h, false);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-qcom-spmi-haptics-v4-0-b0fe0ed30849@oss.qualcomm.com?part=3
^ permalink raw reply
* [PATCH v4 3/3] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: Fenglin Wu @ 2026-07-17 7:29 UTC (permalink / raw)
To: linux-arm-msm, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lee Jones, Stephen Boyd, Bjorn Andersson,
Konrad Dybcio
Cc: David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel, Fenglin Wu
In-Reply-To: <20260717-qcom-spmi-haptics-v4-0-b0fe0ed30849@oss.qualcomm.com>
Add an initial driver for the Qualcomm PMIH0108 PMIC haptics module,
named as HAP530_HV. This module supports several play modes, including
DIRECT_PLAY, FIFO, PAT_MEM, and SWR, each with distinct data sourcing
and hardware data handling logic. Currently, the driver provides support
for two play modes using the input force-feedback framework: FF_CONSTANT
effect for DIRECT_PLAY mode and FF_PERIODIC effect with FF_CUSTOM
waveform for FIFO mode.
Assisted-by: Claude:claude-4-8-opus
Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
drivers/input/misc/Kconfig | 11 +
drivers/input/misc/Makefile | 1 +
drivers/input/misc/qcom-spmi-haptics.c | 1178 ++++++++++++++++++++++++++++++++
3 files changed, 1190 insertions(+)
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 1f6c57dba030..4f40940973e4 100644
--- a/drivers/input/misc/Kconfig
+++ b/drivers/input/misc/Kconfig
@@ -236,6 +236,17 @@ config INPUT_PMIC8XXX_PWRKEY
To compile this driver as a module, choose M here: the
module will be called pmic8xxx-pwrkey.
+config INPUT_QCOM_SPMI_HAPTICS
+ tristate "Qualcomm SPMI PMIC haptics support"
+ depends on MFD_SPMI_PMIC
+ help
+ Say Y to enable support for the Qualcomm PMIH0108 SPMI PMIC haptics
+ module. Supports DIRECT_PLAY, FIFO streaming play modes via the
+ Linux input force-feedback framework.
+
+ To compile this driver as a module, choose M here: the module will
+ be called qcom-spmi-haptics.
+
config INPUT_SPARCSPKR
tristate "SPARC Speaker support"
depends on PCI && SPARC64
diff --git a/drivers/input/misc/Makefile b/drivers/input/misc/Makefile
index 2281d6803fce..c5c9aa139a11 100644
--- a/drivers/input/misc/Makefile
+++ b/drivers/input/misc/Makefile
@@ -69,6 +69,7 @@ obj-$(CONFIG_INPUT_PMIC8XXX_PWRKEY) += pmic8xxx-pwrkey.o
obj-$(CONFIG_INPUT_POWERMATE) += powermate.o
obj-$(CONFIG_INPUT_PWM_BEEPER) += pwm-beeper.o
obj-$(CONFIG_INPUT_PWM_VIBRA) += pwm-vibra.o
+obj-$(CONFIG_INPUT_QCOM_SPMI_HAPTICS) += qcom-spmi-haptics.o
obj-$(CONFIG_INPUT_QNAP_MCU) += qnap-mcu-input.o
obj-$(CONFIG_INPUT_RAVE_SP_PWRBUTTON) += rave-sp-pwrbutton.o
obj-$(CONFIG_INPUT_RB532_BUTTON) += rb532_button.o
diff --git a/drivers/input/misc/qcom-spmi-haptics.c b/drivers/input/misc/qcom-spmi-haptics.c
new file mode 100644
index 000000000000..235f77e0229c
--- /dev/null
+++ b/drivers/input/misc/qcom-spmi-haptics.c
@@ -0,0 +1,1178 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#include <linux/bitfield.h>
+#include <linux/bits.h>
+#include <linux/device.h>
+#include <linux/input.h>
+#include <linux/interrupt.h>
+#include <linux/list.h>
+#include <linux/math64.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/regmap.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+#include <linux/uaccess.h>
+#include <linux/workqueue.h>
+
+/* HAP_CFG register offsets, bit fields, value constants */
+#define HAP_CFG_INT_RT_STS_REG 0x10
+#define FIFO_EMPTY_BIT BIT(1)
+#define HAP_CFG_EN_CTL_REG 0x46
+#define HAPTICS_EN_BIT BIT(7)
+#define HAP_CFG_VMAX_REG 0x48
+#define VMAX_STEP_MV 50
+#define VMAX_MV_MAX 10000
+#define HAP_CFG_SPMI_PLAY_REG 0x4C
+#define PLAY_EN_BIT BIT(7)
+#define PAT_SRC_MASK GENMASK(2, 0)
+#define PAT_SRC_FIFO 0
+#define PAT_SRC_DIRECT_PLAY 1
+#define HAP_CFG_TLRA_OL_HIGH_REG 0x5C
+#define TLRA_OL_MSB_MASK GENMASK(3, 0)
+#define TLRA_STEP_US 5
+#define TLRA_US_MAX 20475
+#define HAP_CFG_TLRA_OL_LOW_REG 0x5D
+#define HAP_CFG_DRV_DUTY_CFG_REG 0x60
+#define ADT_DRV_DUTY_EN_BIT BIT(7)
+#define ADT_BRK_DUTY_EN_BIT BIT(6)
+#define DRV_DUTY_MASK GENMASK(5, 3)
+#define AUTORES_DRV_DUTY_62P5 2
+#define BRK_DUTY_MASK GENMASK(2, 0)
+#define AUTORES_BRK_DUTY_62P5 5
+#define HAP_CFG_ZX_WIND_CFG_REG 0x62
+#define ZX_DEBOUNCE_MASK GENMASK(6, 4)
+#define AUTORES_ZX_DEBOUNCE 3
+#define ZX_WIN_HEIGHT_MASK GENMASK(2, 0)
+#define AUTORES_ZX_WIN_HEIGHT 2
+#define HAP_CFG_AUTORES_CFG_REG 0x63
+#define AUTORES_EN_BIT BIT(7)
+#define AUTORES_EN_DLY_MASK GENMASK(6, 2)
+#define AUTORES_EN_DLY_CYCLES 10
+#define AUTORES_ERR_WIN_MASK GENMASK(1, 0)
+#define AUTORES_ERR_WIN_25PCT 1
+#define HAP_CFG_FAULT_CLR_REG 0x66
+#define ZX_TO_FAULT_CLR_BIT BIT(4)
+#define SC_CLR_BIT BIT(2)
+#define AUTO_RES_ERR_CLR_BIT BIT(1)
+#define HPWR_RDY_FAULT_CLR_BIT BIT(0)
+#define FAULT_CLR_ALL (ZX_TO_FAULT_CLR_BIT | SC_CLR_BIT | \
+ AUTO_RES_ERR_CLR_BIT | HPWR_RDY_FAULT_CLR_BIT)
+#define HAP_CFG_RAMP_DN_CFG2_REG 0x86
+#define AUTORES_PRE_HIZ_DLY_10US 1
+
+/* HAP_PTN register offsets, bit fields, value constants */
+#define HAP_PTN_REVISION2_REG 0x01
+#define HAP_PTN_FIFO_DIN_0_REG 0x20
+#define HAP_PTN_FIFO_PLAY_RATE_REG 0x24
+#define FIFO_PLAY_RATE_MASK GENMASK(3, 0)
+#define HAP_PTN_DIRECT_PLAY_REG 0x26
+#define HAP_PTN_FIFO_EMPTY_CFG_REG 0x2A
+#define FIFO_THRESH_LSB 64
+#define HAP_PTN_FIFO_DIN_1B_REG 0x2C
+#define HAP_PTN_MEM_OP_ACCESS_REG 0x2D
+#define MEM_FLUSH_RELOAD_BIT BIT(0)
+#define HAP_PTN_MMAP_FIFO_REG 0xA0
+#define MMAP_FIFO_EXIST_BIT BIT(7)
+#define MMAP_FIFO_LEN_MASK GENMASK(6, 0)
+#define HAP_PTN_PATX_PLAY_CFG_REG 0xA2
+
+#define HAP530_MEM_TOTAL_BYTES 8192
+#define FIFO_EMPTY_THRESH 280
+#define FIFO_INIT_FILL 320
+
+#define HAPTICS_AUTOSUSPEND_MS 1000
+
+/*
+ * FF_CUSTOM data layout (custom_data[] of type s16):
+ * [0] = play rate (PLAY_RATE_*)
+ * [1] = vmax in mV (0 = use device default from qcom,vmax-microvolt)
+ * [2..N-1] = signed 8-bit PCM samples packed one per s16 (lower byte used)
+ */
+#define CUSTOM_DATA_RATE_IDX 0
+#define CUSTOM_DATA_VMAX_IDX 1
+#define CUSTOM_DATA_SAMPLE_START 2
+#define CUSTOM_DATA_MAX_LEN (48 * 1024)
+
+#define HAPTICS_MAX_EFFECTS 8
+
+enum qcom_haptics_mode {
+ HAPTICS_MODE_NONE,
+ HAPTICS_DIRECT_PLAY,
+ HAPTICS_FIFO,
+};
+
+enum qcom_haptics_play_rate {
+ PLAY_RATE_T_LRA = 0,
+ PLAY_RATE_T_LRA_DIV_2 = 1,
+ PLAY_RATE_T_LRA_DIV_4 = 2,
+ PLAY_RATE_T_LRA_DIV_8 = 3,
+ PLAY_RATE_T_LRA_X_2 = 4,
+ PLAY_RATE_T_LRA_X_4 = 5,
+ PLAY_RATE_T_LRA_X_8 = 6,
+ PLAY_RATE_RESERVED = 7,
+ PLAY_RATE_F_8KHZ = 8,
+ PLAY_RATE_F_16KHZ = 9,
+ PLAY_RATE_F_24KHZ = 10,
+ PLAY_RATE_F_32KHZ = 11,
+ PLAY_RATE_F_44P1KHZ = 12,
+ PLAY_RATE_F_48KHZ = 13,
+ PLAY_RATE_MAX = PLAY_RATE_F_48KHZ,
+};
+
+/**
+ * struct qcom_haptics_effect: A haptics effect
+ * @mode: haptics HW play mode
+ * @vmax_mv: peak voltage of the haptics output waveform
+ * @length_us: vibration play duration
+ * @amplitude: DIRECT_PLAY mode output waveform amplitude
+ * @play_rate: FIFO mode play rate
+ * @fifo_data: 8-bit data samples consumed in FIFO mode
+ * @data_len: length of the FIFO data samples
+ */
+struct qcom_haptics_effect {
+ enum qcom_haptics_mode mode;
+ u32 vmax_mv;
+ u32 length_us;
+
+ u8 amplitude;
+
+ enum qcom_haptics_play_rate play_rate;
+ s8 *fifo_data;
+ u32 data_len;
+};
+
+/**
+ * struct haptics_play_req: A haptics play request
+ * @node: list node of the request
+ * @effect_id: effect index of the request
+ * @play: flag of starting or stopping the play
+ */
+struct haptics_play_req {
+ struct list_head node;
+ int effect_id;
+ bool play;
+};
+
+/**
+ * struct qcom_haptics
+ * @dev: underlying SPMI device
+ * @regmap: regmap for SPMI register access
+ * @input: input device exposing the FF interface
+ * @cfg_base: base address of the CFG peripheral
+ * @ptn_base: base address of the PTN peripheral
+ * @t_lra_us: LRA resonance period in microseconds
+ * @vmax_mv: maximum actuator drive voltage in millivolts
+ * @fifo_len: programmed HW FIFO depth in bytes
+ * @gain: playback gain scaler
+ * @play_work: delayed work that plays the queued requests
+ * @play_lock: mutex lock to serialize playbacks
+ * @play_queue_lock: spinlock protecting @play_queue
+ * @play_queue: The list of pending start/stop requests
+ * @active_effect_id: index into @effects[] currently under play
+ * @active_mode: mode of the effect currently armed in hardware
+ * @fifo_empty_irq: IRQ number for the FIFO-empty interrupt
+ * @pm_ref_held: true while a pm_runtime_get is held
+ * @irq_enabled: true if fifo_empty_irq is enabled
+ * @fifo_lock: mutex protecting the FIFO streaming data
+ * @fifo_data: pointer of the data buffer for FIFO streaming
+ * @data_len: length of the data buffer for current effect
+ * @data_written: number of samples written to the hardware FIFO
+ * @effects: table of the effects
+ */
+struct qcom_haptics {
+ struct device *dev;
+ struct regmap *regmap;
+ struct input_dev *input;
+
+ u32 cfg_base;
+ u32 ptn_base;
+ u32 t_lra_us;
+ u32 vmax_mv;
+ u32 fifo_len;
+ atomic_t gain;
+
+ struct delayed_work play_work;
+ struct mutex play_lock; /* mutex used to serialize playbacks */
+
+ spinlock_t play_queue_lock; /* protects play_queue */
+ struct list_head play_queue;
+
+ int active_effect_id;
+ enum qcom_haptics_mode active_mode;
+
+ int fifo_empty_irq;
+ bool pm_ref_held;
+ bool irq_enabled;
+
+ struct mutex fifo_lock; /* protect the FIFO data during play */
+ const s8 *fifo_data;
+ u32 data_len;
+ u32 data_written;
+
+ struct qcom_haptics_effect effects[HAPTICS_MAX_EFFECTS];
+};
+
+static int cfg_write(struct qcom_haptics *h, u32 off, u32 val)
+{
+ return regmap_write(h->regmap, h->cfg_base + off, val);
+}
+
+static int cfg_update_bits(struct qcom_haptics *h, u32 off, u32 mask, u32 val)
+{
+ return regmap_update_bits(h->regmap, h->cfg_base + off, mask, val);
+}
+
+static int ptn_write(struct qcom_haptics *h, u32 off, u32 val)
+{
+ return regmap_write(h->regmap, h->ptn_base + off, val);
+}
+
+static int ptn_update_bits(struct qcom_haptics *h, u32 off, u32 mask, u32 val)
+{
+ return regmap_update_bits(h->regmap, h->ptn_base + off, mask, val);
+}
+
+static int ptn_bulk_write(struct qcom_haptics *h, u32 off,
+ const void *buf, size_t count)
+{
+ return regmap_bulk_write(h->regmap, h->ptn_base + off, buf, count);
+}
+
+static int haptics_clear_faults(struct qcom_haptics *h)
+{
+ return cfg_write(h, HAP_CFG_FAULT_CLR_REG, FAULT_CLR_ALL);
+}
+
+static int haptics_set_vmax(struct qcom_haptics *h, u32 vmax_mv)
+{
+ return cfg_write(h, HAP_CFG_VMAX_REG, vmax_mv / VMAX_STEP_MV);
+}
+
+static int haptics_config_lra_period(struct qcom_haptics *h)
+{
+ u32 tmp = h->t_lra_us / TLRA_STEP_US;
+ int ret;
+
+ ret = cfg_write(h, HAP_CFG_TLRA_OL_HIGH_REG, (tmp >> 8) & TLRA_OL_MSB_MASK);
+ if (ret)
+ return ret;
+
+ return cfg_write(h, HAP_CFG_TLRA_OL_LOW_REG, tmp & 0xFF);
+}
+
+static int haptics_enable_module(struct qcom_haptics *h, bool enable)
+{
+ return cfg_update_bits(h, HAP_CFG_EN_CTL_REG, HAPTICS_EN_BIT,
+ enable ? HAPTICS_EN_BIT : 0);
+}
+
+static int haptics_configure_autores(struct qcom_haptics *h)
+{
+ int ret;
+
+ /* AUTORES_CFG: enable, 10-cycle delay, 25% error window */
+ ret = cfg_write(h, HAP_CFG_AUTORES_CFG_REG,
+ AUTORES_EN_BIT |
+ FIELD_PREP(AUTORES_EN_DLY_MASK, AUTORES_EN_DLY_CYCLES) |
+ FIELD_PREP(AUTORES_ERR_WIN_MASK, AUTORES_ERR_WIN_25PCT));
+ if (ret)
+ return ret;
+
+ /* DRV_DUTY: adaptive drive/brake duty cycles at 62.5% */
+ ret = cfg_write(h, HAP_CFG_DRV_DUTY_CFG_REG,
+ ADT_DRV_DUTY_EN_BIT | ADT_BRK_DUTY_EN_BIT |
+ FIELD_PREP(DRV_DUTY_MASK, AUTORES_DRV_DUTY_62P5) |
+ FIELD_PREP(BRK_DUTY_MASK, AUTORES_BRK_DUTY_62P5));
+ if (ret)
+ return ret;
+
+ /* Pre-HIZ delay: 10 µs */
+ ret = cfg_write(h, HAP_CFG_RAMP_DN_CFG2_REG, AUTORES_PRE_HIZ_DLY_10US);
+ if (ret)
+ return ret;
+
+ /* Zero-cross window: debounce 3, no hysteresis, height 2 */
+ return cfg_write(h, HAP_CFG_ZX_WIND_CFG_REG,
+ FIELD_PREP(ZX_DEBOUNCE_MASK, AUTORES_ZX_DEBOUNCE) |
+ FIELD_PREP(ZX_WIN_HEIGHT_MASK, AUTORES_ZX_WIN_HEIGHT));
+}
+
+static int haptics_write_fifo_chunk(struct qcom_haptics *h,
+ const s8 *data, u32 len)
+{
+ u32 bulk_len = ALIGN_DOWN(len, 4);
+ int i, ret;
+
+ /*
+ * FIFO data writing supports both 4-byte bulk writes using registers
+ * [HAP_PTN_FIFO_DIN_0_REG ... HAP_PTN_FIFO_DIN_3_REG], and 1-byte writes
+ * using the HAP_PTN_FIFO_DIN_1B_REG register. The 4-byte bulk write is more
+ * efficient, so use 4-byte writes for the initial 4-byte aligned data,
+ * and 1-byte writes for any trailing remainder.
+ */
+ for (i = 0; i < bulk_len; i += 4) {
+ ret = ptn_bulk_write(h, HAP_PTN_FIFO_DIN_0_REG, &data[i], 4);
+ if (ret)
+ return ret;
+ }
+
+ for (; i < len; i++) {
+ ret = ptn_write(h, HAP_PTN_FIFO_DIN_1B_REG, (u8)data[i]);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static int haptics_configure_fifo_mmap(struct qcom_haptics *h)
+{
+ u32 fifo_len, fifo_units;
+
+ /* Config all memory space for FIFO usage for now */
+ fifo_len = HAP530_MEM_TOTAL_BYTES;
+ fifo_len = ALIGN_DOWN(fifo_len, 64);
+ fifo_units = fifo_len / 64;
+ h->fifo_len = fifo_len;
+
+ return ptn_write(h, HAP_PTN_MMAP_FIFO_REG,
+ MMAP_FIFO_EXIST_BIT |
+ FIELD_PREP(MMAP_FIFO_LEN_MASK, fifo_units - 1));
+}
+
+static u32 haptics_gain_scaled_vmax(struct qcom_haptics *h, u32 vmax_mv)
+{
+ u16 gain = atomic_read(&h->gain);
+ u32 v = mult_frac(vmax_mv, gain, 0xFFFF);
+
+ return max_t(u32, v, VMAX_STEP_MV);
+}
+
+static void haptics_fifo_irq_enable(struct qcom_haptics *h, bool enable)
+{
+ if (h->irq_enabled == enable)
+ return;
+
+ if (enable)
+ enable_irq(h->fifo_empty_irq);
+ else
+ disable_irq(h->fifo_empty_irq);
+
+ h->irq_enabled = enable;
+}
+
+static int haptics_enqueue_play_req(struct qcom_haptics *h, int effect_id, bool play)
+{
+ struct haptics_play_req *req;
+
+ guard(spinlock_irqsave)(&h->play_queue_lock);
+
+ /*
+ * Coalesce with an already queued request for
+ * the same effect instead of appending.
+ */
+ list_for_each_entry(req, &h->play_queue, node) {
+ if (req->effect_id == effect_id) {
+ req->play = play;
+ return 0;
+ }
+ }
+
+ req = kmalloc_obj(*req, GFP_ATOMIC);
+ if (!req)
+ return -ENOMEM;
+
+ req->effect_id = effect_id;
+ req->play = play;
+
+ list_add_tail(&req->node, &h->play_queue);
+
+ return 0;
+}
+
+static struct haptics_play_req *haptics_dequeue_play_req(struct qcom_haptics *h)
+{
+ struct haptics_play_req *req = NULL;
+
+ guard(spinlock_irqsave)(&h->play_queue_lock);
+
+ if (!list_empty(&h->play_queue)) {
+ req = list_first_entry(&h->play_queue, struct haptics_play_req, node);
+ list_del(&req->node);
+ }
+
+ return req;
+}
+
+static bool haptics_queue_pending(struct qcom_haptics *h)
+{
+ guard(spinlock_irqsave)(&h->play_queue_lock);
+
+ return !list_empty(&h->play_queue);
+}
+
+static void haptics_queue_remove_effect(struct qcom_haptics *h, int effect_id)
+{
+ struct haptics_play_req *req, *tmp;
+
+ guard(spinlock_irqsave)(&h->play_queue_lock);
+
+ list_for_each_entry_safe(req, tmp, &h->play_queue, node) {
+ if (req->effect_id == effect_id) {
+ list_del(&req->node);
+ kfree(req);
+ }
+ }
+}
+
+static void haptics_queue_flush(struct qcom_haptics *h)
+{
+ struct haptics_play_req *req, *tmp;
+
+ guard(spinlock_irqsave)(&h->play_queue_lock);
+
+ list_for_each_entry_safe(req, tmp, &h->play_queue, node) {
+ list_del(&req->node);
+ kfree(req);
+ }
+}
+
+static int haptics_start_direct_play(struct qcom_haptics *h, int effect_id)
+{
+ struct qcom_haptics_effect *eff = &h->effects[effect_id];
+ int ret;
+
+ ret = haptics_clear_faults(h);
+ if (ret)
+ return ret;
+
+ /* Enable auto-resonance for DIRECT_PLAY mode */
+ ret = cfg_update_bits(h, HAP_CFG_AUTORES_CFG_REG,
+ AUTORES_EN_BIT, AUTORES_EN_BIT);
+ if (ret)
+ return ret;
+
+ ret = haptics_set_vmax(h, haptics_gain_scaled_vmax(h, h->vmax_mv));
+ if (ret)
+ return ret;
+
+ ret = ptn_write(h, HAP_PTN_DIRECT_PLAY_REG, eff->amplitude);
+ if (ret)
+ return ret;
+
+ return cfg_write(h, HAP_CFG_SPMI_PLAY_REG,
+ PLAY_EN_BIT | FIELD_PREP(PAT_SRC_MASK, PAT_SRC_DIRECT_PLAY));
+}
+
+static int haptics_start_fifo(struct qcom_haptics *h, int effect_id)
+{
+ struct qcom_haptics_effect *eff = &h->effects[effect_id];
+ u32 vmax = eff->vmax_mv ? eff->vmax_mv : h->vmax_mv;
+ u32 init_len;
+ bool data_done;
+ int ret;
+
+ if (!eff->fifo_data || !eff->data_len)
+ return -EINVAL;
+
+ ret = haptics_clear_faults(h);
+ if (ret)
+ return ret;
+
+ /* Disable auto-resonance for FIFO mode */
+ ret = cfg_update_bits(h, HAP_CFG_AUTORES_CFG_REG, AUTORES_EN_BIT, 0);
+ if (ret)
+ return ret;
+
+ ret = haptics_set_vmax(h, haptics_gain_scaled_vmax(h, vmax));
+ if (ret)
+ return ret;
+
+ ret = ptn_update_bits(h, HAP_PTN_FIFO_PLAY_RATE_REG,
+ FIFO_PLAY_RATE_MASK,
+ FIELD_PREP(FIFO_PLAY_RATE_MASK, eff->play_rate));
+ if (ret)
+ return ret;
+
+ /* Flush FIFO before loading new data */
+ ret = ptn_write(h, HAP_PTN_MEM_OP_ACCESS_REG, MEM_FLUSH_RELOAD_BIT);
+ if (ret)
+ return ret;
+ ret = ptn_write(h, HAP_PTN_MEM_OP_ACCESS_REG, 0);
+ if (ret)
+ return ret;
+
+ guard(mutex)(&h->fifo_lock);
+
+ /* Write the initial chunk and initialise streaming state */
+ init_len = min_t(u32, eff->data_len, FIFO_INIT_FILL);
+ ret = haptics_write_fifo_chunk(h, eff->fifo_data, init_len);
+ if (ret)
+ return ret;
+
+ h->fifo_data = eff->fifo_data;
+ h->data_len = eff->data_len;
+ h->data_written = init_len;
+
+ /*
+ * Set empty threshold. When threshold > 0 the hardware fires the
+ * FIFO-empty interrupt when occupancy drops below the threshold,
+ * allowing the driver to refill.
+ */
+ data_done = (h->data_written >= h->data_len);
+ if (!data_done) {
+ ret = ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG,
+ FIFO_EMPTY_THRESH / FIFO_THRESH_LSB);
+ if (ret) {
+ dev_err(h->dev, "set FIFO empty threshold failed, ret=%d\n", ret);
+ h->fifo_data = NULL;
+ return ret;
+ }
+
+ haptics_fifo_irq_enable(h, true);
+ }
+
+ ret = cfg_write(h, HAP_CFG_SPMI_PLAY_REG,
+ PLAY_EN_BIT | FIELD_PREP(PAT_SRC_MASK, PAT_SRC_FIFO));
+ if (ret) {
+ dev_err(h->dev, "trigger FIFO play failed, ret=%d\n", ret);
+ ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
+ /*
+ * HW play never started since this SPMI write itself failed,
+ * and the FIFO empty IRQ has never fired, so disabling the
+ * IRQ synchronously here can't deadlock against 'fifo_lock'.
+ */
+ haptics_fifo_irq_enable(h, false);
+ h->fifo_data = NULL;
+ return ret;
+ }
+
+ return 0;
+}
+
+/*
+ * haptics_fifo_empty_irq: Threaded IRQ handler for the FIFO-empty interrupt.
+ *
+ * While a FIFO play is in progress the hardware fires this interrupt when
+ * the number of samples in the FIFO drops below the programmed threshold.
+ * The handler refills the FIFO from the effect's data buffer. When all
+ * samples have been written the threshold is set to zero. The HW would
+ * stop the play automatically after all of the samples in FIFO memory are
+ * played out.
+ */
+static irqreturn_t haptics_fifo_empty_irq(int irq, void *dev_id)
+{
+ struct qcom_haptics *h = dev_id;
+ u32 sts, to_write;
+ int ret;
+
+ ret = regmap_read(h->regmap,
+ h->cfg_base + HAP_CFG_INT_RT_STS_REG, &sts);
+ if (ret || !(sts & FIFO_EMPTY_BIT))
+ return IRQ_HANDLED;
+
+ guard(mutex)(&h->fifo_lock);
+
+ if (!h->fifo_data)
+ return IRQ_HANDLED;
+
+ /* Refill: write the next chunk */
+ to_write = min_t(u32, h->data_len - h->data_written,
+ h->fifo_len - FIFO_EMPTY_THRESH);
+ ret = haptics_write_fifo_chunk(h, &h->fifo_data[h->data_written], to_write);
+ if (ret) {
+ dev_err(h->dev, "refill FIFO samples failed, ret=%d\n", ret);
+ /*
+ * If data refilling is failed,stop the HW play and disable the
+ * IRQ to prevent the FIFO empty IRQ being fired continuously.
+ */
+ cfg_write(h, HAP_CFG_SPMI_PLAY_REG, 0);
+ disable_irq_nosync(h->fifo_empty_irq);
+ h->irq_enabled = false;
+ return IRQ_HANDLED;
+ }
+
+ h->data_written += to_write;
+
+ /* Disable the interrupt after all the data is queued */
+ if (h->data_written >= h->data_len)
+ ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
+
+ return IRQ_HANDLED;
+}
+
+/*
+ * haptics_stop_locked: stop play in HW and put runtime PM
+ * @h: haptics device
+ * @put_noidle: a flag to put noidle
+ *
+ * Stop HW play, clear the FIFO data if the active effect was FIFO-mode, and
+ * put runtime PM into either noidle or autosuspend based on put_noidle flag.
+ *
+ * Must be called with play_lock held.
+ */
+static void haptics_stop_locked(struct qcom_haptics *h, bool put_noidle)
+{
+ cfg_write(h, HAP_CFG_SPMI_PLAY_REG, 0);
+
+ if (h->active_mode == HAPTICS_FIFO) {
+ ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
+ haptics_fifo_irq_enable(h, false);
+ scoped_guard(mutex, &h->fifo_lock) {
+ h->fifo_data = NULL;
+ }
+ }
+
+ h->active_effect_id = -1;
+ h->active_mode = HAPTICS_MODE_NONE;
+
+ if (h->pm_ref_held) {
+ if (put_noidle)
+ pm_runtime_put_noidle(h->dev);
+ else
+ pm_runtime_put_autosuspend(h->dev);
+
+ h->pm_ref_held = false;
+ }
+}
+
+/*
+ * haptics_start_locked: start to play an effect
+ * @h: haptics device
+ * @effect_id: the index of the effect
+ *
+ * If an effect is currently active, stop it 1st. Acquires a PM ref
+ * if not already held, and then trigger the play based on the
+ * play mode. Update active_effect_id/active_mode on success.
+ *
+ * Must be called with play_lock held.
+ */
+static int haptics_start_locked(struct qcom_haptics *h, int effect_id)
+{
+ int ret;
+
+ if (h->active_effect_id != -1)
+ haptics_stop_locked(h, false);
+
+ if (!h->pm_ref_held) {
+ ret = pm_runtime_resume_and_get(h->dev);
+ if (ret < 0) {
+ dev_err(h->dev, "failed to resume device: %d\n", ret);
+ return ret;
+ }
+
+ h->pm_ref_held = true;
+ }
+
+ switch (h->effects[effect_id].mode) {
+ case HAPTICS_DIRECT_PLAY:
+ ret = haptics_start_direct_play(h, effect_id);
+ break;
+ case HAPTICS_FIFO:
+ ret = haptics_start_fifo(h, effect_id);
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+ if (ret) {
+ dev_err(h->dev, "failed to start effect %d: %d\n", effect_id, ret);
+ if (h->pm_ref_held) {
+ pm_runtime_put_autosuspend(h->dev);
+ h->pm_ref_held = false;
+ }
+
+ return ret;
+ }
+
+ h->active_effect_id = effect_id;
+ h->active_mode = h->effects[effect_id].mode;
+
+ return 0;
+}
+
+static void haptics_play_work(struct work_struct *work)
+{
+ struct qcom_haptics *h = container_of(to_delayed_work(work),
+ struct qcom_haptics, play_work);
+ struct haptics_play_req *req = haptics_dequeue_play_req(h);
+ bool rearmed = false;
+ u32 length_us;
+ int ret;
+
+ guard(mutex)(&h->play_lock);
+
+ /* No valid request available */
+ if (req == NULL)
+ return;
+
+ if (req->play) {
+ ret = haptics_start_locked(h, req->effect_id);
+ if (ret) {
+ dev_err(h->dev, "play haptics failed, ret=%d\n", ret);
+ } else {
+ /*
+ * Re-arm the work to stop the play or advance
+ * to next play after the play duration
+ */
+ length_us = h->effects[req->effect_id].length_us;
+ if (length_us) {
+ schedule_delayed_work(&h->play_work,
+ usecs_to_jiffies(length_us));
+ rearmed = true;
+ }
+ }
+ } else {
+ if (req->effect_id != h->active_effect_id)
+ dev_warn(h->dev, "effect %d is not under playing\n", req->effect_id);
+ else
+ haptics_stop_locked(h, false);
+ }
+
+ kfree(req);
+
+ /*
+ * Handle the remaining requests if doesn't need to wait
+ */
+ if (!rearmed && haptics_queue_pending(h))
+ schedule_delayed_work(&h->play_work, 0);
+}
+
+static void haptics_clear_effect(struct qcom_haptics *h,
+ struct qcom_haptics_effect *effect)
+{
+ guard(mutex)(&h->fifo_lock);
+
+ if (h->fifo_data == effect->fifo_data)
+ h->fifo_data = NULL;
+
+ kvfree(effect->fifo_data);
+ effect->fifo_data = NULL;
+ effect->data_len = 0;
+}
+
+/*
+ * haptics_fifo_length_us: Calculate the play duration of a FIFO effect.
+ * @h: haptics device pointer
+ * @rate: FIFO data play rate
+ * @data_len: FFO data length
+ *
+ * Each FIFO sample is played out over one play-rate period, so the total
+ * duration is the number of samples times that period:
+ *
+ * length_us = (clk_base * data_len * multiplier) / divider
+ *
+ * For T_LRA-based rates the period derives from the LRA resonance period
+ * (h->t_lra_us): DIV_2/4/8 shorten it, X_2/4/8 lengthen it. For the
+ * kHz-based rates the period is 1000 us / freq_khz.
+ */
+static u32 haptics_fifo_length_us(struct qcom_haptics *h,
+ enum qcom_haptics_play_rate rate,
+ u32 data_len)
+{
+ u32 clk_base = 1000, multiplier = 1, divider = 1;
+
+ switch (rate) {
+ case PLAY_RATE_T_LRA:
+ clk_base = h->t_lra_us;
+ break;
+ case PLAY_RATE_T_LRA_DIV_2:
+ clk_base = h->t_lra_us;
+ divider = 2;
+ break;
+ case PLAY_RATE_T_LRA_DIV_4:
+ clk_base = h->t_lra_us;
+ divider = 4;
+ break;
+ case PLAY_RATE_T_LRA_DIV_8:
+ clk_base = h->t_lra_us;
+ divider = 8;
+ break;
+ case PLAY_RATE_T_LRA_X_2:
+ clk_base = h->t_lra_us;
+ multiplier = 2;
+ break;
+ case PLAY_RATE_T_LRA_X_4:
+ clk_base = h->t_lra_us;
+ multiplier = 4;
+ break;
+ case PLAY_RATE_T_LRA_X_8:
+ clk_base = h->t_lra_us;
+ multiplier = 8;
+ break;
+ case PLAY_RATE_F_8KHZ:
+ divider = 8;
+ break;
+ case PLAY_RATE_F_16KHZ:
+ divider = 16;
+ break;
+ case PLAY_RATE_F_24KHZ:
+ divider = 24;
+ break;
+ case PLAY_RATE_F_32KHZ:
+ divider = 32;
+ break;
+ case PLAY_RATE_F_44P1KHZ:
+ clk_base *= 10;
+ divider = 441;
+ break;
+ case PLAY_RATE_F_48KHZ:
+ divider = 48;
+ break;
+ default:
+ /* Unexpected rate: fall back to the resonance period. */
+ clk_base = h->t_lra_us;
+ break;
+ }
+
+ return div_u64((u64)clk_base * data_len * multiplier, divider);
+}
+
+static int haptics_upload_effect(struct input_dev *dev,
+ struct ff_effect *effect,
+ struct ff_effect *old)
+{
+ struct qcom_haptics *h = input_get_drvdata(dev);
+ struct qcom_haptics_effect *priv;
+ int id = effect->id;
+ u32 data_len, level;
+ s8 *fifo;
+
+ s16 *buf __free(kvfree) = NULL;
+
+ guard(mutex)(&h->play_lock);
+
+ if (id < 0 || id >= HAPTICS_MAX_EFFECTS)
+ return -EINVAL;
+
+ if (id == h->active_effect_id) {
+ dev_err(h->dev, "effect %d is under playing\n", id);
+ return -EBUSY;
+ }
+
+ priv = &h->effects[id];
+
+ switch (effect->type) {
+ case FF_CONSTANT:
+ haptics_clear_effect(h, priv);
+ level = effect->u.constant.level <= 0 ? 0 : effect->u.constant.level;
+ priv->amplitude = (u8)mult_frac(level, 255, 0x7FFF);
+ priv->length_us = effect->replay.length * USEC_PER_MSEC;
+ priv->mode = HAPTICS_DIRECT_PLAY;
+ return 0;
+
+ case FF_PERIODIC:
+ if (effect->u.periodic.waveform != FF_CUSTOM)
+ return -EINVAL;
+ /*
+ * Minimum 3 elements: play-rate code + vmax + at least one sample.
+ * Limit the maximum data length to ~48K so that it can at least
+ * handle ~1s vibration at the fast (48K) play rate.
+ */
+ if (effect->u.periodic.custom_len < 3 ||
+ effect->u.periodic.custom_len > CUSTOM_DATA_MAX_LEN + 2)
+ return -EINVAL;
+
+ buf = vmemdup_array_user(effect->u.periodic.custom_data,
+ effect->u.periodic.custom_len,
+ sizeof(s16));
+ if (IS_ERR(buf))
+ return PTR_ERR(no_free_ptr(buf));
+
+ if (buf[CUSTOM_DATA_RATE_IDX] < 0 ||
+ buf[CUSTOM_DATA_RATE_IDX] > PLAY_RATE_MAX ||
+ buf[CUSTOM_DATA_RATE_IDX] == PLAY_RATE_RESERVED)
+ return -EINVAL;
+
+ data_len = effect->u.periodic.custom_len - CUSTOM_DATA_SAMPLE_START;
+
+ fifo = kvmalloc(data_len, GFP_KERNEL);
+ if (!fifo)
+ return -ENOMEM;
+
+ /* Pack: one s8 sample per s16 slot (lower byte) */
+ for (int i = 0; i < data_len; i++)
+ fifo[i] = (s8)buf[CUSTOM_DATA_SAMPLE_START + i];
+
+ haptics_clear_effect(h, priv);
+
+ scoped_guard(mutex, &h->fifo_lock) {
+ priv->fifo_data = fifo;
+ priv->data_len = data_len;
+ }
+
+ priv->play_rate = (u8)buf[CUSTOM_DATA_RATE_IDX];
+ priv->vmax_mv = (u32)clamp_val(buf[CUSTOM_DATA_VMAX_IDX], 0, VMAX_MV_MAX);
+ priv->length_us = haptics_fifo_length_us(h, priv->play_rate, data_len);
+ priv->mode = HAPTICS_FIFO;
+
+ return 0;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static int haptics_playback(struct input_dev *dev, int effect_id, int val)
+{
+ struct qcom_haptics *h = input_get_drvdata(dev);
+ int ret;
+
+ ret = haptics_enqueue_play_req(h, effect_id, val > 0);
+ if (ret)
+ return ret;
+
+ if (val > 0)
+ /*
+ * Queue the play. If a duration re-arm is already pending this
+ * is a no-op, so the new play waits for the current effect to
+ * finish before the worker dequeues it.
+ */
+ schedule_delayed_work(&h->play_work, 0);
+ else
+ /*
+ * Run the worker now, cancelling any pending duration re-arm,
+ * so an explicit stop takes effect immediately.
+ */
+ mod_delayed_work(system_percpu_wq, &h->play_work, 0);
+
+ return 0;
+}
+
+static int haptics_erase(struct input_dev *dev, int effect_id)
+{
+ struct qcom_haptics *h = input_get_drvdata(dev);
+ struct qcom_haptics_effect *priv = &h->effects[effect_id];
+
+ guard(mutex)(&h->play_lock);
+
+ haptics_queue_remove_effect(h, effect_id);
+
+ if (h->active_effect_id == effect_id)
+ haptics_stop_locked(h, false);
+
+ haptics_clear_effect(h, priv);
+ priv->mode = HAPTICS_MODE_NONE;
+
+ return 0;
+}
+
+static void haptics_set_gain(struct input_dev *dev, u16 gain)
+{
+ struct qcom_haptics *h = input_get_drvdata(dev);
+
+ atomic_set(&h->gain, gain);
+}
+
+static int qcom_haptics_probe(struct platform_device *pdev)
+{
+ struct qcom_haptics *h;
+ struct input_dev *input;
+ struct ff_device *ff;
+ u32 regs[2], vmax_uv;
+ int ret, irq;
+
+ h = devm_kzalloc(&pdev->dev, sizeof(*h), GFP_KERNEL);
+ if (!h)
+ return -ENOMEM;
+
+ h->dev = &pdev->dev;
+
+ h->regmap = dev_get_regmap(pdev->dev.parent, NULL);
+ if (!h->regmap)
+ return dev_err_probe(h->dev, -ENODEV, "no regmap from parent\n");
+
+ ret = device_property_read_u32_array(h->dev, "reg", regs, ARRAY_SIZE(regs));
+ if (ret)
+ return dev_err_probe(h->dev, ret, "failed to read 'reg' property\n");
+
+ h->cfg_base = regs[0];
+ h->ptn_base = regs[1];
+
+ ret = device_property_read_u32(h->dev, "qcom,lra-period-us", &h->t_lra_us);
+ if (ret)
+ return dev_err_probe(h->dev, ret, "missing qcom,lra-period-us\n");
+
+ h->t_lra_us = clamp(h->t_lra_us, (u32)TLRA_STEP_US, (u32)TLRA_US_MAX);
+
+ ret = device_property_read_u32(h->dev, "qcom,vmax-microvolt", &vmax_uv);
+ if (ret)
+ return dev_err_probe(h->dev, ret, "missing qcom,vmax-microvolt\n");
+
+ h->vmax_mv = clamp(vmax_uv / 1000, (u32)VMAX_STEP_MV, (u32)VMAX_MV_MAX);
+
+ ret = haptics_config_lra_period(h);
+ if (ret)
+ return ret;
+
+ ret = haptics_configure_autores(h);
+ if (ret)
+ return ret;
+
+ ret = haptics_set_vmax(h, h->vmax_mv);
+ if (ret)
+ return ret;
+
+ ret = haptics_configure_fifo_mmap(h);
+ if (ret)
+ return ret;
+
+ mutex_init(&h->play_lock);
+ mutex_init(&h->fifo_lock);
+ spin_lock_init(&h->play_queue_lock);
+ INIT_LIST_HEAD(&h->play_queue);
+ INIT_DELAYED_WORK(&h->play_work, haptics_play_work);
+ atomic_set(&h->gain, 0xFFFF);
+ h->active_effect_id = -1;
+
+ irq = platform_get_irq_byname(pdev, "fifo-empty");
+ if (irq < 0)
+ return dev_err_probe(h->dev, irq, "failed to get fifo-empty IRQ\n");
+
+ ret = devm_request_threaded_irq(h->dev, irq, NULL,
+ haptics_fifo_empty_irq,
+ IRQF_ONESHOT | IRQF_NO_AUTOEN,
+ "qcom-haptics-fifo-empty", h);
+ if (ret)
+ return dev_err_probe(h->dev, ret, "failed to request fifo-empty IRQ\n");
+
+ h->fifo_empty_irq = irq;
+ platform_set_drvdata(pdev, h);
+
+ pm_runtime_use_autosuspend(h->dev);
+ pm_runtime_set_autosuspend_delay(h->dev, HAPTICS_AUTOSUSPEND_MS);
+ ret = devm_pm_runtime_enable(h->dev);
+ if (ret)
+ return dev_err_probe(h->dev, ret, "enable runtime PM failed\n");
+
+ input = devm_input_allocate_device(h->dev);
+ if (!input)
+ return -ENOMEM;
+
+ input->name = "qcom-spmi-haptics";
+ input_set_drvdata(input, h);
+ h->input = input;
+
+ input_set_capability(input, EV_FF, FF_CONSTANT);
+ input_set_capability(input, EV_FF, FF_PERIODIC);
+ input_set_capability(input, EV_FF, FF_CUSTOM);
+ input_set_capability(input, EV_FF, FF_GAIN);
+
+ ret = input_ff_create(input, HAPTICS_MAX_EFFECTS);
+ if (ret)
+ return ret;
+
+ ff = input->ff;
+ ff->upload = haptics_upload_effect;
+ ff->playback = haptics_playback;
+ ff->erase = haptics_erase;
+ ff->set_gain = haptics_set_gain;
+
+ ret = input_register_device(input);
+ if (ret) {
+ input_ff_destroy(input);
+ return dev_err_probe(h->dev, ret, "failed to register input device\n");
+ }
+
+ return 0;
+}
+
+static void qcom_haptics_remove(struct platform_device *pdev)
+{
+ struct qcom_haptics *h = platform_get_drvdata(pdev);
+
+ /*
+ * Unregister the input device explicitly at the beginning
+ * to avoid the input device being used after the resources
+ * are freed.
+ */
+ input_unregister_device(h->input);
+ disable_delayed_work_sync(&h->play_work);
+ scoped_guard(mutex, &h->play_lock) {
+ haptics_queue_flush(h);
+ haptics_stop_locked(h, true);
+ }
+
+ haptics_enable_module(h, false);
+
+ scoped_guard(mutex, &h->fifo_lock) {
+ for (int i = 0; i < HAPTICS_MAX_EFFECTS; i++) {
+ kvfree(h->effects[i].fifo_data);
+ h->effects[i].fifo_data = NULL;
+ }
+ }
+}
+
+static int qcom_haptics_runtime_suspend(struct device *dev)
+{
+ struct qcom_haptics *h = dev_get_drvdata(dev);
+
+ return haptics_enable_module(h, false);
+}
+
+static int qcom_haptics_runtime_resume(struct device *dev)
+{
+ struct qcom_haptics *h = dev_get_drvdata(dev);
+
+ return haptics_enable_module(h, true);
+}
+
+static int qcom_haptics_suspend(struct device *dev)
+{
+ struct qcom_haptics *h = dev_get_drvdata(dev);
+ int ret;
+
+ disable_delayed_work_sync(&h->play_work);
+ scoped_guard(mutex, &h->play_lock) {
+ haptics_queue_flush(h);
+ haptics_stop_locked(h, true);
+ }
+
+ ret = pm_runtime_force_suspend(dev);
+ if (ret)
+ enable_delayed_work(&h->play_work);
+
+ return ret;
+}
+
+static int qcom_haptics_resume(struct device *dev)
+{
+ struct qcom_haptics *h = dev_get_drvdata(dev);
+
+ enable_delayed_work(&h->play_work);
+
+ return pm_runtime_force_resume(dev);
+}
+
+static const struct dev_pm_ops qcom_haptics_pm_ops = {
+ SYSTEM_SLEEP_PM_OPS(qcom_haptics_suspend, qcom_haptics_resume)
+ RUNTIME_PM_OPS(qcom_haptics_runtime_suspend, qcom_haptics_runtime_resume,
+ NULL)
+};
+
+static const struct of_device_id qcom_haptics_of_match[] = {
+ { .compatible = "qcom,spmi-haptics" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, qcom_haptics_of_match);
+
+static struct platform_driver qcom_haptics_driver = {
+ .probe = qcom_haptics_probe,
+ .remove = qcom_haptics_remove,
+ .driver = {
+ .name = "qcom-spmi-haptics",
+ .of_match_table = qcom_haptics_of_match,
+ .pm = pm_ptr(&qcom_haptics_pm_ops),
+ },
+};
+module_platform_driver(qcom_haptics_driver);
+
+MODULE_DESCRIPTION("Qualcomm SPMI PMIC Haptics driver");
+MODULE_LICENSE("GPL");
--
2.43.0
^ permalink raw reply related
* [PATCH v4 2/3] dt-bindings: mfd: qcom,spmi-pmic: Document haptics device
From: Fenglin Wu @ 2026-07-17 7:29 UTC (permalink / raw)
To: linux-arm-msm, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lee Jones, Stephen Boyd, Bjorn Andersson,
Konrad Dybcio
Cc: David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel, Fenglin Wu,
Krzysztof Kozlowski
In-Reply-To: <20260717-qcom-spmi-haptics-v4-0-b0fe0ed30849@oss.qualcomm.com>
Some of the Qualcomm SPMI PMIC has haptics device in it, add it in the
device list.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
index 644c42b5e2e5..773f4cba5935 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
@@ -165,6 +165,10 @@ patternProperties:
type: object
$ref: /schemas/pinctrl/qcom,pmic-gpio.yaml#
+ "^haptics@[0-9a-f]+$":
+ type: object
+ $ref: /schemas/input/qcom,spmi-haptics.yaml#
+
"^led-controller@[0-9a-f]+$":
type: object
$ref: /schemas/leds/qcom,spmi-flash-led.yaml#
--
2.43.0
^ permalink raw reply related
* [PATCH v4 0/3] input: misc: Add an initial driver for haptics inside Qcom PMIH010x PMIC
From: Fenglin Wu @ 2026-07-17 7:28 UTC (permalink / raw)
To: linux-arm-msm, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lee Jones, Stephen Boyd, Bjorn Andersson,
Konrad Dybcio
Cc: David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel, Fenglin Wu,
Krzysztof Kozlowski
Dependencies:
- [patch 2/3] depends on [patch 1/3] and they should be applied together
Qualcomm PMIH0108 PMIC has a haptics module inside and it could drive
a LRA actuator with several play modes, including: DIRECT_PLAY, FIFO,
PAT_MEM, SWR, etc. Add an initial driver to support two of the play
modes using the input force-feedback framework:
-- FF_CONSTANT effect for DIRECT_PLAY mode which drives sinusoidual
waveforms with fixed period and amplitude, which would generate
a constant vibration effect on the LRA actuator.
-- FF_PERIODIC effect with FF_CUSTOM for FIFO streaming mode, which
can play an arbitrary waveform composed of a sequence of 8-bit
samples at a configurable play rate.
Also, add the device node in the existing pmih0108 dtsi files, and enble
the haptics device for several boards by updating the vmax and
lra-period sttings according to the LRA components that mounted on each
of them.
Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
Changes in v4:
Fixed Sashiko AI review comments in the driver:
- In haptics_start_fifo(), remove the goto and do the clear in the error
paths directly, also add a comment to explain that disabling the IRQ
synchronously won't cause a deadlock.
- When data refilling failed in IRQ handler, stop the play and disable
the IRQ to avoid the potential IRQ storming issue
- in play_work(), check if the effect_id in the request is the one which
under playing before stopping the play.
- Link to v3: https://patch.msgid.link/20260713-qcom-spmi-haptics-v3-0-c931bb7cb94f@oss.qualcomm.com
Also, add trailers in the binding changes.
Changes in v3:
In the binding:
- Removed the ref for qcom,vmax-microvolt as the property with standard
unit already has a ref in dtschema
- Added 'qcom,pmih0108-haptics' as a device-specific compatible
In the driver, fixed Sashiko AI review comments with below changes:
- Added a list to queue and serialize all of the request, which helps to
avoid the races between playback(), which is protected by evdev's event_lock,
and play_work(), which is protected by play_lock.
- Changed to use guard(mutex) or scoped_guard(mutex, ) for cleaner mutex logic
usages, and update protection section to prevent race conditions.
- Added runtime pm control in haptics_stop() function, use it as an unified interface
under the guard of 'play_lock' in play_work()/erase()/remove()/suspend(),
to ensure the runtime pm control correctness in race conditions.
- Removed unnecessary stop play sequence in fifo_empty_irq() as the HW would
automatically stop after the FIFO samples are played out.
- Added a common interface haptics_clear_effect() to clear the FIFO data with gaurd
of 'fifo_lock', and use it before upload() and in erase() to prevent race
condition.
- used __free() for safe memory cleanup
- Checked play_rate against negative value when loading FF_PERIOD effect
- Limited the custom_data length to 48K prevent potential OOM
- Link to v2: https://patch.msgid.link/20260624-qcom-spmi-haptics-v2-0-b9118e60f3e3@oss.qualcomm.com
Changes in v2:
Dropped dtsi change and I will resend them after the driver and binding changes get accepted.
Updated haptics binding and addressed review comments from Krzysztof and Konrad:
- Extended the description to clarify the 'PAT_MEM' mode (not yet supported in the driver)
by comparing it with the 'FIFO' mode.
- Updated the compatible string to 'qcom,spmi-haptics' to match the file name and removed
the PMIC wildcard.
- Simplified register names to 'cfg' and 'ptn'.
- Corrected the unit naming for the 'qcom,vmax-microvolt' property.
- Added an additional clarification for the 'qcom,lra-period-us' property.
Updated the driver to address review comments from Konrad and Julian:
- In haptics_write_fifo_chunk(), separated variable declaration and assignment, and added
comments explaining the 4-byte and 1-byte FIFO writes.
- Replaced manual 'x * n / d' calculations with mult_frac().
- Switched to disable_irq() to prevent late IRQs after device removal.
- Replaced property reads with device_property_read_u32().
- Remove the 'INPUT' dependency in Kconfig
Updated the driver to address feedback from Sashiko AI:
- Guarded pm_runtime_resume()/suspend() with 'pm_ref_held' to prevent runtime PM reference leaks.
- Replaced spinlock with a mutex to protect FIFO data during playback and avoid calling
sleepable regmap APIs under spinlock.
- Adjusted suspend/remove() sequence to stop playback before canceling work, and freed
FIFO buffers to prevent potential memory leaks.
- In FF_PERIODIC handling, allocated 'fifo_data' before assigning data to ensure its
consistency with 'data_len'.
- Registered the input device after enabling runtime PM.
- Unify to use 'h->dev' pointer in probe()
- Link to v1: https://patch.msgid.link/20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com
---
Fenglin Wu (3):
dt-bindings: input: Add Qualcomm SPMI PMIC haptics
dt-bindings: mfd: qcom,spmi-pmic: Document haptics device
input: misc: Add Qualcomm SPMI PMIC haptics driver
.../bindings/input/qcom,spmi-haptics.yaml | 136 +++
.../devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 4 +
drivers/input/misc/Kconfig | 11 +
drivers/input/misc/Makefile | 1 +
drivers/input/misc/qcom-spmi-haptics.c | 1178 ++++++++++++++++++++
5 files changed, 1330 insertions(+)
---
base-commit: 66725039f7090afe14c31bd259e2059a68f04023
change-id: 20260616-qcom-spmi-haptics-3cc97e7b232e
Best regards,
--
Fenglin Wu <fenglin.wu@oss.qualcomm.com>
^ permalink raw reply
* [PATCH v4 1/3] dt-bindings: input: Add Qualcomm SPMI PMIC haptics
From: Fenglin Wu @ 2026-07-17 7:28 UTC (permalink / raw)
To: linux-arm-msm, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lee Jones, Stephen Boyd, Bjorn Andersson,
Konrad Dybcio
Cc: David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel, Fenglin Wu,
Krzysztof Kozlowski
In-Reply-To: <20260717-qcom-spmi-haptics-v4-0-b0fe0ed30849@oss.qualcomm.com>
Add binding document for the haptics module inside Qualcomm PMIC
PMIH0108.
Assisted-by: Claude:claude-4-8-opus
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
.../bindings/input/qcom,spmi-haptics.yaml | 136 +++++++++++++++++++++
1 file changed, 136 insertions(+)
diff --git a/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
new file mode 100644
index 000000000000..b1a69e9886ee
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
@@ -0,0 +1,136 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/input/qcom,spmi-haptics.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Haptics device inside Qualcomm Technologies, Inc. PMIC
+
+maintainers:
+ - Fenglin Wu <fenglin.wu@oss.qualcomm.com>
+
+description: |
+ Certain Qualcomm PMICs integrate a haptics module, such as the HAP530_HV haptics
+ module in the PMIH0108 PMIC, which drives an LRA (Linear Resonant Actuator) with
+ an output voltage up to 10 V. Several play modes are supported in HAP530_HV:
+
+ DIRECT_PLAY: The hardware outputs sinusoidal waveforms whose period is
+ defined by qcom,lra-period-us and whose peak voltage is defined by
+ qcom,vmax-microvolt. The driving amplitude can be scaled in the range
+ [0, 255] via a single register byte. Hardware-based LRA auto-resonance
+ tracking is enabled by default in this mode, allowing the haptics engine
+ to follow the actual resonant frequency of the LRA and update the driving
+ period accordingly to achieve stronger vibration magnitude.
+
+ FIFO: The hardware can play an arbitrary waveform composed of a sequence
+ of 8-bit samples at a configurable play rate. Samples are pre-filled
+ into the internal FIFO memory of the haptics module and continuously
+ replenished via the FIFO-empty IRQ until all samples have been played.
+ An 8K-byte FIFO memory bank is available in the HAP530_HV haptics module,
+ shared between the FIFO and PAT_MEM play modes. The memory partition
+ between the two modes is configurable via registers, and FIFO mode always
+ uses the 1st partition starting from offset 0.
+
+ PAT_MEM: This mode is very similar to FIFO streaming mode but without the
+ data refilling capability. It is designed mainly for short, latency-critical
+ vibrations. The memory space for PAT_MEM mode must be reserved for dedicated
+ usage, and the waveform data should be preloaded and remain unchanged
+ thereafter. The haptics module can play the waveform data from the memory
+ region specified by the PAT_MEM play start address and length registers.
+
+ In either FIFO mode or PAT_MEM mode, the following play rates are supported:
+ -- 0(T_LRA): each FIFO byte drives one full sinusoidal cycle with the
+ period defined in qcom,lra-period-us.
+ -- 1/2/3(T_LRA_DIV_2/4/8): each FIFO byte drives a half/quarter/eighth
+ sinusoidal cycle with the period defined in qcom,lra-period-us.
+ -- 4/5/6(T_LRA_X_2/4/8): each FIFO byte drives 2/4/8 sinusoidal cycles
+ with the period defined in qcom,lra-period-us.
+ -- 8/9/10/11/12/13(8KHz/16KHz/24KHz/32KHz/44.1KHz/48KHz): the FIFO
+ data is treated as PCM samples and drives the output with an
+ arbitrarily shaped waveform. This mode is typically used to define
+ custom driving waveforms for specific vibration effects such as fast
+ attack, crisp brake, etc.
+
+ The drive voltage in FIFO or PAT_MEM mode can exceed the value defined in
+ qcom,vmax-microvolt to achieve a special vibration effect, but the waveform
+ must be short enough to prevent the LRA from being damaged by operating at
+ an overvoltage.
+
+ Also, hardware-based LRA auto-resonance tracking is normally disabled in
+ FIFO or PAT_MEM mode, as these modes are intended to drive arbitrary
+ waveforms that may not follow the resonant frequency; autonomous hardware
+ resonance correction would interfere with the intended output.
+
+properties:
+ compatible:
+ items:
+ - const: qcom,pmih0108-haptics
+ - const: qcom,spmi-haptics
+
+ reg:
+ items:
+ - description: HAP_CFG module base address
+ - description: HAP_PTN module base address
+
+ reg-names:
+ items:
+ - const: cfg
+ - const: ptn
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-names:
+ items:
+ - const: fifo-empty
+
+ qcom,vmax-microvolt:
+ description:
+ Maximum allowed output driving voltage in microvolts, must be a multiple
+ of 50,000 uV. This is the peak driving voltage in DIRECT_PLAY mode,
+ which outputs sinusoidal waveforms. The value should be equal to the
+ square root of 2 times the Vrms voltage of the LRA.
+ minimum: 50000
+ maximum: 10000000
+ multipleOf: 50000
+
+ qcom,lra-period-us:
+ description:
+ LRA actuator initial resonance period in microseconds
+ (1,000,000 / resonant_freq_hz). Used to configure T_LRA-based play
+ rates and the auto-resonance zero-crossing window. It could be also used
+ as the initial period if the LRA wants to be driven off resonance.
+ minimum: 5
+ maximum: 20475
+ multipleOf: 5
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+ - interrupt-names
+ - qcom,vmax-microvolt
+ - qcom,lra-period-us
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/irq.h>
+
+ pmic {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ haptics@f000 {
+ compatible = "qcom,pmih0108-haptics", "qcom,spmi-haptics";
+ reg = <0xf000>, <0xf100>;
+ reg-names = "cfg", "ptn";
+ interrupts = <0x7 0xf0 0x1 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "fifo-empty";
+
+ qcom,vmax-microvolt = <1300000>;
+ qcom,lra-period-us = <5880>;
+ };
+ };
--
2.43.0
^ permalink raw reply related
* Re: [PATCH] Input: uinput/uhid - disallow control characters in phys paths
From: sashiko-bot @ 2026-07-17 5:07 UTC (permalink / raw)
To: Peter Hutterer; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260717-wip-uinput-sanitize-phys-v1-1-f6459542ce4b@who-t.net>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The patch provides an incomplete mitigation for CVE-2026-50292. It sanitizes the `phys` property to prevent udev property injection via control characters (newlines), but completely fails to sanitize the `name` and `uniq` properties, which are also exported to uevents and controlled by userspace.
--
--- Patch [1]: [PATCH] Input: uinput/uhid - disallow control characters in phys paths ---
commit 1a76e90e7acc09a84a52efddc46e598419020a19
Author: Peter Hutterer <peter.hutterer@who-t.net>
Input: uinput/uhid - disallow control characters in phys paths
This commit sanitizes the phys property in uinput and uhid devices to replace
control characters with '?'. This prevents userspace from injecting malicious
udev properties via newlines, which previously triggered CVE-2026-50292.
> diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
> index 37b60c3aaf66a..baf1fe8290f75 100644
> --- a/drivers/hid/uhid.c
> +++ b/drivers/hid/uhid.c
> @@ -518,6 +518,7 @@ static int uhid_dev_create2(struct uhid_device *uhid,
> strscpy(hid->name, ev->u.create2.name, sizeof(hid->name));
> BUILD_BUG_ON(sizeof(hid->phys) != sizeof(ev->u.create2.phys));
> strscpy(hid->phys, ev->u.create2.phys, sizeof(hid->phys));
> + input_sanitize_phys(hid->phys);
> BUILD_BUG_ON(sizeof(hid->uniq) != sizeof(ev->u.create2.uniq));
> strscpy(hid->uniq, ev->u.create2.uniq, sizeof(hid->uniq));
[Severity: High]
This isn't a bug introduced by this patch, but while this successfully mitigates
CVE-2026-50292 for the phys property, does it still leave the name and uniq
properties vulnerable?
In uhid_dev_create2(), the user-controlled ev->u.create2.name and
ev->u.create2.uniq strings are copied directly without passing through a
similar sanitization function.
Additionally, in drivers/input/misc/uinput.c:uinput_dev_setup(), the name
property is also set without sanitization:
dev->name = kstrndup(setup.name, UINPUT_MAX_NAME_SIZE, GFP_KERNEL);
If a user with write access to /dev/uinput or /dev/uhid sets the name or uniq
properties to strings containing newlines, wouldn't this still allow the
injection of arbitrary udev environment variables when INPUT_ADD_HOTPLUG_VAR
formats them for uevents?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717-wip-uinput-sanitize-phys-v1-1-f6459542ce4b@who-t.net?part=1
^ permalink raw reply
* [PATCH] Input: uinput/uhid - disallow control characters in phys paths
From: Peter Hutterer @ 2026-07-17 4:49 UTC (permalink / raw)
To: David Rheinsberg, Jiri Kosina, Benjamin Tissoires,
Dmitry Torokhov
Cc: linux-input, linux-kernel, Peter Hutterer
There is no good reason to support those, no physical device will ever
produce those. Allowing \n in phys previously triggered CVE-2026-50292
in libinput - there the PHYS udev property value was used as part of
another udev property value. The linebreak then caused the property
to be split across two lines, allowing uinput devices to inject
malicious properties. While the bug is squarely inlibinput's court
there still isn't a good reason for control characters in uinput/uhid.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
drivers/hid/uhid.c | 1 +
drivers/input/misc/uinput.c | 1 +
include/linux/input.h | 15 +++++++++++++++
3 files changed, 17 insertions(+)
diff --git a/drivers/hid/uhid.c b/drivers/hid/uhid.c
index 37b60c3aaf66..baf1fe8290f7 100644
--- a/drivers/hid/uhid.c
+++ b/drivers/hid/uhid.c
@@ -513,16 +513,17 @@ static int uhid_dev_create2(struct uhid_device *uhid,
ret = PTR_ERR(hid);
goto err_free;
}
BUILD_BUG_ON(sizeof(hid->name) != sizeof(ev->u.create2.name));
strscpy(hid->name, ev->u.create2.name, sizeof(hid->name));
BUILD_BUG_ON(sizeof(hid->phys) != sizeof(ev->u.create2.phys));
strscpy(hid->phys, ev->u.create2.phys, sizeof(hid->phys));
+ input_sanitize_phys(hid->phys);
BUILD_BUG_ON(sizeof(hid->uniq) != sizeof(ev->u.create2.uniq));
strscpy(hid->uniq, ev->u.create2.uniq, sizeof(hid->uniq));
hid->ll_driver = &uhid_hid_driver;
hid->bus = ev->u.create2.bus;
hid->vendor = ev->u.create2.vendor;
hid->product = ev->u.create2.product;
hid->version = ev->u.create2.version;
diff --git a/drivers/input/misc/uinput.c b/drivers/input/misc/uinput.c
index d32fa4b508fc..70fe4f3e73bf 100644
--- a/drivers/input/misc/uinput.c
+++ b/drivers/input/misc/uinput.c
@@ -998,16 +998,17 @@ static long uinput_ioctl_handler(struct file *file, unsigned int cmd,
phys = strndup_user(p, 1024);
if (IS_ERR(phys)) {
retval = PTR_ERR(phys);
goto out;
}
kfree(udev->dev->phys);
+ input_sanitize_phys(phys);
udev->dev->phys = phys;
goto out;
case UI_BEGIN_FF_UPLOAD:
retval = uinput_ff_upload_from_user(p, &ff_up);
if (retval)
goto out;
diff --git a/include/linux/input.h b/include/linux/input.h
index 76f7aa226202..6c182f5c783f 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -527,16 +527,31 @@ int input_set_keycode(struct input_dev *dev,
bool input_match_device_id(const struct input_dev *dev,
const struct input_device_id *id);
void input_enable_softrepeat(struct input_dev *dev, int delay, int period);
bool input_device_enabled(struct input_dev *dev);
+/**
+ * input_sanitize_phys - replace invalid characters in a phys string
+ * @phys: the phys path to sanitize (modified in place)
+ *
+ * Replaces any control characters and non-ASCII characters with '?'.
+ **/
+static inline void input_sanitize_phys(char *phys)
+{
+ char *p;
+
+ for (p = phys; *p; p++)
+ if (*p < 0x20 || *p > 0x7e)
+ *p = '?';
+}
+
extern const struct class input_class;
/**
* struct ff_device - force-feedback part of an input device
* @upload: Called to upload an new effect into device
* @erase: Called to erase an effect from device
* @playback: Called to request device to start playing specified effect
* @set_gain: Called to set specified gain
---
base-commit: 58717b2a1365d06c8c64b72aa948541b53fe31eb
change-id: 20260717-wip-uinput-sanitize-phys-abb6cf40e577
Best regards,
--
Peter Hutterer <peter.hutterer@who-t.net>
^ permalink raw reply related
* [dtor-input:next] BUILD SUCCESS 3ed4ba919eb35860ee1867e2851ee3e6f9fe37f5
From: kernel test robot @ 2026-07-17 4:13 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: 3ed4ba919eb35860ee1867e2851ee3e6f9fe37f5 Input: wacom_w8001 - replace strlcat() with a strscpy() helper
elapsed time: 1703m
configs tested: 118
configs skipped: 5
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 gcc-16.1.0
arc allnoconfig gcc-16.1.0
arc allyesconfig gcc-16.1.0
arc randconfig-001-20260717 gcc-11.5.0
arc randconfig-002-20260717 gcc-11.5.0
arm allnoconfig clang-17
arm allyesconfig gcc-16.1.0
arm exynos_defconfig clang-17
arm randconfig-001-20260717 gcc-11.5.0
arm randconfig-002-20260717 gcc-11.5.0
arm randconfig-003-20260717 gcc-11.5.0
arm randconfig-004-20260717 gcc-11.5.0
arm64 allmodconfig clang-24
arm64 allnoconfig gcc-16.1.0
arm64 randconfig-001-20260717 gcc-13.4.0
arm64 randconfig-002-20260717 gcc-13.4.0
arm64 randconfig-003-20260717 gcc-13.4.0
arm64 randconfig-004-20260717 gcc-13.4.0
csky allmodconfig gcc-16.1.0
csky allnoconfig gcc-16.1.0
csky randconfig-001-20260717 gcc-13.4.0
csky randconfig-002-20260717 gcc-13.4.0
hexagon allnoconfig clang-24
i386 allmodconfig gcc-14
i386 allnoconfig gcc-14
i386 buildonly-randconfig-001-20260717 gcc-14
i386 buildonly-randconfig-002-20260717 gcc-14
i386 buildonly-randconfig-003-20260717 gcc-14
i386 buildonly-randconfig-004-20260717 gcc-14
i386 buildonly-randconfig-005-20260717 gcc-14
i386 buildonly-randconfig-006-20260717 gcc-14
loongarch allmodconfig clang-19
loongarch allnoconfig clang-20
loongarch defconfig clang-24
m68k allmodconfig gcc-16.1.0
m68k allnoconfig gcc-16.1.0
m68k allyesconfig gcc-16.1.0
m68k mvme147_defconfig gcc-16.1.0
microblaze allnoconfig gcc-16.1.0
microblaze allyesconfig gcc-16.1.0
mips allmodconfig gcc-16.1.0
mips allnoconfig gcc-16.1.0
mips allyesconfig gcc-16.1.0
nios2 allmodconfig gcc-11.5.0
nios2 allnoconfig gcc-11.5.0
openrisc allmodconfig gcc-16.1.0
openrisc allnoconfig gcc-16.1.0
parisc allmodconfig gcc-16.1.0
parisc allnoconfig gcc-16.1.0
parisc allyesconfig gcc-16.1.0
parisc defconfig gcc-16.1.0
parisc randconfig-001-20260717 gcc-11.5.0
parisc randconfig-002-20260717 gcc-11.5.0
powerpc allnoconfig gcc-16.1.0
powerpc randconfig-001-20260717 gcc-11.5.0
powerpc randconfig-002-20260717 gcc-11.5.0
powerpc64 randconfig-001-20260717 gcc-11.5.0
powerpc64 randconfig-002-20260717 gcc-11.5.0
riscv allnoconfig gcc-16.1.0
riscv allyesconfig clang-24
riscv randconfig-001-20260717 clang-24
riscv randconfig-002-20260717 clang-24
s390 allmodconfig clang-24
s390 allnoconfig clang-24
s390 allyesconfig gcc-16.1.0
s390 randconfig-001-20260717 clang-24
s390 randconfig-002-20260717 clang-24
sh allmodconfig gcc-16.1.0
sh allnoconfig gcc-16.1.0
sh allyesconfig gcc-16.1.0
sh defconfig gcc-16.1.0
sh randconfig-001-20260717 clang-24
sh randconfig-002-20260717 clang-24
sparc allnoconfig gcc-16.1.0
sparc randconfig-001-20260717 gcc-8.5.0
sparc randconfig-002-20260717 gcc-8.5.0
sparc64 allmodconfig clang-20
sparc64 randconfig-001-20260717 gcc-8.5.0
sparc64 randconfig-002-20260717 gcc-8.5.0
um allmodconfig clang-17
um allnoconfig clang-17
um allyesconfig gcc-14
um randconfig-001-20260717 gcc-8.5.0
um randconfig-002-20260717 gcc-8.5.0
x86_64 allnoconfig clang-22
x86_64 buildonly-randconfig-001-20260717 gcc-14
x86_64 buildonly-randconfig-002-20260717 gcc-14
x86_64 buildonly-randconfig-003-20260717 gcc-14
x86_64 buildonly-randconfig-004-20260717 gcc-14
x86_64 buildonly-randconfig-005-20260717 gcc-14
x86_64 buildonly-randconfig-006-20260717 gcc-14
x86_64 kexec clang-22
x86_64 randconfig-001-20260717 gcc-14
x86_64 randconfig-002-20260717 gcc-14
x86_64 randconfig-003-20260717 gcc-14
x86_64 randconfig-004-20260717 gcc-14
x86_64 randconfig-005-20260717 gcc-14
x86_64 randconfig-006-20260717 gcc-14
x86_64 randconfig-011-20260717 gcc-12
x86_64 randconfig-012-20260717 gcc-12
x86_64 randconfig-013-20260717 gcc-12
x86_64 randconfig-014-20260717 gcc-12
x86_64 randconfig-015-20260717 gcc-12
x86_64 randconfig-016-20260717 gcc-12
x86_64 randconfig-071-20260717 clang-22
x86_64 randconfig-072-20260717 clang-22
x86_64 randconfig-073-20260717 clang-22
x86_64 randconfig-074-20260717 clang-22
x86_64 randconfig-075-20260717 clang-22
x86_64 randconfig-076-20260717 clang-22
x86_64 rhel-9.4-bpf gcc-14
xtensa allnoconfig gcc-16.1.0
xtensa allyesconfig gcc-16.1.0
xtensa randconfig-001-20260717 gcc-8.5.0
xtensa randconfig-002-20260717 gcc-8.5.0
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v4 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA
From: sashiko-bot @ 2026-07-17 4:08 UTC (permalink / raw)
To: Even Xu; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260717035609.3874015-2-even.xu@intel.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] `thc_rxdma_reset()` aborts the reset sequence if pausing THC_RXDMA1 fails, leaving THC_RXDMA2 running.
Pre-existing issues:
- [High] thc_dma_unconfigure() zeroes PRD base addresses before DMA engines are stopped, risking IOMMU faults.
--
--- Patch [1]: [PATCH v4 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA ---
commit b0b099ba1dc08199ebeeb23013c7c472a6f2c1bb
Author: Even Xu <even.xu@intel.com>
HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA
This commit adds a helper function thc_rxdma_reset() to perform a read DMA
reset. It is intended to be called for recovery when a fatal DMA error
happens.
> 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
[ ... ]
> @@ -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
[ ... ]
> +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: Medium]
If pausing THC_RXDMA1 fails and times out, does jumping to the end label
leave THC_RXDMA2 running?
Bailing out here without attempting to stop the second RX DMA engine might
leave the hardware in an inconsistent state during a fatal error recovery.
> + 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: High]
This isn't a bug introduced by this patch, but does calling
thc_dma_unconfigure() here without first pausing TXDMA or SWDMA risk IOMMU
faults?
Looking at thc_dma_unconfigure(), it zeroes the PRD base addresses for all
channels before clearing the START bits:
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]);
...
If a previous thc_dma_write() timed out, its TXDMA engine might still be
active since the timeout handler does not pause it.
Zeroing the PRD base address of an active DMA engine could force the
hardware to fetch descriptors at physical address 0, which would cause an
immediate memory corruption or IOMMU panic.
> + 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/20260717035609.3874015-1-even.xu@intel.com?part=1
^ permalink raw reply
* Re: [PATCH v4 3/3] HID: Intel-thc-hid: Intel-quickspi: Refine recover callback
From: sashiko-bot @ 2026-07-17 4:08 UTC (permalink / raw)
To: Even Xu; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260717035609.3874015-4-even.xu@intel.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Race condition during device teardown allows the IRQ thread handler to unconditionally schedule `recover_work` after `cancel_work_sync()` has completed, leading to a Use-After-Free.
--
--- Patch [3]: [PATCH v4 3/3] HID: Intel-thc-hid: Intel-quickspi: Refine recover callback ---
commit 476846b5d6215f9a1a461a86f427618ddd19b6d1
Author: Even Xu <even.xu@intel.com>
HID: Intel-thc-hid: Intel-quickspi: Refine recover callback
This patch updates the recovery flow to execute in a workqueue rather than
directly in the IRQ handler. It introduces a recover_work item and a
recovery_disabled flag to manage the teardown process.
> 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..db7ede5cc7a29 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
[ ... ]
> @@ -337,11 +334,10 @@ static irqreturn_t quickspi_irq_thread_handler(int irq, void *dev_id)
> }
>
> end:
> if (err_recover)
> - if (try_recover(qsdev))
> - qsdev->state = QUICKSPI_DISABLED;
> + schedule_work(&qsdev->recover_work);
[Severity: High]
Can this schedule the work after it has been cancelled during device teardown?
Since the threaded IRQ handler doesn't check the recovery_disabled flag
before calling schedule_work(), if quickspi_irq_thread_handler() runs
concurrently with quickspi_dev_deinit():
CPU 1
quickspi_dev_deinit()
WRITE_ONCE(qsdev->recovery_disabled, true);
cancel_work_sync(&qsdev->recover_work);
// returns, and eventually devres frees qsdev
CPU 2
quickspi_irq_thread_handler()
...
if (err_recover)
schedule_work(&qsdev->recover_work);
Could CPU 2 queue the work after CPU 1 finishes cancel_work_sync()?
If the scheduled work executes after qsdev has been freed, will this cause a
use-after-free when try_recover() reads qsdev->recovery_disabled?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717035609.3874015-1-even.xu@intel.com?part=3
^ permalink raw reply
* Re: [PATCH v4 2/3] HID: Intel-thc-hid: Intel-quicki2c: Refine recover callback
From: sashiko-bot @ 2026-07-17 4:06 UTC (permalink / raw)
To: Even Xu; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260717035609.3874015-3-even.xu@intel.com>
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
New issues:
- [High] Race condition during device teardown leading to a Use-After-Free of `qcdev`.
- [Medium] Race condition in resume/thaw paths permanently disables device interrupts.
- [Medium] Premature interrupt unmasking causes an interrupt storm for shared IRQs.
- [Low] Redundant hardware teardown in `quicki2c_poweroff()` triggers kernel warnings.
Pre-existing issues:
- [High] Hardirq handler accesses memory-mapped hardware registers without verifying runtime PM state.
- [High] Unvalidated length field from hardware allows out-of-bounds read in the HID core.
--
--- Patch [2]: [PATCH v4 2/3] HID: Intel-thc-hid: Intel-quicki2c: Refine recover callback ---
commit 8f342c689f3a83c1f792680893d01fe944f899d6
Author: Even Xu <even.xu@intel.com>
Subject: HID: Intel-thc-hid: Intel-quicki2c: Refine recover callback
This commit refines the recover flow by offloading it to a workqueue instead
of processing it in the IRQ handler. It simplifies the operation using the
thc_rxdma_reset() API and ensures interrupts are disabled during the entire
recovery flow.
> 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 46d3e9a019995..3ccbddfe8e5e8 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,31 @@ static irqreturn_t quicki2c_irq_quick_handler(int irq, void *dev_id)
> }
[Severity: High]
This is a pre-existing issue, but does quicki2c_irq_quick_handler() risk
accessing memory-mapped hardware registers while the device is in a
runtime-suspended state?
The driver allows fallback to shared INTx interrupts, so the hardirq
handler could be invoked by another device on the same IRQ line while this
device is suspended. Calling thc_interrupt_enable() unconditionally could
access powered-off registers and cause Unsupported Requests (UR).
>
> /**
> - * 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 (READ_ONCE(qcdev->recovery_disabled))
> + return;
>
> - ret = thc_dma_configure(qcdev->thc_hw);
> - if (ret) {
> - dev_err(qcdev->dev, "Reconfig DMA failed\n");
> - return ret;
> - }
> + if (pm_runtime_resume_and_get(qcdev->dev))
> + return;
>
> - return 0;
> + thc_interrupt_enable(qcdev->thc_hw, false);
> +
> + 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)
[Severity: High]
This is a pre-existing issue, but does handle_input_report() trust the
length field from the hardware without validating it?
If a buggy or malicious device sends a packet with pkt->len equal to 1,
HIDI2C_DATA_LEN could underflow to SIZE_MAX when passed to the HID core:
quicki2c_hid_send_report(qcdev, pkt->data,
HIDI2C_DATA_LEN(le16_to_cpu(pkt->len)));
Could this unvalidated length cause an out-of-bounds read in the HID core?
[ ... ]
> @@ -343,11 +346,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);
[Severity: Medium]
Could this unconditionally re-enable the hardware interrupt while a recovery
is already pending?
If err_recover is 0 (for instance, if another shared interrupt fires), the
else branch unmasks the broken device's interrupt prematurely.
Does this cause an interrupt storm before try_recover() has a chance to run?
[ ... ]
> @@ -439,6 +443,9 @@ static struct quicki2c_device *quicki2c_dev_init(struct pci_dev *pdev, void __io
> */
> static void quicki2c_dev_deinit(struct quicki2c_device *qcdev)
> {
> + WRITE_ONCE(qcdev->recovery_disabled, true);
> + cancel_work_sync(&qcdev->recover_work);
> +
> thc_interrupt_quiesce(qcdev->thc_hw, true);
> thc_interrupt_enable(qcdev->thc_hw, false);
> thc_ltr_unconfig(qcdev->thc_hw);
[Severity: High]
Is there a race condition here that could lead to a use-after-free of qcdev?
cancel_work_sync() is called before the hardware interrupts are quiesced
and disabled. If the threaded IRQ handler is preempted right before calling
schedule_work(), it could queue recover_work after cancel_work_sync()
completes:
CPU1 CPU2
quicki2c_irq_thread_handler() quicki2c_dev_deinit()
err_recover = 1;
// preempted
cancel_work_sync();
thc_interrupt_quiesce();
// device removed, qcdev freed
schedule_work(&qcdev->recover_work);
When try_recover() eventually runs, will it dereference the freed qcdev
pointer?
[ ... ]
> @@ -867,6 +878,8 @@ static int quicki2c_resume(struct device *device)
> if (ret)
> return ret;
>
> + WRITE_ONCE(qcdev->recovery_disabled, false);
> +
[Severity: Medium]
Does unmasking the interrupt before setting recovery_disabled to false
create a window where the device can become permanently disabled?
If the hardware asserts an interrupt after thc_interrupt_quiesce() enables
it, but before WRITE_ONCE(qcdev->recovery_disabled, false) runs, the IRQ
handler will queue the recovery work.
The work will execute try_recover(), read recovery_disabled as true, and
return immediately without re-enabling interrupts. Will this leave the
device permanently unresponsive?
> if (!device_may_wakeup(qcdev->dev))
> return quicki2c_set_power(qcdev, HIDI2C_ON);
[ ... ]
> @@ -938,6 +956,8 @@ static int quicki2c_poweroff(struct device *device)
>
> thc_ltr_unconfig(qcdev->thc_hw);
>
> + quicki2c_dev_deinit(qcdev);
> +
> quicki2c_dma_deinit(qcdev);
>
> return 0;
[Severity: Low]
Does adding quicki2c_dev_deinit() here result in redundant hardware teardown
steps?
quicki2c_poweroff() manually calls thc_interrupt_quiesce() and
thc_ltr_unconfig() just before this line. Since quicki2c_dev_deinit() also
calls those functions, it looks like they are executed twice, which might
trigger kernel warnings from the underlying hardware functions.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260717035609.3874015-1-even.xu@intel.com?part=2
^ permalink raw reply
* [PATCH v4 3/3] HID: Intel-thc-hid: Intel-quickspi: Refine recover callback
From: Even Xu @ 2026-07-17 3:56 UTC (permalink / raw)
To: bentiss, jikos; +Cc: srinivas.pandruvada, linux-input, linux-kernel, Even Xu
In-Reply-To: <20260717035609.3874015-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 | 71 +++++++++++--------
.../intel-quickspi/quickspi-dev.h | 6 ++
2 files changed, 49 insertions(+), 28 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 4ae2e1718b30..db7ede5cc7a2 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,31 @@ 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 (READ_ONCE(qsdev->recovery_disabled))
+ return;
- thc_dma_unconfigure(qsdev->thc_hw);
+ if (pm_runtime_resume_and_get(qsdev->dev))
+ return;
- ret = thc_dma_configure(qsdev->thc_hw);
- if (ret) {
- dev_err(qsdev->dev, "Re-configure THC DMA failed, ret = %d\n", ret);
- return ret;
- }
+ thc_interrupt_enable(qsdev->thc_hw, false);
- return 0;
+ if (thc_rxdma_reset(qsdev->thc_hw))
+ qsdev->state = QUICKSPI_DISABLED;
+ else
+ thc_interrupt_enable(qsdev->thc_hw, true);
+
+ pm_runtime_put_autosuspend(qsdev->dev);
}
/**
@@ -337,11 +334,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 +381,8 @@ 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);
+ WRITE_ONCE(qsdev->recovery_disabled, false);
+ INIT_WORK(&qsdev->recover_work, try_recover);
/* thc hw init */
qsdev->thc_hw = thc_dev_init(qsdev->dev, qsdev->mem_addr);
@@ -461,6 +459,10 @@ static struct quickspi_device *quickspi_dev_init(struct pci_dev *pdev, void __io
*/
static void quickspi_dev_deinit(struct quickspi_device *qsdev)
{
+ WRITE_ONCE(qsdev->recovery_disabled, true);
+ cancel_work_sync(&qsdev->recover_work);
+
+ thc_interrupt_quiesce(qsdev->thc_hw, true);
thc_interrupt_enable(qsdev->thc_hw, false);
thc_ltr_unconfig(qsdev->thc_hw);
thc_wot_unconfig(qsdev->thc_hw);
@@ -711,12 +713,13 @@ static void quickspi_remove(struct pci_dev *pdev)
return;
quickspi_hid_remove(qsdev);
+
+ quickspi_dev_deinit(qsdev);
+
quickspi_dma_deinit(qsdev);
pm_runtime_get_noresume(qsdev->dev);
- quickspi_dev_deinit(qsdev);
-
pci_clear_master(pdev);
}
@@ -737,10 +740,10 @@ static void quickspi_shutdown(struct pci_dev *pdev)
if (!qsdev)
return;
+ quickspi_dev_deinit(qsdev);
+
/* Must stop DMA before reboot to avoid DMA entering into unknown state */
quickspi_dma_deinit(qsdev);
-
- quickspi_dev_deinit(qsdev);
}
static int quickspi_suspend(struct device *device)
@@ -759,6 +762,9 @@ static int quickspi_suspend(struct device *device)
return ret;
}
+ WRITE_ONCE(qsdev->recovery_disabled, true);
+ cancel_work_sync(&qsdev->recover_work);
+
ret = thc_interrupt_quiesce(qsdev->thc_hw, true);
if (ret)
return ret;
@@ -784,6 +790,8 @@ static int quickspi_resume(struct device *device)
if (ret)
return ret;
+ WRITE_ONCE(qsdev->recovery_disabled, false);
+
/*
* A wake-enabled device keeps its power and state across suspend, so
* only restore the THC context. Resetting it here would discard a
@@ -864,6 +872,9 @@ static int quickspi_freeze(struct device *device)
if (!qsdev)
return -ENODEV;
+ WRITE_ONCE(qsdev->recovery_disabled, true);
+ cancel_work_sync(&qsdev->recover_work);
+
ret = thc_interrupt_quiesce(qsdev->thc_hw, true);
if (ret)
return ret;
@@ -895,6 +906,8 @@ static int quickspi_thaw(struct device *device)
if (ret)
return ret;
+ WRITE_ONCE(qsdev->recovery_disabled, false);
+
return 0;
}
@@ -919,6 +932,8 @@ static int quickspi_poweroff(struct device *device)
thc_ltr_unconfig(qsdev->thc_hw);
+ quickspi_dev_deinit(qsdev);
+
quickspi_dma_deinit(qsdev);
return 0;
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..2936c8b1532c 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"
@@ -126,6 +127,8 @@ struct acpi_device;
* @get_feature_cmpl: indicate get feature received or not
* @set_feature_cmpl_wq: workqueue for waiting set feature to device
* @set_feature_cmpl: indicate set feature send complete or not
+ * @recover_work: Work structure for recovery
+ * @recovery_disabled: Whether recovery work is blocked during teardown
*/
struct quickspi_device {
struct device *dev;
@@ -173,6 +176,9 @@ struct quickspi_device {
wait_queue_head_t set_report_cmpl_wq;
bool set_report_cmpl;
+
+ struct work_struct recover_work;
+ bool recovery_disabled;
};
#endif /* _QUICKSPI_DEV_H_ */
--
2.43.0
^ permalink raw reply related
* [PATCH v4 2/3] HID: Intel-thc-hid: Intel-quicki2c: Refine recover callback
From: Even Xu @ 2026-07-17 3:56 UTC (permalink / raw)
To: bentiss, jikos; +Cc: srinivas.pandruvada, linux-input, linux-kernel, Even Xu
In-Reply-To: <20260717035609.3874015-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 | 66 ++++++++++++-------
.../intel-quicki2c/quicki2c-dev.h | 5 ++
2 files changed, 48 insertions(+), 23 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..3ccbddfe8e5e 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,31 @@ 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 (READ_ONCE(qcdev->recovery_disabled))
+ return;
- ret = thc_dma_configure(qcdev->thc_hw);
- if (ret) {
- dev_err(qcdev->dev, "Reconfig DMA failed\n");
- return ret;
- }
+ if (pm_runtime_resume_and_get(qcdev->dev))
+ return;
- return 0;
+ thc_interrupt_enable(qcdev->thc_hw, false);
+
+ 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 +346,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 +388,8 @@ static struct quicki2c_device *quicki2c_dev_init(struct pci_dev *pdev, void __io
qcdev->ddata = ddata;
init_waitqueue_head(&qcdev->reset_ack_wq);
+ WRITE_ONCE(qcdev->recovery_disabled, false);
+ INIT_WORK(&qcdev->recover_work, try_recover);
/* THC hardware init */
qcdev->thc_hw = thc_dev_init(qcdev->dev, qcdev->mem_addr);
@@ -439,6 +443,9 @@ static struct quicki2c_device *quicki2c_dev_init(struct pci_dev *pdev, void __io
*/
static void quicki2c_dev_deinit(struct quicki2c_device *qcdev)
{
+ WRITE_ONCE(qcdev->recovery_disabled, true);
+ cancel_work_sync(&qcdev->recover_work);
+
thc_interrupt_quiesce(qcdev->thc_hw, true);
thc_interrupt_enable(qcdev->thc_hw, false);
thc_ltr_unconfig(qcdev->thc_hw);
@@ -772,12 +779,13 @@ static void quicki2c_remove(struct pci_dev *pdev)
return;
quicki2c_hid_remove(qcdev);
+
+ quicki2c_dev_deinit(qcdev);
+
quicki2c_dma_deinit(qcdev);
pm_runtime_get_noresume(qcdev->dev);
- quicki2c_dev_deinit(qcdev);
-
pci_clear_master(pdev);
}
@@ -796,10 +804,10 @@ static void quicki2c_shutdown(struct pci_dev *pdev)
if (!qcdev)
return;
+ quicki2c_dev_deinit(qcdev);
+
/* Must stop DMA before reboot to avoid DMA entering into unknown state */
quicki2c_dma_deinit(qcdev);
-
- quicki2c_dev_deinit(qcdev);
}
static int quicki2c_suspend(struct device *device)
@@ -826,6 +834,9 @@ static int quicki2c_suspend(struct device *device)
if (ret)
return ret;
+ WRITE_ONCE(qcdev->recovery_disabled, true);
+ cancel_work_sync(&qcdev->recover_work);
+
ret = thc_interrupt_quiesce(qcdev->thc_hw, true);
if (ret)
return ret;
@@ -867,6 +878,8 @@ static int quicki2c_resume(struct device *device)
if (ret)
return ret;
+ WRITE_ONCE(qcdev->recovery_disabled, false);
+
if (!device_may_wakeup(qcdev->dev))
return quicki2c_set_power(qcdev, HIDI2C_ON);
@@ -883,6 +896,9 @@ static int quicki2c_freeze(struct device *device)
if (!qcdev)
return -ENODEV;
+ WRITE_ONCE(qcdev->recovery_disabled, true);
+ cancel_work_sync(&qcdev->recover_work);
+
ret = thc_interrupt_quiesce(qcdev->thc_hw, true);
if (ret)
return ret;
@@ -914,6 +930,8 @@ static int quicki2c_thaw(struct device *device)
if (ret)
return ret;
+ WRITE_ONCE(qcdev->recovery_disabled, false);
+
return 0;
}
@@ -938,6 +956,8 @@ static int quicki2c_poweroff(struct device *device)
thc_ltr_unconfig(qcdev->thc_hw);
+ quicki2c_dev_deinit(qcdev);
+
quicki2c_dma_deinit(qcdev);
return 0;
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..d72565de67b9 100644
--- a/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
+++ b/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
@@ -191,6 +191,8 @@ struct acpi_device;
* @i2c_max_frame_size: Max RX frame size (unit in Bytes)
* @i2c_int_delay_enable: Indicate interrupt delay feature enabled or not
* @i2c_int_delay: Interrupt detection delay value (unit in 10 us)
+ * @recover_work: Work structure for recovery
+ * @recovery_disabled: Whether recovery work is blocked during teardown
*/
struct quicki2c_device {
struct device *dev;
@@ -226,6 +228,9 @@ struct quicki2c_device {
u32 i2c_max_frame_size;
u32 i2c_int_delay_enable;
u32 i2c_int_delay;
+
+ struct work_struct recover_work;
+ bool recovery_disabled;
};
#endif /* _QUICKI2C_DEV_H_ */
--
2.43.0
^ permalink raw reply related
* [PATCH v4 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA
From: Even Xu @ 2026-07-17 3:56 UTC (permalink / raw)
To: bentiss, jikos; +Cc: srinivas.pandruvada, linux-input, linux-kernel, Even Xu
In-Reply-To: <20260717035609.3874015-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 RESEND v4 0/3] HID: Intel-thc-hid: Refine error recovery flow
From: Even Xu @ 2026-07-17 3:56 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:
v4:
- Refine cancel_work_sync() flow to reduce race condition of DMA
operation confliction and reschedule.
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 | 66 +++++++++++------
.../intel-quicki2c/quicki2c-dev.h | 5 ++
.../intel-quickspi/pci-quickspi.c | 71 +++++++++++--------
.../intel-quickspi/quickspi-dev.h | 6 ++
.../intel-thc-hid/intel-thc/intel-thc-dma.c | 51 +++++++++++++
.../intel-thc-hid/intel-thc/intel-thc-dma.h | 1 +
6 files changed, 149 insertions(+), 51 deletions(-)
--
2.43.0
^ permalink raw reply
* RE: [PATCH v2 4/4] HID: Intel-thc-hid: Remove redundant dev_err()
From: Xu, Even @ 2026-07-17 3:41 UTC (permalink / raw)
To: Pan Chuang, Sun, Xinpeng, Jiri Kosina, Benjamin Tissoires,
Sakari Ailus, Abhishek Tamboli, Danny D.,
open list:HID CORE LAYER, open list
In-Reply-To: <20260716075203.593012-5-panchuang@vivo.com>
> -----Original Message-----
> From: Pan Chuang <panchuang@vivo.com>
> Sent: Thursday, July 16, 2026 3:52 PM
> To: Xu, Even <even.xu@intel.com>; Sun, Xinpeng <xinpeng.sun@intel.com>; Jiri
> Kosina <jikos@kernel.org>; Benjamin Tissoires <bentiss@kernel.org>; Sakari Ailus
> <sakari.ailus@linux.intel.com>; Pan Chuang <panchuang@vivo.com>; Abhishek
> Tamboli <abhishektamboli9@gmail.com>; Danny D. <d3z.the.dev@gmail.com>;
> open list:HID CORE LAYER <linux-input@vger.kernel.org>; open list <linux-
> kernel@vger.kernel.org>
> Subject: [PATCH v2 4/4] HID: Intel-thc-hid: Remove redundant dev_err()
>
> Since commit 55b48e23f5c4 ("genirq/devres: Add error handling in
> devm_request_*_irq()"), devm_request_threaded_irq() automatically logs
> detailed error messages on failure. Remove the now-redundant driver-specific
> dev_err() calls.
>
> Signed-off-by: Pan Chuang <panchuang@vivo.com>
> ---
> drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c | 5 +---- drivers/hid/intel-
> thc-hid/intel-quickspi/pci-quickspi.c | 5 +----
> 2 files changed, 2 insertions(+), 8 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..59f500345acb 100644
> --- a/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> +++ b/drivers/hid/intel-thc-hid/intel-quicki2c/pci-quicki2c.c
> @@ -682,11 +682,8 @@ static int quicki2c_probe(struct pci_dev *pdev, const
> struct pci_device_id *id)
> quicki2c_irq_thread_handler,
> IRQF_ONESHOT, KBUILD_MODNAME,
> qcdev);
> - if (ret) {
> - dev_err_once(&pdev->dev,
> - "Failed to request threaded IRQ, irq = %d.\n", pdev-
> >irq);
> + if (ret)
> goto dev_deinit;
> - }
>
> ret = quicki2c_get_device_descriptor(qcdev);
> if (ret) {
> 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 4ae2e1718b30..504ef3c842ab 100644
> --- a/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> +++ b/drivers/hid/intel-thc-hid/intel-quickspi/pci-quickspi.c
> @@ -636,11 +636,8 @@ static int quickspi_probe(struct pci_dev *pdev,
> quickspi_irq_thread_handler,
> IRQF_ONESHOT, KBUILD_MODNAME,
> qsdev);
> - if (ret) {
> - dev_err(&pdev->dev,
> - "Failed to request threaded IRQ, irq = %d.\n", pdev-
> >irq);
> + if (ret)
> goto dev_deinit;
> - }
Thank you for the patches!
Reviewed-by: Even Xu <even.xu@intel.com>
>
> ret = reset_tic(qsdev);
> if (ret) {
> --
> 2.34.1
^ permalink raw reply
* [PATCH v4 3/3] HID: Intel-thc-hid: Intel-quickspi: Refine recover callback
From: Even Xu @ 2026-07-17 3:37 UTC (permalink / raw)
To: bentiss, jikos; +Cc: srinivas.pandruvada, linux-input, linux-kernel, Even Xu
In-Reply-To: <20260717033754.3689055-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 | 71 +++++++++++--------
.../intel-quickspi/quickspi-dev.h | 6 ++
2 files changed, 49 insertions(+), 28 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..f048f5b1f7eb 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,31 @@ 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 (READ_ONCE(qsdev->recovery_disabled))
+ return;
- thc_dma_unconfigure(qsdev->thc_hw);
+ if (pm_runtime_resume_and_get(qsdev->dev))
+ return;
- ret = thc_dma_configure(qsdev->thc_hw);
- if (ret) {
- dev_err(qsdev->dev, "Re-configure THC DMA failed, ret = %d\n", ret);
- return ret;
- }
+ thc_interrupt_enable(qsdev->thc_hw, false);
- return 0;
+ if (thc_rxdma_reset(qsdev->thc_hw))
+ qsdev->state = QUICKSPI_DISABLED;
+ else
+ thc_interrupt_enable(qsdev->thc_hw, true);
+
+ pm_runtime_put_autosuspend(qsdev->dev);
}
/**
@@ -337,11 +334,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 +381,8 @@ 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);
+ WRITE_ONCE(qsdev->recovery_disabled, false);
+ INIT_WORK(&qsdev->recover_work, try_recover);
/* thc hw init */
qsdev->thc_hw = thc_dev_init(qsdev->dev, qsdev->mem_addr);
@@ -461,6 +459,10 @@ static struct quickspi_device *quickspi_dev_init(struct pci_dev *pdev, void __io
*/
static void quickspi_dev_deinit(struct quickspi_device *qsdev)
{
+ WRITE_ONCE(qsdev->recovery_disabled, true);
+ cancel_work_sync(&qsdev->recover_work);
+
+ thc_interrupt_quiesce(qsdev->thc_hw, true);
thc_interrupt_enable(qsdev->thc_hw, false);
thc_ltr_unconfig(qsdev->thc_hw);
thc_wot_unconfig(qsdev->thc_hw);
@@ -711,12 +713,13 @@ static void quickspi_remove(struct pci_dev *pdev)
return;
quickspi_hid_remove(qsdev);
+
+ quickspi_dev_deinit(qsdev);
+
quickspi_dma_deinit(qsdev);
pm_runtime_get_noresume(qsdev->dev);
- quickspi_dev_deinit(qsdev);
-
pci_clear_master(pdev);
}
@@ -737,10 +740,10 @@ static void quickspi_shutdown(struct pci_dev *pdev)
if (!qsdev)
return;
+ quickspi_dev_deinit(qsdev);
+
/* Must stop DMA before reboot to avoid DMA entering into unknown state */
quickspi_dma_deinit(qsdev);
-
- quickspi_dev_deinit(qsdev);
}
static int quickspi_suspend(struct device *device)
@@ -759,6 +762,9 @@ static int quickspi_suspend(struct device *device)
return ret;
}
+ WRITE_ONCE(qsdev->recovery_disabled, true);
+ cancel_work_sync(&qsdev->recover_work);
+
ret = thc_interrupt_quiesce(qsdev->thc_hw, true);
if (ret)
return ret;
@@ -796,6 +802,8 @@ static int quickspi_resume(struct device *device)
if (ret)
return ret;
+ WRITE_ONCE(qsdev->recovery_disabled, false);
+
if (!device_may_wakeup(qsdev->dev))
return quickspi_set_power(qsdev, HIDSPI_ON);
@@ -812,6 +820,9 @@ static int quickspi_freeze(struct device *device)
if (!qsdev)
return -ENODEV;
+ WRITE_ONCE(qsdev->recovery_disabled, true);
+ cancel_work_sync(&qsdev->recover_work);
+
ret = thc_interrupt_quiesce(qsdev->thc_hw, true);
if (ret)
return ret;
@@ -843,6 +854,8 @@ static int quickspi_thaw(struct device *device)
if (ret)
return ret;
+ WRITE_ONCE(qsdev->recovery_disabled, false);
+
return 0;
}
@@ -867,6 +880,8 @@ static int quickspi_poweroff(struct device *device)
thc_ltr_unconfig(qsdev->thc_hw);
+ quickspi_dev_deinit(qsdev);
+
quickspi_dma_deinit(qsdev);
return 0;
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..2936c8b1532c 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"
@@ -126,6 +127,8 @@ struct acpi_device;
* @get_feature_cmpl: indicate get feature received or not
* @set_feature_cmpl_wq: workqueue for waiting set feature to device
* @set_feature_cmpl: indicate set feature send complete or not
+ * @recover_work: Work structure for recovery
+ * @recovery_disabled: Whether recovery work is blocked during teardown
*/
struct quickspi_device {
struct device *dev;
@@ -173,6 +176,9 @@ struct quickspi_device {
wait_queue_head_t set_report_cmpl_wq;
bool set_report_cmpl;
+
+ struct work_struct recover_work;
+ bool recovery_disabled;
};
#endif /* _QUICKSPI_DEV_H_ */
--
2.43.0
^ permalink raw reply related
* [PATCH v4 2/3] HID: Intel-thc-hid: Intel-quicki2c: Refine recover callback
From: Even Xu @ 2026-07-17 3:37 UTC (permalink / raw)
To: bentiss, jikos; +Cc: srinivas.pandruvada, linux-input, linux-kernel, Even Xu
In-Reply-To: <20260717033754.3689055-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 | 66 ++++++++++++-------
.../intel-quicki2c/quicki2c-dev.h | 5 ++
2 files changed, 48 insertions(+), 23 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..3ccbddfe8e5e 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,31 @@ 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 (READ_ONCE(qcdev->recovery_disabled))
+ return;
- ret = thc_dma_configure(qcdev->thc_hw);
- if (ret) {
- dev_err(qcdev->dev, "Reconfig DMA failed\n");
- return ret;
- }
+ if (pm_runtime_resume_and_get(qcdev->dev))
+ return;
- return 0;
+ thc_interrupt_enable(qcdev->thc_hw, false);
+
+ 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 +346,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 +388,8 @@ static struct quicki2c_device *quicki2c_dev_init(struct pci_dev *pdev, void __io
qcdev->ddata = ddata;
init_waitqueue_head(&qcdev->reset_ack_wq);
+ WRITE_ONCE(qcdev->recovery_disabled, false);
+ INIT_WORK(&qcdev->recover_work, try_recover);
/* THC hardware init */
qcdev->thc_hw = thc_dev_init(qcdev->dev, qcdev->mem_addr);
@@ -439,6 +443,9 @@ static struct quicki2c_device *quicki2c_dev_init(struct pci_dev *pdev, void __io
*/
static void quicki2c_dev_deinit(struct quicki2c_device *qcdev)
{
+ WRITE_ONCE(qcdev->recovery_disabled, true);
+ cancel_work_sync(&qcdev->recover_work);
+
thc_interrupt_quiesce(qcdev->thc_hw, true);
thc_interrupt_enable(qcdev->thc_hw, false);
thc_ltr_unconfig(qcdev->thc_hw);
@@ -772,12 +779,13 @@ static void quicki2c_remove(struct pci_dev *pdev)
return;
quicki2c_hid_remove(qcdev);
+
+ quicki2c_dev_deinit(qcdev);
+
quicki2c_dma_deinit(qcdev);
pm_runtime_get_noresume(qcdev->dev);
- quicki2c_dev_deinit(qcdev);
-
pci_clear_master(pdev);
}
@@ -796,10 +804,10 @@ static void quicki2c_shutdown(struct pci_dev *pdev)
if (!qcdev)
return;
+ quicki2c_dev_deinit(qcdev);
+
/* Must stop DMA before reboot to avoid DMA entering into unknown state */
quicki2c_dma_deinit(qcdev);
-
- quicki2c_dev_deinit(qcdev);
}
static int quicki2c_suspend(struct device *device)
@@ -826,6 +834,9 @@ static int quicki2c_suspend(struct device *device)
if (ret)
return ret;
+ WRITE_ONCE(qcdev->recovery_disabled, true);
+ cancel_work_sync(&qcdev->recover_work);
+
ret = thc_interrupt_quiesce(qcdev->thc_hw, true);
if (ret)
return ret;
@@ -867,6 +878,8 @@ static int quicki2c_resume(struct device *device)
if (ret)
return ret;
+ WRITE_ONCE(qcdev->recovery_disabled, false);
+
if (!device_may_wakeup(qcdev->dev))
return quicki2c_set_power(qcdev, HIDI2C_ON);
@@ -883,6 +896,9 @@ static int quicki2c_freeze(struct device *device)
if (!qcdev)
return -ENODEV;
+ WRITE_ONCE(qcdev->recovery_disabled, true);
+ cancel_work_sync(&qcdev->recover_work);
+
ret = thc_interrupt_quiesce(qcdev->thc_hw, true);
if (ret)
return ret;
@@ -914,6 +930,8 @@ static int quicki2c_thaw(struct device *device)
if (ret)
return ret;
+ WRITE_ONCE(qcdev->recovery_disabled, false);
+
return 0;
}
@@ -938,6 +956,8 @@ static int quicki2c_poweroff(struct device *device)
thc_ltr_unconfig(qcdev->thc_hw);
+ quicki2c_dev_deinit(qcdev);
+
quicki2c_dma_deinit(qcdev);
return 0;
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..d72565de67b9 100644
--- a/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
+++ b/drivers/hid/intel-thc-hid/intel-quicki2c/quicki2c-dev.h
@@ -191,6 +191,8 @@ struct acpi_device;
* @i2c_max_frame_size: Max RX frame size (unit in Bytes)
* @i2c_int_delay_enable: Indicate interrupt delay feature enabled or not
* @i2c_int_delay: Interrupt detection delay value (unit in 10 us)
+ * @recover_work: Work structure for recovery
+ * @recovery_disabled: Whether recovery work is blocked during teardown
*/
struct quicki2c_device {
struct device *dev;
@@ -226,6 +228,9 @@ struct quicki2c_device {
u32 i2c_max_frame_size;
u32 i2c_int_delay_enable;
u32 i2c_int_delay;
+
+ struct work_struct recover_work;
+ bool recovery_disabled;
};
#endif /* _QUICKI2C_DEV_H_ */
--
2.43.0
^ permalink raw reply related
* [PATCH v4 1/3] HID: Intel-thc-hid: Intel-thc: Add API to reset read DMA
From: Even Xu @ 2026-07-17 3:37 UTC (permalink / raw)
To: bentiss, jikos; +Cc: srinivas.pandruvada, linux-input, linux-kernel, Even Xu
In-Reply-To: <20260717033754.3689055-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 v4 0/3] HID: Intel-thc-hid: Refine error recovery flow
From: Even Xu @ 2026-07-17 3:37 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:
v4:
- Refine cancel_work_sync() flow to reduce race condition of DMA
operation confliction and reschedule.
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 | 66 +++++++++++------
.../intel-quicki2c/quicki2c-dev.h | 5 ++
.../intel-quickspi/pci-quickspi.c | 71 +++++++++++--------
.../intel-quickspi/quickspi-dev.h | 6 ++
.../intel-thc-hid/intel-thc/intel-thc-dma.c | 51 +++++++++++++
.../intel-thc-hid/intel-thc/intel-thc-dma.h | 1 +
6 files changed, 149 insertions(+), 51 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH] dt-bindings: input: Convert TI Keypad Controller to DT schema
From: Rob Herring @ 2026-07-16 22:12 UTC (permalink / raw)
To: sashiko-reviews
Cc: Eduard Bostina, dmitry.torokhov, conor+dt, linux-input,
devicetree
In-Reply-To: <20260708124112.1DC541F00A3A@smtp.kernel.org>
On Wed, Jul 08, 2026 at 12:41:11PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Properties strictly required by the hardware and driver ('keypad,num-rows', 'keypad,num-columns', 'interrupts', and 'linux,keymap') are omitted from the 'required:' list, allowing non-functional device trees to silently pass validation.
> --
>
> --- Patch [1]: [PATCH] dt-bindings: input: Convert TI Keypad Controller to DT schema ---
> commit 31e1662350e854e7b96beb7e7de83fc2b0d265e2
> Author: Eduard Bostina <egbostina@gmail.com>
>
> dt-bindings: input: Convert TI Keypad Controller to DT schema
>
> This commit converts the Texas Instruments Keypad Controller bindings to DT
> schema format. It updates the autorepeat property name, adds the reg-names
> property, and omits the keypad row and column properties from the required
> list to prevent validation failures.
>
> > diff --git a/Documentation/devicetree/bindings/input/ti,omap4-keypad.yaml b/Documentation/devicetree/bindings/input/ti,omap4-keypad.yaml
> > new file mode 100644
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/input/ti,omap4-keypad.yaml
> [ ... ]
> > +required:
> > + - compatible
> > + - reg
>
> [Severity: Medium]
> The commit message mentions that keypad,num-rows and keypad,num-columns were
> omitted from this required list because enforcing them causes validation
> failures. Should these properties, along with interrupts and linux,keymap,
> be included here to accurately reflect the driver's probing requirements?
>
> Omitting them allows non-functional device tree nodes to silently pass
> schema validation. At runtime, the omap4-keypad driver will unconditionally
> fail to probe via platform_get_irq(), matrix_keypad_parse_properties(), and
> matrix_keypad_build_keymap() when these properties are missing.
>
> Would it be better to enforce the strict hardware and driver requirements in
> the schema, and instead fix the incomplete device tree nodes that are
> failing validation?
Yes. sashiko is right.
Rob
^ permalink raw reply
* Re: [PATCH 2/4] virtio_input: validate device-reported multitouch slot count
From: Dmitry Torokhov @ 2026-07-16 17:33 UTC (permalink / raw)
To: Hari Mishal
Cc: Michael S. Tsirkin, Amit Shah, Arnd Bergmann, Greg Kroah-Hartman,
Gerd Hoffmann, Jason Wang, David Hildenbrand, Henrik Rydberg,
Xuan Zhuo, Eugenio Pérez, virtualization, linux-kernel,
linux-input
In-Reply-To: <alkTnRb9qhgcMGGi@google.com>
[ Just realized that CC was dropped in the email I was replying to, so
restoring and resending... ]
On Thu, Jul 16, 2026 at 10:28:36AM -0700, Dmitry Torokhov wrote:
> On Thu, Jul 16, 2026 at 12:34:57PM +0200, Hari Mishal wrote:
> > > What is the failure mode if we keep the ABS_MT_SLOT capability? Does the
> > > kernel crash? And if this can cause crash then we should fix
> > > input_mt_init_slots() to reject requests for 0 slots with -EINVAL.
> > >
> >
> > No, it doesn't crash. I think every place in the input core that touches
> > dev->mt guards against it being NULL: input_handle_abs_event() and
> > the mt_slots check in input.c, and evdev's EVIOCGMTSLOTS ioctl
> > handler all explicitly check for NULL and degrade cleanly instead of
> > dereferencing. From my understanding, the worst case is what the
> > original commit message already covers: the device advertises
> > multitouch support it can't back.
>
> So what? I still do not see the problem. Let's say I have a device that
> properly supports multitouch and has slots, but then never sends any
> events because firmware is buggy. How would that affect anything?
>
> If there is no crash that I would leave the driver alone.
>
> And we need to remember that we are dealing with a hypervisor here that
> normally had higher level of trust than the VM. If it messes up we do
> not have to clean up after it. This scenario is different from user
> attaching a malicious USB device to their system and getting owned.
>
> Thanks.
>
--
Dmitry
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox