From patchwork Wed Nov 30 09:02:22 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "\(Exiting\) Baolin Wang" X-Patchwork-Id: 84977 Delivered-To: patch@linaro.org Received: by 10.140.20.101 with SMTP id 92csp143313qgi; Wed, 30 Nov 2016 01:09:37 -0800 (PST) X-Received: by 10.84.216.6 with SMTP id m6mr71688365pli.130.1480496977428; Wed, 30 Nov 2016 01:09:37 -0800 (PST) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id p73si63453876pfl.79.2016.11.30.01.09.37; Wed, 30 Nov 2016 01:09:37 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757552AbcK3JJZ (ORCPT + 25 others); Wed, 30 Nov 2016 04:09:25 -0500 Received: from mail-pg0-f45.google.com ([74.125.83.45]:36095 "EHLO mail-pg0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757264AbcK3JJI (ORCPT ); Wed, 30 Nov 2016 04:09:08 -0500 Received: by mail-pg0-f45.google.com with SMTP id f188so79811457pgc.3 for ; Wed, 30 Nov 2016 01:09:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=iV6XTou/eSwVotdNY7EsRTiIYY81uOcNflaxjQULiX4=; b=Zeufzm8i9QgyiHuusNJg8p+piPqX0WicecfwIb4SPO3RVbCKrrGwqhMEh7Uz3r46jS aL9yCUX7mIW1AakUiDe+LoUpi6PTwMjJn8lOuD38ZUfsvGnHTlYYoyfKgSFCkTr6cR1n BwLHl2XRKBdkIvNcMl9w7+9Wxqhk2yyJta2v4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=iV6XTou/eSwVotdNY7EsRTiIYY81uOcNflaxjQULiX4=; b=iYEQ+4JkOA5F/NBJViSQ6C4ZBcQT5OUccilsRPY1SidP60KR8u5ZY63cSXF5LpPccl gXGKLLi3gWpSTlFwWvWLfHnF2++A2pR3Nxmo/+eiY0Zw/7AmHM67EtoT0szJ/WbhGyIb Z1HeqSedyUQdQ2/6K4X9Cs74TTyG586CzizsgKX23lNijh5AyMR7Hhrm25ZJ1h5imiFl itZrbgg0sUHS5okyN/RDlRiI/q2XHvinZhClqTkFBRmt7qRtiwKOWVfJ6vQrL6nczvNV sewD6mTnbzDf5Ic8lxAkEdjrUxxsrmbfeZcy7OG+zcKWz23TOn656u25EX+s56gY8UJv VcOA== X-Gm-Message-State: AKaTC02HmkYa5MJ6y7r18IXuyBGI0bg7ahvYMesBJUS54NC25tVBZXwILIfMaNxmhcEtVsWr X-Received: by 10.99.111.201 with SMTP id k192mr57430356pgc.176.1480496568013; Wed, 30 Nov 2016 01:02:48 -0800 (PST) Received: from baolinwangubtpc.spreadtrum.com ([175.111.195.49]) by smtp.gmail.com with ESMTPSA id r124sm83052103pgr.6.2016.11.30.01.02.45 (version=TLS1 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 30 Nov 2016 01:02:47 -0800 (PST) From: Baolin Wang To: mathias.nyman@intel.com, gregkh@linuxfoundation.org Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, broonie@kernel.org, baolin.wang@linaro.org Subject: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command Date: Wed, 30 Nov 2016 17:02:22 +0800 Message-Id: <613dafc211127a4589306e91e231e151feb5ce80.1480496291.git.baolin.wang@linaro.org> X-Mailer: git-send-email 1.7.9.5 Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org If the hardware never responds to the stop endpoint command, the URBs will never be completed, and we might hang the USB subsystem. The original watchdog timer is used to watch if one stop endpoint command is timeout, if timeout, then the watchdog timer will set XHCI_STATE_DYING, try to halt the xHCI host, and give back all pending URBs. But now we already have one command timer to control command timeout, thus we can also use the command timer to watch the stop endpoint command, instead of one duplicate watchdog timer which need to be removed. Meanwhile we don't need the 'stop_cmds_pending' flag to identy if this is the last stop endpoint command of one endpoint. Since we can make sure we only set one stop endpoint command for one endpoint by 'EP_HALT_PENDING' flag in xhci_urb_dequeue() function. Thus remove this flag. We also need to clean up the command queue before trying to halt the xHCI host in xhci_stop_endpoint_command_timeout() function. Signed-off-by: Baolin Wang --- drivers/usb/host/xhci-mem.c | 10 +------ drivers/usb/host/xhci-ring.c | 61 +++++++++++++----------------------------- drivers/usb/host/xhci.c | 8 +----- drivers/usb/host/xhci.h | 5 ---- 4 files changed, 21 insertions(+), 63 deletions(-) -- 1.7.9.5 diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c index d6f59a3..2a5cd89 100644 --- a/drivers/usb/host/xhci-mem.c +++ b/drivers/usb/host/xhci-mem.c @@ -844,14 +844,6 @@ void xhci_free_stream_info(struct xhci_hcd *xhci, /***************** Device context manipulation *************************/ -static void xhci_init_endpoint_timer(struct xhci_hcd *xhci, - struct xhci_virt_ep *ep) -{ - setup_timer(&ep->stop_cmd_timer, xhci_stop_endpoint_command_watchdog, - (unsigned long)ep); - ep->xhci = xhci; -} - static void xhci_free_tt_info(struct xhci_hcd *xhci, struct xhci_virt_device *virt_dev, int slot_id) @@ -1014,7 +1006,7 @@ int xhci_alloc_virt_device(struct xhci_hcd *xhci, int slot_id, /* Initialize the cancellation list and watchdog timers for each ep */ for (i = 0; i < 31; i++) { - xhci_init_endpoint_timer(xhci, &dev->eps[i]); + dev->eps[i].xhci = xhci; INIT_LIST_HEAD(&dev->eps[i].cancelled_td_list); INIT_LIST_HEAD(&dev->eps[i].bw_endpoint_list); } diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 2057d08..67a1bd6 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -561,18 +561,6 @@ static void td_to_noop(struct xhci_hcd *xhci, struct xhci_ring *ep_ring, } } -static void xhci_stop_watchdog_timer_in_irq(struct xhci_hcd *xhci, - struct xhci_virt_ep *ep) -{ - ep->ep_state &= ~EP_HALT_PENDING; - /* Can't del_timer_sync in interrupt, so we attempt to cancel. If the - * timer is running on another CPU, we don't decrement stop_cmds_pending - * (since we didn't successfully stop the watchdog timer). - */ - if (del_timer(&ep->stop_cmd_timer)) - ep->stop_cmds_pending--; -} - /* Must be called with xhci->lock held in interrupt context */ static void xhci_giveback_urb_in_irq(struct xhci_hcd *xhci, struct xhci_td *cur_td, int status) @@ -664,7 +652,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, ep = &xhci->devs[slot_id]->eps[ep_index]; if (list_empty(&ep->cancelled_td_list)) { - xhci_stop_watchdog_timer_in_irq(xhci, ep); + ep->ep_state &= ~EP_HALT_PENDING; ep->stopped_td = NULL; ring_doorbell_for_active_rings(xhci, slot_id, ep_index); return; @@ -719,7 +707,7 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, list_del_init(&cur_td->td_list); } last_unlinked_td = cur_td; - xhci_stop_watchdog_timer_in_irq(xhci, ep); + ep->ep_state &= ~EP_HALT_PENDING; /* If necessary, queue a Set Transfer Ring Dequeue Pointer command */ if (deq_state.new_deq_ptr && deq_state.new_deq_seg) { @@ -817,38 +805,19 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci, } } -/* Watchdog timer function for when a stop endpoint command fails to complete. +/* This timeout function for when a stop endpoint command fails to complete. * In this case, we assume the host controller is broken or dying or dead. The * host may still be completing some other events, so we have to be careful to * let the event ring handler and the URB dequeueing/enqueueing functions know * through xhci->state. - * - * The timer may also fire if the host takes a very long time to respond to the - * command, and the stop endpoint command completion handler cannot delete the - * timer before the timer function is called. Another endpoint cancellation may - * sneak in before the timer function can grab the lock, and that may queue - * another stop endpoint command and add the timer back. So we cannot use a - * simple flag to say whether there is a pending stop endpoint command for a - * particular endpoint. - * - * Instead we use a combination of that flag and a counter for the number of - * pending stop endpoint commands. If the timer is the tail end of the last - * stop endpoint command, and the endpoint's command is still pending, we assume - * the host is dying. */ -void xhci_stop_endpoint_command_watchdog(unsigned long arg) +void xhci_stop_endpoint_command_timeout(struct xhci_hcd *xhci) { - struct xhci_hcd *xhci; - struct xhci_virt_ep *ep; int ret, i, j; unsigned long flags; - ep = (struct xhci_virt_ep *) arg; - xhci = ep->xhci; - spin_lock_irqsave(&xhci->lock, flags); - ep->stop_cmds_pending--; if (xhci->xhc_state & XHCI_STATE_REMOVING) { spin_unlock_irqrestore(&xhci->lock, flags); return; @@ -860,13 +829,6 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) spin_unlock_irqrestore(&xhci->lock, flags); return; } - if (!(ep->stop_cmds_pending == 0 && (ep->ep_state & EP_HALT_PENDING))) { - xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, - "Stop EP timer ran, but no command pending, " - "exiting."); - spin_unlock_irqrestore(&xhci->lock, flags); - return; - } xhci_warn(xhci, "xHCI host not responding to stop endpoint command.\n"); xhci_warn(xhci, "Assuming host is dying, halting host.\n"); @@ -904,6 +866,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) xhci_kill_endpoint_urbs(xhci, i, j); } spin_unlock_irqrestore(&xhci->lock, flags); + xhci_cleanup_command_queue(xhci); xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb, "Calling usb_hc_died()"); usb_hc_died(xhci_to_hcd(xhci)); @@ -1266,7 +1229,21 @@ void xhci_handle_command_timeout(unsigned long data) unsigned long flags; u64 hw_ring_state; bool second_timeout = false; + union xhci_trb *cmd_trb; + u32 cmd_type = 0; + xhci = (struct xhci_hcd *) data; + spin_lock_irqsave(&xhci->lock, flags); + if (xhci->current_cmd) { + cmd_trb = xhci->current_cmd->command_trb; + cmd_type = TRB_FIELD_TO_TYPE(le32_to_cpu(cmd_trb->generic.field[3])); + } + spin_unlock_irqrestore(&xhci->lock, flags); + + if (cmd_type == TRB_STOP_RING) { + xhci_stop_endpoint_command_timeout(xhci); + return; + } /* mark this command to be cancelled */ spin_lock_irqsave(&xhci->lock, flags); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index dc337b3..8b45040 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1575,10 +1575,6 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) goto done; } ep->ep_state |= EP_HALT_PENDING; - ep->stop_cmds_pending++; - ep->stop_cmd_timer.expires = jiffies + - XHCI_STOP_EP_CMD_TIMEOUT * HZ; - add_timer(&ep->stop_cmd_timer); xhci_queue_stop_endpoint(xhci, command, urb->dev->slot_id, ep_index, 0); xhci_ring_cmd_db(xhci); @@ -3615,10 +3611,8 @@ void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev) virt_dev = xhci->devs[udev->slot_id]; /* Stop any wayward timer functions (which may grab the lock) */ - for (i = 0; i < 31; ++i) { + for (i = 0; i < 31; ++i) virt_dev->eps[i].ep_state &= ~EP_HALT_PENDING; - del_timer_sync(&virt_dev->eps[i].stop_cmd_timer); - } spin_lock_irqsave(&xhci->lock, flags); /* Don't disable the slot if the host controller is dead. */ diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 9dbaacf..4c23c27 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -919,9 +919,6 @@ struct xhci_virt_ep { struct list_head cancelled_td_list; struct xhci_td *stopped_td; unsigned int stopped_stream; - /* Watchdog timer for stop endpoint command to cancel URBs */ - struct timer_list stop_cmd_timer; - int stop_cmds_pending; struct xhci_hcd *xhci; /* Dequeue pointer and dequeue segment for a submitted Set TR Dequeue * command. We'll need to update the ring's dequeue segment and dequeue @@ -1456,8 +1453,6 @@ struct urb_priv { #define ERST_ENTRIES 1 /* Poll every 60 seconds */ #define POLL_TIMEOUT 60 -/* Stop endpoint command timeout (secs) for URB cancellation watchdog timer */ -#define XHCI_STOP_EP_CMD_TIMEOUT 5 /* XXX: Make these module parameters */ struct s3_save {