[PATCH v3] ata: ahci: work around lost interrupts on Marvell 88SE61xx

[PATCH v3] ata: ahci: work around lost interrupts on Marvell 88SE61xx

[Hajo Noerenberg]

ahci_single_level_irq_intr() services the ports first and clears the
global HOST_IRQ_STAT afterwards, as recommended by AHCI 1.1 section
10.6.2.  The Marvell 88SE6111/6121/6145 family stops reporting interrupts
for a port when HOST_IRQ_STAT is cleared while PxIS still holds bits:
PxIS keeps its content, HOST_IRQ_STAT reads back as 0, the port is never
looked at again, and the command in flight only ends in a timeout.

Measured on a Seagate Blackarmor NAS440 (Marvell 88F6281 Kirkwood,
88SE6121 rev B2 behind PCIe) by polling the AHCI registers from userspace
while an IDENTIFY was outstanding:

  t=303.046  irqs 127  PxIS 0x00000000  PxCI 0x00000001
             IDENTIFY issued
  t=303.057  irqs 128  PxIS 0x00000020  PxCI 0x00000000
             CI cleared, DPS set, one interrupt taken
             ... PxIS stays 0x00000020, HOST_IRQ_STAT stays 0 ...
  t~308.05   qc timeout after 5000 msecs

The command had completed - PxCI was clear and PxIS had DPS set - so
ahci_qc_complete() would have completed it.  It never got the chance
because the handler read HOST_IRQ_STAT as 0 and returned IRQ_NONE.

Marvell's own driver for these chips clears the two registers in the
opposite order and says so ("clear global before channel"), and
ahci_xgene handles its broken edge latch the same way.  Since the
reordering costs at most one spurious interrupt per valid one on
conforming controllers, do it in a private interrupt handler selected for
board_ahci_mv instead of changing libahci for everyone.

With this applied, SATA-2 and SATA-3 disks work at 3.0 Gbps on the
88SE6121 without the drive-side 1.5 Gbps jumper that was needed before.
Time from link up to a successful IDENTIFY:

  WDC WD5000AADS-00S9B0  port 0    7 ms  (never identified before)
  WDC WD3202ABYS-01B7A0  port 1   28 ms
  WDC WD30EFRX-68EUZN0   port 1  200 ms  (3 TB, HPA detection ok)

Only the 88SE6121 was tested; board_ahci_mv also covers the 88SE6145,
which Marvell's driver treats identically.

Fixes: cd70c26617f4 ("[libata] AHCI: Add support for Marvell AHCI-like chips (initially 6145)")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/linux-ide/db6b48b7-d69a-564b-24f0-75fbd6a9e543@noerenberg.de/
Link: https://bugzilla.kernel.org/show_bug.cgi?id=216094
Signed-off-by: Hajo Noerenberg <hajo-linux-ide@noerenberg.de>
Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
Acked-by: Pali Rohar <pali@kernel.org>

---
Niklas, sorry for the wasted round trip.  Kernel patches are not my daily
business and I had not realised how far the tree I was working against had
drifted - v2 was generated against 6.10-rc6.  Rebased on v7.3-rc1 now, and
the base commit is recorded at the bottom so this cannot happen unnoticed
again.  Thank you for taking the time to look at it anyway.

v3:
 - rebase onto v7.3-rc1; v2 did not apply
 - add Fixes: and Cc: stable@vger.kernel.org, as requested
 - collect Reviewed-by from Damien and Acked-by from Pali
 - no code change

v2:
 - retitle "clear HOST_IRQ_STAT before the ports" -> "work around lost
   interrupts", per review

 drivers/ata/ahci.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -2614,6 +2614,51 @@
 }
 #endif
 
+/*
+ * The Marvell 88SE6111/6121/6145 ("Thor") family stops reporting interrupts
+ * for a port when HOST_IRQ_STAT is cleared while PxIS still holds bits: PxIS
+ * keeps its content, HOST_IRQ_STAT reads back as 0, the port is never looked
+ * at again and the command in flight only ends in a timeout.  On a 88SE6121
+ * this makes every SATA-2 or SATA-3 disk fail to IDENTIFY, while SATA-1 disks
+ * happen to win the race often enough to work.
+ *
+ * Clearing the host status before servicing the ports avoids it.  Marvell's
+ * own driver for these chips does the same and says so ("clear global before
+ * channel"), and ahci_xgene handles its broken edge latch the same way.  The
+ * price is at most one spurious interrupt per valid one, which is why this is
+ * not the generic behaviour - see AHCI 1.1 section 10.6.2.
+ *
+ * Link: https://bugzilla.kernel.org/show_bug.cgi?id=216094
+ */
+static irqreturn_t ahci_mv_irq_handler(int irq, void *dev_instance)
+{
+	struct ata_host *host = dev_instance;
+	struct ahci_host_priv *hpriv = host->private_data;
+	void __iomem *mmio = hpriv->mmio;
+	unsigned int rc;
+	u32 irq_stat, irq_masked;
+
+	irq_stat = readl(mmio + HOST_IRQ_STAT);
+	if (!irq_stat)
+		return IRQ_NONE;
+
+	irq_masked = irq_stat & hpriv->port_map;
+
+	spin_lock(&host->lock);
+
+	/*
+	 * Use the unmasked value to clear the interrupt, as a spurious pending
+	 * event on a dummy port might cause a screaming IRQ.
+	 */
+	writel(irq_stat, mmio + HOST_IRQ_STAT);
+
+	rc = ahci_handle_port_intr(host, irq_masked);
+
+	spin_unlock(&host->lock);
+
+	return IRQ_RETVAL(rc);
+}
+
 static void ahci_remap_check(struct pci_dev *pdev, int bar,
 		struct ahci_host_priv *hpriv)
 {
@@ -2917,6 +2962,10 @@
 		return -ENOMEM;
 	hpriv->flags |= (unsigned long)pi.private_data;
 
+	/* the Marvell "Thor" family needs HOST_IRQ_STAT cleared first */
+	if (board_id == board_ahci_mv)
+		hpriv->irq_handler = ahci_mv_irq_handler;
+
 	/* MCP65 revision A1 and A2 can't do MSI */
 	if (board_id == board_ahci_mcp65 &&
 	    (pdev->revision == 0xa1 || pdev->revision == 0xa2))

base-commit: cee9395acd8043be0644b25c34bfa86623f2b935

Re: [PATCH v3] ata: ahci: work around lost interrupts on Marvell 88SE61xx

[Niklas Cassel]

On Mon, 31 Aug 2026 14:43:03 +0200, Hajo Noerenberg wrote:
> ahci_single_level_irq_intr() services the ports first and clears the
> global HOST_IRQ_STAT afterwards, as recommended by AHCI 1.1 section
> 10.6.2.  The Marvell 88SE6111/6121/6145 family stops reporting interrupts
> for a port when HOST_IRQ_STAT is cleared while PxIS still holds bits:
> PxIS keeps its content, HOST_IRQ_STAT reads back as 0, the port is never
> looked at again, and the command in flight only ends in a timeout.
> 
> [...]

Applied to libata/linux.git (for-7.4), thanks!

[1/1] ata: ahci: work around lost interrupts on Marvell 88SE61xx
      https://git.kernel.org/libata/linux/c/22f2ba34

Kind regards,
Niklas

Re: [PATCH v3] ata: ahci: work around lost interrupts on Marvell 88SE61xx

[Niklas Cassel]

On Mon, Aug 31, 2026 at 04:27:24PM +0200, Niklas Cassel wrote:
> On Mon, 31 Aug 2026 14:43:03 +0200, Hajo Noerenberg wrote:
> > ahci_single_level_irq_intr() services the ports first and clears the
> > global HOST_IRQ_STAT afterwards, as recommended by AHCI 1.1 section
> > 10.6.2.  The Marvell 88SE6111/6121/6145 family stops reporting interrupts
> > for a port when HOST_IRQ_STAT is cleared while PxIS still holds bits:
> > PxIS keeps its content, HOST_IRQ_STAT reads back as 0, the port is never
> > looked at again, and the command in flight only ends in a timeout.
> > 
> > [...]
> 
> Applied to libata/linux.git (for-7.4), thanks!
> 
> [1/1] ata: ahci: work around lost interrupts on Marvell 88SE61xx
>       https://git.kernel.org/libata/linux/c/22f2ba34

Decided to send this for 7.3-rc2 instead:
https://lore.kernel.org/linux-ide/20260904144228.1473602-1-cassel@kernel.org/T/#u


Kind regards,
Niklas

Re: [PATCH v3] ata: ahci: work around lost interrupts on Marvell 88SE61xx

[Hajo Noerenberg]

On 04.09.2026 at 16:45 Niklas Cassel wrote:
> On Mon, Aug 31, 2026 at 04:27:24PM +0200, Niklas Cassel wrote:
>> On Mon, 31 Aug 2026 14:43:03 +0200, Hajo Noerenberg wrote:
>>> ahci_single_level_irq_intr() services the ports first and clears the
>>> global HOST_IRQ_STAT afterwards, as recommended by AHCI 1.1 section
>>> 10.6.2.  The Marvell 88SE6111/6121/6145 family stops reporting interrupts
>>> for a port when HOST_IRQ_STAT is cleared while PxIS still holds bits:
>>> PxIS keeps its content, HOST_IRQ_STAT reads back as 0, the port is never
>>> looked at again, and the command in flight only ends in a timeout.
>>>
>>> [...]
>>
>> Applied to libata/linux.git (for-7.4), thanks!
>>
>> [1/1] ata: ahci: work around lost interrupts on Marvell 88SE61xx
>>       https://git.kernel.org/libata/linux/c/22f2ba34
> 
> Decided to send this for 7.3-rc2 instead:
> https://lore.kernel.org/linux-ide/20260904144228.1473602-1-cassel@kernel.org/T/#u
> 
Niklas,

thanks for moving it to 7.3-rc2.

I spent a lot of time on this bug, mostly because I did not know what I was
looking for.  Now that it has a name, searching for it actually works - and
out of idle curiosity I went looking a moment ago, only to find that FreeBSD
has been carrying the same workaround for years.

In FreeBSD's sys/dev/ahci/ahci_pci.c all four chips of the family are tagged
AHCI_Q_EDGEIS:

  {0x611111ab, ... "Marvell 88SE6111", ... AHCI_Q_1CH | AHCI_Q_EDGEIS},
  {0x612111ab, ... "Marvell 88SE6121", ... AHCI_Q_2CH | AHCI_Q_EDGEIS | ...},
  {0x614111ab, ... "Marvell 88SE6141", ... AHCI_Q_4CH | AHCI_Q_EDGEIS | ...},
  {0x614511ab, ... "Marvell 88SE6145", ... AHCI_Q_4CH | AHCI_Q_EDGEIS | ...},

and in ahci_intr() the quirk does precisely what this patch does:

  /* Some controllers have edge triggered IS. */
  if (ctlr->quirks & AHCI_Q_EDGEIS)
          ise |= is;
  if (ise != 0)
          ATA_OUTL(ctlr->r_mem, AHCI_IS, ise);
  for (...)
          ctlr->interrupt[unit].function(arg);
  /* AHCI declares level triggered IS. */
  if (!(ctlr->quirks & AHCI_Q_EDGEIS))
          ATA_OUTL(ctlr->r_mem, AHCI_IS, is);

That is an independent confirmation of both the diagnosis and the fix, by
people who knew nothing about this bug report.

It also suggests Linux covers less of the family than it could.  board_ahci_mv
lists 0x6121 and 0x6145 only, FreeBSD tags 0x6111 and 0x6141 as well, and
Marvell's own driver for these chips knows 6101, 6111, 6121, 6122, 6141 and
6145.  A 88SE6111 or 88SE6141 in AHCI mode therefore ends up on the generic
class entry and plain board_ahci - not only without the new handler, but also
without AHCI_HFLAG_NO_NCQ and AHCI_HFLAG_NO_PMP that the two listed siblings
get.  I have neither chip and no report of one failing, so this is a remark,
not a patch proposal.

FreeBSD does make a silicon revision distinction, but only for the 88SE912x:
1b4b:9123 gets AHCI_Q_EDGEIS below revision 0x11 and AHCI_Q_ALTSIG from 0x11
on, so Marvell appears to have fixed it there in later silicon.  Linux has no
special handling for either of those - 1b4b:9120 is not in ahci.c at all, and
1b4b:9123 gets board_ahci_yes_fbs.  I have none of that hardware either, so
again, an observation and nothing more.

For the 6111/6121/6141/6145 family there is no such silicon revision
distinction anywhere.  FreeBSD sets AHCI_Q_EDGEIS for every revision of all
four, and Marvell's own driver does the reordering unconditionally at all
three places it acknowledges interrupts, with no revision check near any of
them - the only revision-dependent branch in that whole driver is for an
unrelated chip.  Applying the fix unconditionally, as this patch does, is
therefore in line with both.

Unrelated to the interrupt problem: FreeBSD also tags these chips
AHCI_Q_NOCOUNT, which makes it ignore the command header's bytecount field
when computing the transfer residual.  I have seen no symptom of that on Linux
and have not checked whether libata uses that field at all; mentioning it for
completeness.

Thanks again,
Hajo