* Re: [PATCH v2 1/3] Input: samsung-keypad - clean up wakeup configuration logic
From: sashiko-bot @ 2026-07-14 5:51 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260713-samsung-kp-irq-v2-1-acc84b6daf9a@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The threaded IRQ handler accesses memory-mapped registers without ensuring the peripheral clock is enabled if a wakeup interrupt fires while the device is closed, leading to a kernel crash.
--
--- Patch [1]: [PATCH v2 1/3] Input: samsung-keypad - clean up wakeup configuration logic ---
commit c263408953e10fb025a3702953499e150bde4016
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Input: samsung-keypad - clean up wakeup configuration logic
This commit refactors the wakeup configuration logic in the samsung-keypad
driver. It moves the device_may_wakeup check to the caller and adjusts the
ordering of register writes and interrupt configuration.
> diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
> index 17127269e3f05..a578f429d1009 100644
> --- a/drivers/input/keyboard/samsung-keypad.c
> +++ b/drivers/input/keyboard/samsung-keypad.c
[ ... ]
> @@ -516,7 +515,8 @@ static int samsung_keypad_suspend(struct device *dev)
> if (input_device_enabled(input_dev))
> samsung_keypad_stop(keypad);
>
> - samsung_keypad_toggle_wakeup(keypad, true);
> + if (device_may_wakeup(dev))
> + samsung_keypad_toggle_wakeup(keypad, true);
>
> return 0;
> }
[Severity: High]
This is a pre-existing issue, but could a wakeup interrupt firing while the
device is closed cause an unclocked register access and crash the system?
If the device is closed by all users, samsung_keypad_stop() sets
keypad->stopped to true. When the system suspends, the code above enables
wakeup if device_may_wakeup() returns true.
If a key press triggers a wakeup interrupt, upon resume the primary IRQ
handler schedules the threaded handler samsung_keypad_irq():
drivers/input/keyboard/samsung-keypad.c:samsung_keypad_irq() {
...
pm_runtime_get_sync(&keypad->pdev->dev);
do {
readl(keypad->base + SAMSUNG_KEYIFSTSCLR);
...
}
However, pm_runtime_get_sync() calls samsung_keypad_runtime_resume(), which
skips enabling the clock because keypad->stopped is true:
drivers/input/keyboard/samsung-keypad.c:samsung_keypad_runtime_resume() {
...
if (keypad->stopped)
return 0;
clk_enable(keypad->clk);
...
}
Will the subsequent readl() in samsung_keypad_irq() trigger a synchronous
external abort since the peripheral clock was never enabled?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-samsung-kp-irq-v2-0-acc84b6daf9a@gmail.com?part=1
^ permalink raw reply
* [PATCH v2 2/3] Input: samsung-keypad - keep interrupt disabled while closed
From: Dmitry Torokhov @ 2026-07-14 5:39 UTC (permalink / raw)
To: linux-input; +Cc: Krzysztof Kozlowski, linux-kernel
In-Reply-To: <20260713-samsung-kp-irq-v2-0-acc84b6daf9a@gmail.com>
The driver requests the interrupt during probe, which by default enables
it. If the bootloader left the keypad interrupts enabled, or if a
spurious interrupt fires early before the driver is fully initialized
and clocks are enabled, the interrupt handler will attempt to read
registers and may cause a synchronous external abort.
Fix this by requesting the interrupt with IRQF_NO_AUTOEN, keeping it
disabled during probe. Enable the interrupt in samsung_keypad_start()
when the device is opened and ready, and disable it in
samsung_keypad_stop() when the device is closed. Remove the redundant
re-enabling of the interrupt at the end of samsung_keypad_stop().
Additionally, manually clear the pending interrupt status during system
resume when the device is closed to avoid immediate resume.
Fixes: 0fffed27f92d ("Input: samsung-keypad - Add samsung keypad driver")
Assisted-by: Antigravity:gemini-3.1-pro
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/samsung-keypad.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index a578f429d100..a51f0f639e9e 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -183,6 +183,8 @@ static void samsung_keypad_start(struct samsung_keypad *keypad)
writel(0, keypad->base + SAMSUNG_KEYIFCOL);
pm_runtime_put(&keypad->pdev->dev);
+
+ enable_irq(keypad->irq);
}
static void samsung_keypad_stop(struct samsung_keypad *keypad)
@@ -206,12 +208,6 @@ static void samsung_keypad_stop(struct samsung_keypad *keypad)
clk_disable(keypad->clk);
- /*
- * Now that chip should not generate interrupts we can safely
- * re-enable the handler.
- */
- enable_irq(keypad->irq);
-
pm_runtime_put(&keypad->pdev->dev);
}
@@ -412,7 +408,8 @@ static int samsung_keypad_probe(struct platform_device *pdev)
}
error = devm_request_threaded_irq(&pdev->dev, keypad->irq, NULL,
- samsung_keypad_irq, IRQF_ONESHOT,
+ samsung_keypad_irq,
+ IRQF_ONESHOT | IRQF_NO_AUTOEN,
dev_name(&pdev->dev), keypad);
if (error) {
dev_err(&pdev->dev, "failed to register keypad interrupt\n");
@@ -499,6 +496,9 @@ static void samsung_keypad_toggle_wakeup(struct samsung_keypad *keypad,
val &= ~SAMSUNG_KEYIFCON_WAKEUPEN;
writel(val, keypad->base + SAMSUNG_KEYIFCON);
disable_irq_wake(keypad->irq);
+
+ if (!input_device_enabled(keypad->input_dev))
+ writel(~0x0, keypad->base + SAMSUNG_KEYIFSTSCLR);
}
clk_disable(keypad->clk);
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 3/3] Input: samsung-keypad - use pm_runtime_active guard
From: Dmitry Torokhov @ 2026-07-14 5:39 UTC (permalink / raw)
To: linux-input; +Cc: Krzysztof Kozlowski, linux-kernel
In-Reply-To: <20260713-samsung-kp-irq-v2-0-acc84b6daf9a@gmail.com>
Simplify the driver by using the block-scope guard(pm_runtime_active)
instead of manually invoking pm_runtime_get_sync() and
pm_runtime_put().
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/samsung-keypad.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index a51f0f639e9e..deffa98749c0 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -142,7 +142,7 @@ static irqreturn_t samsung_keypad_irq(int irq, void *dev_id)
unsigned int row_state[SAMSUNG_MAX_COLS];
bool key_down;
- pm_runtime_get_sync(&keypad->pdev->dev);
+ guard(pm_runtime_active)(&keypad->pdev->dev);
do {
readl(keypad->base + SAMSUNG_KEYIFSTSCLR);
@@ -158,8 +158,6 @@ static irqreturn_t samsung_keypad_irq(int irq, void *dev_id)
} while (key_down && !keypad->stopped);
- pm_runtime_put(&keypad->pdev->dev);
-
return IRQ_HANDLED;
}
@@ -167,7 +165,7 @@ static void samsung_keypad_start(struct samsung_keypad *keypad)
{
unsigned int val;
- pm_runtime_get_sync(&keypad->pdev->dev);
+ guard(pm_runtime_active)(&keypad->pdev->dev);
/* Tell IRQ thread that it may poll the device. */
keypad->stopped = false;
@@ -182,8 +180,6 @@ static void samsung_keypad_start(struct samsung_keypad *keypad)
/* KEYIFCOL reg clear. */
writel(0, keypad->base + SAMSUNG_KEYIFCOL);
- pm_runtime_put(&keypad->pdev->dev);
-
enable_irq(keypad->irq);
}
@@ -191,7 +187,7 @@ static void samsung_keypad_stop(struct samsung_keypad *keypad)
{
unsigned int val;
- pm_runtime_get_sync(&keypad->pdev->dev);
+ guard(pm_runtime_active)(&keypad->pdev->dev);
/* Signal IRQ thread to stop polling and disable the handler. */
keypad->stopped = true;
@@ -207,8 +203,6 @@ static void samsung_keypad_stop(struct samsung_keypad *keypad)
writel(val, keypad->base + SAMSUNG_KEYIFCON);
clk_disable(keypad->clk);
-
- pm_runtime_put(&keypad->pdev->dev);
}
static int samsung_keypad_open(struct input_dev *input_dev)
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 1/3] Input: samsung-keypad - clean up wakeup configuration logic
From: Dmitry Torokhov @ 2026-07-14 5:39 UTC (permalink / raw)
To: linux-input; +Cc: Krzysztof Kozlowski, linux-kernel
In-Reply-To: <20260713-samsung-kp-irq-v2-0-acc84b6daf9a@gmail.com>
When checking if the device can wake the system, we should pull the
device_may_wakeup() check to the caller instead of repeating it inside
the toggle_wakeup() handler. Furthermore, when configuring the wakeup,
we should safely ensure we write to the registers in the correct order:
configure the interrupt receiver before enabling the peripheral's wake
functionality, and vice-versa.
Assisted-by: Antigravity:gemini-3.1-pro
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/samsung-keypad.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index 17127269e3f0..a578f429d100 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -492,15 +492,14 @@ static void samsung_keypad_toggle_wakeup(struct samsung_keypad *keypad,
val = readl(keypad->base + SAMSUNG_KEYIFCON);
if (enable) {
+ enable_irq_wake(keypad->irq);
val |= SAMSUNG_KEYIFCON_WAKEUPEN;
- if (device_may_wakeup(&keypad->pdev->dev))
- enable_irq_wake(keypad->irq);
+ writel(val, keypad->base + SAMSUNG_KEYIFCON);
} else {
val &= ~SAMSUNG_KEYIFCON_WAKEUPEN;
- if (device_may_wakeup(&keypad->pdev->dev))
- disable_irq_wake(keypad->irq);
+ writel(val, keypad->base + SAMSUNG_KEYIFCON);
+ disable_irq_wake(keypad->irq);
}
- writel(val, keypad->base + SAMSUNG_KEYIFCON);
clk_disable(keypad->clk);
}
@@ -516,7 +515,8 @@ static int samsung_keypad_suspend(struct device *dev)
if (input_device_enabled(input_dev))
samsung_keypad_stop(keypad);
- samsung_keypad_toggle_wakeup(keypad, true);
+ if (device_may_wakeup(dev))
+ samsung_keypad_toggle_wakeup(keypad, true);
return 0;
}
@@ -529,7 +529,8 @@ static int samsung_keypad_resume(struct device *dev)
guard(mutex)(&input_dev->mutex);
- samsung_keypad_toggle_wakeup(keypad, false);
+ if (device_may_wakeup(dev))
+ samsung_keypad_toggle_wakeup(keypad, false);
if (input_device_enabled(input_dev))
samsung_keypad_start(keypad);
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH v2 0/3] Input: samsung-keypad - interrupt and PM fixes
From: Dmitry Torokhov @ 2026-07-14 5:39 UTC (permalink / raw)
To: linux-input; +Cc: Krzysztof Kozlowski, linux-kernel
This series improves the Samsung keypad driver's interrupt handling
and power management.
Changes in v2:
- Fix incorrect clearing of keypad status register during system resume
which caused wakeup key events to be discarded. We now only clear it
if the device is closed.
- We do not expect runtime suspend/resume to fail in this driver, and
power domains are not expected to interfere, so no additional PM
error handling is introduced.
Link to v1:
https://lore.kernel.org/r/20260714015309.2140694-1-dmitry.torokhov@gmail.com
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Dmitry Torokhov (3):
Input: samsung-keypad - clean up wakeup configuration logic
Input: samsung-keypad - keep interrupt disabled while closed
Input: samsung-keypad - use pm_runtime_active guard
drivers/input/keyboard/samsung-keypad.c | 39 ++++++++++++++-------------------
1 file changed, 17 insertions(+), 22 deletions(-)
---
base-commit: 49362394dad7df66c274c867a271394c10ca2bb8
change-id: 20260713-samsung-kp-irq-d9b2652f0b88
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 3/3] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: sashiko-bot @ 2026-07-14 5:34 UTC (permalink / raw)
To: Fenglin Wu; +Cc: devicetree, dmitry.torokhov, robh, linux-input, conor+dt
In-Reply-To: <20260713-qcom-spmi-haptics-v3-3-c931bb7cb94f@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] AB-BA deadlock in the error path of haptics_start_fifo() between `h->fifo_lock` and `disable_irq()`.
- [High] Failure to write to the FIFO in the IRQ handler leaves the interrupt condition asserted, leading to an interrupt storm.
- [Medium] Stop requests for inactive effects incorrectly terminate the currently playing effect.
- [Low] Mixing `goto`-based error handling with `guard()` scope-based cleanup in `haptics_start_fifo()`.
--
--- Patch [3]: [PATCH v3 3/3] input: misc: Add Qualcomm SPMI PMIC haptics driver ---
commit 516c3c8b367d0b2e322ea97c78aee0907ed541f0
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
input: misc: Add Qualcomm SPMI PMIC haptics driver
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.
> diff --git a/drivers/input/misc/qcom-spmi-haptics.c b/drivers/input/misc/qcom-spmi-haptics.c
> new file mode 100644
> index 0000000000000..ffbef7453459a
> --- /dev/null
> +++ b/drivers/input/misc/qcom-spmi-haptics.c
> @@ -0,0 +1,1160 @@
[ ... ]
> +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);
[Severity: Low]
Is it intended to mix goto-based error handling with scope-based guard()
cleanup here? The subsystem cleanup guidelines specify that functions should
use either goto-based cleanups or scope-based cleanups, but never both in
the same function.
> + 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. A threshold of 0 disables the IRQ.
> + */
> + 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)
> + goto free_fifo;
> +
> + 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);
> + goto fail;
> + }
> +
> + return 0;
> +
> +fail:
> + ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
> + haptics_fifo_irq_enable(h, false);
> +free_fifo:
> + h->fifo_data = NULL;
> + return ret;
> +}
[Severity: High]
Can this sequence cause a deadlock? In haptics_start_fifo(), h->fifo_lock is
still held via the guard() scope when jumping to the fail label.
The code then calls haptics_fifo_irq_enable() which disables the IRQ
synchronously using disable_irq().
If the threaded IRQ handler haptics_fifo_empty_irq() is concurrently running,
it will block waiting for h->fifo_lock, while disable_irq() blocks waiting
for the handler to complete.
[ ... ]
> +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);
> + return IRQ_HANDLED;
> + }
[Severity: High]
If haptics_write_fifo_chunk() fails here, could this cause an interrupt
storm?
By returning IRQ_HANDLED without writing samples to the FIFO or disabling
the empty threshold via ptn_write(), the hardware FIFO occupancy remains
below the threshold.
Once the IRQ is unmasked upon return, the hardware will immediately trigger
the interrupt again.
[ ... ]
> +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);
> +
> + if (req && 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 {
> + haptics_stop_locked(h, false);
> + }
[Severity: Medium]
Will this unconditionally stop the active effect even if the stop request
was for a different effect?
Since the ff-core tracks multiple effects, an application might send a stop
command for an inactive effect. The else block here does not verify if
req->effect_id matches h->active_effect_id before terminating the hardware
playback.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-qcom-spmi-haptics-v3-0-c931bb7cb94f@oss.qualcomm.com?part=3
^ permalink raw reply
* Re: [PATCH v3 1/3] dt-bindings: input: Add Qualcomm SPMI PMIC haptics
From: sashiko-bot @ 2026-07-14 5:31 UTC (permalink / raw)
To: Fenglin Wu; +Cc: conor+dt, devicetree, dmitry.torokhov, linux-input, robh
In-Reply-To: <20260713-qcom-spmi-haptics-v3-1-c931bb7cb94f@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing Parent MFD Schema Update for qcom,spmi-haptics
--
--- Patch [1]: [PATCH v3 1/3] dt-bindings: input: Add Qualcomm SPMI PMIC haptics ---
commit f137781e6c2b5a0d469e1be3c8ce374d67796f68
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
dt-bindings: input: Add Qualcomm SPMI PMIC haptics
This patch adds a new device tree binding document for the haptics module
integrated into Qualcomm PMICs. It covers hardware variants like the
HAP530_HV module in the PMIH0108 PMIC.
> 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 0000000000000..b1a69e9886ee6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
[ ... ]
> +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>;
[Severity: Medium]
Will this new haptics node cause validation failures in the parent PMIC node?
The parent MFD binding (Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml)
enforces strict child node validation using additionalProperties: false.
Because this patch introduces a new sub-device schema without adding a
corresponding pattern (like ^haptics@[0-9a-f]+$) to the parent schema's
patternProperties, it seems any actual device tree utilizing this haptics
node will fail dtbs_check with an 'Unevaluated properties are not allowed'
error.
Does the parent schema need to be updated in the same series to permit this
node?
> + 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>;
> + };
> + };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713-qcom-spmi-haptics-v3-0-c931bb7cb94f@oss.qualcomm.com?part=1
^ permalink raw reply
* [PATCH v3 3/3] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: Fenglin Wu @ 2026-07-14 5:22 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: <20260713-qcom-spmi-haptics-v3-0-c931bb7cb94f@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 | 1160 ++++++++++++++++++++++++++++++++
3 files changed, 1172 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..ffbef7453459
--- /dev/null
+++ b/drivers/input/misc/qcom-spmi-haptics.c
@@ -0,0 +1,1160 @@
+// 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. A threshold of 0 disables the IRQ.
+ */
+ 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)
+ goto free_fifo;
+
+ 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);
+ goto fail;
+ }
+
+ return 0;
+
+fail:
+ ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
+ haptics_fifo_irq_enable(h, false);
+free_fifo:
+ h->fifo_data = NULL;
+ return ret;
+}
+
+/*
+ * 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);
+ 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);
+
+ if (req && 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 {
+ 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 v3 2/3] dt-bindings: mfd: qcom,spmi-pmic: Document haptics device
From: Fenglin Wu @ 2026-07-14 5:22 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: <20260713-qcom-spmi-haptics-v3-0-c931bb7cb94f@oss.qualcomm.com>
Some of the Qualcomm SPMI PMIC has haptics device in it, add it in the
device list.
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 v3 0/3] input: misc: Add an initial driver for haptics inside Qcom PMIH010x PMIC
From: Fenglin Wu @ 2026-07-14 5:22 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
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 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 | 1160 ++++++++++++++++++++
5 files changed, 1312 insertions(+)
---
base-commit: 66725039f7090afe14c31bd259e2059a68f04023
change-id: 20260616-qcom-spmi-haptics-3cc97e7b232e
Best regards,
--
Fenglin Wu <fenglin.wu@oss.qualcomm.com>
^ permalink raw reply
* [PATCH v3 1/3] dt-bindings: input: Add Qualcomm SPMI PMIC haptics
From: Fenglin Wu @ 2026-07-14 5:22 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: <20260713-qcom-spmi-haptics-v3-0-c931bb7cb94f@oss.qualcomm.com>
Add binding document for the haptics module inside Qualcomm PMIC
PMIH0108.
Assisted-by: Claude:claude-4-8-opus
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 3/3] Input: samsung-keypad - use pm_runtime_active guard
From: sashiko-bot @ 2026-07-14 2:08 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260714015309.2140694-3-dmitry.torokhov@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Missing Error Handling for pm_runtime_get_sync() implicitly called by guard(pm_runtime_active)
--
--- Patch [3]: [PATCH 3/3] Input: samsung-keypad - use pm_runtime_active guard ---
commit dabb4cc7edb73abb241db678d97750949e93ef10
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Input: samsung-keypad - use pm_runtime_active guard
Simplify the driver by using the block-scope guard(pm_runtime_active)
instead of manually invoking pm_runtime_get_sync() and
pm_runtime_put().
> diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
> index 88e733c1eabea..5f6535b49761c 100644
> --- a/drivers/input/keyboard/samsung-keypad.c
> +++ b/drivers/input/keyboard/samsung-keypad.c
> @@ -142,7 +142,7 @@ static irqreturn_t samsung_keypad_irq(int irq, void *dev_id)
> unsigned int row_state[SAMSUNG_MAX_COLS];
> bool key_down;
>
> - pm_runtime_get_sync(&keypad->pdev->dev);
> + guard(pm_runtime_active)(&keypad->pdev->dev);
>
> do {
> readl(keypad->base + SAMSUNG_KEYIFSTSCLR);
[Severity: High]
This isn't a bug introduced by this patch, but does ignoring the return value
of the implicit pm_runtime_get_sync() call here leave the hardware access
vulnerable to crashes?
If resuming the device fails (e.g., due to a clock or regulator error), the
driver proceeds to access memory-mapped registers like SAMSUNG_KEYIFSTSCLR.
Reading or writing these registers while the power domain is unpowered can
trigger a Data Abort (kernel panic) on ARM.
[ ... ]
> @@ -167,7 +165,7 @@ static void samsung_keypad_start(struct samsung_keypad *keypad)
> {
> unsigned int val;
>
> - pm_runtime_get_sync(&keypad->pdev->dev);
> + guard(pm_runtime_active)(&keypad->pdev->dev);
>
> /* Tell IRQ thread that it may poll the device. */
> keypad->stopped = false;
> @@ -182,8 +180,6 @@ static void samsung_keypad_start(struct samsung_keypad *keypad)
> /* KEYIFCOL reg clear. */
> writel(0, keypad->base + SAMSUNG_KEYIFCOL);
[Severity: High]
This is a pre-existing issue, but this function is reachable via userspace
opening the input device. If the resume fails, could the subsequent hardware
accesses like writel() to SAMSUNG_KEYIFCOL trigger a crash for the same
reason?
[ ... ]
> @@ -191,7 +187,7 @@ static void samsung_keypad_stop(struct samsung_keypad *keypad)
> {
> unsigned int val;
>
> - pm_runtime_get_sync(&keypad->pdev->dev);
> + guard(pm_runtime_active)(&keypad->pdev->dev);
>
> /* Signal IRQ thread to stop polling and disable the handler. */
> keypad->stopped = true;
> @@ -207,8 +203,6 @@ static void samsung_keypad_stop(struct samsung_keypad *keypad)
> writel(val, keypad->base + SAMSUNG_KEYIFCON);
>
> clk_disable(keypad->clk);
> -
> - pm_runtime_put(&keypad->pdev->dev);
> }
[Severity: High]
This is also a pre-existing issue, but does this face the same risk when
writing to SAMSUNG_KEYIFCON if the device failed to wake?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714015309.2140694-1-dmitry.torokhov@gmail.com?part=3
^ permalink raw reply
* Re: [PATCH 2/3] Input: samsung-keypad - keep interrupt disabled while closed
From: sashiko-bot @ 2026-07-14 2:08 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260714015309.2140694-2-dmitry.torokhov@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The patch unconditionally clears the pending hardware interrupt during system resume even when the keypad is open, discarding legitimate wakeup key events.
Pre-existing issues:
- [High] Missing check for pm_runtime_get_sync() return value in hardware access paths leads to clock-gated hardware access.
--
--- Patch [2]: [PATCH 2/3] Input: samsung-keypad - keep interrupt disabled while closed ---
commit 4f3c0f99ee50c72647a3ac611009b6d19d9c230d
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Input: samsung-keypad - keep interrupt disabled while closed
The driver requests the interrupt during probe, which by default enables
it. If the bootloader left the keypad interrupts enabled, or if a
spurious interrupt fires early before the driver is fully initialized
and clocks are enabled, the interrupt handler will attempt to read
registers and may cause a synchronous external abort. Fix this by
requesting the interrupt with IRQF_NO_AUTOEN, keeping it disabled during
probe.
> diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
> index a578f429d1009..88e733c1eabea 100644
> --- a/drivers/input/keyboard/samsung-keypad.c
> +++ b/drivers/input/keyboard/samsung-keypad.c
> @@ -183,6 +183,8 @@ static void samsung_keypad_start(struct samsung_keypad *keypad)
> writel(0, keypad->base + SAMSUNG_KEYIFCOL);
>
> pm_runtime_put(&keypad->pdev->dev);
[Severity: High]
This is a pre-existing issue, but I noticed that earlier in
samsung_keypad_start() (as well as in samsung_keypad_stop() and
samsung_keypad_irq()), we seem to call pm_runtime_get_sync() without
checking its return value.
If runtime PM fails to resume the device (for example, due to a power
domain failure), the device clock remains disabled.
Could this lead to a kernel panic from a synchronous external abort when
the driver subsequently accesses hardware registers via readl() and writel()
on disabled hardware?
> +
> + enable_irq(keypad->irq);
> }
[ ... ]
> @@ -499,6 +496,9 @@ static void samsung_keypad_toggle_wakeup(struct samsung_keypad *keypad,
> val &= ~SAMSUNG_KEYIFCON_WAKEUPEN;
> writel(val, keypad->base + SAMSUNG_KEYIFCON);
> disable_irq_wake(keypad->irq);
> +
> + if (keypad->stopped)
> + writel(~0x0, keypad->base + SAMSUNG_KEYIFSTSCLR);
[Severity: High]
Does this unconditionally clear the pending hardware interrupt during system
resume, even for open devices, potentially discarding legitimate wakeup
events?
During system suspend, samsung_keypad_suspend() stops open devices, which
sets keypad->stopped = true:
samsung_keypad_suspend()
if (input_device_enabled(input_dev))
samsung_keypad_stop(keypad);
On resume, samsung_keypad_resume() calls samsung_keypad_toggle_wakeup(..., false)
before restarting the device:
samsung_keypad_resume()
if (device_may_wakeup(dev))
samsung_keypad_toggle_wakeup(keypad, false);
if (input_device_enabled(input_dev))
samsung_keypad_start(keypad);
Because keypad->stopped is still true here for both closed and open devices,
the if (keypad->stopped) condition evaluates to true.
Will this execute writel(~0x0, keypad->base + SAMSUNG_KEYIFSTSCLR) and
silently discard the keystroke that woke the system before the IRQ is
re-enabled by samsung_keypad_start()?
> }
>
> clk_disable(keypad->clk);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260714015309.2140694-1-dmitry.torokhov@gmail.com?part=2
^ permalink raw reply
* [PATCH 3/3] Input: samsung-keypad - use pm_runtime_active guard
From: Dmitry Torokhov @ 2026-07-14 1:53 UTC (permalink / raw)
To: linux-input; +Cc: Krzysztof Kozlowski, linux-kernel
In-Reply-To: <20260714015309.2140694-1-dmitry.torokhov@gmail.com>
Simplify the driver by using the block-scope guard(pm_runtime_active)
instead of manually invoking pm_runtime_get_sync() and
pm_runtime_put().
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/samsung-keypad.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index 88e733c1eabe..5f6535b49761 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -142,7 +142,7 @@ static irqreturn_t samsung_keypad_irq(int irq, void *dev_id)
unsigned int row_state[SAMSUNG_MAX_COLS];
bool key_down;
- pm_runtime_get_sync(&keypad->pdev->dev);
+ guard(pm_runtime_active)(&keypad->pdev->dev);
do {
readl(keypad->base + SAMSUNG_KEYIFSTSCLR);
@@ -158,8 +158,6 @@ static irqreturn_t samsung_keypad_irq(int irq, void *dev_id)
} while (key_down && !keypad->stopped);
- pm_runtime_put(&keypad->pdev->dev);
-
return IRQ_HANDLED;
}
@@ -167,7 +165,7 @@ static void samsung_keypad_start(struct samsung_keypad *keypad)
{
unsigned int val;
- pm_runtime_get_sync(&keypad->pdev->dev);
+ guard(pm_runtime_active)(&keypad->pdev->dev);
/* Tell IRQ thread that it may poll the device. */
keypad->stopped = false;
@@ -182,8 +180,6 @@ static void samsung_keypad_start(struct samsung_keypad *keypad)
/* KEYIFCOL reg clear. */
writel(0, keypad->base + SAMSUNG_KEYIFCOL);
- pm_runtime_put(&keypad->pdev->dev);
-
enable_irq(keypad->irq);
}
@@ -191,7 +187,7 @@ static void samsung_keypad_stop(struct samsung_keypad *keypad)
{
unsigned int val;
- pm_runtime_get_sync(&keypad->pdev->dev);
+ guard(pm_runtime_active)(&keypad->pdev->dev);
/* Signal IRQ thread to stop polling and disable the handler. */
keypad->stopped = true;
@@ -207,8 +203,6 @@ static void samsung_keypad_stop(struct samsung_keypad *keypad)
writel(val, keypad->base + SAMSUNG_KEYIFCON);
clk_disable(keypad->clk);
-
- pm_runtime_put(&keypad->pdev->dev);
}
static int samsung_keypad_open(struct input_dev *input_dev)
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH 2/3] Input: samsung-keypad - keep interrupt disabled while closed
From: Dmitry Torokhov @ 2026-07-14 1:53 UTC (permalink / raw)
To: linux-input; +Cc: Krzysztof Kozlowski, linux-kernel
In-Reply-To: <20260714015309.2140694-1-dmitry.torokhov@gmail.com>
The driver requests the interrupt during probe, which by default enables
it. If the bootloader left the keypad interrupts enabled, or if a
spurious interrupt fires early before the driver is fully initialized
and clocks are enabled, the interrupt handler will attempt to read
registers and may cause a synchronous external abort.
Fix this by requesting the interrupt with IRQF_NO_AUTOEN, keeping it
disabled during probe. Enable the interrupt in samsung_keypad_start()
when the device is opened and ready, and disable it in
samsung_keypad_stop() when the device is closed. Remove the redundant
re-enabling of the interrupt at the end of samsung_keypad_stop().
Additionally, manually clear the pending interrupt status during system
resume when the device is closed to avoid immediate resume.
Fixes: 0fffed27f92d ("Input: samsung-keypad - Add samsung keypad driver")
Assisted-by: Antigravity:gemini-3.1-pro
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/samsung-keypad.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index a578f429d100..88e733c1eabe 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -183,6 +183,8 @@ static void samsung_keypad_start(struct samsung_keypad *keypad)
writel(0, keypad->base + SAMSUNG_KEYIFCOL);
pm_runtime_put(&keypad->pdev->dev);
+
+ enable_irq(keypad->irq);
}
static void samsung_keypad_stop(struct samsung_keypad *keypad)
@@ -206,12 +208,6 @@ static void samsung_keypad_stop(struct samsung_keypad *keypad)
clk_disable(keypad->clk);
- /*
- * Now that chip should not generate interrupts we can safely
- * re-enable the handler.
- */
- enable_irq(keypad->irq);
-
pm_runtime_put(&keypad->pdev->dev);
}
@@ -412,7 +408,8 @@ static int samsung_keypad_probe(struct platform_device *pdev)
}
error = devm_request_threaded_irq(&pdev->dev, keypad->irq, NULL,
- samsung_keypad_irq, IRQF_ONESHOT,
+ samsung_keypad_irq,
+ IRQF_ONESHOT | IRQF_NO_AUTOEN,
dev_name(&pdev->dev), keypad);
if (error) {
dev_err(&pdev->dev, "failed to register keypad interrupt\n");
@@ -499,6 +496,9 @@ static void samsung_keypad_toggle_wakeup(struct samsung_keypad *keypad,
val &= ~SAMSUNG_KEYIFCON_WAKEUPEN;
writel(val, keypad->base + SAMSUNG_KEYIFCON);
disable_irq_wake(keypad->irq);
+
+ if (keypad->stopped)
+ writel(~0x0, keypad->base + SAMSUNG_KEYIFSTSCLR);
}
clk_disable(keypad->clk);
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* [PATCH 1/3] Input: samsung-keypad - clean up wakeup configuration logic
From: Dmitry Torokhov @ 2026-07-14 1:53 UTC (permalink / raw)
To: linux-input; +Cc: Krzysztof Kozlowski, linux-kernel
When checking if the device can wake the system, we should pull the
device_may_wakeup() check to the caller instead of repeating it inside
the toggle_wakeup() handler. Furthermore, when configuring the wakeup,
we should safely ensure we write to the registers in the correct order:
configure the interrupt receiver before enabling the peripheral's wake
functionality, and vice-versa.
Assisted-by: Antigravity:gemini-3.1-pro
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/keyboard/samsung-keypad.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/input/keyboard/samsung-keypad.c b/drivers/input/keyboard/samsung-keypad.c
index 17127269e3f0..a578f429d100 100644
--- a/drivers/input/keyboard/samsung-keypad.c
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -492,15 +492,14 @@ static void samsung_keypad_toggle_wakeup(struct samsung_keypad *keypad,
val = readl(keypad->base + SAMSUNG_KEYIFCON);
if (enable) {
+ enable_irq_wake(keypad->irq);
val |= SAMSUNG_KEYIFCON_WAKEUPEN;
- if (device_may_wakeup(&keypad->pdev->dev))
- enable_irq_wake(keypad->irq);
+ writel(val, keypad->base + SAMSUNG_KEYIFCON);
} else {
val &= ~SAMSUNG_KEYIFCON_WAKEUPEN;
- if (device_may_wakeup(&keypad->pdev->dev))
- disable_irq_wake(keypad->irq);
+ writel(val, keypad->base + SAMSUNG_KEYIFCON);
+ disable_irq_wake(keypad->irq);
}
- writel(val, keypad->base + SAMSUNG_KEYIFCON);
clk_disable(keypad->clk);
}
@@ -516,7 +515,8 @@ static int samsung_keypad_suspend(struct device *dev)
if (input_device_enabled(input_dev))
samsung_keypad_stop(keypad);
- samsung_keypad_toggle_wakeup(keypad, true);
+ if (device_may_wakeup(dev))
+ samsung_keypad_toggle_wakeup(keypad, true);
return 0;
}
@@ -529,7 +529,8 @@ static int samsung_keypad_resume(struct device *dev)
guard(mutex)(&input_dev->mutex);
- samsung_keypad_toggle_wakeup(keypad, false);
+ if (device_may_wakeup(dev))
+ samsung_keypad_toggle_wakeup(keypad, false);
if (input_device_enabled(input_dev))
samsung_keypad_start(keypad);
--
2.55.0.795.g602f6c329a-goog
^ permalink raw reply related
* Re: [PATCH v2 1/1] HID: logitech: add Bolt receiver support for Logitech HID++ devices
From: Erik Håkansson @ 2026-07-14 0:54 UTC (permalink / raw)
To: k8ie; +Cc: bentiss, hadess, jikos, lains, linux-input, linux-kernel
In-Reply-To: <bcbf28f0-a3da-454d-b05e-a554017fba0e@mcld.eu>
Hi,
Thank you very much for your help.
Unfortunately I don't have a Windows system easily accessible, so I
can't check my firmware version in Options+.
fwupdmgr says:
> Current version: MPR05.03_B0020
so perhaps that means 5.03.20.
Let me know if anything pops up if you dig some more,
Thanks,
Erik
^ permalink raw reply
* Re: [PATCH v2 1/1] HID: logitech: add Bolt receiver support for Logitech HID++ devices
From: Erik Håkansson @ 2026-07-14 0:50 UTC (permalink / raw)
To: sashiko-reviews; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260713201201.391538-2-erikhakan@gmail.com>
Hi,
Thanks for the review. I don't know if anyone reads replies to the bot
review, but I'll reply just in case.
This looks like a pre-existing teardown ordering issue in
hid-logitech-dj. The Bolt unpair path follows the same kfifo_in() +
schedule_work() pattern already used by the existing DJ/HID++ notification
paths.
I agree that fixing the remove ordering may be worthwhile, but I think it
should be handled as a separate patch because it affects all receiver
types, not only Bolt.
Regards,
Erik
^ permalink raw reply
* RE: [PATCH 4/4] HID: Intel-thc-hid: Intel-quickspi: Remove redundant dev_err()
From: Xu, Even @ 2026-07-14 0:50 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: <20260713132439.380121-5-panchuang@vivo.com>
Thanks for the patch!
If this is the case, can you also do the same change for intel-quicki2c driver?
Best Regards,
Even Xu
> -----Original Message-----
> From: Pan Chuang <panchuang@vivo.com>
> Sent: Monday, July 13, 2026 9:25 PM
> To: Xu, Even <even.xu@intel.com>; Sun, Xinpeng <xinpeng.sun@intel.com>; Jiri
> Kosina <jikos@kernel.org>; Benjamin Tissoires <bentiss@kernel.org>; Pan
> Chuang <panchuang@vivo.com>; Sakari Ailus <sakari.ailus@linux.intel.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 4/4] HID: Intel-thc-hid: Intel-quickspi: 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-quickspi/pci-quickspi.c | 5 +----
> 1 file changed, 1 insertion(+), 4 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..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;
> - }
>
> ret = reset_tic(qsdev);
> if (ret) {
> --
> 2.34.1
^ permalink raw reply
* Re: [PATCH v3 00/10] Remove support for platform data from samsung keypad
From: Dmitry Torokhov @ 2026-07-13 23:28 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Peter Griffin,
Alim Akhtar, Russell King, Mark Brown, Linus Walleij,
Charles Keepax, Sam Protsenko, Arnd Bergmann, Bartosz Golaszewski,
linux-input, devicetree, linux-kernel, linux-arm-kernel,
linux-samsung-soc, patches, Conor Dooley, Bartosz Golaszewski
In-Reply-To: <20260712-musical-shrimp-of-fruition-b921df@quoll>
On Sun, Jul 12, 2026 at 02:30:50PM +0200, Krzysztof Kozlowski wrote:
> On Sat, Jul 11, 2026 at 07:12:41PM -0700, Dmitry Torokhov wrote:
> > Hi,
> >
> > This series converts the Samsung keypad driver and the Cragganmore 6410
> > board to generic device properties, allowing the removal of legacy platform
> > data and GPIO lookup tables.
> >
> > We rework the Samsung keypad driver to stop using platform data, utilizing
> > a new compact matrix keypad binding. To support this on the Cragganmore
> > board, we add infrastructure to associate software nodes with Samsung
> > gpio_chips, and add software node propagation support to the wm831x MFD
> > core to route DVS GPIO to the PMIC regulator.
> >
> > Finally, we convert all peripherals on the Cragganmore board (including
> > keypad, GPIO keys, PMIC, WM1250, SPI0, basic-mmio-gpio, and LEDs) to
> > software properties. This allows us to eliminate all legacy GPIO lookup
> > tables and fixes incorrect GPIO chip names from previous conversions.
> >
> > Mark, it would be great if you could give this a spin on Cragganmore.
> >
> > The majority of changes are in arch/board code, with a couple of input
> > matches and one for MFD. If everyone agrees maybe we can merge it all
> > through the soc tree?
>
> Yes, I could take it via samsung-soc. I assume I have your acks for
> unput parts.
Yes, of course.
Regarding Sashiko review:
- IRQ concerns are pre-existing and will be addressed in a separate
series
- TOCTOU with device tree overlay changing DT properties in the middle
of the probe I will not attempt to address. If this is a real concern
it should be addressed globally
- Introduction of 5ms default debounce on proximity and suspend seem
harmless
- Copying of MFD cells is necessary because we need to "mix in" software
nodes from platform data. I originally tried to handle this is the
regulator, but the result was worse as it needed to add software node
to the device after it was created. Doing it in the core is much
cleaner overall.
Thanks.
--
Dmitry
^ permalink raw reply
* Re: [PATCH v3 1/2] HID: uclogic: fix UAF on inrange_timer at teardown and probe error
From: sashiko-bot @ 2026-07-13 22:17 UTC (permalink / raw)
To: Doruk Tan Ozturk; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260713215950.24193-1-doruk@0sec.ai>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free on `drvdata->pen_input` during input registration failure.
- [Medium] Memory leak of `drvdata->desc_ptr` on probe error path.
--
--- Patch [1]: [PATCH v3 1/2] HID: uclogic: fix UAF on inrange_timer at teardown and probe error ---
commit 2397dc01e99d0cc8ef59607ebbc881c2a6cb3354
Author: Doruk Tan Ozturk <doruk@0sec.ai>
HID: uclogic: fix UAF on inrange_timer at teardown and probe error
uclogic_probe() arms a per-device timer whose callback
uclogic_inrange_timeout() dereferences drvdata->pen_input, and
uclogic_raw_event_pen() re-arms it with a 100 ms timeout on every
in-range pen report.
[ ... ]
> diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
> index b73f09d26688a..d74f98efa8794 100644
> --- a/drivers/hid/hid-uclogic-core.c
> +++ b/drivers/hid/hid-uclogic-core.c
> @@ -267,6 +267,13 @@ static int uclogic_probe(struct hid_device *hdev,
> /* Assume "remove" might not be called if "probe" failed */
> if (params_initialized)
> uclogic_params_cleanup(&drvdata->params);
[Severity: Medium]
This is a pre-existing issue, but does this error path leak the dynamically
allocated replacement report descriptor buffer?
In uclogic_probe(), uclogic_params_get_desc() allocates a concatenated report
descriptor and assigns it to drvdata->desc_ptr. If subsequent operations such
as hid_parse() or hid_hw_start() fail, the code jumps to the failure path.
The cleanup logic calls uclogic_params_cleanup(&drvdata->params), which frees
params->desc_ptr, but it does not free drvdata->desc_ptr.
(Note: The commit message claims a 2/2 patch fixes this, but that patch is
absent from the provided repository state at the end of the series).
> + /*
> + * If hid_hw_start() started I/O and then failed, raw_event may have
> + * armed the timer; shut it down so it cannot fire on the devm-freed
> + * drvdata after probe returns.
> + */
> + if (drvdata)
> + timer_shutdown_sync(&drvdata->inrange_timer);
[Severity: High]
This is a pre-existing issue, but can this lead to a Use-After-Free on
drvdata->pen_input if input registration fails?
During uclogic_probe(), hid_hw_start() calls hid_connect() which attempts to
register input devices via hidinput_connect(). As part of this process,
uclogic_input_configured() assigns the allocated input_dev to
drvdata->pen_input:
drivers/hid/hid-uclogic-core.c:uclogic_input_configured() {
...
if (hi->report->id == params->pen.id) {
/* Remember the input device so we can simulate events */
drvdata->pen_input = hi->input;
}
...
}
If input_register_device() fails, the code unwinds and frees the input_dev
structure via hidinput_disconnect(), but drvdata->pen_input is never cleared
and remains dangling.
Because the uclogic_driver defines a raw_event callback, hid_connect() still
returns success (0) even if no listeners successfully claimed the device.
Consequently, hid_hw_start() and the driver probe succeed.
If the USB device subsequently sends an in-range pen report,
uclogic_raw_event_pen() rearms inrange_timer. When the timer expires,
uclogic_inrange_timeout() unconditionally dereferences the dangling
drvdata->pen_input pointer:
drivers/hid/hid-uclogic-core.c:uclogic_inrange_timeout() {
...
struct input_dev *input = drvdata->pen_input;
if (input == NULL)
return;
input_report_abs(input, ABS_PRESSURE, 0);
...
}
> return rc;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713215950.24193-1-doruk@0sec.ai?part=1
^ permalink raw reply
* Re: [PATCH v3 2/2] HID: uclogic: fix desc_ptr leak in probe error path
From: sashiko-bot @ 2026-07-13 22:12 UTC (permalink / raw)
To: Doruk Tan Ozturk; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260713215950.24193-2-doruk@0sec.ai>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free of `drvdata->pen_input` in `uclogic_inrange_timeout()` when `hidinput_connect()` fails during probe.
--
--- Patch [2]: [PATCH v3 2/2] HID: uclogic: fix desc_ptr leak in probe error path ---
commit b37d55bfcac132529f188bc93436f75e1da9c66f
Author: Doruk Tan Ozturk <doruk@0sec.ai>
HID: uclogic: fix desc_ptr leak in probe error path
This commit fixes a memory leak in the uclogic_probe() error path. It
ensures that drvdata->desc_ptr is explicitly freed when probe fails, since
the normal cleanup in uclogic_remove() is not called in this scenario.
> diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
> index d74f98efa8794..ad9914ed0c717 100644
> --- a/drivers/hid/hid-uclogic-core.c
> +++ b/drivers/hid/hid-uclogic-core.c
> @@ -265,8 +265,10 @@ static int uclogic_probe(struct hid_device *hdev,
> return 0;
> failure:
[Severity: High]
This is a pre-existing issue, but is there a use-after-free risk involving
drvdata->pen_input if hidinput_connect() fails during hid_hw_start()?
During probe, uclogic_input_configured() (drivers/hid/hid-uclogic-core.c)
stores a pointer to the input device before registration completes:
/* Remember the input device so we can simulate events */
drvdata->pen_input = hi->input;
If a subsequent input registration fails in hidinput_connect()
(drivers/hid/hid-input.c), the code unwinds and frees all inputs:
out_unwind:
/* unwind the ones we already registered */
hidinput_disconnect(hid);
return -1;
Because the driver sets .raw_event, hid_connect() ignores this -1 return
value. As a result, hid_hw_start() returns 0, the driver binds successfully,
and this failure block is bypassed.
When a pen report later arrives and arms drvdata->inrange_timer, the timer
callback uclogic_inrange_timeout() dereferences the freed pointer:
struct input_dev *input = drvdata->pen_input;
if (input == NULL)
return;
input_report_abs(input, ABS_PRESSURE, 0);
Could this sequence lead to a dangling pointer being dereferenced?
> /* Assume "remove" might not be called if "probe" failed */
> - if (params_initialized)
> + if (params_initialized) {
> + kfree(drvdata->desc_ptr);
> uclogic_params_cleanup(&drvdata->params);
> + }
> /*
> * If hid_hw_start() started I/O and then failed, raw_event may have
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260713215950.24193-1-doruk@0sec.ai?part=2
^ permalink raw reply
* [PATCH v3 2/2] HID: uclogic: fix desc_ptr leak in probe error path
From: Doruk Tan Ozturk @ 2026-07-13 21:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Nikolai Kondrashov, linux-input, linux-kernel, stable
In-Reply-To: <20260713215950.24193-1-doruk@0sec.ai>
uclogic_probe() calls uclogic_params_get_desc(), which stores a
kmalloc-allocated replacement report descriptor into drvdata->desc_ptr.
If a later init step (hid_parse() or hid_hw_start()) fails, probe jumps
to the "failure" label, which only runs uclogic_params_cleanup() on
drvdata->params and returns. The device core does not call
uclogic_remove() when probe fails, so the kfree(drvdata->desc_ptr) that
uclogic_remove() normally performs never runs, leaking the descriptor.
drvdata itself is devm-allocated and freed automatically, but desc_ptr
is a plain kmalloc/krealloc buffer and must be freed explicitly. Free it
on the probe error path. The kfree is gated on params_initialized, under
which desc_ptr is either NULL (a no-op) or the allocated descriptor.
Found by 0sec (https://0sec.ai).
Fixes: 9614219e9310 ("HID: uclogic: Extract tablet parameter discovery into a module")
Cc: stable@vger.kernel.org
Assisted-by: 0sec
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
drivers/hid/hid-uclogic-core.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index d74f98efa879..ad9914ed0c71 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -265,8 +265,10 @@ static int uclogic_probe(struct hid_device *hdev,
return 0;
failure:
/* Assume "remove" might not be called if "probe" failed */
- if (params_initialized)
+ if (params_initialized) {
+ kfree(drvdata->desc_ptr);
uclogic_params_cleanup(&drvdata->params);
+ }
/*
* If hid_hw_start() started I/O and then failed, raw_event may have
* armed the timer; shut it down so it cannot fire on the devm-freed
--
2.43.0
^ permalink raw reply related
* [PATCH v3 1/2] HID: uclogic: fix UAF on inrange_timer at teardown and probe error
From: Doruk Tan Ozturk @ 2026-07-13 21:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires
Cc: Nikolai Kondrashov, linux-input, linux-kernel, stable
uclogic_probe() arms a per-device timer whose callback
uclogic_inrange_timeout() dereferences drvdata->pen_input, and
uclogic_raw_event_pen() re-arms it with a 100 ms timeout on every
in-range pen report.
uclogic_remove() drained the timer with timer_delete_sync() before
hid_hw_stop(). timer_delete_sync() does not block re-arming: a pen
report delivered before hid_hw_stop() kills the URBs can re-arm the timer
after it was drained. hid_hw_stop() then frees the hidinput pen_input
(via hidinput_disconnect() -> input_unregister_device()), and the pending
timer fires on freed memory.
Use timer_shutdown_sync() instead, still before hid_hw_stop(). It drains
the callback while pen_input is still valid and permanently blocks
re-arming, so an in-flight raw_event cannot revive the timer; hid_hw_stop()
then frees pen_input with the timer already dead.
The probe error path had the same exposure: if hid_hw_start() started I/O
and then failed, raw_event may have armed the timer, which would fire on
the devm-freed drvdata after probe returns. Shut the timer down there too.
Unlike letsketch, whose input devices are devm-allocated and outlive
hid_hw_stop(), uclogic's pen_input is freed inside hid_hw_stop(), so the
timer must be shut down before it rather than after.
Found by 0sec (https://0sec.ai).
Fixes: 01309e29eb95 ("HID: uclogic: Support in-range reporting emulation")
Cc: stable@vger.kernel.org
Assisted-by: 0sec
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
v3: resend as a 2-patch series. 1/2 is the v2 timer fix, unchanged. 2/2 adds
the desc_ptr leak fix in the probe error path, flagged by the Sashiko AI
review on v2. No functional change to this patch since v2.
v2: shut the timer down on the probe error path too, and clarify in the commit
message why uclogic differs from the letsketch precedent (pen_input is
freed inside hid_hw_stop(), so the timer must be shut down before it).
drivers/hid/hid-uclogic-core.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/hid/hid-uclogic-core.c b/drivers/hid/hid-uclogic-core.c
index b73f09d26688..d74f98efa879 100644
--- a/drivers/hid/hid-uclogic-core.c
+++ b/drivers/hid/hid-uclogic-core.c
@@ -267,6 +267,13 @@ static int uclogic_probe(struct hid_device *hdev,
/* Assume "remove" might not be called if "probe" failed */
if (params_initialized)
uclogic_params_cleanup(&drvdata->params);
+ /*
+ * If hid_hw_start() started I/O and then failed, raw_event may have
+ * armed the timer; shut it down so it cannot fire on the devm-freed
+ * drvdata after probe returns.
+ */
+ if (drvdata)
+ timer_shutdown_sync(&drvdata->inrange_timer);
return rc;
}
@@ -548,7 +555,15 @@ static void uclogic_remove(struct hid_device *hdev)
{
struct uclogic_drvdata *drvdata = hid_get_drvdata(hdev);
- timer_delete_sync(&drvdata->inrange_timer);
+ /*
+ * timer_delete_sync() does not prevent re-arming, so a pen report
+ * delivered before hid_hw_stop() kills the URBs could re-arm the
+ * timer; hid_hw_stop() then frees the hidinput pen_input and the
+ * pending timer fires on freed memory. timer_shutdown_sync() drains
+ * the callback while pen_input is still valid and permanently blocks
+ * re-arming, so an in-flight raw_event cannot revive it.
+ */
+ timer_shutdown_sync(&drvdata->inrange_timer);
hid_hw_stop(hdev);
kfree(drvdata->desc_ptr);
uclogic_params_cleanup(&drvdata->params);
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v2 1/1] HID: logitech: add Bolt receiver support for Logitech HID++ devices
From: Kateřina Medvědová @ 2026-07-13 21:09 UTC (permalink / raw)
To: erikhakan; +Cc: bentiss, hadess, jikos, k8ie, lains, linux-input, linux-kernel
In-Reply-To: <20260713201201.391538-2-erikhakan@gmail.com>
Hi,
I tested your V2 and both issues were resolved.
Not sure what caused issue number 1 on my system. I can try to dig
deeper if it would help.
I have 3 Bolt receivers on hand. Two of them have much newer firmware
(5.3.20, revision 3). The other one is on 5.0.9. Unfortunately I'm not
sure which of them I was testing on.
AFAIK the only way to get this information is through Options+.
I'm so glad you're taking this on! I'll keep this on my system for daily
driving, I love that the battery reporting works!
Thanks,
Kateřina Medvědová
^ 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