[lkml]   [2005]   [Mar]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [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 <>

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



To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to
More majordomo info at
Please read the FAQ at

 \ /
  Last update: 2005-04-06 13:30    [from the cache]
©2003-2011 Jasper Spaans