Hi Xenia,

Thanks for adding this.  It should be a big help for debugging issues
with isochronous transfers to avoid massive log spam on missed service
intervals.

On Fri, Aug 23, 2013 at 11:15:18AM +0300, Xenia Ragiadakou wrote:
> This patch adds a trace event to the class 'xhci_log_msg', called
> 'xhci_dbg_missed_periodic_tx', that traces the debug statements which
> signal that the xHC is unable to service an isochronous endpoint within
> its service interval either because the endpoint's ring is full and can
> not receive further data, for IN endpoints, or because the endpoint's ring
> is empty, for OUT endpoints, or due to xHC internal buffers' overrun or
> underrun caused by excessive latency on the transfer path.

Could you break this description up into a couple sentences?

You also need to add a trace event for this code in handle_tx_event:

                /* We've skipped all the TDs on the ep ring when ep->skip set */
                if (ep->skip && td_num == 0) {
                        ep->skip = false;
                        xhci_dbg(xhci, "All tds on the ep_ring skipped. "
                                                "Clear skip flag.\n");
                        ret = 0;
                        goto cleanup;
                }

I would appreciate it if you could clean up that split string, as well.

I also need you to add this code to the trace event:

                if (ep->skip) {
                        xhci_dbg(xhci, "Found td. Clear skip flag.\n");
                        ep->skip = false;
                }


Thanks,
Sarah Sharp

> Signed-off-by: Xenia Ragiadakou <burzalod...@gmail.com>
> ---
>  drivers/usb/host/xhci-ring.c  | 19 ++++++++++++-------
>  drivers/usb/host/xhci-trace.h |  5 +++++
>  2 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> index 03f65dc..cbf3e2a 100644
> --- a/drivers/usb/host/xhci-ring.c
> +++ b/drivers/usb/host/xhci-ring.c
> @@ -2484,18 +2484,22 @@ static int handle_tx_event(struct xhci_hcd *xhci,
>                * a Ring Overrun Event for IN Isoch endpoint or Ring
>                * Underrun Event for OUT Isoch endpoint.
>                */
> -             xhci_dbg(xhci, "underrun event on endpoint\n");
> +             xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
> +                             "underrun event on endpoint");
>               if (!list_empty(&ep_ring->td_list))
> -                     xhci_dbg(xhci, "Underrun Event for slot %d ep %d "
> -                                     "still with TDs queued?\n",
> +                     xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
> +                                     "Underrun Event for slot %d ep %d "
> +                                     "still with TDs queued?",
>                                TRB_TO_SLOT_ID(le32_to_cpu(event->flags)),
>                                ep_index);
>               goto cleanup;
>       case COMP_OVERRUN:
> -             xhci_dbg(xhci, "overrun event on endpoint\n");
> +             xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
> +                             "overrun event on endpoint");
>               if (!list_empty(&ep_ring->td_list))
> -                     xhci_dbg(xhci, "Overrun Event for slot %d ep %d "
> -                                     "still with TDs queued?\n",
> +                     xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
> +                                     "Overrun Event for slot %d ep %d "
> +                                     "still with TDs queued?",
>                                TRB_TO_SLOT_ID(le32_to_cpu(event->flags)),
>                                ep_index);
>               goto cleanup;
> @@ -2511,7 +2515,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
>                * short transfer when process the ep_ring next time.
>                */
>               ep->skip = true;
> -             xhci_dbg(xhci, "Miss service interval error, set skip flag\n");
> +             xhci_dbg_trace(xhci, trace_xhci_dbg_missed_periodic_tx,
> +                             "Miss service interval error, set skip flag");
>               goto cleanup;
>       default:
>               if (xhci_is_vendor_info_code(xhci, trb_comp_code)) {

> diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
> index 20364cc..c156685 100644
> --- a/drivers/usb/host/xhci-trace.h
> +++ b/drivers/usb/host/xhci-trace.h
> @@ -67,6 +67,11 @@ DEFINE_EVENT(xhci_log_msg, xhci_dbg_ring_expansion,
>       TP_ARGS(vaf)
>  );
>  
> +DEFINE_EVENT(xhci_log_msg, xhci_dbg_missed_periodic_tx,
> +     TP_PROTO(struct va_format *vaf),
> +     TP_ARGS(vaf)
> +);
> +
>  DECLARE_EVENT_CLASS(xhci_log_ctx,
>       TP_PROTO(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx,
>                unsigned int ep_num),
> -- 
> 1.8.3.4
> 
> --
> 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
--
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