Re: [PATCH] usb: xhci: enable interrupt only after xhci_start()

[Mathias Nyman <mathias.nyman@linux.intel.com>]

On 19.2.2020 1.50, Ajay Gupta wrote:
> From: Ajay Gupta <ajayg@nvidia.com>
> 
> Xhci interrupt must be enabled only after controller is
> initialized and started. Currently interrupt is enabled
> first in xhci_run() and later hcd state is set to running
> in xhci_run_finished().
> 
> On a slow system (such as FPGA based platform) the time
> difference between enabling interrupt and setting the hcd
> state becomes huge enough where interrupt is triggered but
> controller initialization is not complete yet.
> 
> Fixing the same by moving the interrupt enable (CMD_EIE)
> part of code snippet from xhci_run() to xhci_run_finished().
> 
> Signed-off-by: Ajay Gupta <ajayg@nvidia.com>
> ---

Sounds reasonable, but xHCI specs wants interrupts set and enabled before
xHC is running state.

I see this can be an issue if we get a port event for a USB 3 port before
the USB3 hcd is added.
What kind of issues did you see? I'd guess NULL pointer dereference in 
handle_port_status()?.

We could move interrupt enabling to xhci_run_finished() before 
xhci_start() is called, then the USB3 hcd should be initialized before
we receive interrupts.

Does that work for you?

Details:
xHCI section 4.2 "Host Controller Initialization" has the following sequence:

- Enable host system interrupt (CMD_EIE),
- Enable primary interupter (set IE bit in IMAN register)
- set run bit in USBCMD register.

And section 5.5.2 has a note:

"All registers of the Primary Interrupter shall be initialized before setting the
Run/Stop (RS) flag in the USBCMD register to ‘1’."

-Mathias