Mark Lord wrote:
Mark Lord wrote:
Tejun Heo wrote:
..
w00t w00t I thought about that when I wrote NCQ EH. All you have to
do is to export ata_eh_analyze_ncq_error() can call it right after
error_handler starts and put the controller into a working state (so
that SCR accesses work again). After it finishes, call the generic
handler. The second time around ata_eh_analyze_ncq_error() will be
no-op and you should get what you want.
..
Tejun, I've implemented this, and it sort of works now.
But libata-eh is interfering in a different way.
It reads the SCR_ERROR register for the PMP link,
notices that the SERR_COMM_RECOVERED bit is set,
and then decides to perform a full reset.
This bit seems to be always set whenever sata_mv tries
to report device-errors (media errors) in NCQ for a pmp.
I'll have to dig some more, but it's probably just leftover
from the initial probe/reset time or something.
This is a Marvell PM.
My understanding was, that this particular bit is supposed
to be for information purposes only, letting us know that
the hardware has automatically recovered from a soft error.
So why are we taking a hammer to things there?
..
FWIW, this patch fixes it for me (and fixes a misleading printk).
Or I could just clear that bit from sata_mv before invoking EH.
..
Mmmm.. my first attempts to clear it from within sata_mv didn't work.
But the patch below is still fine. (?)
----------------- SNIP ------------------
Prevent libata-eh from taking too drastic an action in response
to a simple SATA "Recovered Communication Error".
Fix a misleading (port vs. link) printk while we're at it.
Signed-off-by: Mark Lord <mlord@xxxxxxxxx>
--- upstream/drivers/ata/libata-eh.c 2008-04-30 17:35:36.000000000 -0400
+++ linux/drivers/ata/libata-eh.c 2008-04-30 17:35:45.000000000 -0400
@@ -1312,8 +1312,7 @@
err_mask |= AC_ERR_ATA_BUS;
action |= ATA_EH_RESET;
}
- if (serror &
- (SERR_DATA_RECOVERED | SERR_COMM_RECOVERED | SERR_DATA)) {
+ if (serror & (SERR_DATA_RECOVERED | SERR_DATA)) {
err_mask |= AC_ERR_ATA_BUS;
action |= ATA_EH_RESET;
}
@@ -1924,7 +1923,7 @@
}
if (ehc->i.serror)
- ata_port_printk(ap, KERN_ERR,
+ ata_link_printk(link, KERN_ERR,
"SError: { %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s}\n",
ehc->i.serror & SERR_DATA_RECOVERED ? "RecovData " : "",
ehc->i.serror & SERR_COMM_RECOVERED ? "RecovComm " : "",
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
