Hi Mathias
The kernel version is 3.8.2, but patches of 3.17 are back ported into it.

> Event ring dequeue is increased either in handle_tx_event(), cleanup:
> if (trb_comp_code == COMP_MISSED_INT || !ep->skip) {
>                        inc_deq(xhci, xhci->event_ring);

Event ring dequeue should be incremented for "stop,-invalid length” transfer 
event as well (as described in the patch below):
Signed-off-by: Saurov Shyam <sauro...@samsung.com>
---
 drivers/usb/host/xhci-ring.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 7d34cbf..4d2817d 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2500,7 +2500,9 @@ cleanup:
         * Do not update event ring dequeue pointer if ep->skip is set.
         * Will roll back to continue process missed tds.
         */
-       if (trb_comp_code == COMP_MISSED_INT || !ep->skip) {
+       if (trb_comp_code == COMP_MISSED_INT ||
+           trb_comp_code == COMP_STOP_INVAL ||
+           !ep->skip) {
            inc_deq(xhci, xhci->event_ring);
        }
--
1.7.9.5 

> Or then in xhci_handle_event() if handle_tx_event() returns error.
> For example in handle_tx_event() if event->buffer is null, then
> ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event->buffer));  is NULL
> and the check immediately after would return with -ENODEV 
If event-> buffer is valid for "stop,-invalid length” transfer event, hence 
error condition will not happen.
So the above patch is applicable in this sense.

> P.S. The potion of code was not a suggestion, it was copypasted from the 
> upstream xhci driver, and has
> looked like that since 3.17 kernel
By the phrase “code you suggested” I meant is “code what you referred”. Sorry 
for miscommunication!

------- Original Message -------
Sender : Mathias Nyman<mathias.ny...@linux.intel.com>
Date : Jul 02, 2015 17:59 (GMT+05:30)
Title : Re: [PATCH] xHCI: FSE handled cleanly by incrementing event dequeue 
pointer

Hi

On 02.07.2015 13:45, SAUROV KANTI SHYAM wrote:
> Some undefined values, mostly null are coming in event->buffer in my case for 
> "stop,-invalid length” transfer event.

That doesn't sound good, what xhci vendor and version are you using?
Have you seen/tried this on other xhci hw as well? 

What kernel version are you using? are there any custom xhci driver 
modification in it?

> However, in the portion of the code you suggested does not increment the 
> dequeue pointer of transfer event ring, for trb_comp_code = COMP_STOP_INVAL.
> In handle_tx_event() cleanup: does not increment the dequeue pointer of 
> transfer event ring.
> In xhci_handle_event():
> …
> case TRB_TYPE(TRB_TRANSFER):
>         ret = handle_tx_event(xhci, &event->trans_event);
>         if (ret < 0)
>             xhci->error_bitmask |= 1 << 9;
>         else
>             update_ptrs = 0;
> …
> if (update_ptrs)
>         /* Update SW event ring dequeue pointer */
>         inc_deq(xhci, xhci->event_ring);
> …
> As update_ptrs=0, inc_deq(xhci, xhci->event_ring) won't run.
> Regards,

Event ring dequeue is increased either in handle_tx_event(), cleanup:

if (trb_comp_code == COMP_MISSED_INT || !ep->skip) {
                        inc_deq(xhci, xhci->event_ring);

Or then in xhci_handle_event() if handle_tx_event() returns error.

For example in handle_tx_event() if event->buffer is null, then
ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event->buffer));  is NULL
and the check immediately after would return with -ENODEV 

-Mathias

P.S. The potion of code was not a suggestion, it was copypasted from the 
upstream xhci driver, and has
looked like that since 3.17 kernel

Reply via email to