Re: [PATCH 2/3] usb: musb: fix bug in musb_start_urb

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Friday 06 February 2009, Sergei Shtylyov wrote:
> Ajay Kumar Gupta wrote:
> 
> > urb->transfer_buffer_length and urb->transfer_buffer should be
> > updated based on urb->actual_length.For a fresh and first time urb,
> > actual_length will be zero but for urbs which has been stopped and
> > restarted (as bulk nak scheme does) actual_length may not be zero.
> 
> > Signed-off-by: Ajay Kumar Gupta <ajay.gupta@xxxxxx>
> 
>     NAK, this is not a problem for the current driver since URBs do not ever 
> get restarted.

Resolvable by changing the patch description to not say
this is a "fix".

However, since this is a two line change, I think I'll
just merge this with the patch adding the bulk RX retry
logic.


> Also, musb_host_tx() doesn't update urb->actual_length --  
> please fix it too.

That would be a good point if the retry patch touched
any TX paths.  But it doesn't.


> Also, you must not clear qh->iso_idx when restarting -- you  
> must not start ISO transfer all over again too. Also, you should not set  
> musb->ep0_state to MUSB_EP0_START again in this case (I agree that control 
> transfers will remain not restartable from an arbitatry place even then).

But the [3/3] patch only adds NAK timeout support for
bulk RX.  And ISO transfers can't NAK in the first place.

Plus, as noted in a comment you could see in this patch,
this only touches (re)start for bulk/interrupt transfers.
Not ISO; not control.

- Dave
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux