This patch series attempts to clean up the SCSI error handling a bit.
See comments in below TOC or patch emails. All of the below have been
tested in success and error paths through the VERIFY_10 and ATA_16
commands using the AHCI driver.
IMPORTANT: the patchset below against libata-dev-2.6 relies on the
recent AHCI driver fixes recently patched into libata-2.6. I am
including the two specific patches as 1 and 2 of this series for
completeness, although of course they should be merged from libata-2.6
instead. Therefore, you may ignore these two unless you want to test
this series now on libata-dev.
[ Start of patch descriptions ]
01_libata_garzik-ahci-tf-read.patch
: AHCI tf_read() support
(included in libata-2.6) This is Jeff's tf_read() support
patch for AHCI.
Signed-off-by: Jeff Garzik <[email protected]>
02_libata_ahci-err-int.patch
: AHCI error handling fix
(included in libata-2.6) Fixes AHCI bits during handling of
fatal error int.
03_libata_update_desc_code.patch
: update ATA PT sense desc code
Change the ATA pass through sense block descriptor code to
0x09 per SAT
04_libata_control_pg_desc_bit.patch
: support descriptor sense in ctrl page
libata must support the descriptor format sense blocks as they
are required to properly report results of ATA pass through
commands as well as other SCSI commands reporting 48b LBAs.
This patch adjusts the control mode page to properly report
this.
05_libata_split_ata_to_sense_error.patch
: rework how CCs generated
This patch fixes several bugs as well as reorganizes the way
check conditions are generated. Bugs fixed: 1) in
ata_scsi_qc_complete(), ATA_12/16 commands wouldn't call
ata_pass_thru_cc() on error status; 2) ata_pass_thru_cc()
wouldn't put the SK, ASC, and ASCQ from ata_to_sense_error()
in the correct place in the sense block because
ata_to_sense_error() was writing a fixed sense block.
Per the recommendations in the comments, ata_to_sense_error()
is now split into 3 parts. The existing fcn is only used for
outputting a sense key/ASC/ASCQ triplicate. A new function
ata_dump_status() was created to print the error info, similar
to the ide variety. A third function ata_gen_fixed_sense()
was created to generate a fixed length sense block. I added
the use of the info field for 28b LBAs only.
ata_pass_thru_cc() renamed to ata_gen_ata_desc_sense() to
match naming convention, presumably to include another
descriptor format function in the future (see question 2
below).
Questions:
1) I made the ata_gen_..._sense() routines read the status
register themselves rather than use the drv_stat values
that used to be passed in? These values seemed
unreliable/useless since they were often hard coded (see
calls to ata_qc_complete() for origins of most drv_stat
variables). Sound ok?
2) the SAT spec has little about error handling and sense
information, sepcifically what descriptor format is valid
for use by SAT commands. I want to use descriptor type 00
(information) in my next patch until a spec says
differently. Sound ok?
[ End of patch descriptions ]
BR
02_libata_ahci-err-int.patch
(included in libata-2.6) Fixes AHCI bits during handling of
fatal error int.
Signed-off-by: Brett Russ <[email protected]>
ahci.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
Index: libata-dev-2.6/drivers/scsi/ahci.c
===================================================================
--- libata-dev-2.6.orig/drivers/scsi/ahci.c 2005-03-17 17:16:57.000000000 -0500
+++ libata-dev-2.6/drivers/scsi/ahci.c 2005-03-17 17:16:57.000000000 -0500
@@ -548,7 +548,7 @@ static void ahci_intr_error(struct ata_p
/* stop DMA */
tmp = readl(port_mmio + PORT_CMD);
- tmp &= PORT_CMD_START | PORT_CMD_FIS_RX;
+ tmp &= ~PORT_CMD_START;
writel(tmp, port_mmio + PORT_CMD);
/* wait for engine to stop. TODO: this could be
@@ -580,7 +580,7 @@ static void ahci_intr_error(struct ata_p
/* re-start DMA */
tmp = readl(port_mmio + PORT_CMD);
- tmp |= PORT_CMD_START | PORT_CMD_FIS_RX;
+ tmp |= PORT_CMD_START;
writel(tmp, port_mmio + PORT_CMD);
readl(port_mmio + PORT_CMD); /* flush */
03_libata_update_desc_code.patch
Change the ATA pass through sense block descriptor code to
0x09 per SAT
Signed-off-by: Brett Russ <[email protected]>
libata-scsi.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
Index: libata-dev-2.6/drivers/scsi/libata-scsi.c
===================================================================
--- libata-dev-2.6.orig/drivers/scsi/libata-scsi.c 2005-03-08 08:47:48.000000000 -0500
+++ libata-dev-2.6/drivers/scsi/libata-scsi.c 2005-03-17 17:16:58.000000000 -0500
@@ -531,7 +531,7 @@ void ata_pass_thru_cc(struct ata_queued_
*/
sb[0] = 0x72 ;
- desc[0] = 0x8e ; /* TODO: replace with official value. */
+ desc[0] = 0x09;
/*
* Set length of additional sense data.
@@ -2059,7 +2059,7 @@ void ata_scsi_simulate(u16 *id,
ata_scsi_rbuf_fill(&args, ata_scsiop_report_luns);
break;
- /* mandantory commands we haven't implemented yet */
+ /* mandatory commands we haven't implemented yet */
case REQUEST_SENSE:
/* all other commands */
05_libata_split_ata_to_sense_error.patch
This patch fixes several bugs as well as reorganizes the way
check conditions are generated. Bugs fixed: 1) in
ata_scsi_qc_complete(), ATA_12/16 commands wouldn't call
ata_pass_thru_cc() on error status; 2) ata_pass_thru_cc()
wouldn't put the SK, ASC, and ASCQ from ata_to_sense_error()
in the correct place in the sense block because
ata_to_sense_error() was writing a fixed sense block.
Per the recommendations in the comments, ata_to_sense_error()
is now split into 3 parts. The existing fcn is only used for
outputting a sense key/ASC/ASCQ triplicate. A new function
ata_dump_status() was created to print the error info, similar
to the ide variety. A third function ata_gen_fixed_sense()
was created to generate a fixed length sense block. I added
the use of the info field for 28b LBAs only.
ata_pass_thru_cc() renamed to ata_gen_ata_desc_sense() to
match naming convention, presumably to include another
descriptor format function in the future (see question 2
below).
Questions:
1) I made the ata_gen_..._sense() routines read the status
register themselves rather than use the drv_stat values
that used to be passed in? These values seemed
unreliable/useless since they were often hard coded (see
calls to ata_qc_complete() for origins of most drv_stat
variables). Sound ok?
2) the SAT spec has little about error handling and sense
information, sepcifically what descriptor format is valid
for use by SAT commands. I want to use descriptor type 00
(information) in my next patch until a spec says
differently. Sound ok?
Signed-off-by: Brett Russ <[email protected]>
libata-scsi.c | 342 +++++++++++++++++++++++++++++++++-------------------------
libata.h | 1
2 files changed, 197 insertions(+), 146 deletions(-)
Index: libata-dev-2.6/drivers/scsi/libata-scsi.c
===================================================================
--- libata-dev-2.6.orig/drivers/scsi/libata-scsi.c 2005-03-17 17:16:58.000000000 -0500
+++ libata-dev-2.6/drivers/scsi/libata-scsi.c 2005-03-17 17:16:59.000000000 -0500
@@ -331,24 +331,69 @@ struct ata_queued_cmd *ata_scsi_qc_new(s
}
/**
+ * ata_dump_status - user friendly display of error info
+ * @id: id of the port in question
+ * @tf: ptr to filled out taskfile
+ *
+ * Decode and dump the ATA error/status registers for the user so
+ * that they have some idea what really happened at the non
+ * make-believe layer.
+ *
+ * LOCKING:
+ * inherited from caller
+ */
+void ata_dump_status(unsigned id, struct ata_taskfile *tf)
+{
+ u8 stat = tf->command, err = tf->feature;
+
+ printk(KERN_WARNING "ata%u: status=0x%02x { ", id, stat);
+ if (stat & ATA_BUSY) {
+ printk("Busy }\n"); /* Data is not valid in this case */
+ } else {
+ if (stat & 0x40) printk("DriveReady ");
+ if (stat & 0x20) printk("DeviceFault ");
+ if (stat & 0x10) printk("SeekComplete ");
+ if (stat & 0x08) printk("DataRequest ");
+ if (stat & 0x04) printk("CorrectedError ");
+ if (stat & 0x02) printk("Index ");
+ if (stat & 0x01) printk("Error ");
+ printk("}\n");
+
+ if (err) {
+ printk(KERN_WARNING "ata%u: error=0x%02x { ", id, err);
+ if (err & 0x04) printk("DriveStatusError ");
+ if (err & 0x80) {
+ if (err & 0x04) printk("BadCRC ");
+ else printk("Sector ");
+ }
+ if (err & 0x40) printk("UncorrectableError ");
+ if (err & 0x10) printk("SectorIdNotFound ");
+ if (err & 0x02) printk("TrackZeroNotFound ");
+ if (err & 0x01) printk("AddrMarkNotFound ");
+ printk("}\n");
+ }
+ }
+}
+
+/**
* ata_to_sense_error - convert ATA error to SCSI error
- * @qc: Command that we are erroring out
* @drv_stat: value contained in ATA status register
+ * @drv_err: value contained in ATA error register
+ * @sk: the sense key we'll fill out
+ * @asc: the additional sense code we'll fill out
+ * @ascq: the additional sense code qualifier we'll fill out
*
- * Converts an ATA error into a SCSI error. While we are at it
- * we decode and dump the ATA error for the user so that they
- * have some idea what really happened at the non make-believe
- * layer.
+ * Converts an ATA error into a SCSI error. Fill out pointers to
+ * SK, ASC, and ASCQ bytes for later use in fixed or descriptor
+ * format sense blocks.
*
* LOCKING:
* spin_lock_irqsave(host_set lock)
*/
-
-void ata_to_sense_error(struct ata_queued_cmd *qc, u8 drv_stat)
+void ata_to_sense_error(unsigned id, u8 drv_stat, u8 drv_err, u8 *sk, u8 *asc,
+ u8 *ascq)
{
- struct scsi_cmnd *cmd = qc->scsicmd;
- u8 err = 0;
- unsigned char *sb = cmd->sense_buffer;
+ int i;
/* Based on the 3ware driver translation table */
static unsigned char sense_table[][4] = {
/* BBD|ECC|ID|MAR */
@@ -389,147 +434,99 @@ void ata_to_sense_error(struct ata_queue
{0x04, RECOVERED_ERROR, 0x11, 0x00}, // Recovered ECC error Medium error, recovered
{0xFF, 0xFF, 0xFF, 0xFF}, // END mark
};
- int i = 0;
-
- cmd->result = SAM_STAT_CHECK_CONDITION;
/*
* Is this an error we can process/parse
*/
-
- if(drv_stat & ATA_ERR)
- /* Read the err bits */
- err = ata_chk_err(qc->ap);
-
- /* Display the ATA level error info */
-
- printk(KERN_WARNING "ata%u: status=0x%02x { ", qc->ap->id, drv_stat);
- if(drv_stat & 0x80)
- {
- printk("Busy ");
- err = 0; /* Data is not valid in this case */
- }
- else {
- if(drv_stat & 0x40) printk("DriveReady ");
- if(drv_stat & 0x20) printk("DeviceFault ");
- if(drv_stat & 0x10) printk("SeekComplete ");
- if(drv_stat & 0x08) printk("DataRequest ");
- if(drv_stat & 0x04) printk("CorrectedError ");
- if(drv_stat & 0x02) printk("Index ");
- if(drv_stat & 0x01) printk("Error ");
- }
- printk("}\n");
-
- if(err)
- {
- printk(KERN_WARNING "ata%u: error=0x%02x { ", qc->ap->id, err);
- if(err & 0x04) printk("DriveStatusError ");
- if(err & 0x80)
- {
- if(err & 0x04)
- printk("BadCRC ");
- else
- printk("Sector ");
- }
- if(err & 0x40) printk("UncorrectableError ");
- if(err & 0x10) printk("SectorIdNotFound ");
- if(err & 0x02) printk("TrackZeroNotFound ");
- if(err & 0x01) printk("AddrMarkNotFound ");
- printk("}\n");
-
- /* Should we dump sector info here too ?? */
+ if (drv_stat & ATA_BUSY) {
+ drv_err = 0; /* Ignore the err bits, they're invalid */
}
+ printk(KERN_ERR "ata%u: translating stat 0x%02x err 0x%02x to sense\n",
+ id,drv_stat,drv_err);
- /* Look for err */
- while(sense_table[i][0] != 0xFF)
- {
- /* Look for best matches first */
- if((sense_table[i][0] & err) == sense_table[i][0])
- {
- sb[0] = 0x70;
- sb[2] = sense_table[i][1];
- sb[7] = 0x0a;
- sb[12] = sense_table[i][2];
- sb[13] = sense_table[i][3];
- return;
+ if (drv_err) {
+ /* Look for drv_err */
+ for (i = 0; sense_table[i][0] != 0xFF; i++) {
+ /* Look for best matches first */
+ if ((sense_table[i][0] & drv_err) == sense_table[i][0]) {
+ *sk = sense_table[i][1];
+ *asc = sense_table[i][2];
+ *ascq = sense_table[i][3];
+ return;
+ }
}
- i++;
+ /* No immediate match */
+ printk(KERN_DEBUG "ata%u: no sense translation for "
+ "error 0x%02x\n", id, drv_err);
}
- /* No immediate match */
- if(err)
- printk(KERN_DEBUG "ata%u: no sense translation for 0x%02x\n", qc->ap->id, err);
- i = 0;
/* Fall back to interpreting status bits */
- while(stat_table[i][0] != 0xFF)
- {
- if(stat_table[i][0] & drv_stat)
- {
- sb[0] = 0x70;
- sb[2] = stat_table[i][1];
- sb[7] = 0x0a;
- sb[12] = stat_table[i][2];
- sb[13] = stat_table[i][3];
+ for (i = 0; stat_table[i][0] != 0xFF; i++) {
+ if (stat_table[i][0] & drv_stat) {
+ *sk = stat_table[i][1];
+ *asc = stat_table[i][2];
+ *ascq = stat_table[i][3];
return;
}
- i++;
- }
- /* No error ?? */
- printk(KERN_ERR "ata%u: called with no error (%02X)!\n", qc->ap->id, drv_stat);
- /* additional-sense-code[-qualifier] */
-
- sb[0] = 0x70;
- sb[2] = MEDIUM_ERROR;
- sb[7] = 0x0A;
- if (cmd->sc_data_direction == SCSI_DATA_READ) {
- sb[12] = 0x11; /* "unrecovered read error" */
- sb[13] = 0x04;
- } else {
- sb[12] = 0x0C; /* "write error - */
- sb[13] = 0x02; /* auto-reallocation failed" */
}
+ /* No error? Undecoded? */
+ printk(KERN_ERR "ata%u: no sense translation for status: 0x%02x\n",
+ id, drv_stat);
+
+ /* For our last chance pick, use medium read error because
+ * it's much more common than an ATA drive telling you a write
+ * has failed.
+ */
+ *sk = MEDIUM_ERROR;
+ *asc = 0x11; /* "unrecovered read error" */
+ *ascq = 0x04; /* "auto-reallocation failed" */
}
/*
- * ata_pass_thru_cc - Generate check condition sense block.
+ * ata_gen_ata_desc_sense - Generate check condition sense block.
* @qc: Command that completed.
*
- * Regardless of whether the command errored or not, return
- * a sense block. Copy all controller registers into
- * the sense block. Clear sense key, ASC & ASCQ if
- * there is no error.
+ * This function is specific to the ATA descriptor format sense
+ * block specified for the ATA pass through commands. Regardless
+ * of whether the command errored or not, return a sense
+ * block. Copy all controller registers into the sense
+ * block. Clear sense key, ASC & ASCQ if there is no error.
*
* LOCKING:
* spin_lock_irqsave(host_set lock)
*/
-void ata_pass_thru_cc(struct ata_queued_cmd *qc, u8 drv_stat)
+void ata_gen_ata_desc_sense(struct ata_queued_cmd *qc)
{
struct scsi_cmnd *cmd = qc->scsicmd;
struct ata_taskfile *tf = &qc->tf;
unsigned char *sb = cmd->sense_buffer;
- unsigned char *desc = sb + 8 ;
+ unsigned char *desc = sb + 8;
+
+ memset(sb, 0, 32);
cmd->result = SAM_STAT_CHECK_CONDITION;
/*
+ * Read the controller registers.
+ */
+ assert(NULL != qc->ap->ops->tf_read);
+ qc->ap->ops->tf_read(qc->ap, tf);
+
+ /*
* Use ata_to_sense_error() to map status register bits
- * onto sense key, asc & ascq. We will overwrite some
- * (many) of the fields later.
- *
- * TODO: reorganise better, by splitting ata_to_sense_error()
+ * onto sense key, asc & ascq.
*/
- if (unlikely(drv_stat & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ))) {
- ata_to_sense_error(qc, drv_stat) ;
- } else {
- sb[3] = sb[2] = sb[1] = 0x00 ;
+ if (unlikely(tf->command & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ))) {
+ ata_to_sense_error(qc->ap->id, tf->command, tf->feature,
+ &sb[1], &sb[2], &sb[3]);
+ sb[1] &= 0x0f;
}
/*
- * Sense data is current and format is
- * descriptor.
+ * Sense data is current and format is descriptor.
*/
- sb[0] = 0x72 ;
+ sb[0] = 0x72;
desc[0] = 0x09;
@@ -538,35 +535,78 @@ void ata_pass_thru_cc(struct ata_queued_
* Since we only populate descriptor 0, the total
* length is the same (fixed) length as descriptor 0.
*/
- desc[1] = sb[7] = 14 ;
-
- /*
- * Read the controller registers.
- */
- qc->ap->ops->tf_read(qc->ap, tf);
+ desc[1] = sb[7] = 14;
/*
* Copy registers into sense buffer.
*/
- desc[2] = 0x00 ;
- desc[3] = tf->feature ; /* Note: becomes error register when read. */
- desc[5] = tf->nsect ;
- desc[7] = tf->lbal ;
- desc[9] = tf->lbam ;
- desc[11] = tf->lbah ;
- desc[12] = tf->device ;
- desc[13] = drv_stat ;
+ desc[2] = 0x00;
+ desc[3] = tf->feature; /* == error reg */
+ desc[5] = tf->nsect;
+ desc[7] = tf->lbal;
+ desc[9] = tf->lbam;
+ desc[11] = tf->lbah;
+ desc[12] = tf->device;
+ desc[13] = tf->command; /* == status reg */
/*
* Fill in Extend bit, and the high order bytes
* if applicable.
*/
if (tf->flags & ATA_TFLAG_LBA48) {
- desc[2] |= 0x01 ;
- desc[4] = tf->hob_nsect ;
- desc[6] = tf->hob_lbal ;
- desc[8] = tf->hob_lbam ;
- desc[10] = tf->hob_lbah ;
+ desc[2] |= 0x01;
+ desc[4] = tf->hob_nsect;
+ desc[6] = tf->hob_lbal;
+ desc[8] = tf->hob_lbam;
+ desc[10] = tf->hob_lbah;
+ }
+}
+
+/**
+ * ata_gen_fixed_sense - generate a SCSI fixed sense block
+ * @qc: Command that we are erroring out
+ *
+ * Leverage ata_to_sense_error() to give us the codes. Fit our
+ * LBA in here if there's room.
+ *
+ * LOCKING:
+ * inherited from caller
+ */
+void ata_gen_fixed_sense(struct ata_queued_cmd *qc)
+{
+ struct scsi_cmnd *cmd = qc->scsicmd;
+ struct ata_taskfile *tf = &qc->tf;
+ unsigned char *sb = cmd->sense_buffer;
+
+ memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
+
+ cmd->result = SAM_STAT_CHECK_CONDITION;
+
+ /*
+ * Read the controller registers.
+ */
+ assert(NULL != qc->ap->ops->tf_read);
+ qc->ap->ops->tf_read(qc->ap, tf);
+
+ /*
+ * Use ata_to_sense_error() to map status register bits
+ * onto sense key, asc & ascq.
+ */
+ if (unlikely(tf->command & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ))) {
+ ata_to_sense_error(qc->ap->id, tf->command, tf->feature,
+ &sb[2], &sb[12], &sb[13]);
+ sb[2] &= 0x0f;
+ }
+
+ sb[0] = 0x70;
+ sb[7] = 0x0a;
+ if (tf->flags & ATA_TFLAG_LBA && !(tf->flags & ATA_TFLAG_LBA48)) {
+ /* A small (28b) LBA will fit in the 32b info field */
+ sb[0] |= 0x80; /* set valid bit */
+ sb[3] = tf->device & 0x0f;
+ sb[4] = tf->lbah;
+ sb[5] = tf->lbam;
+ sb[6] = tf->lbal;
}
}
@@ -946,23 +986,35 @@ static unsigned int ata_scsi_rw_xlat(str
static int ata_scsi_qc_complete(struct ata_queued_cmd *qc, u8 drv_stat)
{
struct scsi_cmnd *cmd = qc->scsicmd;
+ int need_sense = drv_stat & (ATA_ERR | ATA_BUSY | ATA_DRQ);
- /*
- * If this was a pass-thru command, and the user requested
- * a check condition return including register values.
- * Note that check condition is generated, and the ATA
- * register values are returned, whether the command completed
- * successfully or not. If there was no error, SK, ASC and
- * ASCQ will all be zero.
+ /* For ATA pass thru (SAT) commands, generate a sense block if
+ * user mandated it or if there's an error. Note that if we
+ * generate because the user forced us to, a check condition
+ * is generated and the ATA register values are returned
+ * whether the command completed successfully or not. If there
+ * was no error, SK, ASC and ASCQ will all be zero.
*/
if (((cmd->cmnd[0] == ATA_16) || (cmd->cmnd[0] == ATA_12)) &&
- (cmd->cmnd[2] & 0x20)) {
- ata_pass_thru_cc(qc, drv_stat) ;
+ ((cmd->cmnd[2] & 0x20) || need_sense)) {
+ ata_gen_ata_desc_sense(qc);
} else {
- if (unlikely(drv_stat & (ATA_ERR | ATA_BUSY | ATA_DRQ)))
- ata_to_sense_error(qc, drv_stat);
- else
+ if (!need_sense) {
cmd->result = SAM_STAT_GOOD;
+ } else {
+ /* TODO: decide which descriptor format to use
+ * for 48b LBA devices and call that here
+ * instead of the fixed desc, which is only
+ * good for smaller LBA (and maybe CHS?)
+ * devices.
+ */
+ ata_gen_fixed_sense(qc);
+ }
+ }
+
+ if (need_sense) {
+ /* The ata_gen_..._sense routines fill in tf */
+ ata_dump_status(qc->ap->id, &qc->tf);
}
qc->scsidone(cmd);
Index: libata-dev-2.6/drivers/scsi/libata.h
===================================================================
--- libata-dev-2.6.orig/drivers/scsi/libata.h 2005-03-17 01:33:26.000000000 -0500
+++ libata-dev-2.6/drivers/scsi/libata.h 2005-03-17 17:16:59.000000000 -0500
@@ -45,7 +45,6 @@ extern int ata_cmd_ioctl(struct scsi_dev
/* libata-scsi.c */
-extern void ata_to_sense_error(struct ata_queued_cmd *qc, u8 drv_stat);
extern int ata_scsi_error(struct Scsi_Host *host);
extern unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf,
unsigned int buflen);
01_libata_garzik-ahci-tf-read.patch
(included in libata-2.6) This is Jeff's tf_read() support
patch for AHCI.
Signed-off-by: Jeff Garzik <[email protected]>
ahci.c | 11 +++++++++++
1 files changed, 11 insertions(+)
Index: libata-dev-2.6/drivers/scsi/ahci.c
===================================================================
--- libata-dev-2.6.orig/drivers/scsi/ahci.c 2005-03-17 12:36:29.000000000 -0500
+++ libata-dev-2.6/drivers/scsi/ahci.c 2005-03-17 17:16:57.000000000 -0500
@@ -179,6 +179,7 @@ static void ahci_eng_timeout(struct ata_
static int ahci_port_start(struct ata_port *ap);
static void ahci_port_stop(struct ata_port *ap);
static void ahci_host_stop(struct ata_host_set *host_set);
+static void ahci_tf_read(struct ata_port *ap, struct ata_taskfile *tf);
static void ahci_qc_prep(struct ata_queued_cmd *qc);
static u8 ahci_check_status(struct ata_port *ap);
static u8 ahci_check_err(struct ata_port *ap);
@@ -213,6 +214,8 @@ static struct ata_port_operations ahci_o
.check_err = ahci_check_err,
.dev_select = ata_noop_dev_select,
+ .tf_read = ahci_tf_read,
+
.phy_reset = ahci_phy_reset,
.qc_prep = ahci_qc_prep,
@@ -466,6 +469,14 @@ static u8 ahci_check_err(struct ata_port
return (readl(mmio + PORT_TFDATA) >> 8) & 0xFF;
}
+static void ahci_tf_read(struct ata_port *ap, struct ata_taskfile *tf)
+{
+ struct ahci_port_priv *pp = ap->private_data;
+ u8 *d2h_fis = pp->rx_fis + RX_FIS_D2H_REG;
+
+ ata_tf_from_fis(d2h_fis, tf);
+}
+
static void ahci_fill_sg(struct ata_queued_cmd *qc)
{
struct ahci_port_priv *pp = qc->ap->private_data;
04_libata_control_pg_desc_bit.patch
libata must support the descriptor format sense blocks as they
are required to properly report results of ATA pass through
commands as well as other SCSI commands reporting 48b LBAs.
This patch adjusts the control mode page to properly report
this.
Signed-off-by: Brett Russ <[email protected]>
libata-scsi.c | 7 ++++++-
1 files changed, 6 insertions(+), 1 deletion(-)
Index: libata-dev-2.6/drivers/scsi/libata-scsi.c
===================================================================
--- libata-dev-2.6.orig/drivers/scsi/libata-scsi.c 2005-03-17 17:16:58.000000000 -0500
+++ libata-dev-2.6/drivers/scsi/libata-scsi.c 2005-03-17 17:16:58.000000000 -0500
@@ -1370,7 +1370,12 @@ static unsigned int ata_msense_caching(u
static unsigned int ata_msense_ctl_mode(u8 **ptr_io, const u8 *last)
{
- const u8 page[] = {0xa, 0xa, 2, 0, 0, 0, 0, 0, 0xff, 0xff, 0, 30};
+ const u8 page[] = {0xa, 0xa, 6, 0, 0, 0, 0, 0, 0xff, 0xff, 0, 30};
+
+ /* byte 2: set the descriptor format sense data bit (bit 2)
+ * since we need to support returning this format for SAT
+ * commands and any SCSI commands against a 48b LBA device.
+ */
ata_msense_push(ptr_io, last, page, sizeof(page));
return sizeof(page);
Brett Russ wrote:
> 04_libata_control_pg_desc_bit.patch
>
> libata must support the descriptor format sense blocks as they
> are required to properly report results of ATA pass through
> commands as well as other SCSI commands reporting 48b LBAs.
> This patch adjusts the control mode page to properly report
> this.
>
> Signed-off-by: Brett Russ <[email protected]>
applied
Brett Russ wrote:
> 03_libata_update_desc_code.patch
>
> Change the ATA pass through sense block descriptor code to
> 0x09 per SAT
>
> Signed-off-by: Brett Russ <[email protected]>
applied
Brett Russ wrote:
> 05_libata_split_ata_to_sense_error.patch
>
> This patch fixes several bugs as well as reorganizes the way
> check conditions are generated. Bugs fixed: 1) in
> ata_scsi_qc_complete(), ATA_12/16 commands wouldn't call
> ata_pass_thru_cc() on error status; 2) ata_pass_thru_cc()
> wouldn't put the SK, ASC, and ASCQ from ata_to_sense_error()
> in the correct place in the sense block because
> ata_to_sense_error() was writing a fixed sense block.
>
> Per the recommendations in the comments, ata_to_sense_error()
> is now split into 3 parts. The existing fcn is only used for
> outputting a sense key/ASC/ASCQ triplicate. A new function
> ata_dump_status() was created to print the error info, similar
> to the ide variety. A third function ata_gen_fixed_sense()
> was created to generate a fixed length sense block. I added
> the use of the info field for 28b LBAs only.
> ata_pass_thru_cc() renamed to ata_gen_ata_desc_sense() to
> match naming convention, presumably to include another
> descriptor format function in the future (see question 2
> below).
>
> Questions:
>
> 1) I made the ata_gen_..._sense() routines read the status
> register themselves rather than use the drv_stat values
> that used to be passed in? These values seemed
> unreliable/useless since they were often hard coded (see
> calls to ata_qc_complete() for origins of most drv_stat
> variables). Sound ok?
>
> 2) the SAT spec has little about error handling and sense
> information, sepcifically what descriptor format is valid
> for use by SAT commands. I want to use descriptor type 00
> (information) in my next patch until a spec says
> differently. Sound ok?
>
> Signed-off-by: Brett Russ <[email protected]>
Patch in general is OK, but I would prefer that it be split up a bit
more. Suggested split:
* whitespace changes (obscures reviewing the code)
* create and use ata_dump_status()
* the rest of the changes
Regards,
Jeff
These patches are a resubmit of patch 5/5 of the first series
submitted 2005-03-17. Jeff requested that the single patch be split
into a suggested 3 smaller patches; the results are below. I also
took this opportunity to further clean up the changes.
[ Start of patch descriptions ]
01_libata_libata-whitespace.patch
: whitespace updates
This patch adjusts some whitespace to bring the format of
libata-scsi.c to a consistent state.
02_libata_ata_dump_status.patch
: create/use ata_dump_status()
This patch introduces the ata_dump_status() function, which
for now is called from ata_to_sense_error() only.
03_libata_rework-cc-generation.patch
: rework check condition handling
This patch refactors the check condition creation within
libata. Changes include:
- ata_to_sense_error() now *only* performs the translation
from an ATA status/error register combination to a SCSI
SK/ASC/ASCQ combination. Additionally, the translation is
logged at level KERNEL_ERR and any untranslatable combos are
logged at level KERNEL_WARNING.
- ata_dump_status() is modified to take a taskfile struct as
argument in preparation for a future patch which will add
proper display of the failing location (LBA or CHS)
- created ata_gen_fixed_sense() to generate a fixed length CC
sense block.
- ata_pass_thru_cc() has been renamed to
ata_gen_ata_desc_sense() to fit the naming convention
mentioned above. Its guts were changed a bit as well.
- ata_scsi_qc_complete() has been modified to fix a bug where
ATA_12/16 commands would not generate a sense block on
error. Other changes made here as well, including the call
to ata_dump_status().
[ End of patch descriptions ]
01_libata_libata-whitespace.patch
This patch adjusts some whitespace to bring the format of
libata-scsi.c to a consistent state.
Signed-off-by: Brett Russ <[email protected]>
libata-scsi.c | 112 +++++++++++++++++++++++++---------------------------------
1 files changed, 50 insertions(+), 62 deletions(-)
Index: libata-dev-2.6/drivers/scsi/libata-scsi.c
===================================================================
--- libata-dev-2.6.orig/drivers/scsi/libata-scsi.c 2005-03-23 15:35:08.000000000 -0500
+++ libata-dev-2.6/drivers/scsi/libata-scsi.c 2005-03-23 15:35:09.000000000 -0500
@@ -397,56 +397,46 @@ void ata_to_sense_error(struct ata_queue
* Is this an error we can process/parse
*/
- if(drv_stat & ATA_ERR)
- /* Read the err bits */
- err = ata_chk_err(qc->ap);
+ if (drv_stat & ATA_ERR)
+ err = ata_chk_err(qc->ap); /* Read the err bits */
/* Display the ATA level error info */
printk(KERN_WARNING "ata%u: status=0x%02x { ", qc->ap->id, drv_stat);
- if(drv_stat & 0x80)
- {
+ if (drv_stat & 0x80) {
printk("Busy ");
err = 0; /* Data is not valid in this case */
- }
- else {
- if(drv_stat & 0x40) printk("DriveReady ");
- if(drv_stat & 0x20) printk("DeviceFault ");
- if(drv_stat & 0x10) printk("SeekComplete ");
- if(drv_stat & 0x08) printk("DataRequest ");
- if(drv_stat & 0x04) printk("CorrectedError ");
- if(drv_stat & 0x02) printk("Index ");
- if(drv_stat & 0x01) printk("Error ");
+ } else {
+ if (drv_stat & 0x40) printk("DriveReady ");
+ if (drv_stat & 0x20) printk("DeviceFault ");
+ if (drv_stat & 0x10) printk("SeekComplete ");
+ if (drv_stat & 0x08) printk("DataRequest ");
+ if (drv_stat & 0x04) printk("CorrectedError ");
+ if (drv_stat & 0x02) printk("Index ");
+ if (drv_stat & 0x01) printk("Error ");
}
printk("}\n");
- if(err)
- {
+ if (err) {
printk(KERN_WARNING "ata%u: error=0x%02x { ", qc->ap->id, err);
- if(err & 0x04) printk("DriveStatusError ");
- if(err & 0x80)
- {
- if(err & 0x04)
- printk("BadCRC ");
- else
- printk("Sector ");
+ if (err & 0x04) printk("DriveStatusError ");
+ if (err & 0x80) {
+ if (err & 0x04) printk("BadCRC ");
+ else printk("Sector ");
}
- if(err & 0x40) printk("UncorrectableError ");
- if(err & 0x10) printk("SectorIdNotFound ");
- if(err & 0x02) printk("TrackZeroNotFound ");
- if(err & 0x01) printk("AddrMarkNotFound ");
+ if (err & 0x40) printk("UncorrectableError ");
+ if (err & 0x10) printk("SectorIdNotFound ");
+ if (err & 0x02) printk("TrackZeroNotFound ");
+ if (err & 0x01) printk("AddrMarkNotFound ");
printk("}\n");
/* Should we dump sector info here too ?? */
}
-
/* Look for err */
- while(sense_table[i][0] != 0xFF)
- {
+ while (sense_table[i][0] != 0xFF) {
/* Look for best matches first */
- if((sense_table[i][0] & err) == sense_table[i][0])
- {
+ if ((sense_table[i][0] & err) == sense_table[i][0]) {
sb[0] = 0x70;
sb[2] = sense_table[i][1];
sb[7] = 0x0a;
@@ -457,15 +447,13 @@ void ata_to_sense_error(struct ata_queue
i++;
}
/* No immediate match */
- if(err)
+ if (err)
printk(KERN_DEBUG "ata%u: no sense translation for 0x%02x\n", qc->ap->id, err);
i = 0;
/* Fall back to interpreting status bits */
- while(stat_table[i][0] != 0xFF)
- {
- if(stat_table[i][0] & drv_stat)
- {
+ while (stat_table[i][0] != 0xFF) {
+ if (stat_table[i][0] & drv_stat) {
sb[0] = 0x70;
sb[2] = stat_table[i][1];
sb[7] = 0x0a;
@@ -508,7 +496,7 @@ void ata_pass_thru_cc(struct ata_queued_
struct scsi_cmnd *cmd = qc->scsicmd;
struct ata_taskfile *tf = &qc->tf;
unsigned char *sb = cmd->sense_buffer;
- unsigned char *desc = sb + 8 ;
+ unsigned char *desc = sb + 8;
cmd->result = SAM_STAT_CHECK_CONDITION;
@@ -520,16 +508,16 @@ void ata_pass_thru_cc(struct ata_queued_
* TODO: reorganise better, by splitting ata_to_sense_error()
*/
if (unlikely(drv_stat & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ))) {
- ata_to_sense_error(qc, drv_stat) ;
+ ata_to_sense_error(qc, drv_stat);
} else {
- sb[3] = sb[2] = sb[1] = 0x00 ;
+ sb[3] = sb[2] = sb[1] = 0x00;
}
/*
* Sense data is current and format is
* descriptor.
*/
- sb[0] = 0x72 ;
+ sb[0] = 0x72;
desc[0] = 0x09;
@@ -538,7 +526,7 @@ void ata_pass_thru_cc(struct ata_queued_
* Since we only populate descriptor 0, the total
* length is the same (fixed) length as descriptor 0.
*/
- desc[1] = sb[7] = 14 ;
+ desc[1] = sb[7] = 14;
/*
* Read the controller registers.
@@ -548,25 +536,25 @@ void ata_pass_thru_cc(struct ata_queued_
/*
* Copy registers into sense buffer.
*/
- desc[2] = 0x00 ;
- desc[3] = tf->feature ; /* Note: becomes error register when read. */
- desc[5] = tf->nsect ;
- desc[7] = tf->lbal ;
- desc[9] = tf->lbam ;
- desc[11] = tf->lbah ;
- desc[12] = tf->device ;
- desc[13] = drv_stat ;
+ desc[2] = 0x00;
+ desc[3] = tf->feature; /* Note: becomes error register when read. */
+ desc[5] = tf->nsect;
+ desc[7] = tf->lbal;
+ desc[9] = tf->lbam;
+ desc[11] = tf->lbah;
+ desc[12] = tf->device;
+ desc[13] = drv_stat;
/*
* Fill in Extend bit, and the high order bytes
* if applicable.
*/
if (tf->flags & ATA_TFLAG_LBA48) {
- desc[2] |= 0x01 ;
- desc[4] = tf->hob_nsect ;
- desc[6] = tf->hob_lbal ;
- desc[8] = tf->hob_lbam ;
- desc[10] = tf->hob_lbah ;
+ desc[2] |= 0x01;
+ desc[4] = tf->hob_nsect;
+ desc[6] = tf->hob_lbal;
+ desc[8] = tf->hob_lbam;
+ desc[10] = tf->hob_lbah;
}
}
@@ -957,7 +945,7 @@ static int ata_scsi_qc_complete(struct a
*/
if (((cmd->cmnd[0] == ATA_16) || (cmd->cmnd[0] == ATA_12)) &&
(cmd->cmnd[2] & 0x20)) {
- ata_pass_thru_cc(qc, drv_stat) ;
+ ata_pass_thru_cc(qc, drv_stat);
} else {
if (unlikely(drv_stat & (ATA_ERR | ATA_BUSY | ATA_DRQ)))
ata_to_sense_error(qc, drv_stat);
@@ -1811,9 +1799,9 @@ ata_scsi_pass_thru(struct ata_queued_cmd
tf->hob_lbal = scsicmd[7];
tf->hob_lbam = scsicmd[9];
tf->hob_lbah = scsicmd[11];
- tf->flags |= ATA_TFLAG_LBA48 ;
+ tf->flags |= ATA_TFLAG_LBA48;
} else
- tf->flags &= ~ATA_TFLAG_LBA48 ;
+ tf->flags &= ~ATA_TFLAG_LBA48;
/*
* Always copy low byte, device and command registers.
@@ -1829,7 +1817,7 @@ ata_scsi_pass_thru(struct ata_queued_cmd
/*
* 12-byte CDB - incapable of extended commands.
*/
- tf->flags &= ~ATA_TFLAG_LBA48 ;
+ tf->flags &= ~ATA_TFLAG_LBA48;
tf->feature = scsicmd[3];
tf->nsect = scsicmd[4];
@@ -1856,7 +1844,7 @@ ata_scsi_pass_thru(struct ata_queued_cmd
* and pass on write indication (used for PIO/DMA
* setup.)
*/
- tf->flags |= (ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE) ;
+ tf->flags |= (ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE);
if (cmd->sc_data_direction == SCSI_DATA_WRITE)
tf->flags |= ATA_TFLAG_WRITE;
@@ -1867,7 +1855,7 @@ ata_scsi_pass_thru(struct ata_queued_cmd
* TODO: find out if we need to do more here to
* cover scatter/gather case.
*/
- qc->nsect = cmd->bufflen / ATA_SECT_SIZE ;
+ qc->nsect = cmd->bufflen / ATA_SECT_SIZE;
return 0;
}
@@ -1907,7 +1895,7 @@ static inline ata_xlat_func_t ata_get_xl
case ATA_12:
case ATA_16:
- return ata_scsi_pass_thru ;
+ return ata_scsi_pass_thru;
}
return NULL;
03_libata_rework-cc-generation.patch
This patch refactors the check condition creation within
libata. Changes include:
- ata_to_sense_error() now *only* performs the translation
from an ATA status/error register combination to a SCSI
SK/ASC/ASCQ combination. Additionally, the translation is
logged at level KERNEL_ERR and any untranslatable combos are
logged at level KERNEL_WARNING.
- ata_dump_status() is modified to take a taskfile struct as
argument in preparation for a future patch which will add
proper display of the failing location (LBA or CHS)
- created ata_gen_fixed_sense() to generate a fixed length CC
sense block.
- ata_pass_thru_cc() has been renamed to
ata_gen_ata_desc_sense() to fit the naming convention
mentioned above. Its guts were changed a bit as well.
- ata_scsi_qc_complete() has been modified to fix a bug where
ATA_12/16 commands would not generate a sense block on
error. Other changes made here as well, including the call
to ata_dump_status().
Signed-off-by: Brett Russ <[email protected]>
libata-scsi.c | 238 +++++++++++++++++++++++++++++++++++-----------------------
libata.h | 1
2 files changed, 147 insertions(+), 92 deletions(-)
Index: libata-dev-2.6/drivers/scsi/libata-scsi.c
===================================================================
--- libata-dev-2.6.orig/drivers/scsi/libata-scsi.c 2005-03-23 15:35:09.000000000 -0500
+++ libata-dev-2.6/drivers/scsi/libata-scsi.c 2005-03-23 15:35:10.000000000 -0500
@@ -342,8 +342,10 @@ struct ata_queued_cmd *ata_scsi_qc_new(s
* LOCKING:
* inherited from caller
*/
-void ata_dump_status(unsigned id, u8 stat, u8 err)
+void ata_dump_status(unsigned id, struct ata_taskfile *tf)
{
+ u8 stat = tf->command, err = tf->feature;
+
printk(KERN_WARNING "ata%u: status=0x%02x { ", id, stat);
if (stat & ATA_BUSY) {
printk("Busy }\n"); /* Data is not valid in this case */
@@ -375,23 +377,23 @@ void ata_dump_status(unsigned id, u8 sta
/**
* ata_to_sense_error - convert ATA error to SCSI error
- * @qc: Command that we are erroring out
* @drv_stat: value contained in ATA status register
+ * @drv_err: value contained in ATA error register
+ * @sk: the sense key we'll fill out
+ * @asc: the additional sense code we'll fill out
+ * @ascq: the additional sense code qualifier we'll fill out
*
- * Converts an ATA error into a SCSI error. While we are at it
- * we decode and dump the ATA error for the user so that they
- * have some idea what really happened at the non make-believe
- * layer.
+ * Converts an ATA error into a SCSI error. Fill out pointers to
+ * SK, ASC, and ASCQ bytes for later use in fixed or descriptor
+ * format sense blocks.
*
* LOCKING:
* spin_lock_irqsave(host_set lock)
*/
-
-void ata_to_sense_error(struct ata_queued_cmd *qc, u8 drv_stat)
+void ata_to_sense_error(unsigned id, u8 drv_stat, u8 drv_err, u8 *sk, u8 *asc,
+ u8 *ascq)
{
- struct scsi_cmnd *cmd = qc->scsicmd;
- u8 err = 0;
- unsigned char *sb = cmd->sense_buffer;
+ int i;
/* Based on the 3ware driver translation table */
static unsigned char sense_table[][4] = {
/* BBD|ECC|ID|MAR */
@@ -432,102 +434,101 @@ void ata_to_sense_error(struct ata_queue
{0x04, RECOVERED_ERROR, 0x11, 0x00}, // Recovered ECC error Medium error, recovered
{0xFF, 0xFF, 0xFF, 0xFF}, // END mark
};
- int i = 0;
-
- cmd->result = SAM_STAT_CHECK_CONDITION;
/*
* Is this an error we can process/parse
*/
+ if (drv_stat & ATA_BUSY) {
+ drv_err = 0; /* Ignore the err bits, they're invalid */
+ }
- if (drv_stat & ATA_ERR)
- err = ata_chk_err(qc->ap); /* Read the err bits */
-
- ata_dump_status(qc->ap->id, drv_stat, err);
-
- /* Look for err */
- while (sense_table[i][0] != 0xFF) {
- /* Look for best matches first */
- if ((sense_table[i][0] & err) == sense_table[i][0]) {
- sb[0] = 0x70;
- sb[2] = sense_table[i][1];
- sb[7] = 0x0a;
- sb[12] = sense_table[i][2];
- sb[13] = sense_table[i][3];
- return;
+ if (drv_err) {
+ /* Look for drv_err */
+ for (i = 0; sense_table[i][0] != 0xFF; i++) {
+ /* Look for best matches first */
+ if ((sense_table[i][0] & drv_err) ==
+ sense_table[i][0]) {
+ *sk = sense_table[i][1];
+ *asc = sense_table[i][2];
+ *ascq = sense_table[i][3];
+ goto translate_done;
+ }
}
- i++;
+ /* No immediate match */
+ printk(KERN_WARNING "ata%u: no sense translation for "
+ "error 0x%02x\n", id, drv_err);
}
- /* No immediate match */
- if (err)
- printk(KERN_DEBUG "ata%u: no sense translation for 0x%02x\n", qc->ap->id, err);
- i = 0;
/* Fall back to interpreting status bits */
- while (stat_table[i][0] != 0xFF) {
+ for (i = 0; stat_table[i][0] != 0xFF; i++) {
if (stat_table[i][0] & drv_stat) {
- sb[0] = 0x70;
- sb[2] = stat_table[i][1];
- sb[7] = 0x0a;
- sb[12] = stat_table[i][2];
- sb[13] = stat_table[i][3];
- return;
+ *sk = stat_table[i][1];
+ *asc = stat_table[i][2];
+ *ascq = stat_table[i][3];
+ goto translate_done;
}
- i++;
- }
- /* No error ?? */
- printk(KERN_ERR "ata%u: called with no error (%02X)!\n", qc->ap->id, drv_stat);
- /* additional-sense-code[-qualifier] */
-
- sb[0] = 0x70;
- sb[2] = MEDIUM_ERROR;
- sb[7] = 0x0A;
- if (cmd->sc_data_direction == SCSI_DATA_READ) {
- sb[12] = 0x11; /* "unrecovered read error" */
- sb[13] = 0x04;
- } else {
- sb[12] = 0x0C; /* "write error - */
- sb[13] = 0x02; /* auto-reallocation failed" */
}
+ /* No error? Undecoded? */
+ printk(KERN_WARNING "ata%u: no sense translation for status: 0x%02x\n",
+ id, drv_stat);
+
+ /* For our last chance pick, use medium read error because
+ * it's much more common than an ATA drive telling you a write
+ * has failed.
+ */
+ *sk = MEDIUM_ERROR;
+ *asc = 0x11; /* "unrecovered read error" */
+ *ascq = 0x04; /* "auto-reallocation failed" */
+
+ translate_done:
+ printk(KERN_ERR "ata%u: translated ATA stat/err 0x%02x/%02x to "
+ "SCSI SK/ASC/ASCQ 0x%x/%02x/%02x\n", id, drv_stat, drv_err,
+ *sk, *asc, *ascq);
+ return;
}
/*
- * ata_pass_thru_cc - Generate check condition sense block.
+ * ata_gen_ata_desc_sense - Generate check condition sense block.
* @qc: Command that completed.
*
- * Regardless of whether the command errored or not, return
- * a sense block. Copy all controller registers into
- * the sense block. Clear sense key, ASC & ASCQ if
- * there is no error.
+ * This function is specific to the ATA descriptor format sense
+ * block specified for the ATA pass through commands. Regardless
+ * of whether the command errored or not, return a sense
+ * block. Copy all controller registers into the sense
+ * block. Clear sense key, ASC & ASCQ if there is no error.
*
* LOCKING:
* spin_lock_irqsave(host_set lock)
*/
-void ata_pass_thru_cc(struct ata_queued_cmd *qc, u8 drv_stat)
+void ata_gen_ata_desc_sense(struct ata_queued_cmd *qc)
{
struct scsi_cmnd *cmd = qc->scsicmd;
struct ata_taskfile *tf = &qc->tf;
unsigned char *sb = cmd->sense_buffer;
unsigned char *desc = sb + 8;
+ memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
+
cmd->result = SAM_STAT_CHECK_CONDITION;
/*
+ * Read the controller registers.
+ */
+ assert(NULL != qc->ap->ops->tf_read);
+ qc->ap->ops->tf_read(qc->ap, tf);
+
+ /*
* Use ata_to_sense_error() to map status register bits
- * onto sense key, asc & ascq. We will overwrite some
- * (many) of the fields later.
- *
- * TODO: reorganise better, by splitting ata_to_sense_error()
+ * onto sense key, asc & ascq.
*/
- if (unlikely(drv_stat & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ))) {
- ata_to_sense_error(qc, drv_stat);
- } else {
- sb[3] = sb[2] = sb[1] = 0x00;
+ if (unlikely(tf->command & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ))) {
+ ata_to_sense_error(qc->ap->id, tf->command, tf->feature,
+ &sb[1], &sb[2], &sb[3]);
+ sb[1] &= 0x0f;
}
/*
- * Sense data is current and format is
- * descriptor.
+ * Sense data is current and format is descriptor.
*/
sb[0] = 0x72;
@@ -541,21 +542,16 @@ void ata_pass_thru_cc(struct ata_queued_
desc[1] = sb[7] = 14;
/*
- * Read the controller registers.
- */
- qc->ap->ops->tf_read(qc->ap, tf);
-
- /*
* Copy registers into sense buffer.
*/
desc[2] = 0x00;
- desc[3] = tf->feature; /* Note: becomes error register when read. */
+ desc[3] = tf->feature; /* == error reg */
desc[5] = tf->nsect;
desc[7] = tf->lbal;
desc[9] = tf->lbam;
desc[11] = tf->lbah;
desc[12] = tf->device;
- desc[13] = drv_stat;
+ desc[13] = tf->command; /* == status reg */
/*
* Fill in Extend bit, and the high order bytes
@@ -571,6 +567,54 @@ void ata_pass_thru_cc(struct ata_queued_
}
/**
+ * ata_gen_fixed_sense - generate a SCSI fixed sense block
+ * @qc: Command that we are erroring out
+ *
+ * Leverage ata_to_sense_error() to give us the codes. Fit our
+ * LBA in here if there's room.
+ *
+ * LOCKING:
+ * inherited from caller
+ */
+void ata_gen_fixed_sense(struct ata_queued_cmd *qc)
+{
+ struct scsi_cmnd *cmd = qc->scsicmd;
+ struct ata_taskfile *tf = &qc->tf;
+ unsigned char *sb = cmd->sense_buffer;
+
+ memset(sb, 0, SCSI_SENSE_BUFFERSIZE);
+
+ cmd->result = SAM_STAT_CHECK_CONDITION;
+
+ /*
+ * Read the controller registers.
+ */
+ assert(NULL != qc->ap->ops->tf_read);
+ qc->ap->ops->tf_read(qc->ap, tf);
+
+ /*
+ * Use ata_to_sense_error() to map status register bits
+ * onto sense key, asc & ascq.
+ */
+ if (unlikely(tf->command & (ATA_BUSY | ATA_DF | ATA_ERR | ATA_DRQ))) {
+ ata_to_sense_error(qc->ap->id, tf->command, tf->feature,
+ &sb[2], &sb[12], &sb[13]);
+ sb[2] &= 0x0f;
+ }
+
+ sb[0] = 0x70;
+ sb[7] = 0x0a;
+ if (tf->flags & ATA_TFLAG_LBA && !(tf->flags & ATA_TFLAG_LBA48)) {
+ /* A small (28b) LBA will fit in the 32b info field */
+ sb[0] |= 0x80; /* set valid bit */
+ sb[3] = tf->device & 0x0f;
+ sb[4] = tf->lbah;
+ sb[5] = tf->lbam;
+ sb[6] = tf->lbal;
+ }
+}
+
+/**
* ata_scsi_slave_config - Set SCSI device attributes
* @sdev: SCSI device to examine
*
@@ -946,23 +990,35 @@ static unsigned int ata_scsi_rw_xlat(str
static int ata_scsi_qc_complete(struct ata_queued_cmd *qc, u8 drv_stat)
{
struct scsi_cmnd *cmd = qc->scsicmd;
+ int need_sense = drv_stat & (ATA_ERR | ATA_BUSY | ATA_DRQ);
- /*
- * If this was a pass-thru command, and the user requested
- * a check condition return including register values.
- * Note that check condition is generated, and the ATA
- * register values are returned, whether the command completed
- * successfully or not. If there was no error, SK, ASC and
- * ASCQ will all be zero.
+ /* For ATA pass thru (SAT) commands, generate a sense block if
+ * user mandated it or if there's an error. Note that if we
+ * generate because the user forced us to, a check condition
+ * is generated and the ATA register values are returned
+ * whether the command completed successfully or not. If there
+ * was no error, SK, ASC and ASCQ will all be zero.
*/
if (((cmd->cmnd[0] == ATA_16) || (cmd->cmnd[0] == ATA_12)) &&
- (cmd->cmnd[2] & 0x20)) {
- ata_pass_thru_cc(qc, drv_stat);
+ ((cmd->cmnd[2] & 0x20) || need_sense)) {
+ ata_gen_ata_desc_sense(qc);
} else {
- if (unlikely(drv_stat & (ATA_ERR | ATA_BUSY | ATA_DRQ)))
- ata_to_sense_error(qc, drv_stat);
- else
+ if (!need_sense) {
cmd->result = SAM_STAT_GOOD;
+ } else {
+ /* TODO: decide which descriptor format to use
+ * for 48b LBA devices and call that here
+ * instead of the fixed desc, which is only
+ * good for smaller LBA (and maybe CHS?)
+ * devices.
+ */
+ ata_gen_fixed_sense(qc);
+ }
+ }
+
+ if (need_sense) {
+ /* The ata_gen_..._sense routines fill in tf */
+ ata_dump_status(qc->ap->id, &qc->tf);
}
qc->scsidone(cmd);
Index: libata-dev-2.6/drivers/scsi/libata.h
===================================================================
--- libata-dev-2.6.orig/drivers/scsi/libata.h 2005-03-17 01:33:26.000000000 -0500
+++ libata-dev-2.6/drivers/scsi/libata.h 2005-03-23 15:35:10.000000000 -0500
@@ -45,7 +45,6 @@ extern int ata_cmd_ioctl(struct scsi_dev
/* libata-scsi.c */
-extern void ata_to_sense_error(struct ata_queued_cmd *qc, u8 drv_stat);
extern int ata_scsi_error(struct Scsi_Host *host);
extern unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf,
unsigned int buflen);
02_libata_ata_dump_status.patch
This patch introduces the ata_dump_status() function, which
for now is called from ata_to_sense_error() only.
Signed-off-by: Brett Russ <[email protected]>
libata-scsi.c | 76 +++++++++++++++++++++++++++++++++-------------------------
1 files changed, 44 insertions(+), 32 deletions(-)
Index: libata-dev-2.6/drivers/scsi/libata-scsi.c
===================================================================
--- libata-dev-2.6.orig/drivers/scsi/libata-scsi.c 2005-03-23 15:35:09.000000000 -0500
+++ libata-dev-2.6/drivers/scsi/libata-scsi.c 2005-03-23 15:35:09.000000000 -0500
@@ -331,6 +331,49 @@ struct ata_queued_cmd *ata_scsi_qc_new(s
}
/**
+ * ata_dump_status - user friendly display of error info
+ * @id: id of the port in question
+ * @tf: ptr to filled out taskfile
+ *
+ * Decode and dump the ATA error/status registers for the user so
+ * that they have some idea what really happened at the non
+ * make-believe layer.
+ *
+ * LOCKING:
+ * inherited from caller
+ */
+void ata_dump_status(unsigned id, u8 stat, u8 err)
+{
+ printk(KERN_WARNING "ata%u: status=0x%02x { ", id, stat);
+ if (stat & ATA_BUSY) {
+ printk("Busy }\n"); /* Data is not valid in this case */
+ } else {
+ if (stat & 0x40) printk("DriveReady ");
+ if (stat & 0x20) printk("DeviceFault ");
+ if (stat & 0x10) printk("SeekComplete ");
+ if (stat & 0x08) printk("DataRequest ");
+ if (stat & 0x04) printk("CorrectedError ");
+ if (stat & 0x02) printk("Index ");
+ if (stat & 0x01) printk("Error ");
+ printk("}\n");
+
+ if (err) {
+ printk(KERN_WARNING "ata%u: error=0x%02x { ", id, err);
+ if (err & 0x04) printk("DriveStatusError ");
+ if (err & 0x80) {
+ if (err & 0x04) printk("BadCRC ");
+ else printk("Sector ");
+ }
+ if (err & 0x40) printk("UncorrectableError ");
+ if (err & 0x10) printk("SectorIdNotFound ");
+ if (err & 0x02) printk("TrackZeroNotFound ");
+ if (err & 0x01) printk("AddrMarkNotFound ");
+ printk("}\n");
+ }
+ }
+}
+
+/**
* ata_to_sense_error - convert ATA error to SCSI error
* @qc: Command that we are erroring out
* @drv_stat: value contained in ATA status register
@@ -400,38 +443,7 @@ void ata_to_sense_error(struct ata_queue
if (drv_stat & ATA_ERR)
err = ata_chk_err(qc->ap); /* Read the err bits */
- /* Display the ATA level error info */
-
- printk(KERN_WARNING "ata%u: status=0x%02x { ", qc->ap->id, drv_stat);
- if (drv_stat & 0x80) {
- printk("Busy ");
- err = 0; /* Data is not valid in this case */
- } else {
- if (drv_stat & 0x40) printk("DriveReady ");
- if (drv_stat & 0x20) printk("DeviceFault ");
- if (drv_stat & 0x10) printk("SeekComplete ");
- if (drv_stat & 0x08) printk("DataRequest ");
- if (drv_stat & 0x04) printk("CorrectedError ");
- if (drv_stat & 0x02) printk("Index ");
- if (drv_stat & 0x01) printk("Error ");
- }
- printk("}\n");
-
- if (err) {
- printk(KERN_WARNING "ata%u: error=0x%02x { ", qc->ap->id, err);
- if (err & 0x04) printk("DriveStatusError ");
- if (err & 0x80) {
- if (err & 0x04) printk("BadCRC ");
- else printk("Sector ");
- }
- if (err & 0x40) printk("UncorrectableError ");
- if (err & 0x10) printk("SectorIdNotFound ");
- if (err & 0x02) printk("TrackZeroNotFound ");
- if (err & 0x01) printk("AddrMarkNotFound ");
- printk("}\n");
-
- /* Should we dump sector info here too ?? */
- }
+ ata_dump_status(qc->ap->id, drv_stat, err);
/* Look for err */
while (sense_table[i][0] != 0xFF) {
applied patches 1-3, thanks much.
Small administrivia request, though: after using my patch merging
script to merge your patches, I have to do the following things manually:
* delete diffstat output
* delete patch filename
* un-indent patch description text
It would be great if I didn't have to do those manual steps :)
On the other hand, I really like your "patch 0/N" email with a
description of the entire patchset. Keep sending those, thanks.
Everything else looks great.
Jeff