[SRU] [B/C/D/Unstable] [PATCH 0/2] Fix and issue that LG I2C touchscreen stops working after reboot

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

[SRU] [B/C/D/Unstable] [PATCH 0/2] Fix and issue that LG I2C touchscreen stops working after reboot

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

[Impact]
LG I2C touchscreen stops working after reboot.

[Fix]
Disable runtime PM for the touchscreen, which doesn't work well with
consecutive ON/SLEEP commands.

[Test]
The touchscreen works well after reboot, when runtime PM is disabled.

[Regression Potential]
None. This is a new device and the fix is limited to this particular
device.

Anisse Astier (1):
  HID: i2c-hid: disable runtime PM operations on hantick touchpad

Kai-Heng Feng (1):
  HID: i2c-hid: Disable runtime PM for LG touchscreen

 drivers/hid/hid-ids.h         |  1 +
 drivers/hid/i2c-hid/i2c-hid.c | 13 ++++++++++---
 2 files changed, 11 insertions(+), 3 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
|

[Bionic] [PATCH 1/2] HID: i2c-hid: disable runtime PM operations on hantick touchpad

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

From: Anisse Astier <[hidden email]>

This hantick HTIX5288 touchpad can quickly fall in a wrong state if
there are too many open/close operations. This will either make it stop
reporting any input, or will shift all the input reads by a few bytes,
making it impossible to decode.

Here, we never release the probed touchpad runtime pm while the driver
is loaded, which should disable all runtime pm suspend/resumes.

This fast repetition of sleep/wakeup is also more likely to happen when
using runtime PM, which is why the quirk is done there, and not for all
power downs, which would include suspend or module removal.

Signed-off-by: Anisse Astier <[hidden email]>
Cc: [hidden email]
Acked-by: Benjamin Tissoires <[hidden email]>
Reviewed-by: Hans de Goede <[hidden email]>
Tested-by: Philip Müller <[hidden email]>
Signed-off-by: Jiri Kosina <[hidden email]>
(cherry picked from commit 807588ac92018bde88a1958f546438e840eb0158)
Signed-off-by: Kai-Heng Feng <[hidden email]>
---
 drivers/hid/i2c-hid/i2c-hid.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 46037a692384..72df970c60ef 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -48,6 +48,7 @@
 #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV BIT(0)
 #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
 #define I2C_HID_QUIRK_RESEND_REPORT_DESCR BIT(2)
+#define I2C_HID_QUIRK_NO_RUNTIME_PM BIT(3)
 
 /* flags */
 #define I2C_HID_STARTED 0
@@ -171,7 +172,8 @@ static const struct i2c_hid_quirks {
  { USB_VENDOR_ID_WEIDA, USB_DEVICE_ID_WEIDA_8755,
  I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
  { I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
- I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
+ I2C_HID_QUIRK_NO_IRQ_AFTER_RESET |
+ I2C_HID_QUIRK_NO_RUNTIME_PM },
  { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS10FB_TOUCH,
  I2C_HID_QUIRK_RESEND_REPORT_DESCR },
  { 0, 0 }
@@ -1081,7 +1083,9 @@ static int i2c_hid_probe(struct i2c_client *client,
  goto err_mem_free;
  }
 
- pm_runtime_put(&client->dev);
+ if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))
+ pm_runtime_put(&client->dev);
+
  return 0;
 
 err_mem_free:
@@ -1108,7 +1112,8 @@ static int i2c_hid_remove(struct i2c_client *client)
  struct i2c_hid *ihid = i2c_get_clientdata(client);
  struct hid_device *hid;
 
- pm_runtime_get_sync(&client->dev);
+ if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))
+ pm_runtime_get_sync(&client->dev);
  pm_runtime_disable(&client->dev);
  pm_runtime_set_suspended(&client->dev);
  pm_runtime_put_noidle(&client->dev);
--
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
|

[C/D/Unstable] [PATCH] HID: i2c-hid: Disable runtime PM for LG touchscreen

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

LG touchscreen (1fd2:8001) stops working after reboot:
[ 4.859153] i2c_hid i2c-SAPS2101:00: i2c_hid_get_input: incomplete report (64/66)
[ 4.936070] i2c_hid i2c-SAPS2101:00: i2c_hid_get_input: incomplete report (64/66)
[ 9.948224] i2c_hid i2c-SAPS2101:00: failed to reset device.

The device in question stops working after receives SLEEP, ON, SLEEP
commands in a short period. The scenario is like this:
- Once the desktop session closes, it also closed the hid device, so the
device gets runtime suspended and receives a SLEEP command.
- Before calling shutdown callback, it gets runtime resumed and received
an ON command.
- In the shutdown callback, it receives another SLEEP command.

I failed to find a reliable interval between ON/SLEEP commands that can
make it work, so let's simply disable runtime PM for the device.

Signed-off-by: Kai-Heng Feng <[hidden email]>
Signed-off-by: Jiri Kosina <[hidden email]>
(backported from commit 86c31524b27c7e686841dd4a79eda95cfd989f16)
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 9590394b55fc..f32be708b324 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -701,6 +701,7 @@
 #define USB_VENDOR_ID_LG 0x1fd2
 #define USB_DEVICE_ID_LG_MULTITOUCH 0x0064
 #define USB_DEVICE_ID_LG_MELFAS_MT 0x6007
+#define I2C_DEVICE_ID_LG_8001 0x8001
 
 #define USB_VENDOR_ID_LOGITECH 0x046d
 #define USB_DEVICE_ID_LOGITECH_AUDIOHUB 0x0a0e
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index d17cf6e323b2..0babeae0f2a0 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -171,6 +171,8 @@ static const struct i2c_hid_quirks {
  { I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
  I2C_HID_QUIRK_NO_IRQ_AFTER_RESET |
  I2C_HID_QUIRK_NO_RUNTIME_PM },
+ { USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
+ I2C_HID_QUIRK_NO_RUNTIME_PM },
  { 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
|

[Bionic] [PATCH 2/2] HID: i2c-hid: Disable runtime PM for LG touchscreen

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

LG touchscreen (1fd2:8001) stops working after reboot:
[ 4.859153] i2c_hid i2c-SAPS2101:00: i2c_hid_get_input: incomplete report (64/66)
[ 4.936070] i2c_hid i2c-SAPS2101:00: i2c_hid_get_input: incomplete report (64/66)
[ 9.948224] i2c_hid i2c-SAPS2101:00: failed to reset device.

The device in question stops working after receives SLEEP, ON, SLEEP
commands in a short period. The scenario is like this:
- Once the desktop session closes, it also closed the hid device, so the
device gets runtime suspended and receives a SLEEP command.
- Before calling shutdown callback, it gets runtime resumed and received
an ON command.
- In the shutdown callback, it receives another SLEEP command.

I failed to find a reliable interval between ON/SLEEP commands that can
make it work, so let's simply disable runtime PM for the device.

Signed-off-by: Kai-Heng Feng <[hidden email]>
Signed-off-by: Jiri Kosina <[hidden email]>
(backported from commit 86c31524b27c7e686841dd4a79eda95cfd989f16)
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 92de8444b921..83e25c6688a0 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -687,6 +687,7 @@
 #define USB_VENDOR_ID_LG 0x1fd2
 #define USB_DEVICE_ID_LG_MULTITOUCH 0x0064
 #define USB_DEVICE_ID_LG_MELFAS_MT 0x6007
+#define I2C_DEVICE_ID_LG_8001 0x8001
 
 #define USB_VENDOR_ID_LOGITECH 0x046d
 #define USB_DEVICE_ID_LOGITECH_AUDIOHUB 0x0a0e
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 72df970c60ef..def6b5fdb915 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_NO_RUNTIME_PM },
  { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS10FB_TOUCH,
  I2C_HID_QUIRK_RESEND_REPORT_DESCR },
+ { USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
+ I2C_HID_QUIRK_NO_RUNTIME_PM },
  { 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
|

APPLIED[Unstable] / Cmt: [SRU] [B/C/D/Unstable] [PATCH 0/2] Fix and issue that LG I2C touchscreen stops working after reboot

Seth Forshee
In reply to this post by Kai-Heng Feng
On Tue, Nov 27, 2018 at 06:59:06AM +0000, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1805085
>
> [Impact]
> LG I2C touchscreen stops working after reboot.
>
> [Fix]
> Disable runtime PM for the touchscreen, which doesn't work well with
> consecutive ON/SLEEP commands.
>
> [Test]
> The touchscreen works well after reboot, when runtime PM is disabled.
>
> [Regression Potential]
> None. This is a new device and the fix is limited to this particular
> device.

The patch did not apply to unstable, so I cherry picked the fix
directly.

Wrt bionic though, there's the extra prerequisite patch that looks to be
affecting a different touchscreen. If so you can't really say that the
regression potential is limited to a single device. You should either
explain why we should also change runtime PM for that device and
describe what regression testing was done, or else omit the changes for
that device.

Thanks,
Seth

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

Re: APPLIED[Unstable] / Cmt: [SRU] [B/C/D/Unstable] [PATCH 0/2] Fix and issue that LG I2C touchscreen stops working after reboot

Kai-Heng Feng


> On Nov 28, 2018, at 12:36 AM, Seth Forshee <[hidden email]> wrote:
>
> On Tue, Nov 27, 2018 at 06:59:06AM +0000, Kai-Heng Feng wrote:
>> BugLink: https://bugs.launchpad.net/bugs/1805085
>>
>> [Impact]
>> LG I2C touchscreen stops working after reboot.
>>
>> [Fix]
>> Disable runtime PM for the touchscreen, which doesn't work well with
>> consecutive ON/SLEEP commands.
>>
>> [Test]
>> The touchscreen works well after reboot, when runtime PM is disabled.
>>
>> [Regression Potential]
>> None. This is a new device and the fix is limited to this particular
>> device.
>
> The patch did not apply to unstable, so I cherry picked the fix
> directly.

Thanks!

>
> Wrt bionic though, there's the extra prerequisite patch that looks to be
> affecting a different touchscreen. If so you can't really say that the
> regression potential is limited to a single device. You should either
> explain why we should also change runtime PM for that device and
> describe what regression testing was done, or else omit the changes for
> that device.

I missed that one, sorry. It does introduce a fix for another device.

The change to runtime PM is to let the driver never tries to runtime
suspend the device, avoiding sending too many ON/SLEEP command to
the device.

Kai-Heng

>
> Thanks,
> Seth


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

APPLIED[OEM-B]: [SRU] [B/C/D/Unstable] [PATCH 0/2] Fix and issue that LG I2C touchscreen stops working after reboot

AceLan Kao
In reply to this post by Kai-Heng Feng
Applied on oem kernel 4.15.0-1029.34

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

Re: APPLIED[Unstable] / Cmt: [SRU] [B/C/D/Unstable] [PATCH 0/2] Fix and issue that LG I2C touchscreen stops working after reboot

Kleber Souza
In reply to this post by Kai-Heng Feng
On 11/29/18 6:14 AM, Kai Heng Feng wrote:

>
>> On Nov 28, 2018, at 12:36 AM, Seth Forshee <[hidden email]> wrote:
>>
>> On Tue, Nov 27, 2018 at 06:59:06AM +0000, Kai-Heng Feng wrote:
>>> BugLink: https://bugs.launchpad.net/bugs/1805085
>>>
>>> [Impact]
>>> LG I2C touchscreen stops working after reboot.
>>>
>>> [Fix]
>>> Disable runtime PM for the touchscreen, which doesn't work well with
>>> consecutive ON/SLEEP commands.
>>>
>>> [Test]
>>> The touchscreen works well after reboot, when runtime PM is disabled.
>>>
>>> [Regression Potential]
>>> None. This is a new device and the fix is limited to this particular
>>> device.
>> The patch did not apply to unstable, so I cherry picked the fix
>> directly.
> Thanks!
>
>> Wrt bionic though, there's the extra prerequisite patch that looks to be
>> affecting a different touchscreen. If so you can't really say that the
>> regression potential is limited to a single device. You should either
>> explain why we should also change runtime PM for that device and
>> describe what regression testing was done, or else omit the changes for
>> that device.
> I missed that one, sorry. It does introduce a fix for another device.
>
> The change to runtime PM is to let the driver never tries to runtime
> suspend the device, avoiding sending too many ON/SLEEP command to
> the device.
>
> Kai-Heng
>
Hi Kai-Heng,

Was any regression test done with the hantick device affected by the
first patch for Bionic?


Thanks,

Kleber


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

Re: APPLIED[Unstable] / Cmt: [SRU] [B/C/D/Unstable] [PATCH 0/2] Fix and issue that LG I2C touchscreen stops working after reboot

Kai-Heng Feng


> On Dec 19, 2018, at 00:33, Kleber Souza <[hidden email]> wrote:
>
> On 11/29/18 6:14 AM, Kai Heng Feng wrote:
>>
>>> On Nov 28, 2018, at 12:36 AM, Seth Forshee <[hidden email]> wrote:
>>>
>>> On Tue, Nov 27, 2018 at 06:59:06AM +0000, Kai-Heng Feng wrote:
>>>> BugLink: https://bugs.launchpad.net/bugs/1805085
>>>>
>>>> [Impact]
>>>> LG I2C touchscreen stops working after reboot.
>>>>
>>>> [Fix]
>>>> Disable runtime PM for the touchscreen, which doesn't work well with
>>>> consecutive ON/SLEEP commands.
>>>>
>>>> [Test]
>>>> The touchscreen works well after reboot, when runtime PM is disabled.
>>>>
>>>> [Regression Potential]
>>>> None. This is a new device and the fix is limited to this particular
>>>> device.
>>> The patch did not apply to unstable, so I cherry picked the fix
>>> directly.
>> Thanks!
>>
>>> Wrt bionic though, there's the extra prerequisite patch that looks to be
>>> affecting a different touchscreen. If so you can't really say that the
>>> regression potential is limited to a single device. You should either
>>> explain why we should also change runtime PM for that device and
>>> describe what regression testing was done, or else omit the changes for
>>> that device.
>> I missed that one, sorry. It does introduce a fix for another device.
>>
>> The change to runtime PM is to let the driver never tries to runtime
>> suspend the device, avoiding sending too many ON/SLEEP command to
>> the device.
>>
>> Kai-Heng
>>
> Hi Kai-Heng,
>
> Was any regression test done with the hantick device affected by the
> first patch for Bionic?

No not really, since we don’t have that particular device.

That commit is a fix for LP: #1728244 but I haven’t sent an SRU for that bug.
I can split the SRU into two if you think it’s better.

Kai-Heng

>
>
> Thanks,
>
> Kleber


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

Re: APPLIED[Unstable] / Cmt: [SRU] [B/C/D/Unstable] [PATCH 0/2] Fix and issue that LG I2C touchscreen stops working after reboot

Kleber Souza
On 12/19/18 8:05 AM, Kai Heng Feng wrote:

>
>> On Dec 19, 2018, at 00:33, Kleber Souza <[hidden email]> wrote:
>>
>> On 11/29/18 6:14 AM, Kai Heng Feng wrote:
>>>> On Nov 28, 2018, at 12:36 AM, Seth Forshee <[hidden email]> wrote:
>>>>
>>>> On Tue, Nov 27, 2018 at 06:59:06AM +0000, Kai-Heng Feng wrote:
>>>>> BugLink: https://bugs.launchpad.net/bugs/1805085
>>>>>
>>>>> [Impact]
>>>>> LG I2C touchscreen stops working after reboot.
>>>>>
>>>>> [Fix]
>>>>> Disable runtime PM for the touchscreen, which doesn't work well with
>>>>> consecutive ON/SLEEP commands.
>>>>>
>>>>> [Test]
>>>>> The touchscreen works well after reboot, when runtime PM is disabled.
>>>>>
>>>>> [Regression Potential]
>>>>> None. This is a new device and the fix is limited to this particular
>>>>> device.
>>>> The patch did not apply to unstable, so I cherry picked the fix
>>>> directly.
>>> Thanks!
>>>
>>>> Wrt bionic though, there's the extra prerequisite patch that looks to be
>>>> affecting a different touchscreen. If so you can't really say that the
>>>> regression potential is limited to a single device. You should either
>>>> explain why we should also change runtime PM for that device and
>>>> describe what regression testing was done, or else omit the changes for
>>>> that device.
>>> I missed that one, sorry. It does introduce a fix for another device.
>>>
>>> The change to runtime PM is to let the driver never tries to runtime
>>> suspend the device, avoiding sending too many ON/SLEEP command to
>>> the device.
>>>
>>> Kai-Heng
>>>
>> Hi Kai-Heng,
>>
>> Was any regression test done with the hantick device affected by the
>> first patch for Bionic?
> No not really, since we don’t have that particular device.
>
> That commit is a fix for LP: #1728244 but I haven’t sent an SRU for that bug.
> I can split the SRU into two if you think it’s better.
>
> Kai-Heng

Hi Kai-Heng,

So we definitely need that first patch in full version. It would be good
to have a reference to LP: #1728244 in that patch so we can track its fix.

Could you please send it in a separate SRU request? Then we can hold
this patch series for a bit, ignoring patch 1/2 for bionic, and when
both are reviewed we apply them in the right order.


Thanks!

Kleber


>
>>
>> Thanks,
>>
>> Kleber




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

Re: APPLIED[Unstable] / Cmt: [SRU] [B/C/D/Unstable] [PATCH 0/2] Fix and issue that LG I2C touchscreen stops working after reboot

Kai-Heng Feng


> On Dec 19, 2018, at 16:17, Kleber Souza <[hidden email]> wrote:
>
> On 12/19/18 8:05 AM, Kai Heng Feng wrote:
>>
>>> On Dec 19, 2018, at 00:33, Kleber Souza <[hidden email]> wrote:
>>>
>>> On 11/29/18 6:14 AM, Kai Heng Feng wrote:
>>>>> On Nov 28, 2018, at 12:36 AM, Seth Forshee <[hidden email]> wrote:
>>>>>
>>>>> On Tue, Nov 27, 2018 at 06:59:06AM +0000, Kai-Heng Feng wrote:
>>>>>> BugLink: https://bugs.launchpad.net/bugs/1805085
>>>>>>
>>>>>> [Impact]
>>>>>> LG I2C touchscreen stops working after reboot.
>>>>>>
>>>>>> [Fix]
>>>>>> Disable runtime PM for the touchscreen, which doesn't work well with
>>>>>> consecutive ON/SLEEP commands.
>>>>>>
>>>>>> [Test]
>>>>>> The touchscreen works well after reboot, when runtime PM is disabled.
>>>>>>
>>>>>> [Regression Potential]
>>>>>> None. This is a new device and the fix is limited to this particular
>>>>>> device.
>>>>> The patch did not apply to unstable, so I cherry picked the fix
>>>>> directly.
>>>> Thanks!
>>>>
>>>>> Wrt bionic though, there's the extra prerequisite patch that looks to be
>>>>> affecting a different touchscreen. If so you can't really say that the
>>>>> regression potential is limited to a single device. You should either
>>>>> explain why we should also change runtime PM for that device and
>>>>> describe what regression testing was done, or else omit the changes for
>>>>> that device.
>>>> I missed that one, sorry. It does introduce a fix for another device.
>>>>
>>>> The change to runtime PM is to let the driver never tries to runtime
>>>> suspend the device, avoiding sending too many ON/SLEEP command to
>>>> the device.
>>>>
>>>> Kai-Heng
>>>>
>>> Hi Kai-Heng,
>>>
>>> Was any regression test done with the hantick device affected by the
>>> first patch for Bionic?
>> No not really, since we don’t have that particular device.
>>
>> That commit is a fix for LP: #1728244 but I haven’t sent an SRU for that bug.
>> I can split the SRU into two if you think it’s better.
>>
>> Kai-Heng
>
> Hi Kai-Heng,
>
> So we definitely need that first patch in full version. It would be good
> to have a reference to LP: #1728244 in that patch so we can track its fix.
>
> Could you please send it in a separate SRU request? Then we can hold
> this patch series for a bit, ignoring patch 1/2 for bionic, and when
> both are reviewed we apply them in the right order.

Ok, I’ll send a separate SRU request for Hantick device.
Thanks!

Kai-Heng

>
>
> Thanks!
>
> Kleber
>
>
>>
>>>
>>> Thanks,
>>>
>>> Kleber


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

NAK: [Bionic] [PATCH 1/2] HID: i2c-hid: disable runtime PM operations on hantick touchpad

Kleber Souza
In reply to this post by Kai-Heng Feng
On 11/27/18 7:59 AM, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1805085
>
> From: Anisse Astier <[hidden email]>
>
> This hantick HTIX5288 touchpad can quickly fall in a wrong state if
> there are too many open/close operations. This will either make it stop
> reporting any input, or will shift all the input reads by a few bytes,
> making it impossible to decode.
>
> Here, we never release the probed touchpad runtime pm while the driver
> is loaded, which should disable all runtime pm suspend/resumes.
>
> This fast repetition of sleep/wakeup is also more likely to happen when
> using runtime PM, which is why the quirk is done there, and not for all
> power downs, which would include suspend or module removal.
>
> Signed-off-by: Anisse Astier <[hidden email]>
> Cc: [hidden email]
> Acked-by: Benjamin Tissoires <[hidden email]>
> Reviewed-by: Hans de Goede <[hidden email]>
> Tested-by: Philip Müller <[hidden email]>
> Signed-off-by: Jiri Kosina <[hidden email]>
> (cherry picked from commit 807588ac92018bde88a1958f546438e840eb0158)
> Signed-off-by: Kai-Heng Feng <[hidden email]>
> ---
>  drivers/hid/i2c-hid/i2c-hid.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 46037a692384..72df970c60ef 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -48,6 +48,7 @@
>  #define I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV BIT(0)
>  #define I2C_HID_QUIRK_NO_IRQ_AFTER_RESET BIT(1)
>  #define I2C_HID_QUIRK_RESEND_REPORT_DESCR BIT(2)
> +#define I2C_HID_QUIRK_NO_RUNTIME_PM BIT(3)
>  
>  /* flags */
>  #define I2C_HID_STARTED 0
> @@ -171,7 +172,8 @@ static const struct i2c_hid_quirks {
>   { USB_VENDOR_ID_WEIDA, USB_DEVICE_ID_WEIDA_8755,
>   I2C_HID_QUIRK_SET_PWR_WAKEUP_DEV },
>   { I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
> - I2C_HID_QUIRK_NO_IRQ_AFTER_RESET },
> + I2C_HID_QUIRK_NO_IRQ_AFTER_RESET |
> + I2C_HID_QUIRK_NO_RUNTIME_PM },
>   { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS10FB_TOUCH,
>   I2C_HID_QUIRK_RESEND_REPORT_DESCR },
>   { 0, 0 }
> @@ -1081,7 +1083,9 @@ static int i2c_hid_probe(struct i2c_client *client,
>   goto err_mem_free;
>   }
>  
> - pm_runtime_put(&client->dev);
> + if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))
> + pm_runtime_put(&client->dev);
> +
>   return 0;
>  
>  err_mem_free:
> @@ -1108,7 +1112,8 @@ static int i2c_hid_remove(struct i2c_client *client)
>   struct i2c_hid *ihid = i2c_get_clientdata(client);
>   struct hid_device *hid;
>  
> - pm_runtime_get_sync(&client->dev);
> + if (!(ihid->quirks & I2C_HID_QUIRK_NO_RUNTIME_PM))
> + pm_runtime_get_sync(&client->dev);
>   pm_runtime_disable(&client->dev);
>   pm_runtime_set_suspended(&client->dev);
>   pm_runtime_put_noidle(&client->dev);

As per previous discussion, this patch will be resent as SRU request for
another bug report specific to the hantick issue.


Thanks,

Kleber


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

ACK[Bionic 2/2][Cosmic]: [SRU] [B/C/D/Unstable] [PATCH 0/2] Fix and issue that LG I2C touchscreen stops working after reboot

Kleber Souza
In reply to this post by Kai-Heng Feng
On 11/27/18 7:59 AM, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1805085
>
> [Impact]
> LG I2C touchscreen stops working after reboot.
>
> [Fix]
> Disable runtime PM for the touchscreen, which doesn't work well with
> consecutive ON/SLEEP commands.
>
> [Test]
> The touchscreen works well after reboot, when runtime PM is disabled.
>
> [Regression Potential]
> None. This is a new device and the fix is limited to this particular
> device.
>
> Anisse Astier (1):
>   HID: i2c-hid: disable runtime PM operations on hantick touchpad
>
> Kai-Heng Feng (1):
>   HID: i2c-hid: Disable runtime PM for LG touchscreen
>
>  drivers/hid/hid-ids.h         |  1 +
>  drivers/hid/i2c-hid/i2c-hid.c | 13 ++++++++++---
>  2 files changed, 11 insertions(+), 3 deletions(-)
>
Bionic patch 1/2 will be split into a separate SRU request, so for patch
2/2 for Bionic and 1/1 for Cosmic:

Acked-by: Kleber Sacilotto de Souza <[hidden email]>


Please note that the patch for Bionic depends on the fix that will be
sent as a separate request, so that one needs to be applied before.


Thanks,

Kleber


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

ACK/Cmnt: [Bionic] [PATCH 2/2] HID: i2c-hid: Disable runtime PM for LG touchscreen

Stefan Bader-2
In reply to this post by Kai-Heng Feng
On 27.11.18 07:59, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1805085
>
> LG touchscreen (1fd2:8001) stops working after reboot:
> [ 4.859153] i2c_hid i2c-SAPS2101:00: i2c_hid_get_input: incomplete report (64/66)
> [ 4.936070] i2c_hid i2c-SAPS2101:00: i2c_hid_get_input: incomplete report (64/66)
> [ 9.948224] i2c_hid i2c-SAPS2101:00: failed to reset device.
>
> The device in question stops working after receives SLEEP, ON, SLEEP
> commands in a short period. The scenario is like this:
> - Once the desktop session closes, it also closed the hid device, so the
> device gets runtime suspended and receives a SLEEP command.
> - Before calling shutdown callback, it gets runtime resumed and received
> an ON command.
> - In the shutdown callback, it receives another SLEEP command.
>
> I failed to find a reliable interval between ON/SLEEP commands that can
> make it work, so let's simply disable runtime PM for the device.
>
> Signed-off-by: Kai-Heng Feng <[hidden email]>
> Signed-off-by: Jiri Kosina <[hidden email]>
> (backported from commit 86c31524b27c7e686841dd4a79eda95cfd989f16)
> Signed-off-by: Kai-Heng Feng <[hidden email]>
Acked-by: Stefan Bader <[hidden email]>
> ---

With the discussion done, only for this patch.

>  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 92de8444b921..83e25c6688a0 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -687,6 +687,7 @@
>  #define USB_VENDOR_ID_LG 0x1fd2
>  #define USB_DEVICE_ID_LG_MULTITOUCH 0x0064
>  #define USB_DEVICE_ID_LG_MELFAS_MT 0x6007
> +#define I2C_DEVICE_ID_LG_8001 0x8001
>  
>  #define USB_VENDOR_ID_LOGITECH 0x046d
>  #define USB_DEVICE_ID_LOGITECH_AUDIOHUB 0x0a0e
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 72df970c60ef..def6b5fdb915 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_NO_RUNTIME_PM },
>   { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS10FB_TOUCH,
>   I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> + { USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
> + I2C_HID_QUIRK_NO_RUNTIME_PM },
>   { 0, 0 }
>  };
>  
>


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

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

APPLIED/cmnt: [Bionic] [PATCH 2/2] HID: i2c-hid: Disable runtime PM for LG touchscreen

Kleber Souza
In reply to this post by Kai-Heng Feng
On 11/27/18 7:59 AM, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1805085
>
> LG touchscreen (1fd2:8001) stops working after reboot:
> [ 4.859153] i2c_hid i2c-SAPS2101:00: i2c_hid_get_input: incomplete report (64/66)
> [ 4.936070] i2c_hid i2c-SAPS2101:00: i2c_hid_get_input: incomplete report (64/66)
> [ 9.948224] i2c_hid i2c-SAPS2101:00: failed to reset device.
>
> The device in question stops working after receives SLEEP, ON, SLEEP
> commands in a short period. The scenario is like this:
> - Once the desktop session closes, it also closed the hid device, so the
> device gets runtime suspended and receives a SLEEP command.
> - Before calling shutdown callback, it gets runtime resumed and received
> an ON command.
> - In the shutdown callback, it receives another SLEEP command.
>
> I failed to find a reliable interval between ON/SLEEP commands that can
> make it work, so let's simply disable runtime PM for the device.
>
> Signed-off-by: Kai-Heng Feng <[hidden email]>
> Signed-off-by: Jiri Kosina <[hidden email]>
> (backported from commit 86c31524b27c7e686841dd4a79eda95cfd989f16)
> 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 92de8444b921..83e25c6688a0 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -687,6 +687,7 @@
>  #define USB_VENDOR_ID_LG 0x1fd2
>  #define USB_DEVICE_ID_LG_MULTITOUCH 0x0064
>  #define USB_DEVICE_ID_LG_MELFAS_MT 0x6007
> +#define I2C_DEVICE_ID_LG_8001 0x8001
>  
>  #define USB_VENDOR_ID_LOGITECH 0x046d
>  #define USB_DEVICE_ID_LOGITECH_AUDIOHUB 0x0a0e
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index 72df970c60ef..def6b5fdb915 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_NO_RUNTIME_PM },
>   { USB_VENDOR_ID_SIS_TOUCH, USB_DEVICE_ID_SIS10FB_TOUCH,
>   I2C_HID_QUIRK_RESEND_REPORT_DESCR },
> + { USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
> + I2C_HID_QUIRK_NO_RUNTIME_PM },
>   { 0, 0 }
>  };
>  

Applied to bionic/master-next branch. Patch 1/2 has already been applied
as a separate fix for LP: #1728244.

Thanks,
Kleber


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

APPLIED[C]: [C/D/Unstable] [PATCH] HID: i2c-hid: Disable runtime PM for LG touchscreen

Kleber Souza
In reply to this post by Kai-Heng Feng
On 11/27/18 7:59 AM, Kai-Heng Feng wrote:

> BugLink: https://bugs.launchpad.net/bugs/1805085
>
> LG touchscreen (1fd2:8001) stops working after reboot:
> [ 4.859153] i2c_hid i2c-SAPS2101:00: i2c_hid_get_input: incomplete report (64/66)
> [ 4.936070] i2c_hid i2c-SAPS2101:00: i2c_hid_get_input: incomplete report (64/66)
> [ 9.948224] i2c_hid i2c-SAPS2101:00: failed to reset device.
>
> The device in question stops working after receives SLEEP, ON, SLEEP
> commands in a short period. The scenario is like this:
> - Once the desktop session closes, it also closed the hid device, so the
> device gets runtime suspended and receives a SLEEP command.
> - Before calling shutdown callback, it gets runtime resumed and received
> an ON command.
> - In the shutdown callback, it receives another SLEEP command.
>
> I failed to find a reliable interval between ON/SLEEP commands that can
> make it work, so let's simply disable runtime PM for the device.
>
> Signed-off-by: Kai-Heng Feng <[hidden email]>
> Signed-off-by: Jiri Kosina <[hidden email]>
> (backported from commit 86c31524b27c7e686841dd4a79eda95cfd989f16)
> 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 9590394b55fc..f32be708b324 100644
> --- a/drivers/hid/hid-ids.h
> +++ b/drivers/hid/hid-ids.h
> @@ -701,6 +701,7 @@
>  #define USB_VENDOR_ID_LG 0x1fd2
>  #define USB_DEVICE_ID_LG_MULTITOUCH 0x0064
>  #define USB_DEVICE_ID_LG_MELFAS_MT 0x6007
> +#define I2C_DEVICE_ID_LG_8001 0x8001
>  
>  #define USB_VENDOR_ID_LOGITECH 0x046d
>  #define USB_DEVICE_ID_LOGITECH_AUDIOHUB 0x0a0e
> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
> index d17cf6e323b2..0babeae0f2a0 100644
> --- a/drivers/hid/i2c-hid/i2c-hid.c
> +++ b/drivers/hid/i2c-hid/i2c-hid.c
> @@ -171,6 +171,8 @@ static const struct i2c_hid_quirks {
>   { I2C_VENDOR_ID_HANTICK, I2C_PRODUCT_ID_HANTICK_5288,
>   I2C_HID_QUIRK_NO_IRQ_AFTER_RESET |
>   I2C_HID_QUIRK_NO_RUNTIME_PM },
> + { USB_VENDOR_ID_LG, I2C_DEVICE_ID_LG_8001,
> + I2C_HID_QUIRK_NO_RUNTIME_PM },
>   { 0, 0 }
>  };
>  

Applied to cosmic/master-next branch.

Thanks,
Kleber


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