Re: Active URB submitted twice in pegasus driver

[Sarah Sharp <sarah.a.sharp@linux.intel.com>]

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