[PATCH] iso pipe support for usbatm

[Roman Kagan]

On Sat, Mar 19, 2005 at 12:38:19AM +0100, matthieu castet wrote:
> @@ -472,17 +510,44 @@
>  	instance = rcv->instance;
>  	buf = rcv->buffer;
>  
> -	buf->filled_cells = urb->actual_length / (ATM_CELL_SIZE + instance->rx_padding);
> +	if ( urb->actual_length <= 0 && !urb->status) {
> +		int err;
> +		if ((err = usb_submit_urb(rcv->urb, GFP_ATOMIC)) < 0) {

Why do you resubmit the failed urb here?  It used to go in the normal
codepath and get resubmitted in receive_tasklet.

> +			dbg("usbatm_complete_receive: urb submission failed (%d)!", err);
> +			list_add(&buf->list, &instance->spare_receive_buffers);
> +			spin_lock_irq(&instance->receive_lock);
> +			list_add(&rcv->list, &instance->spare_receivers);
> +			spin_unlock_irq(&instance->receive_lock);

->spare_receive_buffers used to be manipulated in receive_tasklet only,
and thus the access was serialized.  If you touch it here it'll need to
be protected by the ->receive_lock too (both here and in
usbatm_process_receive).

Besides, the completion handler may be run in interrupt context so you
have to use spin_lock_irqsave() and spin_unlock_irqrestore().

> -	if (likely(!urb->status))
> +	if (likely(!urb->status) ||
> +			/* via chipset produce lot's of crc error...*/
> +			(usb_pipeisoc(instance->rx_endpoint) && urb->status == -EILSEQ))
>  		tasklet_schedule(&instance->receive_tasklet);

Sounds like iso occasionally misses data too?  BTW the condition you add
here is ignored because it's masked by the other one.

Anyway I think this test should be removed here (for bulk too),
otherwise how the buffers and urbs are going to be recycled?  The urb
submission errors are problematic too (I've marked that in my patch),
but urb return status should only be used to make sure the buffer passed
on to the processing routine is empty.  For bulk relying on
urb->actual_length is enough.

> @@ -1064,9 +1151,18 @@
>  	instance->usb_dev = dev;
>  	instance->usb_intf = intf;
>  	instance->tx_endpoint = usb_sndbulkpipe(dev, driver->out);
> -	instance->rx_endpoint = usb_rcvbulkpipe(dev, driver->in);
> +	if (driver->iso_frame_size) {
> +		nb_frames = rcv_buf_per_urb;
> +		urb_size = driver->iso_frame_size * rcv_buf_per_urb;

What is the typical ->iso_frame_size you're using? 

Roman.