From: Albert Lee <albertcc@tw.ibm.com>
To: Jeff Garzik <jeff@garzik.org>
Cc: Tejun Heo <htejun@gmail.com>,
IDE Linux <linux-ide@vger.kernel.org>,
Doug Maxey <dwm@maxeymade.com>
Subject: [PATCH/RFC] libata-dev: make the returned err_mask more accurate
Date: Thu, 06 Apr 2006 15:02:36 +0800 [thread overview]
Message-ID: <4434BD0C.9080302@tw.ibm.com> (raw)
make the returned err_mask more accurate
Changes:
- add ac_err_drq() and ac_err_idle() to map err_mask from drv_status for data xfer and idle states.
- fix ata_hsm_move() to use the functions for err_mask mapping.
Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
The current err_mask returned by HSM doesn't look right.
e.g. When we found DRQ=0 when data transfer is expected, AC_ERR_HSM is returned.
However, DRQ=0 maybe caused by device error. So, during data transfer,
if we see DRQ=0 and ERR=1, returning AC_ERR_DEV should be more accurate.
e.g. In HSM_ST_LAST state, AC_ERR_OTHER is returned for BSY=0 DRQ=1 currently.
AC_ERR_HSM shoule be more accurate.
Patch against irq-pio (79fa1b677be3a985cc66b9218a4dd09818f1051b).
For your review, thanks.
--- irq-pio0/drivers/scsi/libata-core.c 2006-04-06 10:08:44.000000000 +0800
+++ irq-pio1/drivers/scsi/libata-core.c 2006-04-06 11:20:15.000000000 +0800
@@ -3906,7 +3906,7 @@ fsm_start:
/* check device status */
if (unlikely((status & (ATA_BUSY | ATA_DRQ)) != ATA_DRQ)) {
/* Wrong status. Let EH handle this */
- qc->err_mask |= AC_ERR_HSM;
+ qc->err_mask |= ac_err_drq(status);
ap->hsm_task_state = HSM_ST_ERR;
goto fsm_start;
}
@@ -3920,7 +3920,7 @@ fsm_start:
if (unlikely(status & (ATA_ERR | ATA_DF))) {
printk(KERN_WARNING "ata%d: DRQ=1 with device error, dev_stat 0x%X\n",
ap->id, status);
- qc->err_mask |= AC_ERR_DEV;
+ qc->err_mask |= AC_ERR_HSM;
ap->hsm_task_state = HSM_ST_ERR;
goto fsm_start;
}
@@ -3976,7 +3976,7 @@ fsm_start:
if (unlikely(status & (ATA_ERR | ATA_DF))) {
printk(KERN_WARNING "ata%d: DRQ=1 with device error, dev_stat 0x%X\n",
ap->id, status);
- qc->err_mask |= AC_ERR_DEV;
+ qc->err_mask |= AC_ERR_HSM;
ap->hsm_task_state = HSM_ST_ERR;
goto fsm_start;
}
@@ -3991,7 +3991,7 @@ fsm_start:
/* ATA PIO protocol */
if (unlikely((status & ATA_DRQ) == 0)) {
/* handle BSY=0, DRQ=0 as error */
- qc->err_mask |= AC_ERR_HSM;
+ qc->err_mask |= ac_err_drq(status);
ap->hsm_task_state = HSM_ST_ERR;
goto fsm_start;
}
@@ -4007,13 +4007,16 @@ fsm_start:
* transferred to the device.
*/
if (unlikely(status & (ATA_ERR | ATA_DF))) {
- /* data might be corrputed */
- qc->err_mask |= AC_ERR_DEV;
- if (!(qc->tf.flags & ATA_TFLAG_WRITE)) {
+ if (qc->tf.flags & ATA_TFLAG_WRITE)
+ qc->err_mask |= AC_ERR_HSM;
+ else {
+ /* data might be corrputed */
+ qc->err_mask |= AC_ERR_DEV;
ata_pio_sectors(qc);
ata_altstatus(ap);
status = ata_wait_idle(ap);
+ qc->err_mask |= ac_err_idle(status);
}
/* ata_pio_sectors() might change the
@@ -4041,7 +4044,7 @@ fsm_start:
case HSM_ST_LAST:
if (unlikely(!ata_ok(status))) {
- qc->err_mask |= __ac_err_mask(status);
+ qc->err_mask |= ac_err_idle(status);
ap->hsm_task_state = HSM_ST_ERR;
goto fsm_start;
}
--- irq-pio0/include/linux/libata.h 2006-04-06 10:08:48.000000000 +0800
+++ irq-pio1/include/linux/libata.h 2006-04-06 13:44:25.000000000 +0800
@@ -942,6 +942,46 @@ static inline int ata_try_flush_cache(co
ata_id_has_flush_ext(dev->id);
}
+static inline unsigned int ac_err_drq(u8 status)
+{
+ /* BSY = 1*/
+ if (status & ATA_BUSY)
+ return AC_ERR_HSM;
+
+ /* BSY = 0, DRQ = 1 */
+ if (status & ATA_DRQ)
+ return 0;
+
+ /* BSY = 0, DRQ = 0, device err */
+ if (status & (ATA_ERR | ATA_DF))
+ return AC_ERR_DEV;
+
+ /* BSY = 0, DRQ = 0, no device err
+ * when we are expecting data transfer.
+ */
+ return AC_ERR_HSM;
+}
+
+static inline unsigned int ac_err_idle(u8 status)
+{
+ /* BSY = 1 or DRQ = 1 */
+ if (status & (ATA_BUSY | ATA_DRQ))
+ return AC_ERR_HSM;
+
+ /* BSY = 0, DRQ = 0, device err */
+ if (status & (ATA_ERR | ATA_DF))
+ return AC_ERR_DEV;
+
+ /* BSY = 0, DRQ = 0, no device err. DRDY = 1 */
+ if (status & ATA_DRDY)
+ return 0;
+
+ /* thing looks good.
+ * don't know why DRDY not set.
+ */
+ return AC_ERR_OTHER
+}
+
static inline unsigned int ac_err_mask(u8 status)
{
if (status & ATA_BUSY)
next reply other threads:[~2006-04-06 7:02 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-04-06 7:02 Albert Lee [this message]
2006-04-06 7:14 ` [PATCH/RFC] libata-dev: make the returned err_mask more accurate Tejun Heo
2006-04-06 7:34 ` Albert Lee
2006-04-06 7:43 ` [PATCH/RFC] libata-dev: make the returned err_mask more accurate (revised) Albert Lee
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4434BD0C.9080302@tw.ibm.com \
--to=albertcc@tw.ibm.com \
--cc=albertl@mail.com \
--cc=dwm@maxeymade.com \
--cc=htejun@gmail.com \
--cc=jeff@garzik.org \
--cc=linux-ide@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).