[PATCH/RFC 3/3] libata: check qc->err_mask for the internal commands

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

Patch 3/3:
  check qc->err_mask for the internal commands.

Problem:
  The results of internal commands issued by libata itself were not checked.

  For example, ata_dev_identify() only checks the device status (qc->tf.command).
  However, checking the device status is not enough to detect all errors.

Changes:
  - check qc->err_mask for the internal commands:
    - ata_dev_identify()
    - ata_dev_set_xfermode()
    - ata_dev_reread_id()
    - ata_dev_init_params() 
    - atapi_request_sense()

The patch accesses qc (and ap) after the qc is completed.
This is bad from the object life cycle point of view and might cause race.
However, since libata has complete control over the hardware when libata issues the internal commands
and it seems noboby else may issue command at the same time, 
accessing qc after ata_qc_complete() looks to be ok here.

p.s. 
     The patch above can also make ata_dev_identify() detect the
     "phantom slave device" problem.

     The "phantom slave device" was seen in specific PATA master-only configuration.
     The master device responds to some commands issued to the slave device, and
     make a illusion that the slave device exists.
     The device status register of the "phantom slave device" always read as zero.
     So, using (qc->tf.command & ATA_ERR) cannot detect the error.
     Checking qc->err_mask can make ata_dev_identify() detect such 
     "phantom slave device" and exclude the phantom device.

For your review and advice, thanks.

Albert
Signed-off-by: Albert Lee <albertcc@tw.ibm.com> 

============
--- err_mask/drivers/scsi/libata-core.c	2005-11-09 10:15:33.000000000 +0800
+++ phantom/drivers/scsi/libata-core.c	2005-11-09 10:15:54.000000000 +0800
@@ -1132,7 +1132,7 @@ retry:
 	ap->ops->tf_read(ap, &qc->tf);
 	spin_unlock_irqrestore(&ap->host_set->lock, flags);
 
-	if (qc->tf.command & ATA_ERR) {
+	if (qc->err_mask) {
 		/*
 		 * arg!  EDD works for all test cases, but seems to return
 		 * the ATA signature for some ATAPI devices.  Until the
@@ -1144,8 +1144,10 @@ retry:
 		 * ATA software reset (SRST, the default) does not appear
 		 * to have this problem.
 		 */
-		if ((using_edd) && (dev->class == ATA_DEV_ATA)) {
+		if ((using_edd) && (dev->class == ATA_DEV_ATA) &&
+		    (qc->tf.command & ATA_ERR)) {
 			u8 err = qc->tf.feature;
+
 			if (err & ATA_ABORTED) {
 				dev->class = ATA_DEV_ATAPI;
 				qc->cursg = 0;
@@ -2267,11 +2269,19 @@ static void ata_dev_set_xfermode(struct 
 	spin_unlock_irqrestore(&ap->host_set->lock, flags);
 
 	if (rc)
-		ata_port_disable(ap);
-	else
-		wait_for_completion(&wait);
+		goto err_out;
+
+	wait_for_completion(&wait);
+
+	if (qc->err_mask)
+		goto err_out;
 
 	DPRINTK("EXIT\n");
+
+	return;
+err_out:
+	printk(KERN_ERR "ata%u: set XFER MODE failed\n", ap->id);
+	ata_port_disable(ap);
 }
 
 /**
@@ -2319,6 +2329,9 @@ static void ata_dev_reread_id(struct ata
 
 	wait_for_completion(&wait);
 
+	if (qc->err_mask)
+		goto err_out;
+
 	swap_buf_le16(dev->id, ATA_ID_WORDS);
 
 	ata_dump_id(dev);
@@ -2327,6 +2340,7 @@ static void ata_dev_reread_id(struct ata
 
 	return;
 err_out:
+	printk(KERN_ERR "ata%u: reread IDENTIFY device data failed\n", ap->id);
 	ata_port_disable(ap);
 }
 
@@ -2371,11 +2385,19 @@ static void ata_dev_init_params(struct a
 	spin_unlock_irqrestore(&ap->host_set->lock, flags);
 
 	if (rc)
-		ata_port_disable(ap);
-	else
-		wait_for_completion(&wait);
+		goto err_out;
+
+	wait_for_completion(&wait);
+
+	if (qc->err_mask)
+		goto err_out;
 
 	DPRINTK("EXIT\n");
+
+	return;
+err_out:
+	printk(KERN_ERR "ata%u: init CHS dev params failed\n", ap->id);
+	ata_port_disable(ap);
 }
 
 /**
--- err_mask/drivers/scsi/libata-scsi.c	2005-11-08 16:25:51.000000000 +0800
+++ phantom/drivers/scsi/libata-scsi.c	2005-11-09 10:37:39.000000000 +0800
@@ -2008,11 +2008,19 @@ void atapi_request_sense(struct ata_port
 	spin_unlock_irqrestore(&ap->host_set->lock, flags);
 
 	if (rc)
-		ata_port_disable(ap);
-	else
-		wait_for_completion(&wait);
+		goto err_out;
+
+	wait_for_completion(&wait);
+
+	if (qc->err_mask)
+		goto err_out;
 
 	DPRINTK("EXIT\n");
+
+	return;
+err_out:
+	printk(KERN_ERR "ata%u: atapi request sense failed\n", ap->id);
+	ata_port_disable(ap);
 }
 
 static int atapi_qc_complete(struct ata_queued_cmd *qc)