* Re: [PATCH 2/4] Input: sur40 - fix V4L error path cleanup
From: sashiko-bot @ 2026-06-16 5:28 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: dmitry.torokhov, linux-input
In-Reply-To: <20260616051235.1549517-2-dmitry.torokhov@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Calling `vb2_wait_for_all_buffers` in `sur40_stop_streaming` causes a system deadlock.
--
commit 1d8742f82651a7f0160c799526e70838d3c9fa29
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Input: sur40 - fix V4L error path cleanup
In sur40_probe(), if video_register_device() fails, the error path jumps to
err_unreg_video. This incorrectly attempts to unregister a video device
that was never successfully registered, and fails to free the V4L2 control
handler (v4l2_ctrl_handler_free) that was initialized immediately prior.
> diff --git a/drivers/input/touchscreen/sur40.c b/drivers/input/touchscreen/sur40.c
> index 8639ec3ad7038..e9089b0c3e2f4 100644
> --- a/drivers/input/touchscreen/sur40.c
> +++ b/drivers/input/touchscreen/sur40.c
[ ... ]
[Severity: High]
This is a pre-existing issue, but can the current implementation of
sur40_stop_streaming() cause a system deadlock?
Looking at drivers/input/touchscreen/sur40.c:sur40_stop_streaming():
vb2_wait_for_all_buffers(vq);
sur40->sequence = -1;
/* Release all active buffers */
return_all_buffers(sur40, VB2_BUF_STATE_ERROR);
The driver unconditionally calls vb2_wait_for_all_buffers() before forcibly
returning the active buffers. This blocks infinitely until all queued buffers
are returned via vb2_buffer_done().
The driver expects the poller thread (sur40_poll) to process and return these
buffers. Is it possible for the poller thread to be stopped or not running
when this is called?
For instance, if userspace opens only /dev/videoX without ever opening
/dev/input/eventX, the input-bound poller never starts.
Additionally, if the device is abruptly unplugged while streaming is active,
sur40_disconnect() calls input_unregister_device(), which synchronously stops
the poller. When the video file descriptor is subsequently closed,
sur40_stop_streaming() is triggered.
In both of these scenarios, won't vb2_wait_for_all_buffers() wait forever for
buffers to be returned by a thread that is not running? Should the driver
synchronously cancel transfers and immediately return all queued buffers with
VB2_BUF_STATE_ERROR instead of waiting?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616051235.1549517-1-dmitry.torokhov@gmail.com?part=2
^ permalink raw reply
* Re: [PATCH v8 4/7] input: keyboard: Add driver for ASUS Transformer dock multimedia keys
From: Svyatoslav Ryhel @ 2026-06-16 6:25 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Lee Jones,
Pavel Machek, Sebastian Reichel, Ion Agorria,
Michał Mirosław, devicetree, linux-kernel, linux-input,
linux-leds, linux-pm
In-Reply-To: <ajDPtOyr8GJYaVYQ@google.com>
вт, 16 черв. 2026 р. о 07:26 Dmitry Torokhov <dmitry.torokhov@gmail.com> пише:
>
> Hi Svyatoslav,
>
> On Thu, May 28, 2026 at 08:32:00AM +0300, Svyatoslav Ryhel wrote:
> > From: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> >
> > Add support for multimedia top button row of ASUS Transformer's Mobile
> > Dock keyboard. Driver is made that function keys (F1-F12) are used by
> > default which suits average Linux use better and with pressing
> > ScreenLock + AltGr function keys layout is switched to multimedia keys.
> > Since this only modifies codes sent by asus-ec-keys it doesn't affect
> > normal keyboards at all.
>
> I think using input handler to intercept ScreenLock + AltGr is quite
> awkward. I think this also passes the original key events (unless you
> make it a filter not a regular handler).
>
> I do not see benefit for reacting to AltGr+ScreenLock on other keyboards
> to activate the special mode on this one. So given the fact that you
> already mange the data stream when you split it into "serio" ports,
> maybe just intercept this key combo right there and create the input
> device and signal input events right there?
>
Though it seems awkward at a first glance, media keys are integrated
with a standard keyboard in a detachable dock. It is highly unlikely
that media keys will be used with a different keyboard then the one
that is integrated with dock. Additionally, the ScreenLock key has a
code specific to this driver and is not in general use, so even if any
standard keyboard has AltGr but none has ScreenLock specific to this
driver except the dock itself. Handler is also set as observer so it
should not interfere with work of other input devices.
> Thanks.
>
> --
> Dmitry
^ permalink raw reply
* Re: [PATCH v5 2/2] Input: isa1200 - new driver for Imagis ISA1200
From: Svyatoslav Ryhel @ 2026-06-16 6:45 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: linux-input, devicetree, linux-kernel, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Linus Walleij
In-Reply-To: <ajDEsU8oZWT7KB9d@google.com>
вт, 16 черв. 2026 р. о 07:16 Dmitry Torokhov <dmitry.torokhov@gmail.com> пише:
>
> Hi Svyatoslav,
>
> On Mon, Jun 15, 2026 at 09:19:27AM +0300, Svyatoslav Ryhel wrote:
> > чт, 28 трав. 2026 р. о 08:38 Svyatoslav Ryhel <clamor95@gmail.com> пише:
> > >
> > > вт, 12 трав. 2026 р. о 13:24 Svyatoslav Ryhel <clamor95@gmail.com> пише:
> > > >
> > > > From: Linus Walleij <linusw@kernel.org>
> > > >
> > > > The ISA1200 is a haptic feedback unit from Imagis Technology using two
> > > > motors for haptic feedback in mobile phones. Used in many mobile devices
> > > > c. 2012 including Samsung Galxy S Advance GT-I9070 (Janice), Samsung Beam
> > > > GT-I8350 (Gavini), LG Optimus 4X P880 and LG Optimus Vu P895.
> > > >
> > > > The exact datasheet for the ISA1200 is not available; all data was modeled
> > > > based on available downstream kernel sources for various devices and
> > > > fragments of information scattered across the internet.
> > > >
> > > > Tested-by: Linus Walleij <linusw@kernel.org> # GT-I9070 Janice
> > > > Signed-off-by: Linus Walleij <linusw@kernel.org>
> > > > Co-developed-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > Signed-off-by: Svyatoslav Ryhel <clamor95@gmail.com>
> > > > ---
> > > > drivers/input/misc/Kconfig | 12 +
> > > > drivers/input/misc/Makefile | 1 +
> > > > drivers/input/misc/isa1200.c | 524 +++++++++++++++++++++++++++++++++++
> > > > 3 files changed, 537 insertions(+)
> > > > create mode 100644 drivers/input/misc/isa1200.c
> > > >
> > >
> > > Hello Dmitry! Do I need to make any further adjustments to this driver?
> >
> > Hello Dmitry! Do I need to make any further adjustments to this
> > driver? This driver is hanging in LKML for some time already without
> > responds from input maintainer. It is still relevant and I would like
> > it to move forward.
>
> There were valid sashiko comments on the patch regarding resetting
> "level" to 0 and also potential racing conditions, as well as suggestion
> to check number of gpios specified in the device tree.
>
> Please see if the following works for you:
>
> diff --git a/drivers/input/misc/isa1200.c b/drivers/input/misc/isa1200.c
> index ff82252a08e1..c61adc4b605c 100644
> --- a/drivers/input/misc/isa1200.c
> +++ b/drivers/input/misc/isa1200.c
> @@ -131,6 +131,7 @@ struct isa1200 {
> struct work_struct play_work;
> struct isa1200_config config;
>
> + bool suspended;
> bool active;
> int level;
> };
> @@ -247,17 +248,21 @@ static void isa1200_stop(struct isa1200 *isa)
> isa->supplies);
>
> isa->active = false;
> - isa->level = 0;
> }
>
> static void isa1200_play_work(struct work_struct *work)
> {
> struct isa1200 *isa = container_of(work, struct isa1200, play_work);
> -
> - if (isa->level)
> - isa1200_start(isa);
> - else
> - isa1200_stop(isa);
> + struct input_dev *input = isa->input;
> +
> + scoped_guard(mutex_try, &input->mutex) {
> + if (!isa->suspended) {
> + if (isa->level)
> + isa1200_start(isa);
> + else
> + isa1200_stop(isa);
> + }
> + }
> }
>
> static int isa1200_vibrator_play_effect(struct input_dev *input, void *data,
> @@ -280,7 +285,8 @@ static int isa1200_vibrator_play_effect(struct input_dev *input, void *data,
>
> if (isa->level != level) {
> isa->level = level;
> - schedule_work(&isa->play_work);
> + if (!READ_ONCE(isa->suspended))
> + schedule_work(&isa->play_work);
> }
>
> return 0;
> @@ -292,6 +298,7 @@ static void isa1200_vibrator_close(struct input_dev *input)
>
> cancel_work_sync(&isa->play_work);
> isa1200_stop(isa);
> + isa->level = 0;
> }
>
> static int isa1200_of_probe(struct i2c_client *client)
> @@ -331,6 +338,9 @@ static int isa1200_of_probe(struct i2c_client *client)
> return dev_err_probe(dev, PTR_ERR(isa->enable_gpios),
> "failed to get enable gpios\n");
>
> + if (isa->enable_gpios && isa->enable_gpios->ndescs > ISA1200_EN_PINS_MAX)
> + return dev_err_probe(dev, -EINVAL, "too many enable gpios\n");
> +
> ldo_node = device_get_named_child_node(dev, "ldo");
> if (!ldo_node)
> return dev_err_probe(dev, -ENODEV,
> @@ -479,9 +489,9 @@ static int isa1200_suspend(struct device *dev)
> guard(mutex)(&isa->input->mutex);
>
> if (input_device_enabled(isa->input)) {
> + WRITE_ONCE(isa->suspended, true);
> cancel_work_sync(&isa->play_work);
> - if (isa->level)
> - isa1200_stop(isa);
> + isa1200_stop(isa);
> }
>
> return 0;
> @@ -493,9 +503,11 @@ static int isa1200_resume(struct device *dev)
>
> guard(mutex)(&isa->input->mutex);
>
> - if (input_device_enabled(isa->input))
> + if (input_device_enabled(isa->input)) {
> + WRITE_ONCE(isa->suspended, false);
> if (isa->level)
> - isa1200_start(isa);
> + schedule_work(&isa->play_work);
> + }
>
> return 0;
> }
>
> --
> Dmitry
I have tested your code on my P895 and it works perfectly fine. Should
I resend with these changes or you can integrate them while picking
patchset?
Thank you for your suggestions and efforts!
Best regards,
Svyatoslav R.
^ permalink raw reply
* Re: [PATCH 1/6] Input: mms114 - fix touch indexing for MMS134S and MMS136
From: Bryam Vargas @ 2026-06-16 7:05 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Linus Walleij, linux-kernel
In-Reply-To: <20260616050912.1531241-1-dmitry.torokhov@gmail.com>
Hi Dmitry,
The indexing fix looks correct -- deriving the byte offset from event_size
instead of leaning on sizeof(struct mms114_touch) is the right call, and the
cast is fine since the struct is __packed (no alignment issue at the 6-byte
stride, and the consumers only touch bytes 0..5).
Two things that might be worth folding into the series:
1) Fixes: tag. The 6-byte event path for MMS136 -- and therefore this
stride-vs-index mismatch -- predates ab108678195f. It was introduced in
53fefdd1d3a3 ("Input: mms114 - support MMS136")
which added MMS136_EVENT_SIZE and the "packet_size / MMS136_EVENT_SIZE"
branch while the loop already indexed with the 8-byte struct stride;
ab108678195f ("support MMS134S") only extended that branch to MMS134S.
So MMS136 hardware has mis-parsed multi-touch since v5.13. Tagging
Fixes: 53fefdd1d3a3 ("Input: mms114 - support MMS136")
(in addition to, or instead of, ab108678195f) lets stable pick it up for
the MMS136 range as well.
2) Unbounded packet_size. Since 1/6 already rewrites this handler: packet_size
comes straight from the device's PACKET_SIZE register (a single byte, so
1..255 after the "<= 0" guard) and is then used unclamped both as the read
length
__mms114_read_reg(data, MMS114_INFORMATION, packet_size, (u8 *)touch);
into the 80-byte touch[MMS114_MAX_TOUCH] stack buffer, and as the divisor
for touch_size -- which is never bounded by MMS114_MAX_TOUCH. A device that
reports packet_size > 80 overflows the stack buffer on the read, and even
with the indexing fix the loop still walks past it (touch_size > 10). A
one-liner before the division closes both:
if (packet_size <= 0)
goto out;
+ packet_size = min_t(int, packet_size, sizeof(touch));
This one is pre-existing -- the unbounded read goes back to the original
driver -- so it is really a separate patch in the series rather than part
of the indexing fix, but it seemed worth raising while the code is in
flight.
Thanks,
Bryam
^ permalink raw reply
* Re: [PATCH 5/6] Input: mms114 - replace BUG() and fix alignment
From: Bryam Vargas @ 2026-06-16 7:21 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Linus Walleij, linux-kernel
In-Reply-To: <20260616050912.1531241-5-dmitry.torokhov@gmail.com>
Hi Dmitry,
Replacing BUG() with WARN_ON() + return is the right call, and the rest of
the patch is fine.
One nit on the changelog though: this guard isn't reachable from userspace.
reg and len are always driver-internal constants -- there is no sysfs/debugfs/
ioctl that lets userspace pick a register, every __mms114_read_reg() call site
passes a fixed reg >= MMS114_INFORMATION (0x10), and the single MMS114_MODE_CONTROL
read is short-circuited by the cache in mms114_read_reg() before it ever reaches
__mms114_read_reg(). So the condition only fires if a *future in-kernel caller*
requests a read that spans the write-only MODE_CONTROL register -- a driver-
internal invariant, not a userspace-triggered one.
Worth tightening the wording, since that distinction is exactly what justifies
WARN_ON() here: a genuinely userspace-reachable WARN_ON() would itself be a
problem (log spam / panic_on_warn). Maybe something like:
"Avoid taking the machine down with BUG() if a caller ever requests a read
spanning the write-only MODE_CONTROL register; warn and return -EINVAL so
the driver can recover."
The code is good either way.
Thanks,
Bryam
^ permalink raw reply
* Re: [PATCH 6/6] Input: mms114 - refactor chip variant handling using descriptors
From: Bryam Vargas @ 2026-06-16 7:42 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input, Linus Walleij, linux-kernel
In-Reply-To: <20260616050912.1531241-6-dmitry.torokhov@gmail.com>
Hi Dmitry,
The descriptor refactor looks good -- it preserves the per-variant behaviour
(event size, config-reg gating, get_version, fuzz handling) for all five chips.
One user-visible side effect worth a line in the changelog though: switching
input_dev->name from "MELFAS MMS%d" (the enum value) to "MELFAS %s" (chip->name)
changes the reported name for the two variants whose part number carries a
non-numeric suffix:
MMS134S: "MELFAS MMS134 Touchscreen" -> "MELFAS MMS134S Touchscreen"
MMS345L: "MELFAS MMS345 Touchscreen" -> "MELFAS MMS345L Touchscreen"
(MMS114/136/152 are byte-identical.) The old "%d" on the enum silently dropped
the S/L. That string is userspace-visible via EVIOCGNAME, /sys/class/input/*/name
and /proc/bus/input/devices, so any udev/hwdb/libinput rule keyed on the old name
would stop matching. The in-tree boards affected are ste-ux500-samsung-kyle
(melfas,mms134s) and msm8916-samsung-a5u-eur / msm8916-samsung-e5 (melfas,mms345l).
The new names are more correct, so this is just worth a heads-up in the commit
message rather than a code change -- in case an out-of-tree quirk keys on the old
truncated strings.
Thanks,
Bryam
^ permalink raw reply
* [PATCH v4 1/3] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: Lee Jones @ 2026-06-16 9:26 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Peter Hutterer, Dmitry Torokhov, linux-input, linux-kernel
wacom_intuos_pad() accesses wacom->shared->touch_input locklessly
inside the interrupt handler context. If the Touch sibling device
is disconnected, wacom_remove_shared_data() clears 'touch_input'
outside any lock, creating a Time-of-Check to Time-of-Use (TOCTOU)
race condition where a preempted reader in interrupt context
dereferences the freed pointer, leading to a Use-After-Free.
Resolve this by introducing RCU protection for the touch_input
pointer:
- Annotate 'touch_input' in wacom_shared struct with __rcu
- Wrap all lockless readers in wacom_wac.c with rcu_read_lock() and
rcu_dereference() using a unified wacom_report_touch_mute()
helper
- Update writers in wacom_sys.c using rcu_assign_pointer()
- Call synchronize_rcu() in wacom_remove_shared_data() to ensure
all active RCU readers have finished before the input device is
freed
Also wrap wacom_set_shared_values() and touch/pen assignments in
wacom_add_shared_data() inside the wacom_udev_list_lock to serialize
concurrent probe assignments, and verify that 'shared->touch == hdev'
before setting touch_input to prevent concurrent sibling probe state
desynchronization.
Finally, advertise the SW_MUTE_DEVICE capability on Touch input
devices prior to registration in wacom_setup_touch_input_capabilities()
to prevent invalid post-registration capability modifications.
Fixes: 961794a00eab ("Input: wacom - add reporting of SW_MUTE_DEVICE events")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
v2 -> v3: Sashiko fixes
v3 -> v4: Dmitry's review [redundant check and guard()]
drivers/hid/wacom_sys.c | 41 ++++++++++++++-----------
drivers/hid/wacom_wac.c | 66 +++++++++++++++++++++--------------------
drivers/hid/wacom_wac.h | 2 +-
3 files changed, 58 insertions(+), 51 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 2220168bf116..86895f13dbae 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -877,10 +877,16 @@ static void wacom_remove_shared_data(void *res)
data = container_of(wacom_wac->shared, struct wacom_hdev_data,
shared);
- if (wacom_wac->shared->touch == wacom->hdev)
- wacom_wac->shared->touch = NULL;
- else if (wacom_wac->shared->pen == wacom->hdev)
- wacom_wac->shared->pen = NULL;
+ scoped_guard(mutex, &wacom_udev_list_lock) {
+ if (wacom_wac->shared->touch == wacom->hdev) {
+ wacom_wac->shared->touch = NULL;
+ rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
+ } else if (wacom_wac->shared->pen == wacom->hdev) {
+ wacom_wac->shared->pen = NULL;
+ }
+ }
+
+ synchronize_rcu();
kref_put(&data->kref, wacom_release_shared_data);
wacom_wac->shared = NULL;
@@ -909,6 +915,11 @@ static int wacom_add_shared_data(struct hid_device *hdev)
list_add_tail(&data->list, &wacom_udev_list);
}
+ if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
+ data->shared.touch = hdev;
+ else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
+ data->shared.pen = hdev;
+
mutex_unlock(&wacom_udev_list_lock);
wacom_wac->shared = &data->shared;
@@ -917,11 +928,6 @@ static int wacom_add_shared_data(struct hid_device *hdev)
if (retval)
return retval;
- if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
- wacom_wac->shared->touch = hdev;
- else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
- wacom_wac->shared->pen = hdev;
-
return retval;
}
@@ -2345,9 +2351,15 @@ static void wacom_release_resources(struct wacom *wacom)
static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
{
+ struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+
+ guard(mutex)(&wacom_udev_list_lock);
+
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
- wacom_wac->shared->type = wacom_wac->features.type;
- wacom_wac->shared->touch_input = wacom_wac->touch_input;
+ if (wacom_wac->shared->touch == wacom->hdev) {
+ wacom_wac->shared->type = wacom_wac->features.type;
+ rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
+ }
}
if (wacom_wac->has_mute_touch_switch) {
@@ -2360,13 +2372,6 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
if (wacom_wac->is_soft_touch_switch)
wacom_wac->shared->is_touch_on = true;
}
-
- if (wacom_wac->shared->has_mute_touch_switch &&
- wacom_wac->shared->touch_input) {
- set_bit(EV_SW, wacom_wac->shared->touch_input->evbit);
- input_set_capability(wacom_wac->shared->touch_input, EV_SW,
- SW_MUTE_DEVICE);
- }
}
static int wacom_parse_and_register(struct wacom *wacom, bool wireless)
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index da1f0ea85625..23eaa81cd827 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -510,6 +510,18 @@ static void wacom_intuos_schedule_prox_event(struct wacom_wac *wacom_wac)
}
}
+static void wacom_report_touch_mute(struct wacom_wac *wacom_wac, bool mute)
+{
+ struct input_dev *touch_input;
+
+ guard(rcu)();
+ touch_input = rcu_dereference(wacom_wac->shared->touch_input);
+ if (touch_input) {
+ input_report_switch(touch_input, SW_MUTE_DEVICE, mute);
+ input_sync(touch_input);
+ }
+}
+
static int wacom_intuos_pad(struct wacom_wac *wacom)
{
struct wacom_features *features = &wacom->features;
@@ -650,12 +662,8 @@ static int wacom_intuos_pad(struct wacom_wac *wacom)
input_report_key(input, KEY_CONTROLPANEL, menu);
input_report_key(input, KEY_INFO, info);
- if (wacom->shared && wacom->shared->touch_input) {
- input_report_switch(wacom->shared->touch_input,
- SW_MUTE_DEVICE,
- !wacom->shared->is_touch_on);
- input_sync(wacom->shared->touch_input);
- }
+ if (wacom->shared)
+ wacom_report_touch_mute(wacom, !wacom->shared->is_touch_on);
input_report_abs(input, ABS_RX, strip1);
input_report_abs(input, ABS_RY, strip2);
@@ -2153,7 +2161,7 @@ static void wacom_wac_pad_event(struct hid_device *hdev, struct hid_field *field
*/
if ((equivalent_usage == WACOM_HID_WD_MUTE_DEVICE) ||
(equivalent_usage == WACOM_HID_WD_TOUCHONOFF)) {
- if (wacom_wac->shared->touch_input) {
+ if (wacom_wac->shared) {
bool *is_touch_on = &wacom_wac->shared->is_touch_on;
if (equivalent_usage == WACOM_HID_WD_MUTE_DEVICE && value)
@@ -2161,9 +2169,7 @@ static void wacom_wac_pad_event(struct hid_device *hdev, struct hid_field *field
else if (equivalent_usage == WACOM_HID_WD_TOUCHONOFF)
*is_touch_on = value;
- input_report_switch(wacom_wac->shared->touch_input,
- SW_MUTE_DEVICE, !(*is_touch_on));
- input_sync(wacom_wac->shared->touch_input);
+ wacom_report_touch_mute(wacom_wac, !(*is_touch_on));
}
return;
}
@@ -3381,11 +3387,8 @@ static int wacom_wireless_irq(struct wacom_wac *wacom, size_t len)
if ((wacom->shared->type == INTUOSHT ||
wacom->shared->type == INTUOSHT2) &&
- wacom->shared->touch_input &&
wacom->shared->touch_max) {
- input_report_switch(wacom->shared->touch_input,
- SW_MUTE_DEVICE, data[5] & 0x40);
- input_sync(wacom->shared->touch_input);
+ wacom_report_touch_mute(wacom, data[5] & 0x40);
}
pid = get_unaligned_be16(&data[6]);
@@ -3420,11 +3423,8 @@ static int wacom_status_irq(struct wacom_wac *wacom_wac, size_t len)
if ((features->type == INTUOSHT ||
features->type == INTUOSHT2) &&
- wacom_wac->shared->touch_input &&
features->touch_max) {
- input_report_switch(wacom_wac->shared->touch_input,
- SW_MUTE_DEVICE, data[8] & 0x40);
- input_sync(wacom_wac->shared->touch_input);
+ wacom_report_touch_mute(wacom_wac, data[8] & 0x40);
}
if (data[9] & 0x02) { /* wireless module is attached */
@@ -3951,11 +3951,22 @@ int wacom_setup_pen_input_capabilities(struct input_dev *input_dev,
int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
struct wacom_wac *wacom_wac)
{
+ struct wacom *wacom = container_of(wacom_wac, struct wacom, wacom_wac);
+ struct hid_device *hdev = wacom->hdev;
struct wacom_features *features = &wacom_wac->features;
if (!(features->device_type & WACOM_DEVICETYPE_TOUCH))
return -ENODEV;
+ if (features->type != TABLETPC &&
+ features->type != TABLETPC2FG &&
+ features->type != MTSCREEN &&
+ features->type != MTTPC &&
+ features->type != MTTPC_B) {
+ input_dev->evbit[0] |= BIT_MASK(EV_SW);
+ __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
+ }
+
if (features->device_type & WACOM_DEVICETYPE_DIRECT)
__set_bit(INPUT_PROP_DIRECT, input_dev->propbit);
else
@@ -3992,22 +4003,17 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
switch (features->type) {
case INTUOSP2_BT:
case INTUOSP2S_BT:
- input_dev->evbit[0] |= BIT_MASK(EV_SW);
- __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
-
- if (wacom_wac->shared->touch->product == 0x361) {
+ if (hdev->product == 0x361) {
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
0, 12440, 4, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
0, 8640, 4, 0);
- }
- else if (wacom_wac->shared->touch->product == 0x360) {
+ } else if (hdev->product == 0x360) {
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
0, 8960, 4, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
0, 5920, 4, 0);
- }
- else if (wacom_wac->shared->touch->product == 0x393) {
+ } else if (hdev->product == 0x393) {
input_set_abs_params(input_dev, ABS_MT_POSITION_X,
0, 6400, 4, 0);
input_set_abs_params(input_dev, ABS_MT_POSITION_Y,
@@ -4037,10 +4043,8 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
fallthrough;
case WACOM_27QHDT:
- if (wacom_wac->shared->touch->product == 0x32C ||
- wacom_wac->shared->touch->product == 0xF6) {
- input_dev->evbit[0] |= BIT_MASK(EV_SW);
- __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
+ if (hdev->product == 0x32C ||
+ hdev->product == 0xF6) {
wacom_wac->has_mute_touch_switch = true;
wacom_wac->is_soft_touch_switch = true;
}
@@ -4059,8 +4063,6 @@ int wacom_setup_touch_input_capabilities(struct input_dev *input_dev,
case INTUOSHT:
case INTUOSHT2:
- input_dev->evbit[0] |= BIT_MASK(EV_SW);
- __set_bit(SW_MUTE_DEVICE, input_dev->swbit);
fallthrough;
case BAMBOO_PT:
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index 126bec6e5c0c..a8bbba4a6f37 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -285,7 +285,7 @@ struct wacom_shared {
/* for wireless device to access USB interfaces */
unsigned touch_max;
int type;
- struct input_dev *touch_input;
+ struct input_dev __rcu *touch_input;
struct hid_device *pen;
struct hid_device *touch;
bool has_mute_touch_switch;
--
2.54.0.1136.gdb2ca164c4-goog
^ permalink raw reply related
* [PATCH v4 2/3] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
From: Lee Jones @ 2026-06-16 9:26 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Peter Hutterer, Dmitry Torokhov, linux-input, linux-kernel
In-Reply-To: <20260616092658.1714548-1-lee@kernel.org>
wacom_bamboo_pad_pen_event() accesses wacom->shared->pen locklessly
relative to wacom_remove_shared_data() which nullifies it. This
can lead to a Use-After-Free if the sibling device is removed while
events are being processed.
Resolve this by introducing RCU protection for pen and touch
pointers:
- Annotate 'pen' and 'touch' in wacom_shared struct with __rcu.
- Wrap lockless readers in wacom_bamboo_pad_pen_event() with
rcu_read_lock() and rcu_dereference().
- Update writers in wacom_sys.c using rcu_assign_pointer().
- Use rcu_dereference_protected for comparisons under
wacom_udev_list_lock.
- Also use rcu_access_pointer in wacom_mode_change_work() to avoid
warnings (while lockless access there remains a pre-existing issue).
Fixes: 8c97a765467c ("HID: wacom: add full support of the Wacom Bamboo PAD")
Signed-off-by: Lee Jones <lee@kernel.org>
Reviewed-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
v1 -> v2: Split and use RCU as per Dmitry's review
v2 -> v3: Sashiko fixes
v3 -> v4: Dmitry's review [guard()]
drivers/hid/wacom_sys.c | 36 +++++++++++++++++++++++++-----------
drivers/hid/wacom_wac.c | 13 ++++++-------
drivers/hid/wacom_wac.h | 4 ++--
3 files changed, 33 insertions(+), 20 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 86895f13dbae..3990d8d0b40c 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -878,11 +878,18 @@ static void wacom_remove_shared_data(void *res)
shared);
scoped_guard(mutex, &wacom_udev_list_lock) {
- if (wacom_wac->shared->touch == wacom->hdev) {
- wacom_wac->shared->touch = NULL;
+ struct hid_device *touch =
+ rcu_dereference_protected(wacom_wac->shared->touch,
+ lockdep_is_held(&wacom_udev_list_lock));
+ struct hid_device *pen =
+ rcu_dereference_protected(wacom_wac->shared->pen,
+ lockdep_is_held(&wacom_udev_list_lock));
+
+ if (touch == wacom->hdev) {
+ rcu_assign_pointer(wacom_wac->shared->touch, NULL);
rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
- } else if (wacom_wac->shared->pen == wacom->hdev) {
- wacom_wac->shared->pen = NULL;
+ } else if (pen == wacom->hdev) {
+ rcu_assign_pointer(wacom_wac->shared->pen, NULL);
}
}
@@ -916,9 +923,9 @@ static int wacom_add_shared_data(struct hid_device *hdev)
}
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH)
- data->shared.touch = hdev;
+ rcu_assign_pointer(data->shared.touch, hdev);
else if (wacom_wac->features.device_type & WACOM_DEVICETYPE_PEN)
- data->shared.pen = hdev;
+ rcu_assign_pointer(data->shared.pen, hdev);
mutex_unlock(&wacom_udev_list_lock);
@@ -2356,7 +2363,11 @@ static void wacom_set_shared_values(struct wacom_wac *wacom_wac)
guard(mutex)(&wacom_udev_list_lock);
if (wacom_wac->features.device_type & WACOM_DEVICETYPE_TOUCH) {
- if (wacom_wac->shared->touch == wacom->hdev) {
+ struct hid_device *touch =
+ rcu_dereference_protected(wacom_wac->shared->touch,
+ lockdep_is_held(&wacom_udev_list_lock));
+
+ if (touch == wacom->hdev) {
wacom_wac->shared->type = wacom_wac->features.type;
rcu_assign_pointer(wacom_wac->shared->touch_input, wacom_wac->touch_input);
}
@@ -2795,16 +2806,19 @@ static void wacom_mode_change_work(struct work_struct *work)
bool is_direct = wacom->wacom_wac.is_direct_mode;
int error = 0;
- if (shared->pen) {
- wacom1 = hid_get_drvdata(shared->pen);
+ struct hid_device *pen = rcu_access_pointer(shared->pen);
+ struct hid_device *touch = rcu_access_pointer(shared->touch);
+
+ if (pen) {
+ wacom1 = hid_get_drvdata(pen);
wacom_release_resources(wacom1);
hid_hw_stop(wacom1->hdev);
wacom1->wacom_wac.has_mode_change = true;
wacom1->wacom_wac.is_direct_mode = is_direct;
}
- if (shared->touch) {
- wacom2 = hid_get_drvdata(shared->touch);
+ if (touch) {
+ wacom2 = hid_get_drvdata(touch);
wacom_release_resources(wacom2);
hid_hw_stop(wacom2->hdev);
wacom2->wacom_wac.has_mode_change = true;
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c
index 23eaa81cd827..92d2f2f3f552 100644
--- a/drivers/hid/wacom_wac.c
+++ b/drivers/hid/wacom_wac.c
@@ -3292,6 +3292,7 @@ static int wacom_bpt_irq(struct wacom_wac *wacom, size_t len)
static void wacom_bamboo_pad_pen_event(struct wacom_wac *wacom,
unsigned char *data)
{
+ struct hid_device *pen;
unsigned char prefix;
/*
@@ -3304,13 +3305,11 @@ static void wacom_bamboo_pad_pen_event(struct wacom_wac *wacom,
prefix = data[0];
data[0] = WACOM_REPORT_BPAD_PEN;
- /*
- * actually reroute the event.
- * No need to check if wacom->shared->pen is valid, hid_input_report()
- * will check for us.
- */
- hid_input_report(wacom->shared->pen, HID_INPUT_REPORT, data,
- WACOM_PKGLEN_PENABLED, 1);
+ guard(rcu)();
+ pen = rcu_dereference(wacom->shared->pen);
+ if (pen)
+ hid_input_report(pen, HID_INPUT_REPORT, data,
+ WACOM_PKGLEN_PENABLED, 1);
data[0] = prefix;
}
diff --git a/drivers/hid/wacom_wac.h b/drivers/hid/wacom_wac.h
index a8bbba4a6f37..170d6adbe02a 100644
--- a/drivers/hid/wacom_wac.h
+++ b/drivers/hid/wacom_wac.h
@@ -286,8 +286,8 @@ struct wacom_shared {
unsigned touch_max;
int type;
struct input_dev __rcu *touch_input;
- struct hid_device *pen;
- struct hid_device *touch;
+ struct hid_device __rcu *pen;
+ struct hid_device __rcu *touch;
bool has_mute_touch_switch;
bool is_touch_on;
};
--
2.54.0.1136.gdb2ca164c4-goog
^ permalink raw reply related
* [PATCH v4 3/3] HID: wacom: Redesign shared sibling data lifecycle
From: Lee Jones @ 2026-06-16 9:26 UTC (permalink / raw)
To: lee, Ping Cheng, Jason Gerecke, Jiri Kosina, Benjamin Tissoires,
Peter Hutterer, Dmitry Torokhov, linux-input, linux-kernel
In-Reply-To: <20260616092658.1714548-1-lee@kernel.org>
The Wacom driver coordinates state between sibling interfaces of
the same physical device using a shared structure 'wacom_shared'
inside 'wacom_hdev_data'. The driver kept a volatile representative
pointer 'data->dev' pointing to a sibling 'hid_device' for physical
path comparisons during sibling matching.
This pointer management is fragile. When the representative device
is disconnected, wacom_remove_shared_data() failed to clear/update
'data->dev', leading to a Use-After-Free vulnerability when
subsequent sibling probes dereference the dangling 'data->dev'
pointer.
Resolve this issue by redesigning the sibling data lifecycle:
- Eliminate the volatile 'data->dev' representative pointer
completely
- Redesign 'wacom_hdev_data' to store stable static copies of the
required attributes upon first allocation: 'phys' path string,
'vendor', 'product' IDs and the sibling's 'device_type'
- Use these static attributes for stable sibling matching in
wacom_are_sibling() and wacom_get_hdev_data()
This ensures sibling matching remains safe and stable even if
individual siblings are dynamically added or removed.
To secure the lifecycle against concurrent probe/disconnect races:
- Switch kref_put() to kref_put_mutex() in
wacom_remove_shared_data() to serialize refcount drops with list
traversal and lookup
- Modify wacom_release_shared_data() to assume the list lock is
already held
Also, do not accumulate the 'device_type' capability flag during
subsequent sibling probes. Keeping only the first probed sibling's
device_type exactly preserves the original sibling matching behavior
without introducing side effects.
Fixes: 4492efffffeb ("Input: wacom - share pen info with touch of the same ID")
Signed-off-by: Lee Jones <lee@kernel.org>
---
v1 -> v2: Split and use RCU as per Dmitry's review
v2 -> v3: Sashiko fixes
v3 -> v4: No change
drivers/hid/wacom_sys.c | 70 +++++++++++++++++++++++++++--------------
1 file changed, 46 insertions(+), 24 deletions(-)
diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
index 3990d8d0b40c..9fb9dde7dd73 100644
--- a/drivers/hid/wacom_sys.c
+++ b/drivers/hid/wacom_sys.c
@@ -759,27 +759,47 @@ static void wacom_retrieve_hid_descriptor(struct hid_device *hdev,
struct wacom_hdev_data {
struct list_head list;
struct kref kref;
- struct hid_device *dev;
+ char phys[64];
+ __u32 vendor;
+ __u32 product;
+ __u32 device_type;
struct wacom_shared shared;
};
+static bool wacom_compare_device_paths(struct hid_device *hdev_a,
+ const char *phys_b, char separator)
+{
+ const char *p1 = strrchr(hdev_a->phys, separator);
+ const char *p2 = strrchr(phys_b, separator);
+ int n1, n2;
+
+ if (!p1 || !p2)
+ return false;
+
+ n1 = p1 - hdev_a->phys;
+ n2 = p2 - phys_b;
+
+ if (n1 != n2 || n1 <= 0 || n2 <= 0)
+ return false;
+
+ return !strncmp(hdev_a->phys, phys_b, n1);
+}
+
static LIST_HEAD(wacom_udev_list);
static DEFINE_MUTEX(wacom_udev_list_lock);
static bool wacom_are_sibling(struct hid_device *hdev,
- struct hid_device *sibling)
+ struct wacom_hdev_data *data)
{
struct wacom *wacom = hid_get_drvdata(hdev);
struct wacom_features *features = &wacom->wacom_wac.features;
- struct wacom *sibling_wacom = hid_get_drvdata(sibling);
- struct wacom_features *sibling_features = &sibling_wacom->wacom_wac.features;
__u32 oVid = features->oVid ? features->oVid : hdev->vendor;
__u32 oPid = features->oPid ? features->oPid : hdev->product;
/* The defined oVid/oPid must match that of the sibling */
- if (features->oVid != HID_ANY_ID && sibling->vendor != oVid)
+ if (features->oVid != HID_ANY_ID && data->vendor != oVid)
return false;
- if (features->oPid != HID_ANY_ID && sibling->product != oPid)
+ if (features->oPid != HID_ANY_ID && data->product != oPid)
return false;
/*
@@ -787,11 +807,11 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* device path, while those with different VID/PID must share
* the same physical parent device path.
*/
- if (hdev->vendor == sibling->vendor && hdev->product == sibling->product) {
- if (!hid_compare_device_paths(hdev, sibling, '/'))
+ if (hdev->vendor == data->vendor && hdev->product == data->product) {
+ if (!wacom_compare_device_paths(hdev, data->phys, '/'))
return false;
} else {
- if (!hid_compare_device_paths(hdev, sibling, '.'))
+ if (!wacom_compare_device_paths(hdev, data->phys, '.'))
return false;
}
@@ -804,7 +824,7 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* devices.
*/
if ((features->device_type & WACOM_DEVICETYPE_DIRECT) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
+ !(data->device_type & WACOM_DEVICETYPE_DIRECT))
return false;
/*
@@ -812,17 +832,17 @@ static bool wacom_are_sibling(struct hid_device *hdev,
* devices.
*/
if (!(features->device_type & WACOM_DEVICETYPE_DIRECT) &&
- (sibling_features->device_type & WACOM_DEVICETYPE_DIRECT))
+ (data->device_type & WACOM_DEVICETYPE_DIRECT))
return false;
/* Pen devices may only be siblings of touch devices */
if ((features->device_type & WACOM_DEVICETYPE_PEN) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_TOUCH))
+ !(data->device_type & WACOM_DEVICETYPE_TOUCH))
return false;
/* Touch devices may only be siblings of pen devices */
if ((features->device_type & WACOM_DEVICETYPE_TOUCH) &&
- !(sibling_features->device_type & WACOM_DEVICETYPE_PEN))
+ !(data->device_type & WACOM_DEVICETYPE_PEN))
return false;
/*
@@ -838,7 +858,7 @@ static struct wacom_hdev_data *wacom_get_hdev_data(struct hid_device *hdev)
/* Try to find an already-probed interface from the same device */
list_for_each_entry(data, &wacom_udev_list, list) {
- if (hid_compare_device_paths(hdev, data->dev, '/')) {
+ if (wacom_compare_device_paths(hdev, data->phys, '/')) {
kref_get(&data->kref);
return data;
}
@@ -846,7 +866,7 @@ static struct wacom_hdev_data *wacom_get_hdev_data(struct hid_device *hdev)
/* Fallback to finding devices that appear to be "siblings" */
list_for_each_entry(data, &wacom_udev_list, list) {
- if (wacom_are_sibling(hdev, data->dev)) {
+ if (wacom_are_sibling(hdev, data)) {
kref_get(&data->kref);
return data;
}
@@ -860,18 +880,15 @@ static void wacom_release_shared_data(struct kref *kref)
struct wacom_hdev_data *data =
container_of(kref, struct wacom_hdev_data, kref);
- mutex_lock(&wacom_udev_list_lock);
list_del(&data->list);
- mutex_unlock(&wacom_udev_list_lock);
-
kfree(data);
}
static void wacom_remove_shared_data(void *res)
{
- struct wacom *wacom = res;
+ struct wacom *res_wacom = res;
struct wacom_hdev_data *data;
- struct wacom_wac *wacom_wac = &wacom->wacom_wac;
+ struct wacom_wac *wacom_wac = &res_wacom->wacom_wac;
if (wacom_wac->shared) {
data = container_of(wacom_wac->shared, struct wacom_hdev_data,
@@ -885,17 +902,19 @@ static void wacom_remove_shared_data(void *res)
rcu_dereference_protected(wacom_wac->shared->pen,
lockdep_is_held(&wacom_udev_list_lock));
- if (touch == wacom->hdev) {
+ if (touch == res_wacom->hdev) {
rcu_assign_pointer(wacom_wac->shared->touch, NULL);
rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
- } else if (pen == wacom->hdev) {
+ } else if (pen == res_wacom->hdev) {
rcu_assign_pointer(wacom_wac->shared->pen, NULL);
}
}
synchronize_rcu();
- kref_put(&data->kref, wacom_release_shared_data);
+ if (kref_put_mutex(&data->kref, wacom_release_shared_data, &wacom_udev_list_lock))
+ mutex_unlock(&wacom_udev_list_lock);
+
wacom_wac->shared = NULL;
}
}
@@ -918,7 +937,10 @@ static int wacom_add_shared_data(struct hid_device *hdev)
}
kref_init(&data->kref);
- data->dev = hdev;
+ strscpy(data->phys, hdev->phys, sizeof(data->phys));
+ data->vendor = hdev->vendor;
+ data->product = hdev->product;
+ data->device_type = wacom_wac->features.device_type;
list_add_tail(&data->list, &wacom_udev_list);
}
--
2.54.0.1136.gdb2ca164c4-goog
^ permalink raw reply related
* Re: [PATCH v4 2/3] HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
From: sashiko-bot @ 2026-06-16 9:43 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260616092658.1714548-2-lee@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Lockless access to `shared->pen` and `shared->touch` in `wacom_mode_change_work()` can lead to Use-After-Free.
--
commit e951961530e94aa8b795598b1d198c32fdbac428
Author: Lee Jones <lee@kernel.org>
HID: wacom: Fix Use-After-Free in wacom_bamboo_pad
Resolves a use-after-free when events are processed while a sibling device
is removed by introducing RCU protection for the shared pen and touch
pointers.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 86895f13dbaea..3990d8d0b40c0 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
[ ... ]
> @@ -2795,16 +2806,19 @@ static void wacom_mode_change_work(struct work_struct *work)
> bool is_direct = wacom->wacom_wac.is_direct_mode;
> int error = 0;
>
> - if (shared->pen) {
> - wacom1 = hid_get_drvdata(shared->pen);
> + struct hid_device *pen = rcu_access_pointer(shared->pen);
> + struct hid_device *touch = rcu_access_pointer(shared->touch);
> +
> + if (pen) {
> + wacom1 = hid_get_drvdata(pen);
> wacom_release_resources(wacom1);
> hid_hw_stop(wacom1->hdev);
> wacom1->wacom_wac.has_mode_change = true;
> wacom1->wacom_wac.is_direct_mode = is_direct;
> }
[Severity: High]
This is a pre-existing issue, but can this lockless access to the sibling
device pointers lead to a use-after-free if the sibling is concurrently
disconnected?
In wacom_mode_change_work(), rcu_access_pointer() reads the pointer outside
of an RCU read-side critical section. Since the pointer is then dereferenced
to fetch wacom1 via hid_get_drvdata(), what happens if wacom_remove_shared_data()
runs concurrently on the sibling?
The removal path would clear the shared pointer and call synchronize_rcu().
Because wacom_mode_change_work() is not holding the RCU read lock,
synchronize_rcu() will not wait for the work item to complete. This could
allow wacom1's driver data to be freed while wacom_mode_change_work() is
still using it to call wacom_release_resources(wacom1) and hid_hw_stop().
Since wacom_release_resources() and hid_hw_stop() can sleep, simply wrapping
this block in rcu_read_lock() wouldn't be sufficient. Is there a plan to
use reference counting or SRCU here to safely access the sibling device?
> - if (shared->touch) {
> - wacom2 = hid_get_drvdata(shared->touch);
> + if (touch) {
> + wacom2 = hid_get_drvdata(touch);
> wacom_release_resources(wacom2);
> hid_hw_stop(wacom2->hdev);
> wacom2->wacom_wac.has_mode_change = true;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616092658.1714548-1-lee@kernel.org?part=2
^ permalink raw reply
* Re: [PATCH v4 1/3] HID: wacom: Fix Use-After-Free in wacom_intuos_pad
From: sashiko-bot @ 2026-06-16 9:45 UTC (permalink / raw)
To: Lee Jones; +Cc: linux-input, dmitry.torokhov
In-Reply-To: <20260616092658.1714548-1-lee@kernel.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free in `wacom_mode_change_work()` due to unprotected lockless access of sibling device pointers.
--
commit 2e75e17f7579b493b3120751d2b8cfa23bc2c814
Author: Lee Jones <lee@kernel.org>
HID: wacom: Fix Use-After-Free in wacom_intuos_pad
This commit resolves a Time-of-Check to Time-of-Use race condition by
introducing RCU protection for the touch_input pointer. It also wraps
shared value assignments in the udev list lock to prevent concurrent
desynchronization, and advertises the SW_MUTE_DEVICE capability before
registration.
> diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c
> index 2220168bf1164..86895f13dbaea 100644
> --- a/drivers/hid/wacom_sys.c
> +++ b/drivers/hid/wacom_sys.c
> @@ -877,10 +877,16 @@ static void wacom_remove_shared_data(void *res)
> data = container_of(wacom_wac->shared, struct wacom_hdev_data,
> shared);
>
> - if (wacom_wac->shared->touch == wacom->hdev)
> - wacom_wac->shared->touch = NULL;
> - else if (wacom_wac->shared->pen == wacom->hdev)
> - wacom_wac->shared->pen = NULL;
> + scoped_guard(mutex, &wacom_udev_list_lock) {
> + if (wacom_wac->shared->touch == wacom->hdev) {
> + wacom_wac->shared->touch = NULL;
> + rcu_assign_pointer(wacom_wac->shared->touch_input, NULL);
> + } else if (wacom_wac->shared->pen == wacom->hdev) {
> + wacom_wac->shared->pen = NULL;
> + }
> + }
> +
> + synchronize_rcu();
[Severity: High]
This is a pre-existing issue, but I noticed a potential Use-After-Free in
wacom_mode_change_work() due to unprotected lockless access of sibling device
pointers.
While synchronize_rcu() is called here to ensure active RCU readers have
finished, wacom_mode_change_work() accesses sibling pointers locklessly
without an rcu_read_lock():
wacom_mode_change_work() {
...
struct hid_device *pen = rcu_access_pointer(shared->pen);
struct hid_device *touch = rcu_access_pointer(shared->touch);
...
if (pen) {
wacom1 = hid_get_drvdata(pen);
...
}
Because there is no rcu_read_lock() wrapping the access in
wacom_mode_change_work(), this synchronize_rcu() call will return without
waiting for the preempted work to finish.
If a sibling device (like the pen device) is concurrently disconnected,
wacom_remove() cancels the pen device's mode change work but leaves the
touch device's work running.
When wacom_remove_shared_data() executes for the pen, it nullifies
shared->pen and calls synchronize_rcu(). Since it won't wait for the
unprotected touch device's work, the pen device memory could be freed by
the HID core before that work resumes.
When the work eventually resumes and calls hid_get_drvdata(pen), won't it
dereference the freed pen pointer and cause a Use-After-Free?
I see from the patch 2 commit message that this lockless access is
intentionally left as-is because wacom_mode_change_work() sleeps and cannot
use RCU read locks. Could an alternative approach be used here to prevent
this use-after-free?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616092658.1714548-1-lee@kernel.org?part=1
^ permalink raw reply
* [PATCH 0/4] input: misc: Add an initial driver for haptics inside Qcom PMIH010x PMIC
From: Fenglin Wu @ 2026-06-16 10:08 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 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:
-- 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>
---
Fenglin Wu (4):
dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics
dt-bindings: mfd: qcom,spmi-pmic: Document haptics device
input: misc: Add Qualcomm SPMI PMIC haptics driver
arm64: dts: qcom: Add PMIH0108 haptics device node
.../bindings/input/qcom,spmi-haptics.yaml | 119 +++
.../devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 4 +
arch/arm64/boot/dts/qcom/kaanapali-mtp.dts | 7 +
arch/arm64/boot/dts/qcom/kaanapali-qrd.dts | 7 +
arch/arm64/boot/dts/qcom/pmih0108-kaanapali.dtsi | 9 +
arch/arm64/boot/dts/qcom/pmih0108.dtsi | 9 +
arch/arm64/boot/dts/qcom/sm8750-mtp.dts | 7 +
arch/arm64/boot/dts/qcom/sm8750-qrd.dts | 7 +
drivers/input/misc/Kconfig | 11 +
drivers/input/misc/Makefile | 1 +
drivers/input/misc/qcom-spmi-haptics.c | 831 +++++++++++++++++++++
11 files changed, 1012 insertions(+)
---
base-commit: 66725039f7090afe14c31bd259e2059a68f04023
change-id: 20260616-qcom-spmi-haptics-3cc97e7b232e
Best regards,
--
Fenglin Wu <fenglin.wu@oss.qualcomm.com>
^ permalink raw reply
* [PATCH 1/4] dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics
From: Fenglin Wu @ 2026-06-16 10:08 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: <20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com>
Add binding document for the haptics module inside Qualcomm PMIH010X.
Assisted-by: Claude:claude-4-6-sonnet
Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
.../bindings/input/qcom,spmi-haptics.yaml | 119 +++++++++++++++++++++
1 file changed, 119 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..0e26d68563dc
--- /dev/null
+++ b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
@@ -0,0 +1,119 @@
+# 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. PMIH010X
+
+maintainers:
+ - Fenglin Wu <fenglin.wu@oss.qualcomm.com>
+
+description: |
+ The Qualcomm PMIH010X PMIC integrates a haptics module (HAP530_HV) that
+ drives an LRA (Linear Resonant Actuator) with output voltage up to 10 V.
+ Two play modes are supported:
+
+ DIRECT_PLAY: The hardware outputs sinusoidal waveforms whose period is
+ defined by lra-period-us and whose peak voltage is defined by vmax-mv.
+ 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 streaming: 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. The following play rate values are accepted:
+ -- 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.
+
+ In FIFO streaming mode, hardware-based LRA auto-resonance tracking is
+ disabled by default. Because this mode is intended to drive arbitrary
+ waveforms that may not follow the resonant frequency, autonomous hardware
+ resonance correction would interfere with the intended output.
+
+ In the driver, FIFO streaming is implemented using an FF_PERIODIC effect
+ with an FF_CUSTOM waveform. The expected custom data layout is:
+ custom_data[0] = play rate code (see qcom,wf-play-rate values below)
+ custom_data[1] = vmax in mV; 0 = use device default (qcom,vmax-mv)
+ custom_data[2..] = signed 8-bit PCM samples (at least one required)
+
+properties:
+ compatible:
+ const: qcom,pmih010x-haptics
+
+ reg:
+ items:
+ - description: HAP_CFG module base address
+ - description: HAP_PTN module base address
+
+ reg-names:
+ items:
+ - const: hap-cfg
+ - const: hap-ptn
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-names:
+ items:
+ - const: fifo-empty
+
+ qcom,vmax-mv:
+ description:
+ Maximum allowed output driving voltage in millivolts, rounded to the
+ nearest 50 mV 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: 50
+ maximum: 10000
+
+ 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.
+ minimum: 5
+ maximum: 20475
+
+required:
+ - compatible
+ - reg
+ - reg-names
+ - interrupts
+ - interrupt-names
+ - qcom,vmax-mv
+ - 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,pmih010x-haptics";
+ reg = <0xf000>, <0xf100>;
+ reg-names = "hap-cfg", "hap-ptn";
+ interrupts = <0x7 0xf0 0x1 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "fifo-empty";
+
+ qcom,vmax-mv = <1300>;
+ qcom,lra-period-us = <5880>;
+ };
+ };
--
2.43.0
^ permalink raw reply related
* [PATCH 2/4] dt-bindings: mfd: qcom,spmi-pmic: Document haptics device
From: Fenglin Wu @ 2026-06-16 10:08 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: <20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com>
Some of the Qualcomm SPMI PMIC has haptics device in it, add it in the
device list.
Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
index 644c42b5e2e5..773f4cba5935 100644
--- a/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
+++ b/Documentation/devicetree/bindings/mfd/qcom,spmi-pmic.yaml
@@ -165,6 +165,10 @@ patternProperties:
type: object
$ref: /schemas/pinctrl/qcom,pmic-gpio.yaml#
+ "^haptics@[0-9a-f]+$":
+ type: object
+ $ref: /schemas/input/qcom,spmi-haptics.yaml#
+
"^led-controller@[0-9a-f]+$":
type: object
$ref: /schemas/leds/qcom,spmi-flash-led.yaml#
--
2.43.0
^ permalink raw reply related
* [PATCH 4/4] arm64: dts: qcom: Add PMIH0108 haptics device node
From: Fenglin Wu @ 2026-06-16 10:08 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: <20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com>
Add haptics device node in the PMIH0108 PMIC base dtsi files, and enable
it on several boards according to the LRA (Linear Resonant Actuator)
component mounted on each of them.
Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/kaanapali-mtp.dts | 7 +++++++
arch/arm64/boot/dts/qcom/kaanapali-qrd.dts | 7 +++++++
arch/arm64/boot/dts/qcom/pmih0108-kaanapali.dtsi | 9 +++++++++
arch/arm64/boot/dts/qcom/pmih0108.dtsi | 9 +++++++++
arch/arm64/boot/dts/qcom/sm8750-mtp.dts | 7 +++++++
arch/arm64/boot/dts/qcom/sm8750-qrd.dts | 7 +++++++
6 files changed, 46 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts b/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts
index 07247dc98b70..7e3f59fc008e 100644
--- a/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/kaanapali-mtp.dts
@@ -952,6 +952,13 @@ wifi@0 {
};
};
+&pmih0108_e1_haptics {
+ status = "okay";
+
+ qcom,lra-period-us = <6667>;
+ qcom,vmax-mv = <3600>;
+};
+
&pmh0101_flash {
status = "okay";
diff --git a/arch/arm64/boot/dts/qcom/kaanapali-qrd.dts b/arch/arm64/boot/dts/qcom/kaanapali-qrd.dts
index da0e8f9091c3..0865afec47ac 100644
--- a/arch/arm64/boot/dts/qcom/kaanapali-qrd.dts
+++ b/arch/arm64/boot/dts/qcom/kaanapali-qrd.dts
@@ -744,6 +744,13 @@ led@3 {
};
};
+&pmih0108_e1_haptics {
+ status = "okay";
+
+ qcom,lra-period-us = <7692>;
+ qcom,vmax-mv = <1850>;
+};
+
&pon_resin {
linux,code = <KEY_VOLUMEDOWN>;
diff --git a/arch/arm64/boot/dts/qcom/pmih0108-kaanapali.dtsi b/arch/arm64/boot/dts/qcom/pmih0108-kaanapali.dtsi
index b73b0e82c3d3..22c83c549ce9 100644
--- a/arch/arm64/boot/dts/qcom/pmih0108-kaanapali.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmih0108-kaanapali.dtsi
@@ -59,6 +59,15 @@ pmih0108_e1_gpios: gpio@8800 {
#interrupt-cells = <2>;
};
+ pmih0108_e1_haptics: haptics@f000 {
+ compatible = "qcom,pmih010x-haptics";
+ reg = <0xf000>, <0xf100>;
+ reg-names = "hap-cfg", "hap-ptn";
+ interrupts = <0x7 0xf0 0x1 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "fifo-empty";
+ status = "disabled";
+ };
+
pmih0108_e1_eusb2_repeater: phy@fd00 {
compatible = "qcom,pm8550b-eusb2-repeater";
reg = <0xfd00>;
diff --git a/arch/arm64/boot/dts/qcom/pmih0108.dtsi b/arch/arm64/boot/dts/qcom/pmih0108.dtsi
index 1c875995d881..d942d6c2fd03 100644
--- a/arch/arm64/boot/dts/qcom/pmih0108.dtsi
+++ b/arch/arm64/boot/dts/qcom/pmih0108.dtsi
@@ -59,6 +59,15 @@ pmih0108_gpios: gpio@8800 {
#interrupt-cells = <2>;
};
+ pmih0108_haptics: haptics@f000 {
+ compatible = "qcom,pmih010x-haptics";
+ reg = <0xf000>, <0xf100>;
+ reg-names = "hap-cfg", "hap-ptn";
+ interrupts = <0x7 0xf0 0x1 IRQ_TYPE_EDGE_RISING>;
+ interrupt-names = "fifo-empty";
+ status = "disabled";
+ };
+
pmih0108_eusb2_repeater: phy@fd00 {
compatible = "qcom,pm8550b-eusb2-repeater";
reg = <0xfd00>;
diff --git a/arch/arm64/boot/dts/qcom/sm8750-mtp.dts b/arch/arm64/boot/dts/qcom/sm8750-mtp.dts
index 3837f6785320..7a3b8c440d00 100644
--- a/arch/arm64/boot/dts/qcom/sm8750-mtp.dts
+++ b/arch/arm64/boot/dts/qcom/sm8750-mtp.dts
@@ -1138,6 +1138,13 @@ wifi@0 {
};
};
+&pmih0108_haptics {
+ status = "okay";
+
+ qcom,lra-period-us = <6667>;
+ qcom,vmax-mv = <3600>;
+};
+
&pmih0108_eusb2_repeater {
qcom,tune-usb2-preem = /bits/ 8 <0x3>;
qcom,tune-usb2-amplitude = /bits/ 8 <0xa>;
diff --git a/arch/arm64/boot/dts/qcom/sm8750-qrd.dts b/arch/arm64/boot/dts/qcom/sm8750-qrd.dts
index 801c46d55602..4a1079984307 100644
--- a/arch/arm64/boot/dts/qcom/sm8750-qrd.dts
+++ b/arch/arm64/boot/dts/qcom/sm8750-qrd.dts
@@ -933,6 +933,13 @@ &pon_resin {
status = "okay";
};
+&pmih0108_haptics {
+ status = "okay";
+
+ qcom,lra-period-us = <5880>;
+ qcom,vmax-mv = <1700>;
+};
+
&pmih0108_eusb2_repeater {
status = "okay";
--
2.43.0
^ permalink raw reply related
* [PATCH 3/4] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: Fenglin Wu @ 2026-06-16 10:08 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: <20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com>
Add an initial driver for the Qualcomm PMIH010x 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 | 831 +++++++++++++++++++++++++++++++++
3 files changed, 843 insertions(+)
diff --git a/drivers/input/misc/Kconfig b/drivers/input/misc/Kconfig
index 1f6c57dba030..eac939978ce4 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 INPUT && MFD_SPMI_PMIC
+ help
+ Say Y to enable support for the Qualcomm PMIH010X 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..75b3e338b54e
--- /dev/null
+++ b/drivers/input/misc/qcom-spmi-haptics.c
@@ -0,0 +1,831 @@
+// 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/spinlock.h>
+#include <linux/uaccess.h>
+#include <linux/workqueue.h>
+
+/* HAP_CFG register offsets, bit fields, value constants */
+#define HAP_CFG_INT_RT_STS_REG 0x10
+#define FIFO_EMPTY_BIT BIT(1)
+#define HAP_CFG_EN_CTL_REG 0x46
+#define HAPTICS_EN_BIT BIT(7)
+#define HAP_CFG_VMAX_REG 0x48
+#define VMAX_STEP_MV 50
+#define VMAX_MV_MAX 10000
+#define HAP_CFG_SPMI_PLAY_REG 0x4C
+#define PLAY_EN_BIT BIT(7)
+#define PAT_SRC_MASK GENMASK(2, 0)
+#define PAT_SRC_FIFO 0
+#define PAT_SRC_DIRECT_PLAY 1
+#define HAP_CFG_TLRA_OL_HIGH_REG 0x5C
+#define TLRA_OL_MSB_MASK GENMASK(3, 0)
+#define TLRA_STEP_US 5
+#define 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-mv)
+ * [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: spinlock 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;
+
+ spinlock_t 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 i, bulk_len = ALIGN_DOWN(len, 4);
+ int ret;
+
+ 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 = (u32)((u64)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_nosync(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)
+{
+ unsigned long flags;
+
+ cfg_write(h, HAP_CFG_SPMI_PLAY_REG, 0);
+
+ if (h->effects[h->cur_effect_id].mode == HAPTICS_FIFO) {
+ ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
+ haptics_fifo_irq_enable(h, false);
+ spin_lock_irqsave(&h->fifo_lock, flags);
+ h->fifo_data = NULL;
+ spin_unlock_irqrestore(&h->fifo_lock, flags);
+ }
+}
+
+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)((u32)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;
+ unsigned long flags;
+ 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;
+
+ spin_lock_irqsave(&h->fifo_lock, flags);
+ 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);
+ spin_unlock_irqrestore(&h->fifo_lock, flags);
+
+ /*
+ * 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;
+ unsigned long flags;
+ 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;
+
+ spin_lock_irqsave(&h->fifo_lock, flags);
+
+ if (!h->fifo_data) {
+ spin_unlock_irqrestore(&h->fifo_lock, flags);
+ 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);
+ spin_unlock_irqrestore(&h->fifo_lock, flags);
+ 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);
+ }
+
+ spin_unlock_irqrestore(&h->fifo_lock, flags);
+ 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_mark_last_busy(h->dev);
+ pm_runtime_put_autosuspend(h->dev);
+ h->pm_ref_held = false;
+ }
+ goto unlock;
+ }
+
+ 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);
+ 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;
+ s16 *buf;
+ u32 i;
+
+ 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;
+ }
+
+ 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->data_len = effect->u.periodic.custom_len - CUSTOM_DATA_SAMPLE_START;
+
+ kfree(priv->fifo_data);
+ priv->fifo_data = kmalloc(priv->data_len, GFP_KERNEL);
+ if (!priv->fifo_data) {
+ kfree(buf);
+ return -ENOMEM;
+ }
+
+ /* Pack: one s8 sample per s16 slot (lower byte) */
+ for (i = 0; i < priv->data_len; i++)
+ priv->fifo_data[i] = (s8)buf[CUSTOM_DATA_SAMPLE_START + i];
+
+ kfree(buf);
+ priv->mode = HAPTICS_FIFO;
+ return 0;
+
+ default:
+ return -EINVAL;
+ }
+}
+
+static int haptics_playback(struct input_dev *dev, int effect_id, int val)
+{
+ struct qcom_haptics *h = input_get_drvdata(dev);
+
+ 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];
+ 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(&pdev->dev, -ENODEV,
+ "no regmap from parent\n");
+
+ ret = device_property_read_u32_array(&pdev->dev, "reg", regs,
+ ARRAY_SIZE(regs));
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to read 'reg' property\n");
+
+ h->cfg_base = regs[0];
+ h->ptn_base = regs[1];
+
+ ret = of_property_read_u32(h->dev->of_node, "qcom,lra-period-us",
+ &h->t_lra_us);
+ if (ret)
+ return dev_err_probe(h->dev, ret, "missing qcom,lra-period-us\n");
+
+ ret = of_property_read_u32(h->dev->of_node, "qcom,vmax-mv", &h->vmax_mv);
+ if (ret)
+ return dev_err_probe(h->dev, ret, "missing qcom,vmax-mv\n");
+
+ h->vmax_mv = clamp(h->vmax_mv, (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);
+ spin_lock_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(&pdev->dev, irq,
+ "failed to get fifo-empty IRQ\n");
+
+ ret = devm_request_threaded_irq(&pdev->dev, irq, NULL,
+ haptics_fifo_empty_irq,
+ IRQF_ONESHOT,
+ "qcom-haptics-fifo-empty", h);
+ if (ret)
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to request fifo-empty IRQ\n");
+
+ h->fifo_empty_irq = irq;
+ disable_irq_nosync(irq);
+
+ input = devm_input_allocate_device(&pdev->dev);
+ if (!input)
+ return -ENOMEM;
+
+ input->name = "qcom-spmi-haptics";
+ input_set_drvdata(input, h);
+ h->input = input;
+
+ input_set_capability(input, EV_FF, FF_CONSTANT);
+ input_set_capability(input, EV_FF, FF_PERIODIC);
+ input_set_capability(input, EV_FF, FF_CUSTOM);
+ input_set_capability(input, EV_FF, FF_GAIN);
+
+ ret = input_ff_create(input, HAPTICS_MAX_EFFECTS);
+ if (ret)
+ return ret;
+
+ ff = input->ff;
+ ff->upload = haptics_upload_effect;
+ ff->playback = haptics_playback;
+ ff->erase = haptics_erase;
+ ff->set_gain = haptics_set_gain;
+
+ ret = input_register_device(input);
+ if (ret) {
+ input_ff_destroy(input);
+ return dev_err_probe(&pdev->dev, ret,
+ "failed to register input device\n");
+ }
+
+ platform_set_drvdata(pdev, h);
+
+ /*
+ * Grab a reference on behalf of probe (usage_count → 1), mark the
+ * device active, then enable runtime PM.
+ */
+ pm_runtime_get_noresume(&pdev->dev);
+ pm_runtime_use_autosuspend(&pdev->dev);
+ pm_runtime_set_autosuspend_delay(&pdev->dev, HAPTICS_AUTOSUSPEND_MS);
+ devm_pm_runtime_set_active_enabled(&pdev->dev);
+ pm_runtime_put_autosuspend(&pdev->dev);
+
+ return 0;
+}
+
+static void qcom_haptics_remove(struct platform_device *pdev)
+{
+ struct qcom_haptics *h = platform_get_drvdata(pdev);
+
+ pm_runtime_disable(&pdev->dev);
+ pm_runtime_set_suspended(&pdev->dev);
+
+ cancel_work_sync(&h->play_work);
+ mutex_lock(&h->play_lock);
+ haptics_stop_locked(h);
+ haptics_enable_module(h, false);
+ mutex_unlock(&h->play_lock);
+
+ 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);
+
+ cancel_work_sync(&h->play_work);
+ 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);
+ 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,pmih010x-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
* Re: [PATCH 1/4] dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics
From: Konrad Dybcio @ 2026-06-16 10:11 UTC (permalink / raw)
To: Fenglin Wu, linux-arm-msm, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Stephen Boyd,
Bjorn Andersson, Konrad Dybcio
Cc: David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260616-qcom-spmi-haptics-v1-1-d24e422de6b4@oss.qualcomm.com>
On 6/16/26 12:08 PM, Fenglin Wu wrote:
> Add binding document for the haptics module inside Qualcomm PMIH010X.
>
> Assisted-by: Claude:claude-4-6-sonnet
> Signed-off-by: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
> ---
[...]
> + qcom,vmax-mv:
> + description:
> + Maximum allowed output driving voltage in millivolts, rounded to the
> + nearest 50 mV 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: 50
> + maximum: 10000
multipleOf: 50?
Konrad
^ permalink raw reply
* Re: [PATCH 2/4] dt-bindings: mfd: qcom,spmi-pmic: Document haptics device
From: sashiko-bot @ 2026-06-16 10:17 UTC (permalink / raw)
To: Fenglin Wu; +Cc: conor+dt, robh, linux-input, dmitry.torokhov, devicetree
In-Reply-To: <20260616-qcom-spmi-haptics-v1-2-d24e422de6b4@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Use of a wildcard ('x') in the compatible string.
--
commit 24bf9f2257f4052d27a53163621c8f5173fe1da8
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics
Add binding document for the haptics module inside Qualcomm PMIH010X.
> diff --git a/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
> new file mode 100644
> index 0000000000000..0e26d68563dcd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
> @@ -0,0 +1,119 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
[ ... ]
> +properties:
> + compatible:
> + const: qcom,pmih010x-haptics
[Severity: Low]
Does this introduce a regression by using a wildcard character ('x') in the
compatible string?
Device tree specifications generally forbid wildcards in compatible strings.
Should the specific model name of the first chip in the family
(e.g., "qcom,pmih0108-haptics") be used instead of a wildcard?
[ ... ]
> +examples:
> + - |
> + #include <dt-bindings/interrupt-controller/irq.h>
> +
> + pmic {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + haptics@f000 {
> + compatible = "qcom,pmih010x-haptics";
[Severity: Low]
Does this example share the same regression with the wildcard compatible
string?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com?part=2
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics
From: sashiko-bot @ 2026-06-16 10:18 UTC (permalink / raw)
To: Fenglin Wu; +Cc: conor+dt, robh, dmitry.torokhov, linux-input, devicetree
In-Reply-To: <20260616-qcom-spmi-haptics-v1-1-d24e422de6b4@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The device tree binding description contains Linux-specific driver implementation details, violating the OS-agnostic principle.
- [Low] Stale reference to an undefined property 'qcom,wf-play-rate' in the binding description.
--
commit 24bf9f2257f4052d27a53163621c8f5173fe1da8
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
dt-bindings: input: Add binding for Qualcomm SPMI PMIC haptics
Add binding document for the haptics module inside Qualcomm PMIH010X.
> diff --git a/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
> new file mode 100644
> index 0000000000000..0e26d68563dcd
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/qcom,spmi-haptics.yaml
[ ... ]
> + In FIFO streaming mode, hardware-based LRA auto-resonance tracking is
> + disabled by default. Because this mode is intended to drive arbitrary
> + waveforms that may not follow the resonant frequency, autonomous hardware
> + resonance correction would interfere with the intended output.
> +
> + In the driver, FIFO streaming is implemented using an FF_PERIODIC effect
> + with an FF_CUSTOM waveform. The expected custom data layout is:
[Severity: Low]
Are these Linux-specific driver implementation details, such as FF_PERIODIC
and FF_CUSTOM, appropriate for this binding document?
Device tree bindings are typically expected to be strictly OS-agnostic
hardware descriptions. Should the documentation for the driver's ioctl payload
and custom_data memory layout be moved to the Linux driver documentation
instead?
> + custom_data[0] = play rate code (see qcom,wf-play-rate values below)
[Severity: Low]
Is there a missing property definition for qcom,wf-play-rate?
This line references qcom,wf-play-rate values, but the property doesn't appear
to be defined anywhere in the schema's properties section. Does this need to
be added, or is this a stale reference that should be removed?
> + custom_data[1] = vmax in mV; 0 = use device default (qcom,vmax-mv)
> + custom_data[2..] = signed 8-bit PCM samples (at least one required)
> +
> +properties:
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com?part=1
^ permalink raw reply
* Re: [PATCH 3/4] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: sashiko-bot @ 2026-06-16 10:23 UTC (permalink / raw)
To: Fenglin Wu; +Cc: robh, linux-input, conor+dt, devicetree, dmitry.torokhov
In-Reply-To: <20260616-qcom-spmi-haptics-v1-3-d24e422de6b4@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [High] Repeated playback requests for an actively playing effect leak runtime PM usage counters, permanently preventing the device from suspending.
- [High] The threaded IRQ handler sleeps in atomic context by executing sleepable regmap operations while holding a spinlock.
- [High] Incorrect teardown ordering in `remove()` and `suspend()` allows a work item to be re-queued and executed on freed memory (Use-After-Free).
- [High] Erasing or updating a force feedback effect frees the effect's data buffer synchronously, causing a Use-After-Free in the IRQ handler.
- [High] A failed memory allocation during effect upload leaves internal state corrupted, guaranteeing a NULL pointer dereference upon playback.
- [High] A lockless race condition between the input playback callback and the FIFO empty IRQ handler can cause new playback requests to be silently dropped.
- [High] A data race on `h->cur_effect_id` allows userspace to misdirect the stop sequence, leaving hardware IRQs enabled while dropping PM references.
- [Medium] Dynamically allocated custom waveform buffers are permanently leaked when the device is unbound.
- [Medium] Missing `abs()` on `s16` magnitude level causes massive arithmetic overflow for negative effects.
- [Medium] Registering the input device before PM runtime is fully configured creates a race condition for early playback events.
--
commit 84c087855458c41a5a109b1ffb5594db116b5170
Author: Fenglin Wu <fenglin.wu@oss.qualcomm.com>
input: misc: Add Qualcomm SPMI PMIC haptics driver
Add an initial driver for the Qualcomm PMIH010x 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..75b3e338b54eb
> --- /dev/null
> +++ b/drivers/input/misc/qcom-spmi-haptics.c
> @@ -0,0 +1,831 @@
[ ... ]
> +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)((u32)ffe->u.constant.level * 255 / 0x7FFF);
[Severity: Medium]
Does this cast cause an arithmetic overflow for negative effect levels?
The ffe->u.constant.level is an s16 which can be negative to represent
opposite-direction effects. Casting a negative s16 directly to u32
sign-extends it, resulting in a large unsigned value. Multiplying this by
255 overflows the 32-bit boundary, producing a garbage amplitude for the
hardware register.
Consider taking the absolute value of the level before casting and scaling.
[ ... ]
> +static irqreturn_t haptics_fifo_empty_irq(int irq, void *dev_id)
> +{
> + struct qcom_haptics *h = dev_id;
> + unsigned long flags;
> + 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;
> +
> + spin_lock_irqsave(&h->fifo_lock, flags);
[Severity: High]
Is it safe to hold a spinlock while calling sleepable regmap operations?
The haptics_write_fifo_chunk() function maps to regmap_bulk_write(). For an
SPMI PMIC, the regmap operates over a slow bus and takes a mutex to sleep
during transactions. Calling these sleepable functions while holding
fifo_lock results in a scheduling while atomic bug.
[ ... ]
> + 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);
[Severity: High]
Can a lockless race condition here cause new playback requests to be silently
dropped?
When an effect completes, this sets play_request to false. Concurrently, if a
new playback request arrives, haptics_playback() sets it to true. If the IRQ
handler runs immediately after haptics_playback(), it overwrites the new true
value with false.
When haptics_play_work() executes, it would observe play_request as false,
stop the hardware, and drop the newly queued playback request.
> + spin_unlock_irqrestore(&h->fifo_lock, flags);
> + 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]
Is there a risk of a use-after-free here if an effect is erased while playing?
The input framework allows effect upload or erase to occur concurrently with
playback. In haptics_erase() and haptics_upload_effect(), the driver calls
kfree(priv->fifo_data) synchronously.
However, the stop request only schedules an asynchronous work item to stop
playback. Because the hardware stop is deferred, the hardware may still be
actively playing and asserting the FIFO-empty interrupt. The IRQ handler
will then attempt to read from the freed fifo_data.
[ ... ]
> +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_mark_last_busy(h->dev);
> + pm_runtime_put_autosuspend(h->dev);
> + h->pm_ref_held = false;
> + }
> + goto unlock;
> + }
> +
> + 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;
[Severity: High]
Can this leak runtime PM usage counters?
The pm_ref_held flag is set to true without checking if it is already true.
If userspace issues multiple start events without interleaved stops (for
example, to update parameters), the PM usage counter is incremented multiple
times.
When playback is eventually stopped, the single check drops only one
reference via pm_runtime_put_autosuspend(), permanently leaking all other
references and preventing the device from suspending.
[ ... ]
> +static int haptics_upload_effect(struct input_dev *dev,
> + struct ff_effect *effect,
> + struct ff_effect *old)
> +{
[ ... ]
> + 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->data_len = effect->u.periodic.custom_len - CUSTOM_DATA_SAMPLE_START;
> +
> + kfree(priv->fifo_data);
> + priv->fifo_data = kmalloc(priv->data_len, GFP_KERNEL);
> + if (!priv->fifo_data) {
> + kfree(buf);
> + return -ENOMEM;
> + }
[Severity: High]
Could a failed memory allocation leave the internal state corrupted?
When updating an existing FF_PERIODIC effect, data_len is updated to a new
size before kmalloc() is called. If kmalloc() fails, the function returns
-ENOMEM, leaving data_len non-zero while fifo_data is NULL.
The input core keeps the existing effect ID valid. If userspace subsequently
requests to play this effect, haptics_start_fifo() computes a non-zero
init_len and passes the NULL pointer to haptics_write_fifo_chunk(), causing
a crash.
[ ... ]
> +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: High]
Could this lockless update of cur_effect_id misdirect the stop sequence?
If a FIFO effect is actively playing and userspace issues a playback request
for a different DIRECT_PLAY effect, cur_effect_id is updated asynchronously.
When play_work eventually processes the stop/start sequence,
haptics_stop_locked() checks the newly updated mode:
if (h->effects[h->cur_effect_id].mode == HAPTICS_FIFO) {
ptn_write(h, HAP_PTN_FIFO_EMPTY_CFG_REG, 0);
Seeing DIRECT_PLAY, it would skip disabling the FIFO IRQ and clearing
fifo_data, while still dropping the PM reference. The device auto-suspends,
but the IRQ remains enabled and fires spuriously.
[ ... ]
> +static int qcom_haptics_probe(struct platform_device *pdev)
> +{
[ ... ]
> + ff = input->ff;
> + ff->upload = haptics_upload_effect;
> + ff->playback = haptics_playback;
> + ff->erase = haptics_erase;
> + ff->set_gain = haptics_set_gain;
[Severity: Medium]
Should the ff->destroy callback be implemented to prevent memory leaks?
Custom waveform buffers are allocated using kmalloc() and stored in
fifo_data. This memory is normally freed in haptics_erase(). However, when
the driver is unbound and the input device is unregistered, the input core
destroys the ff device but does not automatically invoke erase() for
surviving effects.
Without a destroy callback to clean up the effects array, the allocated
buffers appear to be permanently leaked.
> +
> + ret = input_register_device(input);
> + if (ret) {
> + input_ff_destroy(input);
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to register input device\n");
> + }
> +
> + platform_set_drvdata(pdev, h);
> +
> + /*
> + * Grab a reference on behalf of probe (usage_count → 1), mark the
> + * device active, then enable runtime PM.
> + */
> + pm_runtime_get_noresume(&pdev->dev);
> + pm_runtime_use_autosuspend(&pdev->dev);
> + pm_runtime_set_autosuspend_delay(&pdev->dev, HAPTICS_AUTOSUSPEND_MS);
> + devm_pm_runtime_set_active_enabled(&pdev->dev);
[Severity: Medium]
Is there a race condition by registering the input device before PM runtime
is fully configured?
This call makes the force-feedback device immediately visible to userspace.
If an application sends a play request before the probe function finishes,
haptics_play_work() will run and call pm_runtime_resume_and_get(). Since PM
runtime is not yet fully enabled, the PM runtime API will return an error
and fail the playback.
[ ... ]
> +static void qcom_haptics_remove(struct platform_device *pdev)
> +{
> + struct qcom_haptics *h = platform_get_drvdata(pdev);
> +
> + pm_runtime_disable(&pdev->dev);
> + pm_runtime_set_suspended(&pdev->dev);
> +
> + cancel_work_sync(&h->play_work);
> + mutex_lock(&h->play_lock);
> + haptics_stop_locked(h);
> + haptics_enable_module(h, false);
> + mutex_unlock(&h->play_lock);
> +
> + input_unregister_device(h->input);
[Severity: High]
Does this teardown ordering allow a work item to be re-queued and executed
on freed memory?
During driver removal, cancel_work_sync() is called before the input device
is unregistered. This creates a race window where the input device is still
visible to userspace, allowing applications to issue playback events that
call schedule_work().
Once remove() completes, the devres framework frees the qcom_haptics memory.
The mistakenly re-queued background work will then execute, dereferencing
the freed driver context.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-qcom-spmi-haptics-v1-0-d24e422de6b4@oss.qualcomm.com?part=3
^ permalink raw reply
* Re: [PATCH 3/4] input: misc: Add Qualcomm SPMI PMIC haptics driver
From: Konrad Dybcio @ 2026-06-16 10:25 UTC (permalink / raw)
To: Fenglin Wu, linux-arm-msm, Dmitry Torokhov, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Lee Jones, Stephen Boyd,
Bjorn Andersson, Konrad Dybcio
Cc: David Collins, Subbaraman Narayanamurthy, Kamal Wadhwa, kernel,
linux-input, devicetree, linux-kernel
In-Reply-To: <20260616-qcom-spmi-haptics-v1-3-d24e422de6b4@oss.qualcomm.com>
On 6/16/26 12:08 PM, Fenglin Wu wrote:
> Add an initial driver for the Qualcomm PMIH010x 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>
> ---
[...]
> +static int cfg_write(struct qcom_haptics *h, u32 off, u32 val)
static inline
although I have mixed feelings about having so many accessors
[...]
> +static int haptics_write_fifo_chunk(struct qcom_haptics *h,
> + const s8 *data, u32 len)
> +{
> + u32 i, bulk_len = ALIGN_DOWN(len, 4);
Please avoid mixing multiple declarations and assignments
> + int ret;
> +
> + for (i = 0; i < bulk_len; i += 4) {
You can do 'int i' in loops nowadays
> + 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;
> + }
So if i'm reading this right, the first loop will always write
4*(len//4) bytes and the second one will be entered at most once,
to write len rem 4 bytes.. should this be an if instead?
> +
> + 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 */
What's the not-"for now" endgame for this?
> + 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 = (u32)((u64)vmax_mv * h->gain / 0xFFFF);
mult_frac()
> +
> + 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_nosync(h->fifo_empty_irq);
This is called in the .remove() path, I think you may need the
sync variant as the underlying device may be destroyed before the
ISR completes if there's a late interrupt
[...]
> +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;
nit: \n before return is 'nice'
[...]
> + ret = device_property_read_u32_array(&pdev->dev, "reg", regs,
> + ARRAY_SIZE(regs));
Here you use device_property_
> + if (ret)
> + return dev_err_probe(&pdev->dev, ret,
> + "failed to read 'reg' property\n");
> +
> + h->cfg_base = regs[0];
> + h->ptn_base = regs[1];
> +
> + ret = of_property_read_u32(h->dev->of_node, "qcom,lra-period-us",
> + &h->t_lra_us);
And here you use of_property_ (please use device_)
[...]
> + input->name = "qcom-spmi-haptics";
Odd whitespace
Konrad
^ permalink raw reply
* [PATCH 00/11] HID: iio: warning clean up and prefer kernel coding style
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda, Zhang Lixu
Hi all,
This series updates HID IIO drivers to resolve checkpatch and
kernel coding style issue.
This improves readability and follow standard kernel coding style
No functional changes are introduced.
Testing:
- Compiled with W=1 for each patch in the series
---
Sanjay Chitroda (11):
iio: hid-sensors: add missing blank line after declarations
iio: humidity: hid-sensor-humidity: align parenthesis for readability
iio: gyro: hid-sensor-gyro-3d: align parenthesis for readability
iio: magnetometer: hid-sensor-magn-3d: align parenthesis for readability
iio: humidity: hid-sensor-humidity: use common device for devres
iio: position: hid-sensor-custom-intel-hinge: use common device for devres
iio: temperature: hid-sensor-temperature: use common device for devres
iio: hid-sensor-magn-3d: use ! instead of explicit NULL check
iio: hid-sensor-rotation: use ! instead of explicit NULL check
iio: hid-sensor-incl-3d: use ! instead of explicit NULL check
iio: hid-sensor-accel-3d: use ! instead of explicit NULL check
drivers/iio/accel/hid-sensor-accel-3d.c | 2 +-
.../iio/common/hid-sensors/hid-sensor-trigger.c | 3 +
drivers/iio/gyro/hid-sensor-gyro-3d.c | 58 ++++++++--------
drivers/iio/humidity/hid-sensor-humidity.c | 48 ++++++-------
drivers/iio/magnetometer/hid-sensor-magn-3d.c | 80 +++++++++++-----------
drivers/iio/orientation/hid-sensor-incl-3d.c | 2 +-
drivers/iio/orientation/hid-sensor-rotation.c | 2 +-
.../iio/position/hid-sensor-custom-intel-hinge.c | 2 +-
drivers/iio/temperature/hid-sensor-temperature.c | 4 +-
9 files changed, 102 insertions(+), 99 deletions(-)
---
base-commit: a50909aa46dec46de3c73235fc15a7d6f763d996
change-id: 20260615-15-jun-hid-iio-alignment-46bba7279be3
Best regards,
--
Sanjay Chitroda <sanjayembeddedse@gmail.com>
^ permalink raw reply
* [PATCH 01/11] iio: hid-sensors: add missing blank line after declarations
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Add a blank line after variable declarations to improve
readability and align with kernel coding style.
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
changes in v4:
- Rectify commit message with input from David
---
drivers/iio/common/hid-sensors/hid-sensor-trigger.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
index 417c4ab8c1b2..c8ccf96f3d03 100644
--- a/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
+++ b/drivers/iio/common/hid-sensors/hid-sensor-trigger.c
@@ -314,7 +314,9 @@ static int __maybe_unused hid_sensor_resume(struct device *dev)
{
struct iio_dev *indio_dev = dev_get_drvdata(dev);
struct hid_sensor_common *attrb = iio_device_get_drvdata(indio_dev);
+
schedule_work(&attrb->work);
+
return 0;
}
@@ -322,6 +324,7 @@ static int __maybe_unused hid_sensor_runtime_resume(struct device *dev)
{
struct iio_dev *indio_dev = dev_get_drvdata(dev);
struct hid_sensor_common *attrb = iio_device_get_drvdata(indio_dev);
+
return _hid_sensor_power_state(attrb, true);
}
--
2.34.1
^ permalink raw reply related
* [PATCH 02/11] iio: humidity: hid-sensor-humidity: align parenthesis for readability
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Adjust alignment of parentheses to improve readability and
maintain consistency with kernel coding style
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/humidity/hid-sensor-humidity.c | 46 +++++++++++++++---------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/drivers/iio/humidity/hid-sensor-humidity.c b/drivers/iio/humidity/hid-sensor-humidity.c
index be2338d5f407..6fed019f4099 100644
--- a/drivers/iio/humidity/hid-sensor-humidity.c
+++ b/drivers/iio/humidity/hid-sensor-humidity.c
@@ -45,7 +45,7 @@ static const struct iio_chan_spec humidity_channels[] = {
/* Adjust channel real bits based on report descriptor */
static void humidity_adjust_channel_bit_mask(struct iio_chan_spec *channels,
- int channel, int size)
+ int channel, int size)
{
channels[channel].scan_type.sign = 's';
/* Real storage bits will change based on the report desc. */
@@ -55,8 +55,8 @@ static void humidity_adjust_channel_bit_mask(struct iio_chan_spec *channels,
}
static int humidity_read_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int *val, int *val2, long mask)
+ struct iio_chan_spec const *chan,
+ int *val, int *val2, long mask)
{
struct hid_humidity_state *humid_st = iio_priv(indio_dev);
@@ -101,8 +101,8 @@ static int humidity_read_raw(struct iio_dev *indio_dev,
}
static int humidity_write_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int val, int val2, long mask)
+ struct iio_chan_spec const *chan,
+ int val, int val2, long mask)
{
struct hid_humidity_state *humid_st = iio_priv(indio_dev);
@@ -127,7 +127,7 @@ static const struct iio_info humidity_info = {
/* Callback handler to send event after all samples are received and captured */
static int humidity_proc_event(struct hid_sensor_hub_device *hsdev,
- unsigned int usage_id, void *pdev)
+ unsigned int usage_id, void *pdev)
{
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct hid_humidity_state *humid_st = iio_priv(indio_dev);
@@ -141,8 +141,8 @@ static int humidity_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int humidity_capture_sample(struct hid_sensor_hub_device *hsdev,
- unsigned int usage_id, size_t raw_len,
- char *raw_data, void *pdev)
+ unsigned int usage_id, size_t raw_len,
+ char *raw_data, void *pdev)
{
struct iio_dev *indio_dev = platform_get_drvdata(pdev);
struct hid_humidity_state *humid_st = iio_priv(indio_dev);
@@ -159,17 +159,17 @@ static int humidity_capture_sample(struct hid_sensor_hub_device *hsdev,
/* Parse report which is specific to an usage id */
static int humidity_parse_report(struct platform_device *pdev,
- struct hid_sensor_hub_device *hsdev,
- struct iio_chan_spec *channels,
- unsigned int usage_id,
- struct hid_humidity_state *st)
+ struct hid_sensor_hub_device *hsdev,
+ struct iio_chan_spec *channels,
+ unsigned int usage_id,
+ struct hid_humidity_state *st)
{
int ret;
ret = sensor_hub_input_get_attribute_info(hsdev, HID_INPUT_REPORT,
- usage_id,
- HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY,
- &st->humidity_attr);
+ usage_id,
+ HID_USAGE_SENSOR_ATMOSPHERIC_HUMIDITY,
+ &st->humidity_attr);
if (ret < 0)
return ret;
@@ -208,20 +208,20 @@ static int hid_humidity_probe(struct platform_device *pdev)
humid_st->common_attributes.pdev = pdev;
ret = hid_sensor_parse_common_attributes(hsdev,
- HID_USAGE_SENSOR_HUMIDITY,
- &humid_st->common_attributes,
- humidity_sensitivity_addresses,
- ARRAY_SIZE(humidity_sensitivity_addresses));
+ HID_USAGE_SENSOR_HUMIDITY,
+ &humid_st->common_attributes,
+ humidity_sensitivity_addresses,
+ ARRAY_SIZE(humidity_sensitivity_addresses));
if (ret)
return ret;
humid_chans = devm_kmemdup(&indio_dev->dev, humidity_channels,
- sizeof(humidity_channels), GFP_KERNEL);
+ sizeof(humidity_channels), GFP_KERNEL);
if (!humid_chans)
return -ENOMEM;
ret = humidity_parse_report(pdev, hsdev, humid_chans,
- HID_USAGE_SENSOR_HUMIDITY, humid_st);
+ HID_USAGE_SENSOR_HUMIDITY, humid_st);
if (ret)
return ret;
@@ -234,7 +234,7 @@ static int hid_humidity_probe(struct platform_device *pdev)
atomic_set(&humid_st->common_attributes.data_ready, 0);
ret = hid_sensor_setup_trigger(indio_dev, name,
- &humid_st->common_attributes);
+ &humid_st->common_attributes);
if (ret)
return ret;
@@ -242,7 +242,7 @@ static int hid_humidity_probe(struct platform_device *pdev)
humidity_callbacks.pdev = pdev;
ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_HUMIDITY,
- &humidity_callbacks);
+ &humidity_callbacks);
if (ret)
goto error_remove_trigger;
--
2.34.1
^ permalink raw reply related
* [PATCH 03/11] iio: gyro: hid-sensor-gyro-3d: align parenthesis for readability
From: Sanjay Chitroda @ 2026-06-16 10:26 UTC (permalink / raw)
To: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Jiri Kosina, Srinivas Pandruvada
Cc: linux-iio, linux-kernel, linux-input, Sanjay Chitroda
In-Reply-To: <20260616-15-jun-hid-iio-alignment-v1-0-0cd544286575@gmail.com>
From: Sanjay Chitroda <sanjayembeddedse@gmail.com>
Adjust alignment of parentheses to improve readability and
maintain consistency with kernel coding style
Signed-off-by: Sanjay Chitroda <sanjayembeddedse@gmail.com>
---
drivers/iio/gyro/hid-sensor-gyro-3d.c | 58 +++++++++++++++++------------------
1 file changed, 29 insertions(+), 29 deletions(-)
diff --git a/drivers/iio/gyro/hid-sensor-gyro-3d.c b/drivers/iio/gyro/hid-sensor-gyro-3d.c
index bbca2111e79b..fa604cf26d06 100644
--- a/drivers/iio/gyro/hid-sensor-gyro-3d.c
+++ b/drivers/iio/gyro/hid-sensor-gyro-3d.c
@@ -85,9 +85,9 @@ static const struct iio_chan_spec gyro_3d_channels[] = {
/* Channel read_raw handler */
static int gyro_3d_read_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int *val, int *val2,
- long mask)
+ struct iio_chan_spec const *chan,
+ int *val, int *val2,
+ long mask)
{
struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
int report_id = -1;
@@ -112,8 +112,7 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
min < 0);
else {
*val = 0;
- hid_sensor_power_state(&gyro_state->common_attributes,
- false);
+ hid_sensor_power_state(&gyro_state->common_attributes, false);
return -EINVAL;
}
hid_sensor_power_state(&gyro_state->common_attributes, false);
@@ -146,10 +145,10 @@ static int gyro_3d_read_raw(struct iio_dev *indio_dev,
/* Channel write_raw handler */
static int gyro_3d_write_raw(struct iio_dev *indio_dev,
- struct iio_chan_spec const *chan,
- int val,
- int val2,
- long mask)
+ struct iio_chan_spec const *chan,
+ int val,
+ int val2,
+ long mask)
{
struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
int ret = 0;
@@ -177,8 +176,8 @@ static const struct iio_info gyro_3d_info = {
/* Callback handler to send event after all samples are received and captured */
static int gyro_3d_proc_event(struct hid_sensor_hub_device *hsdev,
- u32 usage_id,
- void *priv)
+ u32 usage_id,
+ void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
@@ -199,9 +198,9 @@ static int gyro_3d_proc_event(struct hid_sensor_hub_device *hsdev,
/* Capture samples in local storage */
static int gyro_3d_capture_sample(struct hid_sensor_hub_device *hsdev,
- u32 usage_id,
- size_t raw_len, char *raw_data,
- void *priv)
+ u32 usage_id,
+ size_t raw_len, char *raw_data,
+ void *priv)
{
struct iio_dev *indio_dev = platform_get_drvdata(priv);
struct gyro_3d_state *gyro_state = iio_priv(indio_dev);
@@ -240,11 +239,12 @@ static int gyro_3d_parse_report(struct platform_device *pdev,
int ret;
for (unsigned int ch = CHANNEL_SCAN_INDEX_X; ch <= CHANNEL_SCAN_INDEX_Z; ch++) {
- ret = sensor_hub_input_get_attribute_info(hsdev,
- HID_INPUT_REPORT,
- usage_id,
- HID_USAGE_SENSOR_ANGL_VELOCITY_X_AXIS + ch,
- &st->gyro[ch]);
+ ret = sensor_hub_input_get_attribute_info(
+ hsdev,
+ HID_INPUT_REPORT,
+ usage_id,
+ HID_USAGE_SENSOR_ANGL_VELOCITY_X_AXIS + ch,
+ &st->gyro[ch]);
if (ret < 0)
break;
channels[ch].scan_type = (struct iio_scan_type) {
@@ -254,10 +254,10 @@ static int gyro_3d_parse_report(struct platform_device *pdev,
};
}
dev_dbg(&pdev->dev, "gyro_3d %x:%x, %x:%x, %x:%x\n",
- st->gyro[0].index,
- st->gyro[0].report_id,
- st->gyro[1].index, st->gyro[1].report_id,
- st->gyro[2].index, st->gyro[2].report_id);
+ st->gyro[0].index,
+ st->gyro[0].report_id,
+ st->gyro[1].index, st->gyro[1].report_id,
+ st->gyro[2].index, st->gyro[2].report_id);
st->scale_precision = hid_sensor_format_scale(
HID_USAGE_SENSOR_GYRO_3D,
@@ -286,10 +286,10 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
gyro_state->common_attributes.pdev = pdev;
ret = hid_sensor_parse_common_attributes(hsdev,
- HID_USAGE_SENSOR_GYRO_3D,
- &gyro_state->common_attributes,
- gyro_3d_sensitivity_addresses,
- ARRAY_SIZE(gyro_3d_sensitivity_addresses));
+ HID_USAGE_SENSOR_GYRO_3D,
+ &gyro_state->common_attributes,
+ gyro_3d_sensitivity_addresses,
+ ARRAY_SIZE(gyro_3d_sensitivity_addresses));
if (ret) {
dev_err(&pdev->dev, "failed to setup common attributes\n");
return ret;
@@ -318,7 +318,7 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
atomic_set(&gyro_state->common_attributes.data_ready, 0);
ret = hid_sensor_setup_trigger(indio_dev, name,
- &gyro_state->common_attributes);
+ &gyro_state->common_attributes);
if (ret < 0) {
dev_err(&pdev->dev, "trigger setup failed\n");
return ret;
@@ -334,7 +334,7 @@ static int hid_gyro_3d_probe(struct platform_device *pdev)
gyro_state->callbacks.capture_sample = gyro_3d_capture_sample;
gyro_state->callbacks.pdev = pdev;
ret = sensor_hub_register_callback(hsdev, HID_USAGE_SENSOR_GYRO_3D,
- &gyro_state->callbacks);
+ &gyro_state->callbacks);
if (ret < 0) {
dev_err(&pdev->dev, "callback reg failed\n");
goto error_iio_unreg;
--
2.34.1
^ 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