[PATCH 0/5] libata-dev: irq-pio minor fixes

[PATCH 0/5] libata-dev: irq-pio minor fixes

[Albert Lee]

Minor fixes for irq-pio to catch up with upstream.

Changes:
1/5: Add back missing dev->cdb_len = 16
2/5: Make dev->id dynamically allocated
3/5: Remove ata_dev_identify(), ata_dev_config() and ata_bus_probe().
4/5: Add back ata_dev_configure(), ata_bus_probe(), ata_dev_same_device() and ata_dev_revalidate().
5/5: Add back irq-pio patches, etc.

Patch against the irq-pio branch (c2956a3b0d1c17b38da369811a6ce93eb7a01a04). 

For your review, thanks,

Albert

[PATCH 1/5] libata-dev: Add back missing dev->cdb_len = 16

[Albert Lee]

Add back missing dev->cdb_len = 16 in ata_dev_identify().

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

---
Originally done in Tejun's patch.
Add back to make hard drive probed.

--- irq-pio/drivers/scsi/libata-core.c	2006-03-10 16:03:36.000000000 +0800
+++ catchup0/drivers/scsi/libata-core.c	2006-03-10 18:18:04.000000000 +0800
@@ -1143,6 +1143,7 @@ static void ata_dev_identify(struct ata_
 				ap->id, device, dev->multi_count);
 		}
 
+		dev->cdb_len = 16;
 	}
 
 	/* ATAPI-specific feature tests */

[PATCH 2/5] libata-dev: Make dev->id dynamically allocated

[Albert Lee]

Make dev->id dynamically allocated

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

This is respin of Tejun's patch, too.
Maybe git can pull it from upstream easily?

--- catchup0/include/linux/libata.h	2006-03-10 17:25:48.000000000 +0800
+++ catchup1/include/linux/libata.h	2006-03-10 17:40:50.000000000 +0800
@@ -344,7 +344,7 @@ struct ata_device {
 	unsigned long		flags;		/* ATA_DFLAG_xxx */
 	unsigned int		class;		/* ATA_DEV_xxx */
 	unsigned int		devno;		/* 0 or 1 */
-	u16			id[ATA_ID_WORDS]; /* IDENTIFY xxx DEVICE data */
+	u16			*id;		/* IDENTIFY xxx DEVICE data */
 	u8			pio_mode;
 	u8			dma_mode;
 	u8			xfer_mode;
@@ -648,10 +648,14 @@ static inline unsigned int ata_tag_valid
 	return (tag < ATA_MAX_QUEUE) ? 1 : 0;
 }
 
+static inline unsigned int ata_class_present(unsigned int class)
+{
+	return class == ATA_DEV_ATA || class == ATA_DEV_ATAPI;
+}
+
 static inline unsigned int ata_dev_present(const struct ata_device *dev)
 {
-	return ((dev->class == ATA_DEV_ATA) ||
-		(dev->class == ATA_DEV_ATAPI));
+	return ata_class_present(dev->class);
 }
 
 static inline u8 ata_chk_status(struct ata_port *ap)
--- catchup0/drivers/scsi/libata-core.c	2006-03-10 18:18:04.000000000 +0800
+++ catchup1/drivers/scsi/libata-core.c	2006-03-10 18:19:11.000000000 +0800
@@ -904,7 +904,7 @@ unsigned int ata_pio_need_iordy(const st
  *	@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
+ *	@p_id: read IDENTIFY page (newly allocated)
  *
  *	Read ID data from the specified device.  ATA_CMD_ID_ATA is
  *	performed on ATA devices and ATA_CMD_ID_ATAPI on ATAPI
@@ -919,12 +919,13 @@ unsigned int ata_pio_need_iordy(const st
  *	0 on success, -errno otherwise.
  */
 static int ata_dev_read_id(struct ata_port *ap, struct ata_device *dev,
-			   unsigned int *p_class, int post_reset, u16 *id)
+			   unsigned int *p_class, int post_reset, u16 **p_id)
 {
 	unsigned int class = *p_class;
 	unsigned int using_edd;
 	struct ata_taskfile tf;
 	unsigned int err_mask = 0;
+	u16 *id;
 	const char *reason;
 	int rc;
 
@@ -938,6 +939,13 @@ static int ata_dev_read_id(struct ata_po
 
 	ata_dev_select(ap, dev->devno, 1, 1); /* select device 0/1 */
 
+	id = kmalloc(sizeof(id[0]) * ATA_ID_WORDS, GFP_KERNEL);
+	if (id == NULL) {
+		rc = -ENOMEM;
+		reason = "out of memory";
+		goto err_out;
+	}
+
  retry:
 	ata_tf_init(ap, &tf, dev->devno);
 
@@ -1028,11 +1036,13 @@ static int ata_dev_read_id(struct ata_po
 	}
 
 	*p_class = class;
+	*p_id = id;
 	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;
 }
 
@@ -5008,11 +5018,14 @@ void ata_host_set_remove(struct ata_host
 int ata_scsi_release(struct Scsi_Host *host)
 {
 	struct ata_port *ap = (struct ata_port *) &host->hostdata[0];
+	int i;
 
 	DPRINTK("ENTER\n");
 
 	ap->ops->port_disable(ap);
 	ata_host_remove(ap, 0);
+	for (i = 0; i < ATA_MAX_DEVICES; i++)
+		kfree(ap->device[i].id);
 
 	DPRINTK("EXIT\n");
 	return 1;

[PATCH 5/5] libata-dev: Add back irq-pio patches, etc.

[Albert Lee]

Add back irq-pio patches, etc.

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

---

Changes:
- add back irq-pio patches removed in patch 3/5.
- ata_dev_revalidate() by Tejun
- remove redundant ap->hsm_task_state = HSM_ST_IDLE in ata_qc_timeout().

--- catchup3/drivers/scsi/libata-core.c	2006-03-10 18:04:35.000000000 +0800
+++ catchup4/drivers/scsi/libata-core.c	2006-03-10 18:11:56.000000000 +0800
@@ -1159,6 +1159,12 @@ static int ata_dev_configure(struct ata_
 				       dev->cylinders, dev->heads, dev->sectors);
 		}
 
+		if (dev->id[59] & 0x100) {
+			dev->multi_count = dev->id[59] & 0xff;
+			DPRINTK("ata%u: dev %u multi count %u\n",
+				ap->id, device, dev->multi_count);
+		}
+
 		dev->cdb_len = 16;
 	}
 
@@ -1172,6 +1178,9 @@ static int ata_dev_configure(struct ata_
 		}
 		dev->cdb_len = (unsigned int) rc;
 
+		if (ata_id_cdb_intr(dev->id))
+			dev->flags |= ATA_DFLAG_CDB_INTR;
+
 		/* print device info to dmesg */
 		if (print_info)
 			printk(KERN_INFO "ata%u: dev %u ATAPI, max %s\n",
@@ -1618,6 +1627,12 @@ static void ata_dev_set_mode(struct ata_
 	idx = ofs + dev->xfer_shift;
 	WARN_ON(idx >= ARRAY_SIZE(xfer_mode_str));
 
+	if (ata_dev_revalidate(ap, dev, 0)) {
+		printk(KERN_ERR "ata%u: failed to revalidate after set "
+		       "xfermode, disabled\n", ap->id);
+		ata_port_disable(ap);
+	}
+
 	DPRINTK("idx=%d xfer_shift=%u, xfer_mode=0x%x, base=0x%x, offset=%d\n",
 		idx, dev->xfer_shift, (int)dev->xfer_mode, (int)base, ofs);
 
@@ -3853,8 +3868,6 @@ static void ata_qc_timeout(struct ata_qu
 		printk(KERN_ERR "ata%u: command 0x%x timeout, stat 0x%x host_stat 0x%x\n",
 		       ap->id, qc->tf.command, drv_stat, host_stat);
 
-		ap->hsm_task_state = HSM_ST_IDLE;
-
 		/* complete taskfile transaction */
 		qc->err_mask |= AC_ERR_TIMEOUT;
 		break;

Re: [PATCH 0/5] libata-dev: irq-pio minor fixes

[Jeff Garzik]

Albert Lee wrote:
> Minor fixes for irq-pio to catch up with upstream.
> 
> Changes:
> 1/5: Add back missing dev->cdb_len = 16
> 2/5: Make dev->id dynamically allocated
> 3/5: Remove ata_dev_identify(), ata_dev_config() and ata_bus_probe().
> 4/5: Add back ata_dev_configure(), ata_bus_probe(), ata_dev_same_device() and ata_dev_revalidate().
> 5/5: Add back irq-pio patches, etc.
> 
> Patch against the irq-pio branch (c2956a3b0d1c17b38da369811a6ce93eb7a01a04). 

I pulled the current #upstream into the irq-pio, and then fixed the 
merge problems.

Would you be kind enough to resync against this, and resend what's 
needed?  I would rather not have these remove-and-add-back patches.

Also, I never received patch #3 in this series.

	Jeff

Re: [PATCH 0/5] libata-dev: irq-pio minor fixes

[Albert Lee]

Jeff Garzik wrote:

> 
> 
> I pulled the current #upstream into the irq-pio, and then fixed the
> merge problems.

Thanks. :)

> 
> Would you be kind enough to resync against this, and resend what's
> needed?  I would rather not have these remove-and-add-back patches.

Ok, will check and resent.

> 
> Also, I never received patch #3 in this series.
> 

Possibly blocked by the outgoing mail virus scanner, will check it.

Thanks,
Albert
 

Re: [PATCH 0/5] libata-dev: irq-pio minor fixes

[Tejun Heo]

Albert Lee wrote:
> Jeff Garzik wrote:
> 
> 
>>
>>I pulled the current #upstream into the irq-pio, and then fixed the
>>merge problems.
> 
> 
> Thanks. :)
> 
> 
>>Would you be kind enough to resync against this, and resend what's
>>needed?  I would rather not have these remove-and-add-back patches.
> 
> 
> Ok, will check and resent.
> 
> 
>>Also, I never received patch #3 in this series.
>>
> 
> 
> Possibly blocked by the outgoing mail virus scanner, will check it.
> 

FWIW, I've received patch #3 fine.

-- 
tejun

[PATCH 0/3] libata-dev: irq-pio minor fixes (respin)

[Albert Lee]

Minor fixes for irq-pio (respin).

Changes:
1/3: Remove trailing whitespaces
2/3: Fix merge problem with upstream
3/3: Remove atapi_packet_task()

Patch against the irq-pio branch (54da9a3968448681d0ddf193ec90f2480c5cba1c). 

For your review, thanks,

Albert

[PATCH 1/3] libata-dev: Remove trailing whitespaces

[Albert Lee]

Remove trailing whitespaces.

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
Remove trailing whitespaces per Tejun's advice.

--- irq-pio0/drivers/scsi/libata-core.c	2006-03-13 10:02:51.000000000 +0800
+++ 001_whitespace/drivers/scsi/libata-core.c	2006-03-13 12:38:03.000000000 +0800
@@ -4594,7 +4594,7 @@ inline unsigned int ata_host_intr (struc
 fsm_start:
 	switch (ap->hsm_task_state) {
 	case HSM_ST_FIRST:
-		/* Some pre-ATAPI-4 devices assert INTRQ 
+		/* Some pre-ATAPI-4 devices assert INTRQ
 		 * at this state when ready to receive CDB.
 		 */
 
@@ -4619,7 +4619,7 @@ fsm_start:
 				ap->hsm_task_state = HSM_ST_LAST;
 				goto fsm_start;
 			}
-			
+
 			atapi_pio_bytes(qc);
 
 			if (unlikely(ap->hsm_task_state == HSM_ST_ERR))
@@ -4673,7 +4673,7 @@ fsm_start:
 			printk(KERN_ERR "ata%u: command error, drv_stat 0x%x host_stat 0x%x\n",
 			       ap->id, status, host_stat);
 
-		/* make sure qc->err_mask is available to 
+		/* make sure qc->err_mask is available to
 		 * know what's wrong and recover
 		 */
 		WARN_ON(qc->err_mask == 0);

Re: [PATCH 1/3] libata-dev: Remove trailing whitespaces

[Jeff Garzik]

Albert Lee wrote:
> Remove trailing whitespaces.
> 
> Signed-off-by: Albert Lee <albertcc@tw.ibm.com>

applied 1-3 to irq-pio

Perfect patch formatting, thanks.

[PATCH 2/3] libata-dev: Fix merge problem with upstream

[Albert Lee]

Fix merge problem with upstream.

Signed-off-by: Albert Lee <albertcc@tw.ibm.com>
---
Changes:
1. add missing dev->cdb_len = 16 for ATA devices
2. use ata_pio_task instead of atapi_packet_task

--- 001_whitespace/drivers/scsi/libata-core.c	2006-03-13 12:38:03.000000000 +0800
+++ 00_cdblen/drivers/scsi/libata-core.c	2006-03-13 12:25:40.000000000 +0800
@@ -1282,6 +1282,7 @@ static int ata_dev_configure(struct ata_
 				ap->id, device, dev->multi_count);
 		}
 
+		dev->cdb_len = 16;
 	}
 
 	/* ATAPI-specific feature tests */
@@ -4254,7 +4255,7 @@ unsigned int ata_qc_issue_prot(struct at
 		/* send cdb by polling if no cdb interrupt */
 		if ((!(qc->dev->flags & ATA_DFLAG_CDB_INTR)) ||
 		    (qc->tf.flags & ATA_TFLAG_POLLING))
-			ata_port_queue_task(ap, atapi_packet_task, ap, 0);
+			ata_port_queue_task(ap, ata_pio_task, ap, 0);
 		break;
 
 	case ATA_PROT_ATAPI_DMA:
@@ -4266,7 +4267,7 @@ unsigned int ata_qc_issue_prot(struct at
 
 		/* send cdb by polling if no cdb interrupt */
 		if (!(qc->dev->flags & ATA_DFLAG_CDB_INTR))
-			ata_port_queue_task(ap, atapi_packet_task, ap, 0);
+			ata_port_queue_task(ap, ata_pio_task, ap, 0);
 		break;
 
 	default:

[PATCH 3/3] libata-dev: Remove atapi_packet_task()

[Albert Lee]

Remove unused atapi_packet_task().

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

atapi_packet_task() was replaced by ata_pio_task() in the irq-pio branch.

--- 00_cdblen/drivers/scsi/libata-core.c	2006-03-13 12:25:40.000000000 +0800
+++ 00.5_removeatapi/drivers/scsi/libata-core.c	2006-03-13 11:38:06.000000000 +0800
@@ -3815,79 +3815,6 @@ fsm_start:
 }
 
 /**
- *	atapi_packet_task - Write CDB bytes to hardware
- *	@_data: Port to which ATAPI device is attached.
- *
- *	When device has indicated its readiness to accept
- *	a CDB, this function is called.  Send the CDB.
- *	If DMA is to be performed, exit immediately.
- *	Otherwise, we are in polling mode, so poll
- *	status under operation succeeds or fails.
- *
- *	LOCKING:
- *	Kernel thread context (may sleep)
- */
-
-static void atapi_packet_task(void *_data)
-{
-	struct ata_port *ap = _data;
-	struct ata_queued_cmd *qc;
-	u8 status;
-
-	qc = ata_qc_from_tag(ap, ap->active_tag);
-	WARN_ON(qc == NULL);
-	WARN_ON(!(qc->flags & ATA_QCFLAG_ACTIVE));
-
-	/* sleep-wait for BSY to clear */
-	DPRINTK("busy wait\n");
-	if (ata_busy_sleep(ap, ATA_TMOUT_CDB_QUICK, ATA_TMOUT_CDB)) {
-		qc->err_mask |= AC_ERR_TIMEOUT;
-		goto err_out;
-	}
-
-	/* make sure DRQ is set */
-	status = ata_chk_status(ap);
-	if ((status & (ATA_BUSY | ATA_DRQ)) != ATA_DRQ) {
-		qc->err_mask |= AC_ERR_HSM;
-		goto err_out;
-	}
-
-	/* send SCSI cdb */
-	DPRINTK("send cdb\n");
-	WARN_ON(qc->dev->cdb_len < 12);
-
-	if (qc->tf.protocol == ATA_PROT_ATAPI_DMA ||
-	    qc->tf.protocol == ATA_PROT_ATAPI_NODATA) {
-		unsigned long flags;
-
-		/* Once we're done issuing command and kicking bmdma,
-		 * irq handler takes over.  To not lose irq, we need
-		 * to clear NOINTR flag before sending cdb, but
-		 * interrupt handler shouldn't be invoked before we're
-		 * finished.  Hence, the following locking.
-		 */
-		spin_lock_irqsave(&ap->host_set->lock, flags);
-#warning FIXME
-		/* ap->flags &= ~ATA_FLAG_NOINTR; */
-		ata_data_xfer(ap, qc->cdb, qc->dev->cdb_len, 1);
-		if (qc->tf.protocol == ATA_PROT_ATAPI_DMA)
-			ap->ops->bmdma_start(qc);	/* initiate bmdma */
-		spin_unlock_irqrestore(&ap->host_set->lock, flags);
-	} else {
-		ata_data_xfer(ap, qc->cdb, qc->dev->cdb_len, 1);
-
-		/* PIO commands are handled by polling */
-		ap->hsm_task_state = HSM_ST;
-		ata_port_queue_task(ap, ata_pio_task, ap, 0);
-	}
-
-	return;
-
-err_out:
-	ata_poll_qc_complete(qc);
-}
-
-/**
  *	ata_qc_timeout - Handle timeout of queued command
  *	@qc: Command that timed out
  *