On 02/06/2012 09:14 PM, Alan Stern wrote:
You should not continue processing setup packets after you received a
DELAYED_STATUS return code.
No, that's wrong. The gadget _must_ process SETUP packets, always.
But if break the HW or don't ACK the setup packet or just disable the
Well, obviously broken hardware can do anything, but that doesn't make
it right. And the hardware _must_ ACK every valid SETUP packet (see
8.5.3 in the USB-2.0 spec); it is not allowed to reply with NAK.
I'm not saying to reply with NAK. I just say to delay the ACK after we
done processing it. That means:
- fetch the setup packet
- queue processing of the setup data into a workqueue
- until workqueue is done processing there is no ACK or NAK
- once the workqueue is done, the UDC answers with ACK/NAK
- next setup packet can be fetched.
IRQs you never receive a second one packet untill the first SETUP
packet is done. The gadget will process the packet but at a later point
in time. After all you could hold a spinlock with irqs for a long time
and notice the second setup packet before the gadget was done with the
first one.
Doesn't matter. If the hardware is designed properly, it will flush
the ep0 FIFO whenever a new SETUP packet arrives. The UDC driver must
be written so that it doesn't queue a response to an old SETUP packet.
So we receive new setup packet before acking the old one?
What should happen is that the response to the first SETUP (the one
that was delayed) should not be sent after another SETUP is received.
If the workqueue routine hasn't started running yet, cancelling it
would be a good idea.
That isn't actually that easy. Canceling means waiting until it is
done.
Not necessarily. If a workqueue routine hasn't started yet, you can
cancel it right away. But if it has already started then you're right;
it can't be cancelled. Nevertheless, the UDC driver must not accept
transfer requests for ep0 from an old setup() callback after a new
SETUP packet has arrived.
What is wrong processing one setup packet after the other?
Suppose the host sends a Set-Interface request, and you delay handling
it. If the delay is too long, the host will time out and cancel the
request. Then the host sends some completely different request, such
as Set-Feature. If your delayed response goes back now, the host will
think you accepted the Set-Feature request and not realize that you
really carried out the Set-Interface. In addition, the response to the
Set-Feature request will remain sitting in the ep0 FIFO and never get
sent, because the host won't ask for it.
Now I see where you are going with this. So this can happen right now
with threaded_irq, a busy high priority task (higher than the irq) and
we don't need one of the delayed_status user for this. You ask for a
descriptor which may get delayed long enough and by the time you answer
it you may have another setup packet received.
There is the delayed_setup counter which could be used for all
transfers. Once ->setup() returns (winthin the worked) it decrements
the counter and once it reaches 0 you will send the request. Otherwise
discard it. This will make the race window smaller but it will remain.
You *may* receive another SETUP packet right after you looked at the
counter. A better way would be for USB to have an incrementing sequence
counter so if you answer too late it will see that this is an old
answer and ignore it. But it is too late for this.
Receiving another SETUP packet while handling the current one isn't
possible at least on dwc3: Lets say SET_INTERFACE will be delayed. The
controller probably acks it immediately. The only thing the driver does
is to setup the next transfer to receive another SETUP packet. Right
now we delay this step until gadget is done with his SET_INTERFACE. If
we don't, we have another setup packet waiting for us (that one after
SET_INTERFACE). I don't have an USB sniffer to say that this is what
really happens, but it looks like it from the driver perspective.
Alan Stern
Sebastian
--
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
