Re: [linux-usb-devel] Fwd: autosuspend for appletouch driver

On Thu, 27 Sep 2007, Oliver Neukum wrote:

> Am Donnerstag 27 September 2007 schrieb Alan Stern:
> > On Thu, 27 Sep 2007, Oliver Neukum wrote:
> > 
> > > @@ -528,12 +537,20 @@ exit:
> > >  static int atp_open(struct input_dev *input)
> > >  {
> > >  	struct atp *dev = input_get_drvdata(input);
> > > +	int rv = 0;
> > >  
> > > -	if (usb_submit_urb(dev->urb, GFP_ATOMIC))
> > > +	if (usb_autopm_get_interface(dev->intf) < 0)
> > >  		return -EIO;
> > > +	dev->intf->needs_remote_wakeup = 1;
> > > +	if (usb_submit_urb(dev->urb, GFP_KERNEL)) {
> > > +		rv = -EIO;
> > > +		dev->intf->needs_remote_wakeup = 0;
> > > +		goto err;
> > >  
> > >  	dev->open = 1;
> > > -	return 0;
> > > +err:
> > > +	usb_autopm_put_interface(dev->intf);
> > > +	return rv;
> > >  }
> > >  
> > >  static void atp_close(struct input_dev *input)
> > > @@ -543,6 +560,7 @@ static void atp_close(struct input_dev *
> > >  	usb_kill_urb(dev->urb);
> > >  	cancel_work_sync(&dev->work);
> > >  	dev->open = 0;
> > > +	dev->intf->needs_remote_wakeup = 0;
> > >  }
> > 
> > Doesn't atp_close() need to call usb_autopm_put_interface(), to balance 
> > the usb_autopm_get_interface() call in atp_open()?
> 
> No, the get is balanced in atp_open() itself.

I see; I misread the patch.

>  The patch uses the last_busy
> mechanism the same way the version for generic hid does. In fact, it is
> only necessary because there's no nice API call for setting
> needs_remote_wakeup.

That's okay; even if there were a nice API call for setting 
needs_remote_wakeup you would still have to call
usb_autopm_get_interface().  This is because we don't keep track of 
whether or not remote wakeup is currently enabled on a suspended 
device; we only keep track of whether it should be enabled when the 
next suspend occurs.

Alan Stern