On Mon, Jan 19, 2009 at 3:07 PM, Mark Lord <liml@xxxxxx> wrote:
> Enable reliable use of Message Switched Interrupts (MSI) in sata_mv
> by masking further chip interrupts within the main interrupt handler.
> Based upon a suggestion by Grant Grundler.
This is a HW bug workaround so we don't end up with completions
pending but no interrupts pending.
>
> Signed-off-by: Mark Lord <mlord@xxxxxxxxx>
Please add:
Reviewed-by: Grant Grundler <grundler@xxxxxxxxxx>
> --- old/drivers/ata/sata_mv.c 2009-01-19 17:18:31.000000000 -0500
> +++ linux/drivers/ata/sata_mv.c 2009-01-19 17:12:51.000000000 -0500
> @@ -33,8 +33,6 @@
> *
> * --> ATAPI support (Marvell claims the 60xx/70xx chips can do it).
> *
> - * --> Investigate problems with PCI Message Signalled Interrupts (MSI).
> - *
> * --> Develop a low-power-consumption strategy, and implement it.
> *
> * --> [Experiment, low priority] Investigate interrupt coalescing.
> @@ -2199,9 +2197,15 @@
> struct ata_host *host = dev_instance;
> struct mv_host_priv *hpriv = host->private_data;
> unsigned int handled = 0;
> + int using_msi = hpriv->hp_flags & MV_HP_FLAG_MSI;
> u32 main_irq_cause, pending_irqs;
>
> spin_lock(&host->lock);
> +
> + /* for MSI: block new interrupts while in here */
> + if (using_msi)
> + writel(0, hpriv->main_irq_mask_addr);
> +
> main_irq_cause = readl(hpriv->main_irq_cause_addr);
> pending_irqs = main_irq_cause & hpriv->main_irq_mask;
> /*
> @@ -2215,6 +2219,11 @@
> handled = mv_host_intr(host, pending_irqs);
> }
> spin_unlock(&host->lock);
> +
> + /* for MSI: unblock interupts; anything pending will hit us now */
Better comment would be:
/* Unmasking any Cause bit which is set will generate an interrupt. */
And since we've masked all bits above, this effectively applies to all bits
in the mask. This is true for either type of interrupt (Line vs MSI).
We only require this for MSI though.
FYI (everyone else but Mark), MSI only brings two benefits to this driver:
1) ability to spread interrupt load across lots of CPUs if we have lots
of controllers in a machine.
2) Get an exclusive IRQ (avoids spurious calls to IRQ handlers)
thanks,
grant
ps. Kudos to Mark for excellent work to date on this driver.
I'm impressed with how well it's working now compared
to the crap we had a year or two ago.
> + if (using_msi)
> + writel(hpriv->main_irq_mask, hpriv->main_irq_mask_addr);
> +
> return IRQ_RETVAL(handled);
> }
--
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
