RE: [PATCH 3/3] MUSB : Fix for STALL handling in musb gadget code

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

 



Dave,
	My response inlined.

Regards
swami

> -----Original Message-----
> From: David Brownell [mailto:david-b@xxxxxxxxxxx]
> Sent: Saturday, February 21, 2009 12:30 PM
> To: Subbrathnam, Swaminathan
> Cc: Alan Stern; Sergei Shtylyov; Gupta, Ajay Kumar;
> felipe.balbi@xxxxxxxxx; linux-usb@xxxxxxxxxxxxxxx; davinci-linux-open-
> source@xxxxxxxxxxxxxxxxxxxx
> Subject: Re: [PATCH 3/3] MUSB : Fix for STALL handling in musb gadget code
> 
> Swami,
> 
> I'm scanning mail queues for MUSB patches that may need
> merging.  I think this thread didn't have such a patch,
> but it did raise some issues that don't yet seem to have
> been resolved.
> 
> That said, stall handling has always been a PITA -- only
> used by USB mass storage class, and conformance tests,
> which have somewhat conflicting requirements -- so this
> particular flamefest was no real surprise.
> 
> 
> On Sunday 07 December 2008, Subbrathnam, Swaminathan wrote:
> > While working on DaVinci and other platforms that are
> > utilizing the Mentor controller and the Linux MUSB stack
> > we faced the following issues.
> >
> > 1. In-correct cancellation of CSW request in the context of
> > the associated EP being stalled.  This is leading to the USB
> > device being reset.  This happens in the context of MSC gadget.
> 
> Is this still an issue in the current GIT kernel?


<Swami> Yes it still remains an issue today.  In reality this issue impacts basic enumeration with Windows (XP) PC host.  There are instances were STALL handling come into play when we enumerate with a windows PC (typically in MSC case) USB Host.  We can see multiple resets occurring as the stall handling is not done correctly.  This situation recovers some times and fails most of the times.

> 
> (I know that this once worked correctly, FWIW, and I'm not
> sure where the bug would have come from.  Bitrot or the like,
> presumably.)
> 
> 
> > 2. When WHQL tests are run against a RNDIS-CDC gadget the Halt
> > Endpoint tests fail on a BULK IN EP. This is due to the fact
> > that that BULK-In EP had in-flight IO's and the original gadget
> > driver was returning failure status to the Halt Endpoint request
> > coming from the host (FIFO being full).
> 
> It would be nice if those were USB-IF tests not Microsoft
> ones, so they were more accessible.  And so the notion of
> Microsoft creating their own variant of the USB standards
> weren't so up-front-and-obnoxious.  :(
> 
> Is this also still an issue?   This sounds not-quite-like
> the standard USBCV test.  Which I think I need to run again.
> 

<Swami> This issue can be seen by running the USB CV test suite apart from Windows HCT (originally) and not Windows DTM.  You would see failure in "Halt Endpoint" test.

> 
> Alan's comments on this seemed, unsurprisingly, to be the
> most on-the mark.

<Swami> I did respond to Alan on his recommendation.  It was a very well balanced view and thanks to him for providing that.  My only input is that during stall handling it is better to let the application decide which requests to de-queue and which to stay en-queued/re-queued with the underlying controller driver.  As the stall handling is typically application driven mechanism.

The underlying controller driver can indicate that a particular request is getting returned due to stall status on an EP.  In response to this the application (depending on its protocol) can either decide to purge all queued requests or selectively purge.  Underlying controller driver just executes application's requests accordingly instead of taking the decision for the application (by purging all the requests).

> - Dave

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