Re: [linux-pm] s2ram slow (radeon) / failing (usb)

["Bruno Prémont" <bonbons@linux-vserver.org>]

On Thu, 06 May 2010 Alan Stern <stern@rowland.harvard.edu> wrote:
> On Thu, 6 May 2010, Bruno [UTF-8] Prémont wrote:
> 
> > > Bruno, can you confirm that the hang occurs during one of those 
> > > cancel_work_sync() calls?
> > 
> > No, it's not one of the cancel_work_sync() that hangs but it's the
> > del_timer_sync() right before them that hangs!
> > (del_timer_sync() also hangs if I put it last, so the cancel_work_sync()
> > don't hang anything)
> > 
> > static void hid_cancel_delayed_stuff(struct usbhid_device *usbhid)
> > {
> > 	del_timer_sync(&usbhid->io_retry); /* this one never returns */
> > 	cancel_work_sync(&usbhid->restart_work);
> > 	cancel_work_sync(&usbhid->reset_work);
> > }
> 
> Okay, I see what the problem is.  In usbhid_start() there's a bunch of
> statements initializing parts of the usbhid structure.  When probing
> fails those statements don't get executed, so the timer and workqueue
> things aren't set up properly.
> 
> This patch should fix it.

This very much reminds me the resume issue with the same keyboard on a
!CONFIG_SMP system back in February when the fix was to copy/move 
usbhid->intf = intf; from usbhid_start() to usbhid_probe()!

Hopefully these are all those initializations that need to be taken
care of...


With this patch system now it passes "devices" level pm_test as well as
full suspend process, even multiple times in a row (though it's still
damn slow to resume IF no_console_suspend is passed to kernel - radeon
KMS?, but that's a new branch from start of this thread)

Reported-by: Bruno Prémont <bonbons@linux-vserver.org>
Tested-By: Bruno Prémont <bonbons@linux-vserver.org>


Now it can continue hunting the next issues preventing smooth S3
experience...

Thanks,
Bruno

> Index: usb-2.6/drivers/hid/usbhid/hid-core.c
> ===================================================================
> --- usb-2.6.orig/drivers/hid/usbhid/hid-core.c
> +++ usb-2.6/drivers/hid/usbhid/hid-core.c
> @@ -998,13 +998,6 @@ static int usbhid_start(struct hid_devic
>  		}
>  	}
>  
> -	init_waitqueue_head(&usbhid->wait);
> -	INIT_WORK(&usbhid->reset_work, hid_reset);
> -	INIT_WORK(&usbhid->restart_work, __usbhid_restart_queues);
> -	setup_timer(&usbhid->io_retry, hid_retry_timeout, (unsigned long) hid);
> -
> -	spin_lock_init(&usbhid->lock);
> -
>  	usbhid->urbctrl = usb_alloc_urb(0, GFP_KERNEL);
>  	if (!usbhid->urbctrl) {
>  		ret = -ENOMEM;
> @@ -1180,6 +1173,12 @@ static int usbhid_probe(struct usb_inter
>  	usbhid->intf = intf;
>  	usbhid->ifnum = interface->desc.bInterfaceNumber;
>  
> +	init_waitqueue_head(&usbhid->wait);
> +	INIT_WORK(&usbhid->reset_work, hid_reset);
> +	INIT_WORK(&usbhid->restart_work, __usbhid_restart_queues);
> +	setup_timer(&usbhid->io_retry, hid_retry_timeout, (unsigned long) hid);
> +	spin_lock_init(&usbhid->lock);
> +
>  	ret = hid_add_device(hid);
>  	if (ret) {
>  		if (ret != -ENODEV)
>