Thread (17 messages) 17 messages, 2 authors, 2020-03-07

Re: [PATCH v2 01/11] Input: goodix - Refactor IRQ pin GPIO accesses

flat view

From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
Date: 2020-03-06 04:03:08

Hi Hans,

On Thu, Mar 05, 2020 at 11:01:22PM +0100, Hans de Goede wrote:
quoted hunk ↗ jump to hunk
Suspending Goodix touchscreens requires changing the interrupt pin to
output before sending them a power-down command. Followed by wiggling
the interrupt pin to wake the device up, after which it is put back
in input mode.

So far we have only effectively supported this on devices which use
devicetree. On X86 ACPI platforms both looking up the pins; and using a
pin as both IRQ and GPIO is a bit more complicated. E.g. on some devices
we cannot directly access the IRQ pin as GPIO and we need to call ACPI
methods to control it instead.

This commit adds a new irq_pin_access_method field to the goodix_chip_data
struct and adds goodix_irq_direction_output and goodix_irq_direction_input
helpers which together abstract the GPIO accesses to the IRQ pin.

This is a preparation patch for adding support for properly suspending the
touchscreen on X86 ACPI platforms.

BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1786317
BugLink: https://github.com/nexus511/gpd-ubuntu-packages/issues/10
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199207
Cc: Dmitry Mastykin <redacted>
Signed-off-by: Hans de Goede <redacted>
---
Changes in v2:
- Make enum member names upper-case
---
 drivers/input/touchscreen/goodix.c | 62 ++++++++++++++++++++++++------
 1 file changed, 51 insertions(+), 11 deletions(-)
diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c
index 0403102e807e..9cfbcf3cbca8 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -31,6 +31,11 @@
 
 struct goodix_ts_data;
 
+enum goodix_irq_pin_access_method {
+	IRQ_PIN_ACCESS_NONE,
+	IRQ_PIN_ACCESS_GPIO,
+};
+
 struct goodix_chip_data {
 	u16 config_addr;
 	int config_len;
@@ -53,6 +58,7 @@ struct goodix_ts_data {
 	const char *cfg_name;
 	struct completion firmware_loading_complete;
 	unsigned long irq_flags;
+	enum goodix_irq_pin_access_method irq_pin_access_method;
 	unsigned int contact_size;
 };
 
@@ -502,17 +508,48 @@ static int goodix_send_cfg(struct goodix_ts_data *ts,
 	return 0;
 }
 
+static int goodix_irq_direction_output(struct goodix_ts_data *ts,
+				       int value)
+{
+	switch (ts->irq_pin_access_method) {
+	case IRQ_PIN_ACCESS_NONE:
+		dev_err(&ts->client->dev,
+			"%s called without an irq_pin_access_method set\n",
+			__func__);
+		return -EINVAL;
+	case IRQ_PIN_ACCESS_GPIO:
+		return gpiod_direction_output(ts->gpiod_int, value);
+	}
+
+	return -EINVAL; /* Never reached */
I do not like these "never reached" comments. We can either let compiler
issue a warning that we did not cover all switch cases, or stick
"default:" alongside "case IRQ_PIN_ACCESS_NONE:".

Thanks.

-- 
Dmitry
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help