converting appletouch to usb autosuspend again...

converting appletouch to usb autosuspend again...

[Soeren Sonnenburg]

[-- Attachment #1: Type: text/plain, Size: 856 bytes --]

Dear all,

I wonder whether anyone else tried to get appletouch usb autosuspend to
work. I've seen Oliver's patch but as it kept oopsing on me and I
couldn't parse the usb_mark_last_busy logic I came up with the attached
patch against (current git; at least didn't oops for me over the last
several days and suspend/resume cycles).

However things I don't understand, 

a) is autosuspend on the appletouch driver used at all (how can I find
out)

b) do I need dev->intf->needs_remote_wakeup = 1; ?

c) is line 33 in the patch safe to do ?

And while we are at it I am still seeing these X falls back to hid mouse
mode, only switching to console and back resolves this (looks like this
only worked in previous kernels as resume from s2ram took a lot longer
due to the IDE driver doing a couple of resets) - is there anything one
could do about it?

Soeren


[-- Attachment #2: appletouch_autosuspend.diff --]
[-- Type: text/x-patch, Size: 3921 bytes --]

diff --git a/drivers/input/mouse/appletouch.c b/drivers/input/mouse/appletouch.c
index 1f41ae9..19003c8 100644
--- a/drivers/input/mouse/appletouch.c
+++ b/drivers/input/mouse/appletouch.c
@@ -34,6 +34,7 @@
 #include <linux/slab.h>
 #include <linux/module.h>
 #include <linux/usb/input.h>
+#include <linux/mutex.h>
 
 /* Type of touchpad */
 enum atp_touchpad_type {
@@ -140,10 +141,12 @@ MODULE_DEVICE_TABLE(usb, atp_table);
 struct atp {
 	char			phys[64];
 	struct usb_device	*udev;		/* usb device */
+	struct usb_interface	*intf;		/* our interface */
 	struct urb		*urb;		/* usb request block */
 	signed char		*data;		/* transferred data */
 	struct input_dev	*input;		/* input dev */
 	enum atp_touchpad_type	type;		/* type of touchpad */
+	struct mutex		pm_mutex;	/* serialize access to open/suspend */
 	bool			open;
 	bool			valid;		/* are the samples valid? */
 	bool			size_detect_done;
@@ -259,6 +262,11 @@ static void atp_reinit(struct work_struct *work)
 	if (retval)
 		err("atp_reinit: usb_submit_urb failed with error %d",
 		    retval);
+
+	/* device is idle and available for autosuspend */
+	mutex_lock(&dev->pm_mutex);
+	usb_autopm_disable(dev->intf);
+	mutex_unlock(&dev->pm_mutex);
 }
 
 static int atp_calculate_abs(int *xy_sensors, int nb_sensors, int fact,
@@ -547,21 +555,37 @@ static void atp_complete(struct urb *urb)
 static int atp_open(struct input_dev *input)
 {
 	struct atp *dev = input_get_drvdata(input);
+	int error = usb_autopm_get_interface(dev->intf);
+	if (error)
+		return error;
 
-	if (usb_submit_urb(dev->urb, GFP_ATOMIC))
-		return -EIO;
+	mutex_lock(&dev->pm_mutex);
+	error=usb_submit_urb(dev->urb, GFP_ATOMIC);
 
-	dev->open = 1;
-	return 0;
+	if (!error)
+	    dev->open = 1;
+
+	mutex_unlock(&dev->pm_mutex);
+
+	if (error)
+		usb_autopm_put_interface(dev->intf);
+
+	return error;
 }
 
 static void atp_close(struct input_dev *input)
 {
 	struct atp *dev = input_get_drvdata(input);
 
+	mutex_lock(&dev->pm_mutex);
+
 	usb_kill_urb(dev->urb);
 	cancel_work_sync(&dev->work);
 	dev->open = 0;
+
+	mutex_unlock(&dev->pm_mutex);
+
+	usb_autopm_put_interface(dev->intf);
 }
 
 static int atp_handle_geyser(struct atp *dev)
@@ -615,9 +639,12 @@ static int atp_probe(struct usb_interface *iface,
 	}
 
 	dev->udev = udev;
+	dev->intf = iface;
 	dev->input = input_dev;
 	dev->type = id->driver_info;
 	dev->overflow_warned = false;
+	mutex_init(&dev->pm_mutex);
+
 	if (dev->type == ATP_FOUNTAIN || dev->type == ATP_GEYSER1)
 		dev->datalen = 81;
 	else
@@ -750,27 +777,41 @@ static int atp_suspend(struct usb_interface *iface, pm_message_t message)
 {
 	struct atp *dev = usb_get_intfdata(iface);
 
+	mutex_lock(&dev->pm_mutex);
+
 	usb_kill_urb(dev->urb);
 	dev->valid = false;
 
+	mutex_unlock(&dev->pm_mutex);
+
 	return 0;
 }
 
 static int atp_resume(struct usb_interface *iface)
 {
 	struct atp *dev = usb_get_intfdata(iface);
+	int error = 0;
 
-	if (dev->open && usb_submit_urb(dev->urb, GFP_ATOMIC))
-		return -EIO;
+	mutex_lock(&dev->pm_mutex);
 
-	return 0;
+	if (dev->open)
+	    error = usb_submit_urb(dev->urb, GFP_ATOMIC);
+
+	mutex_unlock(&dev->pm_mutex);
+
+	return error;
 }
 
 static int atp_reset_resume(struct usb_interface *iface)
 {
 	struct atp *dev = usb_get_intfdata(iface);
+	int error = 0;
 
-	return atp_recover(dev);
+	mutex_lock(&dev->pm_mutex);
+	error = atp_recover(dev);
+	mutex_unlock(&dev->pm_mutex);
+
+	return error;
 }
 
 static struct usb_driver atp_driver = {
@@ -781,6 +822,7 @@ static struct usb_driver atp_driver = {
 	.resume		= atp_resume,
 	.reset_resume	= atp_reset_resume,
 	.id_table	= atp_table,
+	.supports_autosuspend	= 1,
 };
 
 static int __init atp_init(void)
@@ -795,3 +837,4 @@ static void __exit atp_exit(void)
 
 module_init(atp_init);
 module_exit(atp_exit);
+

Re: converting appletouch to usb autosuspend again...

[Oliver Neukum]

Am Samstag 23 August 2008 23:00:18 schrieb Soeren Sonnenburg:
> Dear all,
> 
> I wonder whether anyone else tried to get appletouch usb autosuspend to
> work. I've seen Oliver's patch but as it kept oopsing on me and I

You might post the oops. I haven't seen it.

> couldn't parse the usb_mark_last_busy logic I came up with the attached
> patch against (current git; at least didn't oops for me over the last
> several days and suspend/resume cycles).
> 
> However things I don't understand, 
> 
> a) is autosuspend on the appletouch driver used at all (how can I find
> out)

Compile your kernel with CONFIG_USB_DEBUG and autosuspend will be logged.
You need to activate autosuspend via sysfs.

> b) do I need dev->intf->needs_remote_wakeup = 1; ?

Yes.

> c) is line 33 in the patch safe to do ?

Why do you want to add it?

> And while we are at it I am still seeing these X falls back to hid mouse
> mode, only switching to console and back resolves this (looks like this
> only worked in previous kernels as resume from s2ram took a lot longer
> due to the IDE driver doing a couple of resets) - is there anything one
> could do about it?

Post a log.

	Regards
		Oliver

Re: converting appletouch to usb autosuspend again...

[Soeren Sonnenburg]

On Sun, 2008-08-24 at 20:39 +0200, Oliver Neukum wrote:
> Am Samstag 23 August 2008 23:00:18 schrieb Soeren Sonnenburg:
> > Dear all,
> > 
> > I wonder whether anyone else tried to get appletouch usb autosuspend to
> > work. I've seen Oliver's patch but as it kept oopsing on me and I
> 
> You might post the oops. I haven't seen it.

Do you have an updated version of your patch that applies to
git-current? I will give your patch another try then.

> > couldn't parse the usb_mark_last_busy logic I came up with the attached
> > patch against (current git; at least didn't oops for me over the last
> > several days and suspend/resume cycles).
> > 
> > However things I don't understand, 
> > 
> > a) is autosuspend on the appletouch driver used at all (how can I find
> > out)
> 
> Compile your kernel with CONFIG_USB_DEBUG and autosuspend will be logged.
> You need to activate autosuspend via sysfs.

OK, indeed it never autosuspends... I guess this is this due to the
keyboard and the mice being on the same usb port and as the kbd part
does not support autosuspend it never suspends?!

> > b) do I need dev->intf->needs_remote_wakeup = 1; ?
> 
> Yes.
> 
> > c) is line 33 in the patch safe to do ?
> 
> Why do you want to add it?

Well reaching this line the touchpad is idling (hasn't been touched+no
button was pressed) - I somehow wanted to force it to suspend in this
case.

> > And while we are at it I am still seeing these X falls back to hid mouse
> > mode, only switching to console and back resolves this (looks like this
> > only worked in previous kernels as resume from s2ram took a lot longer
> > due to the IDE driver doing a couple of resets) - is there anything one
> > could do about it?
> 
> Post a log.

My fault. I was still rmmod'ing appletouch before suspending. It works
perfectly + reliably the way it is.

Thanks,
Soeren

RE:converting appletouch to usb autosuspend again...

[Justin Mattock]

I've notice when letting the system rest for a few and then tapping the
touchpad I see this in dmesg:
appletouch: 17" model detected.
could this be suspend working?
As for appletouch surving suspend
I't seems to work i.g.
one tap, two tap, side scroll,
although with the latest git
and the latest update from sid(of xserver);
I'm not seeing these functions work....

-- 
Justin P. Mattock

Re: converting appletouch to usb autosuspend again...

[Oliver Neukum]

Am Montag 25 August 2008 06:22:45 schrieb Justin Mattock:
> I've notice when letting the system rest for a few and then tapping the
> touchpad I see this in dmesg:
> appletouch: 17" model detected.
> could this be suspend working?

Yes.

> As for appletouch surving suspend
> I't seems to work i.g.
> one tap, two tap, side scroll,

Good.

> although with the latest git
> and the latest update from sid(of xserver);
> I'm not seeing these functions work....

Please post logs. Without logs nobody can help you.

	Regards
		Oliver

Re: converting appletouch to usb autosuspend again...

[Justin Mattock]

On Sun, Aug 24, 2008 at 11:40 PM, Oliver Neukum <oliver@neukum.org> wrote:
> Am Montag 25 August 2008 06:22:45 schrieb Justin Mattock:
>> I've notice when letting the system rest for a few and then tapping the
>> touchpad I see this in dmesg:
>> appletouch: 17" model detected.
>> could this be suspend working?
>
> Yes.
>
>> As for appletouch surving suspend
>> I't seems to work i.g.
>> one tap, two tap, side scroll,
>
> Good.
>
>> although with the latest git
>> and the latest update from sid(of xserver);
>> I'm not seeing these functions work....
>
> Please post logs. Without logs nobody can help you.
>
>        Regards
>                Oliver
>

Alright; as for the appletouch
not working upon wakeup, I just tried it to see,
and it is working...(can't explain why it wasn't, maybe a quirk somwhere);
Although; I am receiving an error, so maybe this has something to
do with it(heres the thread):
http://marc.info/?l=linux-kernel&m=121926203029380&q=raw
from what I remember I issued echo mem > /sys/power/state
and then left the laptop in sleep for a few hours.
Anyways I'll try and grab as much logs as possible.
and thanks for the info and help.

regards;

-- 
Justin P. Mattock

Re: converting appletouch to usb autosuspend again...

[Oliver Neukum]

Am Montag 25 August 2008 09:08:49 schrieb Justin Mattock:
> Alright; as for the appletouch
> not working upon wakeup, I just tried it to see,
> and it is working...(can't explain why it wasn't, maybe a quirk somwhere);
> Although; I am receiving an error, so maybe this has something to
> do with it(heres the thread):
> http://marc.info/?l=linux-kernel&m=121926203029380&q=raw

That's bluetooth. Totally unrelated.

So to summarize: suspend/resume has been tested and works.
What about autosuspend? Did you test that?

	Regards
		Oliver

Re: converting appletouch to usb autosuspend again...

[Justin Mattock]

On Mon, Aug 25, 2008 at 12:32 AM, Oliver Neukum <oliver@neukum.org> wrote:
> Am Montag 25 August 2008 09:08:49 schrieb Justin Mattock:
>> Alright; as for the appletouch
>> not working upon wakeup, I just tried it to see,
>> and it is working...(can't explain why it wasn't, maybe a quirk somwhere);
>> Although; I am receiving an error, so maybe this has something to
>> do with it(heres the thread):
>> http://marc.info/?l=linux-kernel&m=121926203029380&q=raw
>
> That's bluetooth. Totally unrelated.
>
> So to summarize: suspend/resume has been tested and works.
> What about autosuspend? Did you test that?
>
>        Regards
>                Oliver
>
>

autosuspend, meaning when leaving the system
idle, then after a few minuetes tapping the touchpad!
if this is what you are referring to, then yes I see appletouch working
in that area, but if there is something else I'm missing, then yeah I
can look into
it and see what I find.


-- 
Justin P. Mattock