Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758926AbcLAGJT (ORCPT ); Thu, 1 Dec 2016 01:09:19 -0500 Received: from mail-yw0-f182.google.com ([209.85.161.182]:34398 "EHLO mail-yw0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755302AbcLAGJR (ORCPT ); Thu, 1 Dec 2016 01:09:17 -0500 MIME-Version: 1.0 In-Reply-To: References: <613dafc211127a4589306e91e231e151feb5ce80.1480496291.git.baolin.wang@linaro.org> <583FB8F0.6070503@linux.intel.com> From: Baolin Wang Date: Thu, 1 Dec 2016 14:09:16 +0800 Message-ID: Subject: Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command timer to watch stop endpoint command To: Lu Baolu Cc: mathias.nyman@intel.com, Greg KH , USB , LKML , Mark Brown Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1958 Lines: 44 On 1 December 2016 at 14:04, Baolin Wang wrote: > Hi Baolu, > > On 1 December 2016 at 13:45, Lu Baolu wrote: >> Hi, >> >> On 11/30/2016 05:02 PM, Baolin Wang wrote: >>> 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. >> >> I am afraid you can't do this. "stop_cmds_pending" was added >> to fix the problem described in the comments that you want to >> remove. But I didn't find any fix of this problem in your patch. > > Now we can not pending another stop endpoint command for the same one > endpoint, since will check 'EP_HALT_PENDING' flag in > xhci_urb_dequeue() function to avoid this. But after some > investigation, I think I missed the stop endpoint command in > xhci_stop_device() which did not check the 'EP_HALT_PENDING' flag, > maybe need to add 'EP_HALT_PENDING' flag checking in > xhci_stop_device() function. DId I miss something else? Thanks. Sorry, wait, we will wait for the stop endpoint command finished in xhci_stop_device() function. So we don't need check 'EP_HALT_PENDING' flag as the orignal code did. -- Baolin.wang Best Regards