Re: [PATCH] HID: i2c-hid: Don't reset device upon system resume

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Benjamin,

I forgot to include mailing list, first time -in-reply-to user :)

at 17:39, Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> wrote:

On Mon, Sep 3, 2018 at 11:09 AM Kai-Heng Feng
<kai.heng.feng@xxxxxxxxxxxxx> 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.

Hmm, that would explain a lot, thanks for digging into this.

Jiri, I think this patch should partially replace
https://patchwork.kernel.org/patch/10583481/

Kai-heng, if you do not need to re-request the HID descriptors, I
think it should be good to squash these together to:
- remove I2C_HID_QUIRK_RESEND_REPORT_DESCR
- remove the re-request the HID descriptors on resume
- merge with this patch

There's still one user uses I2C_HID_QUIRK_RESEND_REPORT_DESCR, USB_DEVICE_ID_SIS10FB_TOUCH.
So I'd like to have an Ack from Hans first.

I'll send a new patch with your suggestion afterward.




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.

Cc: AceLan Kao <acelan.kao@xxxxxxxxxxxxx>
Cc: Aaron Ma <aaron.ma@xxxxxxxxxxxxx>
Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>
---

Benjamin, Hans,

I guess some vendors interpret spec this way, so hardware reset is an
unexpected request for their devices.
Can you give this patch a try?

The only device I have that you should not is the Surface 3. And I am
not so sure it goes into S3 fluently (it seems to, but I am not 100%
sure). If you have tested on a Dell XPS 9360, I think there is not
much more I could test.

At least for the Raydium touchscreen, the issue can be observed under both S2Idle and S3.
So I guess Surface 3 is still suitable for test if it can do S2Idle.

I've test this on XPS 9370 and Precision 5520, the patch doesn't break them.

Kai-Heng


Cheers,
Benjamin

 drivers/hid/i2c-hid/i2c-hid.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c
index 57126f6837bb..7567034a009f 100644
--- a/drivers/hid/i2c-hid/i2c-hid.c
+++ b/drivers/hid/i2c-hid/i2c-hid.c
@@ -1237,7 +1237,7 @@ static int i2c_hid_resume(struct device *dev)
        pm_runtime_enable(dev);

        enable_irq(client->irq);
-       ret = i2c_hid_hwreset(client);
+       ret = i2c_hid_set_power(client, I2C_HID_PWR_ON);
        if (ret)
                return ret;

--
2.17.1





[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux