diff --git a/drivers/input/touchscreen/stmfts.c b/drivers/input/touchscreen/stmfts.c index 972687797..9ebb517b2 100644 --- a/drivers/input/touchscreen/stmfts.c +++ b/drivers/input/touchscreen/stmfts.c @@ -11,6 +11,7 @@ #include #include #include +#include #include #include #include @@ -106,6 +107,15 @@ struct stmfts_data { bool led_status; bool hover_enabled; bool running; + + /* + * Polls the event queue, because the controller's interrupt is not + * used on this board. See stmfts_poll_thread(). + */ + struct task_struct *poll_thread; + + /* Bring-up diagnostics: dump the first few event reads. */ + unsigned int debug_dumps; }; static int stmfts_brightness_set(struct led_classdev *led_cdev, @@ -174,23 +184,37 @@ static void stmfts_report_contact_event(struct stmfts_data *sdata, const u8 event[]) { u8 slot_id = (event[0] & STMFTS_MASK_TOUCH_ID) >> 4; - u16 x = event[1] | ((event[2] & STMFTS_MASK_X_MSB) << 8); - u16 y = (event[2] >> 4) | (event[3] << 4); - u8 maj = event[4]; - u8 min = event[5]; - u8 orientation = event[6]; - u8 area = event[7]; + u16 x, y; + u8 maj, min, orientation, area; + + /* + * The Switch's controller packs its events differently from the + * layout this driver was written for: both coordinates are 12-bit and + * share the low nibble of the second coordinate byte, while the two + * size fields collapse into one byte. + */ + x = (event[1] << 4) | ((event[3] & 0xf0) >> 4); + y = (event[2] << 4) | (event[3] & 0x0f); + + maj = event[4]; + min = event[4]; + orientation = event[6]; + area = event[7]; input_mt_slot(sdata->input, slot_id); input_mt_report_slot_state(sdata->input, MT_TOOL_FINGER, true); - input_report_abs(sdata->input, ABS_MT_POSITION_X, x); - input_report_abs(sdata->input, ABS_MT_POSITION_Y, y); + touchscreen_report_pos(sdata->input, &sdata->prop, x, y, true); input_report_abs(sdata->input, ABS_MT_TOUCH_MAJOR, maj); input_report_abs(sdata->input, ABS_MT_TOUCH_MINOR, min); input_report_abs(sdata->input, ABS_MT_PRESSURE, area); input_report_abs(sdata->input, ABS_MT_ORIENTATION, orientation); + /* + * Without this, clients see BTN_TOUCH set but never reported and + * treat taps as hovers. + */ + input_mt_sync_frame(sdata->input); input_sync(sdata->input); } @@ -202,6 +226,7 @@ static void stmfts_report_contact_release(struct stmfts_data *sdata, input_mt_slot(sdata->input, slot_id); input_mt_report_slot_inactive(sdata->input); + input_mt_sync_frame(sdata->input); input_sync(sdata->input); } @@ -305,15 +330,71 @@ static irqreturn_t stmfts_irq_handler(int irq, void *dev) guard(mutex)(&sdata->mutex); err = stmfts_read_events(sdata); - if (unlikely(err)) + if (unlikely(err)) { dev_err(&sdata->client->dev, "failed to read events: %d\n", err); - else + } else { + /* + * The command completion depends on recognising a status event + * in here. On this board nothing ever completes, so dump what + * the controller actually returns -- empty, or events we do not + * recognise, are very different problems. + */ + if (sdata->debug_dumps < 8) { + sdata->debug_dumps++; + dev_info(&sdata->client->dev, + "ALARM: event read %u, first 24 bytes: %*ph\n", + sdata->debug_dumps, 24, sdata->data); + } stmfts_parse_events(sdata); + } return IRQ_HANDLED; } +/* + * The controller reports its status events by raising an interrupt, and every + * stmfts_command() waits for the handler to read one. On the Switch that + * interrupt has not been observed to fire at all: the event never arrives and + * the very first command times out. So read the queue from a thread instead. + * switchroot's first touchscreen patch added the same polled mode, for what + * looks like the same reason. + * + * The interrupt is still requested, so if it does fire the response is + * immediate and the poll is just a safety net. Both paths take sdata->mutex, + * so they cannot race. + */ +static int stmfts_poll_thread(void *data) +{ + struct stmfts_data *sdata = data; + + while (!kthread_should_stop()) { + stmfts_irq_handler(0, sdata); + msleep(2); + } + + return 0; +} + +/* + * Read the controller's event queue until it reports a status event, or give + * up after ~1s. The driver's original design waits for an interrupt to do the + * reading; on this board that interrupt never fires, so read it here. + */ +static int stmfts_wait_for_event(struct stmfts_data *sdata) +{ + int i; + + for (i = 0; i < 100; i++) { + stmfts_irq_handler(0, sdata); + if (completion_done(&sdata->cmd_done)) + return 0; + msleep(10); + } + + return -ETIMEDOUT; +} + static int stmfts_command(struct stmfts_data *sdata, const u8 cmd) { int err; @@ -324,11 +405,7 @@ static int stmfts_command(struct stmfts_data *sdata, const u8 cmd) if (err) return err; - if (!wait_for_completion_timeout(&sdata->cmd_done, - msecs_to_jiffies(1000))) - return -ETIMEDOUT; - - return 0; + return stmfts_wait_for_event(sdata); } static int stmfts_input_open(struct input_dev *dev) @@ -540,6 +617,16 @@ static int stmfts_read_system_info(struct stmfts_data *sdata) sdata->config_id = reg[4]; sdata->config_ver = reg[5]; + /* + * ALARM: identify the controller. The vendor DT calls this part + * "stm,ftm4_fts" while mainline's driver targets the FTS family, so + * knowing which one answers here is worth having. + */ + dev_info(&sdata->client->dev, + "ALARM: chip id 0x%04x, chip ver 0x%02x, fw 0x%04x, config id 0x%02x ver 0x%02x\n", + sdata->chip_id, sdata->chip_ver, sdata->fw_ver, + sdata->config_id, sdata->config_ver); + return 0; } @@ -556,6 +643,27 @@ static int stmfts_configure(struct stmfts_data *sdata) { int err; + /* + * Reset the controller and wait for its CONTROLLER_READY event. + * + * The wait matters. The chip's firmware boot includes CRC checks of + * its flash sections and it can take a few hundred ms before its I2C + * state machine acknowledges anything -- during which reads fail and + * the i2c core silently retries them, which is why the event reads + * look slow immediately after the reset. Upstream added "Input: + * stmfts - wait for controller ready after reset" for exactly this, + * for FTS4 and FTS5 alike. + * + * There is deliberately NO interrupt-enable register write here. + * switchroot's patch 0011 wrote 0x2c = 0x48 after the reset so the + * controller would signal its status event by interrupt. This driver + * does not use the interrupt -- it polls the event FIFO -- and on this + * part that write is answered with an error event (0x0f ba d0 00 c0 + * de). The vendor calls the chip "stm,ftm4_fts", and FTM4-family + * panels are documented as using different registers and a different + * init sequence from the FTS driver, so that write is very likely + * aimed at a part we are not talking to. + */ err = stmfts_command(sdata, STMFTS_SYSTEM_RESET); if (err) return err; @@ -589,8 +697,11 @@ static int stmfts_power_on(struct stmfts_data *sdata) err = regulator_bulk_enable(ARRAY_SIZE(stmfts_supplies), sdata->supplies); - if (err) + if (err) { + dev_err(&sdata->client->dev, + "ALARM: failed to enable vdd/avdd supplies: %d\n", err); return err; + } /* * The datasheet does not specify the power on time, but considering @@ -602,10 +713,20 @@ static int stmfts_power_on(struct stmfts_data *sdata) stmfts_reset(sdata); err = stmfts_read_system_info(sdata); - if (err) + if (err) { + /* + * This is the driver's first I2C access, so a -110 here means + * the controller never acknowledged: power, wiring or pinmux, + * not the command protocol. + */ + dev_err(&sdata->client->dev, + "ALARM: no answer from the controller at the first I2C access: %d\n", + err); goto err_disable_regulators; + } - enable_irq(sdata->client->irq); + if (sdata->client->irq) + enable_irq(sdata->client->irq); msleep(50); @@ -613,6 +734,27 @@ static int stmfts_power_on(struct stmfts_data *sdata) if (err) goto err_disable_irq; + /* + * Start polling only now, and the ordering matters. The controller's + * "ready" event is normally already sitting in its queue by the time + * configure() runs, and configure() depends on reading it to complete + * the reset. A poll thread running before that consumes the event + * first, reinit_completion() then discards the completion it produced, + * and the reset is left with nothing to wait on -- which is exactly + * the failure this driver was hitting. + * + * From here the thread's job is reporting touches, which is what the + * interrupt would otherwise do. + */ + sdata->poll_thread = kthread_run(stmfts_poll_thread, sdata, "stmfts_poll"); + if (IS_ERR(sdata->poll_thread)) { + err = PTR_ERR(sdata->poll_thread); + sdata->poll_thread = NULL; + dev_err(&sdata->client->dev, + "ALARM: could not start the poll thread: %d\n", err); + goto err_disable_irq; + } + /* * At this point no one is using the touchscreen * and I don't really care about the return value @@ -622,7 +764,8 @@ static int stmfts_power_on(struct stmfts_data *sdata) return 0; err_disable_irq: - disable_irq(sdata->client->irq); + if (sdata->client->irq) + disable_irq(sdata->client->irq); err_disable_regulators: regulator_bulk_disable(ARRAY_SIZE(stmfts_supplies), sdata->supplies); return err; @@ -632,7 +775,13 @@ static void stmfts_power_off(void *data) { struct stmfts_data *sdata = data; - disable_irq(sdata->client->irq); + if (sdata->poll_thread) { + kthread_stop(sdata->poll_thread); + sdata->poll_thread = NULL; + } + + if (sdata->client->irq) + disable_irq(sdata->client->irq); if (sdata->reset_gpio) gpiod_set_value_cansleep(sdata->reset_gpio, 1); @@ -735,13 +884,22 @@ static int stmfts_probe(struct i2c_client *client) * the status of the irq line that can generate some spurious * interrupts. To be on the safe side it's better to not enable * the interrupts during their request. + * + * The interrupt is optional. On the Switch it is not used at all: + * a level-low interrupt on a pin nothing terminates stays asserted, + * so the handler runs continuously, consumes the controller's ready + * event before the reset can read it, and starves the poll loop on + * the mutex. The device tree therefore has no "interrupts" property + * and everything is driven by stmfts_poll_thread() instead. */ - err = devm_request_threaded_irq(dev, client->irq, - NULL, stmfts_irq_handler, - IRQF_ONESHOT | IRQF_NO_AUTOEN, - "stmfts_irq", sdata); - if (err) - return err; + if (client->irq) { + err = devm_request_threaded_irq(dev, client->irq, + NULL, stmfts_irq_handler, + IRQF_ONESHOT | IRQF_NO_AUTOEN, + "stmfts_irq", sdata); + if (err) + return err; + } dev_dbg(dev, "initializing ST-Microelectronics FTS...\n");