On Mon, Feb 03, 2014 at 06:37:13PM +0100, David Herrmann wrote:
> Hi
>
> On Fri, Jan 31, 2014 at 2:03 PM, Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > From: "Pierre-Loup A. Griffais" <pgriffais@xxxxxxxxxxxxxxxxx>
> >
> > Handle the "a new device is present" message properly by dynamically
> > creating the input device at this point in time. This requires a
> > workqueue as we are in interrupt context when we learn about this.
> >
> > Also properly disconnect any devices that we are told are removed.
> >
> > Signed-off-by: "Pierre-Loup A. Griffais" <pgriffais@xxxxxxxxxxxxxxxxx>
> > Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> > ---
> > drivers/input/joystick/xpad.c | 50 ++++++++++++++++++++++++++++++++++---------
> > 1 file changed, 40 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/input/joystick/xpad.c b/drivers/input/joystick/xpad.c
> > index 7a07b95790d7..d342d41a7a0d 100644
> > --- a/drivers/input/joystick/xpad.c
> > +++ b/drivers/input/joystick/xpad.c
> > @@ -293,8 +293,11 @@ struct usb_xpad {
> > int xtype; /* type of xbox device */
> > int joydev_id; /* the minor of the device */
> > const char *name; /* name of the device */
> > + struct work_struct work; /* to init/remove device from callback */
> > };
> >
> > +static int xpad_init_input(struct usb_xpad *xpad);
> > +
> > /*
> > * xpad_process_packet
> > *
> > @@ -437,6 +440,22 @@ static void xpad360_process_packet(struct usb_xpad *xpad,
> > input_sync(dev);
> > }
> >
> > +static void presence_work_function(struct work_struct *work)
> > +{
> > + struct usb_xpad *xpad = container_of(work, struct usb_xpad, work);
> > + int error;
> > +
> > + if (xpad->pad_present) {
> > + error = xpad_init_input(xpad);
> > + if (error) {
> > + /* complain only, not much else we can do here */
> > + dev_err(&xpad->dev->dev, "unable to init device\n");
> > + }
> > + } else {
> > + input_unregister_device(xpad->dev);
> > + }
> > +}
> > +
> > /*
> > * xpad360w_process_packet
> > *
> > @@ -451,16 +470,22 @@ static void xpad360_process_packet(struct usb_xpad *xpad,
> > * 01.1 - Pad state (Bytes 4+) valid
> > *
> > */
> > -
> > static void xpad360w_process_packet(struct usb_xpad *xpad, u16 cmd, unsigned char *data)
> > {
> > /* Presence change */
> > if (data[0] & 0x08) {
> > if (data[1] & 0x80) {
> > - xpad->pad_present = 1;
> > - usb_submit_urb(xpad->bulk_out, GFP_ATOMIC);
> > - } else
> > - xpad->pad_present = 0;
> > + if (!xpad->pad_present) {
> > + xpad->pad_present = 1;
> > + usb_submit_urb(xpad->bulk_out, GFP_ATOMIC);
> > + schedule_work(&xpad->work);
> > + }
> > + } else {
> > + if (xpad->pad_present) {
> > + xpad->pad_present = 0;
> > + schedule_work(&xpad->work);
> > + }
> > + }
> > }
> >
> > /* Valid pad data */
> > @@ -507,10 +532,13 @@ static void xpad_irq_in(struct urb *urb)
> > }
> >
> > exit:
> > - retval = usb_submit_urb(urb, GFP_ATOMIC);
> > - if (retval)
> > - dev_err(dev, "%s - usb_submit_urb failed with result %d\n",
> > - __func__, retval);
> > + if (xpad->pad_present) {
> > + retval = usb_submit_urb(urb, GFP_ATOMIC);
> > + if (retval)
> > + dev_err(dev,
> > + "%s - usb_submit_urb failed with result %d\n",
> > + __func__, retval);
> > + }
> > }
> >
> > static void xpad_bulk_out(struct urb *urb)
> > @@ -991,6 +1019,7 @@ static int xpad_probe(struct usb_interface *intf, const struct usb_device_id *id
> > xpad->mapping = xpad_device[i].mapping;
> > xpad->xtype = xpad_device[i].xtype;
> > xpad->name = xpad_device[i].name;
> > + INIT_WORK(&xpad->work, presence_work_function);
> >
> > if (xpad->xtype == XTYPE_UNKNOWN) {
> > if (intf->cur_altsetting->desc.bInterfaceClass == USB_CLASS_VENDOR_SPEC) {
> > @@ -1136,7 +1165,8 @@ static void xpad_disconnect(struct usb_interface *intf)
> > struct usb_xpad *xpad = usb_get_intfdata (intf);
> >
> > xpad_led_disconnect(xpad);
> > - input_unregister_device(xpad->dev);
>
> You need cancel_work_sync(&xpad->work) here.
> And I think you need to do that *after* killing the urbs. Otherwise,
> the work might get rescheduled in parallel to this disconnect
> callback.
Ah, you are right, that should fix a USB core warning of submitting an
urb when the device has been removed that I saw a few times in testing.
Thanks for pointing it out.
greg k-h
--
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
