Skip to contentSkip to conversation

libata: more irq driven pio follow-up patches

PATCH In review Albert Lee 2 participants 6 messages Nov 1, 2005 – Nov 9, 2005

Subsystems (2)
lore
Share & tools
lore

Albert Lee

author
#0 lore raw

cover libata: more irq driven pio follow-up patches

Dear all,

More irq driven PIO follow-up patches (revised).

Changes:
1. merge the ata_dataout_task workqueue with the ata_pio_task workqueue
2. add read/write multiple support

patch 1/4: misc fixes
- ata_pio_block(): add ata_altstatus(ap) to prevent reading device status before it is valid
- remove the unnecessary HSM_ST_IDLE state from ata_pio_task()
- raise BUG() when unknown state is found in ata_pio_task()

patch 2/4: merge the ata_dataout_task workqueue with the ata_pio_task workqueue
- remove ap->dataout_task from struct ata_port
- let ata_pio_task() handle the HSM_ST_FIRST state.
- rename ata_dataout_task() to ata_pio_first_block()

patch 3/4: eliminate unnecessary queuing in ata_pio_first_block()
- change the return value of ata_pio_complete()
- add return value to ata_pio_first_block()
- rename the variable "qc_completed" to "has_next" in ata_pio_task()
- use "has_next" to eliminate unnecessary queuing in ata_pio_first_block()

patch 4/4: add read/write multiple support
- add is_multi_taskfile() to ata.h
- initialize ata_device->multi_count with device identify data
- use ata_pio_sectors() to support r/w multiple commands

Patch against the libata-dev irq-pio branch (cd8200e6d4f9f05e6ea48f7c000be890337396ac).

For your review and advice, thanks.

Albert

author
#1 lore raw

PATCH 1/4 irq-pio: misc fixes

Patch 1/4: misc fixes

Changes:
- ata_pio_block(): add ata_altstatus(ap) to prevent reading device status before it is valid
- remove the unnecessary HSM_ST_IDLE state from ata_pio_task()
- raise BUG() when unknown state is found in ata_pio_task()

For your review, thanks.

Albert

============

drivers/scsi/libata-core.c +6 -3
@@ -3224,6 +3224,8 @@ static void ata_pio_block(struct ata_por
 
 		ata_pio_sector(qc);
 	}
+
+	ata_altstatus(ap); /* flush */
 }
 
 static void ata_pio_error(struct ata_port *ap)
@@ -3251,9 +3253,6 @@ fsm_start:
 	qc_completed = 0;
 
 	switch (ap->hsm_task_state) {
-	case HSM_ST_IDLE:
-		return;
-
 	case HSM_ST:
 		ata_pio_block(ap);
 		break;
@@ -3271,6 +3270,10 @@ fsm_start:
 	case HSM_ST_ERR:
 		ata_pio_error(ap);
 		return;
+
+	default:
+		BUG();
+		return;
 	}
 
 	if (timeout)
author
#3 lore raw

PATCH 2/4 irq-pio: merge the ata_dataout_task workqueue with ata_pio_task workqueue

In reply to Albert Lee

Patch 2/4:
merge the ata_dataout_task workqueue with ata_pio_task workqueue

Changes:
- remove ap->dataout_task from struct ata_port
- let ata_pio_task() handle the HSM_ST_FIRST state.
- rename ata_dataout_task() to ata_pio_first_block()
- replace the ata_dataout_task workqueue with ata_pio_task workqueue

For your review, thanks.

Albert

========

include/linux/libata.h +0 -2
@@ -334,8 +334,6 @@ struct ata_port {
 	struct ata_host_stats	stats;
 	struct ata_host_set	*host_set;
 
-	struct work_struct	dataout_task;
-
 	struct work_struct	pio_task;
 	unsigned int		hsm_task_state;
 	unsigned long		pio_task_timeout;
drivers/scsi/libata-core.c +11 -9
@@ -2963,8 +2963,8 @@ static void atapi_send_cdb(struct ata_po
 }
 
 /**
- *	ata_dataout_task - Write first data block to hardware
- *	@_data: Port to which ATA/ATAPI device is attached.
+ *	ata_pio_first_block - Write first data block to hardware
+ *	@ap: Port to which ATA/ATAPI device is attached.
  *
  *	When device has indicated its readiness to accept
  *	the data, this function sends out the CDB or 
@@ -2977,9 +2977,8 @@ static void atapi_send_cdb(struct ata_po
  *	Kernel thread context (may sleep)
  */
 
-static void ata_dataout_task(void *_data)
+static void ata_pio_first_block(struct ata_port *ap)
 {
-	struct ata_port *ap = _data;
 	struct ata_queued_cmd *qc;
 	u8 status;
 	unsigned long flags;
@@ -3253,6 +3252,10 @@ fsm_start:
 	qc_completed = 0;
 
 	switch (ap->hsm_task_state) {
+	case HSM_ST_FIRST:
+		ata_pio_first_block(ap);
+		return;
+
 	case HSM_ST:
 		ata_pio_block(ap);
 		break;
@@ -3703,10 +3706,10 @@ int ata_qc_issue_prot(struct ata_queued_
 		if (qc->tf.flags & ATA_TFLAG_WRITE) {
 			/* PIO data out protocol */
 			ap->hsm_task_state = HSM_ST_FIRST;
-			queue_work(ata_wq, &ap->dataout_task);
+			queue_work(ata_wq, &ap->pio_task);
 
 			/* always send first data block using
-			 * the ata_dataout_task() codepath.
+			 * the ata_pio_task() codepath.
 			 */
 		} else {
 			/* PIO data in protocol */
@@ -3733,7 +3736,7 @@ int ata_qc_issue_prot(struct ata_queued_
 		/* send cdb by polling if no cdb interrupt */
 		if ((!(qc->dev->flags & ATA_DFLAG_CDB_INTR)) ||
 		    (qc->tf.flags & ATA_TFLAG_POLLING))
-			queue_work(ata_wq, &ap->dataout_task);
+			queue_work(ata_wq, &ap->pio_task);
 		break;
 
 	case ATA_PROT_ATAPI_DMA:
@@ -3745,7 +3748,7 @@ int ata_qc_issue_prot(struct ata_queued_
 
 		/* send cdb by polling if no cdb interrupt */
 		if (!(qc->dev->flags & ATA_DFLAG_CDB_INTR))
-			queue_work(ata_wq, &ap->dataout_task);
+			queue_work(ata_wq, &ap->pio_task);
 		break;
 
 	default:
@@ -4325,7 +4328,6 @@ static void ata_host_init(struct ata_por
 	ap->active_tag = ATA_TAG_POISON;
 	ap->last_ctl = 0xFF;
 
-	INIT_WORK(&ap->dataout_task, ata_dataout_task, ap);
 	INIT_WORK(&ap->pio_task, ata_pio_task, ap);
 
 	for (i = 0; i < ATA_MAX_DEVICES; i++)
author
#4 lore raw

PATCH 3/4 irq-pio: eliminate unnecessary queuing in ata_pio_first_block()

In reply to Albert Lee

patch 3/4:
eliminate unnecessary queuing in ata_pio_first_block()

Changes:
- change the return value of ata_pio_complete() 0 <-> 1
- add return value for ata_pio_first_block()
- rename variable "qc_completed" to "has_next" in ata_pio_task()
- use has_next to eliminate unnecessary queuing in ata_pio_first_block()

For your review, thanks.

Albert

==========

drivers/scsi/libata-core.c +33 -20
@@ -2697,7 +2697,8 @@ static unsigned long ata_pio_poll(struct
  *	None.  (executing in kernel thread context)
  *
  *	RETURNS:
- *	Non-zero if qc completed, zero otherwise.
+ *	Zero if qc completed.
+ *	Non-zero if has next.
  */
 
 static int ata_pio_complete (struct ata_port *ap)
@@ -2719,14 +2720,14 @@ static int ata_pio_complete (struct ata_
 		if (drv_stat & (ATA_BUSY | ATA_DRQ)) {
 			ap->hsm_task_state = HSM_ST_LAST_POLL;
 			ap->pio_task_timeout = jiffies + ATA_TMOUT_PIO;
-			return 0;
+			return 1;
 		}
 	}
 
 	drv_stat = ata_wait_idle(ap);
 	if (!ata_ok(drv_stat)) {
 		ap->hsm_task_state = HSM_ST_ERR;
-		return 0;
+		return 1;
 	}
 
 	qc = ata_qc_from_tag(ap, ap->active_tag);
@@ -2738,7 +2739,7 @@ static int ata_pio_complete (struct ata_
 
 	/* another command may start at this point */
 
-	return 1;
+	return 0;
 }
 
 
@@ -2975,27 +2976,42 @@ static void atapi_send_cdb(struct ata_po
  *
  *	LOCKING:
  *	Kernel thread context (may sleep)
+ *
+ *	RETURNS:
+ *	Zero if irq handler takes over
+ *	Non-zero if has next (polling).
  */
 
-static void ata_pio_first_block(struct ata_port *ap)
+static int ata_pio_first_block(struct ata_port *ap)
 {
 	struct ata_queued_cmd *qc;
 	u8 status;
 	unsigned long flags;
+	int has_next;
 
 	qc = ata_qc_from_tag(ap, ap->active_tag);
 	assert(qc != NULL);
 	assert(qc->flags & ATA_QCFLAG_ACTIVE);
 
+	/* if polling, we will stay in the work queue after sending the data.
+	 * otherwise, interrupt handler takes over after sending the data.
+	 */
+	has_next = (qc->tf.flags & ATA_TFLAG_POLLING);
+
 	/* sleep-wait for BSY to clear */
 	DPRINTK("busy wait\n");
-	if (ata_busy_sleep(ap, ATA_TMOUT_DATAOUT_QUICK, ATA_TMOUT_DATAOUT))
+	if (ata_busy_sleep(ap, ATA_TMOUT_DATAOUT_QUICK, ATA_TMOUT_DATAOUT)) {
+		ap->hsm_task_state = HSM_ST_TMOUT;
 		goto err_out;
+	}
 
 	/* make sure DRQ is set */
 	status = ata_chk_status(ap);
-	if ((status & (ATA_BUSY | ATA_DRQ)) != ATA_DRQ)
+	if ((status & (ATA_BUSY | ATA_DRQ)) != ATA_DRQ) {
+		/* device status error */
+		ap->hsm_task_state = HSM_ST_ERR;
 		goto err_out;
+	}
 
 	/* Send the CDB (atapi) or the first data block (ata pio out).
 	 * During the state transition, interrupt handler shouldn't
@@ -3019,18 +3035,15 @@ static void ata_pio_first_block(struct a
 		/* send CDB */
 		atapi_send_cdb(ap, qc);
 
+	spin_unlock_irqrestore(&ap->host_set->lock, flags);
+
 	/* if polling, ata_pio_task() handles the rest.
 	 * otherwise, interrupt handler takes over from here.
 	 */
-	if (qc->tf.flags & ATA_TFLAG_POLLING)
-		queue_work(ata_wq, &ap->pio_task);
-
-	spin_unlock_irqrestore(&ap->host_set->lock, flags);
-
-	return;
+	return has_next;
 
 err_out:
-	ata_pio_error(ap);
+	return 1; /* has next */
 }
 
 /**
@@ -3245,23 +3258,23 @@ static void ata_pio_task(void *_data)
 {
 	struct ata_port *ap = _data;
 	unsigned long timeout;
-	int qc_completed;
+	int has_next;
 
 fsm_start:
 	timeout = 0;
-	qc_completed = 0;
+	has_next = 1;
 
 	switch (ap->hsm_task_state) {
 	case HSM_ST_FIRST:
-		ata_pio_first_block(ap);
-		return;
+		has_next = ata_pio_first_block(ap);
+		break;
 
 	case HSM_ST:
 		ata_pio_block(ap);
 		break;
 
 	case HSM_ST_LAST:
-		qc_completed = ata_pio_complete(ap);
+		has_next = ata_pio_complete(ap);
 		break;
 
 	case HSM_ST_POLL:
@@ -3281,7 +3294,7 @@ fsm_start:
 
 	if (timeout)
 		queue_delayed_work(ata_wq, &ap->pio_task, timeout);
-	else if (!qc_completed)
+	else if (has_next)
 		goto fsm_start;
 }
 
author
#5 lore raw

PATCH 4/4 irq-pio: add read/write multiple support

In reply to Albert Lee

patch 4/4:
add read/write multiple support

Changes:
- add is_multi_taskfile() to ata.h
- initialize ata_device->multi_count with device identify data
- use ata_pio_sectors() to support r/w multiple commands

For your review, thanks.

Albert

========

include/linux/ata.h +8 -0
@@ -293,6 +293,14 @@ static inline int is_atapi_taskfile(cons
 	       (tf->protocol == ATA_PROT_ATAPI_DMA);
 }
 
+static inline int is_multi_taskfile(struct ata_taskfile *tf)
+{
+	return (tf->command == ATA_CMD_READ_MULTI) ||
+	       (tf->command == ATA_CMD_WRITE_MULTI) ||
+	       (tf->command == ATA_CMD_READ_MULTI_EXT) ||
+	       (tf->command == ATA_CMD_WRITE_MULTI_EXT);
+}
+
 static inline int ata_ok(u8 status)
 {
 	return ((status & (ATA_BUSY | ATA_DRDY | ATA_DF | ATA_DRQ | ATA_ERR))
drivers/scsi/libata-core.c +38 -6

82 lines not on this page · raw message