[PATCH 00/12] HID: rmi: Fix report handling, device lifecycle, and clean up code

[PATCH 00/12] HID: rmi: Fix report handling, device lifecycle, and clean up code

[Dmitry Torokhov]

This series fixes several report handling and device lifecycle bugs in
hid-rmi: it treats short output report writes as errors, limits trailing
0xff stripping to attention reports so register read replies are not
truncated, and validates read data report and transfer length bounds. It
also gates reset work and power management callbacks on device start and
suspend state, re-selects page 0 across resets, avoids GFP_ATOMIC
allocations with hard IRQs disabled, and stops modifying the shared
platform data template during probe.

The rest of the series switches output report packing to unaligned
accessors, uses scope-based cleanup for the mode buffer and page mutex,
drops dead code, and cleans up coding style and logging.

Jiri, Benjamin, could this series go through the input tree once
reviewed, as I have follow-up RMI4 core changes that depend on it? Also,
would it make sense to move hid-rmi to drivers/input/rmi4/ alongside the
rest of the RMI4 transport drivers?

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---
Dmitry Torokhov (12):
      HID: rmi: return error on short output report writes
      HID: rmi: only strip trailing 0xff bytes from attention reports
      HID: rmi: validate read data report and protocol transfer bounds
      HID: rmi: fix device lifecycle, suspend, and reset work handling
      HID: rmi: re-select page 0 across device resets
      HID: rmi: avoid GFP_ATOMIC allocation with hard IRQs disabled
      HID: rmi: do not mutate shared platform data template
      HID: rmi: use put_unaligned_le16() to populate output reports
      HID: rmi: use scope-based cleanup for mode feature buffer
      HID: rmi: switch to scoped mutex guards
      HID: rmi: remove dead code and reorder report write helper
      HID: rmi: clean up coding style and logging

 drivers/hid/hid-rmi.c | 506 +++++++++++++++++++++++++-------------------------
 1 file changed, 252 insertions(+), 254 deletions(-)
---
base-commit: 84abb955b1015477aa11a96e2f3ac1b938b77f48
change-id: 20261021-hid-rmi-b884a25f3ad1

Thanks.

-- 
Dmitry

[PATCH 01/12] HID: rmi: return error on short output report writes

[Dmitry Torokhov]

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

[PATCH 02/12] HID: rmi: only strip trailing 0xff bytes from attention reports

[Dmitry Torokhov]

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

[PATCH 03/12] HID: rmi: validate read data report and protocol transfer bounds

[Dmitry Torokhov]

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

[PATCH 04/12] HID: rmi: fix device lifecycle, suspend, and reset work handling

[Dmitry Torokhov]

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

[PATCH 05/12] HID: rmi: re-select page 0 across device resets

[Dmitry Torokhov]

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

[PATCH 06/12] HID: rmi: avoid GFP_ATOMIC allocation with hard IRQs disabled

[Dmitry Torokhov]

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

[PATCH 07/12] HID: rmi: do not mutate shared platform data template

[Dmitry Torokhov]

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

[PATCH 08/12] HID: rmi: use put_unaligned_le16() to populate output reports

[Dmitry Torokhov]

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

[PATCH 09/12] HID: rmi: use scope-based cleanup for mode feature buffer

[Dmitry Torokhov]

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

[PATCH 10/12] HID: rmi: switch to scoped mutex guards

[Dmitry Torokhov]

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

[PATCH 11/12] HID: rmi: remove dead code and reorder report write helper

[Dmitry Torokhov]

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

[PATCH 12/12] HID: rmi: clean up coding style and logging

[Dmitry Torokhov]

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