From dc3565a4ae538e584e5e63b3b3cd1eaf502593c1 Mon Sep 17 00:00:00 2001 From: Hajo Noerenberg Date: Mon, 31 Aug 2026 14:43:03 +0200 Subject: [PATCH 1/2] ata: ahci: work around lost interrupts on Marvell 88SE61xx 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 Reviewed-by: Damien Le Moal Acked-by: Pali Rohar Link: https://lore.kernel.org/r/20260831124303.920391-1-hajo-linux-ide@noerenberg.de Signed-off-by: Niklas Cassel --- drivers/ata/ahci.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 58f512f8952a..9b8c0935001c 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -2614,6 +2614,51 @@ static irqreturn_t ahci_thunderx_irq_handler(int irq, void *dev_instance) } #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 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) 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)) From 6d81700ad7c4871f94fb72e469cb0f3f55843ef7 Mon Sep 17 00:00:00 2001 From: Damien Le Moal Date: Wed, 2 Sep 2026 11:08:28 +0900 Subject: [PATCH 2/2] ata: libata-scsi: do not raise UA for storage element depopulation and restoration The libata command completion for the ATA commands REMOVE ELEMENT AND TRUNCATE and RESTORE ELEMENTS AND REBUILD is handled using the function ata_scsi_depop_ua_cap_changed_complete(). This completion function raises a UNIT ATTENTION with the additional sense code CAPACITY DATA HAS CHANGED. But doing so, the scsi layer seeing the UNIT ATTENTION sense key ends up failing the command, even if the command result is in fact OK. The SAT specifications do provide more details about the capacity change should be notified, and that relies on the ACCESSIBLE CAPACITY field of the IDENTIFY DATA retrieved before or after the command is issued, and then raising a UNIT ATTENTION if the capacity has really changed. However, we do not have any simple mean to raise a unit attention from libata-scsi. So rather than seeing the REMOVE ELEMENT AND TRUNCATE and RESTORE ELEMENTS AND REBUILD commands failing, remove the function ata_scsi_depop_ua_cap_changed_complete() and rely on the regular completion callback. Since for now these commands can only be issued as passthrough commands, the user is responsible for revalidating the device capacity after executing these commands. Fixes: db496721cb0d ("ata: libata-scsi: add support for the REMOVE ELEMENT AND TRUNCATE command") Fixes: 1e307ca61a9c ("ata: libata-scsi: add support for the RESTORE ELEMENTS AND REBUILD command") Signed-off-by: Damien Le Moal Link: https://lore.kernel.org/r/20260902020828.1436048-1-dlemoal@kernel.org Signed-off-by: Niklas Cassel --- drivers/ata/libata-scsi.c | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c index a7d667cfcfec..b3666519b648 100644 --- a/drivers/ata/libata-scsi.c +++ b/drivers/ata/libata-scsi.c @@ -4823,28 +4823,6 @@ ata_scsi_get_phys_element_status_xlat(struct ata_queued_cmd *qc) return 0; } -static void ata_scsi_depop_ua_cap_changed_complete(struct ata_queued_cmd *qc) -{ - struct scsi_cmnd *scmd = qc->scsicmd; - u8 *cdb = scmd->cmnd; - bool is_ata_passthru = cdb[0] == ATA_16 || cdb[0] == ATA_12; - bool is_success = qc->err_mask == 0; - - /* - * For successful non-passthrough commands, raise a UNIT ATTENTION with - * the additional sense code set to CAPACITY DATA HAS CHANGED to be - * raised. Note that this should be done only if the capacity has - * actually changed, which may not be the case if the element that was - * specified for depopulation was already depopulated, or we did not - * restore any removed element. But a capacity change unit attention is - * harmless, so always raise the unit attention. - */ - if (is_success && !is_ata_passthru) - ata_scsi_set_sense(qc->dev, scmd, UNIT_ATTENTION, - UA_CHANGED_ASC, CAPACITY_CHANGED_ASCQ); - ata_scsi_qc_complete(qc); -} - static unsigned int ata_scsi_remove_element_and_truncate_xlat(struct ata_queued_cmd *qc) { @@ -4884,7 +4862,6 @@ ata_scsi_remove_element_and_truncate_xlat(struct ata_queued_cmd *qc) tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48; qc->flags |= ATA_QCFLAG_RESULT_TF; - qc->complete_fn = ata_scsi_depop_ua_cap_changed_complete; return 0; } @@ -4937,7 +4914,6 @@ ata_scsi_restore_elements_and_rebuild_xlat(struct ata_queued_cmd *qc) tf->flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE | ATA_TFLAG_LBA48; qc->flags |= ATA_QCFLAG_RESULT_TF; - qc->complete_fn = ata_scsi_depop_ua_cap_changed_complete; return 0; }