From 799567e7c84593e6fae79f2abcf21eeb11201cc4 Mon Sep 17 00:00:00 2001 From: Dave Stevenson Date: Wed, 4 Mar 2026 12:21:31 +0000 Subject: [PATCH 1/3] Revert "input: goodix: Add option to poll instead of relying on IRQ line" This reverts commit dd4cc673c164c79275f904290a0f1947b2548b28. --- drivers/input/touchscreen/goodix.c | 75 +++--------------------------- drivers/input/touchscreen/goodix.h | 5 -- 2 files changed, 7 insertions(+), 73 deletions(-) diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c index 89c69e9cb1d1a0..4b497540ed2d79 100644 --- a/drivers/input/touchscreen/goodix.c +++ b/drivers/input/touchscreen/goodix.c @@ -48,8 +48,6 @@ #define MAX_CONTACTS_LOC 5 #define TRIGGER_LOC 6 -#define POLL_INTERVAL_MS 17 /* 17ms = 60fps */ - /* Our special handling for GPIO accesses through ACPI is x86 specific */ #if defined CONFIG_X86 && defined CONFIG_ACPI #define ACPI_GPIO_SUPPORT @@ -515,67 +513,16 @@ static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id) return IRQ_HANDLED; } -static void goodix_ts_irq_poll_timer(struct timer_list *t) -{ - struct goodix_ts_data *ts = from_timer(ts, t, timer); - - schedule_work(&ts->work_i2c_poll); - mod_timer(&ts->timer, jiffies + msecs_to_jiffies(POLL_INTERVAL_MS)); -} - -static void goodix_ts_work_i2c_poll(struct work_struct *work) -{ - struct goodix_ts_data *ts = container_of(work, - struct goodix_ts_data, work_i2c_poll); - - goodix_process_events(ts); - goodix_i2c_write_u8(ts->client, GOODIX_READ_COOR_ADDR, 0); -} - -static void goodix_enable_irq(struct goodix_ts_data *ts) -{ - if (ts->client->irq) { - enable_irq(ts->client->irq); - } else { - ts->timer.expires = jiffies + msecs_to_jiffies(POLL_INTERVAL_MS); - add_timer(&ts->timer); - } -} - -static void goodix_disable_irq(struct goodix_ts_data *ts) -{ - if (ts->client->irq) { - disable_irq(ts->client->irq); - } else { - del_timer(&ts->timer); - cancel_work_sync(&ts->work_i2c_poll); - } -} - static void goodix_free_irq(struct goodix_ts_data *ts) { - if (ts->client->irq) { - devm_free_irq(&ts->client->dev, ts->client->irq, ts); - } else { - del_timer(&ts->timer); - cancel_work_sync(&ts->work_i2c_poll); - } + devm_free_irq(&ts->client->dev, ts->client->irq, ts); } static int goodix_request_irq(struct goodix_ts_data *ts) { - if (ts->client->irq) { - return devm_request_threaded_irq(&ts->client->dev, ts->client->irq, - NULL, goodix_ts_irq_handler, - ts->irq_flags, ts->client->name, ts); - } else { - INIT_WORK(&ts->work_i2c_poll, - goodix_ts_work_i2c_poll); - timer_setup(&ts->timer, goodix_ts_irq_poll_timer, 0); - if (ts->irq_pin_access_method == IRQ_PIN_ACCESS_NONE) - goodix_enable_irq(ts); - return 0; - } + return devm_request_threaded_irq(&ts->client->dev, ts->client->irq, + NULL, goodix_ts_irq_handler, + ts->irq_flags, ts->client->name, ts); } static int goodix_check_cfg_8(struct goodix_ts_data *ts, const u8 *cfg, int len) @@ -1194,10 +1141,7 @@ static int goodix_configure_dev(struct goodix_ts_data *ts) return -ENOMEM; } - snprintf(ts->name, GOODIX_NAME_MAX_LEN, "%s Goodix Capacitive TouchScreen", - dev_name(&ts->client->dev)); - - ts->input_dev->name = ts->name; + ts->input_dev->name = "Goodix Capacitive TouchScreen"; ts->input_dev->phys = "input/ts"; ts->input_dev->id.bustype = BUS_I2C; ts->input_dev->id.vendor = 0x0416; @@ -1463,11 +1407,6 @@ static void goodix_ts_remove(struct i2c_client *client) { struct goodix_ts_data *ts = i2c_get_clientdata(client); - if (!client->irq) { - del_timer(&ts->timer); - cancel_work_sync(&ts->work_i2c_poll); - } - if (ts->load_cfg_from_disk) wait_for_completion(&ts->firmware_loading_complete); } @@ -1483,7 +1422,7 @@ static int goodix_suspend(struct device *dev) /* We need gpio pins to suspend/resume */ if (ts->irq_pin_access_method == IRQ_PIN_ACCESS_NONE) { - goodix_disable_irq(ts); + disable_irq(client->irq); return 0; } @@ -1527,7 +1466,7 @@ static int goodix_resume(struct device *dev) int error; if (ts->irq_pin_access_method == IRQ_PIN_ACCESS_NONE) { - goodix_enable_irq(ts); + enable_irq(client->irq); return 0; } diff --git a/drivers/input/touchscreen/goodix.h b/drivers/input/touchscreen/goodix.h index 235dd5f264c520..87797cc88b3243 100644 --- a/drivers/input/touchscreen/goodix.h +++ b/drivers/input/touchscreen/goodix.h @@ -57,8 +57,6 @@ #define GOODIX_CONFIG_MAX_LENGTH 240 #define GOODIX_MAX_KEYS 7 -#define GOODIX_NAME_MAX_LEN 38 - enum goodix_irq_pin_access_method { IRQ_PIN_ACCESS_NONE, IRQ_PIN_ACCESS_GPIO, @@ -93,7 +91,6 @@ struct goodix_ts_data { enum gpiod_flags gpiod_rst_flags; char id[GOODIX_ID_MAX_LEN + 1]; char cfg_name[64]; - char name[GOODIX_NAME_MAX_LEN]; u16 version; bool reset_controller_at_probe; bool load_cfg_from_disk; @@ -107,8 +104,6 @@ struct goodix_ts_data { u8 main_clk[GOODIX_MAIN_CLK_LEN]; int bak_ref_len; u8 *bak_ref; - struct timer_list timer; - struct work_struct work_i2c_poll; }; int goodix_i2c_read(struct i2c_client *client, u16 reg, u8 *buf, int len); From 0bf2d9aa7a17ab214c3229b25f1ed91f9d4e46bf Mon Sep 17 00:00:00 2001 From: Joseph Guo Date: Sun, 29 Jun 2025 17:35:39 -0700 Subject: [PATCH 2/3] Input: goodix - add support for polling mode There are designs incorporating Goodix touch controller that do not connect interrupt pin, for example Raspberry Pi. To support such systems use polling mode for the input device when I2C client does not have interrupt assigned to it. Signed-off-by: Joseph Guo Reviewed-by: Haibo Chen Reviewed-by: Hans de Goede Link: https://lore.kernel.org/r/20250522020418.1963422-1-qijian.guo@nxp.com Signed-off-by: Dmitry Torokhov --- drivers/input/touchscreen/goodix.c | 50 ++++++++++++++++++++++++++---- 1 file changed, 44 insertions(+), 6 deletions(-) diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c index 4b497540ed2d79..5551decb8d2269 100644 --- a/drivers/input/touchscreen/goodix.c +++ b/drivers/input/touchscreen/goodix.c @@ -44,9 +44,11 @@ #define GOODIX_HAVE_KEY BIT(4) #define GOODIX_BUFFER_STATUS_TIMEOUT 20 -#define RESOLUTION_LOC 1 -#define MAX_CONTACTS_LOC 5 -#define TRIGGER_LOC 6 +#define RESOLUTION_LOC 1 +#define MAX_CONTACTS_LOC 5 +#define TRIGGER_LOC 6 + +#define GOODIX_POLL_INTERVAL_MS 17 /* 17ms = 60fps */ /* Our special handling for GPIO accesses through ACPI is x86 specific */ #if defined CONFIG_X86 && defined CONFIG_ACPI @@ -497,6 +499,14 @@ static void goodix_process_events(struct goodix_ts_data *ts) input_sync(ts->input_dev); } +static void goodix_ts_work_i2c_poll(struct input_dev *input) +{ + struct goodix_ts_data *ts = input_get_drvdata(input); + + goodix_process_events(ts); + goodix_i2c_write_u8(ts->client, GOODIX_READ_COOR_ADDR, 0); +} + /** * goodix_ts_irq_handler - The IRQ handler * @@ -513,13 +523,29 @@ static irqreturn_t goodix_ts_irq_handler(int irq, void *dev_id) return IRQ_HANDLED; } +static void goodix_enable_irq(struct goodix_ts_data *ts) +{ + if (ts->client->irq) + enable_irq(ts->client->irq); +} + +static void goodix_disable_irq(struct goodix_ts_data *ts) +{ + if (ts->client->irq) + disable_irq(ts->client->irq); +} + static void goodix_free_irq(struct goodix_ts_data *ts) { - devm_free_irq(&ts->client->dev, ts->client->irq, ts); + if (ts->client->irq) + devm_free_irq(&ts->client->dev, ts->client->irq, ts); } static int goodix_request_irq(struct goodix_ts_data *ts) { + if (!ts->client->irq) + return 0; + return devm_request_threaded_irq(&ts->client->dev, ts->client->irq, NULL, goodix_ts_irq_handler, ts->irq_flags, ts->client->name, ts); @@ -1219,6 +1245,18 @@ static int goodix_configure_dev(struct goodix_ts_data *ts) return error; } + input_set_drvdata(ts->input_dev, ts); + + if (!ts->client->irq) { + error = input_setup_polling(ts->input_dev, goodix_ts_work_i2c_poll); + if (error) { + dev_err(&ts->client->dev, + "could not set up polling mode, %d\n", error); + return error; + } + input_set_poll_interval(ts->input_dev, GOODIX_POLL_INTERVAL_MS); + } + error = input_register_device(ts->input_dev); if (error) { dev_err(&ts->client->dev, @@ -1422,7 +1460,7 @@ static int goodix_suspend(struct device *dev) /* We need gpio pins to suspend/resume */ if (ts->irq_pin_access_method == IRQ_PIN_ACCESS_NONE) { - disable_irq(client->irq); + goodix_disable_irq(ts); return 0; } @@ -1466,7 +1504,7 @@ static int goodix_resume(struct device *dev) int error; if (ts->irq_pin_access_method == IRQ_PIN_ACCESS_NONE) { - enable_irq(client->irq); + goodix_enable_irq(ts); return 0; } From 0f68e5ae6d7224c964747c0a7df8c73af9b24dfc Mon Sep 17 00:00:00 2001 From: Dave Stevenson Date: Wed, 4 Mar 2026 12:28:17 +0000 Subject: [PATCH 3/3] input: goodix: Don't retry individual reads when polling Some Goodix controllers report the buffer isn't ready continuously when there are no touch points to report. That triggers the retry mechanism within the driver required as supposedly the data can be 10ms after the interrupt occurs. Seeing as we don't have an interrupt there is little point in retrying, and we can wait for the next poll event. Signed-off-by: Dave Stevenson --- drivers/input/touchscreen/goodix.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c index 5551decb8d2269..74dea38b9fad55 100644 --- a/drivers/input/touchscreen/goodix.c +++ b/drivers/input/touchscreen/goodix.c @@ -299,6 +299,14 @@ static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8 *data) return 0; } + if (!ts->client->irq) + /* + * No point in retrying if polling, particularly as some + * versions continuously report "not ready" if there are + * no touch points. + */ + break; + usleep_range(1000, 2000); /* Poll every 1 - 2 ms */ } while (time_before(jiffies, max_timeout));