sata_mv always do softreset
PATCH 1/4 sata_mv always do softreset
Always request a softreset after hardreset succeeds.
This fixes a regression reported by Martin Michlmayr <[email protected]>.
---
For 2.6.26.
drivers/ata/sata_mv.c +0 -0
@@ -2728,6 +2728,7 @@
rc = sata_link_hardreset(link, timing, deadline + extra, &online, NULL); + rc = online ? -EAGAIN : rc; if (rc) return rc; sata_scr_read(link, SCR_STATUS, &sstatus);
PATCH 2/4 sata_mv fis irq register fixes
Fix handling of the FIS_IRQ_CAUSE register in sata_mv.
This register exists *only* on GenIIe devices, so don't bother writing to it on older chips. Also, it has to be read/cleared in mv_err_intr() before clearing the main ERR_IRQ_CAUSE register.
This keeps sata_mv from getting stuck forever on certain error types.
---
For 2.6.26.
drivers/ata/sata_mv.c +0 -0
@@ -886,7 +886,8 @@
mv_edma_cfg(ap, want_ncq); /* clear FIS IRQ Cause */
- writelfl(0, port_mmio + SATA_FIS_IRQ_CAUSE_OFS);
+ if (IS_GEN_IIE(hpriv))
+ writelfl(0, port_mmio + SATA_FIS_IRQ_CAUSE_OFS);
mv_set_edma_ptrs(port_mmio, hpriv, pp);
@@ -1812,6 +1813,7 @@
{
void __iomem *port_mmio = mv_ap_base(ap);
u32 edma_err_cause, eh_freeze_mask, serr = 0;
+ u32 fis_cause = 0;
struct mv_port_priv *pp = ap->private_data;
struct mv_host_priv *hpriv = ap->host->private_data;
unsigned int action = 0, err_mask = 0;@@ -1821,16 +1823,19 @@
/* * Read and clear the SError and err_cause bits. + * For GenIIe, if EDMA_ERR_TRANS_IRQ_7 is set, we also must read/clear + * the FIS_IRQ_CAUSE register before clearing edma_err_cause. */ sata_scr_read(&ap->link, SCR_ERROR, &serr); sata_scr_write_flush(&ap->link, SCR_ERROR, serr); edma_err_cause = readl(port_mmio + EDMA_ERR_IRQ_CAUSE_OFS);
+ if (IS_GEN_IIE(hpriv) && (edma_err_cause & EDMA_ERR_TRANS_IRQ_7)) {
+ fis_cause = readl(port_mmio + SATA_FIS_IRQ_CAUSE_OFS);
+ writelfl(~fis_cause, port_mmio + SATA_FIS_IRQ_CAUSE_OFS);
+ }
writelfl(~edma_err_cause, port_mmio + EDMA_ERR_IRQ_CAUSE_OFS);
- ata_port_printk(ap, KERN_INFO, "%s: err_cause=%08x pp_flags=0x%x\n",
- __func__, edma_err_cause, pp->pp_flags);
-
if (edma_err_cause & EDMA_ERR_DEV) {
/*
* Device errors during FIS-based switching operation
@@ -1844,6 +1849,9 @@
ata_ehi_clear_desc(ehi);
ata_ehi_push_desc(ehi, "edma_err_cause=%08x pp_flags=%08x",
edma_err_cause, pp->pp_flags);
+
+ if (IS_GEN_IIE(hpriv) && (edma_err_cause & EDMA_ERR_TRANS_IRQ_7))
+ ata_ehi_push_desc(ehi, "fis_cause=%08x", fis_cause);
/*
* All generations share these EDMA error cause bits:
*/PATCH 3/4 sata_mv group genIIe flags
Group all of the flags for GenIIe devices into a common definition, to ensure that any updates to them are shared by all GenIIe devices.
This will help make future maintenance somewhat simpler.
---
For 2.6.26.
drivers/ata/sata_mv.c +0 -0
@@ -128,8 +128,13 @@
MV_COMMON_FLAGS = ATA_FLAG_SATA | ATA_FLAG_NO_LEGACY |
ATA_FLAG_MMIO | ATA_FLAG_NO_ATAPI | ATA_FLAG_PIO_POLLING,
+
MV_6XXX_FLAGS = MV_FLAG_IRQ_COALESCE,
+ MV_GENIIE_FLAGS = MV_COMMON_FLAGS | MV_6XXX_FLAGS |
+ ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
+ ATA_FLAG_NCQ,
+
CRQB_FLAG_READ = (1 << 0),
CRQB_TAG_SHIFT = 1,
CRQB_IOID_SHIFT = 6, /* CRQB Gen-II/IIE IO Id shift */
@@ -640,25 +645,19 @@
.port_ops = &mv6_ops,
},
{ /* chip_6042 */- .flags = MV_COMMON_FLAGS | MV_6XXX_FLAGS |
- ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
- ATA_FLAG_NCQ,
+ .flags = MV_GENIIE_FLAGS,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = ATA_UDMA6,
.port_ops = &mv_iie_ops,
},
{ /* chip_7042 */- .flags = MV_COMMON_FLAGS | MV_6XXX_FLAGS |
- ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
- ATA_FLAG_NCQ,
+ .flags = MV_GENIIE_FLAGS,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = ATA_UDMA6,
.port_ops = &mv_iie_ops,
},
{ /* chip_soc */
- .flags = MV_COMMON_FLAGS | MV_6XXX_FLAGS |
- ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
- ATA_FLAG_NCQ | MV_FLAG_SOC,
+ .flags = MV_GENIIE_FLAGS | MV_FLAG_SOC,
.pio_mask = 0x1f, /* pio0-4 */
.udma_mask = ATA_UDMA6,
.port_ops = &mv_iie_ops,PATCH 4/4 sata_mv async notify for genIIe only
Now that we handle the FIS_IRQ_CAUSE register correctly, we can also now handle SATA asynchronous notification events.
So enable them, but only for the more modern GenIIe chips. (older chips have unaddressed errata issues related to this).
This fixes hot plug/unplug for port-muliplier ports.
drivers/ata/sata_mv.c +0 -0
@@ -133,7 +133,7 @@
MV_GENIIE_FLAGS = MV_COMMON_FLAGS | MV_6XXX_FLAGS |
ATA_FLAG_PMP | ATA_FLAG_ACPI_SATA |
- ATA_FLAG_NCQ,
+ ATA_FLAG_NCQ | ATA_FLAG_AN,
CRQB_FLAG_READ = (1 << 0), CRQB_TAG_SHIFT = 1, @@ -226,6 +226,7 @@ SATA_STATUS_OFS = 0x300, /* ctrl, err regs follow status */ SATA_ACTIVE_OFS = 0x350, SATA_FIS_IRQ_CAUSE_OFS = 0x364,
+ SATA_FIS_IRQ_AN = (1 << 9), /* async notification */
LTMODE_OFS = 0x30c, LTMODE_BIT8 = (1 << 8), /* unknown, but necessary */ @@ -1849,8 +1850,17 @@ ata_ehi_push_desc(ehi, "edma_err_cause=%08x pp_flags=%08x", edma_err_cause, pp->pp_flags);
- if (IS_GEN_IIE(hpriv) && (edma_err_cause & EDMA_ERR_TRANS_IRQ_7))
+ if (IS_GEN_IIE(hpriv) && (edma_err_cause & EDMA_ERR_TRANS_IRQ_7)) {
ata_ehi_push_desc(ehi, "fis_cause=%08x", fis_cause);+ if (fis_cause & SATA_FIS_IRQ_AN) {
+ u32 ec = edma_err_cause &
+ ~(EDMA_ERR_TRANS_IRQ_7 | EDMA_ERR_IRQ_TRANSIENT);
+ sata_async_notification(ap);
+ if (!ec)
+ return; /* Just an AN; no need for the nukes */
+ ata_ehi_push_desc(ehi, "SDB notify");
+ }
+ }
/* * All generations share these EDMA error cause bits: */
PATCH 5/9 sata_mv don't blindly enable IRQs
These five small patches are on top of the four previously posted, so I'm extending the numbering to be xx/09 now:
-----snip-----
Part one of simplifying/fixing handling of the main_irq_mask register to resolve unexpected interrupt issues observed in 2.6.26-rc*.
Don't blindly enable port IRQs at host init time.
Instead, enable only the bits that we want,
which in this case is simply the PCI_ERR bit.
The per-port bits can wait until the ports are reset/probed for devices.
drivers/ata/sata_mv.c +0 -0
@@ -202,13 +202,6 @@
HC_MAIN_RSVD = (0x7f << 25), /* bits 31-25 */ HC_MAIN_RSVD_5 = (0x1fff << 19), /* bits 31-19 */ HC_MAIN_RSVD_SOC = (0x3fffffb << 6), /* bits 31-9, 7-6 */ - HC_MAIN_MASKED_IRQS = (TRAN_LO_DONE | TRAN_HI_DONE | - PORTS_0_3_COAL_DONE | PORTS_4_7_COAL_DONE | - PORTS_0_7_COAL_DONE | GPIO_INT | TWSI_INT | - HC_MAIN_RSVD),
- HC_MAIN_MASKED_IRQS_5 = (PORTS_0_3_COAL_DONE | PORTS_4_7_COAL_DONE |
- HC_MAIN_RSVD_5),
- HC_MAIN_MASKED_IRQS_SOC = (PORTS_0_3_COAL_DONE | HC_MAIN_RSVD_SOC),
/* SATAHC registers */ HC_CFG_OFS = 0,
@@ -3101,25 +3094,12 @@
/* and unmask interrupt generation for host regs */ writelfl(hpriv->unmask_all_irqs, mmio + hpriv->irq_mask_ofs);
- if (IS_GEN_I(hpriv))
- writelfl(~HC_MAIN_MASKED_IRQS_5,
- hpriv->main_irq_mask_addr);
- else
- writelfl(~HC_MAIN_MASKED_IRQS,
- hpriv->main_irq_mask_addr);
- VPRINTK("HC MAIN IRQ cause/mask=0x%08x/0x%08x "
- "PCI int cause/mask=0x%08x/0x%08x\n",
- readl(hpriv->main_irq_cause_addr),
- readl(hpriv->main_irq_mask_addr),
- readl(mmio + hpriv->irq_cause_ofs),
- readl(mmio + hpriv->irq_mask_ofs));
- } else {
- writelfl(~HC_MAIN_MASKED_IRQS_SOC,
- hpriv->main_irq_mask_addr);
- VPRINTK("HC MAIN IRQ cause/mask=0x%08x/0x%08x\n",
- readl(hpriv->main_irq_cause_addr),
- readl(hpriv->main_irq_mask_addr));
+ /*
+ * enable only global host interrupts for now.
+ * The per-port interrupts get done later as ports are set up.
+ */
+ writelfl(PCI_ERR, hpriv->main_irq_mask_addr); } done: return rc;
PATCH 6/9 sata_mv consolidate main_irq_mask updates
Part two of simplifying/fixing handling of the main_irq_mask register to resolve unexpected interrupt issues observed in 2.6.26-rc*.
Consolidate all updates of the host main_irq_mask register into a single function. This simplifies maintenance, and also prepares the way for caching it (later).
No functionality changes in this update.
drivers/ata/sata_mv.c +0 -0
@@ -837,6 +837,31 @@
port_mmio + EDMA_RSP_Q_OUT_PTR_OFS); }
+static void mv_set_main_irq_mask(struct ata_host *host,
+ u32 disable_bits, u32 enable_bits)
+{
+ struct mv_host_priv *hpriv = host->private_data;
+ u32 old_mask, new_mask;
+
+ old_mask = readl(hpriv->main_irq_mask_addr);
+ new_mask = (old_mask & ~disable_bits) | enable_bits;
+ if (new_mask != old_mask)
+ writelfl(new_mask, hpriv->main_irq_mask_addr);
+}
+
+static void mv_enable_port_irqs(struct ata_port *ap,
+ unsigned int port_bits)
+{
+ unsigned int shift, hardport, port = ap->port_no;
+ u32 disable_bits, enable_bits;
+
+ MV_PORT_TO_SHIFT_AND_HARDPORT(port, shift, hardport);
+
+ disable_bits = (DONE_IRQ | ERR_IRQ) << shift;
+ enable_bits = port_bits << shift;
+ mv_set_main_irq_mask(ap->host, disable_bits, enable_bits);
+}
+
/**
* mv_start_dma - Enable eDMA engine
* @base: port base address
@@ -2383,7 +2408,6 @@
ZERO(MV_PCI_DISC_TIMER); ZERO(MV_PCI_MSI_TRIGGER); writel(0x000100ff, mmio + MV_PCI_XBAR_TMOUT_OFS); - ZERO(PCI_HC_MAIN_IRQ_MASK_OFS); ZERO(MV_PCI_SERR_MASK); ZERO(hpriv->irq_cause_ofs); ZERO(hpriv->irq_mask_ofs);
@@ -2755,32 +2779,18 @@
static void mv_eh_freeze(struct ata_port *ap)
{
- struct mv_host_priv *hpriv = ap->host->private_data;
- unsigned int shift, hardport, port = ap->port_no;
- u32 main_irq_mask;
-
- /* FIXME: handle coalescing completion events properly */
-
mv_stop_edma(ap);
- MV_PORT_TO_SHIFT_AND_HARDPORT(port, shift, hardport);
-
- /* disable assertion of portN err, done events */
- main_irq_mask = readl(hpriv->main_irq_mask_addr);
- main_irq_mask &= ~((DONE_IRQ | ERR_IRQ) << shift);
- writelfl(main_irq_mask, hpriv->main_irq_mask_addr);
+ mv_enable_port_irqs(ap, 0);
}
static void mv_eh_thaw(struct ata_port *ap)
{
struct mv_host_priv *hpriv = ap->host->private_data;
- unsigned int shift, hardport, port = ap->port_no;
+ unsigned int port = ap->port_no;
+ unsigned int hardport = mv_hardport_from_port(port);
void __iomem *hc_mmio = mv_hc_base_from_port(hpriv->base, port);
void __iomem *port_mmio = mv_ap_base(ap);- u32 main_irq_mask, hc_irq_cause;
-
- /* FIXME: handle coalescing completion events properly */
-
- MV_PORT_TO_SHIFT_AND_HARDPORT(port, shift, hardport);
+ u32 hc_irq_cause;
/* clear EDMA errors on this port */ writel(0, port_mmio + EDMA_ERR_IRQ_CAUSE_OFS); @@ -2790,10 +2800,7 @@ hc_irq_cause &= ~((DEV_IRQ | DMA_IRQ) << hardport); writelfl(hc_irq_cause, hc_mmio + HC_IRQ_CAUSE_OFS);
- /* enable assertion of portN err, done events */
- main_irq_mask = readl(hpriv->main_irq_mask_addr);
- main_irq_mask |= ((DONE_IRQ | ERR_IRQ) << shift);
- writelfl(main_irq_mask, hpriv->main_irq_mask_addr);
+ mv_enable_port_irqs(ap, DONE_IRQ | ERR_IRQ);
}
/**
@@ -3046,7 +3053,7 @@
} /* global interrupt mask: 0 == mask everything */
- writel(0, hpriv->main_irq_mask_addr);
+ mv_set_main_irq_mask(host, ~0, 0);
n_hc = mv_get_hc_count(host->ports[0]->flags);
@@ -3099,7 +3106,7 @@
* enable only global host interrupts for now.
* The per-port interrupts get done later as ports are set up. */
- writelfl(PCI_ERR, hpriv->main_irq_mask_addr);
+ mv_set_main_irq_mask(host, 0, PCI_ERR); } done: return rc;
PATCH 7/9 sata_mv fix pmp drives not found
Part three of simplifying/fixing handling of the main_irq_mask register to resolve unexpected interrupt issues observed in 2.6.26-rc*.
Partially fix a reported bug whereby we sometimes miss seeing drives on a port-multiplier, as reported by Gwendal Grignou <[email protected]>.
The problem was that we were receiving unexpected interrupts during EH from POLLed commands while accessing port-multiplier registers. These unexpected interrupts can be prevented by masking the DONE_IRQ bit for the port whenever not operating in EDMA mode.
Also fix port_stop() to mask all port interrupts.
drivers/ata/sata_mv.c +0 -0
@@ -908,6 +908,7 @@
writelfl(0, port_mmio + SATA_FIS_IRQ_CAUSE_OFS); mv_set_edma_ptrs(port_mmio, hpriv, pp);
+ mv_enable_port_irqs(ap, DONE_IRQ|ERR_IRQ);
writelfl(EDMA_EN, port_mmio + EDMA_CMD_OFS);
pp->pp_flags |= MV_PP_FLAG_EDMA_EN;
@@ -1360,6 +1361,7 @@
static void mv_port_stop(struct ata_port *ap)
{
mv_stop_edma(ap);
+ mv_enable_port_irqs(ap, 0);
mv_port_free_dma_mem(ap);
}@@ -1601,6 +1603,7 @@
* shadow block, etc registers. */ mv_stop_edma(ap); + mv_enable_port_irqs(ap, ERR_IRQ); mv_pmp_select(ap, qc->dev->link->pmp); return ata_sff_qc_issue(qc); } @@ -2800,7 +2803,7 @@ hc_irq_cause &= ~((DEV_IRQ | DMA_IRQ) << hardport); writelfl(hc_irq_cause, hc_mmio + HC_IRQ_CAUSE_OFS);
- mv_enable_port_irqs(ap, DONE_IRQ | ERR_IRQ);
+ mv_enable_port_irqs(ap, ERR_IRQ);
}
/**
PATCH 8/9 sata_mv disregard masked irqs
Part four of simplifying/fixing handling of the main_irq_mask register to resolve unexpected interrupt issues observed in 2.6.26-rc*.
Ignore masked IRQs in mv_interrupt().
This prevents "unexpected device interrupt while idle" messages.
drivers/ata/sata_mv.c +0 -0
@@ -2200,20 +2200,21 @@
struct ata_host *host = dev_instance; struct mv_host_priv *hpriv = host->private_data; unsigned int handled = 0;
- u32 main_irq_cause, main_irq_mask;
+ u32 main_irq_cause, main_irq_mask, pending_irqs;
spin_lock(&host->lock); main_irq_cause = readl(hpriv->main_irq_cause_addr); main_irq_mask = readl(hpriv->main_irq_mask_addr); + pending_irqs = main_irq_cause & main_irq_mask; /* * Deal with cases where we either have nothing pending, or have read * a bogus register value which can indicate HW removal or PCI fault. */
- if ((main_irq_cause & main_irq_mask) && (main_irq_cause != 0xffffffffU)) {
- if (unlikely((main_irq_cause & PCI_ERR) && HAS_PCI(host)))
+ if (pending_irqs && main_irq_cause != 0xffffffffU) {
+ if (unlikely((pending_irqs & PCI_ERR) && HAS_PCI(host)))
handled = mv_pci_error(host, hpriv->base); else - handled = mv_host_intr(host, main_irq_cause); + handled = mv_host_intr(host, pending_irqs); } spin_unlock(&host->lock); return IRQ_RETVAL(handled);
PATCH 9/9 sata_mv cache main_irq_mask register in hpriv
Part five of simplifying/fixing handling of the main_irq_mask register to resolve unexpected interrupt issues observed in 2.6.26-rc*.
Keep a cached copy of the main_irq_mask so that we don't have to stall the CPU to read it on every pass through mv_interrupt.
This significantly speeds up interrupt handling, both for sata_mv, and for any other driver/device sharing the same PCI IRQ line.
drivers/ata/sata_mv.c +0 -0
@@ -458,6 +458,7 @@
struct mv_host_priv {
u32 hp_flags;
+ u32 main_irq_mask;
struct mv_port_signal signal[8];
const struct mv_hw_ops *ops;
int n_ports;
@@ -843,10 +844,12 @@
struct mv_host_priv *hpriv = host->private_data;
u32 old_mask, new_mask;- old_mask = readl(hpriv->main_irq_mask_addr);
+ old_mask = hpriv->main_irq_mask;
new_mask = (old_mask & ~disable_bits) | enable_bits;
- if (new_mask != old_mask)
+ if (new_mask != old_mask) {
+ hpriv->main_irq_mask = new_mask;
writelfl(new_mask, hpriv->main_irq_mask_addr);+ }
}
static void mv_enable_port_irqs(struct ata_port *ap, @@ -2200,12 +2203,11 @@
struct ata_host *host = dev_instance; struct mv_host_priv *hpriv = host->private_data; unsigned int handled = 0;
- u32 main_irq_cause, main_irq_mask, pending_irqs;
+ u32 main_irq_cause, pending_irqs;
spin_lock(&host->lock); main_irq_cause = readl(hpriv->main_irq_cause_addr); - main_irq_mask = readl(hpriv->main_irq_mask_addr); - pending_irqs = main_irq_cause & main_irq_mask; + pending_irqs = main_irq_cause & hpriv->main_irq_mask;
/*
* Deal with cases where we either have nothing pending, or have read
* a bogus register value which can indicate HW removal or PCI fault.
PATCH 10/10 sata_mv: ensure empty request queue for FBS-NCQ EH
Check for an empty request queue before stopping EDMA after a FBS-NCQ error, as per recommendation from the Marvell datasheet.
This ensures that the EDMA won't suddenly become active again just after our subsequent check of the empty/idle bits.
Also bump DRV_VERSION.
---
Is DRV_VERSION even useful now?
I could do another patch to just nuke it.
drivers/ata/sata_mv.c +0 -0
@@ -72,7 +72,7 @@
#include <linux/libata.h>
#define DRV_NAME "sata_mv"
-#define DRV_VERSION "1.20"
+#define DRV_VERSION "1.21"
enum {
/* BAR's are enumerated in terms of pci_resource_start() terms */
@@ -1695,6 +1695,18 @@
}
}+static int mv_req_q_empty(struct ata_port *ap) +{
+ void __iomem *port_mmio = mv_ap_base(ap);
+ u32 in_ptr, out_ptr;
+
+ in_ptr = (readl(port_mmio + EDMA_REQ_Q_IN_PTR_OFS)
+ >> EDMA_REQ_Q_PTR_SHIFT) & MV_MAX_Q_DEPTH_MASK;
+ out_ptr = (readl(port_mmio + EDMA_REQ_Q_OUT_PTR_OFS)
+ >> EDMA_REQ_Q_PTR_SHIFT) & MV_MAX_Q_DEPTH_MASK;
+ return (in_ptr == out_ptr); /* 1 == queue_is_empty */
+}
+
static int mv_handle_fbs_ncq_dev_err(struct ata_port *ap)
{
struct mv_port_priv *pp = ap->private_data;
@@ -1728,7 +1740,7 @@
ap->qc_active, failed_links,
ap->nr_active_links);- if (ap->nr_active_links <= failed_links) {
+ if (ap->nr_active_links <= failed_links && mv_req_q_empty(ap)) {
mv_process_crpb_entries(ap, pp);
mv_stop_edma(ap);
mv_eh_freeze(ap);Re: [PATCH 10/10] sata_mv: ensure empty request queue for FBS-NCQ EH
Quoted 6 lines · Mark Lord
Check for an empty request queue before stopping EDMA after a FBS-NCQ error, as per recommendation from the Marvell datasheet.
This ensures that the EDMA won't suddenly become active again just after our subsequent check of the empty/idle bits.
Quoted 7 lines
Also bump
DRV_VERSION.Signed-off-by Mark Lord <[email protected]>---
Is
DRV_VERSIONeven useful now?
I could do another patch to just nuke it.
It is standard practice to export all driver versions via dmesg and modinfo for all libata drivers, so I would prefer that it stay, for consistency.
Is it useful?
That depends on the maintainer's definition and upkeep of the version number. If you never update it, of course its useless. If you update it when the driver achieves certain milestones, i.e. "2.0" when all your changes are complete, then it may be relevant to you and your userbase.
Version numbers tend to be quite handy because drivers get backported all over the place, so you might not be able to assume that sata_mv version 1.21 implies kernel version 2.6.26, for example.
Jeff
Re: [PATCH 10/10] sata_mv: ensure empty request queue for FBS-NCQ EH
In reply to Mark LordQuoted 6 lines · Mark Lord
Check for an empty request queue before stopping EDMA after a FBS-NCQ error, as per recommendation from the Marvell datasheet.
This ensures that the EDMA won't suddenly become active again just after our subsequent check of the empty/idle bits.
Quoted 7 lines
Also bump
DRV_VERSION.Signed-off-by Mark Lord <[email protected]>---
Is
DRV_VERSIONeven useful now?
I could do another patch to just nuke it.
applied 1-10
Re: [PATCH 09/09] sata_mv cache main_irq_mask register in hpriv
In reply to Mark LordQuoted 8 lines · Mark Lord
Part five of simplifying/fixing handling of the
main_irq_maskregister to resolve unexpected interrupt issues observed in 2.6.26-rc*.Keep a cached copy of the
main_irq_maskso that we don't have to stall the CPU to read it on every pass throughmv_interrupt.This significantly speeds up interrupt handling, both for
sata_mv, and for any other driver/device sharing the same PCI IRQ line.
To be precise, it saves about ~1% of cpu cycles of ~2Ghz CPU as measured by oprofile. "fio" provided the workload by sequentially reading 4K blocks from 4 normal SATA II disks (about 8K IO/s per disk). This is expected to be similar to using SSDs.
So not a huge win; just an easy one.
Here is the cost of one MMIO read on a not-to-old intel PCI-e chipset:
0a:00.0 (11ab:7042): sata_mv from CPU0 SATAHC0Cause
(20014): 1345 cycles 576 ns
0a:00.0 (11ab:7042): sata_mv from CPU0 main_mask
(1d64): 1302 cycles 558 ns
0a:00.0 (11ab:7042): sata_mv from CPU0 main_cause
(1d60): 1302 cycles 558 ns
I'll note this is extremely fast since there are no PCI-PCI bridges and the 7042 controller seems to respond very quickly. More typical is 1000ns or so.
thanks,
grant
(1) svn co http://svn.gnumonks.org/trunk/mmio_test/
Plus this patch:
http://iou.parisc-linux.org/~grundler/diff/diff-mmio_test-02
Re: [PATCH 05/09] sata_mv don't blindly enable IRQs
In reply to Mark LordQuoted 8 lines · Mark Lord
These five small patches are on top of the four previously posted, so I'm extending the numbering to be xx/09 now:
-----snip-----
Part one of simplifying/fixing handling of the
main_irq_maskregister to resolve unexpected interrupt issues observed in 2.6.26-rc*.
Quoted 7 lines
Don't blindly enable port IRQs at host init time.
Instead, enable only the bits that we want,
which in this case is simply thePCI_ERRbit.The per-port bits can wait until the ports are reset/probed for devices.
Signed-off-by Mark Lord <[email protected]>
Two process notes for future use:
1) please include a colon (":") or other punctuation after "sata_mv" prefix in subject line.
2) when adding notes like "These five small patches...", we already have a method of doing so: the "---" separator. The format of the patch body should be:
patch description for kernel changelog
signed-off-by: ...
---
These five small patches are on top of the four previously posted, so I'm extending the numbering to be xx/09 now.
Both of these details are documented in Documentation/SubmittingPatches and http://linux.yyz.us/patch-format.html
OTOH, your numbering behavior -- extending the patch numbering as you've done here, when the first patchset is not yet applied to git -- is quite useful and should be emulated by others :)
Tejun sometimes simply posts a new patchseries, noting that that patchseries is dependent upon a previous series. That's fine, too.
Thanks,
Jeff
Re: [PATCH 05/09] sata_mv don't blindly enable IRQs
Quoted 2 lines · Jeff Garzik
1) please include a colon (":") or other punctuation after "
sata_mv" prefix in subject line.
..
Ahh.. will do, thanks.
Quoted 9 lines
2) when adding notes like "These five small patches...", we already have a method of doing so: the "---" separator. The format of the patch body should be:
patch description for kernel changelog
signed-off-by: ...
---
These five small patches are on top of the four previously posted, so I'm extending the numbering to be xx/09 now.
..
I wondered about that one, and decided that making it prominent at the top might be best. But since you're trained to look in the correct place further down, that's where I'll put it from now on.
Thanks Jeff!
Re: [PATCH 05/09] sata_mv don't blindly enable IRQs
Quoted 3 lines · Mark Lord
I wondered about that one, and decided that making it prominent at the top might be best. But since you're trained to look in the correct place further down, that's where I'll put it from now on.
Not only my brain: Linus's standard email tools[1] automatically snip everything after the separator, reducing work on my end.
Thanks,
Jeff
[1] The email tools that come with the git program, namely git-am. Packaged as 'git-email' in Fedora.