Re: [PATCH] libata: Use ATAPI PIO mode for specific request buffer sizes

["Albert Lee" <albertcc@tw.ibm.com>]

[-- Attachment #1: Type: text/plain, Size: 2578 bytes --]

Hi, Jeff:

> > ..
> > Two changes:
> > 1. Use ATAPI PIO mode for ATAPI REQUEST_SENSE
> > 2. Use ATAPI PIO mode for specific SCSI commands issued to ATAPI device that
> >     buffer_size % 256 != 0.
> > 
> >   Since for most commands, buffer_size % 256 is 0,  this patch
> > won't introduce big impact to the performance of libata (hopefully).
> >   
> >   Tested OK on on my machine with pdc20275 and ASUS CD-RW drive.
> 
> hmmmmmmm.
> 
> Your change #1 makes perfect sense, and I agree it is the safer (and 
> more simple) route.
> 
> I must ponder change #2 some more (after some sleep :)).  In particular, 
> I am concerned that making change #2 across all controllers will hurt 
> performance on the commands whose data length is not purely sector-sized 
> chunks.
> 
> Would you mind splitting up change #1 and #2 into separate patches?
> 

  Attached please find the request sense patch for change #1 (the patch is against 
libata-dev-2.6 tree).

  For change #2, agreed the previous patch is problematic since it will impact all 
adapters and the buffer_size % 256 rule might also fail for other host adapters. 

  How about let LLD provide a callback function such that libata core
can ask the LLD: "Which protocol do you like this command to be processed? " or
"Can this command be processed in ATAPI DMA mode reliably? " 
when the the command is received?

Albert

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
-----------------------------------------------------------------------------------------------------------
diff -Nru libata-dev-2.6-ori/drivers/scsi/libata-core.c libata-dev-2.6/drivers/scsi/libata-core.c
--- libata-dev-2.6-ori/drivers/scsi/libata-core.c 2004-11-30 12:52:28.000000000 +0800
+++ libata-dev-2.6/drivers/scsi/libata-core.c 2004-11-30 13:49:36.000000000 +0800
@@ -2435,7 +2435,6 @@
  DECLARE_COMPLETION(wait);
  struct ata_queued_cmd *qc;
  unsigned long flags;
- int using_pio = dev->flags & ATA_DFLAG_PIO;
  int rc;
 
  DPRINTK("ATAPI request sense\n");
@@ -2456,16 +2455,10 @@
  qc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
  qc->tf.command = ATA_CMD_PACKET;
 
- if (using_pio) {
-  qc->tf.protocol = ATA_PROT_ATAPI;
-  qc->tf.lbam = (8 * 1024) & 0xff;
-  qc->tf.lbah = (8 * 1024) >> 8;
-
-  qc->nbytes = SCSI_SENSE_BUFFERSIZE;
- } else {
-  qc->tf.protocol = ATA_PROT_ATAPI_DMA;
-  qc->tf.feature |= ATAPI_PKT_DMA;
- }
+ qc->tf.protocol = ATA_PROT_ATAPI;
+ qc->tf.lbam = (8 * 1024) & 0xff;
+ qc->tf.lbah = (8 * 1024) >> 8;
+ qc->nbytes = SCSI_SENSE_BUFFERSIZE;
 
  qc->waiting = &wait;
  qc->complete_fn = ata_qc_complete_noop;


[-- Attachment #2: request_sense.patch --]
[-- Type: application/octet-stream, Size: 1046 bytes --]

diff -Nru libata-dev-2.6-ori/drivers/scsi/libata-core.c libata-dev-2.6/drivers/scsi/libata-core.c
--- libata-dev-2.6-ori/drivers/scsi/libata-core.c	2004-11-30 12:52:28.000000000 +0800
+++ libata-dev-2.6/drivers/scsi/libata-core.c	2004-11-30 13:49:36.000000000 +0800
@@ -2435,7 +2435,6 @@
 	DECLARE_COMPLETION(wait);
 	struct ata_queued_cmd *qc;
 	unsigned long flags;
-	int using_pio = dev->flags & ATA_DFLAG_PIO;
 	int rc;
 
 	DPRINTK("ATAPI request sense\n");
@@ -2456,16 +2455,10 @@
 	qc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
 	qc->tf.command = ATA_CMD_PACKET;
 
-	if (using_pio) {
-		qc->tf.protocol = ATA_PROT_ATAPI;
-		qc->tf.lbam = (8 * 1024) & 0xff;
-		qc->tf.lbah = (8 * 1024) >> 8;
-
-		qc->nbytes = SCSI_SENSE_BUFFERSIZE;
-	} else {
-		qc->tf.protocol = ATA_PROT_ATAPI_DMA;
-		qc->tf.feature |= ATAPI_PKT_DMA;
-	}
+	qc->tf.protocol = ATA_PROT_ATAPI;
+	qc->tf.lbam = (8 * 1024) & 0xff;
+	qc->tf.lbah = (8 * 1024) >> 8;
+	qc->nbytes = SCSI_SENSE_BUFFERSIZE;
 
 	qc->waiting = &wait;
 	qc->complete_fn = ata_qc_complete_noop;