sata_vsc: refactor vsc_sata_interrupt and hook up error handling

Separate sata_vsc interrupt handling into a normal (per-port) path and an
error path with the addition of vsc_port_intr and vsc_error_intr
respectively.  The error path handles interrupt based
hotplug events which requires the definition of vsc_freeze and vsc_thaw.

Note: vsc_port_intr has a workaround for unexpected interrupts that occur
during polled commands.  This fixes a regression between 2.6.19 and 2.6.20.

Changes in take2:
* removed definition of invalid fis bit
* let standard ata-error-handling handle the serror register
* clear all unhandled interrupts
* revert changes to vsc_intr_mask_update (vsc_thaw enables all interrupts)
* use unlikely() for the pci-abort and not-our-interrupt cases in vsc_sata_interrupt

Changes in take3:
* Unify the "add" + "hook-up" patches into this single patch

[htejun@gmail.com: clean up comments and suggestions]
Cc: Jeremy Higdon <jeremy@sgi.com>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
Signed-off-by: Jeff Garzik <jeff@garzik.org>

diff --git a/drivers/ata/sata_vsc.c b/drivers/ata/sata_vsc.c
index 2fd037bde090d92641a954d82d2017ce3aa375ca..f0d86cbe2e59b5ae9b250e5a4a7fc92e70b7e78a 100644 (file)
--- a/drivers/ata/sata_vsc.c
+++ b/drivers/ata/sata_vsc.c
@@ -98,10 +98,6 @@ enum {
                              VSC_SATA_INT_PHY_CHANGE),
 };
 
-#define is_vsc_sata_int_err(port_idx, int_status) \
-        (int_status & (VSC_SATA_INT_ERROR << (8 * port_idx)))
-
-
 static u32 vsc_sata_scr_read (struct ata_port *ap, unsigned int sc_reg)
 {
        if (sc_reg > SCR_CONTROL)
@@ -119,6 +115,28 @@ static void vsc_sata_scr_write (struct ata_port *ap, unsigned int sc_reg,
 }
 
 
+static void vsc_freeze(struct ata_port *ap)
+{
+       void __iomem *mask_addr;
+
+       mask_addr = ap->host->iomap[VSC_MMIO_BAR] +
+               VSC_SATA_INT_MASK_OFFSET + ap->port_no;
+
+       writeb(0, mask_addr);
+}
+
+
+static void vsc_thaw(struct ata_port *ap)
+{
+       void __iomem *mask_addr;
+
+       mask_addr = ap->host->iomap[VSC_MMIO_BAR] +
+               VSC_SATA_INT_MASK_OFFSET + ap->port_no;
+
+       writeb(0xff, mask_addr);
+}
+
+
 static void vsc_intr_mask_update(struct ata_port *ap, u8 ctl)
 {
        void __iomem *mask_addr;
@@ -203,6 +221,36 @@ static void vsc_sata_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
         }
 }
 
+static inline void vsc_error_intr(u8 port_status, struct ata_port *ap)
+{
+       if (port_status & (VSC_SATA_INT_PHY_CHANGE | VSC_SATA_INT_ERROR_M))
+               ata_port_freeze(ap);
+       else
+               ata_port_abort(ap);
+}
+
+static void vsc_port_intr(u8 port_status, struct ata_port *ap)
+{
+       struct ata_queued_cmd *qc;
+       int handled = 0;
+
+       if (unlikely(port_status & VSC_SATA_INT_ERROR)) {
+               vsc_error_intr(port_status, ap);
+               return;
+       }
+
+       qc = ata_qc_from_tag(ap, ap->active_tag);
+       if (qc && likely(!(qc->tf.flags & ATA_TFLAG_POLLING)))
+               handled = ata_host_intr(ap, qc);
+
+       /* We received an interrupt during a polled command,
+        * or some other spurious condition.  Interrupt reporting
+        * with this hardware is fairly reliable so it is safe to
+        * simply clear the interrupt
+        */
+       if (unlikely(!handled))
+               ata_chk_status(ap);
+}
 
 /*
  * vsc_sata_interrupt
@@ -214,59 +262,36 @@ static irqreturn_t vsc_sata_interrupt (int irq, void *dev_instance)
        struct ata_host *host = dev_instance;
        unsigned int i;
        unsigned int handled = 0;
-       u32 int_status;
-
-       spin_lock(&host->lock);
+       u32 status;
 
-       int_status = readl(host->iomap[VSC_MMIO_BAR] +
-                          VSC_SATA_INT_STAT_OFFSET);
+       status = readl(host->iomap[VSC_MMIO_BAR] + VSC_SATA_INT_STAT_OFFSET);
 
-       for (i = 0; i < host->n_ports; i++) {
-               if (int_status & ((u32) 0xFF << (8 * i))) {
-                       struct ata_port *ap;
+       if (unlikely(status == 0xffffffff || status == 0)) {
+               if (status)
+                       dev_printk(KERN_ERR, host->dev,
+                               ": IRQ status == 0xffffffff, "
+                               "PCI fault or device removal?\n");
+               goto out;
+       }
 
-                       ap = host->ports[i];
+       spin_lock(&host->lock);
 
-                       if (is_vsc_sata_int_err(i, int_status)) {
-                               u32 err_status;
-                               printk(KERN_DEBUG "%s: ignoring interrupt(s)\n", __FUNCTION__);
-                               err_status = ap ? vsc_sata_scr_read(ap, SCR_ERROR) : 0;
-                               vsc_sata_scr_write(ap, SCR_ERROR, err_status);
-                               handled++;
-                       }
+       for (i = 0; i < host->n_ports; i++) {
+               u8 port_status = (status >> (8 * i)) & 0xff;
+               if (port_status) {
+                       struct ata_port *ap = host->ports[i];
 
                        if (ap && !(ap->flags & ATA_FLAG_DISABLED)) {
-                               struct ata_queued_cmd *qc;
-
-                               qc = ata_qc_from_tag(ap, ap->active_tag);
-                               if (qc && (!(qc->tf.flags & ATA_TFLAG_POLLING)))
-                                       handled += ata_host_intr(ap, qc);
-                               else if (is_vsc_sata_int_err(i, int_status)) {
-                                       /*
-                                        * On some chips (i.e. Intel 31244), an error
-                                        * interrupt will sneak in at initialization
-                                        * time (phy state changes).  Clearing the SCR
-                                        * error register is not required, but it prevents
-                                        * the phy state change interrupts from recurring
-                                        * later.
-                                        */
-                                       u32 err_status;
-                                       err_status = vsc_sata_scr_read(ap, SCR_ERROR);
-                                       printk(KERN_DEBUG "%s: clearing interrupt, "
-                                              "status %x; sata err status %x\n",
-                                              __FUNCTION__,
-                                              int_status, err_status);
-                                       vsc_sata_scr_write(ap, SCR_ERROR, err_status);
-                                       /* Clear interrupt status */
-                                       ata_chk_status(ap);
-                                       handled++;
-                               }
-                       }
+                               vsc_port_intr(port_status, ap);
+                               handled++;
+                       } else
+                               dev_printk(KERN_ERR, host->dev,
+                                       ": interrupt from disabled port %d\n", i);
                }
        }
 
        spin_unlock(&host->lock);
-
+out:
        return IRQ_RETVAL(handled);
 }
 
@@ -304,8 +329,8 @@ static const struct ata_port_operations vsc_sata_ops = {
        .qc_prep                = ata_qc_prep,
        .qc_issue               = ata_qc_issue_prot,
        .data_xfer              = ata_data_xfer,
-       .freeze                 = ata_bmdma_freeze,
-       .thaw                   = ata_bmdma_thaw,
+       .freeze                 = vsc_freeze,
+       .thaw                   = vsc_thaw,
        .error_handler          = ata_bmdma_error_handler,
        .post_internal_cmd      = ata_bmdma_post_internal_cmd,
        .irq_handler            = vsc_sata_interrupt,