Subbrathnam, Swaminathan wrote:
-----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.
Hm, I've failed to reproduce it with the Linux hosts I have at hand so far
(those having the outdated kernels), though I'm sure it used to be
reproducible with the recent Linux kernels too...
(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.)
I doubt that it ever worked correctly and that the reason was just a
bitrot. This driver just does different thing from the other drivers WRT the
stall handling: ISO failing set_halt() calls until the queue drains, it allows
to halt the endpoint with pending requests and then gives back the requests
based on the STALL handshakes actually being sent. But it does not differ
between the requests queued before and after the endpoint halt which leads to
the requests containing storage class' CSWs being given back without sending
if a STALL handshake happens despite they should've been sent *after* clearing
the halt state...
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.
Thanks to him indeed.
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).
And how do you imagine that? Some requests are auto-returned in response
to stall status and some are not? And those that are returned -- based on what
criterion: sending a STALL handshake still?
I now have a patch that should deal with the issue #1 though it doesn't
try to address issue #2 and I'm wondering whether it's worth submittig it...
- Dave
WBR, Sergei
--
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
