On Fri, Sep 05, 2014 at 01:07:11PM +0200, Oliver Neukum wrote:
> On Fri, 2014-09-05 at 12:15 +0200, Johan Hovold wrote:
> > On Fri, Sep 05, 2014 at 11:05:46AM +0200, Oliver Neukum wrote:
>
> > > on second thought I think that your moving of remote_wakeup is not good.
> > > If we'd discard input anyway, we don't need to wake up devices for them.
> >
> > I'm afraid we have to. The ELAN touchscreen disconnects when an input
> > event occurs while suspended unless remote wakeup is enabled.
>
> I see. This device sucks in a terrible way. In principle we'd now need
> a special flag for port power off. Otherwise we can never send the HC
> in your laptop to D3cold. Is the small class of devices worth it?
Good point. I personally don't care about the touchscreen and could
live with a disconnect if anyone tries to put fingerprints on my screen.
In fact, when debugging this I did find a less intrusive way to solve
only the major problem with the device, which is the repeated
disconnects. Simply polling the interrupt endpoint for a short time
during start seems to make the device happy. But any input thereafter,
whether suspended or not, would then cause a disconnect.
Out of curiosity, how does your quirky device work with something like
the below? (Patch is generated on top of the always-poll quirk patch.)
This adds a 500ms timeout during probe, and I believe I couldn't reduce
that much further for this particular device if I remember correctly.
Johan
>From 5ea1eab0fa0c57075afb09d428f76a0aba478630 Mon Sep 17 00:00:00 2001
From: Johan Hovold <johan@xxxxxxxxxx>
Date: Fri, 5 Sep 2014 21:48:03 +0200
Subject: [PATCH] HID: usbhid: add hard-coded poll-once quirk
Make sure to poll the interrupt endpoint during start.
This is needed for devices that disconnects from the bus unless the
interrupt endpoint is polled shortly after enumeration.
Note that such a quirky device could still disconnect on any later input
events, but this at least prevents repeated disconnects and
re-enumerations.
Signed-off-by: Johan Hovold <johan@xxxxxxxxxx>
---
drivers/hid/usbhid/hid-core.c | 18 ++++++++++++++++++
1 file changed, 18 insertions(+)
diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index d8b6976..47bf378 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -1045,6 +1045,21 @@ err:
return ret;
}
+static void usbhid_poll_once(struct usbhid_device *uhid)
+{
+ struct device *dev = &uhid->urbin->dev->dev;
+ struct urb *urb = uhid->urbin;
+ int len = -1;
+ int ret = -1;
+
+ dev_info(dev, "%s\n", __func__);
+
+ ret = usb_interrupt_msg(urb->dev, urb->pipe, urb->transfer_buffer,
+ urb->transfer_buffer_length, &len, 500);
+
+ dev_info(dev, "%s - ret = %d, len = %d\n", __func__, ret, len);
+}
+
static int usbhid_start(struct hid_device *hid)
{
struct usb_interface *intf = to_usb_interface(hid->dev.parent);
@@ -1150,6 +1165,9 @@ static int usbhid_start(struct hid_device *hid)
usb_autopm_put_interface(usbhid->intf);
}
+ if (usbhid->urbin)
+ usbhid_poll_once(usbhid);
+
/* Some keyboards don't work until their LEDs have been set.
* Since BIOSes do set the LEDs, it must be safe for any device
* that supports the keyboard boot protocol.
--
1.8.5.5
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
