[PATCH 2.6.11-rc3 00/11] ide: ide driver updates series 2, round 2

[PATCH 2.6.11-rc3 00/11] ide: ide driver updates series 2, round 2

[Tejun Heo]

 Hello, Bartlomiej.

 This is the second round of series 2 patches.  This series focuses
mainly on removing CMD and TASK drive commands.  Patches are against
the latest ide-dev-2.6 bk tree.  Newly added are #01, #04 and #05.

 #01 was #20 in the first posting of 29 patches.  I pushed it back for
later merging but this bug combined with the recent update "[ide] get
driver from rq->rq_disk->private_data" breaks taskfile transport, so I
pulled it back on top (as all drive commands are going to be using
taskfile transport).

 #04 is your ATA_TFLAG_LBA48 patch.  The patch message should contain
proper Signed-off-by line, if my script is finally working correctly.

 #05 adds ATA_TFLAG_IO_16BIT handling for taskfile ioctl.  So, the
race condition involving drive->io_32bit is gone now.

 Other patches are modified as you requested.

[ Start of patch descriptions ]

01_ide_task_end_request_fix.patch
	: task_end_request() fix

	task_end_request() modified to always call ide_end_drive_cmd()
	for taskfile requests.  Previously, ide_end_drive_cmd() was
	called only when task->tf_out_flags.all was set.  Also,
	ide_dma_intr() is modified to use task_end_request().

	* fixes taskfile ioctl oops bug which was caused by referencing
	  NULL rq->rq_disk of taskfile requests.
	* enables TASKFILE ioctls to get valid register outputs on
	  successful completion.

02_ide_taskfile_init_drive_cmd.patch
	: ide_init_drive_cmd() now defaults to REQ_DRIVE_TASKFILE

	ide_init_drive_cmd() now initializes rq->flags to
	REQ_DRIVE_TASKFILE instead of REQ_DRIVE_CMD.  This is
	preparation for removal of REQ_DRIVE_CMD.

03_ide_diag_taskfile_use_init_drive_cmd.patch
	: ide_diag_taskfile() rq initialization fix

	In ide_diag_taskfile(), taskfile rq was initialized using
	memset().  Use init_drive_cmd() instead.

04_ide_ATA_TFLAG_LBA48.patch
	: removes unneeded HOB access using ATA_TFLAG_LBA48 flag

	This small patch fixes unneeded writes/reads to LBA48 taskfile
	registers on LBA48 capable disks for following cases:

	* Power Management requests
	  (WIN_FLUSH_CACHE[_EXT], WIN_STANDBYNOW1, WIN_IDLEIMMEDIATE commands)
	* special commands (WIN_SPECIFY, WIN_RESTORE, WIN_SETMULT)
	* Host Protected Area support (WIN_READ_NATIVE_MAX, WIN_SET_MAX)
	* /proc/ide/ SMART support (WIN_SMART with SMART_ENABLE,
	  SMART_READ_VALUES and SMART_READ_THRESHOLDS subcommands)
	* write cache enabling/disabling in ide-disk
	  (WIN_SETFEATURES with SETFEATURES_{EN,DIS}_WCACHE)
	* write cache flushing in ide-disk (WIN_FLUSH_CACHE[_EXT])
	* acoustic management in ide-disk
	  (WIN_SETFEATURES with SETFEATURES_{EN,DIS}_AAM)
	* door (un)locking in ide-disk (WIN_DOORLOCK, WIN_DOORUNLOCK)
	* /proc/ide/hd?/identify support (WIN_IDENTIFY)

	Patch adds 'unsinged long flags' to ide_task_t and uses
	ATA_TFLAG_LBA48 flag (from <linux/ata.h>) to indicate need of
	accessing LBA48 taskfile registers.

	Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@elka.pw.edu.pl>

05_ide_ATA_TFLAG_IO_16BIT.patch
	: fixes io_32bit race in ide_taskfile_ioctl()

	In ide_taskfile_ioctl(), there was a race condition involving
	drive->io_32bit.  It was cleared and restored during ioctl
	requests but there was no synchronization with other requests.
	So, other requests could execute with the altered io_32bit
	setting or updated drive->io_32bit could be overwritten by
	ide_taskfile_ioctl().

	This patch adds ATA_TFLAG_IO_16BIT flag to indicate to
	ide_pio_datablock() that 16bit IO is needed regardless of
	drive->io_32bit settting.

06_ide_taskfile_flush.patch
	: make disk flush functions use TASKFILE instead of TASK

	* idedisk_issue_flush() converted to use REQ_DRIVE_TASKFILE.
	   This and the changes in ide_get_error_location() remove a
	   possible race condition between ide_get_error_location()
	   and other requests.
	* ide_queue_flush_cmd() converted to use REQ_DRIVE_TASKFILE.

	By this change, when WIN_FLUSH_CACHE_EXT is used, full HOB
	registers are written and read.  This isn't optimal but
	shouldn't be noticeable either.

07_ide_taskfile_task_ioctl.patch
	: make ide_task_ioctl() use TASKFILE

	ide_task_ioctl() rewritten to use taskfile transport.  This is
	the last user of REQ_DRIVE_TASK.

08_ide_remove_task.patch
	: remove REQ_DRIVE_TASK handling

	Unused REQ_DRIVE_TASK handling removed.

09_ide_taskfile_cmd.patch
	: convert uses of REQ_DRIVE_CMD to REQ_DRIVE_TASKFILE

	All in-kernel REQ_DRIVE_CMD users except for ide_cmd_ioctl()
	converted to use REQ_DRIVE_TASKFILE.

10_ide_taskfile_cmd_ioctl.patch
	: make ide_cmd_ioctl() use TASKFILE

	ide_cmd_ioctl() rewritten to use taskfile transport.  This is
	the last user of REQ_DRIVE_CMD.

11_ide_remove_cmd.patch
	: remove REQ_DRIVE_CMD handling

	Unused REQ_DRIVE_CMD handling removed.

[ End of patch descriptions ]

 Thanks.

Re: [PATCH 2.6.11-rc3 01/11] ide: task_end_request() fix

[Tejun Heo]

01_ide_task_end_request_fix.patch

	task_end_request() modified to always call ide_end_drive_cmd()
	for taskfile requests.  Previously, ide_end_drive_cmd() was
	called only when task->tf_out_flags.all was set.  Also,
	ide_dma_intr() is modified to use task_end_request().

	* fixes taskfile ioctl oops bug which was caused by referencing
	  NULL rq->rq_disk of taskfile requests.
	* enables TASKFILE ioctls to get valid register outputs on
	  successful completion.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 drivers/ide/ide-dma.c      |    5 +----
 drivers/ide/ide-taskfile.c |   12 ++++--------
 include/linux/ide.h        |    1 +
 3 files changed, 6 insertions(+), 12 deletions(-)

Index: linux-ide/drivers/ide/ide-dma.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-dma.c	2005-02-10 17:31:48.917654643 +0900
+++ linux-ide/drivers/ide/ide-dma.c	2005-02-10 17:38:00.033092056 +0900
@@ -175,10 +175,7 @@ ide_startstop_t ide_dma_intr (ide_drive_
 	if (OK_STAT(stat,DRIVE_READY,drive->bad_wstat|DRQ_STAT)) {
 		if (!dma_stat) {
 			struct request *rq = HWGROUP(drive)->rq;
-			ide_driver_t *drv;
-
-			drv = *(ide_driver_t **)rq->rq_disk->private_data;;
-			drv->end_request(drive, 1, rq->nr_sectors);
+			task_end_request(drive, rq, stat);
 			return ide_stopped;
 		}
 		printk(KERN_ERR "%s: dma_intr: bad DMA status (dma_stat=%x)\n", 
Index: linux-ide/drivers/ide/ide-taskfile.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-taskfile.c	2005-02-10 17:31:48.917654643 +0900
+++ linux-ide/drivers/ide/ide-taskfile.c	2005-02-10 17:38:00.042090547 +0900
@@ -364,18 +364,14 @@ static ide_startstop_t task_error(ide_dr
 	return ide_error(drive, s, stat);
 }
 
-static void task_end_request(ide_drive_t *drive, struct request *rq, u8 stat)
+void task_end_request(ide_drive_t *drive, struct request *rq, u8 stat)
 {
 	ide_driver_t *drv;
 
 	if (rq->flags & REQ_DRIVE_TASKFILE) {
-		ide_task_t *task = rq->special;
-
-		if (task->tf_out_flags.all) {
-			u8 err = drive->hwif->INB(IDE_ERROR_REG);
-			ide_end_drive_cmd(drive, stat, err);
-			return;
-		}
+		u8 err = drive->hwif->INB(IDE_ERROR_REG);
+		ide_end_drive_cmd(drive, stat, err);
+		return;
 	}
 
 	drv = *(ide_driver_t **)rq->rq_disk->private_data;
Index: linux-ide/include/linux/ide.h
===================================================================
--- linux-ide.orig/include/linux/ide.h	2005-02-10 17:31:48.917654643 +0900
+++ linux-ide/include/linux/ide.h	2005-02-10 17:38:00.044090212 +0900
@@ -1290,6 +1290,7 @@ extern ide_startstop_t do_rw_taskfile(id
  */
 extern ide_startstop_t flagged_taskfile(ide_drive_t *, ide_task_t *);
 
+void task_end_request(ide_drive_t *, struct request *, u8);
 extern ide_startstop_t set_multmode_intr(ide_drive_t *);
 extern ide_startstop_t set_geometry_intr(ide_drive_t *);
 extern ide_startstop_t recal_intr(ide_drive_t *);

Re: [PATCH 2.6.11-rc3 02/11] ide: ide_init_drive_cmd() now defaults to REQ_DRIVE_TASKFILE

[Tejun Heo]

02_ide_taskfile_init_drive_cmd.patch

	ide_init_drive_cmd() now initializes rq->flags to
	REQ_DRIVE_TASKFILE instead of REQ_DRIVE_CMD.  This is
	preparation for removal of REQ_DRIVE_CMD.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 ide-io.c       |    4 ++--
 ide-taskfile.c |    1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

Index: linux-ide/drivers/ide/ide-io.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-io.c	2005-02-10 17:31:48.835669731 +0900
+++ linux-ide/drivers/ide/ide-io.c	2005-02-10 17:38:00.436024471 +0900
@@ -64,7 +64,7 @@ static void ide_fill_flush_cmd(ide_drive
 	 */
 	memset(buf, 0, sizeof(rq->cmd));
 
-	rq->flags |= REQ_DRIVE_TASK | REQ_STARTED;
+	rq->flags = REQ_DRIVE_TASK | REQ_STARTED;
 	rq->buffer = buf;
 	rq->buffer[0] = WIN_FLUSH_CACHE;
 
@@ -1738,7 +1738,7 @@ irqreturn_t ide_intr (int irq, void *dev
 void ide_init_drive_cmd (struct request *rq)
 {
 	memset(rq, 0, sizeof(*rq));
-	rq->flags = REQ_DRIVE_CMD;
+	rq->flags = REQ_DRIVE_TASKFILE;
 	rq->ref_count = 1;
 }
 
Index: linux-ide/drivers/ide/ide-taskfile.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-taskfile.c	2005-02-10 17:38:00.042090547 +0900
+++ linux-ide/drivers/ide/ide-taskfile.c	2005-02-10 17:38:00.437024304 +0900
@@ -668,6 +668,7 @@ int ide_wait_cmd (ide_drive_t *drive, u8
 		buf = buffer;
 	memset(buf, 0, 4 + SECTOR_WORDS * 4 * sectors);
 	ide_init_drive_cmd(&rq);
+	rq.flags = REQ_DRIVE_CMD;
 	rq.buffer = buf;
 	*buf++ = cmd;
 	*buf++ = nsect;

Re: [PATCH 2.6.11-rc3 03/11] ide: ide_diag_taskfile() rq initialization fix

[Tejun Heo]

03_ide_diag_taskfile_use_init_drive_cmd.patch

	In ide_diag_taskfile(), taskfile rq was initialized using
	memset().  Use init_drive_cmd() instead.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 ide-taskfile.c |    3 +--
 1 files changed, 1 insertion(+), 2 deletions(-)

Index: linux-ide/drivers/ide/ide-taskfile.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-taskfile.c	2005-02-10 17:38:00.437024304 +0900
+++ linux-ide/drivers/ide/ide-taskfile.c	2005-02-10 17:38:00.832957893 +0900
@@ -469,8 +469,7 @@ static int ide_diag_taskfile(ide_drive_t
 {
 	struct request rq;
 
-	memset(&rq, 0, sizeof(rq));
-	rq.flags = REQ_DRIVE_TASKFILE;
+	ide_init_drive_cmd(&rq);
 	rq.buffer = buf;
 
 	/*

Re: [PATCH 2.6.11-rc3 04/11] ide: removes unneeded HOB access using ATA_TFLAG_LBA48 flag

[Tejun Heo]

04_ide_ATA_TFLAG_LBA48.patch

	This small patch fixes unneeded writes/reads to LBA48 taskfile
	registers on LBA48 capable disks for following cases:

	* Power Management requests
	  (WIN_FLUSH_CACHE[_EXT], WIN_STANDBYNOW1, WIN_IDLEIMMEDIATE commands)
	* special commands (WIN_SPECIFY, WIN_RESTORE, WIN_SETMULT)
	* Host Protected Area support (WIN_READ_NATIVE_MAX, WIN_SET_MAX)
	* /proc/ide/ SMART support (WIN_SMART with SMART_ENABLE,
	  SMART_READ_VALUES and SMART_READ_THRESHOLDS subcommands)
	* write cache enabling/disabling in ide-disk
	  (WIN_SETFEATURES with SETFEATURES_{EN,DIS}_WCACHE)
	* write cache flushing in ide-disk (WIN_FLUSH_CACHE[_EXT])
	* acoustic management in ide-disk
	  (WIN_SETFEATURES with SETFEATURES_{EN,DIS}_AAM)
	* door (un)locking in ide-disk (WIN_DOORLOCK, WIN_DOORUNLOCK)
	* /proc/ide/hd?/identify support (WIN_IDENTIFY)

	Patch adds 'unsinged long flags' to ide_task_t and uses
	ATA_TFLAG_LBA48 flag (from <linux/ata.h>) to indicate need of
	accessing LBA48 taskfile registers.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@elka.pw.edu.pl>

 drivers/ide/ide-disk.c     |    6 ++++++
 drivers/ide/ide-io.c       |    2 +-
 drivers/ide/ide-taskfile.c |    7 +++++--
 include/linux/ide.h        |    1 +
 4 files changed, 13 insertions(+), 3 deletions(-)

Index: linux-ide/drivers/ide/ide-disk.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-disk.c	2005-02-10 17:31:48.698694939 +0900
+++ linux-ide/drivers/ide/ide-disk.c	2005-02-10 17:38:01.169901379 +0900
@@ -362,6 +362,9 @@ static unsigned long long idedisk_read_n
 	args.tfRegister[IDE_COMMAND_OFFSET]	= WIN_READ_NATIVE_MAX_EXT;
 	args.command_type			= IDE_DRIVE_TASK_NO_DATA;
 	args.handler				= &task_no_data_intr;
+
+	args.flags |= ATA_TFLAG_LBA48;
+
         /* submit command request */
         ide_raw_taskfile(drive, &args, NULL);
 
@@ -431,6 +434,9 @@ static unsigned long long idedisk_set_ma
 	args.hobRegister[IDE_CONTROL_OFFSET_HOB]= (drive->ctl|0x80);
 	args.command_type			= IDE_DRIVE_TASK_NO_DATA;
 	args.handler				= &task_no_data_intr;
+
+	args.flags |= ATA_TFLAG_LBA48;
+
 	/* submit command request */
 	ide_raw_taskfile(drive, &args, NULL);
 	/* if OK, compute maximum address value */
Index: linux-ide/drivers/ide/ide-io.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-io.c	2005-02-10 17:38:00.436024471 +0900
+++ linux-ide/drivers/ide/ide-io.c	2005-02-10 17:38:01.171901044 +0900
@@ -487,7 +487,7 @@ void ide_end_drive_cmd (ide_drive_t *dri
 			args->tfRegister[IDE_SELECT_OFFSET]  = hwif->INB(IDE_SELECT_REG);
 			args->tfRegister[IDE_STATUS_OFFSET]  = stat;
 
-			if (drive->addressing == 1) {
+			if (args->flags & ATA_TFLAG_LBA48) {
 				hwif->OUTB(drive->ctl|0x80, IDE_CONTROL_REG);
 				args->hobRegister[IDE_FEATURE_OFFSET]	= hwif->INB(IDE_FEATURE_REG);
 				args->hobRegister[IDE_NSECTOR_OFFSET]	= hwif->INB(IDE_NSECTOR_REG);
Index: linux-ide/drivers/ide/ide-taskfile.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-taskfile.c	2005-02-10 17:38:00.832957893 +0900
+++ linux-ide/drivers/ide/ide-taskfile.c	2005-02-10 17:38:01.172900876 +0900
@@ -101,7 +101,7 @@ ide_startstop_t do_rw_taskfile (ide_driv
 	ide_hwif_t *hwif	= HWIF(drive);
 	task_struct_t *taskfile	= (task_struct_t *) task->tfRegister;
 	hob_struct_t *hobfile	= (hob_struct_t *) task->hobRegister;
-	u8 HIHI			= (drive->addressing == 1) ? 0xE0 : 0xEF;
+	u8 HIHI			= (task->flags & ATA_TFLAG_LBA48) ? 0xE0 : 0xEF;
 
 	/* ALL Command Block Executions SHALL clear nIEN, unless otherwise */
 	if (IDE_CONTROL_REG) {
@@ -110,7 +110,7 @@ ide_startstop_t do_rw_taskfile (ide_driv
 	}
 	SELECT_MASK(drive, 0);
 
-	if (drive->addressing == 1) {
+	if (task->flags & ATA_TFLAG_LBA48) {
 		hwif->OUTB(hobfile->feature, IDE_FEATURE_REG);
 		hwif->OUTB(hobfile->sector_count, IDE_NSECTOR_REG);
 		hwif->OUTB(hobfile->sector_number, IDE_SECTOR_REG);
@@ -573,6 +573,9 @@ int ide_taskfile_ioctl (ide_drive_t *dri
 	args.data_phase   = req_task->data_phase;
 	args.command_type = req_task->req_cmd;
 
+	if (drive->addressing == 1)
+		args.flags |= ATA_TFLAG_LBA48;
+
 	drive->io_32bit = 0;
 	switch(req_task->data_phase) {
 		case TASKFILE_OUT_DMAQ:
Index: linux-ide/include/linux/ide.h
===================================================================
--- linux-ide.orig/include/linux/ide.h	2005-02-10 17:38:00.044090212 +0900
+++ linux-ide/include/linux/ide.h	2005-02-10 17:38:01.174900541 +0900
@@ -1258,6 +1258,7 @@ typedef struct ide_task_s {
  *	struct hd_drive_hob_hdr		hobf;
  *	hob_struct_t		hobf;
  */
+	unsigned long		flags;		/* ATA_TFLAG_xxx */
 	task_ioreg_t		tfRegister[8];
 	task_ioreg_t		hobRegister[8];
 	ide_reg_valid_t		tf_out_flags;

Re: [PATCH 2.6.11-rc3 05/11] ide: fixes io_32bit race in ide_taskfile_ioctl()

[Tejun Heo]

05_ide_ATA_TFLAG_IO_16BIT.patch

	In ide_taskfile_ioctl(), there was a race condition involving
	drive->io_32bit.  It was cleared and restored during ioctl
	requests but there was no synchronization with other requests.
	So, other requests could execute with the altered io_32bit
	setting or updated drive->io_32bit could be overwritten by
	ide_taskfile_ioctl().

	This patch adds ATA_TFLAG_IO_16BIT flag to indicate to
	ide_pio_datablock() that 16bit IO is needed regardless of
	drive->io_32bit settting.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 drivers/ide/ide-taskfile.c |   14 ++++++++++----
 include/linux/ata.h        |    1 +
 2 files changed, 11 insertions(+), 4 deletions(-)

Index: linux-ide/drivers/ide/ide-taskfile.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-taskfile.c	2005-02-10 17:38:01.172900876 +0900
+++ linux-ide/drivers/ide/ide-taskfile.c	2005-02-10 17:38:01.539839336 +0900
@@ -315,8 +315,15 @@ static void ide_pio_multi(ide_drive_t *d
 static inline void ide_pio_datablock(ide_drive_t *drive, struct request *rq,
 				     unsigned int write)
 {
+	int saved_io_32bit = drive->io_32bit;
+
 	if (rq->bio)	/* fs request */
 		rq->errors = 0;
+	if (rq->flags & REQ_DRIVE_TASKFILE) {
+		ide_task_t *task = rq->special;
+		if (task->flags & ATA_TFLAG_IO_16BIT)
+			drive->io_32bit = 0;
+	}
 
 	switch (drive->hwif->data_phase) {
 	case TASKFILE_MULTI_IN:
@@ -327,6 +334,8 @@ static inline void ide_pio_datablock(ide
 		ide_pio_sector(drive, write);
 		break;
 	}
+
+	drive->io_32bit = saved_io_32bit;
 }
 
 static ide_startstop_t task_error(ide_drive_t *drive, struct request *rq,
@@ -520,7 +529,6 @@ int ide_taskfile_ioctl (ide_drive_t *dri
 	int tasksize		= sizeof(struct ide_task_request_s);
 	int taskin		= 0;
 	int taskout		= 0;
-	u8 io_32bit		= drive->io_32bit;
 	char __user *buf = (char __user *)arg;
 
 //	printk("IDE Taskfile ...\n");
@@ -573,10 +581,10 @@ int ide_taskfile_ioctl (ide_drive_t *dri
 	args.data_phase   = req_task->data_phase;
 	args.command_type = req_task->req_cmd;
 
+	args.flags = ATA_TFLAG_IO_16BIT;
 	if (drive->addressing == 1)
 		args.flags |= ATA_TFLAG_LBA48;
 
-	drive->io_32bit = 0;
 	switch(req_task->data_phase) {
 		case TASKFILE_OUT_DMAQ:
 		case TASKFILE_OUT_DMA:
@@ -656,8 +664,6 @@ abort:
 
 //	printk("IDE Taskfile ioctl ended. rc = %i\n", err);
 
-	drive->io_32bit = io_32bit;
-
 	return err;
 }
 
Index: linux-ide/include/linux/ata.h
===================================================================
--- linux-ide.orig/include/linux/ata.h	2005-02-10 17:31:48.610711131 +0900
+++ linux-ide/include/linux/ata.h	2005-02-10 17:38:01.540839168 +0900
@@ -174,6 +174,7 @@ enum {
 	ATA_TFLAG_ISADDR	= (1 << 1), /* enable r/w to nsect/lba regs */
 	ATA_TFLAG_DEVICE	= (1 << 2), /* enable r/w to device reg */
 	ATA_TFLAG_WRITE		= (1 << 3), /* data dir: host->dev==1 (write) */
+	ATA_TFLAG_IO_16BIT	= (1 << 4), /* force 16bit pio */
 };
 
 enum ata_tf_protocols {

Re: [PATCH 2.6.11-rc3 06/11] ide: make disk flush functions use TASKFILE instead of TASK

[Tejun Heo]

06_ide_taskfile_flush.patch

	* idedisk_issue_flush() converted to use REQ_DRIVE_TASKFILE.
	   This and the changes in ide_get_error_location() remove a
	   possible race condition between ide_get_error_location()
	   and other requests.
	* ide_queue_flush_cmd() converted to use REQ_DRIVE_TASKFILE.

	By this change, when WIN_FLUSH_CACHE_EXT is used, full HOB
	registers are written and read.  This isn't optimal but
	shouldn't be noticeable either.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 drivers/ide/ide-disk.c |   19 +++-------
 drivers/ide/ide-io.c   |   91 ++++++++++++++++++++++++-------------------------
 include/linux/ide.h    |   52 +++++++++++++++-------------
 3 files changed, 81 insertions(+), 81 deletions(-)

Index: linux-ide/drivers/ide/ide-disk.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-disk.c	2005-02-10 17:38:01.169901379 +0900
+++ linux-ide/drivers/ide/ide-disk.c	2005-02-10 17:38:01.744804960 +0900
@@ -688,24 +688,17 @@ static int idedisk_issue_flush(request_q
 {
 	ide_drive_t *drive = q->queuedata;
 	struct request *rq;
+	ide_task_t task;
 	int ret;
 
 	if (!drive->wcache)
 		return 0;
 
-	rq = blk_get_request(q, WRITE, __GFP_WAIT);
-
-	memset(rq->cmd, 0, sizeof(rq->cmd));
-
-	if (ide_id_has_flush_cache_ext(drive->id) &&
-	    (drive->capacity64 >= (1UL << 28)))
-		rq->cmd[0] = WIN_FLUSH_CACHE_EXT;
-	else
-		rq->cmd[0] = WIN_FLUSH_CACHE;
-
+	ide_init_flush_task(drive, &task);
 
-	rq->flags |= REQ_DRIVE_TASK | REQ_SOFTBARRIER;
-	rq->buffer = rq->cmd;
+	rq = blk_get_request(q, WRITE, __GFP_WAIT);
+	rq->flags |= REQ_DRIVE_TASKFILE | REQ_SOFTBARRIER;
+	rq->special = &task;
 
 	ret = blk_execute_rq(q, disk, rq);
 
@@ -713,7 +706,7 @@ static int idedisk_issue_flush(request_q
 	 * if we failed and caller wants error offset, get it
 	 */
 	if (ret && error_sector)
-		*error_sector = ide_get_error_location(drive, rq->cmd);
+		*error_sector = ide_get_error_location(drive, &task);
 
 	blk_put_request(rq);
 	return ret;
Index: linux-ide/drivers/ide/ide-io.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-io.c	2005-02-10 17:38:01.171901044 +0900
+++ linux-ide/drivers/ide/ide-io.c	2005-02-10 17:38:01.746804625 +0900
@@ -55,24 +55,25 @@
 #include <asm/io.h>
 #include <asm/bitops.h>
 
-static void ide_fill_flush_cmd(ide_drive_t *drive, struct request *rq)
+void ide_init_flush_task(ide_drive_t *drive, ide_task_t *task)
 {
-	char *buf = rq->cmd;
-
-	/*
-	 * reuse cdb space for ata command
-	 */
-	memset(buf, 0, sizeof(rq->cmd));
-
-	rq->flags = REQ_DRIVE_TASK | REQ_STARTED;
-	rq->buffer = buf;
-	rq->buffer[0] = WIN_FLUSH_CACHE;
+	memset(task, 0, sizeof(*task));
 
 	if (ide_id_has_flush_cache_ext(drive->id) &&
-	    (drive->capacity64 >= (1UL << 28)))
-		rq->buffer[0] = WIN_FLUSH_CACHE_EXT;
+	    (drive->capacity64 >= (1UL << 28))) {
+		task->tfRegister[IDE_COMMAND_OFFSET] = WIN_FLUSH_CACHE_EXT;
+		task->flags |= ATA_TFLAG_LBA48;
+	} else {
+		task->tfRegister[IDE_COMMAND_OFFSET] = WIN_FLUSH_CACHE;
+	}
+
+	task->command_type = IDE_DRIVE_TASK_NO_DATA;
+	task->data_phase = TASKFILE_NO_DATA;
+	task->handler = task_no_data_intr;
 }
 
+EXPORT_SYMBOL(ide_init_flush_task);
+
 /*
  * preempt pending requests, and store this cache flush for immediate
  * execution
@@ -80,7 +81,9 @@ static void ide_fill_flush_cmd(ide_drive
 static struct request *ide_queue_flush_cmd(ide_drive_t *drive,
 					   struct request *rq, int post)
 {
-	struct request *flush_rq = &HWGROUP(drive)->wrq;
+	ide_hwgroup_t *hwgroup = drive->hwif->hwgroup;
+	struct request *flush_rq = &hwgroup->flush_rq;
+	ide_task_t *task = &hwgroup->flush_task;
 
 	/*
 	 * write cache disabled, clear the barrier bit and treat it like
@@ -92,10 +95,12 @@ static struct request *ide_queue_flush_c
 	}
 
 	ide_init_drive_cmd(flush_rq);
-	ide_fill_flush_cmd(drive, flush_rq);
-
-	flush_rq->special = rq;
+	flush_rq->flags |= REQ_STARTED;
 	flush_rq->nr_sectors = rq->nr_sectors;
+	flush_rq->special = task;
+	hwgroup->flush_real_rq = rq;
+
+	ide_init_flush_task(drive, task);
 
 	if (!post) {
 		drive->doing_barrier = 1;
@@ -105,7 +110,7 @@ static struct request *ide_queue_flush_c
 		flush_rq->flags |= REQ_BAR_POSTFLUSH;
 
 	__elv_add_request(drive->queue, flush_rq, ELEVATOR_INSERT_FRONT, 0);
-	HWGROUP(drive)->rq = NULL;
+	hwgroup->rq = NULL;
 	return flush_rq;
 }
 
@@ -162,12 +167,13 @@ static int __ide_end_request(ide_drive_t
 
 int ide_end_request (ide_drive_t *drive, int uptodate, int nr_sectors)
 {
+	ide_hwgroup_t *hwgroup = drive->hwif->hwgroup;
 	struct request *rq;
 	unsigned long flags;
 	int ret = 1;
 
 	spin_lock_irqsave(&ide_lock, flags);
-	rq = HWGROUP(drive)->rq;
+	rq = hwgroup->rq;
 
 	if (!nr_sectors)
 		nr_sectors = rq->hard_cur_sectors;
@@ -175,7 +181,7 @@ int ide_end_request (ide_drive_t *drive,
 	if (!blk_barrier_rq(rq) || !drive->wcache)
 		ret = __ide_end_request(drive, rq, uptodate, nr_sectors);
 	else {
-		struct request *flush_rq = &HWGROUP(drive)->wrq;
+		struct request *flush_rq = &hwgroup->flush_rq;
 
 		flush_rq->nr_sectors -= nr_sectors;
 		if (!flush_rq->nr_sectors) {
@@ -314,41 +320,36 @@ static void ide_complete_pm_request (ide
 /*
  * FIXME: probably move this somewhere else, name is bad too :)
  */
-u64 ide_get_error_location(ide_drive_t *drive, char *args)
+u64 ide_get_error_location(ide_drive_t *drive, ide_task_t *task)
 {
 	u32 high, low;
 	u8 hcyl, lcyl, sect;
-	u64 sector;
 
-	high = 0;
-	hcyl = args[5];
-	lcyl = args[4];
-	sect = args[3];
-
-	if (ide_id_has_flush_cache_ext(drive->id)) {
-		low = (hcyl << 16) | (lcyl << 8) | sect;
-		HWIF(drive)->OUTB(drive->ctl|0x80, IDE_CONTROL_REG);
-		high = ide_read_24(drive);
-	} else {
-		u8 cur = HWIF(drive)->INB(IDE_SELECT_REG);
-		if (cur & 0x40)
-			low = (hcyl << 16) | (lcyl << 8) | sect;
-		else {
-			low = hcyl * drive->head * drive->sect;
-			low += lcyl * drive->sect;
-			low += sect - 1;
-		}
-	}
+	if (drive->addressing == 1)
+		high = (task->hobRegister[IDE_HCYL_OFFSET] << 16) |
+			(task->hobRegister[IDE_LCYL_OFFSET] << 8) |
+			task->hobRegister[IDE_SECTOR_OFFSET];
+	else
+		high = 0;
+
+	hcyl = task->tfRegister[IDE_HCYL_OFFSET];
+	lcyl = task->tfRegister[IDE_LCYL_OFFSET];
+	sect = task->tfRegister[IDE_SECTOR_OFFSET];
+
+	if (task->tfRegister[IDE_SELECT_OFFSET] & 0x40)
+		low = (hcyl << 16) | (lcyl << 8 ) | sect;
+	else
+		low = hcyl * drive->head * drive->sect +
+			lcyl * drive->sect + sect - 1;
 
-	sector = ((u64) high << 24) | low;
-	return sector;
+	return ((u64)high << 24) | low; 
 }
 EXPORT_SYMBOL(ide_get_error_location);
 
 static void ide_complete_barrier(ide_drive_t *drive, struct request *rq,
 				 int error)
 {
-	struct request *real_rq = rq->special;
+	struct request *real_rq = drive->hwif->hwgroup->flush_real_rq;
 	int good_sectors, bad_sectors;
 	sector_t sector;
 
@@ -395,7 +396,7 @@ static void ide_complete_barrier(ide_dri
 		 */
 		good_sectors = 0;
 		if (blk_barrier_postflush(rq)) {
-			sector = ide_get_error_location(drive, rq->buffer);
+			sector = ide_get_error_location(drive, rq->special);
 
 			if ((sector >= real_rq->hard_sector) &&
 			    (sector < real_rq->hard_sector + real_rq->hard_nr_sectors))
Index: linux-ide/include/linux/ide.h
===================================================================
--- linux-ide.orig/include/linux/ide.h	2005-02-10 17:38:01.174900541 +0900
+++ linux-ide/include/linux/ide.h	2005-02-10 17:38:01.745804793 +0900
@@ -926,6 +926,26 @@ typedef ide_startstop_t (ide_pre_handler
 typedef ide_startstop_t (ide_handler_t)(ide_drive_t *);
 typedef int (ide_expiry_t)(ide_drive_t *);
 
+typedef struct ide_task_s {
+/*
+ *	struct hd_drive_task_hdr	tf;
+ *	task_struct_t		tf;
+ *	struct hd_drive_hob_hdr		hobf;
+ *	hob_struct_t		hobf;
+ */
+	unsigned long		flags;		/* ATA_TFLAG_xxx */
+	task_ioreg_t		tfRegister[8];
+	task_ioreg_t		hobRegister[8];
+	ide_reg_valid_t		tf_out_flags;
+	ide_reg_valid_t		tf_in_flags;
+	int			data_phase;
+	int			command_type;
+	ide_pre_handler_t	*prehandler;
+	ide_handler_t		*handler;
+	struct request		*rq;		/* copy of request */
+	void			*special;	/* valid_t generally */
+} ide_task_t;
+
 typedef struct hwgroup_s {
 		/* irq handler, if active */
 	ide_startstop_t	(*handler)(ide_drive_t *);
@@ -951,8 +971,12 @@ typedef struct hwgroup_s {
 	struct request *rq;
 		/* failsafe timer */
 	struct timer_list timer;
-		/* local copy of current write rq */
-	struct request wrq;
+		/* rq used for flushing */
+	struct request flush_rq;
+		/* task used for flushing */
+	ide_task_t flush_task;
+		/* real_rq for flushing */
+	struct request *flush_real_rq;
 		/* timeout value during long polls */
 	unsigned long poll_timeout;
 		/* queried upon timeouts */
@@ -1191,10 +1215,12 @@ extern ide_startstop_t ide_do_reset (ide
  */
 extern void ide_init_drive_cmd (struct request *rq);
 
+void ide_init_flush_task(ide_drive_t *, ide_task_t *);
+
 /*
  * this function returns error location sector offset in case of a write error
  */
-extern u64 ide_get_error_location(ide_drive_t *, char *);
+u64 ide_get_error_location(ide_drive_t *, ide_task_t *);
 
 /*
  * "action" parameter type for ide_do_drive_cmd() below.
@@ -1251,26 +1277,6 @@ extern void ide_end_drive_cmd(ide_drive_
  */
 extern int ide_wait_cmd(ide_drive_t *, u8, u8, u8, u8, u8 *);
 
-typedef struct ide_task_s {
-/*
- *	struct hd_drive_task_hdr	tf;
- *	task_struct_t		tf;
- *	struct hd_drive_hob_hdr		hobf;
- *	hob_struct_t		hobf;
- */
-	unsigned long		flags;		/* ATA_TFLAG_xxx */
-	task_ioreg_t		tfRegister[8];
-	task_ioreg_t		hobRegister[8];
-	ide_reg_valid_t		tf_out_flags;
-	ide_reg_valid_t		tf_in_flags;
-	int			data_phase;
-	int			command_type;
-	ide_pre_handler_t	*prehandler;
-	ide_handler_t		*handler;
-	struct request		*rq;		/* copy of request */
-	void			*special;	/* valid_t generally */
-} ide_task_t;
-
 extern u32 ide_read_24(ide_drive_t *);
 
 extern void SELECT_DRIVE(ide_drive_t *);

Re: [PATCH 2.6.11-rc3 07/11] ide: make ide_task_ioctl() use TASKFILE

[Tejun Heo]

07_ide_taskfile_task_ioctl.patch

	ide_task_ioctl() rewritten to use taskfile transport.  This is
	the last user of REQ_DRIVE_TASK.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 ide-taskfile.c |   50 +++++++++++++++++++++++++++++++-------------------
 1 files changed, 31 insertions(+), 19 deletions(-)

Index: linux-ide/drivers/ide/ide-taskfile.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-taskfile.c	2005-02-10 17:38:01.539839336 +0900
+++ linux-ide/drivers/ide/ide-taskfile.c	2005-02-10 17:38:01.974766393 +0900
@@ -742,30 +742,42 @@ abort:
 	return err;
 }
 
-static int ide_wait_cmd_task(ide_drive_t *drive, u8 *buf)
-{
-	struct request rq;
-
-	ide_init_drive_cmd(&rq);
-	rq.flags = REQ_DRIVE_TASK;
-	rq.buffer = buf;
-	return ide_do_drive_cmd(drive, &rq, ide_wait);
-}
-
-/*
- * FIXME : this needs to map into at taskfile. <andre@linux-ide.org>
- */
-int ide_task_ioctl (ide_drive_t *drive, unsigned int cmd, unsigned long arg)
+int ide_task_ioctl(ide_drive_t *drive, unsigned int cmd, unsigned long arg)
 {
 	void __user *p = (void __user *)arg;
-	int err = 0;
-	u8 args[7], *argbuf = args;
-	int argsize = 7;
+	int err;
+	u8 args[7];
+	ide_task_t task;
+	u8 *regs = task.tfRegister;
 
 	if (copy_from_user(args, p, 7))
 		return -EFAULT;
-	err = ide_wait_cmd_task(drive, argbuf);
-	if (copy_to_user(p, argbuf, argsize))
+
+	memset(&task, 0, sizeof(task));
+
+	regs[IDE_COMMAND_OFFSET]	= args[0];
+	regs[IDE_FEATURE_OFFSET]	= args[1];
+	regs[IDE_NSECTOR_OFFSET]	= args[2];
+	regs[IDE_SECTOR_OFFSET]		= args[3];
+	regs[IDE_LCYL_OFFSET]		= args[4];
+	regs[IDE_HCYL_OFFSET]		= args[5];
+	regs[IDE_SELECT_OFFSET]		= args[6];
+	
+	task.command_type		= IDE_DRIVE_TASK_NO_DATA;
+	task.data_phase			= TASKFILE_NO_DATA;
+	task.handler			= &task_no_data_intr;
+
+	err = ide_diag_taskfile(drive, &task, 0, NULL);
+
+	args[0] = regs[IDE_COMMAND_OFFSET];
+	args[1] = regs[IDE_FEATURE_OFFSET];
+	args[2] = regs[IDE_NSECTOR_OFFSET];
+	args[3] = regs[IDE_SECTOR_OFFSET];
+	args[4] = regs[IDE_LCYL_OFFSET];
+	args[5] = regs[IDE_HCYL_OFFSET];
+	args[6] = regs[IDE_SELECT_OFFSET];
+
+	if (copy_to_user(p, args, 7))
 		err = -EFAULT;
 	return err;
 }

Re: [PATCH 2.6.11-rc3 08/11] ide: remove REQ_DRIVE_TASK handling

[Tejun Heo]

08_ide_remove_task.patch

	Unused REQ_DRIVE_TASK handling removed.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 drivers/block/ll_rw_blk.c |    1 
 drivers/ide/ide-io.c      |   48 +++-------------------------------------------
 drivers/ide/ide-lib.c     |    2 -
 include/linux/blkdev.h    |    2 -
 4 files changed, 5 insertions(+), 48 deletions(-)

Index: linux-ide/drivers/block/ll_rw_blk.c
===================================================================
--- linux-ide.orig/drivers/block/ll_rw_blk.c	2005-02-10 17:31:48.393751059 +0900
+++ linux-ide/drivers/block/ll_rw_blk.c	2005-02-10 17:38:02.165734368 +0900
@@ -851,7 +851,6 @@ static char *rq_flags[] = {
 	"REQ_QUIET",
 	"REQ_SPECIAL",
 	"REQ_DRIVE_CMD",
-	"REQ_DRIVE_TASK",
 	"REQ_DRIVE_TASKFILE",
 	"REQ_PREEMPT",
 	"REQ_PM_SUSPEND",
Index: linux-ide/drivers/ide/ide-io.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-io.c	2005-02-10 17:38:01.746804625 +0900
+++ linux-ide/drivers/ide/ide-io.c	2005-02-10 17:38:02.162734871 +0900
@@ -453,20 +453,6 @@ void ide_end_drive_cmd (ide_drive_t *dri
 			args[1] = err;
 			args[2] = hwif->INB(IDE_NSECTOR_REG);
 		}
-	} else if (rq->flags & REQ_DRIVE_TASK) {
-		u8 *args = (u8 *) rq->buffer;
-		if (rq->errors == 0)
-			rq->errors = !OK_STAT(stat,READY_STAT,BAD_STAT);
-
-		if (args) {
-			args[0] = stat;
-			args[1] = err;
-			args[2] = hwif->INB(IDE_NSECTOR_REG);
-			args[3] = hwif->INB(IDE_SECTOR_REG);
-			args[4] = hwif->INB(IDE_LCYL_REG);
-			args[5] = hwif->INB(IDE_HCYL_REG);
-			args[6] = hwif->INB(IDE_SELECT_REG);
-		}
 	} else if (rq->flags & REQ_DRIVE_TASKFILE) {
 		ide_task_t *args = (ide_task_t *) rq->special;
 		if (rq->errors == 0)
@@ -666,7 +652,7 @@ ide_startstop_t ide_error (ide_drive_t *
 		return ide_stopped;
 
 	/* retry only "normal" I/O: */
-	if (rq->flags & (REQ_DRIVE_CMD | REQ_DRIVE_TASK | REQ_DRIVE_TASKFILE)) {
+	if (rq->flags & (REQ_DRIVE_CMD | REQ_DRIVE_TASKFILE)) {
 		rq->errors = 1;
 		ide_end_drive_cmd(drive, stat, err);
 		return ide_stopped;
@@ -717,7 +703,7 @@ ide_startstop_t ide_abort(ide_drive_t *d
 		return ide_stopped;
 
 	/* retry only "normal" I/O: */
-	if (rq->flags & (REQ_DRIVE_CMD | REQ_DRIVE_TASK | REQ_DRIVE_TASKFILE)) {
+	if (rq->flags & (REQ_DRIVE_CMD | REQ_DRIVE_TASKFILE)) {
 		rq->errors = 1;
 		ide_end_drive_cmd(drive, BUSY_STAT, 0);
 		return ide_stopped;
@@ -945,33 +931,7 @@ static ide_startstop_t execute_drive_cmd
 		if (args->tf_out_flags.all != 0) 
 			return flagged_taskfile(drive, args);
 		return do_rw_taskfile(drive, args);
-	} else if (rq->flags & REQ_DRIVE_TASK) {
-		u8 *args = rq->buffer;
-		u8 sel;
- 
-		if (!args)
-			goto done;
-#ifdef DEBUG
- 		printk("%s: DRIVE_TASK_CMD ", drive->name);
- 		printk("cmd=0x%02x ", args[0]);
- 		printk("fr=0x%02x ", args[1]);
- 		printk("ns=0x%02x ", args[2]);
- 		printk("sc=0x%02x ", args[3]);
- 		printk("lcyl=0x%02x ", args[4]);
- 		printk("hcyl=0x%02x ", args[5]);
- 		printk("sel=0x%02x\n", args[6]);
-#endif
- 		hwif->OUTB(args[1], IDE_FEATURE_REG);
- 		hwif->OUTB(args[3], IDE_SECTOR_REG);
- 		hwif->OUTB(args[4], IDE_LCYL_REG);
- 		hwif->OUTB(args[5], IDE_HCYL_REG);
- 		sel = (args[6] & ~0x10);
- 		if (drive->select.b.unit)
- 			sel |= 0x10;
- 		hwif->OUTB(sel, IDE_SELECT_REG);
- 		ide_cmd(drive, args[0], args[2], &drive_cmd_intr);
- 		return ide_started;
- 	} else if (rq->flags & REQ_DRIVE_CMD) {
+	} else if (rq->flags & REQ_DRIVE_CMD) {
  		u8 *args = rq->buffer;
 
 		if (!args)
@@ -1084,7 +1044,7 @@ static ide_startstop_t start_request (id
 	if (!drive->special.all) {
 		ide_driver_t *drv;
 
-		if (rq->flags & (REQ_DRIVE_CMD | REQ_DRIVE_TASK))
+		if (rq->flags & REQ_DRIVE_CMD)
 			return execute_drive_cmd(drive, rq);
 		else if (rq->flags & REQ_DRIVE_TASKFILE)
 			return execute_drive_cmd(drive, rq);
Index: linux-ide/drivers/ide/ide-lib.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-lib.c	2005-02-10 17:31:48.393751059 +0900
+++ linux-ide/drivers/ide/ide-lib.c	2005-02-10 17:38:02.163734703 +0900
@@ -458,7 +458,7 @@ static void ide_dump_opcode(ide_drive_t 
 	spin_unlock(&ide_lock);
 	if (!rq)
 		return;
-	if (rq->flags & (REQ_DRIVE_CMD | REQ_DRIVE_TASK)) {
+	if (rq->flags & REQ_DRIVE_CMD) {
 		char *args = rq->buffer;
 		if (args) {
 			opcode = args[0];
Index: linux-ide/include/linux/blkdev.h
===================================================================
--- linux-ide.orig/include/linux/blkdev.h	2005-02-10 17:31:48.393751059 +0900
+++ linux-ide/include/linux/blkdev.h	2005-02-10 17:38:02.163734703 +0900
@@ -202,7 +202,6 @@ enum rq_flag_bits {
 	__REQ_QUIET,		/* don't worry about errors */
 	__REQ_SPECIAL,		/* driver suplied command */
 	__REQ_DRIVE_CMD,
-	__REQ_DRIVE_TASK,
 	__REQ_DRIVE_TASKFILE,
 	__REQ_PREEMPT,		/* set for "ide_preempt" requests */
 	__REQ_PM_SUSPEND,	/* suspend request */
@@ -229,7 +228,6 @@ enum rq_flag_bits {
 #define REQ_QUIET	(1 << __REQ_QUIET)
 #define REQ_SPECIAL	(1 << __REQ_SPECIAL)
 #define REQ_DRIVE_CMD	(1 << __REQ_DRIVE_CMD)
-#define REQ_DRIVE_TASK	(1 << __REQ_DRIVE_TASK)
 #define REQ_DRIVE_TASKFILE	(1 << __REQ_DRIVE_TASKFILE)
 #define REQ_PREEMPT	(1 << __REQ_PREEMPT)
 #define REQ_PM_SUSPEND	(1 << __REQ_PM_SUSPEND)

Re: [PATCH 2.6.11-rc3 09/11] ide: convert uses of REQ_DRIVE_CMD to REQ_DRIVE_TASKFILE

[Tejun Heo]

09_ide_taskfile_cmd.patch

	All in-kernel REQ_DRIVE_CMD users except for ide_cmd_ioctl()
	converted to use REQ_DRIVE_TASKFILE.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 ide-disk.c |    1 -
 ide.c      |   15 ++++++++++++---
 2 files changed, 12 insertions(+), 4 deletions(-)

Index: linux-ide/drivers/ide/ide-disk.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-disk.c	2005-02-10 17:38:01.744804960 +0900
+++ linux-ide/drivers/ide/ide-disk.c	2005-02-10 17:38:02.415692451 +0900
@@ -723,7 +723,6 @@ static int set_multcount(ide_drive_t *dr
 	if (drive->special.b.set_multmode)
 		return -EBUSY;
 	ide_init_drive_cmd (&rq);
-	rq.flags = REQ_DRIVE_CMD;
 	drive->mult_req = arg;
 	drive->special.b.set_multmode = 1;
 	(void) ide_do_drive_cmd (drive, &rq, ide_wait);
Index: linux-ide/drivers/ide/ide.c
===================================================================
--- linux-ide.orig/drivers/ide/ide.c	2005-02-10 17:31:48.305767251 +0900
+++ linux-ide/drivers/ide/ide.c	2005-02-10 17:38:02.416692284 +0900
@@ -1250,9 +1250,18 @@ static int set_pio_mode (ide_drive_t *dr
 
 static int set_xfer_rate (ide_drive_t *drive, int arg)
 {
-	int err = ide_wait_cmd(drive,
-			WIN_SETFEATURES, (u8) arg,
-			SETFEATURES_XFER, 0, NULL);
+	ide_task_t task;
+	int err;
+
+	memset(&task, 0, sizeof(task));
+	task.tfRegister[IDE_COMMAND_OFFSET]	= WIN_SETFEATURES;
+	task.tfRegister[IDE_FEATURE_OFFSET]	= SETFEATURES_XFER;
+	task.tfRegister[IDE_NSECTOR_OFFSET]	= arg;
+	task.command_type			= IDE_DRIVE_TASK_NO_DATA;
+	task.data_phase				= TASKFILE_NO_DATA;
+	task.handler				= task_no_data_intr;
+
+	err = ide_raw_taskfile(drive, &task, NULL);
 
 	if (!err && arg) {
 		ide_set_xfer_rate(drive, (u8) arg);

Re: [PATCH 2.6.11-rc3 10/11] ide: make ide_cmd_ioctl() use TASKFILE

[Tejun Heo]

10_ide_taskfile_cmd_ioctl.patch

	ide_cmd_ioctl() rewritten to use taskfile transport.  This is
	the last user of REQ_DRIVE_CMD.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 drivers/ide/ide-taskfile.c |  118 ++++++++++++++++++++++++---------------------
 include/linux/ide.h        |   12 ----
 2 files changed, 67 insertions(+), 63 deletions(-)

Index: linux-ide/drivers/ide/ide-taskfile.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-taskfile.c	2005-02-10 17:38:01.974766393 +0900
+++ linux-ide/drivers/ide/ide-taskfile.c	2005-02-10 17:38:02.621657912 +0900
@@ -667,78 +667,90 @@ abort:
 	return err;
 }
 
-int ide_wait_cmd (ide_drive_t *drive, u8 cmd, u8 nsect, u8 feature, u8 sectors, u8 *buf)
+int ide_cmd_ioctl(ide_drive_t *drive, unsigned int cmd, unsigned long arg)
 {
-	struct request rq;
-	u8 buffer[4];
-
-	if (!buf)
-		buf = buffer;
-	memset(buf, 0, 4 + SECTOR_WORDS * 4 * sectors);
-	ide_init_drive_cmd(&rq);
-	rq.flags = REQ_DRIVE_CMD;
-	rq.buffer = buf;
-	*buf++ = cmd;
-	*buf++ = nsect;
-	*buf++ = feature;
-	*buf++ = sectors;
-	return ide_do_drive_cmd(drive, &rq, ide_wait);
-}
-
-/*
- * FIXME : this needs to map into at taskfile. <andre@linux-ide.org>
- */
-int ide_cmd_ioctl (ide_drive_t *drive, unsigned int cmd, unsigned long arg)
-{
-	int err = 0;
-	u8 args[4], *argbuf = args;
-	u8 xfer_rate = 0;
-	int argsize = 4;
-	ide_task_t tfargs;
+	void __user *p = (void __user *)arg;
+	u8 args[4];
+	ide_task_t task;
+	ide_reg_valid_t *out_valid = &task.tf_out_flags;
+	ide_reg_valid_t *in_valid = &task.tf_in_flags;
+	task_ioreg_t *regs = task.tfRegister;
+	void *buf = NULL;
+	int xfer_rate = 0, in_size, err;
 
-	if (NULL == (void *) arg) {
+	if (p == NULL) {
 		struct request rq;
 		ide_init_drive_cmd(&rq);
 		return ide_do_drive_cmd(drive, &rq, ide_wait);
 	}
 
-	if (copy_from_user(args, (void __user *)arg, 4))
+	if (copy_from_user(args, p, 4))
 		return -EFAULT;
 
-	memset(&tfargs, 0, sizeof(ide_task_t));
-	tfargs.tfRegister[IDE_FEATURE_OFFSET] = args[2];
-	tfargs.tfRegister[IDE_NSECTOR_OFFSET] = args[3];
-	tfargs.tfRegister[IDE_SECTOR_OFFSET]  = args[1];
-	tfargs.tfRegister[IDE_LCYL_OFFSET]    = 0x00;
-	tfargs.tfRegister[IDE_HCYL_OFFSET]    = 0x00;
-	tfargs.tfRegister[IDE_SELECT_OFFSET]  = 0x00;
-	tfargs.tfRegister[IDE_COMMAND_OFFSET] = args[0];
-
-	if (args[3]) {
-		argsize = 4 + (SECTOR_WORDS * 4 * args[3]);
-		argbuf = kmalloc(argsize, GFP_KERNEL);
-		if (argbuf == NULL)
-			return -ENOMEM;
-		memcpy(argbuf, args, 4);
-	}
-	if (set_transfer(drive, &tfargs)) {
+	memset(&task, 0, sizeof(task));
+
+	out_valid->b.status_command	= 1;
+	out_valid->b.sector		= 1;
+	out_valid->b.error_feature	= 1;
+	out_valid->b.nsector		= 1;
+
+	in_valid->b.status_command	= 1;
+	in_valid->b.error_feature	= 1;
+	in_valid->b.nsector		= 1;
+
+	regs[IDE_COMMAND_OFFSET]	= args[0];
+	regs[IDE_SECTOR_OFFSET]		= args[1];
+	regs[IDE_FEATURE_OFFSET]	= args[2];
+	regs[IDE_NSECTOR_OFFSET]	= args[3];
+
+	if (set_transfer(drive, &task)) {
 		xfer_rate = args[1];
-		if (ide_ata66_check(drive, &tfargs))
-			goto abort;
+		if (ide_ata66_check(drive, &task))
+			return -EIO;
 	}
 
-	err = ide_wait_cmd(drive, args[0], args[1], args[2], args[3], argbuf);
+	/* SMART needs its secret keys in lcyl and hcyl registers. */
+	if (regs[IDE_COMMAND_OFFSET] == WIN_SMART) {
+		out_valid->b.lcyl	= 1;
+		out_valid->b.hcyl	= 1;
+		regs[IDE_LCYL_OFFSET]	= SMART_LCYL_PASS;
+		regs[IDE_HCYL_OFFSET]	= SMART_HCYL_PASS;
+	}
+
+	in_size = 4 * SECTOR_WORDS * regs[IDE_NSECTOR_OFFSET];
+
+	if (!in_size) {
+		task.command_type		= IDE_DRIVE_TASK_NO_DATA;
+		task.data_phase			= TASKFILE_NO_DATA;
+		task.handler			= &task_no_data_intr;
+	} else {
+		task.command_type		= IDE_DRIVE_TASK_IN;
+		task.data_phase			= TASKFILE_IN;
+		task.handler			= &task_in_intr;
+		task.flags		       |= ATA_TFLAG_IO_16BIT;
+
+		if ((buf = kmalloc(in_size, GFP_KERNEL)) == NULL)
+			return -ENOMEM;
+		memset(buf, 0, in_size);	/* paranoia */
+	}
+
+	err = ide_diag_taskfile(drive, &task, in_size, buf);
 
 	if (!err && xfer_rate) {
 		/* active-retuning-calls future */
 		ide_set_xfer_rate(drive, xfer_rate);
 		ide_driveid_update(drive);
 	}
-abort:
-	if (copy_to_user((void __user *)arg, argbuf, argsize))
+
+	args[0] = regs[IDE_STATUS_OFFSET];
+	args[1] = regs[IDE_ERROR_OFFSET];
+	args[2] = regs[IDE_NSECTOR_OFFSET];
+	args[3] = 0;
+
+	if (copy_to_user(p, args, 4) || copy_to_user(p + 4, buf, in_size))
 		err = -EFAULT;
-	if (argsize > 4)
-		kfree(argbuf);
+
+	kfree(buf);
 	return err;
 }
 
Index: linux-ide/include/linux/ide.h
===================================================================
--- linux-ide.orig/include/linux/ide.h	2005-02-10 17:38:01.745804793 +0900
+++ linux-ide/include/linux/ide.h	2005-02-10 17:38:02.622657745 +0900
@@ -1269,14 +1269,6 @@ extern int ide_do_drive_cmd(ide_drive_t 
  */
 extern void ide_end_drive_cmd(ide_drive_t *, u8, u8);
 
-/*
- * Issue ATA command and wait for completion.
- * Use for implementing commands in kernel
- *
- *  (ide_drive_t *drive, u8 cmd, u8 nsect, u8 feature, u8 sectors, u8 *buf)
- */
-extern int ide_wait_cmd(ide_drive_t *, u8, u8, u8, u8, u8 *);
-
 extern u32 ide_read_24(ide_drive_t *);
 
 extern void SELECT_DRIVE(ide_drive_t *);
@@ -1314,10 +1306,10 @@ int ide_task_ioctl(ide_drive_t *, unsign
 extern int system_bus_clock(void);
 
 extern int ide_driveid_update(ide_drive_t *);
-extern int ide_ata66_check(ide_drive_t *, ide_task_t *);
+int ide_ata66_check(ide_drive_t *, ide_task_t *);
 extern int ide_config_drive_speed(ide_drive_t *, u8);
 extern u8 eighty_ninty_three (ide_drive_t *);
-extern int set_transfer(ide_drive_t *, ide_task_t *);
+int set_transfer(ide_drive_t *, ide_task_t *);
 extern int taskfile_lib_get_identify(ide_drive_t *drive, u8 *);
 
 extern int ide_wait_not_busy(ide_hwif_t *hwif, unsigned long timeout);

Re: [PATCH 2.6.11-rc3 11/11] ide: remove REQ_DRIVE_CMD handling

[Tejun Heo]

11_ide_remove_cmd.patch

	Unused REQ_DRIVE_CMD handling removed.

Signed-off-by: Tejun Heo <htejun@gmail.com>

 drivers/block/ll_rw_blk.c |    1 
 drivers/ide/ide-io.c      |  181 ++++++++++------------------------------------
 drivers/ide/ide-lib.c     |    8 --
 include/linux/blkdev.h    |    2 
 4 files changed, 43 insertions(+), 149 deletions(-)

Index: linux-ide/drivers/block/ll_rw_blk.c
===================================================================
--- linux-ide.orig/drivers/block/ll_rw_blk.c	2005-02-10 17:38:02.165734368 +0900
+++ linux-ide/drivers/block/ll_rw_blk.c	2005-02-10 17:38:02.831622702 +0900
@@ -850,7 +850,6 @@ static char *rq_flags[] = {
 	"REQ_FAILED",
 	"REQ_QUIET",
 	"REQ_SPECIAL",
-	"REQ_DRIVE_CMD",
 	"REQ_DRIVE_TASKFILE",
 	"REQ_PREEMPT",
 	"REQ_PM_SUSPEND",
Index: linux-ide/drivers/ide/ide-io.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-io.c	2005-02-10 17:38:02.162734871 +0900
+++ linux-ide/drivers/ide/ide-io.c	2005-02-10 17:38:02.829623038 +0900
@@ -443,17 +443,7 @@ void ide_end_drive_cmd (ide_drive_t *dri
 	rq = HWGROUP(drive)->rq;
 	spin_unlock_irqrestore(&ide_lock, flags);
 
-	if (rq->flags & REQ_DRIVE_CMD) {
-		u8 *args = (u8 *) rq->buffer;
-		if (rq->errors == 0)
-			rq->errors = !OK_STAT(stat,READY_STAT,BAD_STAT);
-
-		if (args) {
-			args[0] = stat;
-			args[1] = err;
-			args[2] = hwif->INB(IDE_NSECTOR_REG);
-		}
-	} else if (rq->flags & REQ_DRIVE_TASKFILE) {
+	if (rq->flags & REQ_DRIVE_TASKFILE) {
 		ide_task_t *args = (ide_task_t *) rq->special;
 		if (rq->errors == 0)
 			rq->errors = !OK_STAT(stat,READY_STAT,BAD_STAT);
@@ -652,7 +642,7 @@ ide_startstop_t ide_error (ide_drive_t *
 		return ide_stopped;
 
 	/* retry only "normal" I/O: */
-	if (rq->flags & (REQ_DRIVE_CMD | REQ_DRIVE_TASKFILE)) {
+	if (rq->flags & REQ_DRIVE_TASKFILE) {
 		rq->errors = 1;
 		ide_end_drive_cmd(drive, stat, err);
 		return ide_stopped;
@@ -703,7 +693,7 @@ ide_startstop_t ide_abort(ide_drive_t *d
 		return ide_stopped;
 
 	/* retry only "normal" I/O: */
-	if (rq->flags & (REQ_DRIVE_CMD | REQ_DRIVE_TASKFILE)) {
+	if (rq->flags & REQ_DRIVE_TASKFILE) {
 		rq->errors = 1;
 		ide_end_drive_cmd(drive, BUSY_STAT, 0);
 		return ide_stopped;
@@ -718,63 +708,6 @@ ide_startstop_t ide_abort(ide_drive_t *d
 		return __ide_abort(drive, rq);
 }
 
-/**
- *	ide_cmd		-	issue a simple drive command
- *	@drive: drive the command is for
- *	@cmd: command byte
- *	@nsect: sector byte
- *	@handler: handler for the command completion
- *
- *	Issue a simple drive command with interrupts.
- *	The drive must be selected beforehand.
- */
-
-static void ide_cmd (ide_drive_t *drive, u8 cmd, u8 nsect,
-		ide_handler_t *handler)
-{
-	ide_hwif_t *hwif = HWIF(drive);
-	if (IDE_CONTROL_REG)
-		hwif->OUTB(drive->ctl,IDE_CONTROL_REG);	/* clear nIEN */
-	SELECT_MASK(drive,0);
-	hwif->OUTB(nsect,IDE_NSECTOR_REG);
-	ide_execute_command(drive, cmd, handler, WAIT_CMD, NULL);
-}
-
-/**
- *	drive_cmd_intr		- 	drive command completion interrupt
- *	@drive: drive the completion interrupt occurred on
- *
- *	drive_cmd_intr() is invoked on completion of a special DRIVE_CMD.
- *	We do any necessary daya reading and then wait for the drive to
- *	go non busy. At that point we may read the error data and complete
- *	the request
- */
- 
-static ide_startstop_t drive_cmd_intr (ide_drive_t *drive)
-{
-	struct request *rq = HWGROUP(drive)->rq;
-	ide_hwif_t *hwif = HWIF(drive);
-	u8 *args = (u8 *) rq->buffer;
-	u8 stat = hwif->INB(IDE_STATUS_REG);
-	int retries = 10;
-
-	local_irq_enable();
-	if ((stat & DRQ_STAT) && args && args[3]) {
-		u8 io_32bit = drive->io_32bit;
-		drive->io_32bit = 0;
-		hwif->ata_input_data(drive, &args[4], args[3] * SECTOR_WORDS);
-		drive->io_32bit = io_32bit;
-		while (((stat = hwif->INB(IDE_STATUS_REG)) & BUSY_STAT) && retries--)
-			udelay(100);
-	}
-
-	if (!OK_STAT(stat, READY_STAT, BAD_STAT))
-		return ide_error(drive, "drive_cmd", stat);
-		/* calls ide_end_drive_cmd */
-	ide_end_drive_cmd(drive, stat, hwif->INB(IDE_ERROR_REG));
-	return ide_stopped;
-}
-
 static void ide_init_specify_cmd(ide_drive_t *drive, ide_task_t *task)
 {
 	task->tfRegister[IDE_NSECTOR_OFFSET] = drive->sect;
@@ -894,80 +827,52 @@ void ide_init_sg_cmd(ide_drive_t *drive,
 EXPORT_SYMBOL_GPL(ide_init_sg_cmd);
 
 /**
- *	execute_drive_command	-	issue special drive command
+ *	execute_drive_taskfile	-	issue special drive command
  *	@drive: the drive to issue th command on
  *	@rq: the request structure holding the command
  *
- *	execute_drive_cmd() issues a special drive command,  usually 
- *	initiated by ioctl() from the external hdparm program. The
- *	command can be a drive command, drive task or taskfile 
- *	operation. Weirdly you can call it with NULL to wait for
- *	all commands to finish. Don't do this as that is due to change
+ *	execute_drive_taskfile() issues a special drive command,
+ *	usually initiated by ioctl() from the external hdparm program.
+ *	Weirdly you can call it with NULL to wait for all commands to
+ *	finish. Don't do this as that is due to change
  */
 
-static ide_startstop_t execute_drive_cmd (ide_drive_t *drive,
-		struct request *rq)
+static ide_startstop_t execute_drive_taskfile (ide_drive_t *drive,
+					       struct request *rq)
 {
 	ide_hwif_t *hwif = HWIF(drive);
-	if (rq->flags & REQ_DRIVE_TASKFILE) {
- 		ide_task_t *args = rq->special;
- 
-		if (!args)
-			goto done;
-
-		hwif->data_phase = args->data_phase;
-
-		switch (hwif->data_phase) {
-		case TASKFILE_MULTI_OUT:
-		case TASKFILE_OUT:
-		case TASKFILE_MULTI_IN:
-		case TASKFILE_IN:
-			ide_init_sg_cmd(drive, rq);
-			ide_map_sg(drive, rq);
-		default:
-			break;
-		}
-
-		if (args->tf_out_flags.all != 0) 
-			return flagged_taskfile(drive, args);
-		return do_rw_taskfile(drive, args);
-	} else if (rq->flags & REQ_DRIVE_CMD) {
- 		u8 *args = rq->buffer;
-
-		if (!args)
-			goto done;
-#ifdef DEBUG
- 		printk("%s: DRIVE_CMD ", drive->name);
- 		printk("cmd=0x%02x ", args[0]);
- 		printk("sc=0x%02x ", args[1]);
- 		printk("fr=0x%02x ", args[2]);
- 		printk("xx=0x%02x\n", args[3]);
-#endif
- 		if (args[0] == WIN_SMART) {
- 			hwif->OUTB(0x4f, IDE_LCYL_REG);
- 			hwif->OUTB(0xc2, IDE_HCYL_REG);
- 			hwif->OUTB(args[2],IDE_FEATURE_REG);
- 			hwif->OUTB(args[1],IDE_SECTOR_REG);
- 			ide_cmd(drive, args[0], args[3], &drive_cmd_intr);
- 			return ide_started;
- 		}
- 		hwif->OUTB(args[2],IDE_FEATURE_REG);
- 		ide_cmd(drive, args[0], args[1], &drive_cmd_intr);
- 		return ide_started;
- 	}
-
-done:
- 	/*
- 	 * NULL is actually a valid way of waiting for
- 	 * all current requests to be flushed from the queue.
- 	 */
+	ide_task_t *task = rq->special;
+	
+	if (!task) {
+		/*
+		 * NULL is actually a valid way of waiting for
+		 * all current requests to be flushed from the queue.
+		 */
 #ifdef DEBUG
- 	printk("%s: DRIVE_CMD (null)\n", drive->name);
+		printk("%s: DRIVE_TASKFILE (null)\n", drive->name);
 #endif
- 	ide_end_drive_cmd(drive,
-			hwif->INB(IDE_STATUS_REG),
-			hwif->INB(IDE_ERROR_REG));
- 	return ide_stopped;
+		ide_end_drive_cmd(drive,
+				  hwif->INB(IDE_STATUS_REG),
+				  hwif->INB(IDE_ERROR_REG));
+		return ide_stopped;
+	}
+
+	hwif->data_phase = task->data_phase;
+
+	switch (hwif->data_phase) {
+	case TASKFILE_MULTI_OUT:
+	case TASKFILE_OUT:
+	case TASKFILE_MULTI_IN:
+	case TASKFILE_IN:
+		ide_init_sg_cmd(drive, rq);
+		ide_map_sg(drive, rq);
+	default:
+		break;
+	}
+
+	if (task->tf_out_flags.all != 0) 
+		return flagged_taskfile(drive, task);
+	return do_rw_taskfile(drive, task);
 }
 
 /**
@@ -1044,10 +949,8 @@ static ide_startstop_t start_request (id
 	if (!drive->special.all) {
 		ide_driver_t *drv;
 
-		if (rq->flags & REQ_DRIVE_CMD)
-			return execute_drive_cmd(drive, rq);
-		else if (rq->flags & REQ_DRIVE_TASKFILE)
-			return execute_drive_cmd(drive, rq);
+		if (rq->flags & REQ_DRIVE_TASKFILE)
+			return execute_drive_taskfile(drive, rq);
 		else if (blk_pm_request(rq)) {
 #ifdef DEBUG_PM
 			printk("%s: start_power_step(step: %d)\n",
Index: linux-ide/drivers/ide/ide-lib.c
===================================================================
--- linux-ide.orig/drivers/ide/ide-lib.c	2005-02-10 17:38:02.163734703 +0900
+++ linux-ide/drivers/ide/ide-lib.c	2005-02-10 17:38:02.830622870 +0900
@@ -458,13 +458,7 @@ static void ide_dump_opcode(ide_drive_t 
 	spin_unlock(&ide_lock);
 	if (!rq)
 		return;
-	if (rq->flags & REQ_DRIVE_CMD) {
-		char *args = rq->buffer;
-		if (args) {
-			opcode = args[0];
-			found = 1;
-		}
-	} else if (rq->flags & REQ_DRIVE_TASKFILE) {
+	if (rq->flags & REQ_DRIVE_TASKFILE) {
 		ide_task_t *args = rq->special;
 		if (args) {
 			task_struct_t *tf = (task_struct_t *) args->tfRegister;
Index: linux-ide/include/linux/blkdev.h
===================================================================
--- linux-ide.orig/include/linux/blkdev.h	2005-02-10 17:38:02.163734703 +0900
+++ linux-ide/include/linux/blkdev.h	2005-02-10 17:38:02.829623038 +0900
@@ -201,7 +201,6 @@ enum rq_flag_bits {
 	__REQ_FAILED,		/* set if the request failed */
 	__REQ_QUIET,		/* don't worry about errors */
 	__REQ_SPECIAL,		/* driver suplied command */
-	__REQ_DRIVE_CMD,
 	__REQ_DRIVE_TASKFILE,
 	__REQ_PREEMPT,		/* set for "ide_preempt" requests */
 	__REQ_PM_SUSPEND,	/* suspend request */
@@ -227,7 +226,6 @@ enum rq_flag_bits {
 #define REQ_FAILED	(1 << __REQ_FAILED)
 #define REQ_QUIET	(1 << __REQ_QUIET)
 #define REQ_SPECIAL	(1 << __REQ_SPECIAL)
-#define REQ_DRIVE_CMD	(1 << __REQ_DRIVE_CMD)
 #define REQ_DRIVE_TASKFILE	(1 << __REQ_DRIVE_TASKFILE)
 #define REQ_PREEMPT	(1 << __REQ_PREEMPT)
 #define REQ_PM_SUSPEND	(1 << __REQ_PM_SUSPEND)

Re: [PATCH 2.6.11-rc3 05/11] ide: fixes io_32bit race in ide_taskfile_ioctl()

[Bartlomiej Zolnierkiewicz]

applied to ide-dev-2.6,  thanks

I need some more time for the other patches

Re: [PATCH 2.6.11-rc3 08/11] ide: remove REQ_DRIVE_TASK handling

[Bartlomiej Zolnierkiewicz]

> @@ -945,33 +931,7 @@ static ide_startstop_t execute_drive_cmd
>                 if (args->tf_out_flags.all != 0)
>                         return flagged_taskfile(drive, args);
>                 return do_rw_taskfile(drive, args);
> -       } else if (rq->flags & REQ_DRIVE_TASK) {
> -               u8 *args = rq->buffer;
> -               u8 sel;
> -
> -               if (!args)
> -                       goto done;
> -#ifdef DEBUG
> -               printk("%s: DRIVE_TASK_CMD ", drive->name);
> -               printk("cmd=0x%02x ", args[0]);
> -               printk("fr=0x%02x ", args[1]);
> -               printk("ns=0x%02x ", args[2]);
> -               printk("sc=0x%02x ", args[3]);
> -               printk("lcyl=0x%02x ", args[4]);
> -               printk("hcyl=0x%02x ", args[5]);
> -               printk("sel=0x%02x\n", args[6]);
> -#endif
> -               hwif->OUTB(args[1], IDE_FEATURE_REG);
> -               hwif->OUTB(args[3], IDE_SECTOR_REG);
> -               hwif->OUTB(args[4], IDE_LCYL_REG);
> -               hwif->OUTB(args[5], IDE_HCYL_REG);
> -               sel = (args[6] & ~0x10);
> -               if (drive->select.b.unit)
> -                       sel |= 0x10;

do_rw_taskfile() always sets LBA bit in Device register for LBA
capable disks, so converting HDIO_DRIVE_TASK to use taskfile
transport removes (theoretical?) possibility of sending CHS
commands to LBA capable disks.

This issue is addressed in my patch series.

Re: [PATCH 2.6.11-rc3 10/11] ide: make ide_cmd_ioctl() use TASKFILE

[Bartlomiej Zolnierkiewicz]

> +       in_valid->b.status_command      = 1;
> +       in_valid->b.error_feature       = 1;
> +       in_valid->b.nsector             = 1;

ide_end_drive_cmd() must be fixed first to respect ->tf_in_flags
and it must be done *without* affecting HDIO_DRIVE_TASKFILE.

>  extern int ide_driveid_update(ide_drive_t *);
> -extern int ide_ata66_check(ide_drive_t *, ide_task_t *);
> +int ide_ata66_check(ide_drive_t *, ide_task_t *);
>  extern int ide_config_drive_speed(ide_drive_t *, u8);
>  extern u8 eighty_ninty_three (ide_drive_t *);
> -extern int set_transfer(ide_drive_t *, ide_task_t *);
> +int set_transfer(ide_drive_t *, ide_task_t *);
>  extern int taskfile_lib_get_identify(ide_drive_t *drive, u8 *);

leftovers from previous version of the patch

Re: [PATCH 2.6.11-rc3 01/11] ide: task_end_request() fix

[Bartlomiej Zolnierkiewicz]

On Thu, 10 Feb 2005 17:38:14 +0900 (KST), Tejun Heo <htejun@gmail.com> wrote:
> 
> 01_ide_task_end_request_fix.patch
> 
>         task_end_request() modified to always call ide_end_drive_cmd()
>         for taskfile requests.  Previously, ide_end_drive_cmd() was
>         called only when task->tf_out_flags.all was set.  Also,
>         ide_dma_intr() is modified to use task_end_request().
> 
>         * fixes taskfile ioctl oops bug which was caused by referencing
>           NULL rq->rq_disk of taskfile requests.

I fixed it in slightly different way in ide-dev-2.6 - by calling
ide_end_request() instead of ->end_request().

>         * enables TASKFILE ioctls to get valid register outputs on
>           successful completion.

This change makes *all* taskfile registers to be read on completion
of *any* command.  Currently this is done only for flagged taskfiles
and commands using no-data protocol.

With all your changes it will be also done for:
* HDIO_DRIVE_[TASKFILE,CMD] ioctls
* /proc/ide/hd?/{identify,smart_thresholds,smart_values}
but reading back all registers is not always needed.

It is already bad enough (and we can't fix it cause it is exported
to user-space through HDIO_DRIVE_TASKFILE), we shouldn't
make it worse.

Re: [PATCH 2.6.11-rc3 01/11] ide: task_end_request() fix

[Tejun Heo]

On Thu, Feb 24, 2005 at 04:58:03PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Thu, 10 Feb 2005 17:38:14 +0900 (KST), Tejun Heo <htejun@gmail.com> wrote:
> > 
> > 01_ide_task_end_request_fix.patch
> > 
> >         task_end_request() modified to always call ide_end_drive_cmd()
> >         for taskfile requests.  Previously, ide_end_drive_cmd() was
> >         called only when task->tf_out_flags.all was set.  Also,
> >         ide_dma_intr() is modified to use task_end_request().
> > 
> >         * fixes taskfile ioctl oops bug which was caused by referencing
> >           NULL rq->rq_disk of taskfile requests.
> 
> I fixed it in slightly different way in ide-dev-2.6 - by calling
> ide_end_request() instead of ->end_request().

 Taskfile DMA path is still broken.  Also calling ide_end_request()
will work there, but IMHO it's just cleaner to finish special commands
inside ide_end_drive_cmd().  Currently,

 * Successful flagged taskfile			-> ide_end_drive_cmd()
 * All other successful non-DMA special cmds	-> ide_end_request()
 * Successful DMA taskfile			-> segfault
 * All failed special cmds			-> ide_end_drive_cmd()

 It just shouldn't be like this.  :-(

> 
> >         * enables TASKFILE ioctls to get valid register outputs on
> >           successful completion.
> 
> This change makes *all* taskfile registers to be read on completion
> of *any* command.  Currently this is done only for flagged taskfiles
> and commands using no-data protocol.
> 
> With all your changes it will be also done for:
> * HDIO_DRIVE_[TASKFILE,CMD] ioctls
> * /proc/ide/hd?/{identify,smart_thresholds,smart_values}
> but reading back all registers is not always needed.

 None is on a hot path or even near to one, but maybe I don't have
enough experience with old hardware.  Are there some old hardware
which make the additional reads stand out?

> It is already bad enough (and we can't fix it cause it is exported
> to user-space through HDIO_DRIVE_TASKFILE), we shouldn't
> make it worse.

 Yeah, the whole IDE ioctl interface seems disturbingly messy. :-(

 * Register output is available only if
	1. The command fails.
	2. The command is a flagged taskfile.
 * taskfile->device_head is used regardless of outflags setting.
 * In flagged taskfile, taskfile->device_head can turn on the device
   bit, so we can issue commands to hdb with permissions to hda.
 * In TASK and TASKFILE, LBA commands can be issued to drives in CHS
   mode but the reverse isn't true.  However, in TASKFILE, if the
   command isn't flagged, the lower nibble of device register is
   zeroed depending on addressing setting.
 * taskfile->data endianess is reversed on big endian machines.
 * ide_reg_valid_t endianess issue.
 * And, none of above is documented.

 So, I don't know.  Do you think we should keep all of the above
behaviors?  Please let me know; then, I'll update ioctl/hdio.txt so
that people can at least know these gotchas.

 Thanks.

-- 
tejun

Re: [PATCH 2.6.11-rc3 10/11] ide: make ide_cmd_ioctl() use TASKFILE

[Tejun Heo]

 Hello, Bartlomiej.

On Thu, Feb 24, 2005 at 04:50:39PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > +       in_valid->b.status_command      = 1;
> > +       in_valid->b.error_feature       = 1;
> > +       in_valid->b.nsector             = 1;
> 
> ide_end_drive_cmd() must be fixed first to respect ->tf_in_flags
> and it must be done *without* affecting HDIO_DRIVE_TASKFILE.

 Hmmm... does it make any difference other than reading more
registers?  Are you worried about performance impacts?  Is there
hardware which acts differently if more registers are read?

> 
> >  extern int ide_driveid_update(ide_drive_t *);
> > -extern int ide_ata66_check(ide_drive_t *, ide_task_t *);
> > +int ide_ata66_check(ide_drive_t *, ide_task_t *);
> >  extern int ide_config_drive_speed(ide_drive_t *, u8);
> >  extern u8 eighty_ninty_three (ide_drive_t *);
> > -extern int set_transfer(ide_drive_t *, ide_task_t *);
> > +int set_transfer(ide_drive_t *, ide_task_t *);
> >  extern int taskfile_lib_get_identify(ide_drive_t *drive, u8 *);
> 
> leftovers from previous version of the patch

 Yeah, sorry.

 Thanks.

-- 
tejun

Re: [PATCH 2.6.11-rc3 10/11] ide: make ide_cmd_ioctl() use TASKFILE

[Jeff Garzik]

For what it's worth...

Some vendor-specific commands on PATA devices require -exact- 
specification of registers in, and registers out.  You never want to 
read more registers than are flagged.  Ditto for write.

	Jeff

Re: [PATCH 2.6.11-rc3 01/11] ide: task_end_request() fix

[Bartlomiej Zolnierkiewicz]

On Sun, 27 Feb 2005 15:49:22 +0900, Tejun Heo <htejun@gmail.com> wrote:
> On Thu, Feb 24, 2005 at 04:58:03PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Thu, 10 Feb 2005 17:38:14 +0900 (KST), Tejun Heo <htejun@gmail.com> wrote:
> > >
> > > 01_ide_task_end_request_fix.patch
> > >
> > >         task_end_request() modified to always call ide_end_drive_cmd()
> > >         for taskfile requests.  Previously, ide_end_drive_cmd() was
> > >         called only when task->tf_out_flags.all was set.  Also,
> > >         ide_dma_intr() is modified to use task_end_request().
> > >
> > >         * fixes taskfile ioctl oops bug which was caused by referencing
> > >           NULL rq->rq_disk of taskfile requests.
> >
> > I fixed it in slightly different way in ide-dev-2.6 - by calling
> > ide_end_request() instead of ->end_request().
> 
>  Taskfile DMA path is still broken.  Also calling ide_end_request()
> will work there, but IMHO it's just cleaner to finish special commands
> inside ide_end_drive_cmd().  Currently,

I agree but please note that your patch makes *all* taskfile registers to
be exposed through HDIO_DRIVE_TASKFILE regardless of ->rf_in_flags
(and obviously later you can't revert this change).

>  * Successful flagged taskfile                  -> ide_end_drive_cmd()
>  * All other successful non-DMA special cmds    -> ide_end_request()
>  * Successful DMA taskfile                      -> segfault

Have you tested it?  Why would it segfault?

>  * All failed special cmds                      -> ide_end_drive_cmd()
> 
>  It just shouldn't be like this.  :-(

Yep.

> >
> > >         * enables TASKFILE ioctls to get valid register outputs on
> > >           successful completion.
> >
> > This change makes *all* taskfile registers to be read on completion
> > of *any* command.  Currently this is done only for flagged taskfiles
> > and commands using no-data protocol.
> >
> > With all your changes it will be also done for:
> > * HDIO_DRIVE_[TASKFILE,CMD] ioctls
> > * /proc/ide/hd?/{identify,smart_thresholds,smart_values}
> > but reading back all registers is not always needed.
> 
>  None is on a hot path or even near to one, but maybe I don't have
> enough experience with old hardware.  Are there some old hardware
> which make the additional reads stand out?

I dunno, better be safe then sorry, after all we are working
on the *stable* kernel tree.

Converting HDIO_DRIVE_CMD and in-kernel users to
soon-to-be-implemented ;-) sane discrete interface shouldn't
require much effort.

> > It is already bad enough (and we can't fix it cause it is exported
> > to user-space through HDIO_DRIVE_TASKFILE), we shouldn't
> > make it worse.
> 
>  Yeah, the whole IDE ioctl interface seems disturbingly messy. :-(
> 
>  * Register output is available only if
>         1. The command fails.
>         2. The command is a flagged taskfile.

3. the command uses no-data protocol

>  * taskfile->device_head is used regardless of outflags setting.
>  * In flagged taskfile, taskfile->device_head can turn on the device
>    bit, so we can issue commands to hdb with permissions to hda.
>  * In TASK and TASKFILE, LBA commands can be issued to drives in CHS
>    mode but the reverse isn't true.  However, in TASKFILE, if the
>    command isn't flagged, the lower nibble of device register is
>    zeroed depending on addressing setting.
>  * taskfile->data endianess is reversed on big endian machines.
>  * ide_reg_valid_t endianess issue.
>  * And, none of above is documented.
> 
>  So, I don't know.  Do you think we should keep all of the above
> behaviors?  Please let me know; then, I'll update ioctl/hdio.txt so

Now read again the list above and think about amount of time required
to *safely* fix all the issues mentioned vs introducing sane interface.
Please also note that once we expose something to user-space
applications may depend on the broken behavior so we can't fix all
issues anyway.

If somebody implements SG_IO ioctl and SCSI command pass-through
from libata for IDE driver (and add possibility for discrete taskfiles), we can
just deprecate HDIO_DRIVE_TASKFILE, forget about it and some time later
remove this FPOS.

> that people can at least know these gotchas.

Please do so, thanks.

Re: [PATCH 2.6.11-rc3 01/11] ide: task_end_request() fix

[Tejun Heo]

 Hello, Bartlomiej.

On Tue, Mar 01, 2005 at 03:30:32PM +0100, Bartlomiej Zolnierkiewicz wrote:
> On Sun, 27 Feb 2005 15:49:22 +0900, Tejun Heo <htejun@gmail.com> wrote:
> > 
> >  Taskfile DMA path is still broken.  Also calling ide_end_request()
> > will work there, but IMHO it's just cleaner to finish special commands
> > inside ide_end_drive_cmd().  Currently,
> 
> I agree but please note that your patch makes *all* taskfile registers to
> be exposed through HDIO_DRIVE_TASKFILE regardless of ->rf_in_flags
> (and obviously later you can't revert this change).
> 
> >  * Successful flagged taskfile                  -> ide_end_drive_cmd()
> >  * All other successful non-DMA special cmds    -> ide_end_request()
> >  * Successful DMA taskfile                      -> segfault
> 
> Have you tested it?  Why would it segfault?
> 

 It's the same reason why PIO taskfiles were broken.  rq->rq_disk is
NULL for taskfile requests.

ide_startstop_t ide_dma_intr (ide_drive_t *drive)
{
...
			printk("**HERE0***\n");
			drv = *(ide_driver_t **)rq->rq_disk->private_data;;
			printk("**HERE1***\n");
			drv->end_request(drive, 1, rq->nr_sectors);
			return ide_stopped;
...
}

 And, my test program does the following TASKFILE ioctl.

	memset(&task, 0, sizeof(task));
	task.req.out_size = sizeof(task.outbuf);
	task.req.in_size = sizeof(task.inbuf);

	task.req.io_ports[2] = 1;			/* nsector */
	task.req.io_ports[3] = sector;			/* LBA [7:0] */
	task.req.io_ports[4] = sector >> 8;		/* LBA [15:8] */
	task.req.io_ports[5] = sector >> 16;		/* LBA [23:16] */
	task.req.io_ports[6] = 0x40 | ((sector >> 24) & 0xf); /* select */
	task.req.io_ports[7] = WIN_READDMA;		/* command */
	task.req.req_cmd = IDE_DRIVE_TASK_IN;
	task.req.data_phase = TASKFILE_IN_DMA;

 Then, the kernel oopses...

**HERE0***
Unable to handle kernel NULL pointer dereference at virtual address 00000038
 printing eip:
c026bce4
*pde = 00000000
Oops: 0000 [#1]
PREEMPT SMP 
Modules linked in: nfs lockd sunrpc e1000 eepro100 e100 mii floppy
CPU:    0
EIP:    0060:[<c026bce4>]    Not tainted VLI
EFLAGS: 00010082   (2.6.11-rc3-work-i386) 
EIP is at ide_dma_intr+0x84/0xd0
eax: 00000000   ebx: f5e79d6c   ecx: c03a678c   edx: 00000001
esi: c048b3b0   edi: c048b3b0   ebp: c0435efc   esp: c0435ee0
ds: 007b   es: 007b   ss: 0068


> >  * All failed special cmds                      -> ide_end_drive_cmd()
> > 
> >  It just shouldn't be like this.  :-(
> 
> Yep.
> 
> > >
> > > >         * enables TASKFILE ioctls to get valid register outputs on
> > > >           successful completion.
> > >
> > > This change makes *all* taskfile registers to be read on completion
> > > of *any* command.  Currently this is done only for flagged taskfiles
> > > and commands using no-data protocol.
> > >
> > > With all your changes it will be also done for:
> > > * HDIO_DRIVE_[TASKFILE,CMD] ioctls
> > > * /proc/ide/hd?/{identify,smart_thresholds,smart_values}
> > > but reading back all registers is not always needed.
> > 
> >  None is on a hot path or even near to one, but maybe I don't have
> > enough experience with old hardware.  Are there some old hardware
> > which make the additional reads stand out?
> 
> I dunno, better be safe then sorry, after all we are working
> on the *stable* kernel tree.
> 
> Converting HDIO_DRIVE_CMD and in-kernel users to
> soon-to-be-implemented ;-) sane discrete interface shouldn't
> require much effort.
> 
> > > It is already bad enough (and we can't fix it cause it is exported
> > > to user-space through HDIO_DRIVE_TASKFILE), we shouldn't
> > > make it worse.
> > 
> >  Yeah, the whole IDE ioctl interface seems disturbingly messy. :-(
> > 
> >  * Register output is available only if
> >         1. The command fails.
> >         2. The command is a flagged taskfile.
> 
> 3. the command uses no-data protocol
> 

 Yes, that one. :-)

> >  * taskfile->device_head is used regardless of outflags setting.
> >  * In flagged taskfile, taskfile->device_head can turn on the device
> >    bit, so we can issue commands to hdb with permissions to hda.
> >  * In TASK and TASKFILE, LBA commands can be issued to drives in CHS
> >    mode but the reverse isn't true.  However, in TASKFILE, if the
> >    command isn't flagged, the lower nibble of device register is
> >    zeroed depending on addressing setting.
> >  * taskfile->data endianess is reversed on big endian machines.
> >  * ide_reg_valid_t endianess issue.
> >  * And, none of above is documented.
> > 
> >  So, I don't know.  Do you think we should keep all of the above
> > behaviors?  Please let me know; then, I'll update ioctl/hdio.txt so
> 
> Now read again the list above and think about amount of time required
> to *safely* fix all the issues mentioned vs introducing sane interface.
> Please also note that once we expose something to user-space
> applications may depend on the broken behavior so we can't fix all
> issues anyway.

 So, I guess we're keeping most of them.

> If somebody implements SG_IO ioctl and SCSI command pass-through
> from libata for IDE driver (and add possibility for discrete taskfiles), we can
> just deprecate HDIO_DRIVE_TASKFILE, forget about it and some time later
> remove this FPOS.

 That sounds sweet.

> > that people can at least know these gotchas.
> 
> Please do so, thanks.

 Yes, I'll do.

 Thanks.

-- 
tejun

Re: [PATCH 2.6.11-rc3 01/11] ide: task_end_request() fix

[Jeff Garzik]

Bartlomiej Zolnierkiewicz wrote:
> If somebody implements SG_IO ioctl and SCSI command pass-through
> from libata for IDE driver (and add possibility for discrete taskfiles), we can
> just deprecate HDIO_DRIVE_TASKFILE, forget about it and some time later
> remove this FPOS.

Can you explain what you mean by "add possibility for discrete taskfiles"?

	Jeff

Re: [PATCH 2.6.11-rc3 01/11] ide: task_end_request() fix

[Bartlomiej Zolnierkiewicz]

On Wed, 02 Mar 2005 00:56:18 -0500, Jeff Garzik <jgarzik@pobox.com> wrote:
> Bartlomiej Zolnierkiewicz wrote:
> > If somebody implements SG_IO ioctl and SCSI command pass-through
> > from libata for IDE driver (and add possibility for discrete taskfiles), we can
> > just deprecate HDIO_DRIVE_TASKFILE, forget about it and some time later
> > remove this FPOS.
> 
> Can you explain what you mean by "add possibility for discrete taskfiles"?

I mean flagged taskfiles.

Re: [PATCH 2.6.11-rc3 01/11] ide: task_end_request() fix

[Bartlomiej Zolnierkiewicz]

On Wed, 2 Mar 2005 01:49:52 +0900, Tejun Heo <htejun@gmail.com> wrote:
>  Hello, Bartlomiej.
> 
> On Tue, Mar 01, 2005 at 03:30:32PM +0100, Bartlomiej Zolnierkiewicz wrote:
> > On Sun, 27 Feb 2005 15:49:22 +0900, Tejun Heo <htejun@gmail.com> wrote:
> > >
> > >  Taskfile DMA path is still broken.  Also calling ide_end_request()
> > > will work there, but IMHO it's just cleaner to finish special commands
> > > inside ide_end_drive_cmd().  Currently,
> >
> > I agree but please note that your patch makes *all* taskfile registers to
> > be exposed through HDIO_DRIVE_TASKFILE regardless of ->rf_in_flags
> > (and obviously later you can't revert this change).
> >
> > >  * Successful flagged taskfile                  -> ide_end_drive_cmd()
> > >  * All other successful non-DMA special cmds    -> ide_end_request()
> > >  * Successful DMA taskfile                      -> segfault
> >
> > Have you tested it?  Why would it segfault?
> >
> 
>  It's the same reason why PIO taskfiles were broken.  rq->rq_disk is
> NULL for taskfile requests.
> 
> ide_startstop_t ide_dma_intr (ide_drive_t *drive)
> {
> ...
>                         printk("**HERE0***\n");
>                         drv = *(ide_driver_t **)rq->rq_disk->private_data;;
>                         printk("**HERE1***\n");
>                         drv->end_request(drive, 1, rq->nr_sectors);
>                         return ide_stopped;
> ...
> }

Arghh, indeed I forgot about HDIO_DRIVE_TASKFILE here.

Could you fix to check if (drv == NULL) and call
ide_end_request() it the condition is true?