Hello, Robert.

Maybe qualifying the subject that it's only for single IRQ would be a
good idea?

> @@ -52,6 +53,7 @@
>  
>  enum {
>       AHCI_PCI_BAR_STA2X11    = 0,
> +     AHCI_PCI_BAR_CAVIUM     = 0,
>       AHCI_PCI_BAR_ENMOTUS    = 2,
>       AHCI_PCI_BAR_STANDARD   = 5,

I thought I already asked but please separate out CAVIUM specific
changes from msix implementation and follow up with why cavium depends
on msix support.

> +static int ahci_init_msix(struct pci_dev *pdev, unsigned int n_ports,
> +                       struct ahci_host_priv *hpriv)
> +{

Please add a comment describing that it's for single msix only and why
that'd be necessary for certain controllers.

...
> +     /*
> +      * 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) {
> +             rc = -ENOSYS;
> +             goto fail;
> +     }

Didn't I ask this one too the last time?  Can you explain why we can't
initialize single IRQ mode if nvec >= n_ports?

> @@ -1260,6 +1339,10 @@ static int ahci_init_interrupts(struct pci_dev *pdev, 
> unsigned int n_ports,
>       if (nvec >= 0)
>               return nvec;
>  
> +     nvec = ahci_init_msix(pdev, n_ports, hpriv);
> +     if (nvec >= 0)
> +             return nvec;

Please add a comment explaining why we're doing msix after msi.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to