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

[PATCH v2] 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.

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>

---
Damien,

as requested, resent with the new title.  The patch itself is byte for byte
v1 [1]; only the commit message changed.

v2:
 - retitle "clear HOST_IRQ_STAT before the ports" -> "work around lost
   interrupts", per review
 - rewrap the commit log at 75 columns and reflow the register trace, which
   had lines up to 79 characters
 - no functional change

The answer to your review question is in that thread as well: registering
ahci_thunderx_irq_handler() unchanged for board_ahci_mv does not work on this
chip.  It still clears HOST_IRQ_STAT after servicing the ports, and the loop
cannot make up for that, because once IS has been written while an unserviced
PxIS bit was standing, this controller never re-asserts it -- the re-read
returns 0 and the loop exits.

[1] https://lore.kernel.org/linux-ide/50ebd35a-086f-40fb-887e-576e36e7a2b8@noerenberg.de/

 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
@@ -1618,6 +1618,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)
 {
@@ -1878,6 +1923,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))

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

[Damien Le Moal]

On 8/28/26 16:05, 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.
> 
> 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.
> 
> 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>

Looks OK to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

-- 
Damien Le Moal
Western Digital Research

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

[Pali Rohár]

On Friday 28 August 2026 09:05:21 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.
> 
> 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.
> 
> 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>

Thank you for successfully addressing this issue after working on it for
a longer time.  It is very nice to see a successful story at the end.

For me the change looks good.

Acked-by: Pali Rohár <pali@kernel.org>

As this change is fixing the support for more disks, I would suggest to
backport this change also into older kernels, ideally by cc: stable
line (so it would be automatic).

> 
> ---
> Damien,
> 
> as requested, resent with the new title.  The patch itself is byte for byte
> v1 [1]; only the commit message changed.
> 
> v2:
>  - retitle "clear HOST_IRQ_STAT before the ports" -> "work around lost
>    interrupts", per review
>  - rewrap the commit log at 75 columns and reflow the register trace, which
>    had lines up to 79 characters
>  - no functional change
> 
> The answer to your review question is in that thread as well: registering
> ahci_thunderx_irq_handler() unchanged for board_ahci_mv does not work on this
> chip.  It still clears HOST_IRQ_STAT after servicing the ports, and the loop
> cannot make up for that, because once IS has been written while an unserviced
> PxIS bit was standing, this controller never re-asserts it -- the re-read
> returns 0 and the loop exits.
> 
> [1] https://lore.kernel.org/linux-ide/50ebd35a-086f-40fb-887e-576e36e7a2b8@noerenberg.de/
> 
>  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
> @@ -1618,6 +1618,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)
>  {
> @@ -1878,6 +1923,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))

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

[Niklas Cassel]

On Fri, Aug 28, 2026 at 09:04:52PM +0200, Pali Rohár wrote:
> On Friday 28 August 2026 09:05:21 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.
> > 
> > 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.
> > 
> > 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>
> 
> Thank you for successfully addressing this issue after working on it for
> a longer time.  It is very nice to see a successful story at the end.
> 
> For me the change looks good.
> 
> Acked-by: Pali Rohár <pali@kernel.org>
> 
> As this change is fixing the support for more disks, I would suggest to
> backport this change also into older kernels, ideally by cc: stable
> line (so it would be automatic).


This patch does not apply.

Looking at the line numbers, this patch looks like it is based on some ancient kernel.

Please:
1) Rebase patch on top of 7.3-rc1
2) Add Cc: stable@vger.kernel.org
3) Add Fixes: cd70c26617f4 ("[libata] AHCI: Add support for Marvell AHCI-like chips (initially 6145)")


Tip: if you use:
$ git format-patch --base=HEAD~ -1

the SHA1 that your patch is based on will be included in the patch trailer.


Kind regards,
Niklas