[PATCH 4/4] Input: edt-ft5x06 - poll while a contact is down to recover dropped releases
Alexandre Hamamdjian <[email protected]>
| Newsgroups | org.kernel.feeds.b4-sent,org.kernel.vger.linux-devicetree,org.kernel.vger.linux-input,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
The driver is purely interrupt driven: a touch-up is only reported when the controller raises an edge for the release frame and that frame is read successfully. On a marginal i2c bus a read can fail or be dropped, and if the read that carried the release is the one lost there is no further edge to re-read it, so the contact stays held down forever - the pointer sticks mid-drag. This is readily reproducible on the AYANEO Pocket DS, whose FT5426 hangs off an unreliable Qualcomm GENI bus. Stop trusting a single edge to deliver the release. Track which slots are held down in a mask and, while any contact is down, re-read the frame on a short timer. A contact missing from the frame is released only after a few consecutive misses so a lone glitchy read cannot cut a still-present tap or drag; conversely, once the finger is really gone the polled reads stop listing it and it is released regardless of whether an explicit touch-up frame ever arrives. The timer stops as soon as the last contact is released, so an idle panel is still fully interrupt driven. The read path also gains a bounded retry over the transient bus errors and, when reads keep failing for over a second, a reset-line pulse to recover a wedged controller, dropping any held contacts afterwards since the post-reset finger state is unknown. The poll worker is cancelled on suspend and, via a devm action registered before the IRQ, on removal, so it can never touch i2c after the device is powered down or the IRQ freed. Signed-off-by: Alexandre Hamamdjian <[email protected]> --- drivers/input/touchscreen/edt-ft5x06.c | 173 ++++++++++++++++++++++++++++++--- 1 file changed, 157 insertions(+), 16 deletions(-) diff --git a/drivers/input/touchscreen/edt-ft5x06.c b/drivers/input/touchscreen/edt-ft5x06.c index ac61ac44fd64..794c0650f5cb 100644 --- a/drivers/input/touchscreen/edt-ft5x06.c +++ b/drivers/input/touchscreen/edt-ft5x06.c @@ -19,6 +19,7 @@ #include <linux/gpio/consumer.h> #include <linux/i2c.h> #include <linux/interrupt.h> +#include <linux/workqueue.h> #include <linux/input.h> #include <linux/input/mt.h> #include <linux/input/touchscreen.h> @@ -147,8 +148,19 @@ struct edt_ft5x06_ts_data { unsigned int crc_errors; unsigned int header_errors; bool no_regmap_bulk_read; + unsigned long last_reset; + unsigned long last_success; + struct delayed_work poll_work; + /* io_lock serialises the frame read between the IRQ and the poll work */ + struct mutex io_lock; + u16 down_mask; + u8 miss[16]; }; +/* poll cadence and release debounce for the poll-while-touched recovery */ +#define EDT_POLL_INTERVAL_MS 15 +#define EDT_RELEASE_MISSES 3 + struct edt_i2c_chip_data { int max_support_points; }; @@ -321,26 +333,83 @@ static int edt_ft5x06_bulk_read(struct regmap *map, unsigned int start, return 0; } -static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id) +static void edt_ft5x06_release_all(struct edt_ft5x06_ts_data *tsdata) +{ + int id; + + if (!tsdata->down_mask) + return; + + for (id = 0; id < tsdata->max_support_points; id++) { + if (!(tsdata->down_mask & BIT(id))) + continue; + input_mt_slot(tsdata->input, id); + input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER, false); + } + tsdata->down_mask = 0; + memset(tsdata->miss, 0, sizeof(tsdata->miss)); + input_mt_report_pointer_emulation(tsdata->input, true); + input_sync(tsdata->input); +} + +static void edt_ft5x06_fetch_and_report(struct edt_ft5x06_ts_data *tsdata) { - struct edt_ft5x06_ts_data *tsdata = dev_id; struct device *dev = &tsdata->client->dev; + u16 new_mask = 0, released = 0; u8 rdbuf[63]; int i, type, x, y, id; - int error; + int error, tries; memset(rdbuf, 0, sizeof(rdbuf)); - if (tsdata->no_regmap_bulk_read) - error = edt_ft5x06_bulk_read(tsdata->regmap, tsdata->tdata_cmd, - rdbuf, tsdata->tdata_len); - else - error = regmap_bulk_read(tsdata->regmap, tsdata->tdata_cmd, - rdbuf, tsdata->tdata_len); + for (tries = 0; tries < 4; tries++) { + if (tsdata->no_regmap_bulk_read) + error = edt_ft5x06_bulk_read(tsdata->regmap, + tsdata->tdata_cmd, rdbuf, + tsdata->tdata_len); + else + error = regmap_bulk_read(tsdata->regmap, + tsdata->tdata_cmd, rdbuf, + tsdata->tdata_len); + if (!error) + break; + if (error != -EAGAIN && error != -ETIMEDOUT && + error != -EIO && error != -ENXIO) + break; + usleep_range(min(1000U << tries, 4000U), + min(2000U << tries, 8000U)); + } if (error) { dev_err_ratelimited(dev, "Unable to fetch data, error: %d\n", error); - goto out; + /* + * A run of failed reads with no success for over a second means + * the controller is wedged rather than just glitching; pulse the + * reset line to recover it and drop any held contacts, since the + * post-reset finger state is unknown. + */ + if (tsdata->reset_gpio && + time_after(jiffies, tsdata->last_success + HZ) && + time_after(jiffies, tsdata->last_reset + 2 * HZ)) { + tsdata->last_reset = jiffies; + gpiod_set_value_cansleep(tsdata->reset_gpio, 1); + usleep_range(5000, 6000); + gpiod_set_value_cansleep(tsdata->reset_gpio, 0); + msleep(300); + tsdata->last_success = jiffies; + dev_warn_ratelimited(dev, "reset to recover controller\n"); + edt_ft5x06_release_all(tsdata); + } + return; } + tsdata->last_success = jiffies; + + /* + * TD_STATUS holds the active-contact count; a value above the panel + * maximum means the frame is corrupt, so keep the previous state. + */ + if (tsdata->version != EDT_M06 && + (rdbuf[2] & 0x0f) > tsdata->max_support_points) + return; for (i = 0; i < tsdata->max_support_points; i++) { u8 *buf = &rdbuf[i * tsdata->point_len + tsdata->tdata_offset]; @@ -349,10 +418,12 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id) /* ignore Reserved events */ if (type == TOUCH_EVENT_RESERVED) continue; - /* M06 sometimes sends bogus coordinates in TOUCH_DOWN */ if (tsdata->version == EDT_M06 && type == TOUCH_EVENT_DOWN) continue; + /* releases are derived from the down-mask diff below */ + if (type == TOUCH_EVENT_UP) + continue; x = get_unaligned_be16(buf) & 0x0fff; y = get_unaligned_be16(buf + 2) & 0x0fff; @@ -363,21 +434,75 @@ static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id) id = (buf[2] >> 4) & 0x0f; if (id >= tsdata->max_support_points) continue; + if (tsdata->prop.max_x && + (x > tsdata->prop.max_x || y > tsdata->prop.max_y)) + continue; input_mt_slot(tsdata->input, id); - if (input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER, - type != TOUCH_EVENT_UP)) - touchscreen_report_pos(tsdata->input, &tsdata->prop, - x, y, true); + input_mt_report_slot_state(tsdata->input, MT_TOOL_FINGER, true); + touchscreen_report_pos(tsdata->input, &tsdata->prop, x, y, true); + new_mask |= BIT(id); } + /* + * Reconcile held contacts with this frame. A contact absent from the + * frame is released only after EDT_RELEASE_MISSES consecutive misses so + * a single glitchy read cannot cut a still-present tap or drag. + */ + for (id = 0; id < tsdata->max_support_points; id++) { + if (new_mask & BIT(id)) { + tsdata->miss[id] = 0; + continue; + } + if (!(tsdata->down_mask & BIT(id))) + continue; + if (++tsdata->miss[id] >= EDT_RELEASE_MISSES) { + input_mt_slot(tsdata->input, id); + input_mt_report_slot_state(tsdata->input, + MT_TOOL_FINGER, false); + tsdata->miss[id] = 0; + released |= BIT(id); + } + } + tsdata->down_mask = (tsdata->down_mask | new_mask) & ~released; + input_mt_report_pointer_emulation(tsdata->input, true); input_sync(tsdata->input); +} + +static irqreturn_t edt_ft5x06_ts_isr(int irq, void *dev_id) +{ + struct edt_ft5x06_ts_data *tsdata = dev_id; + + guard(mutex)(&tsdata->io_lock); + edt_ft5x06_fetch_and_report(tsdata); + if (tsdata->down_mask) + mod_delayed_work(system_wq, &tsdata->poll_work, + msecs_to_jiffies(EDT_POLL_INTERVAL_MS)); -out: return IRQ_HANDLED; } +static void edt_ft5x06_poll_work(struct work_struct *work) +{ + struct edt_ft5x06_ts_data *tsdata = + container_of(to_delayed_work(work), + struct edt_ft5x06_ts_data, poll_work); + + guard(mutex)(&tsdata->io_lock); + edt_ft5x06_fetch_and_report(tsdata); + if (tsdata->down_mask) + mod_delayed_work(system_wq, &tsdata->poll_work, + msecs_to_jiffies(EDT_POLL_INTERVAL_MS)); +} + +static void edt_ft5x06_cancel_poll(void *data) +{ + struct edt_ft5x06_ts_data *tsdata = data; + + cancel_delayed_work_sync(&tsdata->poll_work); +} + struct edt_ft5x06_attribute { struct device_attribute dattr; size_t field_offset; @@ -1244,6 +1369,10 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client) tsdata->no_regmap_bulk_read = device_property_read_bool(&client->dev, "no-regmap-bulk-read"); + tsdata->last_success = jiffies; + tsdata->last_reset = jiffies; + mutex_init(&tsdata->io_lock); + INIT_DELAYED_WORK(&tsdata->poll_work, edt_ft5x06_poll_work); /* * Check which sleep modes we can support. Power-off requires the @@ -1341,6 +1470,15 @@ static int edt_ft5x06_ts_probe(struct i2c_client *client) return error; } + /* + * Registered before the IRQ so it unwinds after the IRQ is freed on + * removal: no edge can re-arm the poll worker once it is cancelled. + */ + error = devm_add_action_or_reset(&client->dev, edt_ft5x06_cancel_poll, + tsdata); + if (error) + return error; + irq_flags = irq_get_trigger_type(client->irq); if (irq_flags == IRQF_TRIGGER_NONE) irq_flags = IRQF_TRIGGER_FALLING; @@ -1383,6 +1521,9 @@ static int edt_ft5x06_ts_suspend(struct device *dev) struct gpio_desc *reset_gpio = tsdata->reset_gpio; int ret; + /* stop the poll worker so it cannot touch i2c after power-down */ + cancel_delayed_work_sync(&tsdata->poll_work); + if (device_may_wakeup(dev)) return 0; -- 2.55.0