Re: [PATCH]suspend/resume for cdc_acm

On Wed, 10 Oct 2007, Oliver Neukum wrote:

> Hi,
> 
> this patch implements suspend() and resume() methods for CDC ACM devices.
> 
> 	Regards
> 		Oliver

> +static void stop_data_traffic(struct acm *acm)
> +{
> +	int i;
> +
> +	tasklet_disable(&acm->urb_task);
> +
> +	usb_kill_urb(acm->ctrlurb);
> +	usb_kill_urb(acm->writeurb);
> +	for (i = 0; i < acm->rx_buflimit; i++)
> +		usb_kill_urb(acm->ru[i].urb);
> +
> +	INIT_LIST_HEAD(&acm->filled_read_bufs);
> +	INIT_LIST_HEAD(&acm->spare_read_bufs);
> +
> +	tasklet_enable(&acm->urb_task);
> +
> +	flush_scheduled_work(); /* wait for acm_softint */

flush_scheduled_work() should be avoided wherever possible.  I realize 
you just moved the existing code.  Nevertheless, that routine is prone 
to deadlocks.  You'd be better off cancelling any outstanding workqueue 
items or calling the work routines directly.

> +static int acm_suspend(struct usb_interface *intf, pm_message_t message)
> +{
> +	struct acm *acm = usb_get_intfdata(intf);
> +
> +	if (acm->susp_count++)
> +		return 0;
> +	/*
> +	we treat opened interface differently,
> +	we must giard against open

Typo.

> +	*/
> +	mutex_lock(&open_mutex);
> +
> +	if (acm->used)
> +		stop_data_traffic(acm);
> +
> +	mutex_unlock(&open_mutex);

You have to be very careful about locking order in the suspend and
resume methods.  I'm not familiar enough with the rest of the driver to
say whether this is okay or not, but the possibility exists for locking
loops involving the device's semaphore, the device's PM mutex, and the
private open_mutex.  You may be okay for now, but then adding
autosuspend support is quite likely to cause a loop.

Alan Stern


-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >> http://get.splunk.com/
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel