Commit 63af2a5c authored by Mark Lord's avatar Mark Lord Committed by Jeff Garzik

[PATCH] sata_mv: three bug fixes

(1) A DMA transfer size of 0x10000 was not being written
as 0x0000 in the PRDs.  Fixed.

(1) The DEV_IRQ interrupt cause bit happens spuriously
during EDMA operation, and was not being ignored by the driver.
This led to various "drive busy" errors being reported,
with associated unpredictable behaviour.  Fixed.

(2) If a SATA or PCI interrupt was received with no outstanding
command, the interrupt handler still attempted to invoke
ata_qc_complete(), triggering assert()/BUG_ON() behaviour
elsewhere in libata.  Fixed.

The driver still has issues with confusion after error-recovery,
but should now  be reliable in the absence of drive errors.
I will be looking more into the error-handling bugs next.
Signed-Off-By: default avatarMark Lord <mlord@pobox.com>
Signed-off-by: default avatarJeff Garzik <jeff@garzik.org>
parent 7705a879
...@@ -915,7 +915,7 @@ static void mv_fill_sg(struct ata_queued_cmd *qc) ...@@ -915,7 +915,7 @@ static void mv_fill_sg(struct ata_queued_cmd *qc)
pp->sg_tbl[i].addr = cpu_to_le32(addr & 0xffffffff); pp->sg_tbl[i].addr = cpu_to_le32(addr & 0xffffffff);
pp->sg_tbl[i].addr_hi = cpu_to_le32((addr >> 16) >> 16); pp->sg_tbl[i].addr_hi = cpu_to_le32((addr >> 16) >> 16);
pp->sg_tbl[i].flags_size = cpu_to_le32(len); pp->sg_tbl[i].flags_size = cpu_to_le32(len & 0xffff);
sg_len -= len; sg_len -= len;
addr += len; addr += len;
...@@ -1187,7 +1187,6 @@ static void mv_host_intr(struct ata_host_set *host_set, u32 relevant, ...@@ -1187,7 +1187,6 @@ static void mv_host_intr(struct ata_host_set *host_set, u32 relevant,
{ {
void __iomem *mmio = host_set->mmio_base; void __iomem *mmio = host_set->mmio_base;
void __iomem *hc_mmio = mv_hc_base(mmio, hc); void __iomem *hc_mmio = mv_hc_base(mmio, hc);
struct ata_port *ap;
struct ata_queued_cmd *qc; struct ata_queued_cmd *qc;
u32 hc_irq_cause; u32 hc_irq_cause;
int shift, port, port0, hard_port, handled; int shift, port, port0, hard_port, handled;
...@@ -1210,25 +1209,31 @@ static void mv_host_intr(struct ata_host_set *host_set, u32 relevant, ...@@ -1210,25 +1209,31 @@ static void mv_host_intr(struct ata_host_set *host_set, u32 relevant,
hc,relevant,hc_irq_cause); hc,relevant,hc_irq_cause);
for (port = port0; port < port0 + MV_PORTS_PER_HC; port++) { for (port = port0; port < port0 + MV_PORTS_PER_HC; port++) {
ap = host_set->ports[port]; struct ata_port *ap = host_set->ports[port];
struct mv_port_priv *pp = ap->private_data;
hard_port = port & MV_PORT_MASK; /* range 0-3 */ hard_port = port & MV_PORT_MASK; /* range 0-3 */
handled = 0; /* ensure ata_status is set if handled++ */ handled = 0; /* ensure ata_status is set if handled++ */
if ((CRPB_DMA_DONE << hard_port) & hc_irq_cause) { /* Note that DEV_IRQ might happen spuriously during EDMA,
/* new CRPB on the queue; just one at a time until NCQ * and should be ignored in such cases. We could mask it,
*/ * but it's pretty rare and may not be worth the overhead.
ata_status = mv_get_crpb_status(ap); */
handled++; if (pp->pp_flags & MV_PP_FLAG_EDMA_EN) {
} else if ((DEV_IRQ << hard_port) & hc_irq_cause) { /* EDMA: check for response queue interrupt */
/* received ATA IRQ; read the status reg to clear INTRQ if ((CRPB_DMA_DONE << hard_port) & hc_irq_cause) {
*/ ata_status = mv_get_crpb_status(ap);
ata_status = readb((void __iomem *) handled = 1;
}
} else {
/* PIO: check for device (drive) interrupt */
if ((DEV_IRQ << hard_port) & hc_irq_cause) {
ata_status = readb((void __iomem *)
ap->ioaddr.status_addr); ap->ioaddr.status_addr);
handled++; handled = 1;
}
} }
if (ap && if (ap->flags & (ATA_FLAG_PORT_DISABLED | ATA_FLAG_NOINTR))
(ap->flags & (ATA_FLAG_PORT_DISABLED | ATA_FLAG_NOINTR)))
continue; continue;
err_mask = ac_err_mask(ata_status); err_mask = ac_err_mask(ata_status);
...@@ -1240,12 +1245,12 @@ static void mv_host_intr(struct ata_host_set *host_set, u32 relevant, ...@@ -1240,12 +1245,12 @@ static void mv_host_intr(struct ata_host_set *host_set, u32 relevant,
if ((PORT0_ERR << shift) & relevant) { if ((PORT0_ERR << shift) & relevant) {
mv_err_intr(ap); mv_err_intr(ap);
err_mask |= AC_ERR_OTHER; err_mask |= AC_ERR_OTHER;
handled++; handled = 1;
} }
if (handled && ap) { if (handled) {
qc = ata_qc_from_tag(ap, ap->active_tag); qc = ata_qc_from_tag(ap, ap->active_tag);
if (NULL != qc) { if (qc && (qc->flags & ATA_QCFLAG_ACTIVE)) {
VPRINTK("port %u IRQ found for qc, " VPRINTK("port %u IRQ found for qc, "
"ata_status 0x%x\n", port,ata_status); "ata_status 0x%x\n", port,ata_status);
/* mark qc status appropriately */ /* mark qc status appropriately */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment