On Wed, 28 Mar 2007, David Brownell wrote:

> > Here's the exact text from the spec:
> > 
> >     If the endpoint instead responds to the OUT/DATA transaction with
> >     a NYET handshake, this means that the endpoint accepted the data
> >     but does not have room for another wMaxPacketSize data payload.
> >     The host controller must return to using a PING token until the
> >     endpoint indicates it has space.
> > 
> > This clearly indicates, even though it doesn't say so explicitly,
> 
> Disagree.  It says explicitly that it "must" return to PING.  It does
> not create an exception like the one you ascribe to that text.

Exactly -- "must return to using a PING token".  I would expect that to 
have a single unambiguous interpretation.  Evidently you do too... but our 
interpretations are different!

> (Note by the way that "wMaxPacketSize" is superfluous.  If it's ready
> for any data at all, it's ready for that much.)

(Not true.  Suppose the FIFO is 128 bytes long and the host has just sent
a 64-byte data packet followed by a 36-byte packet.  Upon receipt of the
second packet the FIFO is ready to hold another 28 bytes.  But since ep0's
wMaxPacketSize is 64, the device must reply with NYET instead of ACK.)

> And implementors are certainly allowed to use the same FIFO for IN transfers,
> which could have the same "fifo must be ready" constraint as OUT ones do.

Of course.  They are also perfectly free to reply with NAK to any IN
tokens until the FIFO is ready.  There's no need and no requirement for
the host to use PING.


> > The  
> > entire discussion of the PING protocol is centered around the idea of 
> > improved handling of OUT transfers; PINGs aren't needed before IN or SETUP
> > transactions at all.
> 
> No, but PINGs *are* needed to finish processing an OUT transfer.
> Which is what the text says...

It does not say that anywhere.  Just the contrary; see the discussion of 
Fig. 8-27 below.


> > After all, what reason could there be for sending a PING before an 
> > IN/DATA?  PING asks whether the endpoint has space to store a new 
> > wMaxPacketSize data payload, but with IN transfers the endpoint doesn't 
> > have to store anything.
> 
> What reason?  Well, as noted above, the spec is written so hosts "must"
> send the PING.  Accordingly, peripherals are allowed to rely on that.

No -- it says that hosts "must return to using a PING token until the
endpoint indicates it has space".  "Return to using a PING token" != "send 
the PING".

> For example, trusting that the FIFO empties before control IN/STATUS
> packets start to arrive.

The peripheral is not allowed to make that assumption, and it has no
reason to.  If it can't accept an IN/STATUS packet because the FIFO is
still full, it can respond to the IN with NAK.


> > > And ... the OUT/DATA stage can't have completed if a PING was due.
> > > As soon as the OUT/DATA stage completes, then the IN transactions
> > > may do their usual thing, and that different paragraph applies.
> > 
> > No.  If a PING is due, it means that the data already sent was received 
> > correctly and hence the data stage _is_ complete.  You can see it in the 
> > section I quoted above: "... this means that the endpoint accepted the 
> > data...".
> 
> You're using a different definition of complete than I am.  I'm using
> the same definition that a normal bulk OUT would use:  ready to accept
> one more OUT packet.

Again you are misinterpreting the spec.  As far as the host is concerned,
a bulk OUT transaction is complete when either an ACK or a NYET is
received from the device.  See Fig. 8-27 and below.

>  (ISTR that peripherals are expected to handle any
> number of zero length OUT packets at the same point they're expected
> to start handling the status stage, for example ...)

(I don't think that's right; can you find it in the spec?  Certainly the
net2280 driver doesn't behave that way.)

> That is:  NYET/PING is a handshake used to mark completion of an exchange.
> Treating the exchange as complete without the full handshake is at best
> error prone ...

No.  ACK is a handshake token used to mark completion of an exchange
together with an indication that the endpoint is ready to accept another
OUT data packet.  NYET is a handshake token used to mark completion of an
exchange together with an indication that the endpoint is _not_ ready to
accept another packet.  By contrast, PING isn't a handshake at all; it is
a poll used to see whether the endpoint is ready to accept another OUT
data packet.

> > I agree about the meaning of "must"; the question is: must do what? 
> 
> The text seems clear to me:  must PING until the endpoint has space.

It seems clear to me:  must PING before sending another OUT.


> > I claim the intention of the spec is that the host must send a PING before
> > starting another OUT/DATA transaction. 
> 
> Thing is, you're inferring an "intention" that's not written, while
> ignoring the text that *IS* written.

I'm _not_ ignoring it.  So there!  :-)

Besides, the figures are described as more explicit and detailed than the
written text in the spec (see p.7 of the Errata document).  We should base
our argument on what the figures say instead of the misinterpretable text.

> Given a spec, standard practice is to stick to what's written, unless
> it's ambiguous.  (Here, it isn't.)  If there's ambiguity, then it's
> fair to use "intention" to make an educated guess ... but then also
> update the spec to remove the ambiguity, once everyone agrees on how
> that should be resolved.

I certainly agree that updating the spec to make it less ambiguous would 
help.

> > You seem to think it means the host must send a PING 
> 
> Period, no matter what, since that's clearly what the spec says.

It says "return to using a PING token".  As I mentioned above, that is 
different from "send a PING".

> > Well, the patch is a separate question.  Thinking about it some more, I
> > doubt it will actually fix the problem.  In fact, my best guess is that
> > for some reason the net2280 doesn't set the DATA_IN_TOKEN_INTERRUPT flag
> > when the IN token arrives.  Maybe some sort of low-level hardware timing 
> > issue.
> 
> Like ... maybe the NetChip folk took the USB spec at its word?  :)

At _your_ word, you mean.  Perhaps.  Or maybe it's just a hardware bug.


Vikram, it would be nice to have more information about what's really 
going on when the failure occurs.  In an earlier email you included this 
change to get the driver working:

> @@ -2204,12 +2206,16 @@ static void handle_ep_small (struct net2
>                                       && req
>                                       && req->req.actual == req->req.length)
>                                       || !req) {
> +printk("\n-->STALL\n");

Please try printing some additional information:

+printk("\n-->STALL: t %x, req %p\n", t, req);
+if (req) printk("actual %d, length %d\n", req->req.actual, req->req.length);

This may provide some extra clues.


> Or alternatively, some of the recent net2280 patches broke some of
> the more subtle race-avoidance logic in that driver.  As I mentioned,
> I saw something rather fishy go flying by at one point; and that
> particular conditional does look fishy lately.

Could be, but I don't see how.  Maybe the additional debugging info will 
tell us.



On Thu, 29 Mar 2007, Pandita, Vikram wrote:

>  The discussion on PING should be based on the Spec Errata -
> errata-092800-1207001.pdf, as the figure 8-27 is wrong in the 2.0 sepc:
> Section Incorrect Figure 8-27 (Host High-speed Bulk OUT/Control Ping   
> State Machine): Page 8.

Just so.

> From this figure, the PING is related to:
> OUT, SETUP (Do_token1 --> Do_data1)
> 
> The confusion is:
> if there is NYET response from a device to a PING, then should the HOST
> interpret that as :
> a) next OUT cannot be sent yet or
> b) the device is not in a ready state and so keep Pinging till ACK and
> then start next transaction
> 
> From the corrected figure 8-27 it is clear that SETUP can still be sent
> irrespective of the PING state of the device. So (a) could be the right 
> interpretation.

Or more to the point, IN can be sent irrespective of the PING state.

In the corrected figure, the questionable state box is labelled "HSU2.PID
= NYET" and the action is "RespondHC(Do_next_ping);".  Unfortunately
Do_next_ping isn't defined anywhere.  Apparently it is meant to be much 
like Do_next_cmd (the action when the device responds with ACK), except 
that HC_cmd.ping is set before proceeding to the next transaction.

Thus, if the next transaction is another OUT/DATA, a PING will be sent
first.  In any case, the "Do_next" part of the name, together with the
fact that the control flow then exits the diagram, indicates the
current transaction _has_ indeed been completed.

But now the next transaction is IN/STATUS, so we have to behave according 
to Fig. 8-33 (Bulk/Control/Interrupt IN Transaction Host State Machine).  
That figure makes no mention of PING packets.  Hence no PING needs to be 
sent.


Another indication that the people who designed the protocol did not
expect these extra PINGs can be found in the design of the EHCI
controller.  The text in section 4.11 of the EHCI spec says that the Ping
state bit is managed only in queue heads whose PIDCode field equals OUT.  
So it doesn't apply during the IN/STATUS part of a control transfer.

See especially footnote 3 in Table 4-12.  When the host sends an OUT 
packet and the device responds with NYET, the controller advances the 
transfer state in addition to changing the Ping State bit.  Since the next 
transfer state requires an IN transaction, not an OUT, the Ping State bit 
will be ignored (until the next OUT transaction occurs).

Alan Stern


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

Reply via email to