Re: [PATCH libata-dev-2.6 05/05] libata: rework how CCs generated
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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH libata-dev-2.6 05/05] libata: rework how CCs generated
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 - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH libata-dev-2.6 05/05] libata: rework how CCs generated
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.0 -0500
+++ libata-dev-2.6/drivers/scsi/libata-scsi.c 2005-03-17 17:16:59.0
-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
Re: [PATCH libata-dev-2.6 05/05] libata: rework how CCs generated
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.0 -0500
+++ libata-dev-2.6/drivers/scsi/libata-scsi.c 2005-03-17 17:16:59.0
-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
