[PATCH] HID: rmi: Check for additional ACM registers appended to F11 data report

[PATCH] HID: rmi: Check for additional ACM registers appended to F11 data report

[Andrew Duggan]

If a touchpad reports the F11 data40 register then this indicates that the touchpad reports
additional ACM (Accidental Contact Mitigation) data after the F11 data in the HID attention
report. These additional bytes shift the position of the F30 button data causing the driver
to incorrectly report button state when this functionality is present. This patch accounts
for the additional data in the report.

Fixes:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1398533

Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
---
 drivers/hid/hid-rmi.c | 61 +++++++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 52 insertions(+), 9 deletions(-)

diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index 1f131df..5065583 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -592,11 +592,15 @@ static int rmi_populate_f11(struct hid_device *hdev)
 	bool has_query10 = false;
 	bool has_query11;
 	bool has_query12;
+	bool has_query27;
+	bool has_query28;
+	bool has_query36 = false;
 	bool has_physical_props;
 	bool has_gestures;
 	bool has_rel;
+	bool has_data40 = false;
 	unsigned x_size, y_size;
-	u16 query12_offset;
+	u16 query_offset;
 
 	if (!data->f11.query_base_addr) {
 		hid_err(hdev, "No 2D sensor found, giving up.\n");
@@ -612,6 +616,8 @@ static int rmi_populate_f11(struct hid_device *hdev)
 	has_query9 = !!(buf[0] & BIT(3));
 	has_query11 = !!(buf[0] & BIT(4));
 	has_query12 = !!(buf[0] & BIT(5));
+	has_query27 = !!(buf[0] & BIT(6));
+	has_query28 = !!(buf[0] & BIT(7));
 
 	/* query 1 to get the max number of fingers */
 	ret = rmi_read(hdev, data->f11.query_base_addr + 1, buf);
@@ -650,27 +656,27 @@ static int rmi_populate_f11(struct hid_device *hdev)
 	 * +1 for query 5 which is present since absolute events are
 	 * reported and +1 for query 12.
 	 */
-	query12_offset = 6;
+	query_offset = 6;
 
 	if (has_rel)
-		++query12_offset; /* query 6 is present */
+		++query_offset; /* query 6 is present */
 
 	if (has_gestures)
-		query12_offset += 2; /* query 7 and 8 are present */
+		query_offset += 2; /* query 7 and 8 are present */
 
 	if (has_query9)
-		++query12_offset;
+		++query_offset;
 
 	if (has_query10)
-		++query12_offset;
+		++query_offset;
 
 	if (has_query11)
-		++query12_offset;
+		++query_offset;
 
 	/* query 12 to know if the physical properties are reported */
 	if (has_query12) {
 		ret = rmi_read(hdev, data->f11.query_base_addr
-				+ query12_offset, buf);
+				+ query_offset, buf);
 		if (ret) {
 			hid_err(hdev, "can not get query 12: %d.\n", ret);
 			return ret;
@@ -678,9 +684,10 @@ static int rmi_populate_f11(struct hid_device *hdev)
 		has_physical_props = !!(buf[0] & BIT(5));
 
 		if (has_physical_props) {
+			query_offset += 1;
 			ret = rmi_read_block(hdev,
 					data->f11.query_base_addr
-						+ query12_offset + 1, buf, 4);
+						+ query_offset, buf, 4);
 			if (ret) {
 				hid_err(hdev, "can not read query 15-18: %d.\n",
 					ret);
@@ -695,9 +702,45 @@ static int rmi_populate_f11(struct hid_device *hdev)
 
 			hid_info(hdev, "%s: size in mm: %d x %d\n",
 				 __func__, data->x_size_mm, data->y_size_mm);
+
+			/*
+			 * query 15 - 18 contain the size of the sensor
+			 * and query 19 - 26 contain bezel dimensions
+			 */
+			query_offset += 12;
+		}
+	}
+
+	if (has_query27)
+		++query_offset;
+
+	if (has_query28) {
+		ret = rmi_read(hdev, data->f11.query_base_addr
+				+ query_offset, buf);
+		if (ret) {
+			hid_err(hdev, "can not get query 28: %d.\n", ret);
+			return ret;
+		}
+
+		has_query36 = !!(buf[0] & BIT(6));
+	}
+
+	if (has_query36) {
+		query_offset += 2;
+		ret = rmi_read(hdev, data->f11.query_base_addr
+				+ query_offset, buf);
+		if (ret) {
+			hid_err(hdev, "can not get query 36: %d.\n", ret);
+			return ret;
 		}
+
+		has_data40 = !!(buf[0] & BIT(5));
 	}
 
+
+	if (has_data40)
+		data->f11.report_size += data->max_fingers * 2;
+
 	/*
 	 * retrieve the ctrl registers
 	 * the ctrl register has a size of 20 but a fw bug split it into 16 + 4,
-- 
2.1.0

[PATCH] HID: rmi: The address of query8 must be calculated based on which query registers are present

[Andrew Duggan]

If a touchpad does not report relative data then query 6 will not be present and the address
of query 8 will be one less. This patches calculates the location of query 8 instead of
hardcoding the offset.

Signed-off-by: Andrew Duggan <aduggan@synaptics.com>
---
 drivers/hid/hid-rmi.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/hid/hid-rmi.c b/drivers/hid/hid-rmi.c
index b069369..bc38c1e 100644
--- a/drivers/hid/hid-rmi.c
+++ b/drivers/hid/hid-rmi.c
@@ -640,17 +640,6 @@ static int rmi_populate_f11(struct hid_device *hdev)
 	has_rel = !!(buf[0] & BIT(3));
 	has_gestures = !!(buf[0] & BIT(5));
 
-	if (has_gestures) {
-		/* query 8 to find out if query 10 exists */
-		ret = rmi_read(hdev, data->f11.query_base_addr + 8, buf);
-		if (ret) {
-			hid_err(hdev, "can not read gesture information: %d.\n",
-				ret);
-			return ret;
-		}
-		has_query10 = !!(buf[0] & BIT(2));
-	}
-
 	/*
 	 * At least 4 queries are guaranteed to be present in F11
 	 * +1 for query 5 which is present since absolute events are
@@ -661,8 +650,19 @@ static int rmi_populate_f11(struct hid_device *hdev)
 	if (has_rel)
 		++query_offset; /* query 6 is present */
 
-	if (has_gestures)
+	if (has_gestures) {
+		/* query 8 to find out if query 10 exists */
+		ret = rmi_read(hdev,
+			data->f11.query_base_addr + query_offset + 1, buf);
+		if (ret) {
+			hid_err(hdev, "can not read gesture information: %d.\n",
+				ret);
+			return ret;
+		}
+		has_query10 = !!(buf[0] & BIT(2));
+
 		query_offset += 2; /* query 7 and 8 are present */
+	}
 
 	if (has_query9)
 		++query_offset;
-- 
2.1.0

Re: [PATCH] HID: rmi: The address of query8 must be calculated based on which query registers are present

[Jiri Kosina]

On Mon, 8 Dec 2014, Andrew Duggan wrote:

> If a touchpad does not report relative data then query 6 will not be present and the address
> of query 8 will be one less. This patches calculates the location of query 8 instead of
> hardcoding the offset.
> 
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>

Applied to hid.git#for-3.19/rmi as well.

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] HID: rmi: Check for additional ACM registers appended to F11 data report

[Jiri Kosina]

On Mon, 8 Dec 2014, Andrew Duggan wrote:

> If a touchpad reports the F11 data40 register then this indicates that the touchpad reports
> additional ACM (Accidental Contact Mitigation) data after the F11 data in the HID attention
> report. These additional bytes shift the position of the F30 button data causing the driver
> to incorrectly report button state when this functionality is present. This patch accounts
> for the additional data in the report.
> 
> Fixes:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1398533
> 
> Signed-off-by: Andrew Duggan <aduggan@synaptics.com>

Applied to hid.git#for-3.19/rmi. Thanks,

-- 
Jiri Kosina
SUSE Labs