* Re: [PATCH v2 2/3] dt-bindings: mfd: qcom,spmi-pmic: Document haptics device
From: Rob Herring (Arm) @ 2026-06-25 3:32 UTC (permalink / raw)
To: Fenglin Wu
Cc: Subbaraman Narayanamurthy, Lee Jones, devicetree, kernel,
Dmitry Torokhov, Bjorn Andersson, linux-kernel, David Collins,
Kamal Wadhwa, Conor Dooley, Krzysztof Kozlowski, linux-input,
Stephen Boyd, Konrad Dybcio, linux-arm-msm
In-Reply-To: <20260624-qcom-spmi-haptics-v2-2-b9118e60f3e3@oss.qualcomm.com>
On Wed, 24 Jun 2026 19:00:37 -0700, Fenglin Wu wrote:
> 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(+)
>
My bot found errors running 'make dt_binding_check' on your patch:
yamllint warnings/errors:
dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml: properties:qcom,vmax-microvolt: '$ref' should not be valid under {'const': '$ref'}
hint: Standard unit suffix properties don't need a type $ref
from schema $id: http://devicetree.org/meta-schemas/core.yaml
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/input/qcom,spmi-haptics.example.dtb: haptics@f000 (qcom,spmi-haptics): qcom,vmax-microvolt: 1300000 is not of type 'array'
from schema $id: http://devicetree.org/schemas/property-units.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.kernel.org/project/devicetree/patch/20260624-qcom-spmi-haptics-v2-2-b9118e60f3e3@oss.qualcomm.com
The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.
If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:
pip3 install dtschema --upgrade
Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
^ permalink raw reply
* Re: [PATCH v2 3/3] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: sashiko-bot @ 2026-06-25 2:13 UTC (permalink / raw)
To: Fenglin Wu; +Cc: linux-input, dmitry.torokhov, conor+dt, devicetree, robh
In-Reply-To: <20260624-qcom-spmi-haptics-v2-3-b9118e60f3e3@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
- [High] Concurrent effect uploads or erasures free `fifo_data` while it is actively used by the playback worker or IRQ handler, leading to a Use-After-Free.
- [High] System suspend races with the asynchronous playback worker, leaving the hardware active during sleep and leaking a PM runtime reference.
- [High] Incorrect teardown ordering allows userspace to trigger Use-After-Free of driver structures after removal.
- [High] The driver fails to terminate FIFO playback when finished, permanently leaking a PM runtime reference.
- [Medium] Unbounded user-controlled kmalloc size allows memory exhaustion.
- [Medium] Missing signedness check on the custom play rate allows negative values to bypass boundary checks.
- [Medium] Lockless, non-atomic accesses to `play_request` and `cur_effect_id` cause new play requests to be lost.
- [Medium] The runtime PM usage counter is leaked during device removal if playback is active.
--
commit 06679c85ddc3489b6ff4ed3aec7d938287e570cd
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. 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.
> diff --git a/drivers/input/misc/qcom-spmi-haptics.c b/drivers/input/misc/qcom-spmi-haptics.c
> new file mode 100644
> index 0000000000000..4b27638df9605
> --- /dev/null
> +++ b/drivers/input/misc/qcom-spmi-haptics.c
[ ... ]
> +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;
> +
> + mutex_lock(&h->fifo_lock);
> +
> + if (!h->fifo_data) {
> + mutex_unlock(&h->fifo_lock);
> + return IRQ_HANDLED;
> + }
> +
> + if (h->data_done) {
> + ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
> + h->fifo_data = NULL;
> + h->play_request = false;
> + schedule_work(&h->play_work);
> + mutex_unlock(&h->fifo_lock);
> + return IRQ_HANDLED;
> + }
> +
> + /* Refill: write the next chunk, conservatively sized to the threshold */
> + to_write = min_t(u32, h->data_len - h->data_written,
> + h->fifo_len - FIFO_EMPTY_THRESH);
> + haptics_write_fifo_chunk(h, &h->fifo_data[h->data_written], to_write);
[Severity: High]
Does this code dereference a freed pointer?
A userspace process can start a FIFO effect, then concurrently call
EVIOCSFF (to re-upload) or EVIOCRMFF (to erase) the effect.
The input core calls haptics_erase() or haptics_upload_effect(), which
immediately execute kfree(priv->fifo_data).
Since the IRQ handler or worker continues to use h->fifo_data under
h->fifo_lock, it appears it will dereference the freed memory.
> + h->data_written += to_write;
> +
> + if (h->data_written >= h->data_len) {
> + /* Last chunk enqueued; disable threshold to stop further IRQs */
> + h->data_done = true;
> + ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
[Severity: High]
Does this fail to terminate FIFO playback when finished?
When the final chunk is written, the code sets FIFO_EMPTY_CFG_REG to 0,
disabling the empty interrupt.
Because the interrupt is disabled, the handler will never fire again to
process the h->data_done block at the start of the function. This leaves
the hardware active indefinitely and leaks the PM runtime usage counter.
> + }
> +
> + mutex_unlock(&h->fifo_lock);
> + return IRQ_HANDLED;
> +}
[ ... ]
> +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;
> + s16 *buf;
> + s8 *fifo;
> +
> + if (id < 0 || id >= HAPTICS_MAX_EFFECTS)
> + return -EINVAL;
> +
> + priv = &h->effects[id];
> +
> + switch (effect->type) {
> + case FF_CONSTANT:
> + kfree(priv->fifo_data);
> + priv->fifo_data = NULL;
> + priv->data_len = 0;
> + 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.
> + * No upper bound: the FIFO is refilled continuously from the IRQ
> + * handler, so any length of PCM data is supported.
> + */
> + if (effect->u.periodic.custom_len < 3)
> + return -EINVAL;
> +
> + buf = memdup_array_user(effect->u.periodic.custom_data,
> + effect->u.periodic.custom_len,
> + sizeof(s16));
[Severity: Medium]
Is there a missing upper bound check on custom_len?
A local user with write access to the input device can provide a very
large custom_len, and this directly allocates kernel memory without an
upper bound, which could lead to memory exhaustion.
> + if (IS_ERR(buf))
> + return PTR_ERR(buf);
> +
> + if (buf[CUSTOM_DATA_RATE_IDX] > PLAY_RATE_MAX) {
[Severity: Medium]
Can negative values bypass this boundary check?
buf is an array of signed 16-bit integers. A user passing a negative value
would bypass this check, get cast to u8 later, and be written to the
hardware registers as an out-of-bounds unsigned value.
> + kfree(buf);
> + return -EINVAL;
> + }
[ ... ]
> +static int haptics_playback(struct input_dev *dev, int effect_id, int val)
> +{
> + struct qcom_haptics *h = input_get_drvdata(dev);
> +
> + h->cur_effect_id = effect_id;
> + h->play_request = (val > 0);
[Severity: Medium]
Are play requests occasionally lost due to lockless accesses?
If the IRQ handler overwrites h->play_request to false immediately after a
new play event from userspace sets it to true here, the new request seems
to be lost. The worker will see play_request == false and stop the
hardware instead of starting the new effect.
> + schedule_work(&h->play_work);
> +
> + return 0;
> +}
[ ... ]
> +static void qcom_haptics_remove(struct platform_device *pdev)
> +{
> + struct qcom_haptics *h = platform_get_drvdata(pdev);
> + int i;
> +
> + pm_runtime_disable(&pdev->dev);
> + pm_runtime_set_suspended(&pdev->dev);
> +
> + mutex_lock(&h->play_lock);
> + haptics_stop_locked(h);
> + mutex_unlock(&h->play_lock);
[Severity: Medium]
Does this leak the runtime PM usage counter?
If remove is invoked while a haptics effect is actively playing,
h->pm_ref_held is true. This stops the hardware but does not call
pm_runtime_put_noidle(dev) to release the reference, leaving the PM
state unbalanced.
> +
> + haptics_enable_module(h, false);
> + cancel_work_sync(&h->play_work);
> + for (i = 0; i < HAPTICS_MAX_EFFECTS; i++) {
> + kfree(h->effects[i].fifo_data);
> + h->effects[i].fifo_data = NULL;
> + }
> +
> + input_unregister_device(h->input);
[Severity: High]
Is it safe to tear down resources before unregistering the input device?
Because the input device is still registered and active, an unprivileged
user can concurrently issue an ioctl (like EVIOCSFF or EVIOCRMFF) on the
open event node. This could access the freed effect structures or schedule
h->play_work after they have been destroyed.
> +}
[ ... ]
> +static int qcom_haptics_suspend(struct device *dev)
> +{
> + struct qcom_haptics *h = dev_get_drvdata(dev);
> +
> + mutex_lock(&h->play_lock);
> + haptics_stop_locked(h);
> + if (h->pm_ref_held) {
> + pm_runtime_put_noidle(dev);
> + h->pm_ref_held = false;
> + }
> + mutex_unlock(&h->play_lock);
[Severity: High]
Does this system suspend race with the asynchronous playback worker?
If userspace requests playback right before system suspend, h->play_request
is set to true and play_work is scheduled.
This suspend function stops the hardware but forgets to clear
h->play_request.
When this drops the lock, play_work can execute, see play_request is still
true, and re-enable the hardware just before the system suspends.
> +
> + cancel_work_sync(&h->play_work);
> + return pm_runtime_force_suspend(dev);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624-qcom-spmi-haptics-v2-0-b9118e60f3e3@oss.qualcomm.com?part=3
^ permalink raw reply
* [PATCH v2 3/3] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: Fenglin Wu @ 2026-06-25 2:00 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: <20260624-qcom-spmi-haptics-v2-0-b9118e60f3e3@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-6-sonnet
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 | 838 +++++++++++++++++++++++++++++++++
3 files changed, 850 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..4b27638df960
--- /dev/null
+++ b/drivers/input/misc/qcom-spmi-haptics.c
@@ -0,0 +1,838 @@
+// 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/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/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 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 HAPTICS_MAX_EFFECTS 8
+
+enum qcom_haptics_mode {
+ 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,
+ /* 4–7 are reserved */
+ 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 {
+ enum qcom_haptics_mode mode;
+ enum qcom_haptics_play_rate play_rate;
+ u32 vmax_mv;
+ s8 *fifo_data;
+ u32 data_len;
+};
+
+/**
+ * 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: deferred work item that starts or stops playback
+ * @play_lock: mutex lock to serialize playbacks
+ * @cur_effect_id: index into @effects[] identifying the active effect
+ * @fifo_empty_irq: IRQ number for the FIFO-empty interrupt
+ * @play_request: true when a playback is requested
+ * @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
+ * @data_done: flag to indicate that all samples have been written
+ * @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;
+ u16 gain;
+
+ struct work_struct play_work;
+ struct mutex play_lock; /* mutex used to serialize playbacks */
+ int cur_effect_id;
+ int fifo_empty_irq;
+ bool play_request;
+ 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;
+ bool data_done;
+
+ 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;
+}
+
+/*
+ * Configure the hardware FIFO memory boundary.
+ * FIFO occupies addresses [0, fifo_len).
+ */
+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)
+{
+ u32 v = mult_frac(vmax_mv, h->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;
+}
+
+/*
+ * Must be called with play_lock held.
+ * Clears PLAY_EN and resets any FIFO-specific state.
+ */
+static void haptics_stop_locked(struct qcom_haptics *h)
+{
+ int id = h->cur_effect_id;
+
+ cfg_write(h, HAP_CFG_SPMI_PLAY_REG, 0);
+
+ if (h->effects[id].mode == HAPTICS_FIFO) {
+ ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
+ haptics_fifo_irq_enable(h, false);
+ mutex_lock(&h->fifo_lock);
+ h->fifo_data = NULL;
+ mutex_unlock(&h->fifo_lock);
+ }
+}
+
+static int haptics_start_direct_play(struct qcom_haptics *h, int effect_id)
+{
+ struct ff_effect *ffe = &h->input->ff->effects[effect_id];
+ u8 amplitude = (u8)mult_frac((u32)abs(ffe->u.constant.level), 255, 0x7FFF);
+ 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, 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;
+ int ret;
+
+ 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;
+
+ /* 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;
+
+ mutex_lock(&h->fifo_lock);
+ h->fifo_data = eff->fifo_data;
+ h->data_len = eff->data_len;
+ h->data_written = init_len;
+ h->data_done = (init_len >= eff->data_len);
+ mutex_unlock(&h->fifo_lock);
+
+ /*
+ * 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.
+ */
+ ret = ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, h->data_done ? 0 :
+ FIFO_EMPTY_THRESH / FIFO_THRESH_LSB);
+ if (ret)
+ return ret;
+ if (!h->data_done)
+ haptics_fifo_irq_enable(h, true);
+
+ return cfg_write(h, HAP_CFG_SPMI_PLAY_REG,
+ PLAY_EN_BIT | FIELD_PREP(PAT_SRC_MASK, PAT_SRC_FIFO));
+}
+
+/*
+ * 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, which suppresses
+ * further interrupts; the hardware drains the remaining samples naturally
+ * and the play work handler stops the engine on the next invocation.
+ */
+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;
+
+ mutex_lock(&h->fifo_lock);
+
+ if (!h->fifo_data) {
+ mutex_unlock(&h->fifo_lock);
+ return IRQ_HANDLED;
+ }
+
+ if (h->data_done) {
+ ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
+ h->fifo_data = NULL;
+ h->play_request = false;
+ schedule_work(&h->play_work);
+ mutex_unlock(&h->fifo_lock);
+ return IRQ_HANDLED;
+ }
+
+ /* Refill: write the next chunk, conservatively sized to the threshold */
+ to_write = min_t(u32, h->data_len - h->data_written,
+ h->fifo_len - FIFO_EMPTY_THRESH);
+ haptics_write_fifo_chunk(h, &h->fifo_data[h->data_written], to_write);
+ h->data_written += to_write;
+
+ if (h->data_written >= h->data_len) {
+ /* Last chunk enqueued; disable threshold to stop further IRQs */
+ h->data_done = true;
+ ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
+ }
+
+ mutex_unlock(&h->fifo_lock);
+ return IRQ_HANDLED;
+}
+
+static void haptics_play_work(struct work_struct *work)
+{
+ struct qcom_haptics *h = container_of(work, struct qcom_haptics, play_work);
+ int id, ret;
+
+ mutex_lock(&h->play_lock);
+
+ if (!h->play_request) {
+ haptics_stop_locked(h);
+ if (h->pm_ref_held) {
+ pm_runtime_put_autosuspend(h->dev);
+ h->pm_ref_held = false;
+ }
+ goto unlock;
+ }
+
+ 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);
+ goto unlock;
+ }
+ h->pm_ref_held = true;
+ }
+
+ id = h->cur_effect_id;
+ switch (h->effects[id].mode) {
+ case HAPTICS_DIRECT_PLAY:
+ ret = haptics_start_direct_play(h, id);
+ break;
+ case HAPTICS_FIFO:
+ ret = haptics_start_fifo(h, id);
+ break;
+ default:
+ ret = -EINVAL;
+ }
+
+ if (ret) {
+ dev_err(h->dev, "failed to start effect %d: %d\n", id, ret);
+ if (h->pm_ref_held) {
+ pm_runtime_put_autosuspend(h->dev);
+ h->pm_ref_held = false;
+ }
+ }
+
+unlock:
+ mutex_unlock(&h->play_lock);
+}
+
+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;
+ s16 *buf;
+ s8 *fifo;
+
+ if (id < 0 || id >= HAPTICS_MAX_EFFECTS)
+ return -EINVAL;
+
+ priv = &h->effects[id];
+
+ switch (effect->type) {
+ case FF_CONSTANT:
+ kfree(priv->fifo_data);
+ priv->fifo_data = NULL;
+ priv->data_len = 0;
+ 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.
+ * No upper bound: the FIFO is refilled continuously from the IRQ
+ * handler, so any length of PCM data is supported.
+ */
+ if (effect->u.periodic.custom_len < 3)
+ return -EINVAL;
+
+ buf = memdup_array_user(effect->u.periodic.custom_data,
+ effect->u.periodic.custom_len,
+ sizeof(s16));
+ if (IS_ERR(buf))
+ return PTR_ERR(buf);
+
+ if (buf[CUSTOM_DATA_RATE_IDX] > PLAY_RATE_MAX) {
+ kfree(buf);
+ return -EINVAL;
+ }
+
+ data_len = effect->u.periodic.custom_len - CUSTOM_DATA_SAMPLE_START;
+ fifo = kmalloc(data_len, GFP_KERNEL);
+ if (!fifo) {
+ kfree(buf);
+ 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];
+
+ kfree(priv->fifo_data);
+ 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->mode = HAPTICS_FIFO;
+
+ kfree(buf);
+ 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);
+
+ h->cur_effect_id = effect_id;
+ h->play_request = (val > 0);
+ schedule_work(&h->play_work);
+
+ 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];
+
+ kfree(priv->fifo_data);
+ priv->fifo_data = NULL;
+ priv->data_len = 0;
+
+ return 0;
+}
+
+static void haptics_set_gain(struct input_dev *dev, u16 gain)
+{
+ struct qcom_haptics *h = input_get_drvdata(dev);
+
+ 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");
+
+ 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);
+ INIT_WORK(&h->play_work, haptics_play_work);
+ h->gain = 0xFFFF;
+
+ 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,
+ "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;
+ disable_irq_nosync(irq);
+
+ 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;
+
+ pm_runtime_get_noresume(h->dev);
+ pm_runtime_use_autosuspend(h->dev);
+ pm_runtime_set_autosuspend_delay(h->dev, HAPTICS_AUTOSUSPEND_MS);
+ devm_pm_runtime_set_active_enabled(h->dev);
+ pm_runtime_put_autosuspend(h->dev);
+
+ ret = input_register_device(input);
+ if (ret) {
+ input_ff_destroy(input);
+ return dev_err_probe(h->dev, ret, "failed to register input device\n");
+ }
+
+ platform_set_drvdata(pdev, h);
+
+ return 0;
+}
+
+static void qcom_haptics_remove(struct platform_device *pdev)
+{
+ struct qcom_haptics *h = platform_get_drvdata(pdev);
+ int i;
+
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_set_suspended(&pdev->dev);
+
+ mutex_lock(&h->play_lock);
+ haptics_stop_locked(h);
+ mutex_unlock(&h->play_lock);
+
+ haptics_enable_module(h, false);
+ cancel_work_sync(&h->play_work);
+ for (i = 0; i < HAPTICS_MAX_EFFECTS; i++) {
+ kfree(h->effects[i].fifo_data);
+ h->effects[i].fifo_data = NULL;
+ }
+
+ input_unregister_device(h->input);
+}
+
+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);
+
+ mutex_lock(&h->play_lock);
+ haptics_stop_locked(h);
+ if (h->pm_ref_held) {
+ pm_runtime_put_noidle(dev);
+ h->pm_ref_held = false;
+ }
+ mutex_unlock(&h->play_lock);
+
+ cancel_work_sync(&h->play_work);
+ return pm_runtime_force_suspend(dev);
+}
+
+static int qcom_haptics_resume(struct device *dev)
+{
+ 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 v2 1/3] dt-bindings: input: Add Qualcomm SPMI PMIC haptics
From: Fenglin Wu @ 2026-06-25 2:00 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: <20260624-qcom-spmi-haptics-v2-0-b9118e60f3e3@oss.qualcomm.com>
Add binding document for the haptics module inside Qualcomm PMIC
PMIH0108.
Assisted-by: Claude:claude-4-6-sonnet
Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
.../bindings/input/qcom,spmi-haptics.yaml | 132 +++++++++++++++++++++
1 file changed, 132 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..3764c3e113a3
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
@@ -0,0 +1,132 @@
+# 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 lra-period-us and whose peak voltage is defined by 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 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 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
+ vmax-mv 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:
+ 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, rounded to the
+ nearest 50,000 uV step. 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.
+ $ref: /schemas/types.yaml#/definitions/uint32
+ 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
+
+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,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
* [PATCH v2 0/3] input: misc: Add an initial driver for haptics inside Qcom PMIH010x PMIC
From: Fenglin Wu @ 2026-06-25 2:00 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
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 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 | 132 ++++
.../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 | 838 +++++++++++++++++++++
5 files changed, 986 insertions(+)
---
base-commit: 66725039f7090afe14c31bd259e2059a68f04023
change-id: 20260616-qcom-spmi-haptics-3cc97e7b232e
Best regards,
--
Fenglin Wu <fenglin.wu@oss.qualcomm.com>
^ permalink raw reply
* [PATCH v2 2/3] dt-bindings: mfd: qcom,spmi-pmic: Document haptics device
From: Fenglin Wu @ 2026-06-25 2:00 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: <20260624-qcom-spmi-haptics-v2-0-b9118e60f3e3@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
* Re: [PATCH v1] iio: temperature: hid-sensor-temperature: switch to non-devm iio_device_register()
From: Sanjay Chitroda @ 2026-06-25 1:52 UTC (permalink / raw)
To: Jonathan Cameron, srinivas pandruvada
Cc: Maxwell Doose, jikos, dlechner, nuno.sa, andy, hongyan.song,
linux-input, linux-iio, linux-kernel
In-Reply-To: <20260623200616.56a44b3e@jic23-huawei>
On 24 June 2026 12:36:16 am IST, Jonathan Cameron <jic23@kernel.org> wrote:
>On Mon, 22 Jun 2026 13:50:22 -0700
>srinivas pandruvada <srinivas.pandruvada@linux.intel.com> wrote:
>
>> On Mon, 2026-06-22 at 10:27 -0500, Maxwell Doose wrote:
>> > On Mon, Jun 22, 2026 at 10:26 AM srinivas pandruvada
>> > <srinivas.pandruvada@linux.intel.com> wrote:
>> > >
>> > > On Mon, 2026-06-22 at 10:51 +0530, Sanjay Chitroda wrote:
>> > > > From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
>> > > >
>> > > > Avoid using devm_iio_device_register(), as this driver requires
>> > > > explicit
>> > > > error handling and teardown ordering.
>> > > >
>> > > > Mixing devm_* APIs with goto-based error unwinding breaks the
>> > > > expected
>> > > > LIFO resource release model and can introduce race windows during
>> > > > device
>> > > > removal. In particular, the IIO device may remain visible to
>> > > > userspace
>> > > > while dependent resources are already being freed, potentially
>> > > > leading
>> > > > to use-after-free issues.
>> > >
>> > > Please explain this use after free case here.
>> > >
>> > > Thanks,
>> > > Srinivas
>> >
>> > My guess is that because the device would still be registered but
>> > would actually be removed, sysfs still has "wild" pointers to
>> > read_raw() and write_raw() (which don't exist anymore), causing the
>> > UAF. If I'm wrong feel free to correct me though.
>>
>> iio_device_unregister() will be last one to be called after device
>> removal from devm action handler. This will cleanup attributes. So,
>> read_raw() or write_raw() can be called. The problem can be handlers
>> for read_raw() and write_raw() if anything there which are dependent on
>> clean done by hid_temperature_remove(). Here callbacks are cleaned up,
>> so nothing to respond to read sensor_hub_input_attr_get_raw_value(),
>> so it has to wait for 5 seconds to timeout, which is not great. So
>> nothing against change done here.
>>
>> But still not sure any use after free case, unless I am missing
>> something.
>>
>Agreed that to call UAF you need an explained path (and preferably
>testing that it happens). The timeout issue Srinivas calls out is
>sufficient for us to merge this as a fix, but the patch description
>should then talk about that.
>
Thank you all for the review.
I investigated the remove path in more detail. I agree that my original use-after-free explanation was not sufficiently justified.
The issue is actually the teardown ordering. With "devm_iio_device_register()", the IIO device remains registered until the devres cleanup phase, while "remove()" first removes the sensor hub callback. During this window, the IIO device is still visible to userspace and "read_raw()" requests may be issued. These requests eventually wait for a response from the sensor hub callback, but the callback has already been removed, resulting in the operation timing out.
I'll update the commit message in v2 to describe this observable behaviour instead of mentioning a potential use-after-free. The code change itself remains the same.
Thanks, Sanjay
>Thanks,
>
>Jonathan
>> Thanks,
>> Srinivas
>>
>>
>
^ permalink raw reply
* Re: [PATCH 0/4] input: misc: Add an initial driver for haptics inside Qcom PMIH010x PMIC
From: Fenglin Wu @ 2026-06-25 1:41 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-arm-msm, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lee Jones, Stephen Boyd, Bjorn Andersson,
Konrad Dybcio, David Collins, Subbaraman Narayanamurthy,
Kamal Wadhwa, kernel, linux-input, devicetree, linux-kernel
In-Reply-To: <4ba2eeed-71f8-4799-b261-e4e2c268043e@kernel.org>
On 6/24/2026 6:05 PM, Krzysztof Kozlowski wrote:
> No. Act as maintainer. Clone Linus tree, apply the patch and see if
> everything works. My claim is that nothing works and maintainer tree is
> broken.
>
> Best regards,
> Krzysztof
Thanks for the explanation. I just did that and I didn't see conflict
when applying the binding and driver changes, but I did see a conflict
when applying the DTS change. I will drop the DTS change 1st and resend
them after the driver and binding changes get accepted.
^ permalink raw reply
* Re: [PATCH v4 03/11] HID: spi-hid: add transport driver skeleton for HID over SPI bus
From: Jingyuan Liang @ 2026-06-24 22:41 UTC (permalink / raw)
To: sashiko-reviews; +Cc: robh, conor+dt, devicetree, dmitry.torokhov, linux-input
In-Reply-To: <20260609045604.44FE71F00893@smtp.kernel.org>
On Mon, Jun 8, 2026 at 9:56 PM <sashiko-bot@kernel.org> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Embedded arrays `read_approval_header` and `read_approval_body` are used for SPI DMA transfers without cacheline alignment, risking memory corruption.
> - [Medium] Unchecked return values of reset operations in `spi_hid_dev_init` and `spi_hid_resume`
> --
>
> commit ea35a3a23739acaca367a8d250ef25c96d22df36
> Author: Angela Czubak <acz@semihalf.com>
>
> HID: spi-hid: add transport driver skeleton for HID over SPI bus
>
> Create spi-hid folder and add Kconfig and Makefile for spi-hid driver.
> Add basic device structure, definitions, and probe/remove functions.
>
> > diff --git a/drivers/hid/spi-hid/spi-hid-core.c b/drivers/hid/spi-hid/spi-hid-core.c
> > new file mode 100644
> > index 0000000000000..02a7608c4b882
> > --- /dev/null
> > +++ b/drivers/hid/spi-hid/spi-hid-core.c
> > @@ -0,0 +1,213 @@
>
> [ ... ]
>
> > +/* Driver context */
> > +struct spi_hid {
> > + struct spi_device *spi; /* spi device. */
> > + struct hid_device *hid; /* pointer to corresponding HID dev. */
> > +
> > + struct spihid_ops *ops;
> > + struct spi_hid_conf *conf;
> > +
> > + enum hidspi_power_state power_state;
> > +
> > + u32 regulator_error_count;
> > + int regulator_last_error;
> > + u32 bus_error_count;
> > + int bus_last_error;
> > + u32 dir_count; /* device initiated reset count. */
> > +};
>
> [Severity: High]
> Does embedding the read_approval_header and read_approval_body arrays directly
> in this struct for SPI DMA transfers risk memory corruption?
>
> When these buffers are passed as tx_buf to spi_sync() (e.g. in
> spi_hid_input_sync()), they lack the ____cacheline_aligned directive. On
> architectures with non-coherent DMA, cache maintenance operations could
> encompass adjacent struct members.
>
> Concurrent CPU writes to those adjacent members during the DMA mapping window
> will be destroyed, leading to memory corruption.
>
> Should these buffers be cacheline aligned, or dynamically allocated?
read_approval_header and read_approval_body arrays are not in this patch.
>
> [ ... ]
>
> > +int spi_hid_core_probe(struct spi_device *spi, struct spihid_ops *ops,
> > + struct spi_hid_conf *conf)
> > +{
>
> [ ... ]
>
> > + /*
> > + * At the end of probe we initialize the device:
> > + * 0) assert reset, bias the interrupt line
> > + * 1) sleep minimal reset delay
> > + * 2) request IRQ
> > + * 3) power up the device
> > + * 4) deassert reset (high)
> > + * After this we expect an IRQ with a reset response.
> > + */
> > +
> > + shid->ops->assert_reset(shid->ops);
> > +
> > + shid->ops->sleep_minimal_reset_delay(shid->ops);
>
> [ ... ]
>
> > + error = shid->ops->power_up(shid->ops);
> > + if (error) {
> > + dev_err(dev, "%s: could not power up\n", __func__);
> > + return error;
> > + }
> > +
> > + shid->ops->deassert_reset(shid->ops);
>
> [Severity: Medium]
> Can hardware reset failures be masked here?
>
> The assert_reset and deassert_reset callbacks return integer error codes.
> However, their return values are ignored in spi_hid_core_probe() (as well as
> resume paths).
>
> If deassert_reset() fails, the device may remain in reset, causing the driver
> to hang waiting for a reset response IRQ that will never arrive.
>
> Should these return values be checked and handled?
>
> > +
> > + dev_dbg(dev, "%s: d3 -> %s\n", __func__,
> > + spi_hid_power_mode_string(shid->power_state));
> > +
> > + return 0;
> > +}
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260609-send-upstream-v4-0-b843d5e6ced3@chromium.org?part=3
^ permalink raw reply
* Re: [git pull] Input updates for v7.2-rc0
From: pr-tracker-bot @ 2026-06-24 22:16 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: Linus Torvalds, linux-kernel, linux-input
In-Reply-To: <ajs_Hmo3Zo4aEFZ7@google.com>
The pull request you sent on Tue, 23 Jun 2026 19:21:25 -0700:
> git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git tags/input-for-v7.2-rc0
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/e1611017870fa1582b5ff9ec0edc09542318daa6
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply
* Re: [PATCH v2] HID: multitouch: Fix stale MT slots when contact count drops to zero
From: Dave Carey @ 2026-06-24 19:19 UTC (permalink / raw)
To: linux-input; +Cc: jikos, benjamin.tissoires, Dave Carey
In-Reply-To: <20260522111527.69428-1-carvsdriver@gmail.com>
Gentle ping on this one — it's been about five weeks since the v2
submission with no feedback.
For context: the INPUT_PROP_BUTTONPAD misclassification fix for the same
device (hid-multitouch) was merged to mainline 2026-05-28, and the ghost
contacts fix (MT_QUIRK_CONTACT_CNT_ACCURATE) was applied to hid/for-next
2026-06-10 by Jiri. This stale-slots fix is the remaining hid-multitouch
patch for the Yoga Book 9 14IAH10.
Happy to rebase, respin, or address any concerns.
Thanks,
Dave
^ permalink raw reply
* [PATCH v6] HID: i2c-hid: Refactor _DSM helper and add i2c-hid-acpi-prp0001 driver
From: 谢致邦 (XIE Zhibang) @ 2026-06-24 17:46 UTC (permalink / raw)
To: linux-input, hansg, dmitry.torokhov, bentiss, yeking
Cc: dianders, jikos, linux-kernel, sashiko-bot, sashiko-reviews,
superm1, 谢致邦 (XIE Zhibang)
In-Reply-To: <tencent_8AE8ED913FEC8151B4BA3D85D6BB7F3ACF08@qq.com>
Move the _DSM call that gets the HID descriptor address from
i2c-hid-acpi.c into i2c-hid-acpi.h as a static inline so both the ACPI
and the new PRP0001 driver can use it. While refactoring, move the
blacklist check and the _DSM call to the top of probe() to avoid a
pointless alloc when the device is blacklisted or does not implement the
_DSM.
Some devices, for example the Lenovo KaiTian N60d and Inspur CP300L3,
are declared with _HID "PRP0001" and _DSD compatible "hid-over-i2c" but
lack "hid-descr-addr" from the _DSD and provide the HID descriptor
address only through an ACPI _DSM. The OF driver fails to probe them
because it requires hid-descr-addr. Add a new driver that handles these
devices by calling the shared _DSM helper.
Fixes: b33752c30023 ("HID: i2c-hid: Reorganize so ACPI and OF are separate modules")
Signed-off-by: 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
---
v2: Name the unused parameter and document why
acpi_device_fix_up_power() is skipped.
v3: Add a dev_warn() asking users to contact vendors for firmware
updates, and use existing locals in devm_kzalloc() and
acpi_device_fix_up_power().
v4: Double the power-up delay from 250ms to 500ms.
v5: Document why of_match_ptr() on the of_match_table is safe when
CONFIG_OF=n.
v6: Increase power-up delay from 500ms to 750ms. During cold boot on low
battery, 500ms causes non-fatal I2C transfer errors (-ENXIO). 750ms
fixes them.
drivers/hid/i2c-hid/Kconfig | 18 ++++
drivers/hid/i2c-hid/Makefile | 1 +
drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c | 104 +++++++++++++++++++++
drivers/hid/i2c-hid/i2c-hid-acpi.c | 52 +++--------
drivers/hid/i2c-hid/i2c-hid-acpi.h | 32 +++++++
5 files changed, 169 insertions(+), 38 deletions(-)
create mode 100644 drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c
create mode 100644 drivers/hid/i2c-hid/i2c-hid-acpi.h
diff --git a/drivers/hid/i2c-hid/Kconfig b/drivers/hid/i2c-hid/Kconfig
index e8d51f410cc1..7db8b2abff78 100644
--- a/drivers/hid/i2c-hid/Kconfig
+++ b/drivers/hid/i2c-hid/Kconfig
@@ -22,6 +22,24 @@ config I2C_HID_ACPI
will be called i2c-hid-acpi. It will also build/depend on the
module i2c-hid.
+config I2C_HID_ACPI_PRP0001
+ tristate "Driver for PRP0001 devices missing hid-descr-addr"
+ depends on ACPI
+ depends on DRM || !DRM
+ select I2C_HID_CORE
+ help
+ Say Y here if you want support for I2C HID touchpads that
+ are declared with _HID "PRP0001" and _DSD compatible
+ "hid-over-i2c" but lack the "hid-descr-addr" property from
+ the _DSD. The HID descriptor address is instead provided
+ through an ACPI _DSM. Known affected devices include the
+ Lenovo KaiTian N60d and Inspur CP300L3.
+
+ If unsure, say N.
+
+ This support is also available as a module. If so, the
+ module will be called i2c-hid-acpi-prp0001.
+
config I2C_HID_OF
tristate "HID over I2C transport layer Open Firmware driver"
# No "depends on OF" because this can also be used for manually
diff --git a/drivers/hid/i2c-hid/Makefile b/drivers/hid/i2c-hid/Makefile
index 55bd5e0f35af..da420c1a8ec6 100644
--- a/drivers/hid/i2c-hid/Makefile
+++ b/drivers/hid/i2c-hid/Makefile
@@ -9,6 +9,7 @@ i2c-hid-objs = i2c-hid-core.o
i2c-hid-$(CONFIG_DMI) += i2c-hid-dmi-quirks.o
obj-$(CONFIG_I2C_HID_ACPI) += i2c-hid-acpi.o
+obj-$(CONFIG_I2C_HID_ACPI_PRP0001) += i2c-hid-acpi-prp0001.o
obj-$(CONFIG_I2C_HID_OF) += i2c-hid-of.o
obj-$(CONFIG_I2C_HID_OF_ELAN) += i2c-hid-of-elan.o
obj-$(CONFIG_I2C_HID_OF_GOODIX) += i2c-hid-of-goodix.o
diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c b/drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c
new file mode 100644
index 000000000000..d2cf4714ae7f
--- /dev/null
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi-prp0001.c
@@ -0,0 +1,104 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * HID over I2C driver for PRP0001 devices missing hid-descr-addr
+ *
+ * Some devices, for example the Lenovo KaiTian N60d and Inspur CP300L3, use
+ * _HID "PRP0001" with _DSD compatible "hid-over-i2c" but lack "hid-descr-addr"
+ * from the _DSD. The HID descriptor address is provided only through an ACPI
+ * _DSM. The TPD0 node in the DSDT shows _DSM Function 1 returning 0x20.
+ *
+ * Copyright (C) 2026 谢致邦 (XIE Zhibang) <Yeking@Red54.com>
+ */
+
+#include <linux/delay.h>
+#include <linux/device.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+
+#include "i2c-hid.h"
+#include "i2c-hid-acpi.h"
+
+static int i2c_hid_acpi_prp0001_power_up(struct i2chid_ops *ops)
+{
+ /* give the device time to power up */
+ msleep(750);
+ return 0;
+}
+
+static struct i2chid_ops i2c_hid_acpi_prp0001_ops = {
+ .power_up = i2c_hid_acpi_prp0001_power_up,
+ /*
+ * No .restore_sequence needed: the _DSM on these devices returns a
+ * constant (0x20) with no side effects, unlike some PNP0C50 _DSM
+ * implementations that switch the hardware between PS/2 and I2C modes.
+ */
+};
+
+static int i2c_hid_acpi_prp0001_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct acpi_device *adev;
+ u16 hid_descriptor_address;
+ int ret;
+
+ /* If hid-descr-addr is present, let i2c-hid-of handle it */
+ if (device_property_present(dev, "hid-descr-addr"))
+ return -ENODEV;
+
+ adev = ACPI_COMPANION(dev);
+ if (!adev)
+ return -ENODEV;
+
+ ret = i2c_hid_acpi_get_descriptor(adev);
+ if (ret < 0)
+ return ret;
+ dev_warn(dev,
+ "hid-descr-addr device property NOT found, using ACPI _DSM fallback. Contact vendor for firmware update!\n");
+ hid_descriptor_address = ret;
+
+ /*
+ * No acpi_device_fix_up_power() needed: TPD0 has no _PS0, _PS3, _PSC
+ * or _PRx methods and follows I2C bus power.
+ */
+ return i2c_hid_core_probe(client, &i2c_hid_acpi_prp0001_ops,
+ hid_descriptor_address, 0);
+}
+
+static const struct of_device_id i2c_hid_acpi_prp0001_of_match[] = {
+ { .compatible = "hid-over-i2c" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, i2c_hid_acpi_prp0001_of_match);
+
+static const struct i2c_device_id i2c_hid_acpi_prp0001_id[] = {
+ { .name = "hid-over-i2c" },
+ { }
+};
+MODULE_DEVICE_TABLE(i2c, i2c_hid_acpi_prp0001_id);
+
+static struct i2c_driver i2c_hid_acpi_prp0001_driver = {
+ .driver = {
+ .name = "i2c_hid_acpi_prp0001",
+ .pm = &i2c_hid_core_pm,
+ .probe_type = PROBE_PREFER_ASYNCHRONOUS,
+ /*
+ * of_match_ptr() makes this NULL when CONFIG_OF=n, but that's
+ * fine: the I2C id_table with "hid-over-i2c" handles matching
+ * via client->name (set by acpi_set_modalias() from the _DSD
+ * compatible property).
+ */
+ .of_match_table = of_match_ptr(i2c_hid_acpi_prp0001_of_match),
+ },
+
+ .probe = i2c_hid_acpi_prp0001_probe,
+ .remove = i2c_hid_core_remove,
+ .shutdown = i2c_hid_core_shutdown,
+ .id_table = i2c_hid_acpi_prp0001_id,
+};
+
+module_i2c_driver(i2c_hid_acpi_prp0001_driver);
+
+MODULE_DESCRIPTION("HID over I2C driver for PRP0001 devices missing hid-descr-addr");
+MODULE_AUTHOR("谢致邦 (XIE Zhibang) <Yeking@Red54.com>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.c b/drivers/hid/i2c-hid/i2c-hid-acpi.c
index abd700a101f4..13f977d6aab6 100644
--- a/drivers/hid/i2c-hid/i2c-hid-acpi.c
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.c
@@ -25,9 +25,9 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/pm.h>
-#include <linux/uuid.h>
#include "i2c-hid.h"
+#include "i2c-hid-acpi.h"
struct i2c_hid_acpi {
struct i2chid_ops ops;
@@ -48,39 +48,11 @@ static const struct acpi_device_id i2c_hid_acpi_blacklist[] = {
{ }
};
-/* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
-static guid_t i2c_hid_guid =
- GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
- 0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
-
-static int i2c_hid_acpi_get_descriptor(struct i2c_hid_acpi *ihid_acpi)
-{
- struct acpi_device *adev = ihid_acpi->adev;
- acpi_handle handle = acpi_device_handle(adev);
- union acpi_object *obj;
- u16 hid_descriptor_address;
-
- if (acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)
- return -ENODEV;
-
- obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid, 1, 1, NULL,
- ACPI_TYPE_INTEGER);
- if (!obj) {
- acpi_handle_err(handle, "Error _DSM call to get HID descriptor address failed\n");
- return -ENODEV;
- }
-
- hid_descriptor_address = obj->integer.value;
- ACPI_FREE(obj);
-
- return hid_descriptor_address;
-}
-
static void i2c_hid_acpi_restore_sequence(struct i2chid_ops *ops)
{
struct i2c_hid_acpi *ihid_acpi = container_of(ops, struct i2c_hid_acpi, ops);
- i2c_hid_acpi_get_descriptor(ihid_acpi);
+ i2c_hid_acpi_get_descriptor(ihid_acpi->adev);
}
static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
@@ -93,24 +65,28 @@ static void i2c_hid_acpi_shutdown_tail(struct i2chid_ops *ops)
static int i2c_hid_acpi_probe(struct i2c_client *client)
{
struct device *dev = &client->dev;
+ struct acpi_device *adev = ACPI_COMPANION(dev);
struct i2c_hid_acpi *ihid_acpi;
u16 hid_descriptor_address;
int ret;
- ihid_acpi = devm_kzalloc(&client->dev, sizeof(*ihid_acpi), GFP_KERNEL);
+ if (acpi_match_device_ids(adev, i2c_hid_acpi_blacklist) == 0)
+ return -ENODEV;
+
+ ret = i2c_hid_acpi_get_descriptor(adev);
+ if (ret < 0)
+ return ret;
+ hid_descriptor_address = ret;
+
+ ihid_acpi = devm_kzalloc(dev, sizeof(*ihid_acpi), GFP_KERNEL);
if (!ihid_acpi)
return -ENOMEM;
- ihid_acpi->adev = ACPI_COMPANION(dev);
+ ihid_acpi->adev = adev;
ihid_acpi->ops.shutdown_tail = i2c_hid_acpi_shutdown_tail;
ihid_acpi->ops.restore_sequence = i2c_hid_acpi_restore_sequence;
- ret = i2c_hid_acpi_get_descriptor(ihid_acpi);
- if (ret < 0)
- return ret;
- hid_descriptor_address = ret;
-
- acpi_device_fix_up_power(ihid_acpi->adev);
+ acpi_device_fix_up_power(adev);
return i2c_hid_core_probe(client, &ihid_acpi->ops,
hid_descriptor_address, 0);
diff --git a/drivers/hid/i2c-hid/i2c-hid-acpi.h b/drivers/hid/i2c-hid/i2c-hid-acpi.h
new file mode 100644
index 000000000000..8cebbeebcc23
--- /dev/null
+++ b/drivers/hid/i2c-hid/i2c-hid-acpi.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _I2C_HID_ACPI_H
+#define _I2C_HID_ACPI_H
+
+#include <linux/acpi.h>
+#include <linux/uuid.h>
+
+static inline int i2c_hid_acpi_get_descriptor(struct acpi_device *adev)
+{
+ /* HID I²C Device: 3cdff6f7-4267-4555-ad05-b30a3d8938de */
+ static const guid_t i2c_hid_guid =
+ GUID_INIT(0x3CDFF6F7, 0x4267, 0x4555,
+ 0xAD, 0x05, 0xB3, 0x0A, 0x3D, 0x89, 0x38, 0xDE);
+
+ acpi_handle handle = acpi_device_handle(adev);
+ union acpi_object *obj;
+ u16 addr;
+
+ obj = acpi_evaluate_dsm_typed(handle, &i2c_hid_guid,
+ 1, 1, NULL, ACPI_TYPE_INTEGER);
+ if (!obj) {
+ acpi_handle_err(handle, "Error _DSM call to get HID descriptor address failed\n");
+ return -ENODEV;
+ }
+
+ addr = obj->integer.value;
+ ACPI_FREE(obj);
+ return addr;
+}
+
+#endif
--
2.52.0
^ permalink raw reply related
* Re: [PATCH] HID: amd_sfh: return an error when response wait times out
From: sashiko-bot @ 2026-06-24 14:52 UTC (permalink / raw)
To: Pengpeng Hou; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260624143558.39306-1-pengpeng@iscas.ac.cn>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The timeout error codes returned by the patch are silently discarded, rendering the fix ineffective.
Pre-existing issues:
- [High] Data race on the shared cli_data->cur_hid_dev variable causes spurious timeouts and missed wakeups.
- [High] A timeout in amdtp_wait_for_response leaves a stalled request in the queue, permanently desynchronizing the state machine.
--
commit e8a7b9fc56953c32e53def387e9ca9eac4750f78
Author: Pengpeng Hou <pengpeng@iscas.ac.cn>
HID: amd_sfh: return an error when response wait times out
amdtp_wait_for_response() waits for request_done before completing a
report request. wait_event_interruptible_timeout() returns 0 when the
wait expires, but the current code treats only negative values as errors
and returns success on timeout. This commit returns -ETIMEDOUT when the
response wait expires.
> diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_hid.c b/drivers/hid/amd-sfh-hid/amd_sfh_hid.c
> index b04f675d49b0d..8f88f965fbd55 100644
> --- a/drivers/hid/amd-sfh-hid/amd_sfh_hid.c
> +++ b/drivers/hid/amd-sfh-hid/amd_sfh_hid.c
> @@ -87,16 +87,17 @@ static int amdtp_wait_for_response(struct hid_device *hid)
> break;
> }
>
> - if (!cli_data->request_done[i])
> + if (!cli_data->request_done[i]) {
> ret = wait_event_interruptible_timeout(hid_data->hid_wait,
> cli_data->request_done[i],
> msecs_to_jiffies(AMD_SFH_RESPONSE_TIMEOUT));
> - if (ret == -ERESTARTSYS)
> - return -ERESTARTSYS;
> - else if (ret < 0)
> - return -ETIMEDOUT;
> - else
> - return 0;
> + if (ret == -ERESTARTSYS)
> + return -ERESTARTSYS;
> + if (ret <= 0)
> + return -ETIMEDOUT;
[Severity: High]
The patch returns -ETIMEDOUT here, but amdtp_wait_for_response() is registered
as the .wait callback in struct hid_ll_driver.
When called via the wrapper hid_hw_wait(), the return value is discarded:
include/linux/hid.h:hid_hw_wait()
static inline void hid_hw_wait(struct hid_device *hdev)
{
if (hdev->ll_driver->wait)
hdev->ll_driver->wait(hdev);
}
Does this mean the caller (HID core) never sees the error code and proceeds as
if the wait succeeded?
[Severity: High]
This isn't a bug introduced by this patch, but if this times out, does it
leave a stalled request in the queue?
In amd_sfh_get_report(), requests are queued to cli_data->req_list. If we
exit here, the request node isn't removed. Later, amd_sfh_work() pops and
processes it:
drivers/hid/amd-sfh-hid/amd_sfh_client.c:amd_sfh_work()
req_node = list_last_entry(&req_list->list, struct request_list, list);
list_del(&req_node->list);
Will this cause the background worker to process timed-out requests and
prematurely wake up future requests with stale data, permanently
desynchronizing the request queue?
> + }
> +
> + return 0;
> }
>
> void amdtp_hid_wakeup(struct hid_device *hid)
> {
> struct amdtp_hid_data *hid_data;
> struct amdtp_cl_data *cli_data;
>
> if (hid) {
> hid_data = hid->driver_data;
> cli_data = hid_data->cli_data;
> cli_data->request_done[cli_data->cur_hid_dev] = true;
[Severity: High]
This is a pre-existing issue, but is there a data race on
cli_data->cur_hid_dev?
amd_sfh_set_report() updates cur_hid_dev locklessly:
drivers/hid/amd-sfh-hid/amd_sfh_client.c:amd_sfh_set_report()
cli_data->cur_hid_dev = i;
If a SET_REPORT request is issued concurrently with a GET_REPORT worker, could
cur_hid_dev be overwritten, causing amdtp_hid_wakeup() to set request_done for
the wrong device index and leaving the original device's waiter to permanently
time out?
> wake_up_interruptible(&hid_data->hid_wait);
> }
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624143558.39306-1-pengpeng@iscas.ac.cn?part=1
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-oneplus: Update compatible to include model
From: David Heidelberg @ 2026-06-24 14:37 UTC (permalink / raw)
To: Dmitry Torokhov, Krzysztof Kozlowski, Konrad Dybcio
Cc: Rob Herring, Conor Dooley, Jason A. Donenfeld, Matthias Schiffer,
Vincent Huang, Bjorn Andersson, Konrad Dybcio, linux-input,
devicetree, linux-kernel, linux-arm-msm, phone-devel,
Krzysztof Kozlowski
In-Reply-To: <ajtaUb4YmyZTDLmQ@google.com>
On 24/06/2026 06:28, Dmitry Torokhov wrote:
> Hi David,
>
> On Sun, Jun 21, 2026 at 07:11:45PM +0200, David Heidelberg wrote:
>> On 28/05/2026 00:13, David Heidelberg wrote:
>>> On 27/05/2026 23:56, Dmitry Torokhov wrote:
>>>> Hi David,
>>>>
>>>> On Sat, May 23, 2026 at 11:45:35AM +0200, David Heidelberg via B4 Relay wrote:
>>>>> From: David Heidelberg <david@ixit.cz>
>>>>>
>>>>> We know the driver is reporting s3706b, introduce the compatible so we
>>>>> can more easily introduce quirks for weird touchscreen replacements in
>>>>> followup series.
>>>>>
>>>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>>>> Signed-off-by: David Heidelberg <david@ixit.cz>
>>>>> ---
>>>>> arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>> b/arch/ arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>> index 6b7378cf4d493..148164d456a5a 100644
>>>>> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>>>> @@ -475,17 +475,17 @@ bq27441_fg: bq27441-battery@55 {
>>>>> };
>>>>> };
>>>>> &i2c12 {
>>>>> status = "okay";
>>>>> clock-frequency = <400000>;
>>>>> synaptics-rmi4-i2c@20 {
>>>>> - compatible = "syna,rmi4-i2c";
>>>>> + compatible = "syna,rmi4-s3706b", "syna,rmi4-i2c";
>>>>
>>>> So I believe we established that this device (s3706b) does not in fact
>>>> implement rmi4 protocol properly. Why do we have "syna,rmi4-i2c" as a
>>>> fallback? Shouldn't it be just "syna,rmi4-s3706b"?
>>>
>>> The vendor supplies s3706b which does implement the RMI4 properly.
>>>
>>> The 3rd party replacement impersonating original parts may not implement
>>> it properly, but I don't address this issue in this initial submission.
>>>
>>> With this compatible we know which original part is used by the vendor
>>> and installed in the phones, so later we can deduct specific sequences
>>> for the replacement aftermarket parts to keep phone touchscreen working
>>> same as they do on Android without affecting other devices.
>>
>> Hello Dmitry.
>>
>> May I ask what is currently preventing this series from moving forward?
>>
>> The first version was posted in 2023 [1]. I picked it up again in 2025 [2]
>> and am now on the 9th iteration (this patchset). At this point, the series
>> has been under discussion for well over a year, with relatively little
>> feedback and increasingly long gaps between review rounds.
>>
>> The current approach is based on the guidance I have received so far,
>> including suggestions from the device-tree maintainers. When concerns were
>> raised, I tried to address them and rework the series accordingly.
>>
>> What I am struggling with is understanding what specific issue still needs
>> to be resolved before these patches can be accepted. If there are remaining
>> requirements, objections to the approach, or technical concerns that I have
>> not addressed, I would appreciate having them stated explicitly so I can
>> work on them.
>>
>> I also split out the straightforward, self-contained changes in the hope
>> that at least those could progress independently while I continued working
>> on any follow-up requirements. However, even those patches do not appear to
>> be moving forward.
>>
>> Could you please clarify what outcome you would like to see from this
>> series, and what concrete changes would be required to get it accepted?
>
> I am still confused about how you want to differentiate between the full
> RMI4 support vs the OnePlus flavor. The "syna,rmi4-s3706b", as you
> mentioned, implements RMI4 protocol properly, so we do not need to
> actually have it documented neither in binding nor in DTS.
--- part 1 ---
This series addresses identification within device-tree. It's normal recommended
practice.
If we know, the device ships specific, but **compliant** variant, we just put it
as compatible = "more-specific", "less-specific"; in this case
"syna,rmi4-s3706b", "syna,rmi4-i2c"
This approach is used everywhere. This has nothing to do with after-market parts.
--- part 2 (irrelevant for this series) ---
>
> The issue you have with after-market parts that are not compliant and we
> need to figure out how to deal with them. Inside the driver I
As was suggested by device-tree folks, this is the first step, there isn't
better one available. If there is, please suggest one, and I'll apply it.
> essentially need a"incomplete protocol" flag that we can use to
> implement additional checks or skip known to be not implemented
> functions/queries. In DT we could introduce something like
> "oneplus,rmi4-i2c" that is decidedly not compatible with "syna,rmi4-i2c"
> and neither one should be a fallback for the other.
>
> This of course needs buy-in from DT maintainers.
As you can see, this still holds Acked-by and Reviewed-by from the relevant
people - Krzysztof and Konrad.
>
> Does this make sense?
For the scope we're discussing it doesn't seems so.
This discussion should be associated with the last revision of the full series I
sent 3 months ago. We're in very unflattering state, where:
2018 - these aftermarket touchscreen worked on Android well enough for people
to have working touch (let's say with slightly worse experience then the original).
2026 in the mainline, we cannot even more forward and report to user-space
there is aftermarket non-compliant piece of hardware installed.
Actionable steps I suggest after this series lands:
1. don't do any changes, but since we know what 3rd party touchscreen do
incorrectly deviating from the standard, REPORT it to the userspace, so USER
know, their device (phone/tablet) doesn't have original part.
2. then figure out, IF we can reasonably well workaround it and HOW to do it
These two steps present some progress which could be discussed and could lead us
somewhere, what do you think?
David
>
> Thanks.
>
^ permalink raw reply
* [PATCH] HID: amd_sfh: return an error when response wait times out
From: Pengpeng Hou @ 2026-06-24 14:35 UTC (permalink / raw)
To: Basavaraj Natikar, Jiri Kosina, Benjamin Tissoires
Cc: Pengpeng Hou, linux-input, linux-kernel
amdtp_wait_for_response() waits for request_done before completing a
report request. wait_event_interruptible_timeout() returns 0 when the
wait expires, but the current code treats only negative values as errors
and returns success on timeout.
Return -ETIMEDOUT when the response wait expires while preserving the
existing success path when the response has already been observed.
Signed-off-by: Pengpeng Hou <pengpeng@iscas.ac.cn>
---
drivers/hid/amd-sfh-hid/amd_sfh_hid.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/amd-sfh-hid/amd_sfh_hid.c b/drivers/hid/amd-sfh-hid/amd_sfh_hid.c
index b04f675d4..8f88f965f 100644
--- a/drivers/hid/amd-sfh-hid/amd_sfh_hid.c
+++ b/drivers/hid/amd-sfh-hid/amd_sfh_hid.c
@@ -87,16 +87,17 @@ static int amdtp_wait_for_response(struct hid_device *hid)
break;
}
- if (!cli_data->request_done[i])
+ if (!cli_data->request_done[i]) {
ret = wait_event_interruptible_timeout(hid_data->hid_wait,
cli_data->request_done[i],
msecs_to_jiffies(AMD_SFH_RESPONSE_TIMEOUT));
- if (ret == -ERESTARTSYS)
- return -ERESTARTSYS;
- else if (ret < 0)
- return -ETIMEDOUT;
- else
- return 0;
+ if (ret == -ERESTARTSYS)
+ return -ERESTARTSYS;
+ if (ret <= 0)
+ return -ETIMEDOUT;
+ }
+
+ return 0;
}
void amdtp_hid_wakeup(struct hid_device *hid)
--
2.50.1 (Apple Git-155)
^ permalink raw reply related
* Re: [PATCH v2 0/8] HID: iio: Avoid race between callback setup and device exposure
From: Andy Shevchenko @ 2026-06-24 11:18 UTC (permalink / raw)
To: Sanjay Chitroda
Cc: Jiri Kosina, Jonathan Cameron, Srinivas Pandruvada, David Lechner,
Nuno Sá, Andy Shevchenko, Archana Patni, Song Hongyan,
linux-input, linux-iio, linux-kernel, srinivas pandruvada
In-Reply-To: <7288DEE7-91CD-4823-959B-229AEF1CAD97@gmail.com>
On Wed, Jun 24, 2026 at 07:13:12AM +0530, Sanjay Chitroda wrote:
> On 23 June 2026 4:00:27 pm IST, Andy Shevchenko <andriy.shevchenko@intel.com> wrote:
> >On Mon, Jun 22, 2026 at 10:59:56AM +0530, Sanjay Chitroda wrote:
> >>
> >> This series avoid a race condition in HID IIO drivers related to the
> >> ordering between callback registration and device exposure.
> >>
> >> Currently, several HID IIO drivers register the IIO device (making it
> >> visible to userspace and other kernel consumers) before all required
> >> callbacks and resources are fully initialized, or rely on devm-based
> >> cleanup in a way that does not guarantee correct teardown ordering.
> >> This creates a window where the device can be accessed while it is
> >
> >There is a difference between "this creates" and "this might create".
> >I believe Srinivas and others were asking for the proof. So, what path
> >in the code makes this happen or possible to happen?
> >
> iio_device_register() exposes the IIO device to user space, while
> sensor_hub_register_callback() registers callbacks for buffered IIO(streaming
> mode).
>
> This might create window where from userspace buffer mode is enabled and
> callbacks are not registered which would result into loss of samples until
> callback registration completes, although no explicit failure. In teardown
> path which can resulting in stale/no data.
>
> This was discussed in the v1 thread and v2 was posted based on discussion and
> agreement:
> https://lore.kernel.org/all/3FED088A-651B-4E8B-840B-1B92CB4DF6F4@gmail.com/
>
> >> not fully initialized or is being torn down, potentially leading to
> >> sample drop or stale/no data.
> >>
> >> To handle this, the series ensures that:
> >> - All required callbacks and resources are set up before the device
> >> is registered with the IIO core
> >> - Resource cleanup is performed explicitly where ordering matters
> >>
> >> PS: This is prepratory series to convert all HID IIO driver to devm.
> >>
> >> Testing:
> >> - Compiled with W=1 for each patch in series
> >>
> >> ---
> >> Changes in v2:
> >> - Drop fixes tag and rectify commit message with reference to that
> >
> >You also dropped my tag. Why?
> >
> Thank you for the review and tag on v1.
>
> While code changes are intact in v2, the rational and commit message were
> updated substantially. Since commit message is as important as change which
> will be permanent in history for future reference, I chose to drop the tag to
> request a fresh review.
Now it's clear, thanks.
This version with changed commit messages seems good to me.
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> I shall highlight the same in change log. I'll make sure to note in future revision.
Yes, please.
>
> >> - Link to v1: https://patch.msgid.link/20260606-5-june-hid-iio-race-fixes-v1-0-27a848c5758f@gmail.com
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH 0/4] input: misc: Add an initial driver for haptics inside Qcom PMIH010x PMIC
From: Krzysztof Kozlowski @ 2026-06-24 10:05 UTC (permalink / raw)
To: Fenglin Wu
Cc: linux-arm-msm, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lee Jones, Stephen Boyd, Bjorn Andersson,
Konrad Dybcio, David Collins, Subbaraman Narayanamurthy,
Kamal Wadhwa, kernel, linux-input, devicetree, linux-kernel
In-Reply-To: <be2b54a5-ce9d-49a2-80e1-60da874350d9@oss.qualcomm.com>
On 24/06/2026 09:20, Fenglin Wu wrote:
>
> On 6/17/2026 6:42 PM, Krzysztof Kozlowski wrote:
>> On Tue, Jun 16, 2026 at 03:08:23AM -0700, Fenglin Wu wrote:
>>
>> Here - the first sentence - is where you mention merging
>> constraints/strategy/dependencies. Your MFD patch depends on ealier
>> ones.
>>
> Did you mean that these 2 MFD binding changes should be listed as the
> dependency of the MFD patch?
No. Act as maintainer. Clone Linus tree, apply the patch and see if
everything works. My claim is that nothing works and maintainer tree is
broken.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] Input: gscps2 - advance receive buffer write index
From: sashiko-bot @ 2026-06-24 9:59 UTC (permalink / raw)
To: raoxu; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <460B5655BA580C60+20260624094739.850306-1-raoxu@uniontech.com>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [High] Lockless access to the ring buffer consumer index causes a data race and buffer corruption.
- [High] Lockless modifications and traversals of the global `ps2port_list` lead to list corruption and use-after-free.
- [High] Registering the shared IRQ before adding the device to the port list creates a window for an unhandled interrupt storm.
- [High] Unbounded hardware polling loops can hang the CPU if the device enters a faulty state.
- [High] The shared interrupt handler unconditionally returns `IRQ_HANDLED`, defeating the kernel's spurious interrupt detector.
--
commit b6fd4271cba05f59b05437ee042428ed8db2fb92
Author: Xu Rao <raoxu@uniontech.com>
Input: gscps2 - advance receive buffer write index
Commit 44f920069911 moved the receive loop into gscps2_read_data() and
gscps2_report_data(), but omitted the producer index update. This patch
restores the omitted index update so data reaches the serio core.
> diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
> index 22b2f57fd91f5..bf9b993f57330 100644
> --- a/drivers/input/serio/gscps2.c
> +++ b/drivers/input/serio/gscps2.c
> @@ -219,6 +219,7 @@ static void gscps2_read_data(struct gscps2port *ps2port)
> ps2port->buffer[ps2port->append].str = status;
> ps2port->buffer[ps2port->append].data =
> gscps2_readb_input(ps2port->addr);
> + ps2port->append = (ps2port->append + 1) & BUFFER_SIZE;
> } while (true);
> }
[Severity: High]
This is a pre-existing issue, but can this unbounded loop hang the CPU?
If the hardware enters a faulty state or is hot-unplugged, reading the status
register might continually return 0xFF. The loop condition earlier in this
function:
if (!(status & GSC_STAT_RBNE))
break;
would remain true forever (0xFF & 0x01). Similar unbounded polling exists in
gscps2_flush() and gscps2_writeb_output(). Should there be a timeout or retry
limit here?
[Severity: High]
This isn't a bug introduced by this patch, but does the lockless access to
ps2port->act cause a data race?
In gscps2_report_data(), ps2port->act is evaluated and advanced without
holding ps2port->lock:
gscps2_report_data() {
...
while (ps2port->act != ps2port->append) {
...
ps2port->act = (ps2port->act + 1) & BUFFER_SIZE;
}
}
Since gscps2_interrupt() can be called synchronously from process context
(like in gscps2_writeb_output() with gscps2_interrupt(0, NULL)) while local
interrupts are enabled, a hardware IRQ could fire concurrently on another CPU.
Could both CPUs execute gscps2_report_data() simultaneously and corrupt the
buffer?
[Severity: High]
This isn't a bug introduced by this patch, but is it safe to iterate
ps2port_list without locks?
gscps2_probe() and gscps2_remove() modify the global ps2port_list locklessly:
gscps2_probe() {
...
list_add_tail(&ps2port->node, &ps2port_list);
}
Meanwhile, gscps2_interrupt() iterates this list without any locks:
gscps2_interrupt() {
...
list_for_each_entry(ps2port, &ps2port_list, node) {
}
Since gscps2_interrupt() can be invoked at any time by a shared hardware IRQ
or manually via gscps2_writeb_output(), could this traversal race with a
concurrent probe or remove operation on a different CPU and lead to a
use-after-free?
[Severity: High]
This isn't a bug introduced by this patch, but does registering the shared
IRQ before adding the device to the port list create an interrupt storm
window?
In gscps2_probe(), the IRQ is requested before the device is added to the
list:
gscps2_probe() {
...
if (request_irq(dev->irq, gscps2_interrupt, IRQF_SHARED, ...))
...
list_add_tail(&ps2port->node, &ps2port_list);
}
If an interrupt fires immediately after registration, gscps2_interrupt() will
iterate the list, find nothing, and return IRQ_HANDLED without clearing the
hardware interrupt condition. For a level-triggered IRQ, could this
immediately re-fire and hang the CPU?
[Severity: High]
This isn't a bug introduced by this patch, but does the interrupt handler
always return IRQ_HANDLED?
gscps2_interrupt() is registered with IRQF_SHARED, but exits unconditionally
claiming it handled the interrupt:
gscps2_interrupt() {
...
return IRQ_HANDLED;
}
If a different device sharing the same IRQ line continuously asserts the
interrupt due to a fault, the kernel relies on handlers returning IRQ_NONE
to detect the spurious storm and safely disable the IRQ line.
By always returning IRQ_HANDLED, does this prevent the kernel from detecting
spurious interrupts from neighboring devices?
--
Sashiko AI review · https://sashiko.dev/#/patchset/460B5655BA580C60+20260624094739.850306-1-raoxu@uniontech.com?part=1
^ permalink raw reply
* [PATCH] Input: gscps2 - advance receive buffer write index
From: raoxu @ 2026-06-24 9:47 UTC (permalink / raw)
To: James.Bottomley
Cc: deller, dmitry.torokhov, linux-parisc, linux-input, linux-kernel,
raoxu, stable
From: Xu Rao <raoxu@uniontech.com>
Commit 44f920069911 ("Input: gscps2 - use guard notation when
acquiring spinlock") moved the receive loop into gscps2_read_data()
and gscps2_report_data().
While moving the code, it preserved the writes to
buffer[ps2port->append], but omitted the following producer index
update from the original loop:
ps2port->append = (ps2port->append + 1) & BUFFER_SIZE;
As a result, append never advances. Since gscps2_report_data() only
reports bytes while act != append, the receive buffer always appears
empty and no keyboard or mouse data reaches the serio core.
Restore the omitted index update.
Fixes: 44f920069911 ("Input: gscps2 - use guard notation when acquiring spinlock")
Cc: stable@vger.kernel.org # 6.13+
Signed-off-by: Xu Rao <raoxu@uniontech.com>
---
drivers/input/serio/gscps2.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/input/serio/gscps2.c b/drivers/input/serio/gscps2.c
index 22b2f57..bf9b993 100644
--- a/drivers/input/serio/gscps2.c
+++ b/drivers/input/serio/gscps2.c
@@ -219,6 +219,7 @@ static void gscps2_read_data(struct gscps2port *ps2port)
ps2port->buffer[ps2port->append].str = status;
ps2port->buffer[ps2port->append].data =
gscps2_readb_input(ps2port->addr);
+ ps2port->append = (ps2port->append + 1) & BUFFER_SIZE;
} while (true);
}
--
2.47.3
^ permalink raw reply related
* Re: [PATCH 0/4] input: misc: Add an initial driver for haptics inside Qcom PMIH010x PMIC
From: Fenglin Wu @ 2026-06-24 7:20 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-arm-msm, Dmitry Torokhov, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Lee Jones, Stephen Boyd, Bjorn Andersson,
Konrad Dybcio, David Collins, Subbaraman Narayanamurthy,
Kamal Wadhwa, kernel, linux-input, devicetree, linux-kernel
In-Reply-To: <20260617-spry-greedy-chital-1276e0@quoll>
On 6/17/2026 6:42 PM, Krzysztof Kozlowski wrote:
> On Tue, Jun 16, 2026 at 03:08:23AM -0700, Fenglin Wu wrote:
>
> Here - the first sentence - is where you mention merging
> constraints/strategy/dependencies. Your MFD patch depends on ealier
> ones.
>
Did you mean that these 2 MFD binding changes should be listed as the
dependency of the MFD patch?
https://lore.kernel.org/linux-arm-msm/20260609-pmg1110-v1-1-6604d0adc907@oss.qualcomm.com/
https://lore.kernel.org/linux-arm-msm/20260610124119.253456-2-krzysztof.kozlowski@oss.qualcomm.com/
>> Qualcomm PMIH010x 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:
>>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCH v5 4/6] dt-bindings: input: sun4i-lradc-keys: Add A100/A133 compatible
From: Krzysztof Kozlowski @ 2026-06-24 6:51 UTC (permalink / raw)
To: Alexander Sverdlin
Cc: linux-arm-kernel, linux-sunxi, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Chen-Yu Tsai, Jernej Skrabec, Samuel Holland,
Hans de Goede, Dmitry Torokhov, Andre Przywara, Jun Yan,
Lukas Schmid, J. Neuschäfer, Eric Biggers, Michal Simek,
Luca Weiss, Sven Peter, Maxime Ripard, devicetree, linux-kernel,
linux-input
In-Reply-To: <20260623204824.691832-5-alexander.sverdlin@gmail.com>
On Tue, Jun 23, 2026 at 10:48:16PM +0200, Alexander Sverdlin wrote:
> The Allwinner A100/A133 SoCs have an LRADC which is compatible with the
> versions in existing SoCs. Add a compatible string for A100, with the R329
> fallback.
>
> Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> ---
Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 4/4] Input: ensure device is ready before delivering events
From: sashiko-bot @ 2026-06-24 6:02 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260624055008.2494980-4-dmitry.torokhov@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing physical feedback teardown in input_close_device causes LEDs/sounds to remain stuck ON indefinitely.
--
commit 75d930ba0fffcfc837221baa51848c81d0ba0c9e
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Input: ensure device is ready before delivering events
This commit introduces a ready flag in the input_dev structure to prevent
events from being delivered to the hardware before the driver's open() method
successfully completes. It ensures logical state is replayed upon opening,
and synchronizes physical feedback teardown during inhibition.
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index 0a95cbdc467ed..724cc146fc093 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
[ ... ]
> @@ -661,6 +708,10 @@ void input_close_device(struct input_handle *handle)
> if (!--dev->users && !dev->inhibited) {
> if (dev->poller)
> input_dev_poller_stop(dev->poller);
> +
> + scoped_guard(spinlock_irq, &dev->event_lock)
> + dev->ready = false;
[Severity: Medium]
Will this leave LEDs and sounds permanently stuck on when a device is closed?
In input_inhibit_device(), there is a call to input_dev_toggle(dev, false)
right before setting dev->ready to false.
However, in input_close_device(), dev->ready is cleared without first
toggling the state off.
Since the suspend and poweroff handlers rely on dev->ready to issue events,
could omitting input_dev_toggle() here prevent them from turning off physical
feedback, resulting in a persistent power drain?
> +
> if (dev->close)
> dev->close(dev);
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624055008.2494980-1-dmitry.torokhov@gmail.com?part=4
^ permalink raw reply
* Re: [PATCH 1/4] Input: fix poller start order on uninhibit
From: sashiko-bot @ 2026-06-24 6:00 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260624055008.2494980-1-dmitry.torokhov@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Initial events from `dev->open()` are dropped during device uninhibit
--
commit f4fc68b8f579c449b7f9056431e51975c324c92e
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Input: fix poller start order on uninhibit
This commit resolves an issue where initial events generated by the poller
are dropped during a device uninhibit operation. It defers starting the
poller until after the device is fully marked as uninhibited.
> diff --git a/drivers/input/input.c b/drivers/input/input.c
> index cf6fecea79b82..d99dfe69e12d8 100644
> --- a/drivers/input/input.c
> +++ b/drivers/input/input.c
> @@ -1793,8 +1793,6 @@ static int input_uninhibit_device(struct input_dev *dev)
> if (error)
> return error;
> }
[Severity: High]
This isn't a bug introduced by this patch, but it appears the exact same
logic flaw exists for dev->open() which is called just above this snippet
in input_uninhibit_device():
if (dev->open) {
error = dev->open(dev);
if (error)
return error;
}
Since dev->inhibited is still true when dev->open() is called, if a driver
generates initial synchronous events during its open routine (for example,
gpio_keys_report_state() called by gpio_keys_open()), won't those events
also be silently filtered out by input_get_disposition() returning
INPUT_IGNORE_EVENT?
Since this patch addresses this state mismatch for the poller, should the
dev->open() call also be deferred until after dev->inhibited is set to false?
> - if (dev->poller)
> - input_dev_poller_start(dev->poller);
> }
>
> dev->inhibited = false;
> @@ -1802,6 +1800,9 @@ static int input_uninhibit_device(struct input_dev *dev)
> scoped_guard(spinlock_irq, &dev->event_lock)
> input_dev_toggle(dev, true);
>
> + if (dev->users && dev->poller)
> + input_dev_poller_start(dev->poller);
> +
> return 0;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260624055008.2494980-1-dmitry.torokhov@gmail.com?part=1
^ permalink raw reply
* [PATCH 4/4] Input: ensure device is ready before delivering events
From: Dmitry Torokhov @ 2026-06-24 5:50 UTC (permalink / raw)
To: linux-input, Jiri Kosina, Benjamin Tissoires; +Cc: linux-kernel
In-Reply-To: <20260624055008.2494980-1-dmitry.torokhov@gmail.com>
When a device is opened via input_open_device(), the driver's open()
callback is invoked. Some drivers, like cm109, submit URBs or perform
other hardware initialization in their open() callbacks.
However, the input core does not prevent dev->event() from being called
concurrently during the driver's open() execution. For instance, if a
console beep occurs, the kbd handler might inject an EV_SND event. This
can lead to double list_add BUGs if the driver submits the same URB in
both open() and event() paths without adequate synchronization.
To fix this, introduce a ready flag in the input_dev structure.
For complex devices (where dev->open is defined), this flag is set to true
only after the driver's open() method successfully completes. The core now
checks ready in input_event_dispose() and input_dev_toggle()
to prevent events from reaching the hardware before it is fully prepared.
For simple devices (no open callback), events are delivered immediately.
We also replay the logical state in input_open_device() by calling
input_dev_toggle() right after marking the device ready, ensuring no
events are permanently lost.
In the inhibit path, we ensure that physical feedback (LEDs/sounds) is
turned off before the device is closed, and we synchronize the inhibited
state transition under the event lock to prevent races with incoming events.
Assisted-by: Antigravity:gemini-3.5-flash
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/input.c | 107 ++++++++++++++++++++++++++----------------
include/linux/input.h | 12 +++--
2 files changed, 75 insertions(+), 44 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index 0a95cbdc467e..724cc146fc09 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -318,7 +318,7 @@ static int input_get_disposition(struct input_dev *dev,
static void input_event_dispose(struct input_dev *dev, int disposition,
unsigned int type, unsigned int code, int value)
{
- if ((disposition & INPUT_PASS_TO_DEVICE) && dev->event)
+ if ((disposition & INPUT_PASS_TO_DEVICE) && dev->event && dev->ready)
dev->event(dev, type, code, value);
if (disposition & INPUT_PASS_TO_HANDLERS) {
@@ -568,6 +568,48 @@ void input_release_device(struct input_handle *handle)
}
EXPORT_SYMBOL(input_release_device);
+#define INPUT_DO_TOGGLE(dev, type, bits, on) \
+ do { \
+ int i; \
+ bool active; \
+ \
+ if (!test_bit(EV_##type, dev->evbit)) \
+ break; \
+ \
+ for_each_set_bit(i, dev->bits##bit, type##_CNT) { \
+ active = test_bit(i, dev->bits); \
+ if (!active && !on) \
+ continue; \
+ \
+ dev->event(dev, EV_##type, i, on ? active : 0); \
+ } \
+ } while (0)
+
+/*
+ * Iterate through the logical state of the input device (LEDs, sounds,
+ * auto-repeat) and explicitly push that state down to the hardware
+ * via dev->event() to match the current logical state (if activate is true),
+ * or forcibly turn off all feedback like LEDs and sounds during teardown
+ * or suspend (if activate is false).
+ *
+ * Primarily used as a state-replay mechanism after a device is opened
+ * or uninhibited, as events might have been dropped by the core while the
+ * hardware was not marked as ready.
+ */
+static void input_dev_toggle(struct input_dev *dev, bool activate)
+{
+ if (!dev->event || !dev->ready)
+ return;
+
+ INPUT_DO_TOGGLE(dev, LED, led, activate);
+ INPUT_DO_TOGGLE(dev, SND, snd, activate);
+
+ if (activate && test_bit(EV_REP, dev->evbit)) {
+ dev->event(dev, EV_REP, REP_PERIOD, dev->rep[REP_PERIOD]);
+ dev->event(dev, EV_REP, REP_DELAY, dev->rep[REP_DELAY]);
+ }
+}
+
static int input_start_device(struct input_dev *dev)
{
int error;
@@ -583,6 +625,11 @@ static int input_start_device(struct input_dev *dev)
}
}
+ scoped_guard(spinlock_irq, &dev->event_lock) {
+ dev->ready = true;
+ input_dev_toggle(dev, true);
+ }
+
if (dev->poller)
input_dev_poller_start(dev->poller);
}
@@ -661,6 +708,10 @@ void input_close_device(struct input_handle *handle)
if (!--dev->users && !dev->inhibited) {
if (dev->poller)
input_dev_poller_stop(dev->poller);
+
+ scoped_guard(spinlock_irq, &dev->event_lock)
+ dev->ready = false;
+
if (dev->close)
dev->close(dev);
}
@@ -1712,37 +1763,6 @@ static int input_dev_uevent(const struct device *device, struct kobj_uevent_env
return 0;
}
-#define INPUT_DO_TOGGLE(dev, type, bits, on) \
- do { \
- int i; \
- bool active; \
- \
- if (!test_bit(EV_##type, dev->evbit)) \
- break; \
- \
- for_each_set_bit(i, dev->bits##bit, type##_CNT) { \
- active = test_bit(i, dev->bits); \
- if (!active && !on) \
- continue; \
- \
- dev->event(dev, EV_##type, i, on ? active : 0); \
- } \
- } while (0)
-
-static void input_dev_toggle(struct input_dev *dev, bool activate)
-{
- if (!dev->event)
- return;
-
- INPUT_DO_TOGGLE(dev, LED, led, activate);
- INPUT_DO_TOGGLE(dev, SND, snd, activate);
-
- if (activate && test_bit(EV_REP, dev->evbit)) {
- dev->event(dev, EV_REP, REP_PERIOD, dev->rep[REP_PERIOD]);
- dev->event(dev, EV_REP, REP_DELAY, dev->rep[REP_DELAY]);
- }
-}
-
/**
* input_reset_device() - reset/restore the state of input device
* @dev: input device whose state needs to be reset
@@ -1770,21 +1790,25 @@ static int input_inhibit_device(struct input_dev *dev)
return 0;
if (dev->users) {
- if (dev->close)
- dev->close(dev);
if (dev->poller)
input_dev_poller_stop(dev->poller);
+
+ scoped_guard(spinlock_irq, &dev->event_lock) {
+ input_dev_toggle(dev, false);
+ dev->ready = false;
+ }
+
+ if (dev->close)
+ dev->close(dev);
}
scoped_guard(spinlock_irq, &dev->event_lock) {
input_mt_release_slots(dev);
input_dev_release_keys(dev);
input_handle_event(dev, EV_SYN, SYN_REPORT, 1);
- input_dev_toggle(dev, false);
+ dev->inhibited = true;
}
- dev->inhibited = true;
-
return 0;
}
@@ -1804,12 +1828,15 @@ static int input_uninhibit_device(struct input_dev *dev)
if (error)
return error;
}
- }
- dev->inhibited = false;
+ guard(spinlock_irq)(&dev->event_lock);
+ dev->ready = true;
+ }
- scoped_guard(spinlock_irq, &dev->event_lock)
+ scoped_guard(spinlock_irq, &dev->event_lock) {
+ dev->inhibited = false;
input_dev_toggle(dev, true);
+ }
if (dev->users && dev->poller)
input_dev_poller_start(dev->poller);
diff --git a/include/linux/input.h b/include/linux/input.h
index f7a2cfad5448..a307a3f6d5e2 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -128,11 +128,14 @@ enum input_clock_type {
* @devres_managed: indicates that devices is managed with devres framework
* and needs not be explicitly unregistered or freed.
* @timestamp: storage for a timestamp set by input_set_timestamp called
- * by a driver
+ * by a driver
* @inhibited: indicates that the input device is inhibited. If that is
- * the case then input core ignores any events generated by the device.
- * Device's close() is called when it is being inhibited and its open()
- * is called when it is being uninhibited.
+ * the case then input core ignores any events generated by the device.
+ * Device's close() is called when it is being inhibited and its open()
+ * is called when it is being uninhibited.
+ * @ready: indicates that the device has been successfully opened and is
+ * prepared to process events (like LEDs or sounds) sent from the
+ * input core.
*/
struct input_dev {
const char *name;
@@ -209,6 +212,7 @@ struct input_dev {
ktime_t timestamp[INPUT_CLK_MAX];
bool inhibited;
+ bool ready;
};
#define to_input_dev(d) container_of(d, struct input_dev, dev)
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
* [PATCH 3/4] Input: defer handler's start() until device is opened
From: Dmitry Torokhov @ 2026-06-24 5:50 UTC (permalink / raw)
To: linux-input, Jiri Kosina, Benjamin Tissoires; +Cc: linux-kernel
In-Reply-To: <20260624055008.2494980-1-dmitry.torokhov@gmail.com>
When registering an input handle, handler->start() is currently called
immediately. However, the input device might not be fully opened or
ready to process events at this stage, meaning any state synchronization
events (like setting LED states) injected by the handler's start method
might be dropped.
Move the handler->start() invocation to input_open_device(). If it is
the first handle opening the device, start() is called after the driver's
open() method has successfully completed and the device is fully prepared.
To facilitate this, factor out the device startup logic (calling driver's
open and starting polling) into input_start_device().
For passive observer handlers, their start() method is also deferred
until the handle is opened. Since opening a passive observer handle does
not start the underlying hardware device, their start() method is called
immediately upon opening, regardless of whether the device is active.
Fixes: c7e8dc6ee6d5 ("Input: add start() method to input handlers")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/input/input.c | 45 +++++++++++++++++++++++++------------------
include/linux/input.h | 5 +++--
2 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/drivers/input/input.c b/drivers/input/input.c
index c2a038d31beb..0a95cbdc467e 100644
--- a/drivers/input/input.c
+++ b/drivers/input/input.c
@@ -568,6 +568,28 @@ void input_release_device(struct input_handle *handle)
}
EXPORT_SYMBOL(input_release_device);
+static int input_start_device(struct input_dev *dev)
+{
+ int error;
+
+ lockdep_assert_held(&dev->mutex);
+
+ if (dev->users++ == 0 && !dev->inhibited) {
+ if (dev->open) {
+ error = dev->open(dev);
+ if (error) {
+ dev->users--;
+ return error;
+ }
+ }
+
+ if (dev->poller)
+ input_dev_poller_start(dev->poller);
+ }
+
+ return 0;
+}
+
/**
* input_open_device - open input device
* @handle: handle through which device is being accessed
@@ -586,21 +608,9 @@ int input_open_device(struct input_handle *handle)
handle->open++;
- if (handle->handler->passive_observer)
- return 0;
-
- if (dev->users++ || dev->inhibited) {
- /*
- * Device is already opened and/or inhibited,
- * so we can exit immediately and report success.
- */
- return 0;
- }
-
- if (dev->open) {
- error = dev->open(dev);
+ if (!handle->handler->passive_observer) {
+ error = input_start_device(dev);
if (error) {
- dev->users--;
handle->open--;
/*
* Make sure we are not delivering any more
@@ -611,8 +621,8 @@ int input_open_device(struct input_handle *handle)
}
}
- if (dev->poller)
- input_dev_poller_start(dev->poller);
+ if (handle->open == 1 && handle->handler->start)
+ handle->handler->start(handle);
}
return 0;
@@ -2662,9 +2672,6 @@ int input_register_handle(struct input_handle *handle)
*/
list_add_tail_rcu(&handle->h_node, &handler->h_list);
- if (handler->start)
- handler->start(handle);
-
return 0;
}
EXPORT_SYMBOL(input_register_handle);
diff --git a/include/linux/input.h b/include/linux/input.h
index 3022bb730898..f7a2cfad5448 100644
--- a/include/linux/input.h
+++ b/include/linux/input.h
@@ -284,8 +284,9 @@ struct input_handle;
* @connect: called when attaching a handler to an input device
* @disconnect: disconnects a handler from input device
* @start: starts handler for given handle. This function is called by
- * input core right after connect() method and also when a process
- * that "grabbed" a device releases it
+ * input core when device is open and ready to process events,
+ * and also when device is uninhibited or when a process that "grabbed"
+ * a device releases it
* @passive_observer: set to %true by drivers only interested in observing
* data stream from devices if there are other users present. Such
* drivers will not result in starting underlying hardware device
--
2.55.0.rc0.799.gd6f94ed593-goog
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox