Re: [PATCH 1/3] usb/dummy_hcd: complete stream support

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

 



On Fri, 13 Jan 2012, Sebastian Andrzej Siewior wrote:

> >Why not pass dum_hcd as the argument instead of passing hcd and
> >converting it again?
> 
> I have convert it anyway: Either to dummy_hcd for normal using or to
> usb_hcd for the dev_err(). So now I pass it directly and use
> dummy_dev(dum_hcd) for the dev_err().

Good.  That's better because dev_err() is not likely to be called.

> >You didn't modify dummy_timer().  That routine won't allow transfers
> >for than one URB per endpoint (unless an URB completes).  In
> >particular, it won't allow concurrent transfers on different streams
> >for the same endpoint.  Is that important for your work?
> 
> Hmm. So if one URB spans over two usb_requests I can't continue the
> transfer until the URB completes.

That doesn't sound quite right.  If you have two URBs queued for the 
same endpoint, the code won't do any transfers for the second URB until 
the first URB completes.  This behavior is correct for endpoints 
without streams, but it's not so good if the two URBs are for different 
streams on the same endpoint.

>  The way streams are working in UAS it
> does not matter because one stream does not depend on the other so it
> won't stall the other streams forever.
> 
> Do you suggest removing '->already_seen'?

No, we should keep it for non-streams endpoints.

One possible approach is to make already_seen into a vector of 
bitflags, one for each stream.  Since the maximum number of streams is 
currently limited to 16, it would easily fit into an unsigned long.

> >> @@ -2290,11 +2372,45 @@ static int dummy_alloc_streams(struct usb_hcd *hcd, struct usb_device *udev,
> >>  	struct usb_host_endpoint **eps, unsigned int num_eps,
> >>  	unsigned int num_streams, gfp_t mem_flags)
> >>  {
> >> -	if (hcd->speed != HCD_USB3)
> >> -		dev_dbg(dummy_dev(hcd_to_dummy_hcd(hcd)),
> >> -			"%s() - ERROR! Not supported for USB2.0 roothub\n",
> >> -			__func__);
> >> -	return 0;
> >
> >Don't you want to retain this behavior?
> Why should do something that has been done in usb_alloc_streams()?

Ah, I had forgotten about that.  Sorry.

Alan Stern

--
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


[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux