[SRU] [Bionic/Cosmic] [PATCH 0/2] Fix I2C touchpanels' interrupt storms after system suspend

classic Classic list List threaded Threaded
10 messages Options
Reply | Threaded
Open this post in threaded view
|

[SRU] [Bionic/Cosmic] [PATCH 0/2] Fix I2C touchpanels' interrupt storms after system suspend

Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1792309

[Impact]
Some I2C touchpanels generate IRQ storm after system suspend/resume.

[Test]
After applying the fix, the IRQ storm stops generated by Raydium
touchpanels.

[Fix]
Instead of requesting i2c reset, simply requesting the device to power
on. The HID over I2C spec doesn't specify how the device should do upon
resume from suspend, so some devices expect a power on instead of reset.

[Regression Potential]
Low. I tested other vendors' touchpanels, didn't observe any side
effect.

AceLan Kao (1):
  HID: i2c-hid: Fix flooded incomplete report after S3 on Rayd
    touchscreen

Kai-Heng Feng (1):
  HID: i2c-hid: Don't reset device upon system resume

 drivers/hid/hid-ids.h         |  3 ---
 drivers/hid/i2c-hid/i2c-hid.c | 11 +++++++----
 2 files changed, 7 insertions(+), 7 deletions(-)

--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU] [Bionic] [PATCH 1/2] HID: i2c-hid: Fix flooded incomplete report after S3 on Rayd touchscreen

Kai-Heng Feng
From: AceLan Kao <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1792309

The incomplete report flooded after S3 and touchscreen becomes
malfunctioned.
[ 1367.646244] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete report (58/18785)
[ 1367.649471] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete report (58/28743)
[ 1367.651092] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete report (58/26757)
[ 1367.652658] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete report (58/52280)
[ 1367.654287] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete report (58/56059)

Adding device ID, 04F3:30CC, to the quirk to re-send report description
after resume.

Cc: [hidden email]
Signed-off-by: AceLan Kao <[hidden email]>
Reviewed-by: Benjamin Tissoires <[hidden email]>
Signed-off-by: Jiri Kosina <[hidden email]>
(backported from commit fb6acf76c3fdd97fea6995e64e2c665725f00fc5)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/hid/hid-ids.h         | 1 +
 drivers/hid/i2c-hid/i2c-hid.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 80098ea4d93f..8cf826f7623a 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -519,6 +519,7 @@
 
 #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
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 37c047110241..3773a7fb4d04 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -174,6 +174,8 @@ static const struct i2c_hid_quirks {
  I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
  { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
  I2C_HID_QUIRK_RESEND_REPORT_DESCR },
+ { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_4B33,
+ I2C_HID_QUIRK_RESEND_REPORT_DESCR },
  { 0, 0 }
 };
 
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU] [Bionic] [PATCH 2/2] HID: i2c-hid: Don't reset device upon system resume

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1792309

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 <[hidden email]>
Cc: AceLan Kao <[hidden email]>
Signed-off-by: Kai-Heng Feng <[hidden email]>
Reviewed-by: Benjamin Tissoires <[hidden email]>
Signed-off-by: Jiri Kosina <[hidden email]>
(backported from commit 52cf93e63ee672a92f349edc6ddad86ec8808fd8)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 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 8cf826f7623a..f0ea0bfd0f6f 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -517,10 +517,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 3773a7fb4d04..ccff94c95f89 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -172,10 +172,6 @@ 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 },
- { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_4B33,
- I2C_HID_QUIRK_RESEND_REPORT_DESCR },
  { 0, 0 }
 };
 
@@ -1213,11 +1209,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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU] [Cosmic] [PATCH 1/2] HID: i2c-hid: Fix flooded incomplete report after S3 on Rayd touchscreen

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
From: AceLan Kao <[hidden email]>

BugLink: https://bugs.launchpad.net/bugs/1792309

The incomplete report flooded after S3 and touchscreen becomes
malfunctioned.
[ 1367.646244] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete report (58/18785)
[ 1367.649471] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete report (58/28743)
[ 1367.651092] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete report (58/26757)
[ 1367.652658] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete report (58/52280)
[ 1367.654287] i2c_hid i2c-CUST0000:00: i2c_hid_get_input: incomplete report (58/56059)

Adding device ID, 04F3:30CC, to the quirk to re-send report description
after resume.

Cc: [hidden email]
Signed-off-by: AceLan Kao <[hidden email]>
Reviewed-by: Benjamin Tissoires <[hidden email]>
Signed-off-by: Jiri Kosina <[hidden email]>
(cherry picked from commit fb6acf76c3fdd97fea6995e64e2c665725f00fc5)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/hid/hid-ids.h         | 1 +
 drivers/hid/i2c-hid/i2c-hid.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index b6f38b825a12..6cd9a73ed758 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -528,6 +528,7 @@
 
 #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
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 8cc2b71c680b..d44c622dc0f2 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -176,6 +176,8 @@ static const struct i2c_hid_quirks {
  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 }
 };
 
--
2.17.1


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

[SRU] [Cosmic] [PATCH 2/2] HID: i2c-hid: Don't reset device upon system resume

Kai-Heng Feng
In reply to this post by Kai-Heng Feng
BugLink: https://bugs.launchpad.net/bugs/1792309

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 <[hidden email]>
Cc: AceLan Kao <[hidden email]>
Signed-off-by: Kai-Heng Feng <[hidden email]>
Reviewed-by: Benjamin Tissoires <[hidden email]>
Signed-off-by: Jiri Kosina <[hidden email]>
(cherry picked from commit 52cf93e63ee672a92f349edc6ddad86ec8808fd8)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 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 6cd9a73ed758..ed64868a7207 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -526,10 +526,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 d44c622dc0f2..847b55ad0d60 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -172,12 +172,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 }
 };
 
@@ -1231,11 +1227,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


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK: [SRU] [Bionic/Cosmic] [PATCH 0/2] Fix I2C touchpanels' interrupt storms after system suspend

Aaron Ma
In reply to this post by Kai-Heng Feng
Backport looks good.

Acked-by: Aaron Ma <[hidden email]>

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

ACK/Cmnt: [SRU] [Bionic/Cosmic] [PATCH 0/2] Fix I2C touchpanels' interrupt storms after system suspend

Stefan Bader-2
In reply to this post by Kai-Heng Feng
On 13.09.2018 10:03, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1792309
>
> [Impact]
> Some I2C touchpanels generate IRQ storm after system suspend/resume.
>
> [Test]
> After applying the fix, the IRQ storm stops generated by Raydium
> touchpanels.
>
> [Fix]
> Instead of requesting i2c reset, simply requesting the device to power
> on. The HID over I2C spec doesn't specify how the device should do upon
> resume from suspend, so some devices expect a power on instead of reset.
>
> [Regression Potential]
> Low. I tested other vendors' touchpanels, didn't observe any side
> effect.
>
> AceLan Kao (1):
>   HID: i2c-hid: Fix flooded incomplete report after S3 on Rayd
>     touchscreen
>
> Kai-Heng Feng (1):
>   HID: i2c-hid: Don't reset device upon system resume
>
>  drivers/hid/hid-ids.h         |  3 ---
>  drivers/hid/i2c-hid/i2c-hid.c | 11 +++++++----
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
I saw you tested other panels, too. Still I feel a little uneasy to change
behaviour generally. As the second patch at least only made it upstream to be
included in 4.19, there is not too much feedback to be expected. At least it got
through upstream so lets bites the bullet for now...

Acked-by: Stefan Bader <[hidden email]>


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

APPLIED[C]: [SRU] [Bionic/Cosmic] [PATCH 0/2] Fix I2C touchpanels' interrupt storms after system suspend

Seth Forshee
In reply to this post by Kai-Heng Feng
On Thu, Sep 13, 2018 at 04:03:30PM +0800, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1792309
>
> [Impact]
> Some I2C touchpanels generate IRQ storm after system suspend/resume.
>
> [Test]
> After applying the fix, the IRQ storm stops generated by Raydium
> touchpanels.
>
> [Fix]
> Instead of requesting i2c reset, simply requesting the device to power
> on. The HID over I2C spec doesn't specify how the device should do upon
> resume from suspend, so some devices expect a power on instead of reset.
>
> [Regression Potential]
> Low. I tested other vendors' touchpanels, didn't observe any side
> effect.

Applied to cosmic/master-next, thanks!

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED[oem]: [SRU] [Bionic/Cosmic] [PATCH 0/2] Fix I2C touchpanels' interrupt storms after system suspend

Timo Aaltonen-6
In reply to this post by Kai-Heng Feng
On 13.09.2018 11:03, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1792309
>
> [Impact]
> Some I2C touchpanels generate IRQ storm after system suspend/resume.
>
> [Test]
> After applying the fix, the IRQ storm stops generated by Raydium
> touchpanels.
>
> [Fix]
> Instead of requesting i2c reset, simply requesting the device to power
> on. The HID over I2C spec doesn't specify how the device should do upon
> resume from suspend, so some devices expect a power on instead of reset.
>
> [Regression Potential]
> Low. I tested other vendors' touchpanels, didn't observe any side
> effect.
>
> AceLan Kao (1):
>   HID: i2c-hid: Fix flooded incomplete report after S3 on Rayd
>     touchscreen
>
> Kai-Heng Feng (1):
>   HID: i2c-hid: Don't reset device upon system resume
>
>  drivers/hid/hid-ids.h         |  3 ---
>  drivers/hid/i2c-hid/i2c-hid.c | 11 +++++++----
>  2 files changed, 7 insertions(+), 7 deletions(-)
>

applied to linux-oem (as requested on irc), thanks

--
t

--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team
Reply | Threaded
Open this post in threaded view
|

APPLIED[B]/cmnt: [SRU] [Bionic/Cosmic] [PATCH 0/2] Fix I2C touchpanels' interrupt storms after system suspend

Stefan Bader-2
In reply to this post by Kai-Heng Feng
On 13.09.2018 10:03, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1792309
>
> [Impact]
> Some I2C touchpanels generate IRQ storm after system suspend/resume.
>
> [Test]
> After applying the fix, the IRQ storm stops generated by Raydium
> touchpanels.
>
> [Fix]
> Instead of requesting i2c reset, simply requesting the device to power
> on. The HID over I2C spec doesn't specify how the device should do upon
> resume from suspend, so some devices expect a power on instead of reset.
>
> [Regression Potential]
> Low. I tested other vendors' touchpanels, didn't observe any side
> effect.
>
> AceLan Kao (1):
>   HID: i2c-hid: Fix flooded incomplete report after S3 on Rayd
>     touchscreen
>
> Kai-Heng Feng (1):
>   HID: i2c-hid: Don't reset device upon system resume
>
>  drivers/hid/hid-ids.h         |  3 ---
>  drivers/hid/i2c-hid/i2c-hid.c | 11 +++++++----
>  2 files changed, 7 insertions(+), 7 deletions(-)
>
Due to another entry Bionic needed some context adjustments around the quirk
table to apply.

Applied to bionic/master-next. Thanks.

-Stefan


--
kernel-team mailing list
[hidden email]
https://lists.ubuntu.com/mailman/listinfo/kernel-team

signature.asc (836 bytes) Download Attachment