On 9/6/2012 10:57 PM, Alan Stern wrote:
> On Thu, 6 Sep 2012, Pavan Kondeti wrote:
>
>> Hi
>>
>> I am debugging "EHCI host system error" (4.15.2.4) issue. The issue
>> happens during unlinking of URB from an interface driver. In our system
>> the device is always connected to the host. some interfaces are always
>> active (I/O can happen). Other interface's I/O depends on the user
>> space. if user opens the device node, we submit IN URB. This issue
>> happens when unlinking URB upon file close. This issue happens very rarely.
>>
>> I have a doubt in unlink path of EHCI driver. Say if an endpoint has two
>> pending IN URB requests at the time of calling unlinking API.
>> (usb_kill_anchored_urbs API).
>>
>> The QH's queue of this endpoint looks like this.
>>
>> Qtd1 --> Qtd2 --> Dummy
>>
>> EHCI driver kicks in the Async Advance Doorbell handshake after
>> manipulating the QH horizontal pointer in start_unlink_async() function.
>> EHCI driver wait for IAAD interrupt from the controller.
>>
>> Say controller is working on Qtd1. Which means transfer overlay of this
>> QH has reference to Qtd2. controller generates IAAD interrupt (Qtd1 is
>> not completely finished. QH active pointer points to Qtd1). EHCI driver
>> finishes Qtd2 unlinking and points qtd1's next pointer to qtd'2 next
>> pointer and puts this QH back to asynchronous schedule.
>>
>> My doubt is that software updated only QTD1 memory to point it to Qtd'2
>> next pointer i.e dummy. What about the QH's transfer overlay memory?
>> Would not the controller try to access the Qtd2 when QH is put back into
>> schedule.
>>
>> To further clarify my question, I am copy pasting the qh_refresh() code
>> here. In the below code, if we enter "first qtd may already be partially
>> processed" case, should not we update qh->hw->hw_qtd_next and
>> qh->hw->hw_alt_next to reflect the current Qtd memory.
>>
>> static void
>> qh_refresh (struct ehci_hcd *ehci, struct ehci_qh *qh)
>> {
>> struct ehci_qtd *qtd;
>>
>> if (list_empty (&qh->qtd_list))
>> qtd = qh->dummy;
>> else {
>> qtd = list_entry (qh->qtd_list.next,
>> struct ehci_qtd, qtd_list);
>> /* first qtd may already be partially processed */
>> if (cpu_to_hc32(ehci, qtd->qtd_dma) == qh->hw->hw_current)
>> qtd = NULL;
>> }
>>
>> if (qtd)
>> qh_update (ehci, qh, qtd);
>> }
>
> You're absolutely right; this is a bug in the driver. Would you like
> to submit a patch to fix it?
>
Thanks for your response. I will submit a patch.
> (It shouldn't be necessary to update the hw_alt_next field. The
> hw_alt_next part of the qTD doesn't get changed when the following qTD
> is removed.)
>
Agreed.
Thanks,
Pavan
--
Sent by a consultant of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation
--
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
