[PATCH 0/2][A/B/OEM] HID:Fix RayD touchscreen failure to resume from S3

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/2][A/B/OEM] HID:Fix RayD touchscreen failure to resume from S3

Aaron Ma
[Impact]
Resume of S3 cause system hang on Lenovo laptops with RayD touchscreen.

[Fix]
RayD touchscreen issued corrupted data after it resume from suspend.
Then HID driver didn't handle the data well,
because of the data overflow then it made a call trace and hang.
Keep memset safe from corrupted data and add command to make RayD
touchscreen back to normal.
Then touchscreen works fine.

[Test Case]
Tested on ThinkPad L380, works fine with fixes.

[Regression Potential]
Low, keep the data in hid-core and i2c-hid more strictly.
Add specific device quirk in i2c-hid.

Aaron Ma (2):
  HID: core: i2c-hid: fix size check and type usage
  HID: i2c-hid: Fix resume issue on Raydium touchscreen device

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

--
2.14.3


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

[PATCH 1/2] HID: core: i2c-hid: fix size check and type usage

Aaron Ma
BugLink: https://bugs.launchpad.net/bugs/1741168

Link: https://patchwork.kernel.org/patch/10141099/

When convert char array with signed int, if the inbuf[x] is negative then
upper bits will be set to 1. Fix this by using u8 instead of char.

ret_size has to be at least 3, hid_input_report use it after minus 2 bytes.

size should be more than 0 to keep memset safe.

Cc: [hidden email]
Signed-off-by: Aaron Ma <[hidden email]>
---
 drivers/hid/hid-core.c        |  4 ++--
 drivers/hid/i2c-hid/i2c-hid.c | 10 +++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
index efb3501b4123..c40dc6406966 100644
--- a/drivers/hid/hid-core.c
+++ b/drivers/hid/hid-core.c
@@ -1506,7 +1506,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
  if (rsize > HID_MAX_BUFFER_SIZE)
  rsize = HID_MAX_BUFFER_SIZE;
 
- if (csize < rsize) {
+ if ((csize < rsize) && (csize > 0)) {
  dbg_hid("report %d is too short, (%d < %d)\n", report->id,
  csize, rsize);
  memset(cdata + csize, 0, rsize - csize);
@@ -1566,7 +1566,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int i
  report_enum = hid->report_enum + type;
  hdrv = hid->driver;
 
- if (!size) {
+ if (size <= 0) {
  dbg_hid("empty report\n");
  ret = -1;
  goto unlock;
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 364150435c62..4b3a703f297b 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -143,10 +143,10 @@ struct i2c_hid {
    * register of the HID
    * descriptor. */
  unsigned int bufsize; /* i2c buffer size */
- char *inbuf; /* Input buffer */
- char *rawbuf; /* Raw Input buffer */
- char *cmdbuf; /* Command buffer */
- char *argsbuf; /* Command arguments buffer */
+ u8 *inbuf; /* Input buffer */
+ u8 *rawbuf; /* Raw Input buffer */
+ u8 *cmdbuf; /* Command buffer */
+ u8 *argsbuf; /* Command arguments buffer */
 
  unsigned long flags; /* device flags */
  unsigned long quirks; /* Various quirks */
@@ -468,7 +468,7 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
 
  ret_size = ihid->inbuf[0] | ihid->inbuf[1] << 8;
 
- if (!ret_size) {
+ if (ret_size <= 2) {
  /* host or device initiated RESET completed */
  if (test_and_clear_bit(I2C_HID_RESET_PENDING, &ihid->flags))
  wake_up(&ihid->wait);
--
2.14.3


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

[PATCH 2/2] HID: i2c-hid: Fix resume issue on Raydium touchscreen device

Aaron Ma
In reply to this post by Aaron Ma
BugLink: https://bugs.launchpad.net/bugs/1741168

Link: https://patchwork.kernel.org/patch/10141097/

When Rayd touchscreen resumed from S3, it issues too many errors like:
i2c_hid i2c-RAYD0001:00: i2c_hid_get_input: incomplete report (58/5442)

And all the report data are corrupted, touchscreen is unresponsive.

Fix this by re-sending report description command after resume.
Add device ID as a quirk.

Cc: [hidden email]
Signed-off-by: Aaron Ma <[hidden email]>
---
 drivers/hid/hid-ids.h         |  3 +++
 drivers/hid/i2c-hid/i2c-hid.c | 13 +++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 60e26e206cd9..903f68419794 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -508,6 +508,9 @@
 #define USB_DEVICE_ID_GYRATION_REMOTE_2 0x0003
 #define USB_DEVICE_ID_GYRATION_REMOTE_3 0x0008
 
+#define I2C_VENDOR_ID_RAYD 0x2386
+#define I2C_PRODUCT_ID_RAYD_3118 0x3118
+
 #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 4b3a703f297b..c8ecbbc470c1 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -46,6 +46,7 @@
 
 /* quirks to control the device */
 #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV BIT(0)
+#define I2C_HID_QUIRK_RESEND_REPORT_DESCR BIT(2)
 
 /* flags */
 #define I2C_HID_STARTED 0
@@ -168,6 +169,8 @@ static const struct i2c_hid_quirks {
  I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
  { USB_VENDOR_ID_WEIDA, USB_DEVICE_ID_WEIDA_8755,
  I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
+ { I2C_VENDOR_ID_RAYD, I2C_PRODUCT_ID_RAYD_3118,
+ I2C_HID_QUIRK_RESEND_REPORT_DESCR },
  { 0, 0 }
 };
 
@@ -1205,6 +1208,16 @@ static int i2c_hid_resume(struct device *dev)
  if (ret)
  return ret;
 
+ /* RAYDIUM device (2386:3118) need to re-send report descr cmd
+ * after resume, after this it will be back normal.
+ * otherwise it issues too many incomplete reports.
+ */
+ if (ihid->quirks & I2C_HID_QUIRK_RESEND_REPORT_DESCR) {
+ ret = i2c_hid_command(client, &hid_report_descr_cmd, NULL, 0);
+ if (!ret)
+ return ret;
+ }
+
  if (hid->driver && hid->driver->reset_resume) {
  ret = hid->driver->reset_resume(hid);
  return ret;
--
2.14.3


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

Re: [PATCH 0/2][A/B/OEM] HID:Fix RayD touchscreen failure to resume from S3

Aaron Ma
In reply to this post by Aaron Ma
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1/2] HID: core: i2c-hid: fix size check and type usage

Kai-Heng Feng
In reply to this post by Aaron Ma


> On 4 Jan 2018, at 2:06 PM, Aaron Ma <[hidden email]> wrote:
>
> BugLink: https://bugs.launchpad.net/bugs/1741168
>
> Link: https://patchwork.kernel.org/patch/10141099/
>
> When convert char array with signed int, if the inbuf[x] is negative then
> upper bits will be set to 1. Fix this by using u8 instead of char.
>
> ret_size has to be at least 3, hid_input_report use it after minus 2 bytes.
>
> size should be more than 0 to keep memset safe.
>
> Cc: [hidden email]
> Signed-off-by: Aaron Ma <[hidden email]>
> ---
> drivers/hid/hid-core.c        |  4 ++--
> drivers/hid/i2c-hid/i2c-hid.c | 10 +++++-----
> 2 files changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/hid/hid-core.c b/drivers/hid/hid-core.c
> index efb3501b4123..c40dc6406966 100644
> --- a/drivers/hid/hid-core.c
> +++ b/drivers/hid/hid-core.c
> @@ -1506,7 +1506,7 @@ int hid_report_raw_event(struct hid_device *hid, int type, u8 *data, int size,
> if (rsize > HID_MAX_BUFFER_SIZE)
> rsize = HID_MAX_BUFFER_SIZE;
>
> - if (csize < rsize) {
> + if ((csize < rsize) && (csize > 0)) {
> dbg_hid("report %d is too short, (%d < %d)\n", report->id,
> csize, rsize);
> memset(cdata + csize, 0, rsize - csize);

Why is skipping memset() when csize == 0 necessary?

Kai-Heng

> @@ -1566,7 +1566,7 @@ int hid_input_report(struct hid_device *hid, int type, u8 *data, int size, int i
> report_enum = hid->report_enum + type;
> hdrv = hid->driver;
>
> - if (!size) {
> + if (size <= 0) {
> dbg_hid("empty report\n");
> ret = -1;
> goto unlock;
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 364150435c62..4b3a703f297b 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -143,10 +143,10 @@ struct i2c_hid {
>   * register of the HID
>   * descriptor. */
> unsigned int bufsize; /* i2c buffer size */
> - char *inbuf; /* Input buffer */
> - char *rawbuf; /* Raw Input buffer */
> - char *cmdbuf; /* Command buffer */
> - char *argsbuf; /* Command arguments buffer */
> + u8 *inbuf; /* Input buffer */
> + u8 *rawbuf; /* Raw Input buffer */
> + u8 *cmdbuf; /* Command buffer */
> + u8 *argsbuf; /* Command arguments buffer */
>
> unsigned long flags; /* device flags */
> unsigned long quirks; /* Various quirks */
> @@ -468,7 +468,7 @@ static void i2c_hid_get_input(struct i2c_hid *ihid)
>
> ret_size = ihid->inbuf[0] | ihid->inbuf[1] << 8;
>
> - if (!ret_size) {
> + if (ret_size <= 2) {
> /* host or device initiated RESET completed */
> if (test_and_clear_bit(I2C_HID_RESET_PENDING, &ihid->flags))
> wake_up(&ihid->wait);
> --
> 2.14.3
>
>
> --
> kernel-team mailing list
> [hidden email]
> https://lists.ubuntu.com/mailman/listinfo/kernel-team


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

Re: [PATCH 0/2][A/B/OEM] HID:Fix RayD touchscreen failure to resume from S3

Hui Wang
In reply to this post by Aaron Ma
Looks like upstream starts to review your patchset, do you plan to
change the patch and resend them in the near future?



On 2018年01月04日 14:06, Aaron Ma wrote:

> [Impact]
> Resume of S3 cause system hang on Lenovo laptops with RayD touchscreen.
>
> [Fix]
> RayD touchscreen issued corrupted data after it resume from suspend.
> Then HID driver didn't handle the data well,
> because of the data overflow then it made a call trace and hang.
> Keep memset safe from corrupted data and add command to make RayD
> touchscreen back to normal.
> Then touchscreen works fine.
>
> [Test Case]
> Tested on ThinkPad L380, works fine with fixes.
>
> [Regression Potential]
> Low, keep the data in hid-core and i2c-hid more strictly.
> Add specific device quirk in i2c-hid.
>
> Aaron Ma (2):
>    HID: core: i2c-hid: fix size check and type usage
>    HID: i2c-hid: Fix resume issue on Raydium touchscreen device
>
>   drivers/hid/hid-core.c        |  4 ++--
>   drivers/hid/hid-ids.h         |  3 +++
>   drivers/hid/i2c-hid/i2c-hid.c | 23 ++++++++++++++++++-----
>   3 files changed, 23 insertions(+), 7 deletions(-)
>


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

Re: [PATCH 0/2][A/B/OEM] HID:Fix RayD touchscreen failure to resume from S3

Aaron Ma
Hmm, got some advice from maintainer.
I will re-work these patches.
Please ignore this SRU for now.

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

NACK: [PATCH 0/2][A/B/OEM] HID:Fix RayD touchscreen failure to resume from S3

Kleber Souza
In reply to this post by Aaron Ma
On 01/04/18 07:06, Aaron Ma wrote:

> [Impact]
> Resume of S3 cause system hang on Lenovo laptops with RayD touchscreen.
>
> [Fix]
> RayD touchscreen issued corrupted data after it resume from suspend.
> Then HID driver didn't handle the data well,
> because of the data overflow then it made a call trace and hang.
> Keep memset safe from corrupted data and add command to make RayD
> touchscreen back to normal.
> Then touchscreen works fine.
>
> [Test Case]
> Tested on ThinkPad L380, works fine with fixes.
>
> [Regression Potential]
> Low, keep the data in hid-core and i2c-hid more strictly.
> Add specific device quirk in i2c-hid.
>
> Aaron Ma (2):
>   HID: core: i2c-hid: fix size check and type usage
>   HID: i2c-hid: Fix resume issue on Raydium touchscreen device
>
>  drivers/hid/hid-core.c        |  4 ++--
>  drivers/hid/hid-ids.h         |  3 +++
>  drivers/hid/i2c-hid/i2c-hid.c | 23 ++++++++++++++++++-----
>  3 files changed, 23 insertions(+), 7 deletions(-)
>

This patchset is being reworked, as stated by Aaron Ma.

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