On Tue, Jun 03, 2008 at 10:11:09PM +0200, Oliver Neukum wrote:
> Am Dienstag 03 Juni 2008 20:45:47 schrieb Ville Syrjala:
> > +static int ati_remote2_open(struct input_dev *idev)
> > +{
> > + struct ati_remote2 *ar2 = input_get_drvdata(idev);
> > + int r;
> > +
> > + dev_dbg(&ar2->intf[0]->dev, "%s()\n", __FUNCTION__);
> > +
> > + r = usb_autopm_get_interface(ar2->intf[0]);
> > + if (r) {
> > + dev_err(&ar2->intf[0]->dev,
> > + "%s(): usb_autopm_get_interface() = %d\n", __FUNCTION__, r);
> > + goto fail1;
> > + }
> > + r = usb_autopm_get_interface(ar2->intf[1]);
> > + if (r) {
> > + dev_err(&ar2->intf[1]->dev,
> > + "%s(): usb_autopm_get_interface() = %d\n", __FUNCTION__, r);
> > + goto fail2;
> > + }
>
> If you have two interfaces on one device upping the count for one of them
> is enough.
OK. But now I wonder why do I even bother calling this function. It
doesn't seem to do anything particularly useful.
> > +
> > + mutex_lock(&ati_remote2_mutex);
>
> Too late. You can race with disconnect()
Hmm. Do you mean open() vs. disconnect()? Doesn't the input_dev's locking
take care of that? ati_remote2_mutex is there just to make ar2->flags
handling and urb submitting/killing atomic, it didn't even exist before
this autosuspend patch. Or perhaps I'm missing something...
> And you should set needs_remote_wakeup.
What exactly does that do? As I said remote wakeup isn't enabled on the
device but it still wakes up just fine.
--
Ville Syrjälä
syrjala@xxxxxx
http://www.sci.fi/~syrjala/
--
To unsubscribe from this list: send the line "unsubscribe linux-input" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
