[PATCH] libata: separate out ata_dev_read_id()

Separate out ata_dev_read_id() from ata_dev_identify().  This is the
first half of splitting ata_dev_identify().  ata_dev_read_id() will
also be used for revalidation.  This patch does not make any behavior
change.

ata_dev_read_id() doesn't modify any of libata-internal data
structures.  It simply reads IDENTIFY page and returns error code on
failure.  INIT_DEV_PARAMS and EDD wrong class code are also handled by
this function.

Re-reading IDENTIFY after INIT_DEV_PARAMS is performed by jumping to
retry: instead of calling ata_dev_reread_id().  This is done because
1. there's retry label anyway 2. ata_dev_reread_id() cannot be used
anywhere else so there's no reason to keep it.

This function is probably the place to set transfer mode to PIO0
before IDENTIFY.  However, reset -> identify -> init_dev_params order
should be kept for pre-ATA4 devices so we cannot set transfer mode
before IDENTIFY for them.  How do we know if a device is post-ATA4
before IDENTIFY?
Signed-off-by: NTejun Heo <htejun@gmail.com>
Signed-off-by: NJeff Garzik <jgarzik@pobox.com>

drivers/scsi/libata-core.c

@@ -903,42 +903,36 @@ unsigned int ata_pio_need_iordy(const struct ata_device *adev)
 }
 
 /**
- *	ata_dev_identify - obtain IDENTIFY x DEVICE page
- *	@ap: port on which device we wish to probe resides
- *	@device: device bus address, starting at zero
- *
- *	Following bus reset, we issue the IDENTIFY [PACKET] DEVICE
- *	command, and read back the 512-byte device information page.
- *	The device information page is fed to us via the standard
- *	PIO-IN protocol, but we hand-code it here. (TODO: investigate
- *	using standard PIO-IN paths)
- *
- *	After reading the device information page, we use several
- *	bits of information from it to initialize data structures
- *	that will be used during the lifetime of the ata_device.
- *	Other data from the info page is used to disqualify certain
- *	older ATA devices we do not wish to support.
+ *	ata_dev_read_id - Read ID data from the specified device
+ *	@ap: port on which target device resides
+ *	@dev: target device
+ *	@p_class: pointer to class of the target device (may be changed)
+ *	@post_reset: is this read ID post-reset?
+ *	@id: buffer to fill IDENTIFY page into
+ *
+ *	Read ID data from the specified device.  ATA_CMD_ID_ATA is
+ *	performed on ATA devices and ATA_CMD_ID_ATAPI on ATAPI
+ *	devices.  This function also takes care of EDD signature
+ *	misreporting (to be removed once EDD support is gone) and
+ *	issues ATA_CMD_INIT_DEV_PARAMS for pre-ATA4 drives.
  *
  *	LOCKING:
- *	Inherited from caller.  Some functions called by this function
- *	obtain the host_set lock.
+ *	Kernel thread context (may sleep)
+ *
+ *	RETURNS:
+ *	0 on success, -errno otherwise.
  */
-
-static void ata_dev_identify(struct ata_port *ap, unsigned int device)
+static int ata_dev_read_id(struct ata_port *ap, struct ata_device *dev,
+			   unsigned int *p_class, int post_reset, u16 *id)
 {
-	struct ata_device *dev = &ap->device[device];
-	unsigned int major_version;
-	unsigned long xfer_modes;
+	unsigned int class = *p_class;
 	unsigned int using_edd;
 	struct ata_taskfile tf;
-	unsigned int err_mask;
-	int i, rc;
+	unsigned int err_mask = 0;
+	const char *reason;
+	int rc;
 
-	if (!ata_dev_present(dev)) {
-		DPRINTK("ENTER/EXIT (host %u, dev %u) -- nodev\n",
-			ap->id, device);
-		return;
-	}
+	DPRINTK("ENTER, host %u, dev %u\n", ap->id, dev->devno);
 
 	if (ap->ops->probe_reset ||
 	    ap->flags & (ATA_FLAG_SRST | ATA_FLAG_SATA_RESET))
@@ -946,30 +940,33 @@ static void ata_dev_identify(struct ata_port *ap, unsigned int device)
 	else
 		using_edd = 1;
 
-	DPRINTK("ENTER, host %u, dev %u\n", ap->id, device);
-
-	WARN_ON(dev->class != ATA_DEV_ATA && dev->class != ATA_DEV_ATAPI &&
-		dev->class != ATA_DEV_NONE);
-
-	ata_dev_select(ap, device, 1, 1); /* select device 0/1 */
+	ata_dev_select(ap, dev->devno, 1, 1); /* select device 0/1 */
 
-retry:
-	ata_tf_init(ap, &tf, device);
+ retry:
+	ata_tf_init(ap, &tf, dev->devno);
 
-	if (dev->class == ATA_DEV_ATA) {
+	switch (class) {
+	case ATA_DEV_ATA:
 		tf.command = ATA_CMD_ID_ATA;
-		DPRINTK("do ATA identify\n");
-	} else {
+		break;
+	case ATA_DEV_ATAPI:
 		tf.command = ATA_CMD_ID_ATAPI;
-		DPRINTK("do ATAPI identify\n");
+		break;
+	default:
+		rc = -ENODEV;
+		reason = "unsupported class";
+		goto err_out;
 	}
 
 	tf.protocol = ATA_PROT_PIO;
 
 	err_mask = ata_exec_internal(ap, dev, &tf, DMA_FROM_DEVICE,
-				     dev->id, sizeof(dev->id));
+				     id, sizeof(id[0]) * ATA_ID_WORDS);
 
 	if (err_mask) {
+		rc = -EIO;
+		reason = "I/O error";
+
 		if (err_mask & ~AC_ERR_DEV)
 			goto err_out;
 
@@ -984,25 +981,105 @@ static void ata_dev_identify(struct ata_port *ap, unsigned int device)
 		 * ATA software reset (SRST, the default) does not appear
 		 * to have this problem.
 		 */
-		if ((using_edd) && (dev->class == ATA_DEV_ATA)) {
+		if ((using_edd) && (class == ATA_DEV_ATA)) {
 			u8 err = tf.feature;
 			if (err & ATA_ABORTED) {
-				dev->class = ATA_DEV_ATAPI;
+				class = ATA_DEV_ATAPI;
 				goto retry;
 			}
 		}
 		goto err_out;
 	}
 
-	swap_buf_le16(dev->id, ATA_ID_WORDS);
+	swap_buf_le16(id, ATA_ID_WORDS);
 
 	/* print device capabilities */
 	printk(KERN_DEBUG "ata%u: dev %u cfg "
 	       "49:%04x 82:%04x 83:%04x 84:%04x 85:%04x 86:%04x 87:%04x 88:%04x\n",
-	       ap->id, device, dev->id[49],
-	       dev->id[82], dev->id[83], dev->id[84],
-	       dev->id[85], dev->id[86], dev->id[87],
-	       dev->id[88]);
+	       ap->id, dev->devno,
+	       id[49], id[82], id[83], id[84], id[85], id[86], id[87], id[88]);
+
+	/* sanity check */
+	if ((class == ATA_DEV_ATA) != ata_id_is_ata(id)) {
+		rc = -EINVAL;
+		reason = "device reports illegal type";
+		goto err_out;
+	}
+
+	if (post_reset && class == ATA_DEV_ATA) {
+		/*
+		 * The exact sequence expected by certain pre-ATA4 drives is:
+		 * SRST RESET
+		 * IDENTIFY
+		 * INITIALIZE DEVICE PARAMETERS
+		 * anything else..
+		 * Some drives were very specific about that exact sequence.
+		 */
+		if (ata_id_major_version(id) < 4 || !ata_id_has_lba(id)) {
+			err_mask = ata_dev_init_params(ap, dev);
+			if (err_mask) {
+				rc = -EIO;
+				reason = "INIT_DEV_PARAMS failed";
+				goto err_out;
+			}
+
+			/* current CHS translation info (id[53-58]) might be
+			 * changed. reread the identify device info.
+			 */
+			post_reset = 0;
+			goto retry;
+		}
+	}
+
+	*p_class = class;
+	return 0;
+
+ err_out:
+	printk(KERN_WARNING "ata%u: dev %u failed to IDENTIFY (%s)\n",
+	       ap->id, dev->devno, reason);
+	kfree(id);
+	return rc;
+}
+
+/**
+ *	ata_dev_identify - obtain IDENTIFY x DEVICE page
+ *	@ap: port on which device we wish to probe resides
+ *	@device: device bus address, starting at zero
+ *
+ *	Following bus reset, we issue the IDENTIFY [PACKET] DEVICE
+ *	command, and read back the 512-byte device information page.
+ *	The device information page is fed to us via the standard
+ *	PIO-IN protocol, but we hand-code it here. (TODO: investigate
+ *	using standard PIO-IN paths)
+ *
+ *	After reading the device information page, we use several
+ *	bits of information from it to initialize data structures
+ *	that will be used during the lifetime of the ata_device.
+ *	Other data from the info page is used to disqualify certain
+ *	older ATA devices we do not wish to support.
+ *
+ *	LOCKING:
+ *	Inherited from caller.  Some functions called by this function
+ *	obtain the host_set lock.
+ */
+
+static void ata_dev_identify(struct ata_port *ap, unsigned int device)
+{
+	struct ata_device *dev = &ap->device[device];
+	unsigned long xfer_modes;
+	int i, rc;
+
+	if (!ata_dev_present(dev)) {
+		DPRINTK("ENTER/EXIT (host %u, dev %u) -- nodev\n",
+			ap->id, device);
+		return;
+	}
+
+	DPRINTK("ENTER, host %u, dev %u\n", ap->id, device);
+
+	rc = ata_dev_read_id(ap, dev, &dev->class, 1, dev->id);
+	if (rc)
+		goto err_out;
 
 	/*
 	 * common ATA, ATAPI feature tests
@@ -1027,34 +1104,6 @@ static void ata_dev_identify(struct ata_port *ap, unsigned int device)
 	if (dev->class == ATA_DEV_ATA) {
 		dev->n_sectors = ata_id_n_sectors(dev->id);
 
-		if (!ata_id_is_ata(dev->id))	/* sanity check */
-			goto err_out_nosup;
-
-		/* get major version */
-		major_version = ata_id_major_version(dev->id);
-
-		/*
-		 * The exact sequence expected by certain pre-ATA4 drives is:
-		 * SRST RESET
-		 * IDENTIFY
-		 * INITIALIZE DEVICE PARAMETERS
-		 * anything else..
-		 * Some drives were very specific about that exact sequence.
-		 */
-		if (major_version < 4 || (!ata_id_has_lba(dev->id))) {
-			err_mask = ata_dev_init_params(ap, dev);
-			if (err_mask) {
-				printk(KERN_ERR "ata%u: failed to init "
-				       "parameters, disabled\n", ap->id);
-				goto err_out;
-			}
-
-			/* current CHS translation info (id[53-58]) might be
-			 * changed. reread the identify device info.
-			 */
-			ata_dev_reread_id(ap, dev);
-		}
-
 		if (ata_id_has_lba(dev->id)) {
 			dev->flags |= ATA_DFLAG_LBA;
 
@@ -1064,7 +1113,7 @@ static void ata_dev_identify(struct ata_port *ap, unsigned int device)
 			/* print device info to dmesg */
 			printk(KERN_INFO "ata%u: dev %u ATA-%d, max %s, %Lu sectors:%s\n",
 			       ap->id, device,
-			       major_version,
+			       ata_id_major_version(dev->id),
 			       ata_mode_string(xfer_modes),
 			       (unsigned long long)dev->n_sectors,
 			       dev->flags & ATA_DFLAG_LBA48 ? " LBA48" : " LBA");
@@ -1086,7 +1135,7 @@ static void ata_dev_identify(struct ata_port *ap, unsigned int device)
 			/* print device info to dmesg */
 			printk(KERN_INFO "ata%u: dev %u ATA-%d, max %s, %Lu sectors: CHS %d/%d/%d\n",
 			       ap->id, device,
-			       major_version,
+			       ata_id_major_version(dev->id),
 			       ata_mode_string(xfer_modes),
 			       (unsigned long long)dev->n_sectors,
 			       (int)dev->cylinders, (int)dev->heads, (int)dev->sectors);
@@ -1098,9 +1147,6 @@ static void ata_dev_identify(struct ata_port *ap, unsigned int device)
 
 	/* ATAPI-specific feature tests */
 	else if (dev->class == ATA_DEV_ATAPI) {
-		if (ata_id_is_ata(dev->id))		/* sanity check */
-			goto err_out_nosup;
-
 		rc = atapi_cdb_len(dev->id);
 		if ((rc < 12) || (rc > ATAPI_CDB_LEN)) {
 			printk(KERN_WARNING "ata%u: unsupported CDB len\n", ap->id);