libata: improve AC_ERR_DEV handling for ->post_internal_cmd

->post_internal_cmd is simplified EH for internal commands.  Its
primary mission is to stop the controller such that no rogue memory
access or other activities occur after the internal command is
released.  It may provide error diagnostics by setting qc->err_mask
but this hasn't been a requirement.

To ignore SETXFER failure for CFA devices, libata needs to know
whether a command was failed by the device or for any other reason.
ie. internal command needs to get AC_ERR_DEV right.

This patch makes the following changes to AC_ERR_DEV handling and
->post_internal_cmd semantics to accomodate this need and simplify
callback implementation.

1. As long as the correct bits in the result TF registers are set,
   there is no need to set AC_ERR_DEV explicitly.  libata EH core
   takes care of that for both normal and internal commands.

2. The only requirement for ->post_internal_cmd() is to put the
   controller into quiescent state.  It needs not to set any err_mask.

3. ata_exec_internal_sg() performs minimal error analysis such that
   AC_ERR_DEV is automatically set as long as result_tf is filled
   correctly.

Signed-off-by: Tejun Heo <htejun@gmail.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index fb9f69252939d97eb3a91a4e0242bf32638115df..cf39987a31c4e89f7874600ea1d576dda8a80e70 100644 (file)
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -1430,10 +1430,7 @@ static void ahci_post_internal_cmd(struct ata_queued_cmd *qc)
        void __iomem *mmio = ap->host->iomap[AHCI_PCI_BAR];
        void __iomem *port_mmio = ahci_port_base(mmio, ap->port_no);
 
-       if (qc->flags & ATA_QCFLAG_FAILED)
-               qc->err_mask |= AC_ERR_OTHER;
-
-       if (qc->err_mask) {
+       if (qc->flags & ATA_QCFLAG_FAILED) {
                /* make DMA engine forget about the failed command */
                ahci_stop_engine(port_mmio);
                ahci_start_engine(port_mmio);

diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
index 96bf86f673882537838ecd1c93b37ce919197214..2f2884b92434fd3c71ba5e297bc707c89aaf5c5e 100644 (file)
--- a/drivers/ata/libata-core.c
+++ b/drivers/ata/libata-core.c
@@ -1270,12 +1270,16 @@ unsigned ata_exec_internal_sg(struct ata_device *dev,
        if (ap->ops->post_internal_cmd)
                ap->ops->post_internal_cmd(qc);
 
-       if ((qc->flags & ATA_QCFLAG_FAILED) && !qc->err_mask) {
-               if (ata_msg_warn(ap))
-                       ata_dev_printk(dev, KERN_WARNING,
-                               "zero err_mask for failed "
-                               "internal command, assuming AC_ERR_OTHER\n");
-               qc->err_mask |= AC_ERR_OTHER;
+       /* perform minimal error analysis */
+       if (qc->flags & ATA_QCFLAG_FAILED) {
+               if (qc->result_tf.command & (ATA_ERR | ATA_DF))
+                       qc->err_mask |= AC_ERR_DEV;
+
+               if (!qc->err_mask)
+                       qc->err_mask |= AC_ERR_OTHER;
+
+               if (qc->err_mask & ~AC_ERR_OTHER)
+                       qc->err_mask &= ~AC_ERR_OTHER;
        }
 
        /* finish up */

diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index 185876aba64742acadef65193c26c286c5e14acf..0dbee550f9e812f22431866896bbee76dfae03f4 100644 (file)
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -1151,7 +1151,9 @@ static unsigned int ata_eh_analyze_tf(struct ata_queued_cmd *qc,
                return ATA_EH_SOFTRESET;
        }
 
-       if (!(qc->err_mask & AC_ERR_DEV))
+       if (stat & (ATA_ERR | ATA_DF))
+               qc->err_mask |= AC_ERR_DEV;
+       else
                return 0;
 
        switch (qc->dev->class) {

diff --git a/drivers/ata/sata_inic162x.c b/drivers/ata/sata_inic162x.c
index 11c3079ab6cc20ed8ff0c4b85493147bdc764923..ca4092aaae5bd98535fa3ed538e7c93e9b8208ca 100644 (file)
--- a/drivers/ata/sata_inic162x.c
+++ b/drivers/ata/sata_inic162x.c
@@ -488,7 +488,7 @@ static void inic_error_handler(struct ata_port *ap)
 static void inic_post_internal_cmd(struct ata_queued_cmd *qc)
 {
        /* make DMA engine forget about the failed command */
-       if (qc->err_mask)
+       if (qc->flags & ATA_QCFLAG_FAILED)
                inic_reset_port(inic_port_base(qc->ap));
 }
 

diff --git a/drivers/ata/sata_promise.c b/drivers/ata/sata_promise.c
index a7916d72c4caccc9282eecc6e19d19faeaa47563..8afde4a9ca7ed2fe199d856f3d1f73da6a572092 100644 (file)
--- a/drivers/ata/sata_promise.c
+++ b/drivers/ata/sata_promise.c
@@ -622,11 +622,8 @@ static void pdc_post_internal_cmd(struct ata_queued_cmd *qc)
 {
        struct ata_port *ap = qc->ap;
 
-       if (qc->flags & ATA_QCFLAG_FAILED)
-               qc->err_mask |= AC_ERR_OTHER;
-
        /* make DMA engine forget about the failed command */
-       if (qc->err_mask)
+       if (qc->flags & ATA_QCFLAG_FAILED)
                pdc_reset_port(ap);
 }
 

diff --git a/drivers/ata/sata_sil24.c b/drivers/ata/sata_sil24.c
index 6698c746e624b63023f18a7586cc43e66652af04..4f522ec04049e6b4de07494ff8353abd4a55c51e 100644 (file)
--- a/drivers/ata/sata_sil24.c
+++ b/drivers/ata/sata_sil24.c
@@ -924,11 +924,8 @@ static void sil24_post_internal_cmd(struct ata_queued_cmd *qc)
 {
        struct ata_port *ap = qc->ap;
 
-       if (qc->flags & ATA_QCFLAG_FAILED)
-               qc->err_mask |= AC_ERR_OTHER;
-
        /* make DMA engine forget about the failed command */
-       if (qc->err_mask)
+       if (qc->flags & ATA_QCFLAG_FAILED)
                sil24_init_port(ap);
 }