Merge tag 'ata-7.3-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/libata/linux

Pull ata fixes from Niklas Cassel:

 - Work around lost interrupts on Marvell 88SE61xx

   The Marvell AHCI controller requires you to clear interrupts in the
   opposite order from what is specified in the AHCI specification in
   order to not lose interrupts (Hajo)

 - Do not raise UNIT ATTENTION for depopulation commands

   The libata completion function unconditionally sets sense data with
   sense key UNIT ATTENTION (UA) for depopulation commands. The SCSI
   layer will fail a command when seeing this sense data. UA is only
   supposed to be raised if the capacity actually changed.

   Since these commands are currently only supported as passthrough
   commands, the user is expected to revalidate the device, which will
   detect a capacity change anyway. Thus drop the unconditional UA until
   a better solution has been implemented (Damien)

* tag 'ata-7.3-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/libata/linux:
  ata: libata-scsi: do not raise UA for storage element depopulation and restoration
  ata: ahci: work around lost interrupts on Marvell 88SE61xx

2 files changed, 49 insertions, 24 deletions

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))
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;
 }