Hello Anisse,
On 12-09-18 12:22, Anisse Astier wrote:
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 synchronously block the the runtime suspend by 2.5 seconds;
this was found experimentally, and sleeping less will not result in a
non-working touchpad.
This quirk will also result in a non-responsive touchpad for 2.5s if
userspace closes, then opens a device quickly, but this was deemed an
appropriate compromise versus a non-working touchpad.
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 <anisse@xxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
> ---
>
> This patch feels a bit hack-ish, but I don't really know how to debug this
> issue further.
>
> I'm looking forward to your feedback.
Interesting, what laptop are you seeing this on, we've had lots of
bug reports about this touchpad, mainly see:
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1728244
But we've been unable to get to the bottom of this, so I'm wondering
if you are using one of the machines mentioned in that bug report and
thus if you are seeing the same problem.
I don't think your msleep(2500) is a good solution though, I think
we just need to disable runtime pm all together on these devices,
can you try changing the
pm_runtime_put(&client->dev);
return 0;
Lines at the end of i2c_hid_probe() to:
if (!(ihid->quirks & I2C_HID_QUIRK_SLOW_DOWN_RUNTIME_PM))
pm_runtime_put(&client->dev);
And in i2c_hid_remove() change:
struct hid_device *hid;
pm_runtime_get_sync(&client->dev);
pm_runtime_disable(&client->dev);
to:
struct hid_device *hid;
if (!(ihid->quirks & I2C_HID_QUIRK_SLOW_DOWN_RUNTIME_PM))
pm_runtime_get_sync(&client->dev);
pm_runtime_disable(&client->dev);
And then drop this bit:
@@ -1259,6 +1262,10 @@ static int i2c_hid_resume(struct device *dev)
static int i2c_hid_runtime_suspend(struct device *dev)
{
struct i2c_client *client = to_i2c_client(dev);
+ struct i2c_hid *ihid = i2c_get_clientdata(client);
+
+ if (ihid->quirks & I2C_HID_QUIRK_SLOW_DOWN_RUNTIME_PM)
+ msleep(2500);
i2c_hid_set_power(client, I2C_HID_PWR_SLEEP);
disable_irq(client->irq);
From your patch and see if that also fixes things for you
(I would expect it will).
If that also fixes things, please also rename the quirk from
I2C_HID_QUIRK_SLOW_DOWN_RUNTIME_PM to I2C_HID_QUIRK_NO_RUNTIME_PM
Also there already is an existing quirk for the hantick
touchpad which is necessary on at least some devices. So you need
to or ('|') your quirk flag together with the existing
I2C_HID_QUIRK_NO_IRQ_AFTER_RESET quirk for these devices
And then submit a v2 upstream.
Regards,
Hans
