Commit a2fd46cd authored by Hans de Goede's avatar Hans de Goede Committed by Dmitry Torokhov
Browse files

Input: goodix - try not to touch the reset-pin on x86/ACPI devices 

Unless the controller is not responding at boot or after suspend/resume,
the driver never resets the controller on x86/ACPI platforms. The driver
still requesting the reset pin at probe() though in case it needs it.

Until now the driver has always requested the reset pin with GPIOD_IN
as type. The idea being to put the pin in high-impedance mode to save
power until the driver actually wants to issue a reset.

But this means that just requesting the pin can cause issues, since
requesting it in another mode then GPIOD_ASIS may cause the pinctrl
driver to touch the pin settings. We have already had issues before
due to a bug in the pinctrl-cherryview.c driver which has been fixed in
commit 921daeec ("pinctrl: cherryview: Preserve
CHV_PADCTRL1_INVRXTX_TXDATA flag on GPIOs").

And now it turns out that requesting the reset-pin as GPIOD_IN also stops
the touchscreen from working on the GPD P2 max mini-laptop. The behavior
of putting the pin in high-impedance mode relies on there being some
external pull-up to keep it high and there seems to be no pull-up on the
GPD P2 max, causing things to break.

This commit fixes this by requesting the reset pin as is when using
the x86/ACPI code paths to lookup the GPIOs; and by not dropping it
back into input-mode in case the driver does end up issuing a reset
for error-recovery.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=209061


Fixes: a7d4b171 ("Input: goodix - add support for getting IRQ + reset GPIOs on Cherry Trail devices")
Cc: stable@vger.kernel.org
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20211206091116.44466-2-hdegoede@redhat.com


Signed-off-by: default avatarDmitry Torokhov <dmitry.torokhov@gmail.com>
parent 44ee250a

drivers/input/touchscreen/goodix.c

+25 −5
Original line number Diff line number Diff line
@@ -695,10 +695,16 @@ int goodix_reset_no_int_sync(struct goodix_ts_data *ts) 


	usleep_range(6000, 10000);		/* T4: > 5ms */



	/* end select I2C slave addr */

	/*

	 * Put the reset pin back in to input / high-impedance mode to save

	 * power. Only do this in the non ACPI case since some ACPI boards

	 * don't have a pull-up, so there the reset pin must stay active-high.

	 */

	if (ts->irq_pin_access_method == IRQ_PIN_ACCESS_GPIO) {

		error = gpiod_direction_input(ts->gpiod_rst);

		if (error)

			goto error;

	}



	return 0;
@@ -832,6 +838,14 @@ static int goodix_add_acpi_gpio_mappings(struct goodix_ts_data *ts) 
		return -EINVAL;

	}



	/*

	 * Normally we put the reset pin in input / high-impedance mode to save

	 * power. But some x86/ACPI boards don't have a pull-up, so for the ACPI

	 * case, leave the pin as is. This results in the pin not being touched

	 * at all on x86/ACPI boards, except when needed for error-recover.

	 */

	ts->gpiod_rst_flags = GPIOD_ASIS;



	return devm_acpi_dev_add_driver_gpios(dev, gpio_mapping);

}

#else
@@ -857,6 +871,12 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts) 
		return -EINVAL;

	dev = &ts->client->dev;



	/*

	 * By default we request the reset pin as input, leaving it in

	 * high-impedance when not resetting the controller to save power.

	 */

	ts->gpiod_rst_flags = GPIOD_IN;



	ts->avdd28 = devm_regulator_get(dev, "AVDD28");

	if (IS_ERR(ts->avdd28)) {

		error = PTR_ERR(ts->avdd28);
@@ -894,7 +914,7 @@ static int goodix_get_gpio_config(struct goodix_ts_data *ts) 
	ts->gpiod_int = gpiod;



	/* Get the reset line GPIO pin number */

	gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_RST_NAME, GPIOD_IN);

	gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_RST_NAME, ts->gpiod_rst_flags);

	if (IS_ERR(gpiod)) {

		error = PTR_ERR(gpiod);

		if (error != -EPROBE_DEFER)

drivers/input/touchscreen/goodix.h

+1 −0
Original line number Diff line number Diff line
@@ -87,6 +87,7 @@ struct goodix_ts_data { 
	struct gpio_desc *gpiod_rst;

	int gpio_count;

	int gpio_int_idx;

	enum gpiod_flags gpiod_rst_flags;

	char id[GOODIX_ID_MAX_LEN + 1];

	char cfg_name[64];

	u16 version;