On Sat, Dec 5, 2020 at 6:48 AM Dmitry Osipenko <digetx@xxxxxxxxx> wrote: > According to datasheets, chips like mXT1386 have a WAKE line, it is used > to wake the chip up from deep sleep mode before communicating with it via > the I2C-compatible interface. > > If the WAKE line is connected to a GPIO line, the line must be asserted > 25 ms before the host attempts to communicate with the controller. If the > WAKE line is connected to the SCL pin, the controller will send a NACK on > the first attempt to address it, the host must then retry 25 ms later. > > Implement the wake-up methods in the driver. Touchscreen now works > properly on devices like Acer A500 tablet, fixing problems like this: > > atmel_mxt_ts 0-004c: __mxt_read_reg: i2c transfer failed (-121) > atmel_mxt_ts 0-004c: mxt_bootloader_read: i2c recv failed (-121) > atmel_mxt_ts 0-004c: Trying alternate bootloader address > atmel_mxt_ts 0-004c: mxt_bootloader_read: i2c recv failed (-121) > atmel_mxt_ts: probe of 0-004c failed with error -121 > > Signed-off-by: Jiada Wang <jiada_wang@xxxxxxxxxx> > Signed-off-by: Dmitry Osipenko <digetx@xxxxxxxxx> OK looks interesting! > + /* Request the WAKE line as asserted so controller won't sleep */ > + data->wake_gpio = devm_gpiod_get_optional(&client->dev, > + "wake", GPIOD_OUT_HIGH); > + if (IS_ERR(data->wake_gpio)) { > + error = PTR_ERR(data->wake_gpio); > + dev_err(&client->dev, "Failed to get wake gpio: %d\n", error); > + return error; > + } That is a bit brutal, don't you think? Now you force the controller to be on at all times. Even across suspend/resume. Shouldn't the same patch drive this low in mxt_suspend() and driver it high + wait 25 ms in mxt_resume()? Waiting 25ms in mxt_resume() is chill, it is anyway on the slowpath. Yours, Linus Walleij