Hi Thinh,
On 11/22/22 7:00 AM, Thinh Nguyen wrote:
Hi Udipto,
On Fri, Nov 18, 2022, Udipto Goswami wrote:
Hi Thinh
On 11/18/22 7:31 AM, Thinh Nguyen wrote:
On Thu, Nov 17, 2022, Udipto Goswami wrote:
A dequeue for ep0 need to adjust the handling based on the
data stage and status stage. Currently if ep0 is in data/status
stage the handling isn't that different, driver will try giveback
as part of dequeue process which might potentially lead to the
controller accessing invalid trbs.
Also for ep0 the requests aren't moved into the started_list,
which might potentially lead to the un-mapping of the request
buffers without sending endxfer.
Maybe we need to track started_list for control endpoint? If the request
isn't prepared yet or that the transfer had completed, then there's no
need to issue End Tranfer command.
But I believe sending End Transfer for inactive endpoint should be fine
also. Then we maybe able to get away without checking the started list.
If you're planning to do that, please test and note it somewhere.
thanks for the suggestion, sure i'll do some more experiments and confirm
it.
Just curious, how do you hit/test this scenario?
For other endpoint types, I can see possible scenarios where a dequeue
may be needed, but I don't see one for control transfer.
The host can cancel the control transfer, and the controller will see
"setup_packet_pending" and handle accordingly. If there's a disconnect,
that's also handled separately by the controller driver also. So, where
does ep0_dequeue come into play here?
adding the reference to other thread [1]
[1]: https://www.spinics.net/lists/linux-usb/msg233862.html
was trying to address a race condition in the ffs driver where
ep_dequeue was suggested, before freeing the request dequeue it.
as per the current code, since ep0 req isn't moved to started list
therefore it will exit from this in ep_dequeue:
list_for_each_entry(r, &dep->pending_list, list) {
if (r == req) {
dwc3_gadget_giveback(dep, req, -ECONNRESET);
goto out;
}
}
but if the ep0 is in data/status phase technically it is still active.
We will unmap the buffer and giveback then the ep0 is in data/status stage.
This could potentially happen right?
The intent of a separate dequeue was to address that scenario when the
data/status phase isn't completed.
Hope this give some clarity.
Thanks,
-Udipto
Thanks,
Thinh
