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

[Colin Ian King]

On 06/03/14 13:28, Tim Gardner wrote:
> 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

I now see the root cause of my issues, the linux-image-extras* package
was missing causing me not to see the USB devices when I originally
tested the -proposed kernel and when I installed my rebuilt debug kernel
it worked because I installed the *extras.  So, this is a NACK from me.

Colin