* [PATCH v2] HID: i2c-hid: Don't reset device upon system resume
@ 2018-09-06 2:55 Kai-Heng Feng
2018-09-06 7:07 ` Benjamin Tissoires
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Kai-Heng Feng @ 2018-09-06 2:55 UTC (permalink / raw)
To: jikos
Cc: benjamin.tissoires, hdegoede, linux-input, linux-kernel,
Kai-Heng Feng, Aaron Ma, AceLan Kao
Raydium touchscreen triggers interrupt storm after system-wide suspend:
[ 179.085033] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete
report (58/65535)
According to Raydium, Windows driver does not reset the device after
system resume.
The HID over I2C spec does specify a reset should be used at
intialization, but it doesn't specify if reset is required for system
suspend.
Tested this patch on other i2c-hid touchpanels I have and those
touchpanels do work after S3 without doing reset. If any regression
happens to other touchpanel vendors, we can use quirk for Raydium
devices.
There's still one device uses I2C_HID_QUIRK_RESEND_REPORT_DESCR so keep
it there.
Cc: Aaron Ma <aaron.ma@canonical.com>
Cc: AceLan Kao <acelan.kao@canonical.com>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
---
v2:
Remove Raydium devices' ID and quirk.
Rewording.
drivers/hid/hid-ids.h | 4 ----
drivers/hid/i2c-hid/i2c-hid.c | 13 +++++++------
2 files changed, 7 insertions(+), 10 deletions(-)
diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 7da93d789080..e254ae802688 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -530,10 +530,6 @@
#define I2C_VENDOR_ID_HANTICK 0x0911
#define I2C_PRODUCT_ID_HANTICK_5288 0x5288
-#define I2C_VENDOR_ID_RAYD 0x2386
-#define I2C_PRODUCT_ID_RAYD_3118 0x3118
-#define I2C_PRODUCT_ID_RAYD_4B33 0x4B33
-
#define USB_VENDOR_ID_HANWANG 0x0b57
#define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000
#define USB_DEVICE_ID_HANWANG_TABLET_LAST 0x8fff
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 57126f6837bb..f3076659361a 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -170,12 +170,8 @@ static const struct i2c_hid_quirks {
I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
{ I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
- { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
- I2C_HID_QUIRK_RESEND_REPORT_DESCR },
{ USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS10FB_TOUCH,
I2C_HID_QUIRK_RESEND_REPORT_DESCR },
- { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_4B33,
- I2C_HID_QUIRK_RESEND_REPORT_DESCR },
{ 0, 0 }
};
@@ -1237,11 +1233,16 @@ static int i2c_hid_resume(struct device *dev)
pm_runtime_enable(dev);
enable_irq(client->irq);
- ret = i2c_hid_hwreset(client);
+
+ /* Instead of resetting device, simply powers the device on. This
+ * solves "incomplete reports" on Raydium devices 2386:3118 and
+ * 2386:4B33
+ */
+ ret = i2c_hid_set_power(client, I2C_HID_PWR_ON);
if (ret)
return ret;
- /* RAYDIUM device (2386:3118) need to re-send report descr cmd
+ /* Some devices need to re-send report descr cmd
* after resume, after this it will be back normal.
* otherwise it issues too many incomplete reports.
*/
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH v2] HID: i2c-hid: Don't reset device upon system resume
2018-09-06 2:55 [PATCH v2] HID: i2c-hid: Don't reset device upon system resume Kai-Heng Feng
@ 2018-09-06 7:07 ` Benjamin Tissoires
2018-09-06 14:32 ` Jiri Kosina
2018-09-09 10:50 ` Hans de Goede
2 siblings, 0 replies; 4+ messages in thread
From: Benjamin Tissoires @ 2018-09-06 7:07 UTC (permalink / raw)
To: Kai Heng Feng
Cc: Jiri Kosina, Hans de Goede, open list:HID CORE LAYER, lkml,
Aaron Ma, AceLan Kao
On Thu, Sep 6, 2018 at 4:55 AM Kai-Heng Feng
<kai.heng.feng@canonical.com> wrote:
>
> Raydium touchscreen triggers interrupt storm after system-wide suspend:
> [ 179.085033] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete
> report (58/65535)
>
> According to Raydium, Windows driver does not reset the device after
> system resume.
>
> The HID over I2C spec does specify a reset should be used at
> intialization, but it doesn't specify if reset is required for system
> suspend.
>
> Tested this patch on other i2c-hid touchpanels I have and those
> touchpanels do work after S3 without doing reset. If any regression
> happens to other touchpanel vendors, we can use quirk for Raydium
> devices.
>
> There's still one device uses I2C_HID_QUIRK_RESEND_REPORT_DESCR so keep
> it there.
>
> Cc: Aaron Ma <aaron.ma@canonical.com>
> Cc: AceLan Kao <acelan.kao@canonical.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Reviewed-by: Benjamin Tissoires <benjamin.tissoires@gmail.com>
Jiri, note that this will replace https://patchwork.kernel.org/patch/10583481/
Cheers,
Benjamin
> ---
> v2:
> Remove Raydium devices' ID and quirk.
> Rewording.
>
> drivers/hid/hid-ids.h | 4 ----
> drivers/hid/i2c-hid/i2c-hid.c | 13 +++++++------
> 2 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 7da93d789080..e254ae802688 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -530,10 +530,6 @@
> #define I2C_VENDOR_ID_HANTICK 0x0911
> #define I2C_PRODUCT_ID_HANTICK_5288 0x5288
>
> -#define I2C_VENDOR_ID_RAYD 0x2386
> -#define I2C_PRODUCT_ID_RAYD_3118 0x3118
> -#define I2C_PRODUCT_ID_RAYD_4B33 0x4B33
> -
> #define USB_VENDOR_ID_HANWANG 0x0b57
> #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000
> #define USB_DEVICE_ID_HANWANG_TABLET_LAST 0x8fff
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 57126f6837bb..f3076659361a 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -170,12 +170,8 @@ static const struct i2c_hid_quirks {
> I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
> { I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
> I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
> - { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
> - I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS10FB_TOUCH,
> I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> - { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_4B33,
> - I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> { 0, 0 }
> };
>
> @@ -1237,11 +1233,16 @@ static int i2c_hid_resume(struct device *dev)
> pm_runtime_enable(dev);
>
> enable_irq(client->irq);
> - ret = i2c_hid_hwreset(client);
> +
> + /* Instead of resetting device, simply powers the device on. This
> + * solves "incomplete reports" on Raydium devices 2386:3118 and
> + * 2386:4B33
> + */
> + ret = i2c_hid_set_power(client, I2C_HID_PWR_ON);
> if (ret)
> return ret;
>
> - /* RAYDIUM device (2386:3118) need to re-send report descr cmd
> + /* Some devices need to re-send report descr cmd
> * after resume, after this it will be back normal.
> * otherwise it issues too many incomplete reports.
> */
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH v2] HID: i2c-hid: Don't reset device upon system resume
2018-09-06 2:55 [PATCH v2] HID: i2c-hid: Don't reset device upon system resume Kai-Heng Feng
2018-09-06 7:07 ` Benjamin Tissoires
@ 2018-09-06 14:32 ` Jiri Kosina
2018-09-09 10:50 ` Hans de Goede
2 siblings, 0 replies; 4+ messages in thread
From: Jiri Kosina @ 2018-09-06 14:32 UTC (permalink / raw)
To: Kai-Heng Feng
Cc: benjamin.tissoires, hdegoede, linux-input, linux-kernel,
Aaron Ma, AceLan Kao
On Thu, 6 Sep 2018, Kai-Heng Feng wrote:
> Raydium touchscreen triggers interrupt storm after system-wide suspend:
> [ 179.085033] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete
> report (58/65535)
>
> According to Raydium, Windows driver does not reset the device after
> system resume.
>
> The HID over I2C spec does specify a reset should be used at
> intialization, but it doesn't specify if reset is required for system
> suspend.
>
> Tested this patch on other i2c-hid touchpanels I have and those
> touchpanels do work after S3 without doing reset. If any regression
> happens to other touchpanel vendors, we can use quirk for Raydium
> devices.
>
> There's still one device uses I2C_HID_QUIRK_RESEND_REPORT_DESCR so keep
> it there.
>
> Cc: Aaron Ma <aaron.ma@canonical.com>
> Cc: AceLan Kao <acelan.kao@canonical.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Queued in for-4.19/fixes. Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] HID: i2c-hid: Don't reset device upon system resume
2018-09-06 2:55 [PATCH v2] HID: i2c-hid: Don't reset device upon system resume Kai-Heng Feng
2018-09-06 7:07 ` Benjamin Tissoires
2018-09-06 14:32 ` Jiri Kosina
@ 2018-09-09 10:50 ` Hans de Goede
2 siblings, 0 replies; 4+ messages in thread
From: Hans de Goede @ 2018-09-09 10:50 UTC (permalink / raw)
To: Kai-Heng Feng, jikos
Cc: benjamin.tissoires, linux-input, linux-kernel, Aaron Ma, AceLan Kao
Hi,
On 06-09-18 04:55, Kai-Heng Feng wrote:
> Raydium touchscreen triggers interrupt storm after system-wide suspend:
> [ 179.085033] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete
> report (58/65535)
>
> According to Raydium, Windows driver does not reset the device after
> system resume.
>
> The HID over I2C spec does specify a reset should be used at
> intialization, but it doesn't specify if reset is required for system
> suspend.
>
> Tested this patch on other i2c-hid touchpanels I have and those
> touchpanels do work after S3 without doing reset. If any regression
> happens to other touchpanel vendors, we can use quirk for Raydium
> devices.
>
> There's still one device uses I2C_HID_QUIRK_RESEND_REPORT_DESCR so keep
> it there.
I added that quirk and I still have the hardware for it. I've tested
things with the quirk remove and the touchscreen works after resume
now without the quirk.
I also have some other devices with a SIS i2c-hid touchscreen which
would no longer work after resume where the quirk did not help.
I had looking into those on my TODO but did not get around to
this yet. I've re-tested those with this patch and I'm happy to report
that this patch also fixes resume on the SIS touchscreens in the
following models:
Asus T100HA
Asus T200TA
Peaq C1010
Regards,
Hans
>
> Cc: Aaron Ma <aaron.ma@canonical.com>
> Cc: AceLan Kao <acelan.kao@canonical.com>
> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> ---
> v2:
> Remove Raydium devices' ID and quirk.
> Rewording.
>
> drivers/hid/hid-ids.h | 4 ----
> drivers/hid/i2c-hid/i2c-hid.c | 13 +++++++------
> 2 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
> index 7da93d789080..e254ae802688 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -530,10 +530,6 @@
> #define I2C_VENDOR_ID_HANTICK 0x0911
> #define I2C_PRODUCT_ID_HANTICK_5288 0x5288
>
> -#define I2C_VENDOR_ID_RAYD 0x2386
> -#define I2C_PRODUCT_ID_RAYD_3118 0x3118
> -#define I2C_PRODUCT_ID_RAYD_4B33 0x4B33
> -
> #define USB_VENDOR_ID_HANWANG 0x0b57
> #define USB_DEVICE_ID_HANWANG_TABLET_FIRST 0x5000
> #define USB_DEVICE_ID_HANWANG_TABLET_LAST 0x8fff
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 57126f6837bb..f3076659361a 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -170,12 +170,8 @@ static const struct i2c_hid_quirks {
> I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
> { I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
> I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
> - { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
> - I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS10FB_TOUCH,
> I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> - { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_4B33,
> - I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> { 0, 0 }
> };
>
> @@ -1237,11 +1233,16 @@ static int i2c_hid_resume(struct device *dev)
> pm_runtime_enable(dev);
>
> enable_irq(client->irq);
> - ret = i2c_hid_hwreset(client);
> +
> + /* Instead of resetting device, simply powers the device on. This
> + * solves "incomplete reports" on Raydium devices 2386:3118 and
> + * 2386:4B33
> + */
> + ret = i2c_hid_set_power(client, I2C_HID_PWR_ON);
> if (ret)
> return ret;
>
> - /* RAYDIUM device (2386:3118) need to re-send report descr cmd
> + /* Some devices need to re-send report descr cmd
> * after resume, after this it will be back normal.
> * otherwise it issues too many incomplete reports.
> */
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-09-09 10:50 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-06 2:55 [PATCH v2] HID: i2c-hid: Don't reset device upon system resume Kai-Heng Feng
2018-09-06 7:07 ` Benjamin Tissoires
2018-09-06 14:32 ` Jiri Kosina
2018-09-09 10:50 ` Hans de Goede
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.