[0000/0003]full power management for usb touchscreens

[0000/0003]full power management for usb touchscreens

[Oliver Neukum]

Hi,

this patch series implements:

- full support for suspend/resume/reset_resume
- runtime power management

It does so by adding normal methods for suspend/resume,
splitting device intialization into a part that needs to be repeated
after reset_resume() and a part that does not.
Runtime power management is possible while the device is
active if the device supports remote wakeup, otherwise a fallback
to a method based on open/close is implemented.

I lack the hardware. Therefore I need testers.

	Regards
		Oliver
--
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

Re: [0000/0003]full power management for usb touchscreens

[Dmitry Torokhov]

On Mon, Jun 07, 2010 at 03:13:29PM +0200, Oliver Neukum wrote:
> Hi,
> 
> this patch series implements:
> 
> - full support for suspend/resume/reset_resume
> - runtime power management
> 
> It does so by adding normal methods for suspend/resume,
> splitting device intialization into a part that needs to be repeated
> after reset_resume() and a part that does not.
> Runtime power management is possible while the device is
> active if the device supports remote wakeup, otherwise a fallback
> to a method based on open/close is implemented.
> 
> I lack the hardware. Therefore I need testers.
> 

Ondrej,

You are the latest person I know who users usbtouchscreen driver, any
chance you could lend a hand to Oliver here, please?

Thanks!

-- 
Dmitry
--
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

Re: [0000/0003]full power management for usb touchscreens

[Ondrej Zary]

On Monday 07 June 2010, Dmitry Torokhov wrote:
> On Mon, Jun 07, 2010 at 03:13:29PM +0200, Oliver Neukum wrote:
> > Hi,
> >
> > this patch series implements:
> >
> > - full support for suspend/resume/reset_resume
> > - runtime power management
> >
> > It does so by adding normal methods for suspend/resume,
> > splitting device intialization into a part that needs to be repeated
> > after reset_resume() and a part that does not.
> > Runtime power management is possible while the device is
> > active if the device supports remote wakeup, otherwise a fallback
> > to a method based on open/close is implemented.
> >
> > I lack the hardware. Therefore I need testers.
>
> Ondrej,
>
> You are the latest person I know who users usbtouchscreen driver, any
> chance you could lend a hand to Oliver here, please?

I can test Nexio and IRTOUCHSYSTEMS. After I fix the fscking broken Intel KMS 
driver...


-- 
Ondrej Zary
--
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

Re: [0000/0003]full power management for usb touchscreens

[Ondrej Zary]

On Monday 07 June 2010, Dmitry Torokhov wrote:
> On Mon, Jun 07, 2010 at 03:13:29PM +0200, Oliver Neukum wrote:
> > Hi,
> >
> > this patch series implements:
> >
> > - full support for suspend/resume/reset_resume
> > - runtime power management
> >
> > It does so by adding normal methods for suspend/resume,
> > splitting device intialization into a part that needs to be repeated
> > after reset_resume() and a part that does not.
> > Runtime power management is possible while the device is
> > active if the device supports remote wakeup, otherwise a fallback
> > to a method based on open/close is implemented.
> >
> > I lack the hardware. Therefore I need testers.
>
> Ondrej,
>
> You are the latest person I know who users usbtouchscreen driver, any
> chance you could lend a hand to Oliver here, please?

Looking at the patches from archive:

If I understand it correctly, usbtouch_resume submits the irq URB only if
the input device is open. That will fail with Nexio as it has an internal
watchdog which needs URBs or the device will reconnect - see irq_always
(Zytronic seems to need it too).


>From patch 3/3:
> @@ -1449,14 +1483,23 @@ static int usbtouch_probe(struct usb_interface
> *intf, usbtouch->irq->transfer_dma = usbtouch->data_dma;
>         usbtouch->irq->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>
> -       /* device specific init */
> +       /* device specific allocations */
>         if (type->init) {

Looks like a copy&paste bug, should be "if (type->alloc) {".

> -               err = type->init(usbtouch);
> +               err = type->alloc(usbtouch);
>                 if (err) {
> -                       dbg("%s - type->init() failed, err: %d", __func__,
> err); +                       dbg("%s - type->alloc() failed, err: %d",
> __func__, err); goto out_free_urb;
>                 }
>         }
> +
> +       /* device specific init*/
> +       if (type->init) {
> +               err = type->alloc(usbtouch);
> +               if (err) {
> +                       dbg("%s - type->init() failed, err: %d", __func__,
> err); +                       goto out_do_exit;
> +               }
> +       }
>
>         err = input_register_device(usbtouch->input);
>         if (err) {


-- 
Ondrej Zary

Re: [0000/0003]full power management for usb touchscreens

[Oliver Neukum]

Am Mittwoch, 9. Juni 2010 15:40:04 schrieb Ondrej Zary:

> Looking at the patches from archive:
> 
> If I understand it correctly, usbtouch_resume submits the irq URB only if
> the input device is open. That will fail with Nexio as it has an internal
> watchdog which needs URBs or the device will reconnect - see irq_always
> (Zytronic seems to need it too).

Thank you for the review. Does this patch fix the issues?

	Regards
		Oliver

>From 3fe66013333931b4b0cb7361fbe60d502d306eb4 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oliver@neukum.org>
Date: Wed, 9 Jun 2010 18:32:58 +0200
Subject: [PATCH] usbtouchscreen: fix typo and touchpads which require constant polling

1. fix typo
2. start polling on closed devices if they require it

Signed-off-by: Oliver Neukum <oneukum@suse.de>
---
 drivers/input/touchscreen/usbtouchscreen.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
index 421f168..d13517a 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -1333,7 +1333,7 @@ static int usbtouch_resume(struct usb_interface *intf)
 	int result = 0;
 	
 	mutex_lock(&input->mutex);
-	if (input->users)
+	if (input->users || usbtouch->type->irq_always)
 		result = usb_submit_urb(usbtouch->irq, GFP_NOIO);
 	mutex_unlock(&input->mutex);
 	return result;
@@ -1484,7 +1484,7 @@ static int usbtouch_probe(struct usb_interface *intf,
 	usbtouch->irq->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
 
 	/* device specific allocations */
-	if (type->init) {
+	if (type->alloc) {
 		err = type->alloc(usbtouch);
 		if (err) {
 			dbg("%s - type->alloc() failed, err: %d", __func__, err);
-- 
1.6.4.2

Re: [0000/0003]full power management for usb touchscreens

[Ondrej Zary]

On Wednesday 09 June 2010, Oliver Neukum wrote:
> Am Mittwoch, 9. Juni 2010 15:40:04 schrieb Ondrej Zary:
> > Looking at the patches from archive:
> >
> > If I understand it correctly, usbtouch_resume submits the irq URB only if
> > the input device is open. That will fail with Nexio as it has an internal
> > watchdog which needs URBs or the device will reconnect - see irq_always
> > (Zytronic seems to need it too).
>
> Thank you for the review. Does this patch fix the issues?

I haven't tested the patches before. Now I tested them and the Nexio 
touchscreen does not play well with suspend. It always reconnects after 
resume from STR or hibernation. Autosuspend is probably never activated at 
the URBs are submitted continuously.

> 	Regards
> 		Oliver
>
> From 3fe66013333931b4b0cb7361fbe60d502d306eb4 Mon Sep 17 00:00:00 2001
> From: Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
> Date: Wed, 9 Jun 2010 18:32:58 +0200
> Subject: [PATCH] usbtouchscreen: fix typo and touchpads which require
> constant polling
>
> 1. fix typo
> 2. start polling on closed devices if they require it
>
> Signed-off-by: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
> ---
>  drivers/input/touchscreen/usbtouchscreen.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/input/touchscreen/usbtouchscreen.c
> b/drivers/input/touchscreen/usbtouchscreen.c index 421f168..d13517a 100644
> --- a/drivers/input/touchscreen/usbtouchscreen.c
> +++ b/drivers/input/touchscreen/usbtouchscreen.c
> @@ -1333,7 +1333,7 @@ static int usbtouch_resume(struct usb_interface
> *intf) int result = 0;
>
>  	mutex_lock(&input->mutex);
> -	if (input->users)
> +	if (input->users || usbtouch->type->irq_always)
>  		result = usb_submit_urb(usbtouch->irq, GFP_NOIO);
>  	mutex_unlock(&input->mutex);
>  	return result;
> @@ -1484,7 +1484,7 @@ static int usbtouch_probe(struct usb_interface *intf,
>  	usbtouch->irq->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
>
>  	/* device specific allocations */
> -	if (type->init) {
> +	if (type->alloc) {
>  		err = type->alloc(usbtouch);
>  		if (err) {
>  			dbg("%s - type->alloc() failed, err: %d", __func__, err);



-- 
Ondrej Zary
--
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

Re: [0000/0003]full power management for usb touchscreens

[Oliver Neukum]

Am Donnerstag, 10. Juni 2010 13:33:57 schrieb Ondrej Zary:
> On Wednesday 09 June 2010, Oliver Neukum wrote:
> > Am Mittwoch, 9. Juni 2010 15:40:04 schrieb Ondrej Zary:
> > > Looking at the patches from archive:
> > >
> > > If I understand it correctly, usbtouch_resume submits the irq URB only if
> > > the input device is open. That will fail with Nexio as it has an internal
> > > watchdog which needs URBs or the device will reconnect - see irq_always
> > > (Zytronic seems to need it too).
> >
> > Thank you for the review. Does this patch fix the issues?
> 
> I haven't tested the patches before. Now I tested them and the Nexio 
> touchscreen does not play well with suspend. It always reconnects after 
> resume from STR or hibernation. Autosuspend is probably never activated at 
> the URBs are submitted continuously.

I see. Can you compile with CONFIG_USB_DEBUG and send me the log?

	Thank you
		Oliver

Re: [0000/0003]full power management for usb touchscreens

[Ondrej Zary]

On Thursday 10 June 2010, Oliver Neukum wrote:
> Am Donnerstag, 10. Juni 2010 13:33:57 schrieb Ondrej Zary:
> > On Wednesday 09 June 2010, Oliver Neukum wrote:
> > > Am Mittwoch, 9. Juni 2010 15:40:04 schrieb Ondrej Zary:
> > > > Looking at the patches from archive:
> > > >
> > > > If I understand it correctly, usbtouch_resume submits the irq URB
> > > > only if the input device is open. That will fail with Nexio as it has
> > > > an internal watchdog which needs URBs or the device will reconnect -
> > > > see irq_always (Zytronic seems to need it too).
> > >
> > > Thank you for the review. Does this patch fix the issues?
> >
> > I haven't tested the patches before. Now I tested them and the Nexio
> > touchscreen does not play well with suspend. It always reconnects after
> > resume from STR or hibernation. Autosuspend is probably never activated
> > at the URBs are submitted continuously.
>
> I see. Can you compile with CONFIG_USB_DEBUG and send me the log?

This can't be fixed - no matter what you do, the device reconnects if you stop 
talking to it.

-- 
Ondrej Zary
--
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

Re: [0000/0003]full power management for usb touchscreens

[Oliver Neukum]

Am Donnerstag, 10. Juni 2010 16:13:51 schrieb Ondrej Zary:
> > > I haven't tested the patches before. Now I tested them and the Nexio
> > > touchscreen does not play well with suspend. It always reconnects after
> > > resume from STR or hibernation. Autosuspend is probably never activated
> > > at the URBs are submitted continuously.
> >
> > I see. Can you compile with CONFIG_USB_DEBUG and send me the log?
> 
> This can't be fixed - no matter what you do, the device reconnects if you stop 
> talking to it.

Even if you immediately go into suspend?

	Regards
		Oliver
--
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

Re: [0000/0003]full power management for usb touchscreens

[Oliver Neukum]

Am Donnerstag, 10. Juni 2010 16:13:51 schrieb Ondrej Zary:
> > > I haven't tested the patches before. Now I tested them and the Nexio
> > > touchscreen does not play well with suspend. It always reconnects after
> > > resume from STR or hibernation. Autosuspend is probably never activated
> > > at the URBs are submitted continuously.
> >
> > I see. Can you compile with CONFIG_USB_DEBUG and send me the log?
> 
> This can't be fixed - no matter what you do, the device reconnects if you stop 
> talking to it.

Have you tried setting it back to config 0?

	Regards
		Oliver

Re: [0000/0003]full power management for usb touchscreens

[Ondrej Zary]

On Friday 18 June 2010 20:22:07 Oliver Neukum wrote:
> Am Donnerstag, 10. Juni 2010 16:13:51 schrieb Ondrej Zary:
> > > > I haven't tested the patches before. Now I tested them and the Nexio
> > > > touchscreen does not play well with suspend. It always reconnects
> > > > after resume from STR or hibernation. Autosuspend is probably never
> > > > activated at the URBs are submitted continuously.
> > >
> > > I see. Can you compile with CONFIG_USB_DEBUG and send me the log?
> >
> > This can't be fixed - no matter what you do, the device reconnects if you
> > stop talking to it.
>
> Have you tried setting it back to config 0?

No, I haven't. It reconnects even if there's no driver loaded (e.g. older 
kernel). I don't know if kernel sets configuration on all USB devices upon 
connect (even if there's no driver).

-- 
Ondrej Zary

Re: [0000/0003]full power management for usb touchscreens

[Oliver Neukum]

Am Freitag, 18. Juni 2010 20:34:03 schrieb Ondrej Zary:
> On Friday 18 June 2010 20:22:07 Oliver Neukum wrote:
> > Am Donnerstag, 10. Juni 2010 16:13:51 schrieb Ondrej Zary:
> > > > > I haven't tested the patches before. Now I tested them and the Nexio
> > > > > touchscreen does not play well with suspend. It always reconnects
> > > > > after resume from STR or hibernation. Autosuspend is probably never
> > > > > activated at the URBs are submitted continuously.
> > > >
> > > > I see. Can you compile with CONFIG_USB_DEBUG and send me the log?
> > >
> > > This can't be fixed - no matter what you do, the device reconnects if you
> > > stop talking to it.
> >
> > Have you tried setting it back to config 0?
> 
> No, I haven't. It reconnects even if there's no driver loaded (e.g. older 
> kernel). I don't know if kernel sets configuration on all USB devices upon 
> connect (even if there's no driver).

Yes, the kernel sets a configuration if the power budget allows it.
You can set it back to 0 through sysfs and see what happens.

	Regards
		Oliver
--
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

Re: [0000/0003]full power management for usb touchscreens

[Ondrej Zary]

On Friday 18 June 2010, Oliver Neukum wrote:
> Am Freitag, 18. Juni 2010 20:34:03 schrieb Ondrej Zary:
> > On Friday 18 June 2010 20:22:07 Oliver Neukum wrote:
> > > Am Donnerstag, 10. Juni 2010 16:13:51 schrieb Ondrej Zary:
> > > > > > I haven't tested the patches before. Now I tested them and the
> > > > > > Nexio touchscreen does not play well with suspend. It always
> > > > > > reconnects after resume from STR or hibernation. Autosuspend is
> > > > > > probably never activated at the URBs are submitted continuously.
> > > > >
> > > > > I see. Can you compile with CONFIG_USB_DEBUG and send me the log?
> > > >
> > > > This can't be fixed - no matter what you do, the device reconnects if
> > > > you stop talking to it.
> > >
> > > Have you tried setting it back to config 0?
> >
> > No, I haven't. It reconnects even if there's no driver loaded (e.g. older
> > kernel). I don't know if kernel sets configuration on all USB devices
> > upon connect (even if there's no driver).
>
> Yes, the kernel sets a configuration if the power budget allows it.
> You can set it back to 0 through sysfs and see what happens.

It disconnects and reconnects after bConfigurationValue is set to 0.

-- 
Ondrej Zary

Re: [0000/0003]full power management for usb touchscreens

[Oliver Neukum]

Am Dienstag, 22. Juni 2010, 13:06:12 schrieb Ondrej Zary:
> On Friday 18 June 2010, Oliver Neukum wrote:
> > Am Freitag, 18. Juni 2010 20:34:03 schrieb Ondrej Zary:
> > > On Friday 18 June 2010 20:22:07 Oliver Neukum wrote:
> > > > Am Donnerstag, 10. Juni 2010 16:13:51 schrieb Ondrej Zary:
> > > > > > > I haven't tested the patches before. Now I tested them and the
> > > > > > > Nexio touchscreen does not play well with suspend. It always
> > > > > > > reconnects after resume from STR or hibernation. Autosuspend is
> > > > > > > probably never activated at the URBs are submitted continuously.
> > > > > >
> > > > > > I see. Can you compile with CONFIG_USB_DEBUG and send me the log?
> > > > >
> > > > > This can't be fixed - no matter what you do, the device reconnects if
> > > > > you stop talking to it.
> > > >
> > > > Have you tried setting it back to config 0?
> > >
> > > No, I haven't. It reconnects even if there's no driver loaded (e.g. older
> > > kernel). I don't know if kernel sets configuration on all USB devices
> > > upon connect (even if there's no driver).
> >
> > Yes, the kernel sets a configuration if the power budget allows it.
> > You can set it back to 0 through sysfs and see what happens.
> 
> It disconnects and reconnects after bConfigurationValue is set to 0.

This is a violation of the spec. Can you give me the IDs so that I can
put the devices into the quirks file? Or can I take this from the driver?
Do all devices that marked as always needing an irq transfer behave
this way?

	Regards
		Oliver

Re: [0000/0003]full power management for usb touchscreens

[Ondrej Zary]

On Tuesday 22 June 2010, Oliver Neukum wrote:
> Am Dienstag, 22. Juni 2010, 13:06:12 schrieb Ondrej Zary:
> > On Friday 18 June 2010, Oliver Neukum wrote:
> > > Am Freitag, 18. Juni 2010 20:34:03 schrieb Ondrej Zary:
> > > > On Friday 18 June 2010 20:22:07 Oliver Neukum wrote:
> > > > > Am Donnerstag, 10. Juni 2010 16:13:51 schrieb Ondrej Zary:
> > > > > > > > I haven't tested the patches before. Now I tested them and
> > > > > > > > the Nexio touchscreen does not play well with suspend. It
> > > > > > > > always reconnects after resume from STR or hibernation.
> > > > > > > > Autosuspend is probably never activated at the URBs are
> > > > > > > > submitted continuously.
> > > > > > >
> > > > > > > I see. Can you compile with CONFIG_USB_DEBUG and send me the
> > > > > > > log?
> > > > > >
> > > > > > This can't be fixed - no matter what you do, the device
> > > > > > reconnects if you stop talking to it.
> > > > >
> > > > > Have you tried setting it back to config 0?
> > > >
> > > > No, I haven't. It reconnects even if there's no driver loaded (e.g.
> > > > older kernel). I don't know if kernel sets configuration on all USB
> > > > devices upon connect (even if there's no driver).
> > >
> > > Yes, the kernel sets a configuration if the power budget allows it.
> > > You can set it back to 0 through sysfs and see what happens.
> >
> > It disconnects and reconnects after bConfigurationValue is set to 0.
>
> This is a violation of the spec. Can you give me the IDs so that I can
> put the devices into the quirks file? Or can I take this from the driver?
> Do all devices that marked as always needing an irq transfer behave
> this way?

This is not a bug surprise as this device is broken in many ways. It takes 
ages to detect and identify and looks like cdc_acm. The pseudo-multitouch 
capable protocol is pretty bad too. Also there are some reports about devices 
using the same ID but different protocol.

I have only one - 1870:0001, don't know anything about the other.

-- 
Ondrej Zary
--
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

Re: [0000/0003]full power management for usb touchscreens

[Oliver Neukum]

[-- Attachment #1: Type: Text/Plain, Size: 872 bytes --]

Am Dienstag, 22. Juni 2010, 15:43:24 schrieb Ondrej Zary:
> On Tuesday 22 June 2010, Oliver Neukum wrote:

> > This is a violation of the spec. Can you give me the IDs so that I can
> > put the devices into the quirks file? Or can I take this from the driver?
> > Do all devices that marked as always needing an irq transfer behave
> > this way?
> 
> This is not a bug surprise as this device is broken in many ways. It takes 
> ages to detect and identify and looks like cdc_acm. The pseudo-multitouch 
> capable protocol is pretty bad too. Also there are some reports about devices 
> using the same ID but different protocol.
> 
> I have only one - 1870:0001, don't know anything about the other.

OK, it seems like those devices are hopeless. Could you test this series form
harmlessness on a broken device and functionality on a non-broken device?

	Regards
		Oliver

[-- Attachment #2: 0001-usbtouchscreen-Implement-basic-suspend-resume.patch --]
[-- Type: text/x-patch, Size: 1825 bytes --]

From 0f28efab0c7c48dc02f728f57d4a89a203bac986 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
Date: Thu, 24 Jun 2010 08:20:34 +0200
Subject: [PATCH 1/3] usbtouchscreen: Implement basic suspend/resume

This implements basic support for suspend & resume

Signed-off-by: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
---
 drivers/input/touchscreen/usbtouchscreen.c |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
index 567d572..f51d2fd 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -1291,6 +1291,27 @@ static void usbtouch_close(struct input_dev *input)
 		usb_kill_urb(usbtouch->irq);
 }
 
+static int usbtouch_suspend
+(struct usb_interface *intf, pm_message_t message)
+{
+	struct usbtouch_usb *usbtouch = usb_get_intfdata(intf);
+	
+	usb_kill_urb(usbtouch->irq);
+	return 0;
+}
+
+static int usbtouch_resume(struct usb_interface *intf)
+{
+	struct usbtouch_usb *usbtouch = usb_get_intfdata(intf);
+	struct input_dev *input = usbtouch->input;
+	int result = 0;
+	
+	mutex_lock(&input->mutex);
+	if (input->users || usbtouch->type->irq_always)
+		result = usb_submit_urb(usbtouch->irq, GFP_NOIO);
+	mutex_unlock(&input->mutex);
+	return result;
+}
 
 static void usbtouch_free_buffers(struct usb_device *udev,
 				  struct usbtouch_usb *usbtouch)
@@ -1481,6 +1502,8 @@ static struct usb_driver usbtouch_driver = {
 	.name		= "usbtouchscreen",
 	.probe		= usbtouch_probe,
 	.disconnect	= usbtouch_disconnect,
+	.suspend	= usbtouch_suspend,
+	.resume		= usbtouch_resume,
 	.id_table	= usbtouch_devices,
 };
 
-- 
1.7.1


[-- Attachment #3: 0002-usbtouchscreen-Implement-runtime-power-management.patch --]
[-- Type: text/x-patch, Size: 3046 bytes --]

From 71b07110871d7b9e754a3b56f3e914745c329be6 Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
Date: Thu, 24 Jun 2010 08:38:57 +0200
Subject: [PATCH 2/3] usbtouchscreen: Implement runtime power management

This implement USB autosuspend while the device is opened for
devices that do remote wakeup with a fallback to open/close for
those devices that don't. Devices that require the host to
constantly poll them are never autosuspended.

Signed-off-by: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
---
 drivers/input/touchscreen/usbtouchscreen.c |   27 ++++++++++++++++++++++++---
 1 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
index f51d2fd..3c57df8 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -1263,6 +1263,7 @@ static void usbtouch_irq(struct urb *urb)
 	usbtouch->type->process_pkt(usbtouch, usbtouch->data, urb->actual_length);
 
 exit:
+	usb_mark_last_busy(interface_to_usbdev(usbtouch->interface));
 	retval = usb_submit_urb(urb, GFP_ATOMIC);
 	if (retval)
 		err("%s - usb_submit_urb failed with result: %d",
@@ -1272,23 +1273,39 @@ exit:
 static int usbtouch_open(struct input_dev *input)
 {
 	struct usbtouch_usb *usbtouch = input_get_drvdata(input);
+	int r;
 
 	usbtouch->irq->dev = interface_to_usbdev(usbtouch->interface);
 
+	r = usb_autopm_get_interface(usbtouch->interface) ? -EIO : 0;
+	if (r < 0)
+		goto out;
+	
 	if (!usbtouch->type->irq_always) {
-		if (usb_submit_urb(usbtouch->irq, GFP_KERNEL))
-		  return -EIO;
+		if (usb_submit_urb(usbtouch->irq, GFP_KERNEL)) {
+			r = -EIO;
+			goto out_put;
+		}
 	}
 
-	return 0;
+	usbtouch->interface->needs_remote_wakeup = 1;
+out_put:
+	usb_autopm_put_interface(usbtouch->interface);
+out:
+	return r;
 }
 
 static void usbtouch_close(struct input_dev *input)
 {
 	struct usbtouch_usb *usbtouch = input_get_drvdata(input);
+	int r;
 
 	if (!usbtouch->type->irq_always)
 		usb_kill_urb(usbtouch->irq);
+	r = usb_autopm_get_interface(usbtouch->interface);
+	usbtouch->interface->needs_remote_wakeup = 0;
+	if (!r)
+		usb_autopm_put_interface(usbtouch->interface);
 }
 
 static int usbtouch_suspend
@@ -1450,8 +1467,11 @@ static int usbtouch_probe(struct usb_interface *intf,
 	usb_set_intfdata(intf, usbtouch);
 
 	if (usbtouch->type->irq_always) {
+		/* this can't fail */
+		usb_autopm_get_interface(intf);
 		err = usb_submit_urb(usbtouch->irq, GFP_KERNEL);
 		if (err) {
+			usb_autopm_put_interface(intf);
 			err("%s - usb_submit_urb failed with result: %d",
 			    __func__, err);
 			goto out_unregister_input;
@@ -1505,6 +1525,7 @@ static struct usb_driver usbtouch_driver = {
 	.suspend	= usbtouch_suspend,
 	.resume		= usbtouch_resume,
 	.id_table	= usbtouch_devices,
+	.supports_autosuspend = 1,
 };
 
 static int __init usbtouch_init(void)
-- 
1.7.1


[-- Attachment #4: 0003-usbtouchscreen-Implement-reset_resume.patch --]
[-- Type: text/x-patch, Size: 6156 bytes --]

From c12539f30d0585056be37227a8d65976db49241f Mon Sep 17 00:00:00 2001
From: Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>
Date: Thu, 24 Jun 2010 08:50:25 +0200
Subject: [PATCH 3/3] usbtouchscreen: Implement reset_resume()

This implements reset_resume() by splitting init into allocations
of private data structures and device initializations. Device initializations
are repeated upon reset_resume.

Signed-off-by: Oliver Neukum <oneukum-l3A5Bk7waGM@public.gmane.org>
---
 drivers/input/touchscreen/usbtouchscreen.c |  108 +++++++++++++++++++--------
 1 files changed, 76 insertions(+), 32 deletions(-)

diff --git a/drivers/input/touchscreen/usbtouchscreen.c b/drivers/input/touchscreen/usbtouchscreen.c
index 3c57df8..9c21707 100644
--- a/drivers/input/touchscreen/usbtouchscreen.c
+++ b/drivers/input/touchscreen/usbtouchscreen.c
@@ -95,6 +95,7 @@ struct usbtouch_device_info {
 	int  (*get_pkt_len) (unsigned char *pkt, int len);
 
 	int  (*read_data)   (struct usbtouch_usb *usbtouch, unsigned char *pkt);
+	int  (*alloc)       (struct usbtouch_usb *usbtouch);
 	int  (*init)        (struct usbtouch_usb *usbtouch);
 	void (*exit)	    (struct usbtouch_usb *usbtouch);
 };
@@ -507,7 +508,7 @@ static int dmc_tsc10_init(struct usbtouch_usb *usbtouch)
 	int ret = -ENOMEM;
 	unsigned char *buf;
 
-	buf = kmalloc(2, GFP_KERNEL);
+	buf = kmalloc(2, GFP_NOIO);
 	if (!buf)
 		goto err_nobuf;
 	/* reset */
@@ -732,11 +733,43 @@ static void nexio_ack_complete(struct urb *urb)
 {
 }
 
+static int nexio_alloc(struct usbtouch_usb *usbtouch)
+{
+	struct nexio_priv *priv;
+	int ret = -ENOMEM;
+
+	usbtouch->priv = kmalloc(sizeof(struct nexio_priv), GFP_KERNEL);
+	if (!usbtouch->priv)
+		goto out_buf;
+
+	priv = usbtouch->priv;
+
+	priv->ack_buf = kmemdup(nexio_ack_pkt, sizeof(nexio_ack_pkt),
+				GFP_KERNEL);
+	if (!priv->ack_buf)
+		goto err_priv;
+
+	priv->ack = usb_alloc_urb(0, GFP_KERNEL);
+	if (!priv->ack) {
+		dbg("%s - usb_alloc_urb failed: usbtouch->ack", __func__);
+		goto err_ack_buf;
+	}
+
+	return 0;
+
+err_ack_buf:
+	kfree(priv->ack_buf);
+err_priv:
+	kfree(priv);
+out_buf:
+	return ret;
+}
+
 static int nexio_init(struct usbtouch_usb *usbtouch)
 {
 	struct usb_device *dev = interface_to_usbdev(usbtouch->interface);
 	struct usb_host_interface *interface = usbtouch->interface->cur_altsetting;
-	struct nexio_priv *priv;
+	struct nexio_priv *priv = usbtouch->priv;
 	int ret = -ENOMEM;
 	int actual_len, i;
 	unsigned char *buf;
@@ -755,7 +788,7 @@ static int nexio_init(struct usbtouch_usb *usbtouch)
 	if (!input_ep || !output_ep)
 		return -ENXIO;
 
-	buf = kmalloc(NEXIO_BUFSIZE, GFP_KERNEL);
+	buf = kmalloc(NEXIO_BUFSIZE, GFP_NOIO);
 	if (!buf)
 		goto out_buf;
 
@@ -787,11 +820,11 @@ static int nexio_init(struct usbtouch_usb *usbtouch)
 		switch (buf[0]) {
 		case 0x83:	/* firmware version */
 			if (!firmware_ver)
-				firmware_ver = kstrdup(&buf[2], GFP_KERNEL);
+				firmware_ver = kstrdup(&buf[2], GFP_NOIO);
 			break;
 		case 0x84:	/* device name */
 			if (!device_name)
-				device_name = kstrdup(&buf[2], GFP_KERNEL);
+				device_name = kstrdup(&buf[2], GFP_NOIO);
 			break;
 		}
 	}
@@ -802,36 +835,11 @@ static int nexio_init(struct usbtouch_usb *usbtouch)
 	kfree(firmware_ver);
 	kfree(device_name);
 
-	/* prepare ACK URB */
-	ret = -ENOMEM;
-
-	usbtouch->priv = kmalloc(sizeof(struct nexio_priv), GFP_KERNEL);
-	if (!usbtouch->priv)
-		goto out_buf;
-
-	priv = usbtouch->priv;
-
-	priv->ack_buf = kmemdup(nexio_ack_pkt, sizeof(nexio_ack_pkt),
-				GFP_KERNEL);
-	if (!priv->ack_buf)
-		goto err_priv;
-
-	priv->ack = usb_alloc_urb(0, GFP_KERNEL);
-	if (!priv->ack) {
-		dbg("%s - usb_alloc_urb failed: usbtouch->ack", __func__);
-		goto err_ack_buf;
-	}
-
 	usb_fill_bulk_urb(priv->ack, dev, usb_sndbulkpipe(dev, output_ep),
 			  priv->ack_buf, sizeof(nexio_ack_pkt),
 			  nexio_ack_complete, usbtouch);
 	ret = 0;
-	goto out_buf;
 
-err_ack_buf:
-	kfree(priv->ack_buf);
-err_priv:
-	kfree(priv);
 out_buf:
 	kfree(buf);
 	return ret;
@@ -1120,6 +1128,7 @@ static struct usbtouch_device_info usbtouch_dev_info[] = {
 		.rept_size	= 1024,
 		.irq_always	= true,
 		.read_data	= nexio_read_data,
+		.alloc		= nexio_alloc,
 		.init		= nexio_init,
 		.exit		= nexio_exit,
 	},
@@ -1330,6 +1339,31 @@ static int usbtouch_resume(struct usb_interface *intf)
 	return result;
 }
 
+static int usbtouch_reset_resume(struct usb_interface *intf)
+{
+	struct usbtouch_usb *usbtouch = usb_get_intfdata(intf);
+	struct input_dev *input = usbtouch->input;
+	int err = 0;
+	
+	/* reinit the device */
+	if (usbtouch->type->init) {
+		err = usbtouch->type->init(usbtouch);
+		if (err) {
+			dbg("%s - type->init() failed, err: %d",
+			    __func__, err);
+			return err;
+		}
+	}
+
+	/* restart IO if needed */
+	mutex_lock(&input->mutex);
+	if (input->users)
+		err = usb_submit_urb(usbtouch->irq, GFP_NOIO);
+	mutex_unlock(&input->mutex);
+	
+	return err;
+}
+
 static void usbtouch_free_buffers(struct usb_device *udev,
 				  struct usbtouch_usb *usbtouch)
 {
@@ -1449,12 +1483,21 @@ static int usbtouch_probe(struct usb_interface *intf,
 	usbtouch->irq->transfer_dma = usbtouch->data_dma;
 	usbtouch->irq->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
 
-	/* device specific init */
+	/* device specific allocations */
+	if (type->alloc) {
+		err = type->alloc(usbtouch);
+		if (err) {
+			dbg("%s - type->alloc() failed, err: %d", __func__, err);
+			goto out_free_urb;
+		}
+	}
+	
+	/* device specific initialisation*/ 
 	if (type->init) {
 		err = type->init(usbtouch);
 		if (err) {
 			dbg("%s - type->init() failed, err: %d", __func__, err);
-			goto out_free_urb;
+			goto out_do_exit;
 		}
 	}
 
@@ -1524,6 +1567,7 @@ static struct usb_driver usbtouch_driver = {
 	.disconnect	= usbtouch_disconnect,
 	.suspend	= usbtouch_suspend,
 	.resume		= usbtouch_resume,
+	.reset_resume	= usbtouch_reset_resume,
 	.id_table	= usbtouch_devices,
 	.supports_autosuspend = 1,
 };
-- 
1.7.1

Re: [0000/0003]full power management for usb touchscreens

[Ondrej Zary]

On Thursday 24 June 2010, Oliver Neukum wrote:
> Am Dienstag, 22. Juni 2010, 15:43:24 schrieb Ondrej Zary:
> > On Tuesday 22 June 2010, Oliver Neukum wrote:
> > > This is a violation of the spec. Can you give me the IDs so that I can
> > > put the devices into the quirks file? Or can I take this from the
> > > driver? Do all devices that marked as always needing an irq transfer
> > > behave this way?
> >
> > This is not a bug surprise as this device is broken in many ways. It
> > takes ages to detect and identify and looks like cdc_acm. The
> > pseudo-multitouch capable protocol is pretty bad too. Also there are some
> > reports about devices using the same ID but different protocol.
> >
> > I have only one - 1870:0001, don't know anything about the other.
>
> OK, it seems like those devices are hopeless. Could you test this series
> form harmlessness on a broken device and functionality on a non-broken
> device?

Tested with:
0eef:0001 D-WAV Scientific Co., Ltd eGalax TouchScreen
595a:0001 IRTOUCHSYSTEMS Co. Ltd. Touchscreen
1870:0001 Nexio Co., Ltd iNexio Touchscreen controller

The patches have no visible effect on these devices so they're probably OK.

-- 
Ondrej Zary