From 8d07379d251ab24d937e6cb0748b71106dddbc74 Mon Sep 17 00:00:00 2001 From: Mark Lord Date: Sat, 19 Apr 2008 15:07:49 -0400 Subject: [PATCH] sata_mv: leave SError bits untouched in mv_err_intr Here it is again, minus the checkpatch.pl complaint: Rework mv_err_intr() to leave the SError bits as-is, so that libata-eh has a chance to see/use them. We originally thought that clearing them here was necessary before writing back to edma_err_cause (per the Marvell datasheets), but we will end up reseting the chip regardless in those cases. Signed-off-by: Mark Lord Signed-off-by: Jeff Garzik --- drivers/ata/sata_mv.c | 40 +++++++++++++++++++--------------------- 1 file changed, 19 insertions(+), 21 deletions(-) diff --git a/drivers/ata/sata_mv.c b/drivers/ata/sata_mv.c index 944359256959..4fa54805eb41 100644 --- a/drivers/ata/sata_mv.c +++ b/drivers/ata/sata_mv.c @@ -1523,13 +1523,11 @@ static void mv_unexpected_intr(struct ata_port *ap) /** * mv_err_intr - Handle error interrupts on the port * @ap: ATA channel to manipulate - * @reset_allowed: bool: 0 == don't trigger from reset here + * @qc: affected command (non-NCQ), or NULL * - * In most cases, just clear the interrupt and move on. However, - * some cases require an eDMA reset, which also performs a COMRESET. - * The SERR case requires a clear of pending errors in the SATA - * SERROR register. Finally, if the port disabled DMA, - * update our cached copy to match. + * Most cases require a full reset of the chip's state machine, + * which also performs a COMRESET. + * Also, if the port disabled DMA, update our cached copy to match. * * LOCKING: * Inherited from caller. @@ -1540,21 +1538,18 @@ static void mv_err_intr(struct ata_port *ap, struct ata_queued_cmd *qc) u32 edma_err_cause, eh_freeze_mask, serr = 0; struct mv_port_priv *pp = ap->private_data; struct mv_host_priv *hpriv = ap->host->private_data; - unsigned int edma_enabled = (pp->pp_flags & MV_PP_FLAG_EDMA_EN); unsigned int action = 0, err_mask = 0; struct ata_eh_info *ehi = &ap->link.eh_info; ata_ehi_clear_desc(ehi); - if (!edma_enabled) { - /* just a guess: do we need to do this? should we - * expand this, and do it in all cases? - */ - sata_scr_read(&ap->link, SCR_ERROR, &serr); - sata_scr_write_flush(&ap->link, SCR_ERROR, serr); - } - + /* + * Read and clear the err_cause bits. This won't actually + * clear for some errors (eg. SError), but we will be doing + * a hard reset in those cases regardless, which *will* clear it. + */ edma_err_cause = readl(port_mmio + EDMA_ERR_IRQ_CAUSE_OFS); + writelfl(~edma_err_cause, port_mmio + EDMA_ERR_IRQ_CAUSE_OFS); ata_ehi_push_desc(ehi, "edma_err_cause=%08x", edma_err_cause); @@ -1594,16 +1589,19 @@ static void mv_err_intr(struct ata_port *ap, struct ata_queued_cmd *qc) ata_ehi_push_desc(ehi, "EDMA self-disable"); } if (edma_err_cause & EDMA_ERR_SERR) { - sata_scr_read(&ap->link, SCR_ERROR, &serr); - sata_scr_write_flush(&ap->link, SCR_ERROR, serr); - err_mask = AC_ERR_ATA_BUS; + /* + * Ensure that we read our own SCR, not a pmp link SCR: + */ + ap->ops->scr_read(ap, SCR_ERROR, &serr); + /* + * Don't clear SError here; leave it for libata-eh: + */ + ata_ehi_push_desc(ehi, "SError=%08x", serr); + err_mask |= AC_ERR_ATA_BUS; action |= ATA_EH_RESET; } } - /* Clear EDMA now that SERR cleanup done */ - writelfl(~edma_err_cause, port_mmio + EDMA_ERR_IRQ_CAUSE_OFS); - if (!err_mask) { err_mask = AC_ERR_OTHER; action |= ATA_EH_RESET;