Re: [PATCH] 2.5.67-ac1 IDE - fix Taskfile IOCTLs

Re: [PATCH] 2.5.67-ac1 IDE - fix Taskfile IOCTLs

[Alan Cox]

On Gwe, 2003-04-18 at 00:16, Bartlomiej Zolnierkiewicz wrote:
> Hey,
> 
> This time 5 incremental patches:
> 
> 1       - Fix PIO handlers for Taskfile ioctls.
> 2a + 2b - Taskfile and flagged Taskfile PIO handlers unification.
> 3       - Map HDIO_DRIVE_CMD ioctl onto taskfile.
> 4       - Remove dead ide_diag_taskfile() code.
> 
> [ More comments inside patches. ]

I'll take a look at them for ac3. Can I roll in 1/2a-b and 4 and skip
the experimental one for ac3 ?

[PATCH] 2.5.67-ac1 IDE - fix Taskfile IOCTLs

[Bartlomiej Zolnierkiewicz]

Hey,

This time 5 incremental patches:

1       - Fix PIO handlers for Taskfile ioctls.
2a + 2b - Taskfile and flagged Taskfile PIO handlers unification.
3       - Map HDIO_DRIVE_CMD ioctl onto taskfile.
4       - Remove dead ide_diag_taskfile() code.

[ More comments inside patches. ]

Special care is needed for patch 3 as it is a bit experimental,
but at least hdparm -I /dev/hdx still works :-).
I have also made version using direct IO to user pages,
it works okay too but needs some more work to be elegant...

You can also get them at:
http://home.elka.pw.edu.pl/~bzolnier/patches/2.5.67-ac1/

--
bzolnier

Re: [PATCH] 2.5.67-ac1 IDE - fix Taskfile IOCTLs

[Bartlomiej Zolnierkiewicz]

tf-ioctls-1.diff:

# Fix PIO handlers for Taskfile ioctls.
#
# Detailed changelog:
# - fix process_that_request_first() for rq->buffer (non-bio) based requests
# - set rq->hard_nr_sectors and rq->hard_cur_sectors in ide_diag_taskfile()
# - use ide_rq_offset() in ide_map_buffer() and remove task_rq_offset()
# - fix PIO handlers for rq->buffer based requests
#
# Bartlomiej Zolnierkiewicz <bzolnier@elka.pw.edu.pl>

diff -uNr linux-2.5.67-ac1/drivers/block/ll_rw_blk.c linux/drivers/block/ll_rw_blk.c
--- linux-2.5.67-ac1/drivers/block/ll_rw_blk.c	Thu Apr 17 17:46:53 2003
+++ linux/drivers/block/ll_rw_blk.c	Thu Apr 17 17:47:33 2003
@@ -2265,9 +2265,11 @@
 		nsect = min_t(unsigned int, req->current_nr_sectors,
 			      nr_sectors);
 		req->current_nr_sectors -= nsect;
-		req->nr_bio_sectors -= nsect;
 		nr_sectors -= nsect;
-		blk_rq_next_segment(req);
+		if (req->bio) {
+			req->nr_bio_sectors -= nsect;
+			blk_rq_next_segment(req);
+		}
 	}
 	return 1;
 }
diff -uNr linux-2.5.67-ac1/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
--- linux-2.5.67-ac1/drivers/ide/ide-taskfile.c	Thu Apr 17 20:03:36 2003
+++ linux/drivers/ide/ide-taskfile.c	Thu Apr 17 20:10:59 2003
@@ -406,6 +406,11 @@
 	while (rq->hard_bio != rq->bio)
 		if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
 			return ide_stopped;
+	/* Complete rq->buffer based request (ioctls). */
+	if (!rq->bio && !rq->nr_sectors) {
+		ide_end_drive_cmd(drive, stat, HWIF(drive)->INB(IDE_ERROR_REG));
+		return ide_stopped;
+	}

 	rq->errors = 0;
 	task_sectors(drive, rq, 1, PIO_IN);
@@ -452,6 +457,11 @@
 	while (rq->hard_bio != rq->bio)
 		if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
 			return ide_stopped;
+	/* Complete rq->buffer based request (ioctls). */
+	if (!rq->bio && !rq->nr_sectors) {
+		ide_end_drive_cmd(drive, stat, HWIF(drive)->INB(IDE_ERROR_REG));
+		return ide_stopped;
+	}

 	rq->errors = 0;
 	do {
@@ -510,6 +520,11 @@
 	while (rq->hard_bio != rq->bio)
 		if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
 			return ide_stopped;
+	/* Complete rq->buffer based request (ioctls). */
+	if (!rq->bio && !rq->nr_sectors) {
+		ide_end_drive_cmd(drive, stat, HWIF(drive)->INB(IDE_ERROR_REG));
+		return ide_stopped;
+	}

 	/* Still data left to transfer. */
 	ide_set_handler(drive, &task_out_intr, WAIT_WORSTCASE, NULL);
@@ -569,6 +584,11 @@
 	while (rq->hard_bio != rq->bio)
 		if (!DRIVER(drive)->end_request(drive, 1, bio_sectors(rq->hard_bio)))
 			return ide_stopped;
+	/* Complete rq->buffer based request (ioctls). */
+	if (!rq->bio && !rq->nr_sectors) {
+		ide_end_drive_cmd(drive, stat, HWIF(drive)->INB(IDE_ERROR_REG));
+		return ide_stopped;
+	}

 	/* Still data left to transfer. */
 	ide_set_handler(drive, &task_mulout_intr, WAIT_WORSTCASE, NULL);
@@ -953,10 +973,11 @@
 	if (args->command_type != IDE_DRIVE_TASK_NO_DATA) {
 		if (data_size == 0)
 			rq.current_nr_sectors = rq.nr_sectors = (args->hobRegister[IDE_NSECTOR_OFFSET_HOB] << 8) | args->tfRegister[IDE_NSECTOR_OFFSET];
-		/*	rq.hard_cur_sectors	*/
 		else
 			rq.current_nr_sectors = rq.nr_sectors = data_size / SECTOR_SIZE;
-		/*	rq.hard_cur_sectors	*/
+
+		rq.hard_nr_sectors = rq.nr_sectors;
+		rq.hard_cur_sectors = rq.current_nr_sectors;
 	}

 	if (args->tf_out_flags.all == 0) {
diff -uNr linux-2.5.67-ac1/include/linux/ide.h linux/include/linux/ide.h
--- linux-2.5.67-ac1/include/linux/ide.h	Thu Apr 17 17:46:53 2003
+++ linux/include/linux/ide.h	Thu Apr 17 18:01:18 2003
@@ -840,12 +840,6 @@
 #define ide_rq_offset(rq) \
 	(((rq)->hard_cur_sectors - (rq)->current_nr_sectors) << 9)

-/*
- * taskfiles really should use hard_cur_sectors as well!
- */
-#define task_rq_offset(rq) \
-	(((rq)->nr_sectors - (rq)->current_nr_sectors) * SECTOR_SIZE)
-
 static inline void *ide_map_buffer(struct request *rq, unsigned long *flags)
 {
 	/*
@@ -857,7 +851,7 @@
 	/*
 	 * task request
 	 */
-	return rq->buffer + task_rq_offset(rq);
+	return rq->buffer + ide_rq_offset(rq);
 }

 static inline void ide_unmap_buffer(struct request *rq, char *buffer, unsigned long *flags)

Re: [PATCH] 2.5.67-ac1 IDE - fix Taskfile IOCTLs

[Bartlomiej Zolnierkiewicz]

tf-ioctls-2.diff:

# Taskfile and flagged Taskfile PIO handlers unification [1/2].
# Incremental to tf-ioctls-1 patch.
#
# Detailed changelog:
# - add request validity checking present in flagged PIO handlers
#   to execute_drive_cmd(), so we now check also not flagged Taskfile
#   and we abort bad requests early
# - add flagged_wait_drive_ready() for wating for non-busy drive status
# - add check for DATA_READY to task_out_intr() and task_mulout_intr()
#
# Bartlomiej Zolnierkiewicz <bzolnier@elka.pw.edu.pl>

diff -uNr linux-2.5.67-ac1-tf1/drivers/ide/ide-io.c linux/drivers/ide/ide-io.c
--- linux-2.5.67-ac1-tf1/drivers/ide/ide-io.c	Thu Apr 17 20:03:36 2003
+++ linux/drivers/ide/ide-io.c	Thu Apr 17 21:49:49 2003
@@ -496,7 +496,23 @@

 		if (!args)
 			goto done;
-
+
+		/* Some validity checking. */
+		if (!rq->hard_nr_sectors &&
+		    (args->command_type != IDE_DRIVE_TASK_NO_DATA &&
+		     args->command_type != IDE_DRIVE_TASK_SET_XFER)) {
+			printk(KERN_ERR "%s: %s: no data to transfer\n",
+					drive->name, __FUNCTION__);
+			goto abort;
+		}
+		if (!drive->mult_count &&
+		    (args->data_phase == TASKFILE_MULTI_IN ||
+		     args->data_phase == TASKFILE_MULTI_OUT)) {
+			printk(KERN_ERR "%s: %s: multimode not set\n",
+					drive->name, __FUNCTION__);
+			goto abort;
+		}
+
 		if (args->tf_out_flags.all != 0)
 			return flagged_taskfile(drive, args);
 		return do_rw_taskfile(drive, args);
@@ -559,6 +575,7 @@
 #ifdef DEBUG
  	printk("%s: DRIVE_CMD (null)\n", drive->name);
 #endif
+abort:
  	ide_end_drive_cmd(drive,
 			hwif->INB(IDE_STATUS_REG),
 			hwif->INB(IDE_ERROR_REG));
diff -uNr linux-2.5.67-ac1-tf1/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
--- linux-2.5.67-ac1-tf1/drivers/ide/ide-taskfile.c	Thu Apr 17 20:10:59 2003
+++ linux/drivers/ide/ide-taskfile.c	Thu Apr 17 21:17:43 2003
@@ -355,6 +355,20 @@

 EXPORT_SYMBOL(task_no_data_intr);

+static u8 flagged_wait_drive_ready(ide_drive_t *drive)
+{
+	ide_hwif_t *hwif = HWIF(drive);
+	int retries = 5;
+	u8 stat;
+	/*
+	 * (ks) Last sector was transfered, wait until drive is ready.
+	 * This can take up to 10 usec. We willl wait max 50 us.
+	 */
+	while (((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) && retries--)
+		udelay(10);
+	return stat;
+}
+
 #define PIO_IN	0
 #define PIO_OUT	1

@@ -386,11 +400,12 @@
 ide_startstop_t task_in_intr (ide_drive_t *drive)
 {
 	struct request *rq = HWGROUP(drive)->rq;
+	ide_task_t *task = (ide_task_t *) rq->special;
 	u8 stat, good_stat;

 	good_stat = DATA_READY;
-check_status:
 	stat = HWIF(drive)->INB(IDE_STATUS_REG);
+check_status:
 	if (!OK_STAT(stat, good_stat, BAD_R_STAT)) {
 		if (stat & (ERR_STAT|DRQ_STAT))
 			return DRIVER(drive)->error(drive, __FUNCTION__, stat);
@@ -417,6 +432,11 @@

 	/* If it was the last datablock check status and finish transfer. */
 	if (!rq->nr_sectors) {
+		/* FIXME: Use for all requests?  --bzolnier */
+		if (!blk_fs_request(rq) && task->tf_out_flags.all != 0)
+			stat = flagged_wait_drive_ready(drive);
+		else
+			stat = HWIF(drive)->INB(IDE_STATUS_REG);
 		good_stat = 0;
 		goto check_status;
 	}
@@ -435,13 +455,14 @@
 ide_startstop_t task_mulin_intr (ide_drive_t *drive)
 {
 	struct request *rq = HWGROUP(drive)->rq;
+	ide_task_t *task = (ide_task_t *) rq->special;
 	unsigned int msect = drive->mult_count;
 	unsigned int nsect;
 	u8 stat, good_stat;

 	good_stat = DATA_READY;
-check_status:
 	stat = HWIF(drive)->INB(IDE_STATUS_REG);
+check_status:
 	if (!OK_STAT(stat, good_stat, BAD_R_STAT)) {
 		if (stat & (ERR_STAT|DRQ_STAT))
 			return DRIVER(drive)->error(drive, __FUNCTION__, stat);
@@ -479,6 +500,11 @@

 	/* If it was the last datablock check status and finish transfer. */
 	if (!rq->nr_sectors) {
+		/* FIXME: Use for all requests?  --bzolnier */
+		if (!blk_fs_request(rq) && task->tf_out_flags.all != 0)
+			stat = flagged_wait_drive_ready(drive);
+		else
+			stat = HWIF(drive)->INB(IDE_STATUS_REG);
 		good_stat = 0;
 		goto check_status;
 	}
@@ -513,6 +539,11 @@
 		}
 	}

+	/* Deal with unexpected ATA data phase. */
+	ok_stat = OK_STAT(stat, DATA_READY, drive->bad_wstat);
+	if (!ok_stat && rq->nr_sectors)
+		return DRIVER(drive)->error(drive, __FUNCTION__, stat);
+
 	/*
 	 * Complete previously submitted bios (if any).
 	 * Status was already verifyied.
@@ -577,6 +608,11 @@
 		}
 	}

+	/* Deal with unexpected ATA data phase. */
+	ok_stat = OK_STAT(stat, DATA_READY, drive->bad_wstat);
+	if (!ok_stat && rq->nr_sectors)
+		return DRIVER(drive)->error(drive, __FUNCTION__, stat);
+
 	/*
 	 * Complete previously submitted bios (if any).
 	 * Status was already verifyied.
@@ -1619,7 +1655,6 @@
 	u8 stat			= hwif->INB(IDE_STATUS_REG);
 	struct request *rq	= HWGROUP(drive)->rq;
 	char *pBuf		= NULL;
-	int retries             = 5;

 	if (rq->current_nr_sectors == 0)
 		return DRIVER(drive)->error(drive, "flagged_task_in_intr (no data requested)", stat);
@@ -1650,12 +1685,7 @@
 		ide_set_handler(drive, &flagged_task_in_intr,  WAIT_WORSTCASE, NULL);
 		return ide_started;
 	}
-	/*
-	 * (ks) Last sector was transfered, wait until drive is ready.
-	 * This can take up to 10 usec. We willl wait max 50 us.
-	 */
-	while (((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) && retries--)
-		udelay(10);
+	stat = flagged_wait_drive_ready(drive);
 	ide_end_drive_cmd (drive, stat, hwif->INB(IDE_ERROR_REG));

 	return ide_stopped;
@@ -1667,7 +1697,6 @@
 	u8 stat			= hwif->INB(IDE_STATUS_REG);
 	struct request *rq	= HWGROUP(drive)->rq;
 	char *pBuf		= NULL;
-	int retries             = 5;
 	unsigned int msect, nsect;

 	if (rq->current_nr_sectors == 0)
@@ -1707,13 +1736,7 @@
 		ide_set_handler(drive, &flagged_task_mulin_intr,  WAIT_WORSTCASE, NULL);
 		return ide_started;
 	}
-
-	/*
-	 * (ks) Last sector was transfered, wait until drive is ready.
-	 * This can take up to 10 usec. We willl wait max 50 us.
-	 */
-	while (((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) && retries--)
-		udelay(10);
+	stat = flagged_wait_drive_ready(drive);
 	ide_end_drive_cmd (drive, stat, hwif->INB(IDE_ERROR_REG));

 	return ide_stopped;

Re: [PATCH] 2.5.67-ac1 IDE - fix Taskfile IOCTLs

[Bartlomiej Zolnierkiewicz]

tf-ioctls-2b.diff:

# Taskfile and flagged Taskfile PIO handlers unification [2/2].
# Incremental to tf-ioctls-2a patch.
#
# Detailed changelog:
# - remove all flagged Taskfile handlers, use standard ones instead
#
# Bartlomiej Zolnierkiewicz <bzolnier@elka.pw.edu.pl>

diff -uNr linux-2.5.67-ac1-tf2a/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
--- linux-2.5.67-ac1-tf2a/drivers/ide/ide-taskfile.c	Thu Apr 17 21:17:43 2003
+++ linux/drivers/ide/ide-taskfile.c	Thu Apr 17 22:32:32 2003
@@ -1116,15 +1116,6 @@

 #define MAX_DMA		(256*SECTOR_WORDS)

-ide_startstop_t flagged_taskfile(ide_drive_t *, ide_task_t *);
-ide_startstop_t flagged_task_no_data_intr(ide_drive_t *);
-ide_startstop_t flagged_task_in_intr(ide_drive_t *);
-ide_startstop_t flagged_task_mulin_intr(ide_drive_t *);
-ide_startstop_t flagged_pre_task_out_intr(ide_drive_t *, struct request *);
-ide_startstop_t flagged_task_out_intr(ide_drive_t *);
-ide_startstop_t flagged_pre_task_mulout_intr(ide_drive_t *, struct request *);
-ide_startstop_t flagged_task_mulout_intr(ide_drive_t *);
-
 int ide_taskfile_ioctl (ide_drive_t *drive, unsigned int cmd, unsigned long arg)
 {
 	ide_task_request_t	*req_task;
@@ -1230,23 +1221,13 @@
 				err = -EPERM;
 				goto abort;
 			}
-			if (args.tf_out_flags.all != 0) {
-				args.prehandler = &flagged_pre_task_mulout_intr;
-				args.handler = &flagged_task_mulout_intr;
-			} else {
-				args.prehandler = &pre_task_mulout_intr;
-				args.handler = &task_mulout_intr;
-			}
+			args.prehandler = &pre_task_mulout_intr;
+			args.handler = &task_mulout_intr;
 			err = ide_diag_taskfile(drive, &args, taskout, outbuf);
 			break;
 		case TASKFILE_OUT:
-			if (args.tf_out_flags.all != 0) {
-				args.prehandler = &flagged_pre_task_out_intr;
-				args.handler    = &flagged_task_out_intr;
-			} else {
-				args.prehandler = &pre_task_out_intr;
-				args.handler = &task_out_intr;
-			}
+			args.prehandler = &pre_task_out_intr;
+			args.handler = &task_out_intr;
 			err = ide_diag_taskfile(drive, &args, taskout, outbuf);
 			break;
 		case TASKFILE_MULTI_IN:
@@ -1258,27 +1239,15 @@
 				err = -EPERM;
 				goto abort;
 			}
-			if (args.tf_out_flags.all != 0) {
-				args.handler = &flagged_task_mulin_intr;
-			} else {
-				args.handler = &task_mulin_intr;
-			}
+			args.handler = &task_mulin_intr;
 			err = ide_diag_taskfile(drive, &args, taskin, inbuf);
 			break;
 		case TASKFILE_IN:
-			if (args.tf_out_flags.all != 0) {
-				args.handler = &flagged_task_in_intr;
-			} else {
-				args.handler = &task_in_intr;
-			}
+			args.handler = &task_in_intr;
 			err = ide_diag_taskfile(drive, &args, taskin, inbuf);
 			break;
 		case TASKFILE_NO_DATA:
-			if (args.tf_out_flags.all != 0) {
-				args.handler = &flagged_task_no_data_intr;
-			} else {
-				args.handler = &task_no_data_intr;
-			}
+			args.handler = &task_no_data_intr;
 			err = ide_diag_taskfile(drive, &args, 0, NULL);
 			break;
 		default:
@@ -1621,273 +1590,6 @@

 EXPORT_SYMBOL(flagged_taskfile);

-ide_startstop_t flagged_task_no_data_intr (ide_drive_t *drive)
-{
-	ide_hwif_t *hwif = HWIF(drive);
-	u8 stat;
-
-	local_irq_enable();
-
-	if (!OK_STAT(stat = hwif->INB(IDE_STATUS_REG), READY_STAT, BAD_STAT)) {
-		if (stat & ERR_STAT) {
-			return DRIVER(drive)->error(drive, "flagged_task_no_data_intr", stat);
-		}
-		/*
-		 * (ks) Unexpected ATA data phase detected.
-		 * This should not happen. But, it can !
-		 * I am not sure, which function is best to clean up
-		 * this situation.  I choose: ide_error(...)
-		 */
- 		return DRIVER(drive)->error(drive, "flagged_task_no_data_intr (unexpected phase)", stat);
-	}
-
-	ide_end_drive_cmd(drive, stat, hwif->INB(IDE_ERROR_REG));
-
-	return ide_stopped;
-}
-
-/*
- * Handler for command with PIO data-in phase
- */
-ide_startstop_t flagged_task_in_intr (ide_drive_t *drive)
-{
-	ide_hwif_t *hwif	= HWIF(drive);
-	u8 stat			= hwif->INB(IDE_STATUS_REG);
-	struct request *rq	= HWGROUP(drive)->rq;
-	char *pBuf		= NULL;
-
-	if (rq->current_nr_sectors == 0)
-		return DRIVER(drive)->error(drive, "flagged_task_in_intr (no data requested)", stat);
-
-	if (!OK_STAT(stat, DATA_READY, BAD_R_STAT)) {
-		if (stat & ERR_STAT) {
-			return DRIVER(drive)->error(drive, "flagged_task_in_intr", stat);
-		}
-		/*
-		 * (ks) Unexpected ATA data phase detected.
-		 * This should not happen. But, it can !
-		 * I am not sure, which function is best to clean up
-		 * this situation.  I choose: ide_error(...)
-		 */
-		return DRIVER(drive)->error(drive, "flagged_task_in_intr (unexpected data phase)", stat);
-	}
-
-	pBuf = rq->buffer + ((rq->nr_sectors - rq->current_nr_sectors) * SECTOR_SIZE);
-	DTF("Read - rq->current_nr_sectors: %d, status: %02x\n", (int) rq->current_nr_sectors, stat);
-
-	taskfile_input_data(drive, pBuf, SECTOR_WORDS);
-
-	if (--rq->current_nr_sectors != 0) {
-		/*
-                 * (ks) We don't know which command was executed.
-		 * So, we wait the 'WORSTCASE' value.
-                 */
-		ide_set_handler(drive, &flagged_task_in_intr,  WAIT_WORSTCASE, NULL);
-		return ide_started;
-	}
-	stat = flagged_wait_drive_ready(drive);
-	ide_end_drive_cmd (drive, stat, hwif->INB(IDE_ERROR_REG));
-
-	return ide_stopped;
-}
-
-ide_startstop_t flagged_task_mulin_intr (ide_drive_t *drive)
-{
-	ide_hwif_t *hwif	= HWIF(drive);
-	u8 stat			= hwif->INB(IDE_STATUS_REG);
-	struct request *rq	= HWGROUP(drive)->rq;
-	char *pBuf		= NULL;
-	unsigned int msect, nsect;
-
-	if (rq->current_nr_sectors == 0)
-		return DRIVER(drive)->error(drive, "flagged_task_mulin_intr (no data requested)", stat);
-
-	msect = drive->mult_count;
-	if (msect == 0)
-		return DRIVER(drive)->error(drive, "flagged_task_mulin_intr (multimode not set)", stat);
-
-	if (!OK_STAT(stat, DATA_READY, BAD_R_STAT)) {
-		if (stat & ERR_STAT) {
-			return DRIVER(drive)->error(drive, "flagged_task_mulin_intr", stat);
-		}
-		/*
-		 * (ks) Unexpected ATA data phase detected.
-		 * This should not happen. But, it can !
-		 * I am not sure, which function is best to clean up
-		 * this situation.  I choose: ide_error(...)
-		 */
-		return DRIVER(drive)->error(drive, "flagged_task_mulin_intr (unexpected data phase)", stat);
-	}
-
-	nsect = (rq->current_nr_sectors > msect) ? msect : rq->current_nr_sectors;
-	pBuf = rq->buffer + ((rq->nr_sectors - rq->current_nr_sectors) * SECTOR_SIZE);
-
-	DTF("Multiread: %p, nsect: %d , rq->current_nr_sectors: %ld\n",
-	    pBuf, nsect, rq->current_nr_sectors);
-
-	taskfile_input_data(drive, pBuf, nsect * SECTOR_WORDS);
-
-	rq->current_nr_sectors -= nsect;
-	if (rq->current_nr_sectors != 0) {
-		/*
-                 * (ks) We don't know which command was executed.
-		 * So, we wait the 'WORSTCASE' value.
-                 */
-		ide_set_handler(drive, &flagged_task_mulin_intr,  WAIT_WORSTCASE, NULL);
-		return ide_started;
-	}
-	stat = flagged_wait_drive_ready(drive);
-	ide_end_drive_cmd (drive, stat, hwif->INB(IDE_ERROR_REG));
-
-	return ide_stopped;
-}
-
-/*
- * Pre handler for command with PIO data-out phase
- */
-ide_startstop_t flagged_pre_task_out_intr (ide_drive_t *drive, struct request *rq)
-{
-	ide_hwif_t *hwif	= HWIF(drive);
-	u8 stat			= hwif->INB(IDE_STATUS_REG);
-	ide_startstop_t startstop;
-
-	if (!rq->current_nr_sectors) {
-		return DRIVER(drive)->error(drive, "flagged_pre_task_out_intr (write data not specified)", stat);
-	}
-
-	if (ide_wait_stat(&startstop, drive, DATA_READY,
-			BAD_W_STAT, WAIT_DRQ)) {
-		printk(KERN_ERR "%s: No DRQ bit after issuing write command.\n", drive->name);
-		return startstop;
-	}
-
-	taskfile_output_data(drive, rq->buffer, SECTOR_WORDS);
-	--rq->current_nr_sectors;
-
-	return ide_started;
-}
-
-ide_startstop_t flagged_task_out_intr (ide_drive_t *drive)
-{
-	ide_hwif_t *hwif	= HWIF(drive);
-	u8 stat			= hwif->INB(IDE_STATUS_REG);
-	struct request *rq	= HWGROUP(drive)->rq;
-	char *pBuf		= NULL;
-
-	if (!OK_STAT(stat, DRIVE_READY, BAD_W_STAT))
-		return DRIVER(drive)->error(drive, "flagged_task_out_intr", stat);
-
-	if (!rq->current_nr_sectors) {
-		ide_end_drive_cmd (drive, stat, hwif->INB(IDE_ERROR_REG));
-		return ide_stopped;
-	}
-
-	if (!OK_STAT(stat, DATA_READY, BAD_W_STAT)) {
-		/*
-		 * (ks) Unexpected ATA data phase detected.
-		 * This should not happen. But, it can !
-		 * I am not sure, which function is best to clean up
-		 * this situation.  I choose: ide_error(...)
-		 */
-		return DRIVER(drive)->error(drive, "flagged_task_out_intr (unexpected data phase)", stat);
-	}
-
-	pBuf = rq->buffer + ((rq->nr_sectors - rq->current_nr_sectors) * SECTOR_SIZE);
-	DTF("Write - rq->current_nr_sectors: %d, status: %02x\n",
-		(int) rq->current_nr_sectors, stat);
-
-	taskfile_output_data(drive, pBuf, SECTOR_WORDS);
-	--rq->current_nr_sectors;
-
-	/*
-	 * (ks) We don't know which command was executed.
-	 * So, we wait the 'WORSTCASE' value.
-	 */
-	ide_set_handler(drive, &flagged_task_out_intr, WAIT_WORSTCASE, NULL);
-
-	return ide_started;
-}
-
-ide_startstop_t flagged_pre_task_mulout_intr (ide_drive_t *drive, struct request *rq)
-{
-	ide_hwif_t *hwif	= HWIF(drive);
-	u8 stat			= hwif->INB(IDE_STATUS_REG);
-	char *pBuf		= NULL;
-	ide_startstop_t startstop;
-	unsigned int msect, nsect;
-
-	if (!rq->current_nr_sectors)
-		return DRIVER(drive)->error(drive, "flagged_pre_task_mulout_intr (write data not specified)", stat);
-
-	msect = drive->mult_count;
-	if (msect == 0)
-		return DRIVER(drive)->error(drive, "flagged_pre_task_mulout_intr (multimode not set)", stat);
-
-	if (ide_wait_stat(&startstop, drive, DATA_READY,
-			BAD_W_STAT, WAIT_DRQ)) {
-		printk(KERN_ERR "%s: No DRQ bit after issuing write command.\n", drive->name);
-		return startstop;
-	}
-
-	nsect = (rq->current_nr_sectors > msect) ? msect : rq->current_nr_sectors;
-	pBuf = rq->buffer + ((rq->nr_sectors - rq->current_nr_sectors) * SECTOR_SIZE);
-	DTF("Multiwrite: %p, nsect: %d , rq->current_nr_sectors: %ld\n",
-	    pBuf, nsect, rq->current_nr_sectors);
-
-	taskfile_output_data(drive, pBuf, nsect * SECTOR_WORDS);
-
-	rq->current_nr_sectors -= nsect;
-
-	return ide_started;
-}
-
-ide_startstop_t flagged_task_mulout_intr (ide_drive_t *drive)
-{
-	ide_hwif_t *hwif	= HWIF(drive);
-	u8 stat			= hwif->INB(IDE_STATUS_REG);
-	struct request *rq	= HWGROUP(drive)->rq;
-	char *pBuf		= NULL;
-	unsigned int msect, nsect;
-
-	msect = drive->mult_count;
-	if (msect == 0)
-		return DRIVER(drive)->error(drive, "flagged_task_mulout_intr (multimode not set)", stat);
-
-	if (!OK_STAT(stat, DRIVE_READY, BAD_W_STAT))
-		return DRIVER(drive)->error(drive, "flagged_task_mulout_intr", stat);
-
-	if (!rq->current_nr_sectors) {
-		ide_end_drive_cmd (drive, stat, hwif->INB(IDE_ERROR_REG));
-		return ide_stopped;
-	}
-
-	if (!OK_STAT(stat, DATA_READY, BAD_W_STAT)) {
-		/*
-		 * (ks) Unexpected ATA data phase detected.
-		 * This should not happen. But, it can !
-		 * I am not sure, which function is best to clean up
-		 * this situation.  I choose: ide_error(...)
-		 */
-		return DRIVER(drive)->error(drive, "flagged_task_mulout_intr (unexpected data phase)", stat);
-	}
-
-	nsect = (rq->current_nr_sectors > msect) ? msect : rq->current_nr_sectors;
-	pBuf = rq->buffer + ((rq->nr_sectors - rq->current_nr_sectors) * SECTOR_SIZE);
-	DTF("Multiwrite: %p, nsect: %d , rq->current_nr_sectors: %ld\n",
-	    pBuf, nsect, rq->current_nr_sectors);
-
-	taskfile_output_data(drive, pBuf, nsect * SECTOR_WORDS);
-	rq->current_nr_sectors -= nsect;
-
-	/*
-	 * (ks) We don't know which command was executed.
-	 * So, we wait the 'WORSTCASE' value.
-	 */
-	ide_set_handler(drive, &flagged_task_mulout_intr, WAIT_WORSTCASE, NULL);
-
-	return ide_started;
-}
-
 /*
  * Beginning of Taskfile OPCODE Library and feature sets.
  */

Re: [PATCH] 2.5.67-ac1 IDE - fix Taskfile IOCTLs

[Bartlomiej Zolnierkiewicz]

tf-ioctls-3.diff:

# Map HDIO_DRIVE_CMD ioctl onto taskfile.
# Incremental to tf-ioctls-2b patch.
#
# Detailed changelog:
# - fix taskfile version of ide_cmd_ioctl() and make it backward compatible
# - workaround issuing WIN_IDENTIFY for ATAPI devices for now
#
# To use taskfile ide_cmd_ioctl() you have to turn on CONFIG_IDE_TASKFILE_IO.
#
# Bartlomiej Zolnierkiewicz <bzolnier@elka.pw.edu.pl>

diff -uNr linux-2.5.67-ac1-tf2b/drivers/ide/Kconfig linux/drivers/ide/Kconfig
--- linux-2.5.67-ac1-tf2b/drivers/ide/Kconfig	Thu Apr 17 20:03:36 2003
+++ linux/drivers/ide/Kconfig	Thu Apr 17 22:41:38 2003
@@ -220,7 +220,7 @@
 	  If you are unsure, say N here.

 config IDE_TASKFILE_IO
-	bool
+	bool "IDE Taskfile IO"
 	depends on BLK_DEV_IDE

 comment "IDE chipset support/bugfixes"
diff -uNr linux-2.5.67-ac1-tf2b/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
--- linux-2.5.67-ac1-tf2b/drivers/ide/ide-taskfile.c	Thu Apr 17 22:32:32 2003
+++ linux/drivers/ide/ide-taskfile.c	Thu Apr 17 23:24:51 2003
@@ -1318,7 +1318,7 @@
  */
 int ide_cmd_ioctl (ide_drive_t *drive, unsigned int cmd, unsigned long arg)
 {
-#if 1
+#ifndef CONFIG_IDE_TASKFILE_IO
 	int err = 0;
 	u8 args[4], *argbuf = args;
 	u8 xfer_rate = 0;
@@ -1373,7 +1373,7 @@
 #else

 	int err = -EIO;
-	u8 args[4], *argbuf = args;
+	u8 args[4], *argbuf = NULL;
 	u8 xfer_rate = 0;
 	int argsize = 0;
 	ide_task_t tfargs;
@@ -1396,6 +1396,13 @@
 	tfargs.tfRegister[IDE_SELECT_OFFSET]  = 0x00;
 	tfargs.tfRegister[IDE_COMMAND_OFFSET] = args[0];

+	/*
+	 * FIXME: fix ->error() to do abort properly and hdparm to check
+	 *	  returned error value for abort not ioctl!  --bzolnier
+	 */
+	if (drive->media != ide_disk && args[0] == WIN_IDENTIFY)
+		tfargs.tfRegister[IDE_COMMAND_OFFSET] = WIN_PIDENTIFY;
+
 	if (args[3]) {
 		argsize = (SECTOR_WORDS * 4 * args[3]);
 		argbuf = kmalloc(argsize, GFP_KERNEL);
@@ -1414,19 +1421,20 @@

 	if (!err && xfer_rate) {
 		/* active-retuning-calls future */
-		ide_set_xfer_rate(driver, xfer_rate);
+		ide_set_xfer_rate(drive, xfer_rate);
 		ide_driveid_update(drive);
 	}
-abort:
-	args[0] = tfargs.tfRegister[IDE_COMMAND_OFFSET];
-	args[1] = tfargs.tfRegister[IDE_FEATURE_OFFSET];
+
+	args[0] = tfargs.tfRegister[IDE_STATUS_OFFSET];
+	args[1] = tfargs.tfRegister[IDE_ERROR_OFFSET];
 	args[2] = tfargs.tfRegister[IDE_NSECTOR_OFFSET];
-	args[3] = 0;

-	if (copy_to_user((void *)arg, argbuf, 4))
+abort:
+	if (copy_to_user((void *)arg, &args, 4))
 		err = -EFAULT;
+
 	if (argbuf != NULL) {
-		if (copy_to_user((void *)arg, argbuf + 4, argsize))
+		if (copy_to_user((void *)(arg+4), argbuf, argsize))
 			err = -EFAULT;
 		kfree(argbuf);
 	}

Re: [PATCH] 2.5.67-ac1 IDE - fix Taskfile IOCTLs

[Bartlomiej Zolnierkiewicz]

tf-ioctls-4.diff:

# Remove dead ide_diag_taskfile() code.
# Incremental to tf-ioctls-3 patch.
#
# - rq->flags is already set to REQ_DRIVE_TASKFILE by ide_init_drive_taskfile()
# - remove dead variant of ide_diag_taskfile(), it was broken
#   and had ide_do_drive_cmd() unfolded which IMHO wasn't good idea
#
# Bartlomiej Zolnierkiewicz <bzolnier@elka.pw.edu.pl>

diff -uNr linux-2.5.67-ac1-tf3/drivers/ide/ide-taskfile.c linux/drivers/ide/ide-taskfile.c
--- linux-2.5.67-ac1-tf3/drivers/ide/ide-taskfile.c	Thu Apr 17 23:24:51 2003
+++ linux/drivers/ide/ide-taskfile.c	Thu Apr 17 23:49:03 2003
@@ -990,14 +990,11 @@

 EXPORT_SYMBOL(ide_init_drive_taskfile);

-#if 1
-
 int ide_diag_taskfile (ide_drive_t *drive, ide_task_t *args, unsigned long data_size, u8 *buf)
 {
 	struct request rq;

 	ide_init_drive_taskfile(&rq);
-	rq.flags = REQ_DRIVE_TASKFILE;
 	rq.buffer = buf;

 	/*
@@ -1009,6 +1006,12 @@
 	if (args->command_type != IDE_DRIVE_TASK_NO_DATA) {
 		if (data_size == 0)
 			rq.current_nr_sectors = rq.nr_sectors = (args->hobRegister[IDE_NSECTOR_OFFSET_HOB] << 8) | args->tfRegister[IDE_NSECTOR_OFFSET];
+#if 0
+			ata_nsector_t nsector;
+			nsector.b.low = args->hobRegister[IDE_NSECTOR_OFFSET_HOB];
+			nsector.b.high = args->tfRegister[IDE_NSECTOR_OFFSET];
+			rq.nr_sectors = nsector.all;
+#endif
 		else
 			rq.current_nr_sectors = rq.nr_sectors = data_size / SECTOR_SIZE;

@@ -1030,69 +1033,6 @@
 	return ide_do_drive_cmd(drive, &rq, ide_wait);
 }

-#else
-
-int ide_diag_taskfile (ide_drive_t *drive, ide_task_t *args, unsigned long data_size, u8 *buf)
-{
-	struct request *rq;
-	unsigned long flags;
-	ide_hwgroup_t *hwgroup = HWGROUP(drive);
-	struct list_head *queue_head = &drive->queue.queue_head;
-	DECLARE_COMPLETION(wait);
-
-	if (HWIF(drive)->chipset == ide_pdc4030 && buf != NULL)
-		return -ENOSYS; /* special drive cmds not supported */
-
-	memset(rq, 0, sizeof(*rq));
-	rq->flags = REQ_DRIVE_TASKFILE;
-	rq->buffer = buf;
-
-	/*
-	 * (ks) We transfer currently only whole sectors.
-	 * This is suffient for now.  But, it would be great,
-	 * if we would find a solution to transfer any size.
-	 * To support special commands like READ LONG.
-	 */
-	if (args->command_type != IDE_DRIVE_TASK_NO_DATA) {
-		if (data_size == 0) {
-			ata_nsector_t nsector;
-			nsector.b.low = args->hobRegister[IDE_NSECTOR_OFFSET_HOB];
-			nsector.b.high = args->tfRegister[IDE_NSECTOR_OFFSET];
-			rq.nr_sectors = nsector.all;
-		} else {
-			rq.nr_sectors = data_size / SECTOR_SIZE;
-		}
-		rq.current_nr_sectors = rq.nr_sectors;
-	//	rq.hard_cur_sectors = rq.nr_sectors;
-	}
-
-	if (args->tf_out_flags.all == 0) {
-		/*
-		 * clean up kernel settings for driver sanity, regardless.
-		 * except for discrete diag services.
-		 */
-		args->posthandler = ide_post_handler_parser(
-				(struct hd_drive_task_hdr *) args->tfRegister,
-				(struct hd_drive_hob_hdr *) args->hobRegister);
-	}
-	rq->special = args;
-	rq->errors = 0;
-	rq->rq_status = RQ_ACTIVE;
-	rq->rq_disk = drive->disk;
-	rq->waiting = &wait;
-
-	spin_lock_irqsave(&ide_lock, flags);
-	queue_head = queue_head->prev;
-	list_add(&rq->queue, queue_head);
-	ide_do_request(hwgroup, 0);
-	spin_unlock_irqrestore(&ide_lock, flags);
-
-	wait_for_completion(&wait);	/* wait for it to be serviced */
-	return rq->errors ? -EIO : 0;	/* return -EIO if errors */
-}
-
-#endif
-
 EXPORT_SYMBOL(ide_diag_taskfile);

 int ide_raw_taskfile (ide_drive_t *drive, ide_task_t *args, u8 *buf)

Re: [PATCH] 2.5.67-ac1 IDE - fix Taskfile IOCTLs

[Bartlomiej Zolnierkiewicz]

On 17 Apr 2003, Alan Cox wrote:

> On Gwe, 2003-04-18 at 00:16, Bartlomiej Zolnierkiewicz wrote:
> > Hey,
> >
> > This time 5 incremental patches:
> >
> > 1       - Fix PIO handlers for Taskfile ioctls.
> > 2a + 2b - Taskfile and flagged Taskfile PIO handlers unification.
> > 3       - Map HDIO_DRIVE_CMD ioctl onto taskfile.
> > 4       - Remove dead ide_diag_taskfile() code.
> >
> > [ More comments inside patches. ]
>
> I'll take a look at them for ac3. Can I roll in 1/2a-b and 4 and skip
> the experimental one for ac3 ?

Sure, you can also include 3 because by old function is used by default,
to use the new one CONFIG_IDE_TASKFILE_IO has to be turned on...

Re: [PATCH] 2.5.67-ac1 IDE - fix Taskfile IOCTLs

[Andre Hedrick]

ide_diag_taskfile

Do not do that it will break paths!

On Fri, 18 Apr 2003, Bartlomiej Zolnierkiewicz wrote:

> 
> Hey,
> 
> This time 5 incremental patches:
> 
> 1       - Fix PIO handlers for Taskfile ioctls.
> 2a + 2b - Taskfile and flagged Taskfile PIO handlers unification.
> 3       - Map HDIO_DRIVE_CMD ioctl onto taskfile.
> 4       - Remove dead ide_diag_taskfile() code.
> 
> [ More comments inside patches. ]
> 
> Special care is needed for patch 3 as it is a bit experimental,
> but at least hdparm -I /dev/hdx still works :-).
> I have also made version using direct IO to user pages,
> it works okay too but needs some more work to be elegant...
> 
> You can also get them at:
> http://home.elka.pw.edu.pl/~bzolnier/patches/2.5.67-ac1/
> 
> --
> bzolnier
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Andre Hedrick
LAD Storage Consulting Group

Re: [PATCH] 2.5.67-ac1 IDE - fix Taskfile IOCTLs

[Bartlomiej Zolnierkiewicz]

On Thu, 17 Apr 2003, Andre Hedrick wrote:

> ide_diag_taskfile

???

> Do not do that it will break paths!

What paths?

> On Fri, 18 Apr 2003, Bartlomiej Zolnierkiewicz wrote:
>
> >
> > Hey,
> >
> > This time 5 incremental patches:
> >
> > 1       - Fix PIO handlers for Taskfile ioctls.
> > 2a + 2b - Taskfile and flagged Taskfile PIO handlers unification.
> > 3       - Map HDIO_DRIVE_CMD ioctl onto taskfile.
> > 4       - Remove dead ide_diag_taskfile() code.
> >
> > [ More comments inside patches. ]
> >
> > Special care is needed for patch 3 as it is a bit experimental,
> > but at least hdparm -I /dev/hdx still works :-).
> > I have also made version using direct IO to user pages,
> > it works okay too but needs some more work to be elegant...
> >
> > You can also get them at:
> > http://home.elka.pw.edu.pl/~bzolnier/patches/2.5.67-ac1/
> >
> > --
> > bzolnier
> >
>
> Andre Hedrick
> LAD Storage Consulting Group