From: Dan Williams Date: Fri, 23 Feb 2007 23:36:43 +0000 (-0700) Subject: sata_vsc: refactor vsc_sata_interrupt and hook up error handling X-Git-Tag: v2.6.21-rc2~25^2~10 X-Git-Url: http://pilppa.com/gitweb/?a=commitdiff_plain;h=ea34e45a4670c4fa0da3442fc74789fd66c1201b;p=linux-2.6-omap-h63xx.git 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 Signed-off-by: Dan Williams Signed-off-by: Jeff Garzik --- diff --git a/drivers/ata/sata_vsc.c b/drivers/ata/sata_vsc.c index 2fd037bde09..f0d86cbe2e5 100644 --- 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,