Mark Lord wrote:
This logic sequence bothers me slightly:
Tejun Heo wrote:
Separate out rw ATA taskfile building from ata_scsi_rw_xlat() into
ata_build_tf(). This will be used to improve media error handling.
...
+ if ((dev->flags & (ATA_DFLAG_PIO | ATA_DFLAG_NCQ_OFF |
+ ATA_DFLAG_NCQ)) == ATA_DFLAG_NCQ) {
+ /* yay, NCQ */
+ if (!lba_48_ok(block, n_block))
+ return -ERANGE;
...
+ } else if (dev->flags & ATA_DFLAG_LBA) {
+ tf->flags |= ATA_TFLAG_LBA;
+
+ if (lba_28_ok(block, n_block)) {
+ /* use LBA28 */
+ tf->device |= (block >> 24) & 0xf;
+ } else if (lba_48_ok(block, n_block)) {
+ if (!(dev->flags & ATA_DFLAG_LBA48))
+ return -ERANGE;
...
In the first case, if !lba_48_ok(), then we should at least attempt
to see if the request can be issued via non-NCQ.
Something like this:
if (lba_48_ok() && dev->flags &
(ATA_DFLAG_PIO|ATA_DFLAG_NCQ_OFF|ATA_DFLAG_NCQ)) == ATA_DFLAG_NCQ) {
/* yay, NCQ */
...
} else if (dev->flags & ATA_DFLAG_LBA) {
...
..except prettier! ;)
In theory, it looks like an impossible sequence anyway..
if we have the NCQ flag set, then we probably already *know*
that LBA48 is okay. But since we're not making that assumption,
let's give it the benefit of a doubt and try non-NCQ if LBA48 is a no-go.
First of all, that function is moved verbatim from ata_scsi_rw_xlat(),
and I don't really understand your point. lba_48_ok() checks whether
the given block and n_block are inside LBA48 limit. NCQ commands use
LBA48 (well, nsect is in the different registers but the limits are the
same) which is the largest addressing mode currently available. So, if
a command cannot be issued using NCQ, it cannot be issued w/ any command
mode. ie. if !lba_48_ok(), that command just cannot be issued.
Am I misunderstanding you?
--
tejun
-
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
