[PATCH v2 2/9] Input: iqs7222 - fortify slider event reporting

From: Jeff LaBundy <jeff@labundy.com>
To: dmitry.torokhov@gmail.com, robh+dt@kernel.org
Cc: linux-input@vger.kernel.org, devicetree@vger.kernel.org,
	Jeff LaBundy <jeff@labundy.com>
Subject: [PATCH v2 2/9] Input: iqs7222 - fortify slider event reporting
Date: Sun, 26 Jun 2022 02:24:05 -0500	[thread overview]
Message-ID: <20220626072412.475211-3-jeff@labundy.com> (raw)
In-Reply-To: <20220626072412.475211-1-jeff@labundy.com>

The release cycle of any key mapped to a slider gesture relies upon
trailing interrupts generated by other unmasked sources, the timing
and presence of which are inconsistent.

To solve this problem, explicitly report a release cycle to emulate
a full keystroke. Also, unmask touch interrupts if the slider press
event is defined; this ensures the device reports a final interrupt
with coordinate = 0xFFFF once the finger is lifted.

As a result of how the logic has been refactored, the press/release
event can now be mapped to a GPIO. This is more convenient than the
previous solution, which required each channel within the slider to
specify the same GPIO.

As part of this change, use the device's resolution rather than its
number of interrupt status registers to more safely determine if it
is capable of reporting gestures.

Last but not least, make the code a bit simpler by eliminating some
unnecessarily complex conditional statements and a macro that could
be derived using information that is already available.

Fixes: e505edaedcb9 ("Input: add support for Azoteq IQS7222A/B/C")
Signed-off-by: Jeff LaBundy <jeff@labundy.com>
---
Changes in v2:
 - Minor refactoring to include GPIO-mapped press/release event
 - Added support for proximity-mapped press/release event

 drivers/input/misc/iqs7222.c | 96 ++++++++++++++++++++++--------------
 1 file changed, 58 insertions(+), 38 deletions(-)

diff --git a/drivers/input/misc/iqs7222.c b/drivers/input/misc/iqs7222.c
index 53df74f3a982..57616a7ebeae 100644
--- a/drivers/input/misc/iqs7222.c
+++ b/drivers/input/misc/iqs7222.c
@@ -40,7 +40,6 @@
 #define IQS7222_SLDR_SETUP_2_RES_MASK		GENMASK(15, 8)
 #define IQS7222_SLDR_SETUP_2_RES_SHIFT		8
 #define IQS7222_SLDR_SETUP_2_TOP_SPEED_MASK	GENMASK(7, 0)
-#define IQS7222_SLDR_SETUP_3_CHAN_SEL_MASK	GENMASK(9, 0)
 
 #define IQS7222_GPIO_SETUP_0_GPIO_EN		BIT(0)
 
@@ -54,6 +53,9 @@
 #define IQS7222_SYS_SETUP_ACK_RESET		BIT(0)
 
 #define IQS7222_EVENT_MASK_ATI			BIT(12)
+#define IQS7222_EVENT_MASK_SLDR			BIT(10)
+#define IQS7222_EVENT_MASK_TOUCH		BIT(1)
+#define IQS7222_EVENT_MASK_PROX			BIT(0)
 
 #define IQS7222_COMMS_HOLD			BIT(0)
 #define IQS7222_COMMS_ERROR			0xEEEE
@@ -135,12 +137,12 @@ struct iqs7222_event_desc {
 static const struct iqs7222_event_desc iqs7222_kp_events[] = {
 	{
 		.name = "event-prox",
-		.enable = BIT(0),
+		.enable = IQS7222_EVENT_MASK_PROX,
 		.reg_key = IQS7222_REG_KEY_PROX,
 	},
 	{
 		.name = "event-touch",
-		.enable = BIT(1),
+		.enable = IQS7222_EVENT_MASK_TOUCH,
 		.reg_key = IQS7222_REG_KEY_TOUCH,
 	},
 };
@@ -1957,8 +1959,8 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 	int num_chan = dev_desc->reg_grps[IQS7222_REG_GRP_CHAN].num_row;
 	int ext_chan = rounddown(num_chan, 10);
 	int count, error, reg_offset, i;
+	u16 *event_mask = &iqs7222->sys_setup[dev_desc->event_offset];
 	u16 *sldr_setup = iqs7222->sldr_setup[sldr_index];
-	u16 *sys_setup = iqs7222->sys_setup;
 	unsigned int chan_sel[4], val;
 
 	error = iqs7222_parse_props(iqs7222, &sldr_node, sldr_index,
@@ -2003,7 +2005,7 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 	reg_offset = dev_desc->sldr_res < U16_MAX ? 0 : 1;
 
 	sldr_setup[0] |= count;
-	sldr_setup[3 + reg_offset] &= ~IQS7222_SLDR_SETUP_3_CHAN_SEL_MASK;
+	sldr_setup[3 + reg_offset] &= ~GENMASK(ext_chan - 1, 0);
 
 	for (i = 0; i < ARRAY_SIZE(chan_sel); i++) {
 		sldr_setup[5 + reg_offset + i] = 0;
@@ -2106,6 +2108,22 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 		if (error)
 			return error;
 
+		/*
+		 * The press/release event does not expose a direct GPIO link,
+		 * but one can be emulated by tying each of the participating
+		 * channels to the same GPIO.
+		 */
+		error = iqs7222_gpio_select(iqs7222, event_node,
+					    i ? iqs7222_sl_events[i].enable
+					      : sldr_setup[3 + reg_offset],
+					    i ? 1568 + sldr_index * 30
+					      : sldr_setup[4 + reg_offset]);
+		if (error)
+			return error;
+
+		if (!reg_offset)
+			sldr_setup[9] |= iqs7222_sl_events[i].enable;
+
 		error = fwnode_property_read_u32(event_node, "linux,code",
 						 &val);
 		if (error) {
@@ -2117,26 +2135,20 @@ static int iqs7222_parse_sldr(struct iqs7222_private *iqs7222, int sldr_index)
 		iqs7222->sl_code[sldr_index][i] = val;
 		input_set_capability(iqs7222->keypad, EV_KEY, val);
 
-		/*
-		 * The press/release event is determined based on whether the
-		 * coordinate field reports 0xFFFF and has no explicit enable
-		 * control.
-		 */
-		if (!iqs7222_sl_events[i].enable || reg_offset)
-			continue;
-
-		sldr_setup[9] |= iqs7222_sl_events[i].enable;
-
-		error = iqs7222_gpio_select(iqs7222, event_node,
-					    iqs7222_sl_events[i].enable,
-					    1568 + sldr_index * 30);
-		if (error)
-			return error;
-
 		if (!dev_desc->event_offset)
 			continue;
 
-		sys_setup[dev_desc->event_offset] |= BIT(10 + sldr_index);
+		/*
+		 * The press/release event is determined based on whether the
+		 * coordinate field reports 0xFFFF and solely relies on touch
+		 * or proximity interrupts to be unmasked.
+		 */
+		if (i && !reg_offset)
+			*event_mask |= (IQS7222_EVENT_MASK_SLDR << sldr_index);
+		else if (sldr_setup[4 + reg_offset] == dev_desc->touch_link)
+			*event_mask |= IQS7222_EVENT_MASK_TOUCH;
+		else
+			*event_mask |= IQS7222_EVENT_MASK_PROX;
 	}
 
 	/*
@@ -2301,29 +2313,37 @@ static int iqs7222_report(struct iqs7222_private *iqs7222)
 			input_report_abs(iqs7222->keypad, iqs7222->sl_axis[i],
 					 sldr_pos);
 
-		for (j = 0; j < ARRAY_SIZE(iqs7222_sl_events); j++) {
-			u16 mask = iqs7222_sl_events[j].mask;
-			u16 val = iqs7222_sl_events[j].val;
+		input_report_key(iqs7222->keypad, iqs7222->sl_code[i][0],
+				 sldr_pos < dev_desc->sldr_res);
 
-			if (!iqs7222_sl_events[j].enable) {
-				input_report_key(iqs7222->keypad,
-						 iqs7222->sl_code[i][j],
-						 sldr_pos < dev_desc->sldr_res);
-				continue;
-			}
+		/*
+		 * A maximum resolution indicates the device does not support
+		 * gestures, in which case the remaining fields are ignored.
+		 */
+		if (dev_desc->sldr_res == U16_MAX)
+			continue;
 
-			/*
-			 * The remaining offsets represent gesture state, and
-			 * are discarded in the case of IQS7222C because only
-			 * absolute position is reported.
-			 */
-			if (num_stat < IQS7222_MAX_COLS_STAT)
-				continue;
+		if (!(le16_to_cpu(status[1]) & IQS7222_EVENT_MASK_SLDR << i))
+			continue;
+
+		/*
+		 * Skip the press/release event, as it does not have separate
+		 * status fields and is handled separately.
+		 */
+		for (j = 1; j < ARRAY_SIZE(iqs7222_sl_events); j++) {
+			u16 mask = iqs7222_sl_events[j].mask;
+			u16 val = iqs7222_sl_events[j].val;
 
 			input_report_key(iqs7222->keypad,
 					 iqs7222->sl_code[i][j],
 					 (state & mask) == val);
 		}
+
+		input_sync(iqs7222->keypad);
+
+		for (j = 1; j < ARRAY_SIZE(iqs7222_sl_events); j++)
+			input_report_key(iqs7222->keypad,
+					 iqs7222->sl_code[i][j], 0);
 	}
 
 	input_sync(iqs7222->keypad);
-- 
2.25.1

