18.09.2020 18:55, Wang, Jiada пишет: ... >>> +static void mxt_wake(struct mxt_data *data) >>> +{ >>> + struct i2c_client *client = data->client; >>> + struct device *dev = &data->client->dev; >>> + struct device_node *np = dev->of_node; >>> + union i2c_smbus_data dummy; >>> + >>> + if (!of_device_is_compatible(np, "atmel,mXT1386")) >>> + return; >> I'm not sure whether you misses the previous answers from Dmitry >> Torokhov and Rob Herring, but they suggested to add a new device-tree >> property which should specify the atmel,wakeup-method. >> > I think Rob Herring prefers for the compatible solution than property. Actually, seems you're right. But I'm not sure now whether he just made a typo, because it's actually a board-specific option. It could be more preferred to skip the i2c_smbus_xfer() for the NONE variant, but it also should be harmless in practice. I guess we indeed could keep the current variant of yours patch and then add a clarifying comment to the commit message and to the code, telling that i2c_smbus_xfer() is harmless in a case of the hardwired WAKE-LINE. >> There are 3 possible variants: >> >> - NONE >> - GPIO >> - I2C-SCL >> >> Hence we should bail out from mxt_wake() if method is set to NONE or >> GPIO. >> > for "GPIO", we still need 25 ms sleep. but rather than a dummy read, > WAKE line need to be asserted before sleep. Correct, I just meant to bail out because GPIO is currently unsupported. ... >>> static int mxt_initialize(struct mxt_data *data) >>> { >>> struct i2c_client *client = data->client; >>> int recovery_attempts = 0; >>> int error; >>> + mxt_wake(data); >>> + >>> while (1) { >> >> I assume the mxt_wake() should be placed here, since there is a 3 >> seconds timeout in the end of the while-loop, meaning that device may >> get back into deep-sleep on a retry. >> > Can you elaborate a little more why exit from bootload mode after sleep > for 3 second could enter deep-sleep mode. The loop attempts to exit from bootload mode and then I suppose mxt_read_info_block() may fail if I2C "accidentally" fails, hence the deep-sleep mode still will be enabled on a retry. The datasheet says that there are 2 seconds since the time of the last I2C access before TS is put back into auto-sleep if deep-sleep mode is enabled. The wait-loop has msleep(3000).