| Message ID | 1618276850-27178-1-git-send-email-johnny.chuang.emc@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] HID: i2c-hid: Skip ELAN power-on command after reset | expand |
On Mon, 12 Apr 2021 at 18:20, Johnny Chuang <johnny.chuang.emc@gmail.com> wrote: > > Fixes: 43b7029f475e ("HID: i2c-hid: Send power-on command after reset"). > > For ELAN touchscreen, we found our boot code of IC was not flexible enough > to receive and handle this command. > Once the FW main code of our controller is crashed for some reason, > the controller could not be enumerated successfully to be recognized > by the system host. therefore, it lost touch functionality. > > Add quirk for skip send power-on command after reset. > It will impact to ELAN touchscreen and touchpad on HID over I2C projects. > > Signed-off-by: Johnny Chuang <johnny.chuang.emc@gmail.com> > --- > Changes in V3: > - intent the comment at qurik entry > - add Fixes:flag for previous commit id > > Changes in v2: > - move comment to quirk entry Reviewed-by: Harry Cutts <hcutts@chromium.org> Harry Cutts Chrome OS Touch/Input team > --- > drivers/hid/i2c-hid/i2c-hid-core.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c > index 9993133..32e3287 100644 > --- a/drivers/hid/i2c-hid/i2c-hid-core.c > +++ b/drivers/hid/i2c-hid/i2c-hid-core.c > @@ -45,6 +45,7 @@ > #define I2C_HID_QUIRK_BOGUS_IRQ BIT(4) > #define I2C_HID_QUIRK_RESET_ON_RESUME BIT(5) > #define I2C_HID_QUIRK_BAD_INPUT_SIZE BIT(6) > +#define I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET BIT(7) > > > /* flags */ > @@ -178,6 +179,12 @@ static const struct i2c_hid_quirks { > I2C_HID_QUIRK_RESET_ON_RESUME }, > { USB_VENDOR_ID_ITE, I2C_DEVICE_ID_ITE_LENOVO_LEGION_Y720, > I2C_HID_QUIRK_BAD_INPUT_SIZE }, > + /* > + * Sending the wakeup after reset actually break ELAN touchscreen controller > + * Add I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET to skip wakeup after reset > + */ > + { USB_VENDOR_ID_ELAN, HID_ANY_ID, > + I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET }, > { 0, 0 } > }; > > @@ -461,7 +468,8 @@ static int i2c_hid_hwreset(struct i2c_client *client) > } > > /* At least some SIS devices need this after reset */ > - ret = i2c_hid_set_power(client, I2C_HID_PWR_ON); > + if (!(ihid->quirks & I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET)) > + ret = i2c_hid_set_power(client, I2C_HID_PWR_ON); > > out_unlock: > mutex_unlock(&ihid->reset_lock); > -- > 2.7.4 >
Hi, On Mon, Apr 12, 2021 at 6:20 PM Johnny Chuang <johnny.chuang.emc@gmail.com> wrote: > > Fixes: 43b7029f475e ("HID: i2c-hid: Send power-on command after reset"). Note that the "Fixes" tag actually belongs down at the end. It also shouldn't have a "." at the end. Presumably the maintainer can adjust this when landing? > For ELAN touchscreen, we found our boot code of IC was not flexible enough > to receive and handle this command. > Once the FW main code of our controller is crashed for some reason, > the controller could not be enumerated successfully to be recognized > by the system host. therefore, it lost touch functionality. > > Add quirk for skip send power-on command after reset. > It will impact to ELAN touchscreen and touchpad on HID over I2C projects. > > Signed-off-by: Johnny Chuang <johnny.chuang.emc@gmail.com> This patch looks fine to me, thus: Reviewed-by: Douglas Anderson <dianders@chromium.org> I can confirm that after applying this patch I can recovery my borked touchscreen (which got borked by a failed firmware update ages ago): Tested-by: Douglas Anderson <dianders@chromium.org>
> Hi, > > On Mon, Apr 12, 2021 at 6:20 PM Johnny Chuang > <johnny.chuang.emc@gmail.com> wrote: > > > > Fixes: 43b7029f475e ("HID: i2c-hid: Send power-on command after reset"). > > Note that the "Fixes" tag actually belongs down at the end. It also shouldn't > have a "." at the end. Presumably the maintainer can adjust this when landing? > Hi Dmitry, Could you help to review this patch and give an advice? > > > For ELAN touchscreen, we found our boot code of IC was not flexible > > enough to receive and handle this command. > > Once the FW main code of our controller is crashed for some reason, > > the controller could not be enumerated successfully to be recognized > > by the system host. therefore, it lost touch functionality. > > > > Add quirk for skip send power-on command after reset. > > It will impact to ELAN touchscreen and touchpad on HID over I2C projects. > > > > Signed-off-by: Johnny Chuang <johnny.chuang.emc@gmail.com> > > This patch looks fine to me, thus: > > Reviewed-by: Douglas Anderson <dianders@chromium.org> > > I can confirm that after applying this patch I can recovery my borked > touchscreen (which got borked by a failed firmware update ages ago): > > Tested-by: Douglas Anderson <dianders@chromium.org>
> > Hi, > > > > On Mon, Apr 12, 2021 at 6:20 PM Johnny Chuang > > <johnny.chuang.emc@gmail.com> wrote: > > > > > > Fixes: 43b7029f475e ("HID: i2c-hid: Send power-on command after reset"). > > > > Note that the "Fixes" tag actually belongs down at the end. It also > > shouldn't have a "." at the end. Presumably the maintainer can adjust this > when landing? > > > > Hi Dmitry, > Could you help to review this patch and give an advice? Hi Sirs, Could anyone help to review this patch and give an advice? > > > > > > For ELAN touchscreen, we found our boot code of IC was not flexible > > > enough to receive and handle this command. > > > Once the FW main code of our controller is crashed for some reason, > > > the controller could not be enumerated successfully to be recognized > > > by the system host. therefore, it lost touch functionality. > > > > > > Add quirk for skip send power-on command after reset. > > > It will impact to ELAN touchscreen and touchpad on HID over I2C projects. > > > > > > Signed-off-by: Johnny Chuang <johnny.chuang.emc@gmail.com> > > > > This patch looks fine to me, thus: > > > > Reviewed-by: Douglas Anderson <dianders@chromium.org> > > > > I can confirm that after applying this patch I can recovery my borked > > touchscreen (which got borked by a failed firmware update ages ago): > > > > Tested-by: Douglas Anderson <dianders@chromium.org>
Hi Johnny, Apologies fo the delay. When I first saw your patch I wondered why we need a special case for Elan. But then, we need a special case for SIS, as mentioned by 43b7029f475e. Given that this patch was in for a year and a half, and not many people seemed to complain about it, I decided to apply your patch. On Tue, Apr 13, 2021 at 3:21 AM Johnny Chuang <johnny.chuang.emc@gmail.com> wrote: > > Fixes: 43b7029f475e ("HID: i2c-hid: Send power-on command after reset"). As requested per Doug, I have moved that Fixes tag at the end, though I forgot to remove the extra '.' at the end :-( > > For ELAN touchscreen, we found our boot code of IC was not flexible enough > to receive and handle this command. > Once the FW main code of our controller is crashed for some reason, > the controller could not be enumerated successfully to be recognized > by the system host. therefore, it lost touch functionality. > > Add quirk for skip send power-on command after reset. > It will impact to ELAN touchscreen and touchpad on HID over I2C projects. > > Signed-off-by: Johnny Chuang <johnny.chuang.emc@gmail.com> > --- > Changes in V3: > - intent the comment at qurik entry > - add Fixes:flag for previous commit id > > Changes in v2: > - move comment to quirk entry > --- > drivers/hid/i2c-hid/i2c-hid-core.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c > index 9993133..32e3287 100644 > --- a/drivers/hid/i2c-hid/i2c-hid-core.c > +++ b/drivers/hid/i2c-hid/i2c-hid-core.c > @@ -45,6 +45,7 @@ > #define I2C_HID_QUIRK_BOGUS_IRQ BIT(4) > #define I2C_HID_QUIRK_RESET_ON_RESUME BIT(5) > #define I2C_HID_QUIRK_BAD_INPUT_SIZE BIT(6) > +#define I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET BIT(7) > > > /* flags */ > @@ -178,6 +179,12 @@ static const struct i2c_hid_quirks { > I2C_HID_QUIRK_RESET_ON_RESUME }, > { USB_VENDOR_ID_ITE, I2C_DEVICE_ID_ITE_LENOVO_LEGION_Y720, > I2C_HID_QUIRK_BAD_INPUT_SIZE }, > + /* > + * Sending the wakeup after reset actually break ELAN touchscreen controller > + * Add I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET to skip wakeup after reset I removed that extra second line. Also added Cc: stable, and pushed to for-5.13/upstream-fixes Cheers, Benjamin > + */ > + { USB_VENDOR_ID_ELAN, HID_ANY_ID, > + I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET }, > { 0, 0 } > }; > > @@ -461,7 +468,8 @@ static int i2c_hid_hwreset(struct i2c_client *client) > } > > /* At least some SIS devices need this after reset */ > - ret = i2c_hid_set_power(client, I2C_HID_PWR_ON); > + if (!(ihid->quirks & I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET)) > + ret = i2c_hid_set_power(client, I2C_HID_PWR_ON); > > out_unlock: > mutex_unlock(&ihid->reset_lock); > -- > 2.7.4 >
diff --git a/drivers/hid/i2c-hid/i2c-hid-core.c b/drivers/hid/i2c-hid/i2c-hid-core.c index 9993133..32e3287 100644 --- a/drivers/hid/i2c-hid/i2c-hid-core.c +++ b/drivers/hid/i2c-hid/i2c-hid-core.c @@ -45,6 +45,7 @@ #define I2C_HID_QUIRK_BOGUS_IRQ BIT(4) #define I2C_HID_QUIRK_RESET_ON_RESUME BIT(5) #define I2C_HID_QUIRK_BAD_INPUT_SIZE BIT(6) +#define I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET BIT(7) /* flags */ @@ -178,6 +179,12 @@ static const struct i2c_hid_quirks { I2C_HID_QUIRK_RESET_ON_RESUME }, { USB_VENDOR_ID_ITE, I2C_DEVICE_ID_ITE_LENOVO_LEGION_Y720, I2C_HID_QUIRK_BAD_INPUT_SIZE }, + /* + * Sending the wakeup after reset actually break ELAN touchscreen controller + * Add I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET to skip wakeup after reset + */ + { USB_VENDOR_ID_ELAN, HID_ANY_ID, + I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET }, { 0, 0 } }; @@ -461,7 +468,8 @@ static int i2c_hid_hwreset(struct i2c_client *client) } /* At least some SIS devices need this after reset */ - ret = i2c_hid_set_power(client, I2C_HID_PWR_ON); + if (!(ihid->quirks & I2C_HID_QUIRK_NO_WAKEUP_AFTER_RESET)) + ret = i2c_hid_set_power(client, I2C_HID_PWR_ON); out_unlock: mutex_unlock(&ihid->reset_lock);
Fixes: 43b7029f475e ("HID: i2c-hid: Send power-on command after reset"). For ELAN touchscreen, we found our boot code of IC was not flexible enough to receive and handle this command. Once the FW main code of our controller is crashed for some reason, the controller could not be enumerated successfully to be recognized by the system host. therefore, it lost touch functionality. Add quirk for skip send power-on command after reset. It will impact to ELAN touchscreen and touchpad on HID over I2C projects. Signed-off-by: Johnny Chuang <johnny.chuang.emc@gmail.com> --- Changes in V3: - intent the comment at qurik entry - add Fixes:flag for previous commit id Changes in v2: - move comment to quirk entry --- drivers/hid/i2c-hid/i2c-hid-core.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)