On 13. 07. 21, 12:40, Andy Shevchenko wrote:
There is no need to try MSI/MSI-X only on selected devices.
If MSI is not supported while neing advertised it means device
being
is broken and we rather introduce a list of such devices which
hopefully will be small or never appear.
Hmm, have you checked the commit which introduced the whitelist?
Nevertheless, this needs to handled with care: while many 8250 devices
actually claim to support MSI(-X) interrupts it should not be
enabled be
default. I had at least one device in my hands with broken MSI
implementation.
So better introduce a whitelist with devices that are known to support
MSI(-X) interrupts. I tested all devices mentioned in the patch.
You should have at least CCed the author for an input.
Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
---
drivers/tty/serial/8250/8250_pci.c | 28 ++++++++--------------------
1 file changed, 8 insertions(+), 20 deletions(-)
diff --git a/drivers/tty/serial/8250/8250_pci.c b/drivers/tty/serial/8250/8250_pci.c
index 937861327aca..02825c8c5f84 100644
--- a/drivers/tty/serial/8250/8250_pci.c
+++ b/drivers/tty/serial/8250/8250_pci.c
@@ -58,18 +58,6 @@ struct serial_private {
#define PCI_DEVICE_ID_HPE_PCI_SERIAL 0x37e
-static const struct pci_device_id pci_use_msi[] = {
- { PCI_DEVICE_SUB(PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9900,
- 0xA000, 0x1000) },
- { PCI_DEVICE_SUB(PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9912,
- 0xA000, 0x1000) },
- { PCI_DEVICE_SUB(PCI_VENDOR_ID_NETMOS, PCI_DEVICE_ID_NETMOS_9922,
- 0xA000, 0x1000) },
- { PCI_DEVICE_SUB(PCI_VENDOR_ID_HP_3PAR, PCI_DEVICE_ID_HPE_PCI_SERIAL,
- PCI_ANY_ID, PCI_ANY_ID) },
- { }
-};
-
static int pci_default_setup(struct serial_private*,
const struct pciserial_board*, struct uart_8250_port *, int);
@@ -3994,14 +3982,9 @@ pciserial_init_ports(struct pci_dev *dev, const struct pciserial_board *board)
if (board->flags & FL_NOIRQ) {
uart.port.irq = 0;
} else {
- if (pci_match_id(pci_use_msi, dev)) {
- dev_dbg(&dev->dev, "Using MSI(-X) interrupts\n");
- pci_set_master(dev);
- rc = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_ALL_TYPES);
- } else {
- dev_dbg(&dev->dev, "Using legacy interrupts\n");
- rc = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_LEGACY);
- }
+ pci_set_master(dev);
But bus mastering is not about MSIs. I *think* it's still OK, but you
need to document that in the commit log too.
Actually, why the commit which added this code turns on bus mastering?
+
+ rc = pci_alloc_irq_vectors(dev, 1, 1, PCI_IRQ_ALL_TYPES);
if (rc < 0) {
kfree(priv);
priv = ERR_PTR(rc);
@@ -4009,6 +3992,11 @@ pciserial_init_ports(struct pci_dev *dev, const struct pciserial_board *board)
}
uart.port.irq = pci_irq_vector(dev, 0);
+
+ if (pci_dev_msi_enabled(dev))
+ dev_dbg(&dev->dev, "Using MSI(-X) interrupts\n");
+ else
+ dev_dbg(&dev->dev, "Using legacy interrupts\n");
}
uart.port.dev = &dev->dev;
thanks,
--
js
suse labs
