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
