[PATCH 2/3] Input: cyttsp5 - init IRQ early to avoid app startup problems

[Marc-Olivier Champagne]

Sometimes, the driver reports an error when trying to start the
application.

After a reset, the bootloader will assert the interrupt pin to indicate
when it is ready to communicate with the host; on our hardware this
happens about 3.4 ms after the reset line is released. But since
cyttsp5_probe() configures the IRQ and then immediately jumps to
cyttsp5_startup(), there is a chance that the interrupt pin assertion
can happen at around the same time that we try to deassert it by
calling cyttsp5_deassert_int().

Initializing the IRQ while the reset pin is asserted ensures that this
first interrupt is handled by cyttsp5_handle_irq(), which deasserts the
interrupt pin by reading the input buffer. Therefore, calling
cyttsp5_deassert_int() is no longer needed in that case.

However, when cyttsp5_handle_irq() has processed that first interrupt,
it has set the cmd_done completion. Because of that, the launch
application command would complete before its own response has been
received. Instead of sleeping a fixed 20 ms and hoping the reset
sentinel has already been received, wait for it explicitly after
releasing the reset line, so that its completion is consumed before the
first command is sent.

Without a reset line, the device may still be powering up if enabling
the regulators turned it on, or may already be running, and the reset
sentinel may have been raised before the IRQ was requested. Keep the
20 ms delay and the cyttsp5_deassert_int() call in that case, then wait
for the interrupt handler to finish and drop the completion it may have
set, so that the sentinel does not complete the first command either
way.

This relies on the previous patch, which makes cyttsp5_handle_irq()
reliably identify the reset sentinel.

Fixes: 5b0c03e24a06 ("Input: Add driver for Cypress Generation 5 touchscreen")
Cc: stable@xxxxxxxxxxxxxxx
Suggested-by: Hugo Villeneuve <hvilleneuve@xxxxxxxxxxxx>
Assisted-by: Copilot:claude-fable-5.1
Signed-off-by: Marc-Olivier Champagne <marc-olivier.champagne@xxxxxxxxxxxxxxxxxxxx>
---
 drivers/input/touchscreen/cyttsp5.c | 66 +++++++++++++++++++++++------
 1 file changed, 53 insertions(+), 13 deletions(-)

diff --git a/drivers/input/touchscreen/cyttsp5.c b/drivers/input/touchscreen/cyttsp5.c
index 04e141eceb45..beeb7d5bcd2c 100644
--- a/drivers/input/touchscreen/cyttsp5.c
+++ b/drivers/input/touchscreen/cyttsp5.c
@@ -88,6 +88,7 @@
 #define CY_HID_OUTPUT_GET_SYSINFO_TIMEOUT_MS	3000
 #define CY_HID_GET_HID_DESCRIPTOR_TIMEOUT_MS	4000
 #define CY_HID_SET_POWER_TIMEOUT		500
+#define CY_HID_RESET_SENTINEL_TIMEOUT_MS	1000
 
 /* maximum number of concurrent tracks */
 #define TOUCH_REPORT_SIZE			10
@@ -760,6 +761,30 @@ static int cyttsp5_deassert_int(struct cyttsp5 *ts)
 	return -EINVAL;
 }
 
+/*
+ * After a reset the device asserts the interrupt line with a zero-length
+ * report (reset sentinel) once it is ready to communicate. Consume it so
+ * that it does not complete the next command.
+ */
+static int cyttsp5_wait_reset_sentinel(struct cyttsp5 *ts)
+{
+	unsigned long timeout = msecs_to_jiffies(CY_HID_RESET_SENTINEL_TIMEOUT_MS);
+	int rc;
+
+	rc = wait_for_completion_interruptible_timeout(&ts->cmd_done, timeout);
+	if (rc <= 0) {
+		dev_err(ts->dev, "Reset sentinel timed out\n");
+		return -ETIMEDOUT;
+	}
+
+	if (get_unaligned_le16(&ts->response_buf[0]) != 0) {
+		dev_err(ts->dev, "Unexpected report instead of reset sentinel\n");
+		return -EPROTO;
+	}
+
+	return 0;
+}
+
 static int cyttsp5_fill_all_touch(struct cyttsp5 *ts)
 {
 	struct cyttsp5_sysinfo *si = &ts->sysinfo;
@@ -786,12 +811,6 @@ static int cyttsp5_startup(struct cyttsp5 *ts)
 {
 	int error;
 
-	error = cyttsp5_deassert_int(ts);
-	if (error) {
-		dev_err(ts->dev, "Error on deassert int r=%d\n", error);
-		return -ENODEV;
-	}
-
 	/*
 	 * Launch the application as the device starts in bootloader mode
 	 * because of a power-on-reset
@@ -888,13 +907,6 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
 		return error;
 	}
 
-	fsleep(10); /* Ensure long-enough reset pulse (minimum 10us). */
-
-	gpiod_set_value_cansleep(ts->reset_gpio, 0);
-
-	/* Need a delay to have device up */
-	msleep(20);
-
 	error = devm_request_threaded_irq(dev, irq, NULL, cyttsp5_handle_irq,
 					  IRQF_ONESHOT, name, ts);
 	if (error) {
@@ -902,6 +914,34 @@ static int cyttsp5_probe(struct device *dev, struct regmap *regmap, int irq,
 		return error;
 	}
 
+	fsleep(10); /* Ensure long-enough reset pulse (minimum 10us). */
+
+	gpiod_set_value_cansleep(ts->reset_gpio, 0);
+
+	if (ts->reset_gpio) {
+		error = cyttsp5_wait_reset_sentinel(ts);
+		if (error)
+			return error;
+	} else {
+		/*
+		 * Without a reset line, the device may be powering up or
+		 * already running. Give it time to come up, then make sure a
+		 * reset sentinel does not complete the first command: read it
+		 * if it came before the IRQ was requested, otherwise drop the
+		 * completion set by the interrupt handler.
+		 */
+		msleep(20);
+
+		error = cyttsp5_deassert_int(ts);
+		if (error) {
+			dev_err(ts->dev, "Error on deassert int r=%d\n", error);
+			return error;
+		}
+
+		synchronize_irq(irq);
+		try_wait_for_completion(&ts->cmd_done);
+	}
+
 	error = cyttsp5_startup(ts);
 	if (error) {
 		dev_err(ts->dev, "Fail initial startup r=%d\n", error);
-- 
2.34.1