[libata] REQUEST SENSE handling fixes

- Move ATAPI check-condition handling out of the timeout handler
- Use multi-qc-issue feature to issue REQUEST SENSE ATAPI PACKET
  command upon receiving an ATAPI check-condition.

This cleans things up a lot, and eliminates a nasty recursion bug.

diff --git a/drivers/scsi/libata-core.c b/drivers/scsi/libata-core.c
index 3855bfa8e8d4465a88a2be80383241eec0873bd3..ba1eb8b38e0030a7860187c15a7a405285198e80 100644 (file)
--- a/drivers/scsi/libata-core.c
+++ b/drivers/scsi/libata-core.c
@@ -3284,32 +3284,11 @@ static void ata_qc_timeout(struct ata_queued_cmd *qc)
 {
        struct ata_port *ap = qc->ap;
        struct ata_host_set *host_set = ap->host_set;
-       struct ata_device *dev = qc->dev;
        u8 host_stat = 0, drv_stat;
        unsigned long flags;
 
        DPRINTK("ENTER\n");
 
-       /* FIXME: doesn't this conflict with timeout handling? */
-       if (qc->dev->class == ATA_DEV_ATAPI && qc->scsicmd) {
-               struct scsi_cmnd *cmd = qc->scsicmd;
-
-               if (!(cmd->eh_eflags & SCSI_EH_CANCEL_CMD)) {
-
-                       /* finish completing original command */
-                       spin_lock_irqsave(&host_set->lock, flags);
-                       __ata_qc_complete(qc);
-                       spin_unlock_irqrestore(&host_set->lock, flags);
-
-                       atapi_request_sense(ap, dev, cmd);
-
-                       cmd->result = (CHECK_CONDITION << 1) | (DID_OK << 16);
-                       scsi_finish_command(cmd);
-
-                       goto out;
-               }
-       }
-
        spin_lock_irqsave(&host_set->lock, flags);
 
        /* hack alert!  We cannot use the supplied completion
@@ -3348,7 +3327,6 @@ static void ata_qc_timeout(struct ata_queued_cmd *qc)
 
        spin_unlock_irqrestore(&host_set->lock, flags);
 
-out:
        DPRINTK("EXIT\n");
 }
 

diff --git a/drivers/scsi/libata-scsi.c b/drivers/scsi/libata-scsi.c
index 0df4b682965d41aab2d040edc7914444f3ebe10e..3b4ca55a3332eb02c33a6c4afda97bfa7b4f46fc 100644 (file)
--- a/drivers/scsi/libata-scsi.c
+++ b/drivers/scsi/libata-scsi.c
@@ -1955,22 +1955,44 @@ void ata_scsi_badcmd(struct scsi_cmnd *cmd, void (*done)(struct scsi_cmnd *), u8
        done(cmd);
 }
 
-void atapi_request_sense(struct ata_port *ap, struct ata_device *dev,
-                        struct scsi_cmnd *cmd)
+static int atapi_sense_complete(struct ata_queued_cmd *qc,unsigned int err_mask)
 {
-       DECLARE_COMPLETION(wait);
-       struct ata_queued_cmd *qc;
-       unsigned long flags;
-       int rc;
+       if (err_mask && ((err_mask & AC_ERR_DEV) == 0))
+               /* FIXME: not quite right; we don't want the
+                * translation of taskfile registers into
+                * a sense descriptors, since that's only
+                * correct for ATA, not ATAPI
+                */
+               ata_gen_ata_desc_sense(qc);
 
-       DPRINTK("ATAPI request sense\n");
+       qc->scsidone(qc->scsicmd);
+       return 0;
+}
 
-       qc = ata_qc_new_init(ap, dev);
-       BUG_ON(qc == NULL);
+/* is it pointless to prefer PIO for "safety reasons"? */
+static inline int ata_pio_use_silly(struct ata_port *ap)
+{
+       return (ap->flags & ATA_FLAG_PIO_DMA);
+}
+
+static void atapi_request_sense(struct ata_queued_cmd *qc)
+{
+       struct ata_port *ap = qc->ap;
+       struct scsi_cmnd *cmd = qc->scsicmd;
+
+       DPRINTK("ATAPI request sense\n");
 
        /* FIXME: is this needed? */
        memset(cmd->sense_buffer, 0, sizeof(cmd->sense_buffer));
 
+       ap->ops->tf_read(ap, &qc->tf);
+
+       /* fill these in, for the case where they are -not- overwritten */
+       cmd->sense_buffer[0] = 0x70;
+       cmd->sense_buffer[2] = qc->tf.feature >> 4;
+
+       ata_qc_reinit(qc);
+
        ata_sg_init_one(qc, cmd->sense_buffer, sizeof(cmd->sense_buffer));
        qc->dma_dir = DMA_FROM_DEVICE;
 
@@ -1981,22 +2003,20 @@ void atapi_request_sense(struct ata_port *ap, struct ata_device *dev,
        qc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
        qc->tf.command = ATA_CMD_PACKET;
 
-       qc->tf.protocol = ATA_PROT_ATAPI;
-       qc->tf.lbam = (8 * 1024) & 0xff;
-       qc->tf.lbah = (8 * 1024) >> 8;
+       if (ata_pio_use_silly(ap)) {
+               qc->tf.protocol = ATA_PROT_ATAPI_DMA;
+               qc->tf.feature |= ATAPI_PKT_DMA;
+       } else {
+               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;
+       qc->complete_fn = atapi_sense_complete;
 
-       spin_lock_irqsave(&ap->host_set->lock, flags);
-       rc = ata_qc_issue(qc);
-       spin_unlock_irqrestore(&ap->host_set->lock, flags);
-
-       if (rc)
-               ata_port_disable(ap);
-       else
-               wait_for_completion(&wait);
+       if (ata_qc_issue(qc))
+               ata_qc_complete(qc, AC_ERR_OTHER);
 
        DPRINTK("EXIT\n");
 }
@@ -2008,19 +2028,8 @@ static int atapi_qc_complete(struct ata_queued_cmd *qc, unsigned int err_mask)
        VPRINTK("ENTER, err_mask 0x%X\n", err_mask);
 
        if (unlikely(err_mask & AC_ERR_DEV)) {
-               DPRINTK("request check condition\n");
-
-               /* FIXME: command completion with check condition
-                * but no sense causes the error handler to run,
-                * which then issues REQUEST SENSE, fills in the sense 
-                * buffer, and completes the command (for the second
-                * time).  We need to issue REQUEST SENSE some other
-                * way, to avoid completing the command twice.
-                */
                cmd->result = SAM_STAT_CHECK_CONDITION;
-
-               qc->scsidone(cmd);
-
+               atapi_request_sense(qc);
                return 1;
        }
 

diff --git a/drivers/scsi/libata.h b/drivers/scsi/libata.h
index fad051ca4672bf0daf6ed710cf4155f977bb7662..74a84e0ec0a42d0b077f60bcd420f2651a9e6cad 100644 (file)
--- a/drivers/scsi/libata.h
+++ b/drivers/scsi/libata.h
@@ -54,8 +54,6 @@ extern int ata_cmd_ioctl(struct scsi_device *scsidev, void __user *arg);
 
 
 /* libata-scsi.c */
-extern void atapi_request_sense(struct ata_port *ap, struct ata_device *dev,
-                        struct scsi_cmnd *cmd);
 extern void ata_scsi_scan_host(struct ata_port *ap);
 extern int ata_scsi_error(struct Scsi_Host *host);
 extern unsigned int ata_scsiop_inq_std(struct ata_scsi_args *args, u8 *rbuf,