Hi Hans, On Thu, Mar 05, 2020 at 11:01:22PM +0100, Hans de Goede wrote: > 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 <mastichi@xxxxxxxxx> > Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx> > --- > 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
