* [PATCH] HID: i2c-hid: Add 60ms delay after SET_POWER ON @ 2020-08-10 14:29 Kai-Heng Feng 2020-08-10 16:13 ` Hans de Goede 0 siblings, 1 reply; 4+ messages in thread From: Kai-Heng Feng @ 2020-08-10 14:29 UTC (permalink / raw) To: jikos, benjamin.tissoires Cc: Kai-Heng Feng, Hans de Goede, Pavel Balan, Daniel Playfair Cal, HungNien Chen, You-Sheng Yang, Aaron Ma, open list:HID CORE LAYER, open list Goodix touchpad fails to operate in I2C mode after system suspend. According to the vendor, Windows is more forgiving and there's a 60ms delay after SET_POWER ON command. So let's do the same here, to workaround for the touchpads that depend on the delay. Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> --- drivers/hid/i2c-hid/i2c-hid-core.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c index 294c84e136d7..7b24a27fad95 100644 --- a/drivers/hid/i2c-hid/i2c-hid-core.c +++ b/drivers/hid/i2c-hid/i2c-hid-core.c @@ -419,6 +419,9 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state) if (ret) dev_err(&client->dev, "failed to change power setting.\n"); + if (!ret && power_state == I2C_HID_PWR_ON) + msleep(60); + set_pwr_exit: return ret; } -- 2.17.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] HID: i2c-hid: Add 60ms delay after SET_POWER ON 2020-08-10 14:29 [PATCH] HID: i2c-hid: Add 60ms delay after SET_POWER ON Kai-Heng Feng @ 2020-08-10 16:13 ` Hans de Goede 2020-08-11 6:00 ` Kai-Heng Feng 0 siblings, 1 reply; 4+ messages in thread From: Hans de Goede @ 2020-08-10 16:13 UTC (permalink / raw) To: Kai-Heng Feng, jikos, benjamin.tissoires Cc: Pavel Balan, Daniel Playfair Cal, HungNien Chen, You-Sheng Yang, Aaron Ma, open list:HID CORE LAYER, open list Hi, On 10-08-2020 16:29, Kai-Heng Feng wrote: > Goodix touchpad fails to operate in I2C mode after system suspend. > > According to the vendor, Windows is more forgiving and there's a 60ms > delay after SET_POWER ON command. > > So let's do the same here, to workaround for the touchpads that depend > on the delay. > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> Interesting I send a very similar patch a couple of days ago, after debugging some touchpads issues on a BMAX Y13 laptop: https://patchwork.kernel.org/patch/11701541/ If you look at that patch you will see that if we add a sleep on power-on to i2c_hid_set_power(), we can remove an existing sleep after power-on from i2c_hid_hwreset(). And there is an interesting comment there which should probably be moved (as my patch does) and corrected for the new knowledge so that people reading the code in the future now why the sleep is there. Other then that we've come to the same conclusion, but your sleep is much longer. I guess that is ok though, are you sure we need 60ms as a minimum? Is that what goodix said? Regards, Hans > --- > drivers/hid/i2c-hid/i2c-hid-core.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c > index 294c84e136d7..7b24a27fad95 100644 > --- a/drivers/hid/i2c-hid/i2c-hid-core.c > +++ b/drivers/hid/i2c-hid/i2c-hid-core.c > @@ -419,6 +419,9 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state) > if (ret) > dev_err(&client->dev, "failed to change power setting.\n"); > > + if (!ret && power_state == I2C_HID_PWR_ON) > + msleep(60); > + > set_pwr_exit: > return ret; > } > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] HID: i2c-hid: Add 60ms delay after SET_POWER ON 2020-08-10 16:13 ` Hans de Goede @ 2020-08-11 6:00 ` Kai-Heng Feng 2020-08-11 12:59 ` Hans de Goede 0 siblings, 1 reply; 4+ messages in thread From: Kai-Heng Feng @ 2020-08-11 6:00 UTC (permalink / raw) To: Hans de Goede Cc: Jiri Kosina, Benjamin Tissoires, Pavel Balan, Daniel Playfair Cal, HungNien Chen, You-Sheng Yang, Aaron Ma, open list:HID CORE LAYER, open list Hi Hans, > On Aug 11, 2020, at 00:13, Hans de Goede <hdegoede@redhat.com> wrote: > > Hi, > > On 10-08-2020 16:29, Kai-Heng Feng wrote: >> Goodix touchpad fails to operate in I2C mode after system suspend. >> According to the vendor, Windows is more forgiving and there's a 60ms >> delay after SET_POWER ON command. >> So let's do the same here, to workaround for the touchpads that depend >> on the delay. >> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> > > Interesting I send a very similar patch a couple of days ago, > after debugging some touchpads issues on a BMAX Y13 laptop: > > https://patchwork.kernel.org/patch/11701541/ > > If you look at that patch you will see that if we add a > sleep on power-on to i2c_hid_set_power(), we can remove > an existing sleep after power-on from i2c_hid_hwreset(). > > And there is an interesting comment there which should > probably be moved (as my patch does) and corrected for the > new knowledge so that people reading the code in the future > now why the sleep is there. Thanks for the info. Can you please update your patch with 60ms to supersede mine? > > Other then that we've come to the same conclusion, but > your sleep is much longer. I guess that is ok though, > are you sure we need 60ms as a minimum? > Is that what goodix > said? Yes, I was told by Goodix that the 60ms delay is needed. Kai-Heng > > Regards, > > Hans > > >> --- >> drivers/hid/i2c-hid/i2c-hid-core.c | 3 +++ >> 1 file changed, 3 insertions(+) >> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c >> index 294c84e136d7..7b24a27fad95 100644 >> --- a/drivers/hid/i2c-hid/i2c-hid-core.c >> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c >> @@ -419,6 +419,9 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state) >> if (ret) >> dev_err(&client->dev, "failed to change power setting.\n"); >> + if (!ret && power_state == I2C_HID_PWR_ON) >> + msleep(60); >> + >> set_pwr_exit: >> return ret; >> } > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] HID: i2c-hid: Add 60ms delay after SET_POWER ON 2020-08-11 6:00 ` Kai-Heng Feng @ 2020-08-11 12:59 ` Hans de Goede 0 siblings, 0 replies; 4+ messages in thread From: Hans de Goede @ 2020-08-11 12:59 UTC (permalink / raw) To: Kai-Heng Feng Cc: Jiri Kosina, Benjamin Tissoires, Pavel Balan, Daniel Playfair Cal, HungNien Chen, You-Sheng Yang, Aaron Ma, open list:HID CORE LAYER, open list Hi, On 8/11/20 8:00 AM, Kai-Heng Feng wrote: > Hi Hans, > >> On Aug 11, 2020, at 00:13, Hans de Goede <hdegoede@redhat.com> wrote: >> >> Hi, >> >> On 10-08-2020 16:29, Kai-Heng Feng wrote: >>> Goodix touchpad fails to operate in I2C mode after system suspend. >>> According to the vendor, Windows is more forgiving and there's a 60ms >>> delay after SET_POWER ON command. >>> So let's do the same here, to workaround for the touchpads that depend >>> on the delay. >>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com> >> >> Interesting I send a very similar patch a couple of days ago, >> after debugging some touchpads issues on a BMAX Y13 laptop: >> >> https://patchwork.kernel.org/patch/11701541/ >> >> If you look at that patch you will see that if we add a >> sleep on power-on to i2c_hid_set_power(), we can remove >> an existing sleep after power-on from i2c_hid_hwreset(). >> >> And there is an interesting comment there which should >> probably be moved (as my patch does) and corrected for the >> new knowledge so that people reading the code in the future >> now why the sleep is there. > > Thanks for the info. > Can you please update your patch with 60ms to supersede mine? Sure, done. Regards, Hans > >> >> Other then that we've come to the same conclusion, but >> your sleep is much longer. I guess that is ok though, >> are you sure we need 60ms as a minimum? >> Is that what goodix >> said? > > Yes, I was told by Goodix that the 60ms delay is needed. > > Kai-Heng > >> >> Regards, >> >> Hans >> >> >>> --- >>> drivers/hid/i2c-hid/i2c-hid-core.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c >>> index 294c84e136d7..7b24a27fad95 100644 >>> --- a/drivers/hid/i2c-hid/i2c-hid-core.c >>> +++ b/drivers/hid/i2c-hid/i2c-hid-core.c >>> @@ -419,6 +419,9 @@ static int i2c_hid_set_power(struct i2c_client *client, int power_state) >>> if (ret) >>> dev_err(&client->dev, "failed to change power setting.\n"); >>> + if (!ret && power_state == I2C_HID_PWR_ON) >>> + msleep(60); >>> + >>> set_pwr_exit: >>> return ret; >>> } >> > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-08-11 12:59 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-08-10 14:29 [PATCH] HID: i2c-hid: Add 60ms delay after SET_POWER ON Kai-Heng Feng 2020-08-10 16:13 ` Hans de Goede 2020-08-11 6:00 ` Kai-Heng Feng 2020-08-11 12:59 ` Hans de Goede
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).