2021-04-13 12:28:18

by Johnny Chuang

[permalink] [raw]
Subject: [PATCH v3] HID: i2c-hid: Skip ELAN power-on command after reset

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 <[email protected]>
---
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
+ */
+ { 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


2021-04-13 22:11:07

by Harry Cutts

[permalink] [raw]
Subject: Re: [PATCH v3] HID: i2c-hid: Skip ELAN power-on command after reset

On Mon, 12 Apr 2021 at 18:20, Johnny Chuang <[email protected]> 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 <[email protected]>
> ---
> 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 <[email protected]>

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
>

2021-04-14 17:17:55

by Doug Anderson

[permalink] [raw]
Subject: Re: [PATCH v3] HID: i2c-hid: Skip ELAN power-on command after reset

Hi,

On Mon, Apr 12, 2021 at 6:20 PM Johnny Chuang
<[email protected]> 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 <[email protected]>

This patch looks fine to me, thus:

Reviewed-by: Douglas Anderson <[email protected]>

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 <[email protected]>

2021-04-23 08:05:19

by Johnny.Chuang

[permalink] [raw]
Subject: RE: [PATCH v3] HID: i2c-hid: Skip ELAN power-on command after reset

> Hi,
>
> On Mon, Apr 12, 2021 at 6:20 PM Johnny Chuang
> <[email protected]> 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 <[email protected]>
>
> This patch looks fine to me, thus:
>
> Reviewed-by: Douglas Anderson <[email protected]>
>
> 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 <[email protected]>

2021-05-07 12:16:32

by Johnny.Chuang

[permalink] [raw]
Subject: RE: [PATCH v3] HID: i2c-hid: Skip ELAN power-on command after reset

> > Hi,
> >
> > On Mon, Apr 12, 2021 at 6:20 PM Johnny Chuang
> > <[email protected]> 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 <[email protected]>
> >
> > This patch looks fine to me, thus:
> >
> > Reviewed-by: Douglas Anderson <[email protected]>
> >
> > 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 <[email protected]>

2021-05-07 17:14:31

by Benjamin Tissoires

[permalink] [raw]
Subject: Re: [PATCH v3] HID: i2c-hid: Skip ELAN power-on command after reset

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
<[email protected]> 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 <[email protected]>
> ---
> 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
>