* Re: [PATCH v3] HID: i2c-hid: Skip ELAN power-on command after reset
2021-04-13 1:20 [PATCH v3] HID: i2c-hid: Skip ELAN power-on command after reset Johnny Chuang
@ 2021-04-13 17:29 ` Harry Cutts
2021-04-14 15:28 ` Doug Anderson
2021-05-07 13:38 ` Benjamin Tissoires
2 siblings, 0 replies; 6+ messages in thread
From: Harry Cutts @ 2021-04-13 17:29 UTC (permalink / raw)
To: Johnny Chuang
Cc: Dmitry Torokhov, Benjamin Tissoires, Peter Hutterer, lkml,
linux-input, Johnny Chuang, James Chen, Jennifer Tsai,
Paul Liang, Jeff Chuang, Douglas Anderson, Jingle
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v3] HID: i2c-hid: Skip ELAN power-on command after reset
2021-04-13 1:20 [PATCH v3] HID: i2c-hid: Skip ELAN power-on command after reset Johnny Chuang
2021-04-13 17:29 ` Harry Cutts
@ 2021-04-14 15:28 ` Doug Anderson
2021-04-23 7:54 ` Johnny.Chuang
2021-05-07 8:44 ` Johnny.Chuang
2021-05-07 13:38 ` Benjamin Tissoires
2 siblings, 2 replies; 6+ messages in thread
From: Doug Anderson @ 2021-04-14 15:28 UTC (permalink / raw)
To: Johnny Chuang
Cc: Dmitry Torokhov, Benjamin Tissoires, Peter Hutterer, LKML,
open list:HID CORE LAYER, Harry Cutts, Johnny Chuang, James Chen,
Jennifer Tsai, Paul Liang, Jeff Chuang, Jingle
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>
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH v3] HID: i2c-hid: Skip ELAN power-on command after reset
2021-04-14 15:28 ` Doug Anderson
@ 2021-04-23 7:54 ` Johnny.Chuang
2021-05-07 8:44 ` Johnny.Chuang
1 sibling, 0 replies; 6+ messages in thread
From: Johnny.Chuang @ 2021-04-23 7:54 UTC (permalink / raw)
To: 'Doug Anderson', 'Johnny Chuang'
Cc: 'Dmitry Torokhov', 'Benjamin Tissoires',
'Peter Hutterer', 'LKML',
'open list:HID CORE LAYER', 'Harry Cutts',
'James Chen', 'Jennifer Tsai',
'Paul Liang', 'Jeff Chuang', 'Jingle'
> 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>
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH v3] HID: i2c-hid: Skip ELAN power-on command after reset
2021-04-14 15:28 ` Doug Anderson
2021-04-23 7:54 ` Johnny.Chuang
@ 2021-05-07 8:44 ` Johnny.Chuang
1 sibling, 0 replies; 6+ messages in thread
From: Johnny.Chuang @ 2021-05-07 8:44 UTC (permalink / raw)
To: 'Doug Anderson', 'Johnny Chuang'
Cc: 'Dmitry Torokhov', 'Benjamin Tissoires',
'Peter Hutterer', 'LKML',
'open list:HID CORE LAYER', 'Harry Cutts',
'James Chen', 'Jennifer Tsai',
'Paul Liang', 'Jeff Chuang', 'Jingle'
> > 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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] HID: i2c-hid: Skip ELAN power-on command after reset
2021-04-13 1:20 [PATCH v3] HID: i2c-hid: Skip ELAN power-on command after reset Johnny Chuang
2021-04-13 17:29 ` Harry Cutts
2021-04-14 15:28 ` Doug Anderson
@ 2021-05-07 13:38 ` Benjamin Tissoires
2 siblings, 0 replies; 6+ messages in thread
From: Benjamin Tissoires @ 2021-05-07 13:38 UTC (permalink / raw)
To: Johnny Chuang
Cc: Dmitry Torokhov, Peter Hutterer, lkml, open list:HID CORE LAYER,
Harry Cutts, Johnny Chuang, James Chen, Jennifer Tsai,
Paul Liang, Jeff Chuang, Douglas Anderson, Jingle
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
>
^ permalink raw reply [flat|nested] 6+ messages in thread