Hi,

On 01/16/2017 04:59 PM, Felipe Balbi wrote:
> Hi,
>
> Lu Baolu <baolu...@linux.intel.com> writes:
>> Hi,
>>
>> On 12/29/2016 07:01 PM, Felipe Balbi wrote:
>>> These three new tracers will help us tie TRBs into URBs by *also*
>>> looking into URB lifetime.
>> I didn't see anything related to TRBs in the patch.
>> Anything I missed?
> yes. Ordering of events. Seriously, you ought to compile my xhci-cleanup
> patches and run with tracers enabled. Here's what I'm talking about:
>
> before $subject, we would only see TRBs being queued to HW and
> completed, etc. After $subject, we see URBs being queued by class
> drivers, which in turn triggers xHCI driver to prepare TRBs which will
> be queued to HW. Once a completion IRQ fires, we see TRBs being
> "dequeued" from HW and URBs being given back.
>
> IOW, after $subject we know that a particular TRB (or a set of them)
> were queued because an URB was queued. We know the size of the URB, a
> pointer to it, etc. We can actually match urb->transfer_length to
> sum_of_trb_sizes().

Yes. Get it now. Thanks for this comment.

>>> Signed-off-by: Felipe Balbi <felipe.ba...@linux.intel.com>
>>> ---
>>>  drivers/usb/host/xhci-ring.c  |  1 +
>>>  drivers/usb/host/xhci-trace.h | 70 
>>> +++++++++++++++++++++++++++++++++++++++++++
>>>  drivers/usb/host/xhci.c       |  5 ++++
>>>  3 files changed, 76 insertions(+)
>>>
>>> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
>>> index 0ee7d358b812..1431e0651e78 100644
>>> --- a/drivers/usb/host/xhci-ring.c
>>> +++ b/drivers/usb/host/xhci-ring.c
>>> @@ -627,6 +627,7 @@ static void xhci_giveback_urb_in_irq(struct xhci_hcd 
>>> *xhci,
>>>     usb_hcd_unlink_urb_from_ep(hcd, urb);
>>>     spin_unlock(&xhci->lock);
>>>     usb_hcd_giveback_urb(hcd, urb, status);
>>> +   trace_xhci_urb_giveback(urb);
>> There is another trace point in xhci_urb_dequeue().
>>
>> diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
>> index b49588f..ee46877 100644
>> --- a/drivers/usb/host/xhci.c
>> +++ b/drivers/usb/host/xhci.c
>> @@ -1535,6 +1535,7 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb 
>> *urb, int status)
>>  
>>                 usb_hcd_unlink_urb_from_ep(hcd, urb);
>>                 spin_unlock_irqrestore(&xhci->lock, flags);
>> +               trace_xhci_urb_giveback(urb);
> yes, so? I want to know that the URB *was* indeed given back.
>

Got it. There is no need to trace a giveback which ties no real trbs.

I have no further questions about this patch. Thanks.

Best regards,
Lu Baolu
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to