Hello.
Sergei Shtylyov wrote:
If EP is busy, we have even less reason to do a giveback. This is
not a real fix, just pallialtive. Actually, whether a STALL token has
been sent or not shouldn't play any role in giving back URB, so the
fragment being pacthed is just totally wrong. Other drivers just don't
allow EP that is still active to be halted, and this driver should do
the same.
Your comment is plain wrong.
No, you code is.
Even your proof case contradicts your own code.
I think there are scenarios that would require requests to be given
back during stall handling.
I am laying one out below for
1. USB Device is in the process of transferring data to/from Host.
2. USB Device queues another request to send.
3. For some reason say Host/Device needs to STALL the endpoint
with the in-flight IO.
This can't be done reliably with requests in flight. That's why the
other drivers don't allow halting EP with requests in flight.
4. On completion of the STALL (on SENTSTALL interrupt) driver
would need to abort the current transfer and return the USB request
with appropriate status. Here the EP state is busy handling the
in-flight IO and the request that is active has to be returned/givenback.
This just cannot work reliably. There should be connection between
actually sending STALL tokens and URB completion because this cannot be
handled reliably.
5. Once the CLEAR Stall is completed the Device would need to
process the next request that is queued if one exists and continue
further normally.
The requests *can* be queued while the EP is halted, and that's what
the other drivers do. You just can't stall EP until the request queue
drains.
Or at least until the current request's I/O is complete (this doesn't mean
that it's given back as the driver employs early giveback).
In the above scenario as you can see we need to abort the in-flight
request (musb_ep->busy) on SENTSTALL interrupt and process the queued
request in the context of clear stall. Consider SENTSTALL interrupt
as a completion interrupt for the current in-flight request that needs
to be completed with error.
I'm not seeing why we would need to abort an in-flight request.
Moreover, I'm not seeing a conection between the EP busy state and an
URB giveback. What your patch does is just avoiding a race between the
EP halt and queueing URB afterwards -- with the gadget not intending to
abort that URB but which gets aborted. This for example happens when the
file-backed storage gadget sends short data in response to the MODE
SENSE command and the necessairily halts the endpoint, then queues the
CSW but the MUSB gadget driver would prematurely giveback that URB afetr
having sent STALL. This results in host's failure to get any CSW and USB
reset finally ensues...
And now please tell me how can the driver differ the URB that you
want to abort from one that you must not abort?
And also tell me how you fix helps the driver to abort in-flight requests
when you specifically disallow it to do so.
WBR, Sergei
_______________________________________________
Davinci-linux-open-source mailing list
[email protected]
http://linux.davincidsp.com/mailman/listinfo/davinci-linux-open-source
