On Sat, Dec 5, 2020 at 6:48 AM Dmitry Osipenko [off-list ref] 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 <redacted>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>

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