Hello, Robert. On Mon, May 11, 2015 at 07:18:10PM +0200, Robert Richter wrote: > static int ahci_init_msix(struct pci_dev *pdev, unsigned int n_ports, > struct ahci_host_priv *hpriv) > { > int rc, nvec; > struct msix_entry entry = {}; > > /* check if msix is supported */ > nvec = pci_msix_vec_count(pdev); > if (nvec <= 0) > return 0; > > /* > * Per-port msix interrupts are not supported. Assume single > * port interrupts for: > * > * n_ports == 1, or > * nvec < n_ports. > * > * We also need to check for n_ports != 0 which is implicitly > * covered here since nvec > 0. > */ > if (n_ports != 1 && nvec >= n_ports) > return -ENOSYS; Why are failing the whole thing when nvec >= n_ports? Can't we just print some warning and configure it for single interrupt mode? > > Also, shouldn't we be printing a warning message here explaining why > > probing is failing? > > I didn't want to print a warning in case -ENOSYS for backward > compatability. Only if msi-x code fails there is a message, see > __ahci_init_interrupts(). In any other case the behaviour is as > before, thus no message is printed. I'm confused here. Why are we implementing msix support at all if it only support single interrupt mode? I kinda assumed that that was because you're trying to support a controller which does only msix, no? At any rate, I don't think it's wrong to print an informational / warning message when a controller declares msix support but has wacko parameters. > > > + > > > + /* only enable the first entry (entry.entry = 0) */ > > > + rc = pci_enable_msix_exact(pdev, &entry, 1); > > > > So, enabling the first msix works if nvec > 1 && nvec < n_ports but > > not if nvec >= n_ports? > > For n_ports > 1 && nvec >= n_ports we need to assume per-port > interrupts. There are enough vectors for all ports then. Again, and we fail irq init in that case? Thanks. -- tejun -- To unsubscribe from this list: send the line "unsubscribe linux-ide" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html