libata-sff: separate out BMDMA EH
Tejun Heo [Mon, 10 May 2010 19:41:39 +0000 (21:41 +0200)]
Some of error handling logic in ata_sff_error_handler() and all of
ata_sff_post_internal_cmd() are for BMDMA.  Create
ata_bmdma_error_handler() and ata_bmdma_post_internal_cmd() and move
BMDMA part into those.

While at it, change DMA protocol check to ata_is_dma(), fix
post_internal_cmd to call ap->ops->bmdma_stop instead of directly
calling ata_bmdma_stop() and open code hardreset selection so that
ata_std_error_handler() doesn't have to know about sff hardreset.

As these two functions are BMDMA specific, there's no reason to check
for bmdma_addr before calling bmdma methods if the protocol of the
failed command is DMA.  sata_mv and pata_mpc52xx now don't need to set
.post_internal_cmd to ATA_OP_NULL and pata_icside and sata_qstor don't
need to set it to their bmdma_stop routines.

ata_sff_post_internal_cmd() becomes noop and is removed.

This fixes p3 described in clean-up-BMDMA-initialization patch.

Signed-off-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Jeff Garzik <jgarzik@redhat.com>

drivers/ata/libata-eh.c
drivers/ata/libata-sff.c
drivers/ata/libata.h
drivers/ata/pata_icside.c
drivers/ata/pata_scc.c
drivers/ata/sata_nv.c
drivers/ata/sata_promise.c
drivers/ata/sata_qstor.c
drivers/ata/sata_sx4.c
include/linux/libata.h

index d6e6748..f77a673 100644 (file)
@@ -3684,7 +3684,7 @@ void ata_std_error_handler(struct ata_port *ap)
        ata_reset_fn_t hardreset = ops->hardreset;
 
        /* ignore built-in hardreset if SCR access is not available */
-       if (ata_is_builtin_hardreset(hardreset) && !sata_scr_valid(&ap->link))
+       if (hardreset == sata_std_hardreset && !sata_scr_valid(&ap->link))
                hardreset = NULL;
 
        ata_do_eh(ap, ops->prereset, ops->softreset, hardreset, ops->postreset);
index e78ad76..aa378c0 100644 (file)
@@ -56,7 +56,6 @@ const struct ata_port_operations ata_sff_port_ops = {
        .hardreset              = sata_sff_hardreset,
        .postreset              = ata_sff_postreset,
        .error_handler          = ata_sff_error_handler,
-       .post_internal_cmd      = ata_sff_post_internal_cmd,
 
        .sff_dev_select         = ata_sff_dev_select,
        .sff_check_status       = ata_sff_check_status,
@@ -2361,7 +2360,7 @@ void ata_sff_drain_fifo(struct ata_queued_cmd *qc)
 EXPORT_SYMBOL_GPL(ata_sff_drain_fifo);
 
 /**
- *     ata_sff_error_handler - Stock error handler for BMDMA controller
+ *     ata_sff_error_handler - Stock error handler for SFF controller
  *     @ap: port to handle error for
  *
  *     Stock error handler for SFF controller.  It can handle both
@@ -2378,64 +2377,32 @@ void ata_sff_error_handler(struct ata_port *ap)
        ata_reset_fn_t hardreset = ap->ops->hardreset;
        struct ata_queued_cmd *qc;
        unsigned long flags;
-       bool thaw = false;
 
        qc = __ata_qc_from_tag(ap, ap->link.active_tag);
        if (qc && !(qc->flags & ATA_QCFLAG_FAILED))
                qc = NULL;
 
-       /* reset PIO HSM and stop DMA engine */
        spin_lock_irqsave(ap->lock, flags);
 
-       if (ap->ioaddr.bmdma_addr &&
-           qc && (qc->tf.protocol == ATA_PROT_DMA ||
-                  qc->tf.protocol == ATAPI_PROT_DMA)) {
-               u8 host_stat;
-
-               host_stat = ap->ops->bmdma_status(ap);
-
-               /* BMDMA controllers indicate host bus error by
-                * setting DMA_ERR bit and timing out.  As it wasn't
-                * really a timeout event, adjust error mask and
-                * cancel frozen state.
-                */
-               if (qc->err_mask == AC_ERR_TIMEOUT
-                                               && (host_stat & ATA_DMA_ERR)) {
-                       qc->err_mask = AC_ERR_HOST_BUS;
-                       thaw = true;
-               }
-
-               ap->ops->bmdma_stop(qc);
-
-               /* if we're gonna thaw, make sure IRQ is clear */
-               if (thaw) {
-                       ap->ops->sff_check_status(ap);
-                       ap->ops->sff_irq_clear(ap);
-
-                       spin_unlock_irqrestore(ap->lock, flags);
-                       ata_eh_thaw_port(ap);
-                       spin_lock_irqsave(ap->lock, flags);
-               }
-       }
-
-       /* We *MUST* do FIFO draining before we issue a reset as several
-        * devices helpfully clear their internal state and will lock solid
-        * if we touch the data port post reset. Pass qc in case anyone wants
-        *  to do different PIO/DMA recovery or has per command fixups
+       /*
+        * We *MUST* do FIFO draining before we issue a reset as
+        * several devices helpfully clear their internal state and
+        * will lock solid if we touch the data port post reset. Pass
+        * qc in case anyone wants to do different PIO/DMA recovery or
+        * has per command fixups
         */
        if (ap->ops->sff_drain_fifo)
                ap->ops->sff_drain_fifo(qc);
 
        spin_unlock_irqrestore(ap->lock, flags);
 
-       /* PIO and DMA engines have been stopped, perform recovery */
-
-       /* Ignore ata_sff_softreset if ctl isn't accessible and
-        * built-in hardresets if SCR access isn't available.
-        */
+       /* ignore ata_sff_softreset if ctl isn't accessible */
        if (softreset == ata_sff_softreset && !ap->ioaddr.ctl_addr)
                softreset = NULL;
-       if (ata_is_builtin_hardreset(hardreset) && !sata_scr_valid(&ap->link))
+
+       /* ignore built-in hardresets if SCR access is not available */
+       if ((hardreset == sata_std_hardreset ||
+            hardreset == sata_sff_hardreset) && !sata_scr_valid(&ap->link))
                hardreset = NULL;
 
        ata_do_eh(ap, ap->ops->prereset, softreset, hardreset,
@@ -2444,27 +2411,6 @@ void ata_sff_error_handler(struct ata_port *ap)
 EXPORT_SYMBOL_GPL(ata_sff_error_handler);
 
 /**
- *     ata_sff_post_internal_cmd - Stock post_internal_cmd for SFF controller
- *     @qc: internal command to clean up
- *
- *     LOCKING:
- *     Kernel thread context (may sleep)
- */
-void ata_sff_post_internal_cmd(struct ata_queued_cmd *qc)
-{
-       struct ata_port *ap = qc->ap;
-       unsigned long flags;
-
-       spin_lock_irqsave(ap->lock, flags);
-
-       if (ap->ioaddr.bmdma_addr)
-               ap->ops->bmdma_stop(qc);
-
-       spin_unlock_irqrestore(ap->lock, flags);
-}
-EXPORT_SYMBOL_GPL(ata_sff_post_internal_cmd);
-
-/**
  *     ata_sff_std_ports - initialize ioaddr with standard port offsets.
  *     @ioaddr: IO address structure to be initialized
  *
@@ -2811,6 +2757,9 @@ EXPORT_SYMBOL_GPL(ata_pci_sff_init_one);
 const struct ata_port_operations ata_bmdma_port_ops = {
        .inherits               = &ata_sff_port_ops,
 
+       .error_handler          = ata_bmdma_error_handler,
+       .post_internal_cmd      = ata_bmdma_post_internal_cmd,
+
        .bmdma_setup            = ata_bmdma_setup,
        .bmdma_start            = ata_bmdma_start,
        .bmdma_stop             = ata_bmdma_stop,
@@ -2829,6 +2778,84 @@ const struct ata_port_operations ata_bmdma32_port_ops = {
 EXPORT_SYMBOL_GPL(ata_bmdma32_port_ops);
 
 /**
+ *     ata_bmdma_error_handler - Stock error handler for BMDMA controller
+ *     @ap: port to handle error for
+ *
+ *     Stock error handler for BMDMA controller.  It can handle both
+ *     PATA and SATA controllers.  Most BMDMA controllers should be
+ *     able to use this EH as-is or with some added handling before
+ *     and after.
+ *
+ *     LOCKING:
+ *     Kernel thread context (may sleep)
+ */
+void ata_bmdma_error_handler(struct ata_port *ap)
+{
+       struct ata_queued_cmd *qc;
+       unsigned long flags;
+       bool thaw = false;
+
+       qc = __ata_qc_from_tag(ap, ap->link.active_tag);
+       if (qc && !(qc->flags & ATA_QCFLAG_FAILED))
+               qc = NULL;
+
+       /* reset PIO HSM and stop DMA engine */
+       spin_lock_irqsave(ap->lock, flags);
+
+       if (qc && ata_is_dma(qc->tf.protocol)) {
+               u8 host_stat;
+
+               host_stat = ap->ops->bmdma_status(ap);
+
+               /* BMDMA controllers indicate host bus error by
+                * setting DMA_ERR bit and timing out.  As it wasn't
+                * really a timeout event, adjust error mask and
+                * cancel frozen state.
+                */
+               if (qc->err_mask == AC_ERR_TIMEOUT && (host_stat & ATA_DMA_ERR)) {
+                       qc->err_mask = AC_ERR_HOST_BUS;
+                       thaw = true;
+               }
+
+               ap->ops->bmdma_stop(qc);
+
+               /* if we're gonna thaw, make sure IRQ is clear */
+               if (thaw) {
+                       ap->ops->sff_check_status(ap);
+                       ap->ops->sff_irq_clear(ap);
+               }
+       }
+
+       spin_unlock_irqrestore(ap->lock, flags);
+
+       if (thaw)
+               ata_eh_thaw_port(ap);
+
+       ata_sff_error_handler(ap);
+}
+EXPORT_SYMBOL_GPL(ata_bmdma_error_handler);
+
+/**
+ *     ata_bmdma_post_internal_cmd - Stock post_internal_cmd for BMDMA
+ *     @qc: internal command to clean up
+ *
+ *     LOCKING:
+ *     Kernel thread context (may sleep)
+ */
+void ata_bmdma_post_internal_cmd(struct ata_queued_cmd *qc)
+{
+       struct ata_port *ap = qc->ap;
+       unsigned long flags;
+
+       if (ata_is_dma(qc->tf.protocol)) {
+               spin_lock_irqsave(ap->lock, flags);
+               ap->ops->bmdma_stop(qc);
+               spin_unlock_irqrestore(ap->lock, flags);
+       }
+}
+EXPORT_SYMBOL_GPL(ata_bmdma_post_internal_cmd);
+
+/**
  *     ata_bmdma_setup - Set up PCI IDE BMDMA transaction
  *     @qc: Info associated with this ATA transaction.
  *
index 002390c..4b84ed6 100644 (file)
@@ -38,17 +38,6 @@ struct ata_scsi_args {
        void                    (*done)(struct scsi_cmnd *);
 };
 
-static inline int ata_is_builtin_hardreset(ata_reset_fn_t reset)
-{
-       if (reset == sata_std_hardreset)
-               return 1;
-#ifdef CONFIG_ATA_SFF
-       if (reset == sata_sff_hardreset)
-               return 1;
-#endif
-       return 0;
-}
-
 /* libata-core.c */
 enum {
        /* flags for ata_dev_read_id() */
index ee85a9c..b56e8f7 100644 (file)
@@ -333,7 +333,6 @@ static struct ata_port_operations pata_icside_port_ops = {
        .cable_detect           = ata_cable_40wire,
        .set_dmamode            = pata_icside_set_dmamode,
        .postreset              = pata_icside_postreset,
-       .post_internal_cmd      = pata_icside_bmdma_stop,
 
        .port_start             = ATA_OP_NULL,  /* don't need PRD table */
 };
index 70d549e..93f690e 100644 (file)
@@ -951,7 +951,6 @@ static struct ata_port_operations scc_pata_ops = {
        .prereset               = scc_pata_prereset,
        .softreset              = scc_softreset,
        .postreset              = scc_postreset,
-       .post_internal_cmd      = scc_bmdma_stop,
 
        .sff_irq_clear          = scc_irq_clear,
 
index a007b20..64e9982 100644 (file)
@@ -1131,7 +1131,7 @@ static void nv_adma_post_internal_cmd(struct ata_queued_cmd *qc)
        struct nv_adma_port_priv *pp = qc->ap->private_data;
 
        if (pp->flags & NV_ADMA_PORT_REGISTER_MODE)
-               ata_sff_post_internal_cmd(qc);
+               ata_bmdma_post_internal_cmd(qc);
 }
 
 static int nv_adma_port_start(struct ata_port *ap)
@@ -1739,7 +1739,7 @@ static void nv_adma_error_handler(struct ata_port *ap)
                readw(mmio + NV_ADMA_CTL);      /* flush posted write */
        }
 
-       ata_sff_error_handler(ap);
+       ata_bmdma_error_handler(ap);
 }
 
 static void nv_swncq_qc_to_dq(struct ata_port *ap, struct ata_queued_cmd *qc)
@@ -1865,7 +1865,7 @@ static void nv_swncq_error_handler(struct ata_port *ap)
                ehc->i.action |= ATA_EH_RESET;
        }
 
-       ata_sff_error_handler(ap);
+       ata_bmdma_error_handler(ap);
 }
 
 #ifdef CONFIG_PM
index e80628a..09a6179 100644 (file)
@@ -839,7 +839,7 @@ static void pdc_error_handler(struct ata_port *ap)
        if (!(ap->pflags & ATA_PFLAG_FROZEN))
                pdc_reset_port(ap);
 
-       ata_std_error_handler(ap);
+       ata_sff_error_handler(ap);
 }
 
 static void pdc_post_internal_cmd(struct ata_queued_cmd *qc)
index da84ea9..d3a22f2 100644 (file)
@@ -147,7 +147,6 @@ static struct ata_port_operations qs_ata_ops = {
        .prereset               = qs_prereset,
        .softreset              = ATA_OP_NULL,
        .error_handler          = qs_error_handler,
-       .post_internal_cmd      = ATA_OP_NULL,
        .lost_interrupt         = ATA_OP_NULL,
 
        .scr_read               = qs_scr_read,
@@ -255,7 +254,7 @@ static int qs_scr_read(struct ata_link *link, unsigned int sc_reg, u32 *val)
 static void qs_error_handler(struct ata_port *ap)
 {
        qs_enter_reg_mode(ap);
-       ata_std_error_handler(ap);
+       ata_sff_error_handler(ap);
 }
 
 static int qs_scr_write(struct ata_link *link, unsigned int sc_reg, u32 val)
index a4e552a..bedd518 100644 (file)
@@ -921,7 +921,7 @@ static void pdc_error_handler(struct ata_port *ap)
        if (!(ap->pflags & ATA_PFLAG_FROZEN))
                pdc_reset_port(ap);
 
-       ata_std_error_handler(ap);
+       ata_sff_error_handler(ap);
 }
 
 static void pdc_post_internal_cmd(struct ata_queued_cmd *qc)
index 6888b5c..1d38590 100644 (file)
@@ -1614,7 +1614,6 @@ extern int sata_sff_hardreset(struct ata_link *link, unsigned int *class,
 extern void ata_sff_postreset(struct ata_link *link, unsigned int *classes);
 extern void ata_sff_drain_fifo(struct ata_queued_cmd *qc);
 extern void ata_sff_error_handler(struct ata_port *ap);
-extern void ata_sff_post_internal_cmd(struct ata_queued_cmd *qc);
 extern void ata_sff_std_ports(struct ata_ioports *ioaddr);
 #ifdef CONFIG_PCI
 extern int ata_pci_sff_init_host(struct ata_host *host);
@@ -1629,6 +1628,8 @@ extern int ata_pci_sff_init_one(struct pci_dev *pdev,
                struct scsi_host_template *sht, void *host_priv, int hflags);
 #endif /* CONFIG_PCI */
 
+extern void ata_bmdma_error_handler(struct ata_port *ap);
+extern void ata_bmdma_post_internal_cmd(struct ata_queued_cmd *qc);
 extern void ata_bmdma_setup(struct ata_queued_cmd *qc);
 extern void ata_bmdma_start(struct ata_queued_cmd *qc);
 extern void ata_bmdma_stop(struct ata_queued_cmd *qc);