Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

Input: goodix - remove setting of RST pin to input

The reset line is being set to input on non-ACPI devices apparently to
save power. This isn't being done on ACPI devices as it's been found
that some ACPI devices don't have a pull-up resistor fitted. This can
also be the case for non-ACPI devices, resulting in:

[ 941.672207] Goodix-TS 1-0014: Error reading 10 bytes from 0x814e: -110
[ 942.696168] Goodix-TS 1-0014: Error reading 10 bytes from 0x814e: -110
[ 945.832208] Goodix-TS 1-0014: Error reading 10 bytes from 0x814e: -110

This behaviour appears to have been initialing introduced in
ec6e1b4082d9. This doesn't seem to be based on information in either the
GT911 or GT9271 datasheets cited as sources of information for this
change. Thus it seems likely that it is based on functionality in the
Android driver which it also lists. This behaviour may be viable in very
specific instances where the hardware is well known, but seems unwise in
the upstream kernel where such hardware requirements can't be
guaranteed.

Remove this over optimisation to improve reliability on non-ACPI
devices.

Signed-off-by: Martyn Welch <martyn.welch@collabora.com>
Reviewed-by: Hans de Goede <hansg@kernel.org>
Link: https://lore.kernel.org/r/20251009134138.686215-1-martyn.welch@collabora.com
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

authored by

Martyn Welch and committed by
Dmitry Torokhov
7363096a c6d99e48

+1 -27
+1 -26
drivers/input/touchscreen/goodix.c
··· 796 796 797 797 usleep_range(6000, 10000); /* T4: > 5ms */ 798 798 799 - /* 800 - * Put the reset pin back in to input / high-impedance mode to save 801 - * power. Only do this in the non ACPI case since some ACPI boards 802 - * don't have a pull-up, so there the reset pin must stay active-high. 803 - */ 804 - if (ts->irq_pin_access_method == IRQ_PIN_ACCESS_GPIO) { 805 - error = gpiod_direction_input(ts->gpiod_rst); 806 - if (error) 807 - goto error; 808 - } 809 - 810 799 return 0; 811 800 812 801 error: ··· 946 957 return -EINVAL; 947 958 } 948 959 949 - /* 950 - * Normally we put the reset pin in input / high-impedance mode to save 951 - * power. But some x86/ACPI boards don't have a pull-up, so for the ACPI 952 - * case, leave the pin as is. This results in the pin not being touched 953 - * at all on x86/ACPI boards, except when needed for error-recover. 954 - */ 955 - ts->gpiod_rst_flags = GPIOD_ASIS; 956 - 957 960 return devm_acpi_dev_add_driver_gpios(dev, gpio_mapping); 958 961 } 959 962 #else ··· 969 988 if (!ts->client) 970 989 return -EINVAL; 971 990 dev = &ts->client->dev; 972 - 973 - /* 974 - * By default we request the reset pin as input, leaving it in 975 - * high-impedance when not resetting the controller to save power. 976 - */ 977 - ts->gpiod_rst_flags = GPIOD_IN; 978 991 979 992 ts->avdd28 = devm_regulator_get(dev, "AVDD28"); 980 993 if (IS_ERR(ts->avdd28)) ··· 994 1019 ts->gpiod_int = gpiod; 995 1020 996 1021 /* Get the reset line GPIO pin number */ 997 - gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_RST_NAME, ts->gpiod_rst_flags); 1022 + gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_RST_NAME, GPIOD_ASIS); 998 1023 if (IS_ERR(gpiod)) 999 1024 return dev_err_probe(dev, PTR_ERR(gpiod), "Failed to get %s GPIO\n", 1000 1025 GOODIX_GPIO_RST_NAME);
-1
drivers/input/touchscreen/goodix.h
··· 88 88 struct gpio_desc *gpiod_rst; 89 89 int gpio_count; 90 90 int gpio_int_idx; 91 - enum gpiod_flags gpiod_rst_flags; 92 91 char id[GOODIX_ID_MAX_LEN + 1]; 93 92 char cfg_name[64]; 94 93 u16 version;