* [PATCH 01/12] HID: rmi: return error on short output report writes
2026-11-14 5:59 [PATCH 00/12] HID: rmi: Fix report handling, device lifecycle, and clean up code Dmitry Torokhov
@ 2026-11-14 5:59 ` Dmitry Torokhov
2026-11-14 5:59 ` [PATCH 02/12] HID: rmi: only strip trailing 0xff bytes from attention reports Dmitry Torokhov
` (10 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2026-11-14 5:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Andrew Duggan; +Cc: linux-input, linux-kernel
HID output report transfers return the byte count transferred on
success, allowing short writes to either be ignored or propagate
positive byte counts as errors to callers expecting 0 or a negative
errno.
Treat short transfers as -EIO errors and return 0 when the full output
report is written.
Fixes: 9fb6bf02e3ad ("HID: rmi: introduce RMI driver for Synaptics touchpads")
Fixes: dd8df28459dc ("HID: rmi: Add functions for writing to registers")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-rmi.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index 20dda1992f98..cacf81ed56a7 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -130,7 +130,7 @@ static int rmi_set_page(struct hid_device *hdev, u8 page)
retval = rmi_write_report(hdev, data->writeReport,
data->output_report_size);
- if (retval != data->output_report_size) {
+ if (retval) {
dev_err(&hdev->dev,
"%s: set page failed: %d.", __func__, retval);
return retval;
@@ -182,7 +182,13 @@ static int rmi_write_report(struct hid_device *hdev, u8 *report, int len)
return ret;
}
- return ret;
+ if (ret != len) {
+ dev_err(&hdev->dev, "short write to hid device (%d < %d)\n",
+ ret, len);
+ return -EIO;
+ }
+
+ return 0;
}
static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
@@ -216,7 +222,7 @@ static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
ret = rmi_write_report(hdev, data->writeReport,
data->output_report_size);
- if (ret != data->output_report_size) {
+ if (ret) {
dev_err(&hdev->dev,
"failed to write request output report (%d)\n",
ret);
@@ -300,13 +306,12 @@ static int rmi_hid_write_block(struct rmi_transport_dev *xport, u16 addr,
ret = rmi_write_report(hdev, data->writeReport,
data->output_report_size);
- if (ret < 0) {
+ if (ret) {
dev_err(&hdev->dev,
"failed to write request output report (%d)\n",
ret);
goto exit;
}
- ret = 0;
exit:
mutex_unlock(&data->page_mutex);
--
2.56.0.679.gc527c65306-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 02/12] HID: rmi: only strip trailing 0xff bytes from attention reports
2026-11-14 5:59 [PATCH 00/12] HID: rmi: Fix report handling, device lifecycle, and clean up code Dmitry Torokhov
2026-11-14 5:59 ` [PATCH 01/12] HID: rmi: return error on short output report writes Dmitry Torokhov
@ 2026-11-14 5:59 ` Dmitry Torokhov
2026-11-14 5:59 ` [PATCH 03/12] HID: rmi: validate read data report and protocol transfer bounds Dmitry Torokhov
` (9 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2026-11-14 5:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Andrew Duggan; +Cc: linux-input, linux-kernel
rmi_check_sanity() strips trailing 0xff bytes from incoming HID reports
to work around bus corruption where attention reports are occasionally
filled with 0xff sentinel values. Running this filter on all raw
reports also truncates register read responses that legitimately end
with 0xff bytes, leaving stale data at the end of the read buffer.
Move the sanity check into rmi_input_event() so that only attention
reports are filtered.
Fixes: 5b65c2a02966 ("HID: rmi: check sanity of the incoming report")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-rmi.c | 37 ++++++++++++++++++++-----------------
1 file changed, 20 insertions(+), 17 deletions(-)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index cacf81ed56a7..41676685ce28 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -343,6 +343,22 @@ static void rmi_reset_work(struct work_struct *work)
rmi_reset_attn_mode(hdata->hdev);
}
+static int rmi_check_sanity(struct hid_device *hdev, u8 *data, int size)
+{
+ int valid_size = size;
+ /*
+ * On the Dell XPS 13 9333, the bus sometimes get confused and fills
+ * the report with a sentinel value "ff". Synaptics told us that such
+ * behavior does not comes from the touchpad itself, so we filter out
+ * such reports here.
+ */
+
+ while (valid_size > 0 && data[valid_size - 1] == 0xff)
+ valid_size--;
+
+ return valid_size;
+}
+
static int rmi_input_event(struct hid_device *hdev, u8 *data, int size)
{
struct rmi_data *hdata = hid_get_drvdata(hdev);
@@ -352,6 +368,10 @@ static int rmi_input_event(struct hid_device *hdev, u8 *data, int size)
if (!(test_bit(RMI_STARTED, &hdata->flags)))
return 0;
+ size = rmi_check_sanity(hdev, data, size);
+ if (size < 2)
+ return 0;
+
pm_wakeup_event(hdev->dev.parent, 0);
local_irq_save(flags);
@@ -381,22 +401,6 @@ static int rmi_read_data_event(struct hid_device *hdev, u8 *data, int size)
return 1;
}
-static int rmi_check_sanity(struct hid_device *hdev, u8 *data, int size)
-{
- int valid_size = size;
- /*
- * On the Dell XPS 13 9333, the bus sometimes get confused and fills
- * the report with a sentinel value "ff". Synaptics told us that such
- * behavior does not comes from the touchpad itself, so we filter out
- * such reports here.
- */
-
- while (valid_size > 0 && data[valid_size - 1] == 0xff)
- valid_size--;
-
- return valid_size;
-}
-
static int rmi_raw_event(struct hid_device *hdev,
struct hid_report *report, u8 *data, int size)
{
@@ -405,7 +409,6 @@ static int rmi_raw_event(struct hid_device *hdev,
if (!(hdata->device_flags & RMI_DEVICE))
return 0;
- size = rmi_check_sanity(hdev, data, size);
if (size < 2)
return 0;
--
2.56.0.679.gc527c65306-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 03/12] HID: rmi: validate read data report and protocol transfer bounds
2026-11-14 5:59 [PATCH 00/12] HID: rmi: Fix report handling, device lifecycle, and clean up code Dmitry Torokhov
2026-11-14 5:59 ` [PATCH 01/12] HID: rmi: return error on short output report writes Dmitry Torokhov
2026-11-14 5:59 ` [PATCH 02/12] HID: rmi: only strip trailing 0xff bytes from attention reports Dmitry Torokhov
@ 2026-11-14 5:59 ` Dmitry Torokhov
2026-11-14 5:59 ` [PATCH 04/12] HID: rmi: fix device lifecycle, suspend, and reset work handling Dmitry Torokhov
` (8 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2026-11-14 5:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Andrew Duggan; +Cc: linux-input, linux-kernel
During probe the input report buffer is sized from the attention report
rather than the read data report that uses it, block read and write
requests do not check that lengths fit within the 16-bit and 8-bit
protocol length fields, write length validation occurs only after
switching pages, and incoming read data events do not verify that the
received report is large enough to hold the advertised payload count.
Size the input report buffer from the read data report during probe,
validate protocol and buffer length bounds upfront in block read and
write transfers, and reject malformed or truncated read data reports
with -EPROTO instead of clamping their length.
Fixes: 9fb6bf02e3ad ("HID: rmi: introduce RMI driver for Synaptics touchpads")
Fixes: dd8df28459dc ("HID: rmi: Add functions for writing to registers")
Fixes: 4956993bb3be ("HID: rmi: fix OOB access with undersized RMI reports")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-rmi.c | 65 +++++++++++++++++++++++++--------------------------
1 file changed, 32 insertions(+), 33 deletions(-)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index 41676685ce28..d9181fd66c5c 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -197,10 +197,13 @@ static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
struct rmi_data *data = container_of(xport, struct rmi_data, xport);
struct hid_device *hdev = data->hdev;
int ret;
- int bytes_read;
- int bytes_needed;
int retries;
- int read_input_count;
+ size_t bytes_read;
+ size_t bytes_needed;
+ size_t read_input_count;
+
+ if (len > U16_MAX)
+ return -EINVAL;
mutex_lock(&data->page_mutex);
@@ -241,23 +244,14 @@ static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
break;
}
- read_input_count = min_t(int, data->readReport[1],
- data->input_report_size - 2);
- if (!read_input_count) {
- /*
- * A zero length reply advances neither
- * bytes_read nor bytes_needed, and because a
- * reply did arrive the wait above does not
- * time out either, so a device answering 0
- * forever would spin here indefinitely with
- * page_mutex held.
- */
- hid_warn(hdev, "%s: zero-length read reply\n",
- __func__);
+ read_input_count = data->readReport[1];
+ if (read_input_count == 0 ||
+ read_input_count > data->input_report_size - 2) {
clear_bit(RMI_READ_DATA_PENDING, &data->flags);
- ret = -EIO;
+ ret = -EPROTO;
break;
}
+
memcpy(buf + bytes_read, &data->readReport[2],
min(read_input_count, bytes_needed));
@@ -285,6 +279,9 @@ static int rmi_hid_write_block(struct rmi_transport_dev *xport, u16 addr,
struct hid_device *hdev = data->hdev;
int ret;
+ if (len > data->output_report_size - 4 || len > U8_MAX)
+ return -EINVAL;
+
mutex_lock(&data->page_mutex);
if (RMI_PAGE(addr) != data->page) {
@@ -293,11 +290,6 @@ static int rmi_hid_write_block(struct rmi_transport_dev *xport, u16 addr,
goto exit;
}
- if (len + 4 > data->output_report_size) {
- ret = -EINVAL;
- goto exit;
- }
-
data->writeReport[0] = RMI_WRITE_REPORT_ID;
data->writeReport[1] = len;
data->writeReport[2] = addr & 0xFF;
@@ -305,7 +297,7 @@ static int rmi_hid_write_block(struct rmi_transport_dev *xport, u16 addr,
memcpy(&data->writeReport[4], buf, len);
ret = rmi_write_report(hdev, data->writeReport,
- data->output_report_size);
+ data->output_report_size);
if (ret) {
dev_err(&hdev->dev,
"failed to write request output report (%d)\n",
@@ -388,13 +380,19 @@ static int rmi_input_event(struct hid_device *hdev, u8 *data, int size)
static int rmi_read_data_event(struct hid_device *hdev, u8 *data, int size)
{
struct rmi_data *hdata = hid_get_drvdata(hdev);
+ size_t copy_size;
if (!test_bit(RMI_READ_REQUEST_PENDING, &hdata->flags)) {
hid_dbg(hdev, "no read request pending\n");
return 0;
}
- memcpy(hdata->readReport, data, min((u32)size, hdata->input_report_size));
+ copy_size = min_t(size_t, size, hdata->input_report_size);
+ if (copy_size < 3 || data[1] == 0 || data[1] > copy_size - 2)
+ hdata->readReport[1] = 0;
+ else
+ memcpy(hdata->readReport, data, copy_size);
+
set_bit(RMI_READ_DATA_PENDING, &hdata->flags);
wake_up(&hdata->wait);
@@ -722,6 +720,12 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
goto start;
}
+ if (!rmi_check_valid_report_id(hdev, HID_INPUT_REPORT,
+ RMI_READ_DATA_REPORT_ID, &input_report)) {
+ hid_dbg(hdev, "device does not have read data input report\n");
+ goto start;
+ }
+
data->input_report_size = hid_report_len(input_report);
if (!rmi_check_valid_report_id(hdev, HID_OUTPUT_REPORT,
@@ -733,15 +737,10 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
data->output_report_size = hid_report_len(output_report);
- /*
- * The write reports built by this driver occupy 6 bytes and the read
- * handshake looks at the first 3 bytes of an input report, so refuse
- * to drive a device whose reports cannot hold them.
- */
- if (data->output_report_size < 6 || data->input_report_size < 3) {
- hid_err(hdev, "rmi reports too small (out=%u in=%u)\n",
- data->output_report_size, data->input_report_size);
- goto start;
+ if (data->input_report_size < 3 || data->output_report_size < 6) {
+ hid_err(hdev, "invalid report sizes (in %u, out %u)\n",
+ data->input_report_size, data->output_report_size);
+ return -EINVAL;
}
data->device_flags |= RMI_DEVICE;
--
2.56.0.679.gc527c65306-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 04/12] HID: rmi: fix device lifecycle, suspend, and reset work handling
2026-11-14 5:59 [PATCH 00/12] HID: rmi: Fix report handling, device lifecycle, and clean up code Dmitry Torokhov
` (2 preceding siblings ...)
2026-11-14 5:59 ` [PATCH 03/12] HID: rmi: validate read data report and protocol transfer bounds Dmitry Torokhov
@ 2026-11-14 5:59 ` Dmitry Torokhov
2026-11-14 6:21 ` sashiko-bot
2026-11-14 5:59 ` [PATCH 05/12] HID: rmi: re-select page 0 across device resets Dmitry Torokhov
` (7 subsequent siblings)
11 siblings, 1 reply; 14+ messages in thread
From: Dmitry Torokhov @ 2026-11-14 5:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Andrew Duggan; +Cc: linux-input, linux-kernel
Incoming events can schedule reset work before transport initialization
completes or re-queue it during suspend and removal, power management
callbacks can dereference uninitialized device structures on failure,
and failed resume steps can leave transport interrupts disabled.
Gate reset work scheduling and power management callbacks on device
state, track suspended state to prevent work execution, ensure resume
re-enables transport interrupts, and permanently disable reset work on
removal.
Fixes: 0b2c7a897378 ("HID: rmi: Make hid-rmi a transport driver for synaptics-rmi4")
Fixes: 8725aa4fa7de ("HID: rmi: Check that the RMI_STARTED bit is set before unregistering the RMI transport device")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-rmi.c | 35 ++++++++++++++++++++++-------------
1 file changed, 22 insertions(+), 13 deletions(-)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index d9181fd66c5c..8783edb7fd6a 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -31,6 +31,7 @@
#define RMI_READ_REQUEST_PENDING 0
#define RMI_READ_DATA_PENDING 1
#define RMI_STARTED 2
+#define RMI_SUSPENDED 3
/* device flags */
#define RMI_DEVICE BIT(0)
@@ -428,6 +429,8 @@ static int rmi_event(struct hid_device *hdev, struct hid_field *field,
struct rmi_data *data = hid_get_drvdata(hdev);
if ((data->device_flags & RMI_DEVICE) &&
+ test_bit(RMI_STARTED, &data->flags) &&
+ !test_bit(RMI_SUSPENDED, &data->flags) &&
(field->application == HID_GD_POINTER ||
field->application == HID_GD_MOUSE)) {
if (data->device_flags & RMI_DEVICE_HAS_PHYS_BUTTONS) {
@@ -469,12 +472,16 @@ static int rmi_suspend(struct hid_device *hdev, pm_message_t message)
struct rmi_device *rmi_dev = data->xport.rmi_dev;
int ret;
- if (!(data->device_flags & RMI_DEVICE))
+ if (!test_bit(RMI_STARTED, &data->flags))
return 0;
+ set_bit(RMI_SUSPENDED, &data->flags);
+ cancel_work_sync(&data->reset_work);
+
ret = rmi_driver_suspend(rmi_dev, false);
if (ret) {
hid_warn(hdev, "Failed to suspend device: %d\n", ret);
+ clear_bit(RMI_SUSPENDED, &data->flags);
return ret;
}
@@ -486,27 +493,31 @@ static int rmi_post_resume(struct hid_device *hdev)
struct rmi_data *data = hid_get_drvdata(hdev);
struct rmi_device *rmi_dev = data->xport.rmi_dev;
int ret;
+ int err;
- if (!(data->device_flags & RMI_DEVICE))
+ if (!test_bit(RMI_STARTED, &data->flags))
return 0;
/* Make sure the HID device is ready to receive events */
ret = hid_hw_open(hdev);
if (ret)
- return ret;
+ goto out;
ret = rmi_reset_attn_mode(hdev);
if (ret)
- goto out;
+ hid_warn(hdev, "Failed to reset attention mode: %d\n", ret);
- ret = rmi_driver_resume(rmi_dev, false);
- if (ret) {
- hid_warn(hdev, "Failed to resume device: %d\n", ret);
- goto out;
+ err = rmi_driver_resume(rmi_dev, false);
+ if (err) {
+ hid_warn(hdev, "Failed to resume device: %d\n", err);
+ if (!ret)
+ ret = err;
}
-out:
hid_hw_close(hdev);
+
+out:
+ clear_bit(RMI_SUSPENDED, &data->flags);
return ret;
}
@@ -787,10 +798,8 @@ static void rmi_remove(struct hid_device *hdev)
{
struct rmi_data *hdata = hid_get_drvdata(hdev);
- if ((hdata->device_flags & RMI_DEVICE)
- && test_bit(RMI_STARTED, &hdata->flags)) {
- clear_bit(RMI_STARTED, &hdata->flags);
- cancel_work_sync(&hdata->reset_work);
+ if (test_and_clear_bit(RMI_STARTED, &hdata->flags)) {
+ disable_work_sync(&hdata->reset_work);
rmi_unregister_transport_device(&hdata->xport);
}
--
2.56.0.679.gc527c65306-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* Re: [PATCH 04/12] HID: rmi: fix device lifecycle, suspend, and reset work handling
2026-11-14 5:59 ` [PATCH 04/12] HID: rmi: fix device lifecycle, suspend, and reset work handling Dmitry Torokhov
@ 2026-11-14 6:21 ` sashiko-bot
0 siblings, 0 replies; 14+ messages in thread
From: sashiko-bot @ 2026-11-14 6:21 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: linux-input
Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] HID: rmi: race between rmi_event() and rmi_suspend() queuing
reset_work
- [Medium] HID: rmi: disabled IRQ on hid_hw_open() failure in
rmi_post_resume()
--
commit d4708cefad45f4f15a5ff3b156b07b3725726356
Author: Dmitry Torokhov <dmitry.torokhov@gmail.com>
HID: rmi: fix device lifecycle, suspend, and reset work handling
This commit gates reset work scheduling and power management callbacks on
RMI_STARTED and RMI_SUSPENDED, ensures rmi_driver_resume() runs even if
rmi_reset_attn_mode() fails, and uses disable_work_sync() in rmi_remove().
> diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
> index d9181fd66c5c8..8783edb7fd6a1 100644
> --- a/drivers/hid/hid-rmi.c
> +++ b/drivers/hid/hid-rmi.c
> @@ -31,6 +31,7 @@
> #define RMI_READ_REQUEST_PENDING 0
> #define RMI_READ_DATA_PENDING 1
> #define RMI_STARTED 2
> +#define RMI_SUSPENDED 3
>
> /* device flags */
> #define RMI_DEVICE BIT(0)
> @@ -428,6 +429,8 @@ static int rmi_event(struct hid_device *hdev, struct hid_field *field,
> struct rmi_data *data = hid_get_drvdata(hdev);
>
> if ((data->device_flags & RMI_DEVICE) &&
> + test_bit(RMI_STARTED, &data->flags) &&
> + !test_bit(RMI_SUSPENDED, &data->flags) &&
> (field->application == HID_GD_POINTER ||
> field->application == HID_GD_MOUSE)) {
> if (data->device_flags & RMI_DEVICE_HAS_PHYS_BUTTONS) {
> @@ -469,12 +472,16 @@ static int rmi_suspend(struct hid_device *hdev, pm_message_t message)
> struct rmi_device *rmi_dev = data->xport.rmi_dev;
> int ret;
>
> - if (!(data->device_flags & RMI_DEVICE))
> + if (!test_bit(RMI_STARTED, &data->flags))
> return 0;
>
> + set_bit(RMI_SUSPENDED, &data->flags);
> + cancel_work_sync(&data->reset_work);
> +
[Severity: Medium]
Can rmi_event() still queue data->reset_work after cancel_work_sync() returns
in rmi_suspend()?
Low-level HID transport drivers such as i2c-hid and usbhid invoke
hid_driver_suspend() before disabling hardware interrupts or stopping input
URBs, and without holding hdev->driver_input_lock:
CPU0 CPU1
rmi_event()
!test_bit(RMI_SUSPENDED, ...)
rmi_suspend()
set_bit(RMI_SUSPENDED, ...)
cancel_work_sync(&data->reset_work)
schedule_work(&data->reset_work)
rmi_driver_suspend()
kworker
rmi_reset_work()
rmi_reset_attn_mode() <-- runs while
suspended
Neither rmi_reset_work() nor rmi_reset_attn_mode() checks RMI_SUSPENDED, so
reset_work will issue bus transfers and reconfigure RMI functions concurrently
with rmi_driver_suspend() or while the device is suspended. Using
disable_work_sync() in rmi_suspend() paired with enable_work() on error and in
rmi_post_resume() would close this race.
> ret = rmi_driver_suspend(rmi_dev, false);
> if (ret) {
> hid_warn(hdev, "Failed to suspend device: %d\n", ret);
> + clear_bit(RMI_SUSPENDED, &data->flags);
> return ret;
> }
>
> @@ -486,27 +493,31 @@ static int rmi_post_resume(struct hid_device *hdev)
> struct rmi_data *data = hid_get_drvdata(hdev);
> struct rmi_device *rmi_dev = data->xport.rmi_dev;
> int ret;
> + int err;
>
> - if (!(data->device_flags & RMI_DEVICE))
> + if (!test_bit(RMI_STARTED, &data->flags))
> return 0;
>
> /* Make sure the HID device is ready to receive events */
> ret = hid_hw_open(hdev);
> if (ret)
> - return ret;
> + goto out;
>
> ret = rmi_reset_attn_mode(hdev);
> if (ret)
> - goto out;
> + hid_warn(hdev, "Failed to reset attention mode: %d\n", ret);
>
> - ret = rmi_driver_resume(rmi_dev, false);
> - if (ret) {
> - hid_warn(hdev, "Failed to resume device: %d\n", ret);
> - goto out;
> + err = rmi_driver_resume(rmi_dev, false);
> + if (err) {
> + hid_warn(hdev, "Failed to resume device: %d\n", err);
> + if (!ret)
> + ret = err;
> }
>
> -out:
> hid_hw_close(hdev);
> +
> +out:
> + clear_bit(RMI_SUSPENDED, &data->flags);
> return ret;
> }
[Severity: Medium]
Does jumping to out when hid_hw_open() fails leave data->rmi_irq disabled?
If hid_hw_open() returns an error (for example, -EINTR from
mutex_lock_killable(&hdev->ll_open_lock)), jumping to out clears RMI_SUSPENDED
without calling rmi_driver_resume(). Because rmi_suspend() disabled the
transport interrupt via rmi_driver_suspend() -> rmi_disable_irq(), the device
is left active after resume with data->rmi_irq permanently disabled, dropping
all subsequent attention reports.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261113-hid-rmi-v1-0-afc43d97d766@gmail.com?part=4
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 05/12] HID: rmi: re-select page 0 across device resets
2026-11-14 5:59 [PATCH 00/12] HID: rmi: Fix report handling, device lifecycle, and clean up code Dmitry Torokhov
` (3 preceding siblings ...)
2026-11-14 5:59 ` [PATCH 04/12] HID: rmi: fix device lifecycle, suspend, and reset work handling Dmitry Torokhov
@ 2026-11-14 5:59 ` Dmitry Torokhov
2026-11-14 5:59 ` [PATCH 06/12] HID: rmi: avoid GFP_ATOMIC allocation with hard IRQs disabled Dmitry Torokhov
` (6 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2026-11-14 5:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Andrew Duggan; +Cc: linux-input, linux-kernel
Hardware resets its page select register to page 0 across resets and
power transitions while the driver cache still retains the previous
non-zero page, causing subsequent register accesses to address the wrong
page.
Explicitly re-select page 0 when resetting device attention mode.
Fixes: 9fb6bf02e3ad ("HID: rmi: introduce RMI driver for Synaptics touchpads")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-rmi.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index 8783edb7fd6a..d3e4ee0f62f0 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -321,6 +321,12 @@ static int rmi_reset_attn_mode(struct hid_device *hdev)
if (ret)
return ret;
+ mutex_lock(&data->page_mutex);
+ ret = rmi_set_page(hdev, 0);
+ mutex_unlock(&data->page_mutex);
+ if (ret)
+ return ret;
+
if (test_bit(RMI_STARTED, &data->flags))
ret = rmi_dev->driver->reset_handler(rmi_dev);
--
2.56.0.679.gc527c65306-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 06/12] HID: rmi: avoid GFP_ATOMIC allocation with hard IRQs disabled
2026-11-14 5:59 [PATCH 00/12] HID: rmi: Fix report handling, device lifecycle, and clean up code Dmitry Torokhov
` (4 preceding siblings ...)
2026-11-14 5:59 ` [PATCH 05/12] HID: rmi: re-select page 0 across device resets Dmitry Torokhov
@ 2026-11-14 5:59 ` Dmitry Torokhov
2026-11-14 5:59 ` [PATCH 07/12] HID: rmi: do not mutate shared platform data template Dmitry Torokhov
` (5 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2026-11-14 5:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Andrew Duggan; +Cc: linux-input, linux-kernel
Storing incoming attention data allocates memory with GFP_ATOMIC while
hard interrupts are disabled, triggering sleeping lock warnings on
real-time configurations.
Stage attention data before disabling interrupts and use the safe
generic interrupt handler helper.
Fixes: 0b2c7a897378 ("HID: rmi: Make hid-rmi a transport driver for synaptics-rmi4")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-rmi.c | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index d3e4ee0f62f0..a4406bc6af43 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -362,7 +362,6 @@ static int rmi_input_event(struct hid_device *hdev, u8 *data, int size)
{
struct rmi_data *hdata = hid_get_drvdata(hdev);
struct rmi_device *rmi_dev = hdata->xport.rmi_dev;
- unsigned long flags;
if (!(test_bit(RMI_STARTED, &hdata->flags)))
return 0;
@@ -373,13 +372,9 @@ static int rmi_input_event(struct hid_device *hdev, u8 *data, int size)
pm_wakeup_event(hdev->dev.parent, 0);
- local_irq_save(flags);
-
rmi_set_attn_data(rmi_dev, data[1], &data[2], size - 2);
- generic_handle_irq(hdata->rmi_irq);
-
- local_irq_restore(flags);
+ generic_handle_irq_safe(hdata->rmi_irq);
return 1;
}
--
2.56.0.679.gc527c65306-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 07/12] HID: rmi: do not mutate shared platform data template
2026-11-14 5:59 [PATCH 00/12] HID: rmi: Fix report handling, device lifecycle, and clean up code Dmitry Torokhov
` (5 preceding siblings ...)
2026-11-14 5:59 ` [PATCH 06/12] HID: rmi: avoid GFP_ATOMIC allocation with hard IRQs disabled Dmitry Torokhov
@ 2026-11-14 5:59 ` Dmitry Torokhov
2026-11-14 5:59 ` [PATCH 08/12] HID: rmi: use put_unaligned_le16() to populate output reports Dmitry Torokhov
` (4 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2026-11-14 5:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Andrew Duggan; +Cc: linux-input, linux-kernel
Probing a device with physical buttons modifies the static platform data
template directly, altering GPIO configuration for any devices probed
afterward.
Make the shared template read-only and adjust the per-device platform
data copy instead.
Fixes: 0b2c7a897378 ("HID: rmi: Make hid-rmi a transport driver for synaptics-rmi4")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-rmi.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index a4406bc6af43..0a5d32dbe881 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -613,7 +613,7 @@ static int rmi_check_valid_report_id(struct hid_device *hdev, unsigned type,
return 0;
}
-static struct rmi_device_platform_data rmi_hid_pdata = {
+static const struct rmi_device_platform_data rmi_hid_pdata = {
.sensor_pdata = {
.sensor_type = rmi_sensor_touchpad,
.axis_align.flip_y = true,
@@ -776,12 +776,12 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
return ret;
}
- if (data->device_flags & RMI_DEVICE_HAS_PHYS_BUTTONS)
- rmi_hid_pdata.gpio_data.disable = true;
-
data->xport.dev = hdev->dev.parent;
data->xport.pdata = rmi_hid_pdata;
data->xport.pdata.irq = data->rmi_irq;
+ if (data->device_flags & RMI_DEVICE_HAS_PHYS_BUTTONS)
+ data->xport.pdata.gpio_data.disable = true;
+
data->xport.proto_name = "hid";
data->xport.ops = &hid_rmi_ops;
--
2.56.0.679.gc527c65306-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 08/12] HID: rmi: use put_unaligned_le16() to populate output reports
2026-11-14 5:59 [PATCH 00/12] HID: rmi: Fix report handling, device lifecycle, and clean up code Dmitry Torokhov
` (6 preceding siblings ...)
2026-11-14 5:59 ` [PATCH 07/12] HID: rmi: do not mutate shared platform data template Dmitry Torokhov
@ 2026-11-14 5:59 ` Dmitry Torokhov
2026-11-14 5:59 ` [PATCH 09/12] HID: rmi: use scope-based cleanup for mode feature buffer Dmitry Torokhov
` (3 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2026-11-14 5:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Andrew Duggan; +Cc: linux-input, linux-kernel
Open-coded byte shifts for 16-bit integers are verbose and left the
upper page address byte uninitialized when reusing the output buffer.
Use put_unaligned_le16() when packing 16-bit fields into output reports.
Fixes: 9fb6bf02e3ad ("HID: rmi: introduce RMI driver for Synaptics touchpads")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-rmi.c | 12 +++++-------
1 file changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index 0a5d32dbe881..ad2ab6c13550 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -15,6 +15,7 @@
#include <linux/module.h>
#include <linux/pm.h>
#include <linux/slab.h>
+#include <linux/unaligned.h>
#include <linux/wait.h>
#include <linux/sched.h>
#include <linux/rmi.h>
@@ -126,7 +127,7 @@ static int rmi_set_page(struct hid_device *hdev, u8 page)
data->writeReport[0] = RMI_WRITE_REPORT_ID;
data->writeReport[1] = 1;
- data->writeReport[2] = 0xFF;
+ put_unaligned_le16(0x00ff, &data->writeReport[2]);
data->writeReport[4] = page;
retval = rmi_write_report(hdev, data->writeReport,
@@ -217,10 +218,8 @@ static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
for (retries = 5; retries > 0; retries--) {
data->writeReport[0] = RMI_READ_ADDR_REPORT_ID;
data->writeReport[1] = 0; /* old 1 byte read count */
- data->writeReport[2] = addr & 0xFF;
- data->writeReport[3] = (addr >> 8) & 0xFF;
- data->writeReport[4] = len & 0xFF;
- data->writeReport[5] = (len >> 8) & 0xFF;
+ put_unaligned_le16(addr, &data->writeReport[2]);
+ put_unaligned_le16(len, &data->writeReport[4]);
set_bit(RMI_READ_REQUEST_PENDING, &data->flags);
@@ -293,8 +292,7 @@ static int rmi_hid_write_block(struct rmi_transport_dev *xport, u16 addr,
data->writeReport[0] = RMI_WRITE_REPORT_ID;
data->writeReport[1] = len;
- data->writeReport[2] = addr & 0xFF;
- data->writeReport[3] = (addr >> 8) & 0xFF;
+ put_unaligned_le16(addr, &data->writeReport[2]);
memcpy(&data->writeReport[4], buf, len);
ret = rmi_write_report(hdev, data->writeReport,
--
2.56.0.679.gc527c65306-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 09/12] HID: rmi: use scope-based cleanup for mode feature buffer
2026-11-14 5:59 [PATCH 00/12] HID: rmi: Fix report handling, device lifecycle, and clean up code Dmitry Torokhov
` (7 preceding siblings ...)
2026-11-14 5:59 ` [PATCH 08/12] HID: rmi: use put_unaligned_le16() to populate output reports Dmitry Torokhov
@ 2026-11-14 5:59 ` Dmitry Torokhov
2026-11-14 6:00 ` [PATCH 10/12] HID: rmi: switch to scoped mutex guards Dmitry Torokhov
` (2 subsequent siblings)
11 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2026-11-14 5:59 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Andrew Duggan; +Cc: linux-input, linux-kernel
Manual memory allocation and release adds boilerplate and complicates
unwinding on error.
Switch to automatic scope-based memory cleanup for the temporary mode
configuration buffer.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-rmi.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index ad2ab6c13550..f1d52d55fe0d 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -144,17 +144,15 @@ static int rmi_set_page(struct hid_device *hdev, u8 page)
static int rmi_set_mode(struct hid_device *hdev, u8 mode)
{
- int ret;
const u8 txbuf[2] = {RMI_SET_RMI_MODE_REPORT_ID, mode};
- u8 *buf;
+ int ret;
- buf = kmemdup(txbuf, sizeof(txbuf), GFP_KERNEL);
+ u8 *buf __free(kfree) = kmemdup(txbuf, sizeof(txbuf), GFP_KERNEL);
if (!buf)
return -ENOMEM;
ret = hid_hw_raw_request(hdev, RMI_SET_RMI_MODE_REPORT_ID, buf,
sizeof(txbuf), HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
- kfree(buf);
if (ret < 0) {
dev_err(&hdev->dev, "unable to set rmi mode to %d (%d)\n", mode,
ret);
--
2.56.0.679.gc527c65306-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 10/12] HID: rmi: switch to scoped mutex guards
2026-11-14 5:59 [PATCH 00/12] HID: rmi: Fix report handling, device lifecycle, and clean up code Dmitry Torokhov
` (8 preceding siblings ...)
2026-11-14 5:59 ` [PATCH 09/12] HID: rmi: use scope-based cleanup for mode feature buffer Dmitry Torokhov
@ 2026-11-14 6:00 ` Dmitry Torokhov
2026-11-14 6:00 ` [PATCH 11/12] HID: rmi: remove dead code and reorder report write helper Dmitry Torokhov
2026-11-14 6:00 ` [PATCH 12/12] HID: rmi: clean up coding style and logging Dmitry Torokhov
11 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2026-11-14 6:00 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Andrew Duggan; +Cc: linux-input, linux-kernel
Manual mutex acquisition and release complicates error handling and
early exits.
Convert page mutex locking to automatic scope-based mutex guards.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-rmi.c | 30 +++++++++++++-----------------
1 file changed, 13 insertions(+), 17 deletions(-)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index f1d52d55fe0d..ec0d18cb06f6 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -205,12 +205,12 @@ static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
if (len > U16_MAX)
return -EINVAL;
- mutex_lock(&data->page_mutex);
+ guard(mutex)(&data->page_mutex);
if (RMI_PAGE(addr) != data->page) {
ret = rmi_set_page(hdev, RMI_PAGE(addr));
- if (ret < 0)
- goto exit;
+ if (ret)
+ return ret;
}
for (retries = 5; retries > 0; retries--) {
@@ -227,7 +227,7 @@ static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
dev_err(&hdev->dev,
"failed to write request output report (%d)\n",
ret);
- goto exit;
+ break;
}
bytes_read = 0;
@@ -264,9 +264,7 @@ static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
}
}
-exit:
clear_bit(RMI_READ_REQUEST_PENDING, &data->flags);
- mutex_unlock(&data->page_mutex);
return ret;
}
@@ -280,12 +278,12 @@ static int rmi_hid_write_block(struct rmi_transport_dev *xport, u16 addr,
if (len > data->output_report_size - 4 || len > U8_MAX)
return -EINVAL;
- mutex_lock(&data->page_mutex);
+ guard(mutex)(&data->page_mutex);
if (RMI_PAGE(addr) != data->page) {
ret = rmi_set_page(hdev, RMI_PAGE(addr));
if (ret < 0)
- goto exit;
+ return ret;
}
data->writeReport[0] = RMI_WRITE_REPORT_ID;
@@ -299,12 +297,10 @@ static int rmi_hid_write_block(struct rmi_transport_dev *xport, u16 addr,
dev_err(&hdev->dev,
"failed to write request output report (%d)\n",
ret);
- goto exit;
+ return ret;
}
-exit:
- mutex_unlock(&data->page_mutex);
- return ret;
+ return 0;
}
static int rmi_reset_attn_mode(struct hid_device *hdev)
@@ -317,11 +313,11 @@ static int rmi_reset_attn_mode(struct hid_device *hdev)
if (ret)
return ret;
- mutex_lock(&data->page_mutex);
- ret = rmi_set_page(hdev, 0);
- mutex_unlock(&data->page_mutex);
- if (ret)
- return ret;
+ scoped_guard(mutex, &data->page_mutex) {
+ ret = rmi_set_page(hdev, 0);
+ if (ret)
+ return ret;
+ }
if (test_bit(RMI_STARTED, &data->flags))
ret = rmi_dev->driver->reset_handler(rmi_dev);
--
2.56.0.679.gc527c65306-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 11/12] HID: rmi: remove dead code and reorder report write helper
2026-11-14 5:59 [PATCH 00/12] HID: rmi: Fix report handling, device lifecycle, and clean up code Dmitry Torokhov
` (9 preceding siblings ...)
2026-11-14 6:00 ` [PATCH 10/12] HID: rmi: switch to scoped mutex guards Dmitry Torokhov
@ 2026-11-14 6:00 ` Dmitry Torokhov
2026-11-14 6:00 ` [PATCH 12/12] HID: rmi: clean up coding style and logging Dmitry Torokhov
11 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2026-11-14 6:00 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Andrew Duggan; +Cc: linux-input, linux-kernel
Leftover unused definitions, unreachable control flow, and an
unnecessary forward declaration clutter the driver.
Drop the dead code, remove the redundant pointer cast, and move the
report write helper ahead of its callers to eliminate its forward
declaration.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-rmi.c | 67 +++++++++++++++++++++------------------------------
1 file changed, 28 insertions(+), 39 deletions(-)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index ec0d18cb06f6..d5961d408e72 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -39,13 +39,6 @@
#define RMI_DEVICE_HAS_PHYS_BUTTONS BIT(1)
#define RMI_DEVICE_OUTPUT_SET_REPORT BIT(2)
-/*
- * retrieve the ctrl registers
- * the ctrl register has a size of 20 but a fw bug split it into 16 + 4,
- * and there is no way to know if the first 20 bytes are here or not.
- * We use only the first 12 bytes, so get only them.
- */
-#define RMI_F11_CTRL_REG_COUNT 12
enum rmi_mode_type {
RMI_MODE_OFF = 0,
@@ -104,7 +97,34 @@ struct rmi_data {
#define RMI_PAGE(addr) (((addr) >> 8) & 0xff)
-static int rmi_write_report(struct hid_device *hdev, u8 *report, int len);
+static int rmi_write_report(struct hid_device *hdev, u8 *report, int len)
+{
+ struct rmi_data *data = hid_get_drvdata(hdev);
+ int ret;
+
+ if (data->device_flags & RMI_DEVICE_OUTPUT_SET_REPORT) {
+ /*
+ * Talk to device by using SET_REPORT requests instead.
+ */
+ ret = hid_hw_raw_request(hdev, report[0], report,
+ len, HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+ } else {
+ ret = hid_hw_output_report(hdev, report, len);
+ }
+
+ if (ret < 0) {
+ dev_err(&hdev->dev, "failed to write hid report (%d)\n", ret);
+ return ret;
+ }
+
+ if (ret != len) {
+ dev_err(&hdev->dev, "short write to hid device (%d < %d)\n",
+ ret, len);
+ return -EIO;
+ }
+
+ return 0;
+}
/**
* rmi_set_page - Set RMI page
@@ -162,35 +182,6 @@ static int rmi_set_mode(struct hid_device *hdev, u8 mode)
return 0;
}
-static int rmi_write_report(struct hid_device *hdev, u8 *report, int len)
-{
- struct rmi_data *data = hid_get_drvdata(hdev);
- int ret;
-
- if (data->device_flags & RMI_DEVICE_OUTPUT_SET_REPORT) {
- /*
- * Talk to device by using SET_REPORT requests instead.
- */
- ret = hid_hw_raw_request(hdev, report[0], report,
- len, HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
- } else {
- ret = hid_hw_output_report(hdev, (void *)report, len);
- }
-
- if (ret < 0) {
- dev_err(&hdev->dev, "failed to write hid report (%d)\n", ret);
- return ret;
- }
-
- if (ret != len) {
- dev_err(&hdev->dev, "short write to hid device (%d < %d)\n",
- ret, len);
- return -EIO;
- }
-
- return 0;
-}
-
static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
void *buf, size_t len)
{
@@ -412,8 +403,6 @@ static int rmi_raw_event(struct hid_device *hdev,
default:
return 1;
}
-
- return 0;
}
static int rmi_event(struct hid_device *hdev, struct hid_field *field,
--
2.56.0.679.gc527c65306-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread* [PATCH 12/12] HID: rmi: clean up coding style and logging
2026-11-14 5:59 [PATCH 00/12] HID: rmi: Fix report handling, device lifecycle, and clean up code Dmitry Torokhov
` (10 preceding siblings ...)
2026-11-14 6:00 ` [PATCH 11/12] HID: rmi: remove dead code and reorder report write helper Dmitry Torokhov
@ 2026-11-14 6:00 ` Dmitry Torokhov
11 siblings, 0 replies; 14+ messages in thread
From: Dmitry Torokhov @ 2026-11-14 6:00 UTC (permalink / raw)
To: Jiri Kosina, Benjamin Tissoires, Andrew Duggan; +Cc: linux-input, linux-kernel
Inconsistent naming, mixed status return conventions, non-uniform error
logging, and comment typos reduce code clarity.
Standardize buffer names, use standard negative error code conventions,
switch to subsystem-specific error logging, and fix comment typos and
checkpatch formatting issues.
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
drivers/hid/hid-rmi.c | 310 +++++++++++++++++++++++++-------------------------
1 file changed, 155 insertions(+), 155 deletions(-)
diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index d5961d408e72..3403c4b90d87 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -39,7 +39,6 @@
#define RMI_DEVICE_HAS_PHYS_BUTTONS BIT(1)
#define RMI_DEVICE_OUTPUT_SET_REPORT BIT(2)
-
enum rmi_mode_type {
RMI_MODE_OFF = 0,
RMI_MODE_ATTN_REPORTS = 1,
@@ -55,8 +54,8 @@ enum rmi_mode_type {
*
* @wait: Used for waiting for read data
*
- * @writeReport: output buffer when writing RMI registers
- * @readReport: input buffer when reading RMI registers
+ * @tx_buf: output buffer when writing RMI registers
+ * @rx_buf: input buffer when reading RMI registers
*
* @input_report_size: size of an input report (advertised by HID)
* @output_report_size: size of an output report (advertised by HID)
@@ -72,14 +71,14 @@ enum rmi_mode_type {
* @rmi_irq: the irq that will be used to generate events to rmi-core
*/
struct rmi_data {
- struct mutex page_mutex;
+ struct mutex page_mutex; /* locks current virtual page */
int page;
struct rmi_transport_dev xport;
wait_queue_head_t wait;
- u8 *writeReport;
- u8 *readReport;
+ u8 *tx_buf;
+ u8 *rx_buf;
u32 input_report_size;
u32 output_report_size;
@@ -100,26 +99,27 @@ struct rmi_data {
static int rmi_write_report(struct hid_device *hdev, u8 *report, int len)
{
struct rmi_data *data = hid_get_drvdata(hdev);
- int ret;
+ int error;
if (data->device_flags & RMI_DEVICE_OUTPUT_SET_REPORT) {
/*
* Talk to device by using SET_REPORT requests instead.
*/
- ret = hid_hw_raw_request(hdev, report[0], report,
- len, HID_OUTPUT_REPORT, HID_REQ_SET_REPORT);
+ error = hid_hw_raw_request(hdev, report[0], report,
+ len, HID_OUTPUT_REPORT,
+ HID_REQ_SET_REPORT);
} else {
- ret = hid_hw_output_report(hdev, report, len);
+ error = hid_hw_output_report(hdev, report, len);
}
- if (ret < 0) {
- dev_err(&hdev->dev, "failed to write hid report (%d)\n", ret);
- return ret;
+ if (error < 0) {
+ hid_err(hdev, "failed to write hid report (%d)\n", error);
+ return error;
}
- if (ret != len) {
- dev_err(&hdev->dev, "short write to hid device (%d < %d)\n",
- ret, len);
+ if (error != len) {
+ hid_err(hdev, "short write to hid device (%d < %d)\n",
+ error, len);
return -EIO;
}
@@ -143,19 +143,18 @@ static int rmi_write_report(struct hid_device *hdev, u8 *report, int len)
static int rmi_set_page(struct hid_device *hdev, u8 page)
{
struct rmi_data *data = hid_get_drvdata(hdev);
- int retval;
-
- data->writeReport[0] = RMI_WRITE_REPORT_ID;
- data->writeReport[1] = 1;
- put_unaligned_le16(0x00ff, &data->writeReport[2]);
- data->writeReport[4] = page;
-
- retval = rmi_write_report(hdev, data->writeReport,
- data->output_report_size);
- if (retval) {
- dev_err(&hdev->dev,
- "%s: set page failed: %d.", __func__, retval);
- return retval;
+ int error;
+
+ data->tx_buf[0] = RMI_WRITE_REPORT_ID;
+ data->tx_buf[1] = 1;
+ put_unaligned_le16(0x00ff, &data->tx_buf[2]);
+ data->tx_buf[4] = page;
+
+ error = rmi_write_report(hdev, data->tx_buf,
+ data->output_report_size);
+ if (error) {
+ hid_err(hdev, "%s: set page failed: %d\n", __func__, error);
+ return error;
}
data->page = page;
@@ -165,33 +164,34 @@ static int rmi_set_page(struct hid_device *hdev, u8 page)
static int rmi_set_mode(struct hid_device *hdev, u8 mode)
{
const u8 txbuf[2] = {RMI_SET_RMI_MODE_REPORT_ID, mode};
- int ret;
+ int error;
u8 *buf __free(kfree) = kmemdup(txbuf, sizeof(txbuf), GFP_KERNEL);
if (!buf)
return -ENOMEM;
- ret = hid_hw_raw_request(hdev, RMI_SET_RMI_MODE_REPORT_ID, buf,
- sizeof(txbuf), HID_FEATURE_REPORT, HID_REQ_SET_REPORT);
- if (ret < 0) {
- dev_err(&hdev->dev, "unable to set rmi mode to %d (%d)\n", mode,
- ret);
- return ret;
+ error = hid_hw_raw_request(hdev, RMI_SET_RMI_MODE_REPORT_ID, buf,
+ sizeof(txbuf), HID_FEATURE_REPORT,
+ HID_REQ_SET_REPORT);
+ if (error < 0) {
+ hid_err(hdev, "unable to set rmi mode to %d (%d)\n", mode,
+ error);
+ return error;
}
return 0;
}
static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
- void *buf, size_t len)
+ void *buf, size_t len)
{
struct rmi_data *data = container_of(xport, struct rmi_data, xport);
struct hid_device *hdev = data->hdev;
- int ret;
- int retries;
size_t bytes_read;
size_t bytes_needed;
size_t read_input_count;
+ int retries;
+ int error;
if (len > U16_MAX)
return -EINVAL;
@@ -199,25 +199,24 @@ static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
guard(mutex)(&data->page_mutex);
if (RMI_PAGE(addr) != data->page) {
- ret = rmi_set_page(hdev, RMI_PAGE(addr));
- if (ret)
- return ret;
+ error = rmi_set_page(hdev, RMI_PAGE(addr));
+ if (error)
+ return error;
}
for (retries = 5; retries > 0; retries--) {
- data->writeReport[0] = RMI_READ_ADDR_REPORT_ID;
- data->writeReport[1] = 0; /* old 1 byte read count */
- put_unaligned_le16(addr, &data->writeReport[2]);
- put_unaligned_le16(len, &data->writeReport[4]);
+ data->tx_buf[0] = RMI_READ_ADDR_REPORT_ID;
+ data->tx_buf[1] = 0; /* old 1 byte read count */
+ put_unaligned_le16(addr, &data->tx_buf[2]);
+ put_unaligned_le16(len, &data->tx_buf[4]);
set_bit(RMI_READ_REQUEST_PENDING, &data->flags);
- ret = rmi_write_report(hdev, data->writeReport,
- data->output_report_size);
- if (ret) {
- dev_err(&hdev->dev,
- "failed to write request output report (%d)\n",
- ret);
+ error = rmi_write_report(hdev, data->tx_buf,
+ data->output_report_size);
+ if (error) {
+ hid_err(hdev, "failed to write request output report (%d)\n",
+ error);
break;
}
@@ -225,46 +224,45 @@ static int rmi_hid_read_block(struct rmi_transport_dev *xport, u16 addr,
bytes_needed = len;
while (bytes_read < len) {
if (!wait_event_timeout(data->wait,
- test_bit(RMI_READ_DATA_PENDING, &data->flags),
- msecs_to_jiffies(1000))) {
+ test_bit(RMI_READ_DATA_PENDING,
+ &data->flags),
+ msecs_to_jiffies(1000))) {
hid_warn(hdev, "%s: timeout elapsed\n",
__func__);
- ret = -EAGAIN;
+ error = -EAGAIN;
break;
}
- read_input_count = data->readReport[1];
+ read_input_count = data->rx_buf[1];
if (read_input_count == 0 ||
read_input_count > data->input_report_size - 2) {
clear_bit(RMI_READ_DATA_PENDING, &data->flags);
- ret = -EPROTO;
+ error = -EPROTO;
break;
}
- memcpy(buf + bytes_read, &data->readReport[2],
- min(read_input_count, bytes_needed));
+ memcpy(buf + bytes_read, &data->rx_buf[2],
+ min(read_input_count, bytes_needed));
bytes_read += read_input_count;
bytes_needed -= read_input_count;
clear_bit(RMI_READ_DATA_PENDING, &data->flags);
}
- if (ret >= 0) {
- ret = 0;
+ if (!error)
break;
- }
}
clear_bit(RMI_READ_REQUEST_PENDING, &data->flags);
- return ret;
+ return error;
}
static int rmi_hid_write_block(struct rmi_transport_dev *xport, u16 addr,
- const void *buf, size_t len)
+ const void *buf, size_t len)
{
struct rmi_data *data = container_of(xport, struct rmi_data, xport);
struct hid_device *hdev = data->hdev;
- int ret;
+ int error;
if (len > data->output_report_size - 4 || len > U8_MAX)
return -EINVAL;
@@ -272,23 +270,23 @@ static int rmi_hid_write_block(struct rmi_transport_dev *xport, u16 addr,
guard(mutex)(&data->page_mutex);
if (RMI_PAGE(addr) != data->page) {
- ret = rmi_set_page(hdev, RMI_PAGE(addr));
- if (ret < 0)
- return ret;
+ error = rmi_set_page(hdev, RMI_PAGE(addr));
+ if (error)
+ return error;
}
- data->writeReport[0] = RMI_WRITE_REPORT_ID;
- data->writeReport[1] = len;
- put_unaligned_le16(addr, &data->writeReport[2]);
- memcpy(&data->writeReport[4], buf, len);
+ data->tx_buf[0] = RMI_WRITE_REPORT_ID;
+ data->tx_buf[1] = len;
+ put_unaligned_le16(addr, &data->tx_buf[2]);
+ memcpy(&data->tx_buf[4], buf, len);
- ret = rmi_write_report(hdev, data->writeReport,
- data->output_report_size);
- if (ret) {
- dev_err(&hdev->dev,
+ error = rmi_write_report(hdev, data->tx_buf,
+ data->output_report_size);
+ if (error) {
+ hid_err(hdev,
"failed to write request output report (%d)\n",
- ret);
- return ret;
+ error);
+ return error;
}
return 0;
@@ -298,22 +296,22 @@ static int rmi_reset_attn_mode(struct hid_device *hdev)
{
struct rmi_data *data = hid_get_drvdata(hdev);
struct rmi_device *rmi_dev = data->xport.rmi_dev;
- int ret;
+ int error;
- ret = rmi_set_mode(hdev, RMI_MODE_ATTN_REPORTS);
- if (ret)
- return ret;
+ error = rmi_set_mode(hdev, RMI_MODE_ATTN_REPORTS);
+ if (error)
+ return error;
scoped_guard(mutex, &data->page_mutex) {
- ret = rmi_set_page(hdev, 0);
- if (ret)
- return ret;
+ error = rmi_set_page(hdev, 0);
+ if (error)
+ return error;
}
if (test_bit(RMI_STARTED, &data->flags))
- ret = rmi_dev->driver->reset_handler(rmi_dev);
+ error = rmi_dev->driver->reset_handler(rmi_dev);
- return ret;
+ return error;
}
static void rmi_reset_work(struct work_struct *work)
@@ -374,9 +372,9 @@ static int rmi_read_data_event(struct hid_device *hdev, u8 *data, int size)
copy_size = min_t(size_t, size, hdata->input_report_size);
if (copy_size < 3 || data[1] == 0 || data[1] > copy_size - 2)
- hdata->readReport[1] = 0;
+ hdata->rx_buf[1] = 0;
else
- memcpy(hdata->readReport, data, copy_size);
+ memcpy(hdata->rx_buf, data, copy_size);
set_bit(RMI_READ_DATA_PENDING, &hdata->flags);
wake_up(&hdata->wait);
@@ -385,7 +383,7 @@ static int rmi_read_data_event(struct hid_device *hdev, u8 *data, int size)
}
static int rmi_raw_event(struct hid_device *hdev,
- struct hid_report *report, u8 *data, int size)
+ struct hid_report *report, u8 *data, int size)
{
struct rmi_data *hdata = hid_get_drvdata(hdev);
@@ -406,7 +404,7 @@ static int rmi_raw_event(struct hid_device *hdev,
}
static int rmi_event(struct hid_device *hdev, struct hid_field *field,
- struct hid_usage *usage, __s32 value)
+ struct hid_usage *usage, __s32 value)
{
struct rmi_data *data = hid_get_drvdata(hdev);
@@ -414,13 +412,13 @@ static int rmi_event(struct hid_device *hdev, struct hid_field *field,
test_bit(RMI_STARTED, &data->flags) &&
!test_bit(RMI_SUSPENDED, &data->flags) &&
(field->application == HID_GD_POINTER ||
- field->application == HID_GD_MOUSE)) {
+ field->application == HID_GD_MOUSE)) {
if (data->device_flags & RMI_DEVICE_HAS_PHYS_BUTTONS) {
if ((usage->hid & HID_USAGE_PAGE) == HID_UP_BUTTON)
return 0;
- if ((usage->hid == HID_GD_X || usage->hid == HID_GD_Y)
- && !value)
+ if ((usage->hid == HID_GD_X || usage->hid == HID_GD_Y) &&
+ !value)
return 1;
}
@@ -452,7 +450,7 @@ static int rmi_suspend(struct hid_device *hdev, pm_message_t message)
{
struct rmi_data *data = hid_get_drvdata(hdev);
struct rmi_device *rmi_dev = data->xport.rmi_dev;
- int ret;
+ int error;
if (!test_bit(RMI_STARTED, &data->flags))
return 0;
@@ -460,11 +458,11 @@ static int rmi_suspend(struct hid_device *hdev, pm_message_t message)
set_bit(RMI_SUSPENDED, &data->flags);
cancel_work_sync(&data->reset_work);
- ret = rmi_driver_suspend(rmi_dev, false);
- if (ret) {
- hid_warn(hdev, "Failed to suspend device: %d\n", ret);
+ error = rmi_driver_suspend(rmi_dev, false);
+ if (error) {
+ hid_warn(hdev, "Failed to suspend device: %d\n", error);
clear_bit(RMI_SUSPENDED, &data->flags);
- return ret;
+ return error;
}
return 0;
@@ -474,33 +472,33 @@ static int rmi_post_resume(struct hid_device *hdev)
{
struct rmi_data *data = hid_get_drvdata(hdev);
struct rmi_device *rmi_dev = data->xport.rmi_dev;
- int ret;
+ int error;
int err;
if (!test_bit(RMI_STARTED, &data->flags))
return 0;
/* Make sure the HID device is ready to receive events */
- ret = hid_hw_open(hdev);
- if (ret)
+ error = hid_hw_open(hdev);
+ if (error)
goto out;
- ret = rmi_reset_attn_mode(hdev);
- if (ret)
- hid_warn(hdev, "Failed to reset attention mode: %d\n", ret);
+ error = rmi_reset_attn_mode(hdev);
+ if (error)
+ hid_warn(hdev, "Failed to reset attention mode: %d\n", error);
err = rmi_driver_resume(rmi_dev, false);
if (err) {
hid_warn(hdev, "Failed to resume device: %d\n", err);
- if (!ret)
- ret = err;
+ if (!error)
+ error = err;
}
hid_hw_close(hdev);
out:
clear_bit(RMI_SUSPENDED, &data->flags);
- return ret;
+ return error;
}
static int rmi_hid_reset(struct rmi_transport_dev *xport, u16 reset_addr)
@@ -515,7 +513,7 @@ static int rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
{
struct rmi_data *data = hid_get_drvdata(hdev);
struct input_dev *input = hi->input;
- int ret = 0;
+ int error = 0;
if (!(data->device_flags & RMI_DEVICE))
return 0;
@@ -523,28 +521,28 @@ static int rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
data->xport.input = input;
hid_dbg(hdev, "Opening low level driver\n");
- ret = hid_hw_open(hdev);
- if (ret)
- return ret;
+ error = hid_hw_open(hdev);
+ if (error)
+ return error;
/* Allow incoming hid reports */
hid_device_io_start(hdev);
- ret = rmi_set_mode(hdev, RMI_MODE_ATTN_REPORTS);
- if (ret < 0) {
- dev_err(&hdev->dev, "failed to set rmi mode\n");
+ error = rmi_set_mode(hdev, RMI_MODE_ATTN_REPORTS);
+ if (error) {
+ hid_err(hdev, "failed to set rmi mode\n");
goto exit;
}
- ret = rmi_set_page(hdev, 0);
- if (ret < 0) {
- dev_err(&hdev->dev, "failed to set page select to 0.\n");
+ error = rmi_set_page(hdev, 0);
+ if (error) {
+ hid_err(hdev, "failed to set page select to 0\n");
goto exit;
}
- ret = rmi_register_transport_device(&data->xport);
- if (ret < 0) {
- dev_err(&hdev->dev, "failed to register transport driver\n");
+ error = rmi_register_transport_device(&data->xport);
+ if (error) {
+ hid_err(hdev, "failed to register transport driver\n");
goto exit;
}
@@ -553,18 +551,19 @@ static int rmi_input_configured(struct hid_device *hdev, struct hid_input *hi)
exit:
hid_device_io_stop(hdev);
hid_hw_close(hdev);
- return ret;
+ return error;
}
static int rmi_input_mapping(struct hid_device *hdev,
- struct hid_input *hi, struct hid_field *field,
- struct hid_usage *usage, unsigned long **bit, int *max)
+ struct hid_input *hi, struct hid_field *field,
+ struct hid_usage *usage, unsigned long **bit,
+ int *max)
{
struct rmi_data *data = hid_get_drvdata(hdev);
/*
* we want to make HID ignore the advertised HID collection
- * for RMI deivces
+ * for RMI devices
*/
if (data->device_flags & RMI_DEVICE) {
if ((data->device_flags & RMI_DEVICE_HAS_PHYS_BUTTONS) &&
@@ -577,15 +576,17 @@ static int rmi_input_mapping(struct hid_device *hdev,
return 0;
}
-static int rmi_check_valid_report_id(struct hid_device *hdev, unsigned type,
- unsigned id, struct hid_report **report)
+static int rmi_check_valid_report_id(struct hid_device *hdev,
+ unsigned int type, unsigned int id,
+ struct hid_report **report)
{
int i;
*report = hdev->report_enum[type].report_id_hash[id];
if (*report) {
for (i = 0; i < (*report)->maxfield; i++) {
- unsigned app = (*report)->field[i]->application;
+ unsigned int app = (*report)->field[i]->application;
+
if ((app & HID_USAGE_PAGE) >= HID_UP_MSVENDOR)
return 1;
}
@@ -639,16 +640,16 @@ static const struct irq_domain_ops rmi_irq_ops = {
static int rmi_setup_irq_domain(struct hid_device *hdev)
{
struct rmi_data *hdata = hid_get_drvdata(hdev);
- int ret;
+ int error;
hdata->domain = irq_domain_create_linear(hdev->dev.fwnode, 1,
&rmi_irq_ops, hdata);
if (!hdata->domain)
return -ENOMEM;
- ret = devm_add_action_or_reset(&hdev->dev, &rmi_irq_teardown, hdata);
- if (ret)
- return ret;
+ error = devm_add_action_or_reset(&hdev->dev, &rmi_irq_teardown, hdata);
+ if (error)
+ return error;
hdata->rmi_irq = irq_create_mapping(hdata->domain, 0);
if (hdata->rmi_irq <= 0) {
@@ -661,14 +662,14 @@ static int rmi_setup_irq_domain(struct hid_device *hdev)
static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
{
- struct rmi_data *data = NULL;
- int ret;
+ struct rmi_data *data;
+ int error;
size_t alloc_size;
struct hid_report *input_report;
struct hid_report *output_report;
struct hid_report *feature_report;
- data = devm_kzalloc_obj(&hdev->dev, struct rmi_data);
+ data = devm_kzalloc_obj(&hdev->dev, *data);
if (!data)
return -ENOMEM;
@@ -680,35 +681,36 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
hdev->quirks |= HID_QUIRK_NO_INIT_REPORTS;
hdev->quirks |= HID_QUIRK_NO_INPUT_SYNC;
- ret = hid_parse(hdev);
- if (ret) {
+ error = hid_parse(hdev);
+ if (error) {
hid_err(hdev, "parse failed\n");
- return ret;
+ return error;
}
/*
* RMI_DEVICE can only mean "this probe validated the RMI reports and
- * allocated writeReport": every bail-out to start below skips that
+ * allocated tx_buf": every bail-out to start below skips that
* allocation, and device_flags left carrying RMI_DEVICE from
* driver_data would send rmi_input_configured() into rmi_set_page()
- * with writeReport still NULL. A bind through the new_id sysfs
+ * with tx_buf still NULL. A bind through the new_id sysfs
* attribute can supply driver_data with the bit set, so do not let
* driver_data grant it.
*/
data->device_flags = id->driver_data & ~RMI_DEVICE;
/*
- * Check for the RMI specific report ids. If they are misisng
+ * Check for the RMI specific report ids. If they are missing
* simply return and let the events be processed by hid-input
*/
if (!rmi_check_valid_report_id(hdev, HID_FEATURE_REPORT,
- RMI_SET_RMI_MODE_REPORT_ID, &feature_report)) {
+ RMI_SET_RMI_MODE_REPORT_ID,
+ &feature_report)) {
hid_dbg(hdev, "device does not have set mode feature report\n");
goto start;
}
if (!rmi_check_valid_report_id(hdev, HID_INPUT_REPORT,
- RMI_ATTN_REPORT_ID, &input_report)) {
+ RMI_ATTN_REPORT_ID, &input_report)) {
hid_dbg(hdev, "device does not have attention input report\n");
goto start;
}
@@ -722,7 +724,7 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
data->input_report_size = hid_report_len(input_report);
if (!rmi_check_valid_report_id(hdev, HID_OUTPUT_REPORT,
- RMI_WRITE_REPORT_ID, &output_report)) {
+ RMI_WRITE_REPORT_ID, &output_report)) {
hid_dbg(hdev,
"device does not have rmi write output report\n");
goto start;
@@ -739,22 +741,20 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
data->device_flags |= RMI_DEVICE;
alloc_size = data->output_report_size + data->input_report_size;
- data->writeReport = devm_kzalloc(&hdev->dev, alloc_size, GFP_KERNEL);
- if (!data->writeReport) {
- hid_err(hdev, "failed to allocate buffer for HID reports\n");
+ data->tx_buf = devm_kzalloc(&hdev->dev, alloc_size, GFP_KERNEL);
+ if (!data->tx_buf)
return -ENOMEM;
- }
- data->readReport = data->writeReport + data->output_report_size;
+ data->rx_buf = data->tx_buf + data->output_report_size;
init_waitqueue_head(&data->wait);
mutex_init(&data->page_mutex);
- ret = rmi_setup_irq_domain(hdev);
- if (ret) {
+ error = rmi_setup_irq_domain(hdev);
+ if (error) {
hid_err(hdev, "failed to allocate IRQ domain\n");
- return ret;
+ return error;
}
data->xport.dev = hdev->dev.parent;
@@ -767,10 +767,10 @@ static int rmi_probe(struct hid_device *hdev, const struct hid_device_id *id)
data->xport.ops = &hid_rmi_ops;
start:
- ret = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
- if (ret) {
+ error = hid_hw_start(hdev, HID_CONNECT_DEFAULT);
+ if (error) {
hid_err(hdev, "hw start failed\n");
- return ret;
+ return error;
}
return 0;
--
2.56.0.679.gc527c65306-goog
^ permalink raw reply related [flat|nested] 14+ messages in thread