On Tue, Mar 26, 2013 at 07:28:53PM +0200, Petko Manolov wrote: > On Tue, 26 Mar 2013, Sarah Sharp wrote: > > >On Tue, Mar 26, 2013 at 05:22:07PM +0200, Petko Manolov wrote: > >>On Mon, 25 Mar 2013, Sarah Sharp wrote: > >After taking a brief glance at the pegasus code, > >pegasus_set_multicast looks broken. It sets the control URB > >status to zero and calls ctrl_callback(), which does some stuff if > >the URB status is zero. It doesn't look like > >pegasus_set_multicast() is an URB callback function, so why in the > >world is it touching a control URB that could possibly be in > >flight else where? > > Beats me, i've got nothing to do with this driver. :-P > > Anyway, playing with the URB status looks rather stupid and is > definitely a bug. I'll make a patch that fixes the problem. > > >It looks like the control URB is used to do things like get or set > >registers, so what's stopping the upper layers from calling get > >registers (which will submit the control URB and schedule a wait > >queue), and then pegasus_set_multicast(), which will go overwrite > >the active URB status? USB device drivers should not be writing > >or reading fields in a submitted URB until the completion > >handler's callback function is called. > > Err, see my previous comment. It can't be me. Maybe i've been too > drunk. Or stupid. Or both... :-) Nah, everybody makes silly coding mistakes. I was just trying to clarify, since I don't know the driver. Send me a patch and I'll see if it fixes the warning. But considering the multicast code is pretty old, I bet I'm running into a different bug... Sarah Sharp -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
