linux-input.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: USB reset xhci_hcd for ELAN touchscreen
       [not found]         ` <20140709130733.GA10078@localhost>
@ 2014-07-14 18:25           ` Johan Hovold
  2014-08-20 13:20             ` Jiri Kosina
  0 siblings, 1 reply; 8+ messages in thread
From: Johan Hovold @ 2014-07-14 18:25 UTC (permalink / raw)
  To: Greg KH
  Cc: Johan Hovold, Alan Stern, Bjørn Mork, Sarah Sharp,
	Drew Von Spreecken, linux-usb, Mathias Nyman, Jiri Kosina,
	linux-input

[ +CC: Jiri and linux-input ]

On Wed, Jul 09, 2014 at 03:07:33PM +0200, Johan Hovold wrote:
> On Mon, Jul 07, 2014 at 10:34:30PM -0700, Greg Kroah-Hartman wrote:
> > On Mon, Jul 07, 2014 at 11:24:42AM +0200, Johan Hovold wrote:
> 
> > > I ended up getting the same laptop so now I'm facing the same problem
> > > with repeated disconnects (especially during resume, which is really
> > > annoying as it adds about a minute to resume time), even with a
> > > hard-coded check-high speed "quirk" and the multitouch driver loaded.
> > > 
> > > Did you manage to fix the disconnect issue? Are you seeing something
> > > similar at resume?
> > 
> > No, it all seems to work for me.  Here's my horrible "hack" I'm running
> > with on 3.16-rc.
> > 
> > I have seen some issues at times when the touchscreen stops working on
> > resume, but it just seems "dead".  An unload/load of the xhci-hcd module
> > always fixes that :)
> 
> Ok. I'm effectively using the same hack as you, but I'm seeing these
> repeated disconnects during resume for about a minute still. Everything
> compiled in, so no modules (or fancy hibernate scripts) though. 
> 
> > I'll work on making this a "real" patch this week, when I dig out from
> > my pending patch queue...
> 
> Ok, I'll wait for that, and will try to find time to debug this later.

The timeout I experienced during resume was due to the wifi firmware not
being compiled in, but during that minute the touchscreen device kept
disconnecting every other second.

I get one such disconnect before X is started during normal boot as
well. It appears that something has to open the input device at least
once or the device will keep disconnecting itself (e.g. not booting into
X also triggers the repeated disconnects).

A simple

	cat /dev/input/by-path/pci-0000:00:14.0-usb-0:7:1.0-event

is enough to stop the device from disconnecting. But unless the device
is kept open (and the interrupt-in urb submitted) any input event (i.e.
touching the screen) causes the device to start disconnecting
repeatedly again.

Adding something like the below (on top of the check-highspeed quirk)
fixes the repeated disconnect when the device is not open. (It cannot
prevent the disconnects during a lengthy resume though as reset_resume
will not have run yet.)

Is there any better way to fix this?

Thanks,
Johan


>From 8101c0dfd42a232f1d2872de4f412d8d61d5646f Mon Sep 17 00:00:00 2001
From: Johan Hovold <johan@kernel.org>
Date: Mon, 14 Jul 2014 18:43:31 +0200
Subject: [PATCH] HID: usbhid: add HID_QUIRK_IN

Add quirk to submit the interrupt-in urb already at start() rather than
at open().

This is needed for devices that disconnects from the bus unless the
interrupt endpoint has been polled at least once or when not responding
to input events.
---
 drivers/hid/usbhid/hid-core.c | 26 +++++++++++++++++++++++---
 include/linux/hid.h           |  1 +
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 7b88f4c..4b5d986 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -82,7 +82,7 @@ static int hid_start_in(struct hid_device *hid)
 	struct usbhid_device *usbhid = hid->driver_data;
 
 	spin_lock_irqsave(&usbhid->lock, flags);
-	if (hid->open > 0 &&
+	if ((hid->open > 0 || hid->quirks & HID_QUIRK_IN) &&
 			!test_bit(HID_DISCONNECTED, &usbhid->iofl) &&
 			!test_bit(HID_SUSPENDED, &usbhid->iofl) &&
 			!test_and_set_bit(HID_IN_RUNNING, &usbhid->iofl)) {
@@ -292,6 +292,8 @@ static void hid_irq_in(struct urb *urb)
 	case 0:			/* success */
 		usbhid_mark_busy(usbhid);
 		usbhid->retry_delay = 0;
+		if ((hid->quirks & HID_QUIRK_IN) && !hid->open)
+			break;
 		hid_input_report(urb->context, HID_INPUT_REPORT,
 				 urb->transfer_buffer,
 				 urb->actual_length, 1);
@@ -734,8 +736,10 @@ void usbhid_close(struct hid_device *hid)
 	if (!--hid->open) {
 		spin_unlock_irq(&usbhid->lock);
 		hid_cancel_delayed_stuff(usbhid);
-		usb_kill_urb(usbhid->urbin);
-		usbhid->intf->needs_remote_wakeup = 0;
+		if (!(hid->quirks & HID_QUIRK_IN)) {
+			usb_kill_urb(usbhid->urbin);
+			usbhid->intf->needs_remote_wakeup = 0;
+		}
 	} else {
 		spin_unlock_irq(&usbhid->lock);
 	}
@@ -1133,6 +1137,20 @@ static int usbhid_start(struct hid_device *hid)
 
 	set_bit(HID_STARTED, &usbhid->iofl);
 
+	hid->quirks |= HID_QUIRK_IN;	/* FIXME */
+	if (hid->quirks & HID_QUIRK_IN) {
+		ret = usb_autopm_get_interface(usbhid->intf);
+		if (ret)
+			goto fail;
+		usbhid->intf->needs_remote_wakeup = 1;
+		ret = hid_start_in(hid);
+		if (ret) {
+			dev_err(&hid->dev,
+				"failed to start in urb: %d\n", ret);
+		}
+		usb_autopm_put_interface(usbhid->intf);
+	}
+
 	/* 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.
@@ -1165,6 +1183,8 @@ static void usbhid_stop(struct hid_device *hid)
 	if (WARN_ON(!usbhid))
 		return;
 
+	usbhid->intf->needs_remote_wakeup = 0;
+
 	clear_bit(HID_STARTED, &usbhid->iofl);
 	spin_lock_irq(&usbhid->lock);	/* Sync with error and led handlers */
 	set_bit(HID_DISCONNECTED, &usbhid->iofl);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 77632cf..13f81ae 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -286,6 +286,7 @@ struct hid_item {
 #define HID_QUIRK_HIDINPUT_FORCE		0x00000080
 #define HID_QUIRK_NO_EMPTY_INPUT		0x00000100
 #define HID_QUIRK_NO_INIT_INPUT_REPORTS		0x00000200
+#define HID_QUIRK_IN				0x00000400
 #define HID_QUIRK_SKIP_OUTPUT_REPORTS		0x00010000
 #define HID_QUIRK_SKIP_OUTPUT_REPORT_ID		0x00020000
 #define HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP	0x00040000
-- 
1.8.5.5


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: USB reset xhci_hcd for ELAN touchscreen
  2014-07-14 18:25           ` USB reset xhci_hcd for ELAN touchscreen Johan Hovold
@ 2014-08-20 13:20             ` Jiri Kosina
  2014-08-21 10:54               ` Johan Hovold
  0 siblings, 1 reply; 8+ messages in thread
From: Jiri Kosina @ 2014-08-20 13:20 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Greg KH, Alan Stern, Bjørn Mork, Sarah Sharp,
	Drew Von Spreecken, linux-usb, Mathias Nyman, linux-input

On Mon, 14 Jul 2014, Johan Hovold wrote:

> From 8101c0dfd42a232f1d2872de4f412d8d61d5646f Mon Sep 17 00:00:00 2001
> From: Johan Hovold <johan@kernel.org>
> Date: Mon, 14 Jul 2014 18:43:31 +0200
> Subject: [PATCH] HID: usbhid: add HID_QUIRK_IN
> 
> Add quirk to submit the interrupt-in urb already at start() rather than
> at open().
> 
> This is needed for devices that disconnects from the bus unless the
> interrupt endpoint has been polled at least once or when not responding
> to input events.

It's not really super-nice, but if no other way around it has been found 
to be possible in USB core, I am willing to take this.

> ---
>  drivers/hid/usbhid/hid-core.c | 26 +++++++++++++++++++++++---
>  include/linux/hid.h           |  1 +
>  2 files changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> index 7b88f4c..4b5d986 100644
> --- a/drivers/hid/usbhid/hid-core.c
> +++ b/drivers/hid/usbhid/hid-core.c
> @@ -82,7 +82,7 @@ static int hid_start_in(struct hid_device *hid)
>  	struct usbhid_device *usbhid = hid->driver_data;
>  
>  	spin_lock_irqsave(&usbhid->lock, flags);
> -	if (hid->open > 0 &&
> +	if ((hid->open > 0 || hid->quirks & HID_QUIRK_IN) &&
>  			!test_bit(HID_DISCONNECTED, &usbhid->iofl) &&
>  			!test_bit(HID_SUSPENDED, &usbhid->iofl) &&
>  			!test_and_set_bit(HID_IN_RUNNING, &usbhid->iofl)) {
> @@ -292,6 +292,8 @@ static void hid_irq_in(struct urb *urb)
>  	case 0:			/* success */
>  		usbhid_mark_busy(usbhid);
>  		usbhid->retry_delay = 0;
> +		if ((hid->quirks & HID_QUIRK_IN) && !hid->open)
> +			break;
>  		hid_input_report(urb->context, HID_INPUT_REPORT,
>  				 urb->transfer_buffer,
>  				 urb->actual_length, 1);
> @@ -734,8 +736,10 @@ void usbhid_close(struct hid_device *hid)
>  	if (!--hid->open) {
>  		spin_unlock_irq(&usbhid->lock);
>  		hid_cancel_delayed_stuff(usbhid);
> -		usb_kill_urb(usbhid->urbin);
> -		usbhid->intf->needs_remote_wakeup = 0;
> +		if (!(hid->quirks & HID_QUIRK_IN)) {
> +			usb_kill_urb(usbhid->urbin);
> +			usbhid->intf->needs_remote_wakeup = 0;
> +		}
>  	} else {
>  		spin_unlock_irq(&usbhid->lock);
>  	}
> @@ -1133,6 +1137,20 @@ static int usbhid_start(struct hid_device *hid)
>  
>  	set_bit(HID_STARTED, &usbhid->iofl);
>  
> +	hid->quirks |= HID_QUIRK_IN;	/* FIXME */

This of course needs to be set on per-device basis :)

> +	if (hid->quirks & HID_QUIRK_IN) {
> +		ret = usb_autopm_get_interface(usbhid->intf);
> +		if (ret)
> +			goto fail;
> +		usbhid->intf->needs_remote_wakeup = 1;
> +		ret = hid_start_in(hid);
> +		if (ret) {
> +			dev_err(&hid->dev,
> +				"failed to start in urb: %d\n", ret);
> +		}
> +		usb_autopm_put_interface(usbhid->intf);
> +	}
> +
>  	/* 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.
> @@ -1165,6 +1183,8 @@ static void usbhid_stop(struct hid_device *hid)
>  	if (WARN_ON(!usbhid))
>  		return;
>  
> +	usbhid->intf->needs_remote_wakeup = 0;
> +
>  	clear_bit(HID_STARTED, &usbhid->iofl);
>  	spin_lock_irq(&usbhid->lock);	/* Sync with error and led handlers */
>  	set_bit(HID_DISCONNECTED, &usbhid->iofl);
> diff --git a/include/linux/hid.h b/include/linux/hid.h
> index 77632cf..13f81ae 100644
> --- a/include/linux/hid.h
> +++ b/include/linux/hid.h
> @@ -286,6 +286,7 @@ struct hid_item {
>  #define HID_QUIRK_HIDINPUT_FORCE		0x00000080
>  #define HID_QUIRK_NO_EMPTY_INPUT		0x00000100
>  #define HID_QUIRK_NO_INIT_INPUT_REPORTS		0x00000200
> +#define HID_QUIRK_IN				0x00000400

0x00000400 has been removed before dynamic quirks started to be possible, 
so there is no potential clash, that's fine.

I'd just propose some more descriptive name for the quirk ... how about 
something like HID_QUIRK_EARLY_INTERRUPT?

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: USB reset xhci_hcd for ELAN touchscreen
  2014-08-20 13:20             ` Jiri Kosina
@ 2014-08-21 10:54               ` Johan Hovold
  2014-08-21 18:38                 ` Jiri Kosina
  0 siblings, 1 reply; 8+ messages in thread
From: Johan Hovold @ 2014-08-21 10:54 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Johan Hovold, Greg KH, Alan Stern, Bjørn Mork, Sarah Sharp,
	Drew Von Spreecken, linux-usb, Mathias Nyman, linux-input

On Wed, Aug 20, 2014 at 08:20:43AM -0500, Jiri Kosina wrote:
> On Mon, 14 Jul 2014, Johan Hovold wrote:
> 
> > From 8101c0dfd42a232f1d2872de4f412d8d61d5646f Mon Sep 17 00:00:00 2001
> > From: Johan Hovold <johan@kernel.org>
> > Date: Mon, 14 Jul 2014 18:43:31 +0200
> > Subject: [PATCH] HID: usbhid: add HID_QUIRK_IN
> > 
> > Add quirk to submit the interrupt-in urb already at start() rather than
> > at open().
> > 
> > This is needed for devices that disconnects from the bus unless the
> > interrupt endpoint has been polled at least once or when not responding
> > to input events.
> 
> It's not really super-nice, but if no other way around it has been found 
> to be possible in USB core, I am willing to take this.

Agreed, it's not that nice, but I'm not aware of any other way to
prevent the device firmware from disconnecting.

Autosuspend still seems to work, so it would not cause much overhead
for people not actually using the touchscreen (as long as autosuspend
is enabled) either.
 
> > ---
> >  drivers/hid/usbhid/hid-core.c | 26 +++++++++++++++++++++++---
> >  include/linux/hid.h           |  1 +
> >  2 files changed, 24 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
> > index 7b88f4c..4b5d986 100644
> > --- a/drivers/hid/usbhid/hid-core.c
> > +++ b/drivers/hid/usbhid/hid-core.c
> > @@ -82,7 +82,7 @@ static int hid_start_in(struct hid_device *hid)
> >  	struct usbhid_device *usbhid = hid->driver_data;
> >  
> >  	spin_lock_irqsave(&usbhid->lock, flags);
> > -	if (hid->open > 0 &&
> > +	if ((hid->open > 0 || hid->quirks & HID_QUIRK_IN) &&
> >  			!test_bit(HID_DISCONNECTED, &usbhid->iofl) &&
> >  			!test_bit(HID_SUSPENDED, &usbhid->iofl) &&
> >  			!test_and_set_bit(HID_IN_RUNNING, &usbhid->iofl)) {
> > @@ -292,6 +292,8 @@ static void hid_irq_in(struct urb *urb)
> >  	case 0:			/* success */
> >  		usbhid_mark_busy(usbhid);
> >  		usbhid->retry_delay = 0;
> > +		if ((hid->quirks & HID_QUIRK_IN) && !hid->open)
> > +			break;
> >  		hid_input_report(urb->context, HID_INPUT_REPORT,
> >  				 urb->transfer_buffer,
> >  				 urb->actual_length, 1);
> > @@ -734,8 +736,10 @@ void usbhid_close(struct hid_device *hid)
> >  	if (!--hid->open) {
> >  		spin_unlock_irq(&usbhid->lock);
> >  		hid_cancel_delayed_stuff(usbhid);
> > -		usb_kill_urb(usbhid->urbin);
> > -		usbhid->intf->needs_remote_wakeup = 0;
> > +		if (!(hid->quirks & HID_QUIRK_IN)) {
> > +			usb_kill_urb(usbhid->urbin);
> > +			usbhid->intf->needs_remote_wakeup = 0;
> > +		}
> >  	} else {
> >  		spin_unlock_irq(&usbhid->lock);
> >  	}
> > @@ -1133,6 +1137,20 @@ static int usbhid_start(struct hid_device *hid)
> >  
> >  	set_bit(HID_STARTED, &usbhid->iofl);
> >  
> > +	hid->quirks |= HID_QUIRK_IN;	/* FIXME */
> 
> This of course needs to be set on per-device basis :)

Of course.

> > +	if (hid->quirks & HID_QUIRK_IN) {
> > +		ret = usb_autopm_get_interface(usbhid->intf);
> > +		if (ret)
> > +			goto fail;
> > +		usbhid->intf->needs_remote_wakeup = 1;
> > +		ret = hid_start_in(hid);
> > +		if (ret) {
> > +			dev_err(&hid->dev,
> > +				"failed to start in urb: %d\n", ret);
> > +		}
> > +		usb_autopm_put_interface(usbhid->intf);
> > +	}
> > +
> >  	/* 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.
> > @@ -1165,6 +1183,8 @@ static void usbhid_stop(struct hid_device *hid)
> >  	if (WARN_ON(!usbhid))
> >  		return;
> >  
> > +	usbhid->intf->needs_remote_wakeup = 0;
> > +
> >  	clear_bit(HID_STARTED, &usbhid->iofl);
> >  	spin_lock_irq(&usbhid->lock);	/* Sync with error and led handlers */
> >  	set_bit(HID_DISCONNECTED, &usbhid->iofl);
> > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > index 77632cf..13f81ae 100644
> > --- a/include/linux/hid.h
> > +++ b/include/linux/hid.h
> > @@ -286,6 +286,7 @@ struct hid_item {
> >  #define HID_QUIRK_HIDINPUT_FORCE		0x00000080
> >  #define HID_QUIRK_NO_EMPTY_INPUT		0x00000100
> >  #define HID_QUIRK_NO_INIT_INPUT_REPORTS		0x00000200
> > +#define HID_QUIRK_IN				0x00000400
> 
> 0x00000400 has been removed before dynamic quirks started to be possible, 
> so there is no potential clash, that's fine.
> 
> I'd just propose some more descriptive name for the quirk ... how about 
> something like HID_QUIRK_EARLY_INTERRUPT?

Yeah, IN was just a working name. How about HID_QUIRK_ALWAYS_POLL or
similar as it is not just about disconnect before the device is opened,
but also if there's an event after the device has been closed (e.g.
stopping X)?

Thanks,
Johan

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: USB reset xhci_hcd for ELAN touchscreen
  2014-08-21 10:54               ` Johan Hovold
@ 2014-08-21 18:38                 ` Jiri Kosina
       [not found]                   ` <alpine.LNX.2.00.1408211337050.23162-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>
  0 siblings, 1 reply; 8+ messages in thread
From: Jiri Kosina @ 2014-08-21 18:38 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Greg KH, Alan Stern, Bjørn Mork, Sarah Sharp,
	Drew Von Spreecken, linux-usb, Mathias Nyman, linux-input

On Thu, 21 Aug 2014, Johan Hovold wrote:

> > It's not really super-nice, but if no other way around it has been found 
> > to be possible in USB core, I am willing to take this.
> 
> Agreed, it's not that nice, but I'm not aware of any other way to
> prevent the device firmware from disconnecting.
> 
> Autosuspend still seems to work, so it would not cause much overhead
> for people not actually using the touchscreen (as long as autosuspend
> is enabled) either.

Okay, good. Will be waiting for your polished patch for this.

[ ... snip ... ]
> > > diff --git a/include/linux/hid.h b/include/linux/hid.h
> > > index 77632cf..13f81ae 100644
> > > --- a/include/linux/hid.h
> > > +++ b/include/linux/hid.h
> > > @@ -286,6 +286,7 @@ struct hid_item {
> > >  #define HID_QUIRK_HIDINPUT_FORCE		0x00000080
> > >  #define HID_QUIRK_NO_EMPTY_INPUT		0x00000100
> > >  #define HID_QUIRK_NO_INIT_INPUT_REPORTS		0x00000200
> > > +#define HID_QUIRK_IN				0x00000400
> > 
> > 0x00000400 has been removed before dynamic quirks started to be possible, 
> > so there is no potential clash, that's fine.
> > 
> > I'd just propose some more descriptive name for the quirk ... how about 
> > something like HID_QUIRK_EARLY_INTERRUPT?
> 
> Yeah, IN was just a working name. How about HID_QUIRK_ALWAYS_POLL or
> similar as it is not just about disconnect before the device is opened,
> but also if there's an event after the device has been closed (e.g.
> stopping X)?

HID_QUIRK_ALWAYS_POLL sounds good.

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 0/2] HID: usbhid: fix Elan touchscreen disconnects
       [not found]                   ` <alpine.LNX.2.00.1408211337050.23162-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>
@ 2014-09-05 16:08                     ` Johan Hovold
       [not found]                       ` <1409933328-7190-1-git-send-email-johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  2014-09-08  7:30                       ` [PATCH 0/2] HID: usbhid: fix Elan touchscreen disconnects Jiri Kosina
  0 siblings, 2 replies; 8+ messages in thread
From: Johan Hovold @ 2014-09-05 16:08 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Greg KH, Alan Stern, Bjørn Mork, Sarah Sharp,
	Drew Von Spreecken, Mathias Nyman,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Oliver Neukum, Johan Hovold

Here's the always-poll quirk that is needed to prevent the Elan
touchscreen from disconnecting itself from the bus.

These patches are against v3.16.1, but applies fine to hid-next.

Note that this series is not dependent on the device-qualifier quirk
[1], which is needed to get the same device to enumerate reliably, so
these two quirks could in through the USB and HID tree, respectively.

Johan

[1] http://marc.info/?l=linux-usb&m=140898201107571&w=2

Johan Hovold (2):
  HID: usbhid: add always-poll quirk
  HID: usbhid: enable always-poll quirk for Elan Touchscreen

 drivers/hid/hid-ids.h           |  3 +++
 drivers/hid/usbhid/hid-core.c   | 26 +++++++++++++++++++++++---
 drivers/hid/usbhid/hid-quirks.c |  1 +
 include/linux/hid.h             |  1 +
 4 files changed, 28 insertions(+), 3 deletions(-)

-- 
1.8.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/2] HID: usbhid: add always-poll quirk
       [not found]                       ` <1409933328-7190-1-git-send-email-johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2014-09-05 16:08                         ` Johan Hovold
  2014-09-05 16:08                         ` [PATCH 2/2] HID: usbhid: enable always-poll quirk for Elan Touchscreen Johan Hovold
  1 sibling, 0 replies; 8+ messages in thread
From: Johan Hovold @ 2014-09-05 16:08 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Greg KH, Alan Stern, Bjørn Mork, Sarah Sharp,
	Drew Von Spreecken, Mathias Nyman,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Oliver Neukum, Johan Hovold

Add quirk to make sure that a device is always polled for input events
even if it hasn't been opened.

This is needed for devices that disconnects from the bus unless the
interrupt endpoint has been polled at least once or when not responding
to an input event (e.g. after having shut down X).

Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/hid/usbhid/hid-core.c | 26 +++++++++++++++++++++++---
 include/linux/hid.h           |  1 +
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 7b88f4c..d8b6976 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -82,7 +82,7 @@ static int hid_start_in(struct hid_device *hid)
 	struct usbhid_device *usbhid = hid->driver_data;
 
 	spin_lock_irqsave(&usbhid->lock, flags);
-	if (hid->open > 0 &&
+	if ((hid->open > 0 || hid->quirks & HID_QUIRK_ALWAYS_POLL) &&
 			!test_bit(HID_DISCONNECTED, &usbhid->iofl) &&
 			!test_bit(HID_SUSPENDED, &usbhid->iofl) &&
 			!test_and_set_bit(HID_IN_RUNNING, &usbhid->iofl)) {
@@ -292,6 +292,8 @@ static void hid_irq_in(struct urb *urb)
 	case 0:			/* success */
 		usbhid_mark_busy(usbhid);
 		usbhid->retry_delay = 0;
+		if ((hid->quirks & HID_QUIRK_ALWAYS_POLL) && !hid->open)
+			break;
 		hid_input_report(urb->context, HID_INPUT_REPORT,
 				 urb->transfer_buffer,
 				 urb->actual_length, 1);
@@ -734,8 +736,10 @@ void usbhid_close(struct hid_device *hid)
 	if (!--hid->open) {
 		spin_unlock_irq(&usbhid->lock);
 		hid_cancel_delayed_stuff(usbhid);
-		usb_kill_urb(usbhid->urbin);
-		usbhid->intf->needs_remote_wakeup = 0;
+		if (!(hid->quirks & HID_QUIRK_ALWAYS_POLL)) {
+			usb_kill_urb(usbhid->urbin);
+			usbhid->intf->needs_remote_wakeup = 0;
+		}
 	} else {
 		spin_unlock_irq(&usbhid->lock);
 	}
@@ -1133,6 +1137,19 @@ static int usbhid_start(struct hid_device *hid)
 
 	set_bit(HID_STARTED, &usbhid->iofl);
 
+	if (hid->quirks & HID_QUIRK_ALWAYS_POLL) {
+		ret = usb_autopm_get_interface(usbhid->intf);
+		if (ret)
+			goto fail;
+		usbhid->intf->needs_remote_wakeup = 1;
+		ret = hid_start_in(hid);
+		if (ret) {
+			dev_err(&hid->dev,
+				"failed to start in urb: %d\n", ret);
+		}
+		usb_autopm_put_interface(usbhid->intf);
+	}
+
 	/* 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.
@@ -1165,6 +1182,9 @@ static void usbhid_stop(struct hid_device *hid)
 	if (WARN_ON(!usbhid))
 		return;
 
+	if (hid->quirks & HID_QUIRK_ALWAYS_POLL)
+		usbhid->intf->needs_remote_wakeup = 0;
+
 	clear_bit(HID_STARTED, &usbhid->iofl);
 	spin_lock_irq(&usbhid->lock);	/* Sync with error and led handlers */
 	set_bit(HID_DISCONNECTED, &usbhid->iofl);
diff --git a/include/linux/hid.h b/include/linux/hid.h
index 77632cf..05eb710 100644
--- a/include/linux/hid.h
+++ b/include/linux/hid.h
@@ -286,6 +286,7 @@ struct hid_item {
 #define HID_QUIRK_HIDINPUT_FORCE		0x00000080
 #define HID_QUIRK_NO_EMPTY_INPUT		0x00000100
 #define HID_QUIRK_NO_INIT_INPUT_REPORTS		0x00000200
+#define HID_QUIRK_ALWAYS_POLL			0x00000400
 #define HID_QUIRK_SKIP_OUTPUT_REPORTS		0x00010000
 #define HID_QUIRK_SKIP_OUTPUT_REPORT_ID		0x00020000
 #define HID_QUIRK_NO_OUTPUT_REPORTS_ON_INTR_EP	0x00040000
-- 
1.8.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/2] HID: usbhid: enable always-poll quirk for Elan Touchscreen
       [not found]                       ` <1409933328-7190-1-git-send-email-johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
  2014-09-05 16:08                         ` [PATCH 1/2] HID: usbhid: add always-poll quirk Johan Hovold
@ 2014-09-05 16:08                         ` Johan Hovold
  1 sibling, 0 replies; 8+ messages in thread
From: Johan Hovold @ 2014-09-05 16:08 UTC (permalink / raw)
  To: Jiri Kosina
  Cc: Greg KH, Alan Stern, Bjørn Mork, Sarah Sharp,
	Drew Von Spreecken, Mathias Nyman,
	linux-usb-u79uwXL29TY76Z2rM5mHXA,
	linux-input-u79uwXL29TY76Z2rM5mHXA, Oliver Neukum, Johan Hovold

Enable the always-poll quirk for Elan Touchscreens found on some recent
Samsung laptops.

Without this quirk the device keeps disconnecting from the bus (and is
re-enumerated) unless opened (and kept open, should an input event
occur).

Note that while the device can be run-time suspended, the autosuspend
timeout must be high enough to allow the device to be polled at least
once before being suspended. Specifically, using autosuspend_delay_ms=0
will still cause the device to disconnect on input events.

Signed-off-by: Johan Hovold <johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/hid/hid-ids.h           | 3 +++
 drivers/hid/usbhid/hid-quirks.c | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/hid/hid-ids.h b/drivers/hid/hid-ids.h
index 48b66bb..79b966d 100644
--- a/drivers/hid/hid-ids.h
+++ b/drivers/hid/hid-ids.h
@@ -296,6 +296,9 @@
 #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_73F7	0x73f7
 #define USB_DEVICE_ID_DWAV_EGALAX_MULTITOUCH_A001	0xa001
 
+#define USB_VENDOR_ID_ELAN		0x04f3
+#define USB_DEVICE_ID_ELAN_TOUCHSCREEN	0x0089
+
 #define USB_VENDOR_ID_ELECOM		0x056e
 #define USB_DEVICE_ID_ELECOM_BM084	0x0061
 
diff --git a/drivers/hid/usbhid/hid-quirks.c b/drivers/hid/usbhid/hid-quirks.c
index 31e6727..72da083 100644
--- a/drivers/hid/usbhid/hid-quirks.c
+++ b/drivers/hid/usbhid/hid-quirks.c
@@ -70,6 +70,7 @@ static const struct hid_blacklist {
 	{ USB_VENDOR_ID_CH, USB_DEVICE_ID_CH_3AXIS_5BUTTON_STICK, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_CH, USB_DEVICE_ID_CH_AXIS_295, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_DMI, USB_DEVICE_ID_DMI_ENC, HID_QUIRK_NOGET },
+	{ USB_VENDOR_ID_ELAN, USB_DEVICE_ID_ELAN_TOUCHSCREEN, HID_QUIRK_ALWAYS_POLL },
 	{ USB_VENDOR_ID_ELO, USB_DEVICE_ID_ELO_TS2700, HID_QUIRK_NOGET },
 	{ USB_VENDOR_ID_FORMOSA, USB_DEVICE_ID_FORMOSA_IR_RECEIVER, HID_QUIRK_NO_INIT_REPORTS },
 	{ USB_VENDOR_ID_FREESCALE, USB_DEVICE_ID_FREESCALE_MX28, HID_QUIRK_NOGET },
-- 
1.8.5.5

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/2] HID: usbhid: fix Elan touchscreen disconnects
  2014-09-05 16:08                     ` [PATCH 0/2] HID: usbhid: fix Elan touchscreen disconnects Johan Hovold
       [not found]                       ` <1409933328-7190-1-git-send-email-johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
@ 2014-09-08  7:30                       ` Jiri Kosina
  1 sibling, 0 replies; 8+ messages in thread
From: Jiri Kosina @ 2014-09-08  7:30 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Greg KH, Alan Stern, Bjørn Mork, Sarah Sharp,
	Drew Von Spreecken, Mathias Nyman, linux-usb, linux-input,
	Oliver Neukum

On Fri, 5 Sep 2014, Johan Hovold wrote:

> Here's the always-poll quirk that is needed to prevent the Elan
> touchscreen from disconnecting itself from the bus.
> 
> These patches are against v3.16.1, but applies fine to hid-next.
> 
> Note that this series is not dependent on the device-qualifier quirk
> [1], which is needed to get the same device to enumerate reliably, so
> these two quirks could in through the USB and HID tree, respectively.

Thanks for respinning this patchset. I have queued it for 3.18.

Oliver, could you please send me your patch for 0x093a / 0x2510 so that I 
could apply it on top of this?

Thanks,

-- 
Jiri Kosina
SUSE Labs

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2014-09-08  7:30 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <874n047rre.fsf@nemi.mork.no>
     [not found] ` <Pine.LNX.4.44L0.1406011203140.16904-100000@netrider.rowland.org>
     [not found]   ` <20140601174321.GA29188@kroah.com>
     [not found]     ` <20140707092442.GF29098@localhost>
     [not found]       ` <20140708053430.GA840@kroah.com>
     [not found]         ` <20140709130733.GA10078@localhost>
2014-07-14 18:25           ` USB reset xhci_hcd for ELAN touchscreen Johan Hovold
2014-08-20 13:20             ` Jiri Kosina
2014-08-21 10:54               ` Johan Hovold
2014-08-21 18:38                 ` Jiri Kosina
     [not found]                   ` <alpine.LNX.2.00.1408211337050.23162-ztGlSCb7Y1iN3ZZ/Hiejyg@public.gmane.org>
2014-09-05 16:08                     ` [PATCH 0/2] HID: usbhid: fix Elan touchscreen disconnects Johan Hovold
     [not found]                       ` <1409933328-7190-1-git-send-email-johan-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
2014-09-05 16:08                         ` [PATCH 1/2] HID: usbhid: add always-poll quirk Johan Hovold
2014-09-05 16:08                         ` [PATCH 2/2] HID: usbhid: enable always-poll quirk for Elan Touchscreen Johan Hovold
2014-09-08  7:30                       ` [PATCH 0/2] HID: usbhid: fix Elan touchscreen disconnects Jiri Kosina

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).