[PATCH][TRUSTY] Revert "usb: ehci: fix deadlock when threadirqs option is used"

[Tim Gardner]

On 03/06/2014 02:20 AM, Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> This patch unfortunately breaks USB on my Lenovo x230 and on a Lenovo
> T440. I guess it may break it on a wider range of machines too. I'm
> requesting that it is reverted.
>
> This reverts commit 65482e0c807a50dc3ec9d59a7465801f9c56bf52.
> ---
>   drivers/usb/host/ehci-hcd.c | 13 +++----------
>   1 file changed, 3 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
> index 4dfd6fb..e8ba4c4 100644
> --- a/drivers/usb/host/ehci-hcd.c
> +++ b/drivers/usb/host/ehci-hcd.c
> @@ -686,15 +686,8 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd)
>   	struct ehci_hcd		*ehci = hcd_to_ehci (hcd);
>   	u32			status, masked_status, pcd_status = 0, cmd;
>   	int			bh;
> -	unsigned long		flags;
>
> -	/*
> -	 * For threadirqs option we use spin_lock_irqsave() variant to prevent
> -	 * deadlock with ehci hrtimer callback, because hrtimer callbacks run
> -	 * in interrupt context even when threadirqs is specified. We can go
> -	 * back to spin_lock() variant when hrtimer callbacks become threaded.
> -	 */
> -	spin_lock_irqsave(&ehci->lock, flags);
> +	spin_lock (&ehci->lock);
>
>   	status = ehci_readl(ehci, &ehci->regs->status);
>
> @@ -712,7 +705,7 @@ static irqreturn_t ehci_irq (struct usb_hcd *hcd)
>
>   	/* Shared IRQ? */
>   	if (!masked_status || unlikely(ehci->rh_state == EHCI_RH_HALTED)) {
> -		spin_unlock_irqrestore(&ehci->lock, flags);
> +		spin_unlock(&ehci->lock);
>   		return IRQ_NONE;
>   	}
>
> @@ -830,7 +823,7 @@ dead:
>
>   	if (bh)
>   		ehci_work (ehci);
> -	spin_unlock_irqrestore(&ehci->lock, flags);
> +	spin_unlock (&ehci->lock);
>   	if (pcd_status)
>   		usb_hcd_poll_rh_status(hcd);
>   	return IRQ_HANDLED;
>

Colin - I'm reluctant to do this since it is a significant divergence 
from upstream. Can you work with the maintainer to figure out why your 
machines break ? Or alternatively the comment above spin_lock_irqsave() 
suggests a revert after hrtimer callbacks become threaded (which ought 
not be too difficult). I could take a shot at it later today.

rtg
-- 
Tim Gardner tim.gardner at canonical.com