Subject: Re: [RFC] usb: host: xhci: Remove the watchdog timer and use command
 timer to watch stop endpoint command
To: Baolin Wang <baolin.wang@linaro.org>, mathias.nyman@intel.com,
        gregkh@linuxfoundation.org
References: <613dafc211127a4589306e91e231e151feb5ce80.1480496291.git.baolin.wang@linaro.org>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
        broonie@kernel.org
From: Mathias Nyman <mathias.nyman@linux.intel.com>
Message-ID: <583EDD87.8030307@linux.intel.com>
Date: Wed, 30 Nov 2016 16:09:11 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Thunderbird/38.8.0
MIME-Version: 1.0
In-Reply-To: <613dafc211127a4589306e91e231e151feb5ce80.1480496291.git.baolin.wang@linaro.org>
Content-Type: text/plain; charset=windows-1252; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1373
Lines: 31

On 30.11.2016 11:02, 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.
>
> We also need to clean up the command queue before trying to halt the
> xHCI host in xhci_stop_endpoint_command_timeout() function.

This isn't a bad idea.

There are anyway some corner cases and details that need to be
checked, such as suspend (which will clear the command queue), module unload
and abrupt host removal (like pci hotplug removal of host controller)
we need to make sure we can trust the command timer to always return the canceled URB

-Mathias