[PATCH 01/03] sata_mv Fix MSI irq race

[PATCH 01/03] sata_mv Fix MSI irq race

[Mark Lord]

Fix a (rare) race condition in mv_interrupt() when using MSI.

The value of hpriv->main_irq_mask_addr can change on on the fly,
and without this patch we could end up writing back a stale copy
to the hardware.

Signed-off-by: Mark Lord <mlord@pobox.com>
---

Duplicate of earlier patch submitted for #upstream-fixes (2.6.29).
This one is for libata-dev #upstream

--- old/drivers/ata/sata_mv.c	2009-03-10 14:04:49.000000000 -0400
+++ new/drivers/ata/sata_mv.c	2009-03-10 17:49:50.000000000 -0400
@@ -2631,12 +2631,13 @@
 		else
 			handled = mv_host_intr(host, pending_irqs);
 	}
-	spin_unlock(&host->lock);
 
 	/* for MSI: unmask; interrupt cause bits will retrigger now */
 	if (using_msi)
 		writel(hpriv->main_irq_mask, hpriv->main_irq_mask_addr);
 
+	spin_unlock(&host->lock);
+
 	return IRQ_RETVAL(handled);
 }
 

[PATCH 02/03] sata_mv cosmetic preparations for IRQ coalescing

[Mark Lord]

Various cosmetic changes in preparation for the IRQ coalescing feature.

Note that the various MV_IRQ_COAL_* definitions are restored/renamed
in the folloup patch which adds IRQ coalescing to the driver.

Signed-off-by: Mark Lord <mlord@pobox.com>
---

This is for libata-dev #upstream

--- old/drivers/ata/sata_mv.c	2009-03-10 18:20:20.000000000 -0400
+++ new/drivers/ata/sata_mv.c	2009-03-10 18:26:54.000000000 -0400
@@ -1,10 +1,13 @@
 /*
  * sata_mv.c - Marvell SATA support
  *
- * Copyright 2008: Marvell Corporation, all rights reserved.
+ * Copyright 2008-2009: Marvell Corporation, all rights reserved.
  * Copyright 2005: EMC Corporation, all rights reserved.
  * Copyright 2005 Red Hat, Inc.  All rights reserved.
  *
+ * Originally written by Brett Russ.
+ * Extensive overhaul and enhancement by Mark Lord <mlord@pobox.com>.
+ *
  * Please ALWAYS copy linux-ide@vger.kernel.org on emails.
  *
  * This program is free software; you can redistribute it and/or modify
@@ -25,8 +28,6 @@
 /*
  * sata_mv TODO list:
  *
- * --> Errata workaround for NCQ device errors.
- *
  * --> More errata workarounds for PCI-X.
  *
  * --> Complete a full errata audit for all chipsets to identify others.
@@ -68,6 +69,16 @@
 #define DRV_NAME	"sata_mv"
 #define DRV_VERSION	"1.26"
 
+/*
+ * module options
+ */
+
+static int msi;
+#ifdef CONFIG_PCI
+module_param(msi, int, S_IRUGO);
+MODULE_PARM_DESC(msi, "Enable use of PCI MSI (0=off, 1=on)");
+#endif
+
 enum {
 	/* BAR's are enumerated in terms of pci_resource_start() terms */
 	MV_PRIMARY_BAR		= 0,	/* offset 0x10: memory space */
@@ -78,12 +89,6 @@
 	MV_MINOR_REG_AREA_SZ	= 0x2000,	/* 8KB */
 
 	MV_PCI_REG_BASE		= 0,
-	MV_IRQ_COAL_REG_BASE	= 0x18000,	/* 6xxx part only */
-	MV_IRQ_COAL_CAUSE		= (MV_IRQ_COAL_REG_BASE + 0x08),
-	MV_IRQ_COAL_CAUSE_LO		= (MV_IRQ_COAL_REG_BASE + 0x88),
-	MV_IRQ_COAL_CAUSE_HI		= (MV_IRQ_COAL_REG_BASE + 0x8c),
-	MV_IRQ_COAL_THRESHOLD		= (MV_IRQ_COAL_REG_BASE + 0xcc),
-	MV_IRQ_COAL_TIME_THRESHOLD	= (MV_IRQ_COAL_REG_BASE + 0xd0),
 
 	MV_SATAHC0_REG_BASE	= 0x20000,
 	MV_FLASH_CTL_OFS	= 0x1046c,
@@ -115,16 +120,14 @@
 
 	/* Host Flags */
 	MV_FLAG_DUAL_HC		= (1 << 30),  /* two SATA Host Controllers */
-	MV_FLAG_IRQ_COALESCE	= (1 << 29),  /* IRQ coalescing capability */
 
 	MV_COMMON_FLAGS		= ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
 				  ATA_FLAG_MMIO | ATA_FLAG_PIO_POLLING,
 
 	MV_GEN_I_FLAGS		= MV_COMMON_FLAGS | ATA_FLAG_NO_ATAPI,
 
-	MV_GEN_II_FLAGS		= MV_COMMON_FLAGS | MV_FLAG_IRQ_COALESCE |
-				  ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
-				  ATA_FLAG_NCQ,
+	MV_GEN_II_FLAGS		= MV_COMMON_FLAGS | ATA_FLAG_NCQ |
+				  ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA,
 
 	MV_GEN_IIE_FLAGS	= MV_GEN_II_FLAGS | ATA_FLAG_AN,
 
@@ -179,16 +182,16 @@
 	PCI_HC_MAIN_IRQ_MASK_OFS  = 0x1d64,
 	SOC_HC_MAIN_IRQ_CAUSE_OFS = 0x20020,
 	SOC_HC_MAIN_IRQ_MASK_OFS  = 0x20024,
-	ERR_IRQ			= (1 << 0),	/* shift by port # */
-	DONE_IRQ		= (1 << 1),	/* shift by port # */
+	ERR_IRQ			= (1 << 0),	/* shift by (2 * port #) */
+	DONE_IRQ		= (1 << 1),	/* shift by (2 * port #) */
 	HC0_IRQ_PEND		= 0x1ff,	/* bits 0-8 = HC0's ports */
 	HC_SHIFT		= 9,		/* bits 9-17 = HC1's ports */
 	PCI_ERR			= (1 << 18),
-	TRAN_LO_DONE		= (1 << 19),	/* 6xxx: IRQ coalescing */
-	TRAN_HI_DONE		= (1 << 20),	/* 6xxx: IRQ coalescing */
-	PORTS_0_3_COAL_DONE	= (1 << 8),
-	PORTS_4_7_COAL_DONE	= (1 << 17),
-	PORTS_0_7_COAL_DONE	= (1 << 21),	/* 6xxx: IRQ coalescing */
+	TRAN_COAL_LO_DONE	= (1 << 19),	/* transaction coalescing */
+	TRAN_COAL_HI_DONE	= (1 << 20),	/* transaction coalescing */
+	PORTS_0_3_COAL_DONE	= (1 << 8),	/* HC0 IRQ coalescing */
+	PORTS_4_7_COAL_DONE	= (1 << 17),	/* HC1 IRQ coalescing */
+	ALL_PORTS_COAL_DONE	= (1 << 21),	/* GEN_II(E) IRQ coalescing */
 	GPIO_INT		= (1 << 22),
 	SELF_INT		= (1 << 23),
 	TWSI_INT		= (1 << 24),
@@ -621,7 +624,7 @@
 	.softreset		= mv_softreset,
 	.error_handler		= mv_pmp_error_handler,
 
- 	.sff_check_status	= mv_sff_check_status,
+	.sff_check_status	= mv_sff_check_status,
 	.sff_irq_clear		= mv_sff_irq_clear,
 	.check_atapi_dma	= mv_check_atapi_dma,
 	.bmdma_setup		= mv_bmdma_setup,
@@ -1255,8 +1258,8 @@
 }
 
 /**
- * 	mv_bmdma_enable - set a magic bit on GEN_IIE to allow bmdma
- * 	@ap: Port being initialized
+ *	mv_bmdma_enable - set a magic bit on GEN_IIE to allow bmdma
+ *	@ap: Port being initialized
  *
  *	There are two DMA modes on these chips:  basic DMA, and EDMA.
  *
@@ -2000,7 +2003,7 @@
 		struct mv_host_priv *hpriv = ap->host->private_data;
 		/*
 		 * Workaround for 88SX60x1 FEr SATA#25 (part 2).
-		 * 
+		 *
 		 * After any NCQ error, the READ_LOG_EXT command
 		 * from libata-eh *must* use mv_qc_issue_fis().
 		 * Otherwise it might fail, due to chip errata.
@@ -3704,12 +3707,6 @@
 	.remove			= ata_pci_remove_one,
 };
 
-/*
- * module options
- */
-static int msi;	      /* Use PCI msi; either zero (off, default) or non-zero */
-
-
 /* move to PCI layer or libata core? */
 static int pci_go_64(struct pci_dev *pdev)
 {
@@ -3891,10 +3888,5 @@
 MODULE_VERSION(DRV_VERSION);
 MODULE_ALIAS("platform:" DRV_NAME);
 
-#ifdef CONFIG_PCI
-module_param(msi, int, 0444);
-MODULE_PARM_DESC(msi, "Enable use of PCI MSI (0=off, 1=on)");
-#endif
-
 module_init(mv_init);
 module_exit(mv_exit);

[PATCH 03/03] sata_mv implement IRQ coalescing

[Mark Lord]

Add IRQ coalescing to sata_mv (off by default).

This feature can reduce total interrupt overhead for RAID setups
in some situations, by deferring the interrupt signal until one or both of:

 a) a specified io_count (completed SATA commands) is achieved, or
 b) a specified time interval elapses after an IO completion.
 
For now, module parameters are used to set the irq_coalescing_io_count
and irq_coalescing_usecs (timeout) globally.  These may eventually
be supplemented with sysfs attributes, so that thresholds can be set
on-the-fly and on a per-chip (or even per-host_controller) basis.

Signed-off-by: Mark Lord <mlord@pobox.com>
---

This is for #upstream libata-dev #upstream

--- old/drivers/ata/sata_mv.c	2009-03-10 18:28:30.000000000 -0400
+++ new/drivers/ata/sata_mv.c	2009-03-10 18:48:07.000000000 -0400
@@ -34,10 +34,7 @@
  *
  * --> Develop a low-power-consumption strategy, and implement it.
  *
- * --> [Experiment, low priority] Investigate interrupt coalescing.
- *       Quite often, especially with PCI Message Signalled Interrupts (MSI),
- *       the overhead reduced by interrupt mitigation is quite often not
- *       worth the latency cost.
+ * --> Add sysfs attributes for per-chip / per-HC IRQ coalescing thresholds.
  *
  * --> [Experiment, Marvell value added] Is it possible to use target
  *       mode to cross-connect two Linux boxes with Marvell cards?  If so,
@@ -67,7 +64,7 @@
 #include <linux/libata.h>
 
 #define DRV_NAME	"sata_mv"
-#define DRV_VERSION	"1.26"
+#define DRV_VERSION	"1.27"
 
 /*
  * module options
@@ -79,6 +76,16 @@
 MODULE_PARM_DESC(msi, "Enable use of PCI MSI (0=off, 1=on)");
 #endif
 
+static int irq_coalescing_io_count;
+module_param(irq_coalescing_io_count, int, S_IRUGO);
+MODULE_PARM_DESC(irq_coalescing_io_count,
+		 "IRQ coalescing I/O count threshold (0..255)");
+
+static int irq_coalescing_usecs;
+module_param(irq_coalescing_usecs, int, S_IRUGO);
+MODULE_PARM_DESC(irq_coalescing_usecs,
+		 "IRQ coalescing time threshold in usecs");
+
 enum {
 	/* BAR's are enumerated in terms of pci_resource_start() terms */
 	MV_PRIMARY_BAR		= 0,	/* offset 0x10: memory space */
@@ -88,8 +95,33 @@
 	MV_MAJOR_REG_AREA_SZ	= 0x10000,	/* 64KB */
 	MV_MINOR_REG_AREA_SZ	= 0x2000,	/* 8KB */
 
+	/* For use with both IRQ coalescing methods ("all ports" or "per-HC" */
+	COAL_CLOCKS_PER_USEC	= 150,		/* for calculating COAL_TIMEs */
+	MAX_COAL_TIME_THRESHOLD	= ((1 << 24) - 1), /* internal clocks count */
+	MAX_COAL_IO_COUNT	= 255,		/* completed I/O count */
+
 	MV_PCI_REG_BASE		= 0,
 
+	/*
+	 * Per-chip ("all ports") interrupt coalescing feature.
+	 * This is only for GEN_II / GEN_IIE hardware.
+	 *
+	 * Coalescing defers the interrupt until either the IO_THRESHOLD
+	 * (count of completed I/Os) is met, or the TIME_THRESHOLD is met.
+	 */
+	MV_COAL_REG_BASE	= 0x18000,
+	MV_IRQ_COAL_CAUSE	= (MV_COAL_REG_BASE + 0x08),
+	ALL_PORTS_IRQ_COAL_IRQ	= (1 << 4),	/* all ports irq event */
+
+	MV_IRQ_COAL_IO_THRESHOLD   = (MV_COAL_REG_BASE + 0xcc),
+	MV_IRQ_COAL_TIME_THRESHOLD = (MV_COAL_REG_BASE + 0xd0),
+
+	/*
+	 * Registers for the (unused here) transaction coalescing feature:
+	 */
+	MV_TRAN_COAL_CAUSE_LO	= (MV_COAL_REG_BASE + 0x88),
+	MV_TRAN_COAL_CAUSE_HI	= (MV_COAL_REG_BASE + 0x8c),
+
 	MV_SATAHC0_REG_BASE	= 0x20000,
 	MV_FLASH_CTL_OFS	= 0x1046c,
 	MV_GPIO_PORT_CTL_OFS	= 0x104f0,
@@ -186,6 +218,8 @@
 	DONE_IRQ		= (1 << 1),	/* shift by (2 * port #) */
 	HC0_IRQ_PEND		= 0x1ff,	/* bits 0-8 = HC0's ports */
 	HC_SHIFT		= 9,		/* bits 9-17 = HC1's ports */
+	DONE_IRQ_0_3		= 0x000000aa,	/* DONE_IRQ ports 0,1,2,3 */
+	DONE_IRQ_4_7		= (DONE_IRQ_0_3 << HC_SHIFT),  /* 4,5,6,7 */
 	PCI_ERR			= (1 << 18),
 	TRAN_COAL_LO_DONE	= (1 << 19),	/* transaction coalescing */
 	TRAN_COAL_HI_DONE	= (1 << 20),	/* transaction coalescing */
@@ -207,6 +241,16 @@
 	HC_COAL_IRQ		= (1 << 4),	/* IRQ coalescing */
 	DEV_IRQ			= (1 << 8),	/* shift by port # */
 
+	/*
+	 * Per-HC (Host-Controller) interrupt coalescing feature.
+	 * This is present on all chip generations.
+	 *
+	 * Coalescing defers the interrupt until either the IO_THRESHOLD
+	 * (count of completed I/Os) is met, or the TIME_THRESHOLD is met.
+	 */
+	HC_IRQ_COAL_IO_THRESHOLD_OFS	= 0x000c,
+	HC_IRQ_COAL_TIME_THRESHOLD_OFS	= 0x0010,
+
 	/* Shadow block registers */
 	SHD_BLK_OFS		= 0x100,
 	SHD_CTL_AST_OFS		= 0x20,		/* ofs from SHD_BLK_OFS */
@@ -897,6 +941,20 @@
 		 port_mmio + EDMA_RSP_Q_OUT_PTR_OFS);
 }
 
+static void mv_write_main_irq_mask(u32 mask, struct mv_host_priv *hpriv)
+{
+	/*
+	 * When writing to the main_irq_mask in hardware,
+	 * we must ensure exclusivity between the interrupt coalescing bits
+	 * and the corresponding individual port DONE_IRQ bits.
+	 */
+	if (mask & (ALL_PORTS_COAL_DONE | PORTS_0_3_COAL_DONE))
+		mask &= ~DONE_IRQ_0_3;
+	if (mask & (ALL_PORTS_COAL_DONE | PORTS_4_7_COAL_DONE))
+		mask &= ~DONE_IRQ_4_7;
+	writelfl(mask, hpriv->main_irq_mask_addr);
+}
+
 static void mv_set_main_irq_mask(struct ata_host *host,
 				 u32 disable_bits, u32 enable_bits)
 {
@@ -907,7 +965,7 @@
 	new_mask = (old_mask & ~disable_bits) | enable_bits;
 	if (new_mask != old_mask) {
 		hpriv->main_irq_mask = new_mask;
-		writelfl(new_mask, hpriv->main_irq_mask_addr);
+		mv_write_main_irq_mask(new_mask, hpriv);
 	}
 }
 
@@ -948,6 +1006,55 @@
 	mv_enable_port_irqs(ap, port_irqs);
 }
 
+static void mv_set_irq_coalescing(struct ata_host *host,
+				  unsigned int count, unsigned int usecs)
+{
+	struct mv_host_priv *hpriv = host->private_data;
+	void __iomem *mmio = hpriv->base;
+	u32 coal_bits;
+	unsigned long flags;
+	unsigned int time = (usecs * COAL_CLOCKS_PER_USEC);
+
+	/* Disable IRQ coalescing if the time threshold is zero */
+	if (!time)
+		count = 0;
+
+	/* Respect maximum limits of the hardware */
+	if (count > MAX_COAL_IO_COUNT)
+		count = MAX_COAL_IO_COUNT;
+	if (time > MAX_COAL_TIME_THRESHOLD)
+		time = MAX_COAL_TIME_THRESHOLD;
+
+	spin_lock_irqsave(&host->lock, flags);
+	if (IS_GEN_I(hpriv)) {
+		/*
+		 * GEN_I: independent thresholds for each HC on the chip.
+		 */
+		void __iomem *hc_mmio = mv_hc_base_from_port(mmio, 0);
+		writel(time,  hc_mmio + HC_IRQ_COAL_TIME_THRESHOLD_OFS);
+		writel(count, hc_mmio + HC_IRQ_COAL_IO_THRESHOLD_OFS);
+		coal_bits = PORTS_0_3_COAL_DONE;
+		if (hpriv->n_ports > 4) {
+			hc_mmio = mv_hc_base_from_port(mmio, MV_PORTS_PER_HC);
+			writel(time,  hc_mmio + HC_IRQ_COAL_TIME_THRESHOLD_OFS);
+			writel(count, hc_mmio + HC_IRQ_COAL_IO_THRESHOLD_OFS);
+			coal_bits |= PORTS_4_7_COAL_DONE;
+		}
+	} else {
+		/*
+		 * GEN_II/GEN_IIE: global thresholds for the entire chip.
+		 */
+		writel(time,  mmio + MV_IRQ_COAL_TIME_THRESHOLD);
+		writel(count, mmio + MV_IRQ_COAL_IO_THRESHOLD);
+		coal_bits = ALL_PORTS_COAL_DONE;
+	}
+	if (time)
+		mv_set_main_irq_mask(host, 0, coal_bits); /* unmask coal irqs */
+	else
+		mv_set_main_irq_mask(host, coal_bits, 0); /* mask coal irqs */
+	spin_unlock_irqrestore(&host->lock, flags);
+}
+
 /**
  *      mv_start_edma - Enable eDMA engine
  *      @base: port base address
@@ -2500,6 +2607,10 @@
 	void __iomem *mmio = hpriv->base, *hc_mmio;
 	unsigned int handled = 0, port;
 
+	/* If asserted, clear the "all ports" IRQ coalescing bit */
+	if (main_irq_cause & ALL_PORTS_COAL_DONE)
+		writel(ALL_PORTS_IRQ_COAL_IRQ, mmio + MV_IRQ_COAL_CAUSE);
+
 	for (port = 0; port < hpriv->n_ports; port++) {
 		struct ata_port *ap = host->ports[port];
 		unsigned int p, shift, hardport, port_cause;
@@ -2531,7 +2642,7 @@
 			 * ports which interrupted us, and use that bitmap
 			 * to ack (only) those ports via hc_irq_cause.
 			 */
-			ack_irqs = 0;
+			ack_irqs = (hc_cause & HC_COAL_IRQ);
 			for (p = 0; p < MV_PORTS_PER_HC; ++p) {
 				if ((port + p) >= hpriv->n_ports)
 					break;
@@ -2620,7 +2731,7 @@
 
 	/* for MSI:  block new interrupts while in here */
 	if (using_msi)
-		writel(0, hpriv->main_irq_mask_addr);
+		mv_write_main_irq_mask(0, hpriv);
 
 	main_irq_cause = readl(hpriv->main_irq_cause_addr);
 	pending_irqs   = main_irq_cause & hpriv->main_irq_mask;
@@ -2637,9 +2748,9 @@
 
 	/* for MSI: unmask; interrupt cause bits will retrigger now */
 	if (using_msi)
-		writel(hpriv->main_irq_mask, hpriv->main_irq_mask_addr);
+		mv_write_main_irq_mask(hpriv->main_irq_mask, hpriv);
 
-	spin_unlock(&host->lock);
+	spin_unlock(&host->lock);	/* FIXME: broken in Linus tree? */
 
 	return IRQ_RETVAL(handled);
 }
@@ -3546,6 +3657,8 @@
 	 * The per-port interrupts get done later as ports are set up.
 	 */
 	mv_set_main_irq_mask(host, 0, PCI_ERR);
+	mv_set_irq_coalescing(host, irq_coalescing_io_count,
+				    irq_coalescing_usecs);
 done:
 	return rc;
 }

Re: [PATCH 03/03] sata_mv implement IRQ coalescing

[Grant Grundler]

On Tue, Mar 10, 2009 at 11:52 PM, Mark Lord <liml@rtr.ca> wrote:
> Add IRQ coalescing to sata_mv (off by default).

Excellent!

> This feature can reduce total interrupt overhead for RAID setups

Just to be clear, this is referring to SW RAID (e.g. md or dm) and not
some feature in this SATA controller. I'll point out even without SW RAID,
this feature can helps reduce interrupts even if only one port is in use.

(NCQ allows many commands outstanding and it's possible to defer
generating an interrupt until several commands have completed. Very
similar to how NICs operate by default).

> in some situations, by deferring the interrupt signal until one or both of:
>
> a) a specified io_count (completed SATA commands) is achieved, or
> b) a specified time interval elapses after an IO completion.
>
> For now, module parameters are used to set the irq_coalescing_io_count
> and irq_coalescing_usecs (timeout) globally.  These may eventually
> be supplemented with sysfs attributes, so that thresholds can be set
> on-the-fly and on a per-chip (or even per-host_controller) basis.
>
> Signed-off-by: Mark Lord <mlord@pobox.com>

Reviewed-by: Grant Grundler <grundler@google.com>

thanks,
grant

> ---
>
> This is for #upstream libata-dev #upstream
>
> --- old/drivers/ata/sata_mv.c   2009-03-10 18:28:30.000000000 -0400
> +++ new/drivers/ata/sata_mv.c   2009-03-10 18:48:07.000000000 -0400
> @@ -34,10 +34,7 @@
>  *
>  * --> Develop a low-power-consumption strategy, and implement it.
>  *
> - * --> [Experiment, low priority] Investigate interrupt coalescing.
> - *       Quite often, especially with PCI Message Signalled Interrupts
> (MSI),
> - *       the overhead reduced by interrupt mitigation is quite often not
> - *       worth the latency cost.
> + * --> Add sysfs attributes for per-chip / per-HC IRQ coalescing
> thresholds.
>  *
>  * --> [Experiment, Marvell value added] Is it possible to use target
>  *       mode to cross-connect two Linux boxes with Marvell cards?  If so,
> @@ -67,7 +64,7 @@
> #include <linux/libata.h>
>
> #define DRV_NAME        "sata_mv"
> -#define DRV_VERSION    "1.26"
> +#define DRV_VERSION    "1.27"
>
> /*
>  * module options
> @@ -79,6 +76,16 @@
> MODULE_PARM_DESC(msi, "Enable use of PCI MSI (0=off, 1=on)");
> #endif
>
> +static int irq_coalescing_io_count;
> +module_param(irq_coalescing_io_count, int, S_IRUGO);
> +MODULE_PARM_DESC(irq_coalescing_io_count,
> +                "IRQ coalescing I/O count threshold (0..255)");
> +
> +static int irq_coalescing_usecs;
> +module_param(irq_coalescing_usecs, int, S_IRUGO);
> +MODULE_PARM_DESC(irq_coalescing_usecs,
> +                "IRQ coalescing time threshold in usecs");
> +
> enum {
>        /* BAR's are enumerated in terms of pci_resource_start() terms */
>        MV_PRIMARY_BAR          = 0,    /* offset 0x10: memory space */
> @@ -88,8 +95,33 @@
>        MV_MAJOR_REG_AREA_SZ    = 0x10000,      /* 64KB */
>        MV_MINOR_REG_AREA_SZ    = 0x2000,       /* 8KB */
>
> +       /* For use with both IRQ coalescing methods ("all ports" or "per-HC"
> */
> +       COAL_CLOCKS_PER_USEC    = 150,          /* for calculating
> COAL_TIMEs */
> +       MAX_COAL_TIME_THRESHOLD = ((1 << 24) - 1), /* internal clocks count
> */
> +       MAX_COAL_IO_COUNT       = 255,          /* completed I/O count */
> +
>        MV_PCI_REG_BASE         = 0,
>
> +       /*
> +        * Per-chip ("all ports") interrupt coalescing feature.
> +        * This is only for GEN_II / GEN_IIE hardware.
> +        *
> +        * Coalescing defers the interrupt until either the IO_THRESHOLD
> +        * (count of completed I/Os) is met, or the TIME_THRESHOLD is met.
> +        */
> +       MV_COAL_REG_BASE        = 0x18000,
> +       MV_IRQ_COAL_CAUSE       = (MV_COAL_REG_BASE + 0x08),
> +       ALL_PORTS_IRQ_COAL_IRQ  = (1 << 4),     /* all ports irq event */
> +
> +       MV_IRQ_COAL_IO_THRESHOLD   = (MV_COAL_REG_BASE + 0xcc),
> +       MV_IRQ_COAL_TIME_THRESHOLD = (MV_COAL_REG_BASE + 0xd0),
> +
> +       /*
> +        * Registers for the (unused here) transaction coalescing feature:
> +        */
> +       MV_TRAN_COAL_CAUSE_LO   = (MV_COAL_REG_BASE + 0x88),
> +       MV_TRAN_COAL_CAUSE_HI   = (MV_COAL_REG_BASE + 0x8c),
> +
>        MV_SATAHC0_REG_BASE     = 0x20000,
>        MV_FLASH_CTL_OFS        = 0x1046c,
>        MV_GPIO_PORT_CTL_OFS    = 0x104f0,
> @@ -186,6 +218,8 @@
>        DONE_IRQ                = (1 << 1),     /* shift by (2 * port #) */
>        HC0_IRQ_PEND            = 0x1ff,        /* bits 0-8 = HC0's ports */
>        HC_SHIFT                = 9,            /* bits 9-17 = HC1's ports */
> +       DONE_IRQ_0_3            = 0x000000aa,   /* DONE_IRQ ports 0,1,2,3 */
> +       DONE_IRQ_4_7            = (DONE_IRQ_0_3 << HC_SHIFT),  /* 4,5,6,7 */
>        PCI_ERR                 = (1 << 18),
>        TRAN_COAL_LO_DONE       = (1 << 19),    /* transaction coalescing */
>        TRAN_COAL_HI_DONE       = (1 << 20),    /* transaction coalescing */
> @@ -207,6 +241,16 @@
>        HC_COAL_IRQ             = (1 << 4),     /* IRQ coalescing */
>        DEV_IRQ                 = (1 << 8),     /* shift by port # */
>
> +       /*
> +        * Per-HC (Host-Controller) interrupt coalescing feature.
> +        * This is present on all chip generations.
> +        *
> +        * Coalescing defers the interrupt until either the IO_THRESHOLD
> +        * (count of completed I/Os) is met, or the TIME_THRESHOLD is met.
> +        */
> +       HC_IRQ_COAL_IO_THRESHOLD_OFS    = 0x000c,
> +       HC_IRQ_COAL_TIME_THRESHOLD_OFS  = 0x0010,
> +
>        /* Shadow block registers */
>        SHD_BLK_OFS             = 0x100,
>        SHD_CTL_AST_OFS         = 0x20,         /* ofs from SHD_BLK_OFS */
> @@ -897,6 +941,20 @@
>                 port_mmio + EDMA_RSP_Q_OUT_PTR_OFS);
> }
>
> +static void mv_write_main_irq_mask(u32 mask, struct mv_host_priv *hpriv)
> +{
> +       /*
> +        * When writing to the main_irq_mask in hardware,
> +        * we must ensure exclusivity between the interrupt coalescing bits
> +        * and the corresponding individual port DONE_IRQ bits.
> +        */
> +       if (mask & (ALL_PORTS_COAL_DONE | PORTS_0_3_COAL_DONE))
> +               mask &= ~DONE_IRQ_0_3;
> +       if (mask & (ALL_PORTS_COAL_DONE | PORTS_4_7_COAL_DONE))
> +               mask &= ~DONE_IRQ_4_7;
> +       writelfl(mask, hpriv->main_irq_mask_addr);
> +}
> +
> static void mv_set_main_irq_mask(struct ata_host *host,
>                                 u32 disable_bits, u32 enable_bits)
> {
> @@ -907,7 +965,7 @@
>        new_mask = (old_mask & ~disable_bits) | enable_bits;
>        if (new_mask != old_mask) {
>                hpriv->main_irq_mask = new_mask;
> -               writelfl(new_mask, hpriv->main_irq_mask_addr);
> +               mv_write_main_irq_mask(new_mask, hpriv);
>        }
> }
>
> @@ -948,6 +1006,55 @@
>        mv_enable_port_irqs(ap, port_irqs);
> }
>
> +static void mv_set_irq_coalescing(struct ata_host *host,
> +                                 unsigned int count, unsigned int usecs)
> +{
> +       struct mv_host_priv *hpriv = host->private_data;
> +       void __iomem *mmio = hpriv->base;
> +       u32 coal_bits;
> +       unsigned long flags;
> +       unsigned int time = (usecs * COAL_CLOCKS_PER_USEC);
> +
> +       /* Disable IRQ coalescing if the time threshold is zero */
> +       if (!time)
> +               count = 0;
> +
> +       /* Respect maximum limits of the hardware */
> +       if (count > MAX_COAL_IO_COUNT)
> +               count = MAX_COAL_IO_COUNT;
> +       if (time > MAX_COAL_TIME_THRESHOLD)
> +               time = MAX_COAL_TIME_THRESHOLD;
> +
> +       spin_lock_irqsave(&host->lock, flags);
> +       if (IS_GEN_I(hpriv)) {
> +               /*
> +                * GEN_I: independent thresholds for each HC on the chip.
> +                */
> +               void __iomem *hc_mmio = mv_hc_base_from_port(mmio, 0);
> +               writel(time,  hc_mmio + HC_IRQ_COAL_TIME_THRESHOLD_OFS);
> +               writel(count, hc_mmio + HC_IRQ_COAL_IO_THRESHOLD_OFS);
> +               coal_bits = PORTS_0_3_COAL_DONE;
> +               if (hpriv->n_ports > 4) {
> +                       hc_mmio = mv_hc_base_from_port(mmio,
> MV_PORTS_PER_HC);
> +                       writel(time,  hc_mmio +
> HC_IRQ_COAL_TIME_THRESHOLD_OFS);
> +                       writel(count, hc_mmio +
> HC_IRQ_COAL_IO_THRESHOLD_OFS);
> +                       coal_bits |= PORTS_4_7_COAL_DONE;
> +               }
> +       } else {
> +               /*
> +                * GEN_II/GEN_IIE: global thresholds for the entire chip.
> +                */
> +               writel(time,  mmio + MV_IRQ_COAL_TIME_THRESHOLD);
> +               writel(count, mmio + MV_IRQ_COAL_IO_THRESHOLD);
> +               coal_bits = ALL_PORTS_COAL_DONE;
> +       }
> +       if (time)
> +               mv_set_main_irq_mask(host, 0, coal_bits); /* unmask coal
> irqs */
> +       else
> +               mv_set_main_irq_mask(host, coal_bits, 0); /* mask coal irqs
> */
> +       spin_unlock_irqrestore(&host->lock, flags);
> +}
> +
> /**
>  *      mv_start_edma - Enable eDMA engine
>  *      @base: port base address
> @@ -2500,6 +2607,10 @@
>        void __iomem *mmio = hpriv->base, *hc_mmio;
>        unsigned int handled = 0, port;
>
> +       /* If asserted, clear the "all ports" IRQ coalescing bit */
> +       if (main_irq_cause & ALL_PORTS_COAL_DONE)
> +               writel(ALL_PORTS_IRQ_COAL_IRQ, mmio + MV_IRQ_COAL_CAUSE);
> +
>        for (port = 0; port < hpriv->n_ports; port++) {
>                struct ata_port *ap = host->ports[port];
>                unsigned int p, shift, hardport, port_cause;
> @@ -2531,7 +2642,7 @@
>                         * ports which interrupted us, and use that bitmap
>                         * to ack (only) those ports via hc_irq_cause.
>                         */
> -                       ack_irqs = 0;
> +                       ack_irqs = (hc_cause & HC_COAL_IRQ);
>                        for (p = 0; p < MV_PORTS_PER_HC; ++p) {
>                                if ((port + p) >= hpriv->n_ports)
>                                        break;
> @@ -2620,7 +2731,7 @@
>
>        /* for MSI:  block new interrupts while in here */
>        if (using_msi)
> -               writel(0, hpriv->main_irq_mask_addr);
> +               mv_write_main_irq_mask(0, hpriv);
>
>        main_irq_cause = readl(hpriv->main_irq_cause_addr);
>        pending_irqs   = main_irq_cause & hpriv->main_irq_mask;
> @@ -2637,9 +2748,9 @@
>
>        /* for MSI: unmask; interrupt cause bits will retrigger now */
>        if (using_msi)
> -               writel(hpriv->main_irq_mask, hpriv->main_irq_mask_addr);
> +               mv_write_main_irq_mask(hpriv->main_irq_mask, hpriv);
>
> -       spin_unlock(&host->lock);
> +       spin_unlock(&host->lock);       /* FIXME: broken in Linus tree? */
>
>        return IRQ_RETVAL(handled);
> }
> @@ -3546,6 +3657,8 @@
>         * The per-port interrupts get done later as ports are set up.
>         */
>        mv_set_main_irq_mask(host, 0, PCI_ERR);
> +       mv_set_irq_coalescing(host, irq_coalescing_io_count,
> +                                   irq_coalescing_usecs);
> done:
>        return rc;
> }
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ide" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: [PATCH 03/03] sata_mv implement IRQ coalescing

[Mark Lord]

Mark Lord wrote:
> Add IRQ coalescing to sata_mv (off by default).
> 
> This feature can reduce total interrupt overhead for RAID setups
> in some situations, by deferring the interrupt signal until one or both of:
> 
> a) a specified io_count (completed SATA commands) is achieved, or
> b) a specified time interval elapses after an IO completion.
> 
> For now, module parameters are used to set the irq_coalescing_io_count
> and irq_coalescing_usecs (timeout) globally.  These may eventually
> be supplemented with sysfs attributes, so that thresholds can be set
> on-the-fly and on a per-chip (or even per-host_controller) basis.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>
> ---
> 
> This is for #upstream libata-dev #upstream
> 
> --- old/drivers/ata/sata_mv.c    2009-03-10 18:28:30.000000000 -0400
> +++ new/drivers/ata/sata_mv.c    2009-03-10 18:48:07.000000000 -0400
> @@ -34,10 +34,7 @@
..

Ooops.. hold off on 03/03 for the moment.
I messed up a couple of bits.. will repost (v2) shortly.

-ml

[PATCH 03/03] sata_mv implement IRQ coalescing (v2)

[Mark Lord]

Add IRQ coalescing to sata_mv (off by default).

This feature can reduce total interrupt overhead for RAID setups
in some situations, by deferring the interrupt signal until one or both of:

 a) a specified io_count (completed SATA commands) is achieved, or
 b) a specified time interval elapses after an IO completion.
 
For now, module parameters are used to set the irq_coalescing_io_count
and irq_coalescing_usecs (timeout) globally.  These may eventually
be supplemented with sysfs attributes, so that thresholds can be set
on-the-fly and on a per-chip (or even per-host_controller) basis.

Signed-off-by: Mark Lord <mlord@pobox.com>
---

This (v2) replaces the original 03/03 patch.
This is for #upstream libata-dev #upstream

--- old/drivers/ata/sata_mv.c	2009-03-10 18:28:30.000000000 -0400
+++ new/drivers/ata/sata_mv.c	2009-03-10 21:30:27.000000000 -0400
@@ -34,10 +34,7 @@
  *
  * --> Develop a low-power-consumption strategy, and implement it.
  *
- * --> [Experiment, low priority] Investigate interrupt coalescing.
- *       Quite often, especially with PCI Message Signalled Interrupts (MSI),
- *       the overhead reduced by interrupt mitigation is quite often not
- *       worth the latency cost.
+ * --> Add sysfs attributes for per-chip / per-HC IRQ coalescing thresholds.
  *
  * --> [Experiment, Marvell value added] Is it possible to use target
  *       mode to cross-connect two Linux boxes with Marvell cards?  If so,
@@ -67,7 +64,7 @@
 #include <linux/libata.h>
 
 #define DRV_NAME	"sata_mv"
-#define DRV_VERSION	"1.26"
+#define DRV_VERSION	"1.27"
 
 /*
  * module options
@@ -79,6 +76,16 @@
 MODULE_PARM_DESC(msi, "Enable use of PCI MSI (0=off, 1=on)");
 #endif
 
+static int irq_coalescing_io_count;
+module_param(irq_coalescing_io_count, int, S_IRUGO);
+MODULE_PARM_DESC(irq_coalescing_io_count,
+		 "IRQ coalescing I/O count threshold (0..255)");
+
+static int irq_coalescing_usecs;
+module_param(irq_coalescing_usecs, int, S_IRUGO);
+MODULE_PARM_DESC(irq_coalescing_usecs,
+		 "IRQ coalescing time threshold in usecs");
+
 enum {
 	/* BAR's are enumerated in terms of pci_resource_start() terms */
 	MV_PRIMARY_BAR		= 0,	/* offset 0x10: memory space */
@@ -88,8 +95,33 @@
 	MV_MAJOR_REG_AREA_SZ	= 0x10000,	/* 64KB */
 	MV_MINOR_REG_AREA_SZ	= 0x2000,	/* 8KB */
 
+	/* For use with both IRQ coalescing methods ("all ports" or "per-HC" */
+	COAL_CLOCKS_PER_USEC	= 150,		/* for calculating COAL_TIMEs */
+	MAX_COAL_TIME_THRESHOLD	= ((1 << 24) - 1), /* internal clocks count */
+	MAX_COAL_IO_COUNT	= 255,		/* completed I/O count */
+
 	MV_PCI_REG_BASE		= 0,
 
+	/*
+	 * Per-chip ("all ports") interrupt coalescing feature.
+	 * This is only for GEN_II / GEN_IIE hardware.
+	 *
+	 * Coalescing defers the interrupt until either the IO_THRESHOLD
+	 * (count of completed I/Os) is met, or the TIME_THRESHOLD is met.
+	 */
+	MV_COAL_REG_BASE	= 0x18000,
+	MV_IRQ_COAL_CAUSE	= (MV_COAL_REG_BASE + 0x08),
+	ALL_PORTS_COAL_IRQ	= (1 << 4),	/* all ports irq event */
+
+	MV_IRQ_COAL_IO_THRESHOLD   = (MV_COAL_REG_BASE + 0xcc),
+	MV_IRQ_COAL_TIME_THRESHOLD = (MV_COAL_REG_BASE + 0xd0),
+
+	/*
+	 * Registers for the (unused here) transaction coalescing feature:
+	 */
+	MV_TRAN_COAL_CAUSE_LO	= (MV_COAL_REG_BASE + 0x88),
+	MV_TRAN_COAL_CAUSE_HI	= (MV_COAL_REG_BASE + 0x8c),
+
 	MV_SATAHC0_REG_BASE	= 0x20000,
 	MV_FLASH_CTL_OFS	= 0x1046c,
 	MV_GPIO_PORT_CTL_OFS	= 0x104f0,
@@ -186,6 +218,8 @@
 	DONE_IRQ		= (1 << 1),	/* shift by (2 * port #) */
 	HC0_IRQ_PEND		= 0x1ff,	/* bits 0-8 = HC0's ports */
 	HC_SHIFT		= 9,		/* bits 9-17 = HC1's ports */
+	DONE_IRQ_0_3		= 0x000000aa,	/* DONE_IRQ ports 0,1,2,3 */
+	DONE_IRQ_4_7		= (DONE_IRQ_0_3 << HC_SHIFT),  /* 4,5,6,7 */
 	PCI_ERR			= (1 << 18),
 	TRAN_COAL_LO_DONE	= (1 << 19),	/* transaction coalescing */
 	TRAN_COAL_HI_DONE	= (1 << 20),	/* transaction coalescing */
@@ -207,6 +241,16 @@
 	HC_COAL_IRQ		= (1 << 4),	/* IRQ coalescing */
 	DEV_IRQ			= (1 << 8),	/* shift by port # */
 
+	/*
+	 * Per-HC (Host-Controller) interrupt coalescing feature.
+	 * This is present on all chip generations.
+	 *
+	 * Coalescing defers the interrupt until either the IO_THRESHOLD
+	 * (count of completed I/Os) is met, or the TIME_THRESHOLD is met.
+	 */
+	HC_IRQ_COAL_IO_THRESHOLD_OFS	= 0x000c,
+	HC_IRQ_COAL_TIME_THRESHOLD_OFS	= 0x0010,
+
 	/* Shadow block registers */
 	SHD_BLK_OFS		= 0x100,
 	SHD_CTL_AST_OFS		= 0x20,		/* ofs from SHD_BLK_OFS */
@@ -897,6 +941,23 @@
 		 port_mmio + EDMA_RSP_Q_OUT_PTR_OFS);
 }
 
+static void mv_write_main_irq_mask(u32 mask, struct mv_host_priv *hpriv)
+{
+	/*
+	 * When writing to the main_irq_mask in hardware,
+	 * we must ensure exclusivity between the interrupt coalescing bits
+	 * and the corresponding individual port DONE_IRQ bits.
+	 *
+	 * Note that this register is really an "IRQ enable" register,
+	 * not an "IRQ mask" register as Marvell's naming might suggest.
+	 */
+	if (mask & (ALL_PORTS_COAL_DONE | PORTS_0_3_COAL_DONE))
+		mask &= ~DONE_IRQ_0_3;
+	if (mask & (ALL_PORTS_COAL_DONE | PORTS_4_7_COAL_DONE))
+		mask &= ~DONE_IRQ_4_7;
+	writelfl(mask, hpriv->main_irq_mask_addr);
+}
+
 static void mv_set_main_irq_mask(struct ata_host *host,
 				 u32 disable_bits, u32 enable_bits)
 {
@@ -907,7 +968,7 @@
 	new_mask = (old_mask & ~disable_bits) | enable_bits;
 	if (new_mask != old_mask) {
 		hpriv->main_irq_mask = new_mask;
-		writelfl(new_mask, hpriv->main_irq_mask_addr);
+		mv_write_main_irq_mask(new_mask, hpriv);
 	}
 }
 
@@ -948,6 +1009,64 @@
 	mv_enable_port_irqs(ap, port_irqs);
 }
 
+static void mv_set_irq_coalescing(struct ata_host *host,
+				  unsigned int count, unsigned int usecs)
+{
+	struct mv_host_priv *hpriv = host->private_data;
+	void __iomem *mmio = hpriv->base, *hc_mmio;
+	u32 coal_enable = 0;
+	unsigned long flags;
+	unsigned int clks;
+	const u32 coal_disable = PORTS_0_3_COAL_DONE | PORTS_4_7_COAL_DONE |
+							ALL_PORTS_COAL_DONE;
+
+	/* Disable IRQ coalescing if either threshold is zero */
+	if (!usecs || !count) {
+		clks = count = 0;
+	} else {
+		/* Respect maximum limits of the hardware */
+		clks = usecs * COAL_CLOCKS_PER_USEC;
+		if (clks > MAX_COAL_TIME_THRESHOLD)
+			clks = MAX_COAL_TIME_THRESHOLD;
+		if (count > MAX_COAL_IO_COUNT)
+			count = MAX_COAL_IO_COUNT;
+	}
+
+	spin_lock_irqsave(&host->lock, flags);
+
+#if 0 /* disabled pending functional clarification from Marvell */
+	if (!IS_GEN_I(hpriv)) {
+		/*
+		 * GEN_II/GEN_IIE: global thresholds for the entire chip.
+		 */
+		writel(clks,  mmio + MV_IRQ_COAL_TIME_THRESHOLD);
+		writel(count, mmio + MV_IRQ_COAL_IO_THRESHOLD);
+		/* clear leftover coal IRQ bit */
+		writelfl(~ALL_PORTS_COAL_IRQ, mmio + MV_IRQ_COAL_CAUSE);
+		clks = count = 0; /* so as to clear the alternate regs below */
+		coal_enable = ALL_PORTS_COAL_DONE;
+	}
+#endif
+	/*
+	 * All chips: independent thresholds for each HC on the chip.
+	 */
+	hc_mmio = mv_hc_base_from_port(mmio, 0);
+	writel(clks,  hc_mmio + HC_IRQ_COAL_TIME_THRESHOLD_OFS);
+	writel(count, hc_mmio + HC_IRQ_COAL_IO_THRESHOLD_OFS);
+	coal_enable |= PORTS_0_3_COAL_DONE;
+	if (hpriv->n_ports > 4) {
+		hc_mmio = mv_hc_base_from_port(mmio, MV_PORTS_PER_HC);
+		writel(clks,  hc_mmio + HC_IRQ_COAL_TIME_THRESHOLD_OFS);
+		writel(count, hc_mmio + HC_IRQ_COAL_IO_THRESHOLD_OFS);
+		coal_enable |= PORTS_4_7_COAL_DONE;
+	}
+	if (!count)
+		coal_enable = 0;
+	mv_set_main_irq_mask(host, coal_disable, coal_enable);
+
+	spin_unlock_irqrestore(&host->lock, flags);
+}
+
 /**
  *      mv_start_edma - Enable eDMA engine
  *      @base: port base address
@@ -2500,6 +2619,10 @@
 	void __iomem *mmio = hpriv->base, *hc_mmio;
 	unsigned int handled = 0, port;
 
+	/* If asserted, clear the "all ports" IRQ coalescing bit */
+	if (main_irq_cause & ALL_PORTS_COAL_DONE)
+		writel(~ALL_PORTS_COAL_IRQ, mmio + MV_IRQ_COAL_CAUSE);
+
 	for (port = 0; port < hpriv->n_ports; port++) {
 		struct ata_port *ap = host->ports[port];
 		unsigned int p, shift, hardport, port_cause;
@@ -2532,6 +2655,8 @@
 			 * to ack (only) those ports via hc_irq_cause.
 			 */
 			ack_irqs = 0;
+			if (hc_cause & PORTS_0_3_COAL_DONE)
+				ack_irqs = HC_COAL_IRQ;
 			for (p = 0; p < MV_PORTS_PER_HC; ++p) {
 				if ((port + p) >= hpriv->n_ports)
 					break;
@@ -2620,7 +2745,7 @@
 
 	/* for MSI:  block new interrupts while in here */
 	if (using_msi)
-		writel(0, hpriv->main_irq_mask_addr);
+		mv_write_main_irq_mask(0, hpriv);
 
 	main_irq_cause = readl(hpriv->main_irq_cause_addr);
 	pending_irqs   = main_irq_cause & hpriv->main_irq_mask;
@@ -2637,7 +2762,7 @@
 
 	/* for MSI: unmask; interrupt cause bits will retrigger now */
 	if (using_msi)
-		writel(hpriv->main_irq_mask, hpriv->main_irq_mask_addr);
+		mv_write_main_irq_mask(hpriv->main_irq_mask, hpriv);
 
 	spin_unlock(&host->lock);
 
@@ -3546,6 +3671,8 @@
 	 * The per-port interrupts get done later as ports are set up.
 	 */
 	mv_set_main_irq_mask(host, 0, PCI_ERR);
+	mv_set_irq_coalescing(host, irq_coalescing_io_count,
+				    irq_coalescing_usecs);
 done:
 	return rc;
 }

[PATCH 04/03] sata_mv optimize IRQ coalescing for 8-port chips

[Mark Lord]

Enable use of the "all ports" IRQ coalescing optimization
for GEN_II / GEN_IIE chips that have dual host-controllers (8-ports).
Currently only the 6081 chip qualifies, but other chips may come along someday.

Rather than each half of the chip having to satisfy a local set of coalescing thresholds,
use of this feature groups all ports together under a single set of thresholds.

Signed-off-by: Mark Lord <mlord@pobox.com>
---

This is a follow-up to the earlier 3-patch series for sata_mv IRQ coalescing.

--- old/drivers/ata/sata_mv.c	2009-03-11 00:47:57.000000000 -0400
+++ new/drivers/ata/sata_mv.c	2009-03-11 00:50:48.000000000 -0400
@@ -1016,7 +1016,7 @@
 	void __iomem *mmio = hpriv->base, *hc_mmio;
 	u32 coal_enable = 0;
 	unsigned long flags;
-	unsigned int clks;
+	unsigned int clks, is_dual_hc = hpriv->n_ports > MV_PORTS_PER_HC;
 	const u32 coal_disable = PORTS_0_3_COAL_DONE | PORTS_4_7_COAL_DONE |
 							ALL_PORTS_COAL_DONE;
 
@@ -1033,37 +1033,41 @@
 	}
 
 	spin_lock_irqsave(&host->lock, flags);
+	mv_set_main_irq_mask(host, coal_disable, 0);
 
-#if 0 /* disabled pending functional clarification from Marvell */
-	if (!IS_GEN_I(hpriv)) {
+	if (is_dual_hc && !IS_GEN_I(hpriv)) {
 		/*
-		 * GEN_II/GEN_IIE: global thresholds for the entire chip.
+		 * GEN_II/GEN_IIE with dual host controllers:
+		 * one set of global thresholds for the entire chip.
 		 */
 		writel(clks,  mmio + MV_IRQ_COAL_TIME_THRESHOLD);
 		writel(count, mmio + MV_IRQ_COAL_IO_THRESHOLD);
 		/* clear leftover coal IRQ bit */
-		writelfl(~ALL_PORTS_COAL_IRQ, mmio + MV_IRQ_COAL_CAUSE);
-		clks = count = 0; /* so as to clear the alternate regs below */
-		coal_enable = ALL_PORTS_COAL_DONE;
+		writel(~ALL_PORTS_COAL_IRQ, mmio + MV_IRQ_COAL_CAUSE);
+		if (count)
+			coal_enable = ALL_PORTS_COAL_DONE;
+		clks = count = 0; /* force clearing of regular regs below */
 	}
-#endif
+
 	/*
 	 * All chips: independent thresholds for each HC on the chip.
 	 */
 	hc_mmio = mv_hc_base_from_port(mmio, 0);
 	writel(clks,  hc_mmio + HC_IRQ_COAL_TIME_THRESHOLD_OFS);
 	writel(count, hc_mmio + HC_IRQ_COAL_IO_THRESHOLD_OFS);
-	coal_enable |= PORTS_0_3_COAL_DONE;
-	if (hpriv->n_ports > 4) {
+	writel(~HC_COAL_IRQ, hc_mmio + HC_IRQ_CAUSE_OFS);
+	if (count)
+		coal_enable |= PORTS_0_3_COAL_DONE;
+	if (is_dual_hc) {
 		hc_mmio = mv_hc_base_from_port(mmio, MV_PORTS_PER_HC);
 		writel(clks,  hc_mmio + HC_IRQ_COAL_TIME_THRESHOLD_OFS);
 		writel(count, hc_mmio + HC_IRQ_COAL_IO_THRESHOLD_OFS);
-		coal_enable |= PORTS_4_7_COAL_DONE;
+		writel(~HC_COAL_IRQ, hc_mmio + HC_IRQ_CAUSE_OFS);
+		if (count)
+			coal_enable |= PORTS_4_7_COAL_DONE;
 	}
-	if (!count)
-		coal_enable = 0;
-	mv_set_main_irq_mask(host, coal_disable, coal_enable);
 
+	mv_set_main_irq_mask(host, 0, coal_enable);
 	spin_unlock_irqrestore(&host->lock, flags);
 }
 

[PATCH 05/03] sata_mv fix LED blinking for SoC+NCQ

[Mark Lord]

For Marvell SoC chips, the HDD LED does not blink when there is
disk I/O if NCQ is enabled. Add a quirk that enables blink mode for
the LED while NCQ is enabled on any port of a SoC host controller.
Normal LED function is restored when NCQ is not enabled on any port.

The code to enable the blink mode is based on earlier code
and suggestions from Frans Pop, Saeed Bishara, and possibly others.

Signed-off-by: Mark Lord <mlord@pobox.com>
Tested-by: Frans Pop <elendil@planet.nl>

--- old/drivers/ata/sata_mv.c	2009-03-11 00:50:48.000000000 -0400
+++ new/drivers/ata/sata_mv.c	2009-03-11 10:13:29.000000000 -0400
@@ -251,6 +251,11 @@
 	HC_IRQ_COAL_IO_THRESHOLD_OFS	= 0x000c,
 	HC_IRQ_COAL_TIME_THRESHOLD_OFS	= 0x0010,
 
+	SOC_LED_CTRL_OFS	= 0x2c,
+	SOC_LED_CTRL_BLINK	= (1 << 0),	/* Active LED blink */
+	SOC_LED_CTRL_ACT_PRESENCE = (1 << 2),	/* Multiplex dev presence */
+						/*  with dev activity LED */
+
 	/* Shadow block registers */
 	SHD_BLK_OFS		= 0x100,
 	SHD_CTL_AST_OFS		= 0x20,		/* ofs from SHD_BLK_OFS */
@@ -411,6 +416,7 @@
 	MV_HP_PCIE		= (1 << 9),	/* PCIe bus/regs: 7042 */
 	MV_HP_CUT_THROUGH	= (1 << 10),	/* can use EDMA cut-through */
 	MV_HP_FLAG_SOC		= (1 << 11),	/* SystemOnChip, no PCI */
+	MV_HP_QUIRK_LED_BLINK_EN = (1 << 12),	/* is led blinking enabled? */
 
 	/* Port private flags (pp_flags) */
 	MV_PP_FLAG_EDMA_EN	= (1 << 0),	/* is EDMA engine enabled? */
@@ -1404,6 +1410,61 @@
 	mv_write_cached_reg(mv_ap_base(ap) + EDMA_UNKNOWN_RSVD_OFS, old, new);
 }
 
+/*
+ * SOC chips have an issue whereby the HDD LEDs don't always blink
+ * during I/O when NCQ is enabled. Enabling a special "LED blink" mode
+ * of the SOC takes care of it, generating a steady blink rate when
+ * any drive on the chip is active.
+ *
+ * Unfortunately, the blink mode is a global hardware setting for the SOC,
+ * so we must use it whenever at least one port on the SOC has NCQ enabled.
+ *
+ * We turn "LED blink" off when NCQ is not in use anywhere, because the normal
+ * LED operation works then, and provides better (more accurate) feedback.
+ *
+ * Note that this code assumes that an SOC never has more than one HC onboard.
+ */
+static void mv_soc_led_blink_enable(struct ata_port *ap)
+{
+	struct ata_host *host = ap->host;
+	struct mv_host_priv *hpriv = host->private_data;
+	void __iomem *hc_mmio;
+	u32 led_ctrl;
+
+	if (hpriv->hp_flags & MV_HP_QUIRK_LED_BLINK_EN)
+		return;
+	hpriv->hp_flags |= MV_HP_QUIRK_LED_BLINK_EN;
+	hc_mmio = mv_hc_base_from_port(mv_host_base(host), ap->port_no);
+	led_ctrl = readl(hc_mmio + SOC_LED_CTRL_OFS);
+	writel(led_ctrl | SOC_LED_CTRL_BLINK, hc_mmio + SOC_LED_CTRL_OFS);
+}
+
+static void mv_soc_led_blink_disable(struct ata_port *ap)
+{
+	struct ata_host *host = ap->host;
+	struct mv_host_priv *hpriv = host->private_data;
+	void __iomem *hc_mmio;
+	u32 led_ctrl;
+	unsigned int port;
+
+	if (!(hpriv->hp_flags & MV_HP_QUIRK_LED_BLINK_EN))
+		return;
+
+	/* disable led-blink only if no ports are using NCQ */
+	for (port = 0; port < hpriv->n_ports; port++) {
+		struct ata_port *this_ap = host->ports[port];
+		struct mv_port_priv *pp = this_ap->private_data;
+
+		if (pp->pp_flags & MV_PP_FLAG_NCQ_EN)
+			return;
+	}
+
+	hpriv->hp_flags &= ~MV_HP_QUIRK_LED_BLINK_EN;
+	hc_mmio = mv_hc_base_from_port(mv_host_base(host), ap->port_no);
+	led_ctrl = readl(hc_mmio + SOC_LED_CTRL_OFS);
+	writel(led_ctrl & ~SOC_LED_CTRL_BLINK, hc_mmio + SOC_LED_CTRL_OFS);
+}
+
 static void mv_edma_cfg(struct ata_port *ap, int want_ncq, int want_edma)
 {
 	u32 cfg;
@@ -1451,6 +1512,13 @@
 		if (hpriv->hp_flags & MV_HP_CUT_THROUGH)
 			cfg |= (1 << 17); /* enab cut-thru (dis stor&forwrd) */
 		mv_bmdma_enable_iie(ap, !want_edma);
+
+		if (IS_SOC(hpriv)) {
+			if (want_ncq)
+				mv_soc_led_blink_enable(ap);
+			else
+				mv_soc_led_blink_disable(ap);
+		}
 	}
 
 	if (want_ncq) {

Re: [PATCH 02/03] sata_mv cosmetic preparations for IRQ coalescing

[Jeff Garzik]

Mark Lord wrote:
> Various cosmetic changes in preparation for the IRQ coalescing feature.
> 
> Note that the various MV_IRQ_COAL_* definitions are restored/renamed
> in the folloup patch which adds IRQ coalescing to the driver.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>
> ---
> 
> This is for libata-dev #upstream
> 
> --- old/drivers/ata/sata_mv.c    2009-03-10 18:20:20.000000000 -0400
> +++ new/drivers/ata/sata_mv.c    2009-03-10 18:26:54.000000000 -0400
> @@ -1,10 +1,13 @@
> /*
>  * sata_mv.c - Marvell SATA support
>  *
> - * Copyright 2008: Marvell Corporation, all rights reserved.
> + * Copyright 2008-2009: Marvell Corporation, all rights reserved.
>  * Copyright 2005: EMC Corporation, all rights reserved.
>  * Copyright 2005 Red Hat, Inc.  All rights reserved.
>  *
> + * Originally written by Brett Russ.
> + * Extensive overhaul and enhancement by Mark Lord <mlord@pobox.com>.

I'd like to think my own extensive overhaul, conversion to new EH, 
preparation for NCQ, 64-bit DMA changes, and work on chip generation 
separation amounted to something...

Re: [PATCH 02/03] sata_mv cosmetic preparations for IRQ coalescing

[Jeff Garzik]

Mark Lord wrote:
> Various cosmetic changes in preparation for the IRQ coalescing feature.
> 
> Note that the various MV_IRQ_COAL_* definitions are restored/renamed
> in the folloup patch which adds IRQ coalescing to the driver.
> 
> Signed-off-by: Mark Lord <mlord@pobox.com>

applied 2-5

patch #1 was already applied -- in general, I will merge #upstream-fixes 
patches into #upstream immediately.  Don't worry about sending me 
duplicates for each branch, unless the branches are wildly divergent.

Re: [PATCH 02/03] sata_mv cosmetic preparations for IRQ coalescing

[Mark Lord]

Jeff Garzik wrote:
> Mark Lord wrote:
..
>> @@ -1,10 +1,13 @@
>> /*
>>  * sata_mv.c - Marvell SATA support
>>  *
>> - * Copyright 2008: Marvell Corporation, all rights reserved.
>> + * Copyright 2008-2009: Marvell Corporation, all rights reserved.
>>  * Copyright 2005: EMC Corporation, all rights reserved.
>>  * Copyright 2005 Red Hat, Inc.  All rights reserved.
>>  *
>> + * Originally written by Brett Russ.
>> + * Extensive overhaul and enhancement by Mark Lord <mlord@pobox.com>.
..

The above change is only to help people find the right person
to complain to when they have trouble with the driver.
Seeing as Brent hasn't worked on it for many years now,
I figured it needed an update.
 

> I'd like to think my own extensive overhaul, conversion to new EH, 
> preparation for NCQ, 64-bit DMA changes, and work on chip generation 
> separation amounted to something...
..


So why don't you patch a line for yourself into there?
I could always use an extra hand supporting this beast.  ;)

Cheers