Re: EHCI host broken -- interrupts disabled

* Re: EHCI host broken -- interrupts disabled
       [not found] ` <Pine.LNX.4.44L0.1401141336020.1310-100000@iolanthe.rowland.org>
@ 2014-01-16 17:32   ` Bjorn Helgaas
  2014-01-16 19:40     ` Alan Stern
  0 siblings, 1 reply; 13+ messages in thread
From: Bjorn Helgaas @ 2014-01-16 17:32 UTC (permalink / raw)
  To: Alan Stern; +Cc: Sarah Sharp, USB list, Chen, Jamie, Tsai, Gaggery, linux-pci

[+cc linux-pci]

On Tue, Jan 14, 2014 at 11:47 AM, Alan Stern <stern@rowland.harvard.edu> wrote:
> On Tue, 14 Jan 2014, Sarah Sharp wrote:
>
>> Hi Alan,
>
> I'm not a good person to ask about this.  Maybe Bjorn can help.
>
>> All this info is second- or third-hand, so please excuse the extremely
>> vague bug report.
>>
>> A new Intel Atom system (Baytrail) comes with both an xHCI host and an
>> EHCI host, but the BIOS has an "EHCI only" option that hides the xHCI
>> host PCI device from the operating system.  It appears that when the
>> BIOS is in this mode, the EHCI PCI interrupt is disabled, and the host
>> simply doesn't work.
>>
>> Jamie and Gaggery have more details about the system and the exact
>> failure mode.  They can provide dmesg as well as a PCI register space
>> dump, if necessary.  (Jamie and Gaggery, please note, a publicly
>> archived mailing list is Cc'ed.)
>>
>> A customer reports that the following patch fixes the issue:
>>
>> diff --git a/drivers/usb/host/ehci-pci.c b/drivers/usb/host/ehci-pci.c
>> index 3e86bf4371b3..e77566a021ec 100644
>> --- a/drivers/usb/host/ehci-pci.c
>> +++ b/drivers/usb/host/ehci-pci.c
>> @@ -112,6 +112,9 @@ static int ehci_pci_setup(struct usb_hcd *hcd)
>>       case PCI_VENDOR_ID_INTEL:
>>               if (pdev->device == PCI_DEVICE_ID_INTEL_CE4100_USB)
>>                       hcd->has_tt = 1;
>> +             /* Baytrail BIOS in EHCI only mode fails to enable interrupts */
>> +             if (pdev->device == 0x0f34)
>> +                     pci_intx(pdev, 1);
>>               break;
>>       case PCI_VENDOR_ID_TDI:
>>               if (pdev->device == PCI_DEVICE_ID_TDI_EHCI)
>>
>> I looked through the USB PCI initialization code, and I can't find any
>> place where interrupts are enabled, so I assume the PCI core is supposed
>> to enable interrupts?  Or is it assumed that the system boots with all
>> PCI device interrupts enabled?  In that case, is it a BIOS issue?
>
> As I recall, the original PCI spec did not include any way to enable or
> disable interrupts -- a rather surprising omission.  It was added
> later on.
>
> Anyhow, this is all handled in the PCI core.  The device driver
> shouldn't need to do much more than pci_enable_device() (which handles
> power plus I/O and memory resources) and pci_set_master() (which
> handles DMA).  Nothing about enabling interrupt generation.
>
> A quick search did not show any place where the PCI core explicitly
> enables interrupts on new devices, so maybe it does assume that the
> firmware always enables them.  I don't know what guarantees (if any) a
> BIOS is supposed to fulfill.
>
> It seems like the best place to fix this is probably in the PCI core.

I think dev->irq is supposed to be valid after pci_enable_device(), so
it seems like it would make sense to clear PCI_COMMAND_INTX_DISABLE
there.

I don't know why a BIOS would leave PCI_COMMAND_INTX_DISABLE set for
the EHCI device.  Does it support MSI, and the BIOS assumes the OS
should use that?  I don't see any MSI support in the EHCI code.  In
any case, this doesn't seem like something we need to depend on the
BIOS to do for us.

Bjorn

^ permalink raw reply	[flat|nested] 13+ messages in thread