On 2016-09-23 18:00, Sebastian Andrzej Siewior wrote:

On 2016-09-23 17:39:38 [+0200], Jan Kiszka wrote:

quoted

diff --git a/drivers/spi/spi-pxa2xx-pci.c b/drivers/spi/spi-pxa2xx-pci.c
index 520ed1d..4ec458f 100644
--- a/drivers/spi/spi-pxa2xx-pci.c
+++ b/drivers/spi/spi-pxa2xx-pci.c

@@ -168,6 +168,7 @@ static int pxa2xx_spi_pci_probe(struct pci_dev *dev,
 		dev_err(&dev->dev, "failed to ioremap() registers\n");
 		return -EIO;
 	}
+	pci_enable_msi(dev);
 	ssp->irq = dev->irq;
 	ssp->port_id = (c->port_id >= 0) ? c->port_id : dev->devfn;
 	ssp->type = c->type;

As you've written that code: Any reason why we should refrain from doing
that upstream?

I don't see why this pops up in -RT and not in !RT and this cures it. I
remember that the UART on CE4100 went crazy if you used inb/outb once
you enabled interrupts in multi mode (PCI A/B/C/D instead only A).
Switching over to MMIO access fixed that.

Yeah, it's still not a good overall feeling.

So. Enabling MSI unconditionally isn't usually that bad. If the HW does
not advertise MSI then nothing happens. However there is faulty HW that
advertises MSI and after enabling it you get no interrupts (grep for
XHCI_BROKEN_MSI if you want an example). I would suggest to enable it
and if people complain add an exception list (like XHCI does).
And you might want to disable MSI (but it might happen if you disable /
leave the PCI).

I already checked that: As there is no pcim_enable_msi, I thought there
is no need (pcim_enable_device would take care). But a loop over
bind/unbind showed that we were leaking vectors. Will fix that and send
a proper patch.

I was also trying to turn MSI on in intel_quark_i2c_gpio, but that
device fired back by stopping to deliver interrupts. OK.

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux