]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
Input: goodix - remove setting of RST pin to input
authorMartyn Welch <martyn.welch@collabora.com>
Thu, 9 Oct 2025 13:41:32 +0000 (14:41 +0100)
committerDmitry Torokhov <dmitry.torokhov@gmail.com>
Mon, 13 Oct 2025 16:35:26 +0000 (09:35 -0700)
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>
drivers/input/touchscreen/goodix.c
drivers/input/touchscreen/goodix.h

index 5551decb8d2269a0ebe0760e780dd8b0db25f689..f8798d11ec030fb5027c9bd9260021d092a88309 100644 (file)
@@ -796,17 +796,6 @@ int goodix_reset_no_int_sync(struct goodix_ts_data *ts)
 
        usleep_range(6000, 10000);              /* T4: > 5ms */
 
-       /*
-        * 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;
 
 error:
@@ -957,14 +946,6 @@ 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
@@ -989,12 +970,6 @@ 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))
                return dev_err_probe(dev, PTR_ERR(ts->avdd28), "Failed to get AVDD28 regulator\n");
@@ -1019,7 +994,7 @@ retry_get_irq_gpio:
        ts->gpiod_int = gpiod;
 
        /* Get the reset line GPIO pin number */
-       gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_RST_NAME, ts->gpiod_rst_flags);
+       gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_RST_NAME, GPIOD_ASIS);
        if (IS_ERR(gpiod))
                return dev_err_probe(dev, PTR_ERR(gpiod), "Failed to get %s GPIO\n",
                                     GOODIX_GPIO_RST_NAME);
index 87797cc88b3243cb677d283502b32e0dee885639..0d1e8a8d2cbaac713fdb89f64eb5622086bd312a 100644 (file)
@@ -88,7 +88,6 @@ 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;