On Wed, 28 Mar 2007, David Brownell wrote:
> On Wednesday 28 March 2007 2:29 am, Pandita, Vikram wrote:
> >
> > > (**) 8.5.1 in the USB 2.0 spec says that after a NYET handshake,
> > > "the host controller must return to using a PING token until the
> > > endpoint indicates it has space".
> > >
> > > Clearly, the HDRC did not issue any PINGs ... it just went right
> > > to the status handshake. Fishy, and quite possibly something
> >
> > The PING flow-control is only for the OUT tokens.
> > So Host should send PING only if next OUT token is to be sent.
>
> That "must" language is unambiguous: PING "must" be sent.
> There is no "should" in that paragraph of the spec.
>
> Read it this way: the host must send PING until it reports
> that the next data token may be sent to that endpoint. The
> fact that it will be a zero length packet is irrelevant, as
> is the fact that this is a control endpoint (the data stage
> works like normal BULK data, where that can't happen) so it
> will be an IN token; in either case, the FIFO may still not
> be ready.
That's not right. Here's what the spec says (section 8.5.1):
High-speed devices must support an improved NAK mechanism for Bulk
OUT and Control endpoints and transactions. Control endpoints
must support this protocol for an OUT transaction in the data and
status stages.
The PING protocol is not supported for IN transactions in control
transfers -- i.e., not for the status stage of an OUT transfer. You could
probably deduce this from Figure 8-27, but I have never been able to
figure out those baroque state transition diagrams.
> > The IN token has no restriction and the device should NAK the IN token
> > till it can process the next IN token.
> > So the HDRC flow looks ok. Do you agree?
>
> Clearly not. This is another case where Mentor hasn't paid
> very close attention to the relevant specs. Although in
> this case the workaround should be simple ... it's not like
> the bizarre way their OTG state machine works!
I disagree; the trace shows that the MUSBHDRC behaved correctly. The
question is why did the net2280 send back the STALL?
> > > the net2280 interpreted as "wrong" (ergo the STALL). I'd guess
> > > the RTL from Mentor is triggering the "tx data was ACKed" IRQ
> > > too early ... treating the NYET as an ACK.
NYET _is_ supposed to be treated much like an ACK when it is received in
response to an OUT data packet. That is, it indicates that the data was
received correctly, but the endpoint is not ready to accept another OUT
transaction.
> > > An experiment you could try: before starting the status stage
> > > of an ep0 OUT transfer, stick a udelay(50) or somesuch. Then
> > > compare what the HDRC is doing. With luck, the net2280 only
> > > cares about that illegal state in a small window, and the udelay
> > > will get past it...
> > >
> > > - Dave
> >
> > When I enable all the logging in the code on Host side (which is lot of
> > unnecessary logs) I see that the test case passes.
>
> Then I consider the experiment made, the hypothesis proven. What
> remains is turning it into a workaround on the HDRC side.
The workaround belongs on the net2280 side, since that's where the error
occurs. It could be that there is a mistake in the parenthesization of
one tricky statement. (I blush to admit that I was the last person to
change that particular line of code.) Vikram, does this untested patch
improve your situation?
Alan Stern
Index: usb-2.6/drivers/usb/gadget/net2280.c
===================================================================
--- usb-2.6.orig/drivers/usb/gadget/net2280.c
+++ usb-2.6/drivers/usb/gadget/net2280.c
@@ -2200,16 +2200,16 @@ static void handle_ep_small (struct net2
}
mode = 2;
/* an extra OUT token is an error */
- } else if (((t & (1 << DATA_OUT_PING_TOKEN_INTERRUPT))
- && req
- && req->req.actual == req->req.length)
- || (ep->responded && !req)) {
- ep->dev->protocol_stall = 1;
- set_halt (ep);
- ep->stopped = 1;
- if (req)
- done (ep, req, -EOVERFLOW);
- req = NULL;
+ } else if (t & (1 << DATA_OUT_PING_TOKEN_INTERRUPT)) {
+ if ((req && req->req.actual == req->req.length)
+ || (!req && ep->responded)) {
+ ep->dev->protocol_stall = 1;
+ set_halt(ep);
+ ep->stopped = 1;
+ if (req)
+ done(ep, req, -EOVERFLOW);
+ req = NULL;
+ }
}
}
}
-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys-and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
