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

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

[Johnny Chuang]

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(-)

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

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

[Harry Cutts]

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
>

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

[Doug Anderson]

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>

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

[Johnny.Chuang]

> 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>

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

[Johnny.Chuang]

> > 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>

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

[Benjamin Tissoires]

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
>