linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH REPOST 2.6.11-rc2] ide: driver updates (phase 2)
@ 2005-02-05 10:25 Tejun Heo
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 02/09] ide: ide_init_drive_cmd() now defaults to REQ_DRIVE_TASKFILE Tejun Heo
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Tejun Heo @ 2005-02-05 10:25 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz, linux-ide, lkml

 This thread is repost of phase 2 ide driver updates.  Sorry again.

 Hello, Bartlomiej.

 These are reordered/modified/(hopefully)appliable nine patches from
the previous series of patches.  #01 is the only new one.  It kills
the unused pkt_task_t in ide.h.  #02/#03 are moved upward and #04 is
modified as you've requested.  #05/#08 now directly use taskfile
transport instead of calling ide_taskfile_ioctl().  #08 now also
properly handles WIN_SMART case.

 If these patches go in, I have eight patches left.  One of them is
ide_explicit_TASKFILE_NO_DATA.  As we've discussed, I'm going to add
default initialization of task->[pre]handler.  That patch wasn't
included in this series because I didn't know if do_rw_taskfile() and
flagged_taskfile() are going to be merged or not.  So, please tell
me what you think. :-)

 I'll talk about other patches in their reply threads.

[ Start of patch descriptions ]

01_ide_kill_pkt_task_t.patch
	: kill unused pkt_task_t

	Remove unused pkt_task_t definition from ide.h.

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.

03_ide_diag_taskfile_use_init_drive_cmd.patch
	: ide_diag_taskfile() rq initialization fix

	In ide_diag_taskfile(), when initializing taskfile rq,
	ref_count wasn't initialized properly.  Modified to use
	ide_init_drive_cmd().  This doesn't really change any behavior
	as the request isn't managed via the block layer.

04_ide_taskfile_flush.patch
	: convert REQ_DRIVE_TASK to REQ_DRIVE_TASKFILE

	All REQ_DRIVE_TASK users except ide_task_ioctl() converted
	to use REQ_DRIVE_TASKFILE.
	1. 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.
	2. ide_queue_flush_cmd() converted to use REQ_DRIVE_TASKFILE.

05_ide_taskfile_task_ioctl.patch
	: map ide_task_ioctl() to ide_taskfile_ioctl()

	ide_task_ioctl() modified to map to ide_taskfile_ioctl().
	This is the last user of REQ_DRIVE_TASK.

06_ide_remove_task.patch
	: remove REQ_DRIVE_TASK handling

	Unused REQ_DRIVE_TASK handling removed.

07_ide_taskfile_cmd.patch
	: convert 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.

08_ide_taskfile_cmd_ioctl.patch
	: map ide_cmd_ioctl() to ide_taskfile_ioctl()

	ide_cmd_ioctl() converted to use ide_taskfile_ioctl().  This
	is the last user of REQ_DRIVE_CMD.

09_ide_remove_cmd.patch
	: remove REQ_DRIVE_CMD handling

	Removed unused REQ_DRIVE_CMD handling.

[ End of patch descriptions ]

Thanks.

-- 
tejun


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2.6.11-rc2 01/09] ide: kill unused pkt_task_t
  2005-02-05 10:25 [PATCH REPOST 2.6.11-rc2] ide: driver updates (phase 2) Tejun Heo
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 02/09] ide: ide_init_drive_cmd() now defaults to REQ_DRIVE_TASKFILE Tejun Heo
@ 2005-02-05 10:28 ` Tejun Heo
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 03/09] ide: ide_diag_taskfile() rq initialization fix Tejun Heo
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tejun Heo @ 2005-02-05 10:28 UTC (permalink / raw)
  To: bzolnier, linux-ide, linux-kernel


01_ide_kill_pkt_task_t.patch

	Remove unused pkt_task_t definition from ide.h.


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-ide-series2-export/include/linux/ide.h
===================================================================
--- linux-ide-series2-export.orig/include/linux/ide.h	2005-02-05 19:26:51.369361863 +0900
+++ linux-ide-series2-export/include/linux/ide.h	2005-02-05 19:27:08.104643467 +0900
@@ -1279,20 +1279,6 @@ typedef struct ide_task_s {
 	void			*special;	/* valid_t generally */
 } ide_task_t;
 
-typedef struct pkt_task_s {
-/*
- *	struct hd_drive_task_hdr	pktf;
- *	task_struct_t		pktf;
- *	u8			pkcdb[12];
- */
-	task_ioreg_t		tfRegister[8];
-	int			data_phase;
-	int			command_type;
-	ide_handler_t		*handler;
-	struct request		*rq;		/* copy of request */
-	void			*special;
-} pkt_task_t;
-
 extern u32 ide_read_24(ide_drive_t *);
 
 extern void SELECT_DRIVE(ide_drive_t *);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2.6.11-rc2 02/09] ide: ide_init_drive_cmd() now defaults to REQ_DRIVE_TASKFILE
  2005-02-05 10:25 [PATCH REPOST 2.6.11-rc2] ide: driver updates (phase 2) Tejun Heo
@ 2005-02-05 10:28 ` Tejun Heo
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 01/09] ide: kill unused pkt_task_t Tejun Heo
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tejun Heo @ 2005-02-05 10:28 UTC (permalink / raw)
  To: bzolnier, linux-ide, linux-kernel


02_ide_taskfile_init_drive_cmd.patch

	ide_init_drive_cmd() now initializes rq->flags to
	REQ_DRIVE_TASKFILE.


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-ide-series2-export/drivers/ide/ide-io.c
===================================================================
--- linux-ide-series2-export.orig/drivers/ide/ide-io.c	2005-02-05 19:26:51.303372581 +0900
+++ linux-ide-series2-export/drivers/ide/ide-io.c	2005-02-05 19:27:08.308610336 +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;
 
@@ -1549,7 +1549,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-series2-export/drivers/ide/ide-taskfile.c
===================================================================
--- linux-ide-series2-export.orig/drivers/ide/ide-taskfile.c	2005-02-05 19:26:51.303372581 +0900
+++ linux-ide-series2-export/drivers/ide/ide-taskfile.c	2005-02-05 19:27:08.308610336 +0900
@@ -670,6 +670,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;

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2.6.11-rc2 04/09] ide: convert REQ_DRIVE_TASK to REQ_DRIVE_TASKFILE
  2005-02-05 10:25 [PATCH REPOST 2.6.11-rc2] ide: driver updates (phase 2) Tejun Heo
                   ` (3 preceding siblings ...)
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 06/09] ide: remove REQ_DRIVE_TASK handling Tejun Heo
@ 2005-02-05 10:28 ` Tejun Heo
  2005-02-06 19:08   ` Bartlomiej Zolnierkiewicz
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 07/09] ide: convert REQ_DRIVE_CMD " Tejun Heo
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Tejun Heo @ 2005-02-05 10:28 UTC (permalink / raw)
  To: bzolnier, linux-ide, linux-kernel


04_ide_taskfile_flush.patch

	All REQ_DRIVE_TASK users except ide_task_ioctl() converted
	to use REQ_DRIVE_TASKFILE.
	1. 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.
	2. ide_queue_flush_cmd() converted to use REQ_DRIVE_TASKFILE.


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-ide-series2-export/drivers/ide/ide-disk.c
===================================================================
--- linux-ide-series2-export.orig/drivers/ide/ide-disk.c	2005-02-05 19:26:51.166394831 +0900
+++ linux-ide-series2-export/drivers/ide/ide-disk.c	2005-02-05 19:27:08.735540988 +0900
@@ -705,24 +705,17 @@ static int idedisk_issue_flush(request_q
 {
 	ide_drive_t *drive = q->queuedata;
 	struct request *rq;
+	ide_task_t args;
 	int ret;
 
 	if (!drive->wcache)
 		return 0;
 
-	rq = blk_get_request(q, WRITE, __GFP_WAIT);
-
-	memset(rq->cmd, 0, sizeof(rq->cmd));
+	ide_init_flush_task(drive, &args);
 
-	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;
-
-
-	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 = &args;
 
 	ret = blk_execute_rq(q, disk, rq);
 
@@ -730,8 +723,9 @@ 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, &args);
 
+	rq->special = NULL;	/* just in case */
 	blk_put_request(rq);
 	return ret;
 }
Index: linux-ide-series2-export/drivers/ide/ide-io.c
===================================================================
--- linux-ide-series2-export.orig/drivers/ide/ide-io.c	2005-02-05 19:27:08.308610336 +0900
+++ linux-ide-series2-export/drivers/ide/ide-io.c	2005-02-05 19:27:08.736540825 +0900
@@ -55,22 +55,19 @@
 #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 *args)
 {
-	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(args, 0, sizeof(*args));
 
 	if (ide_id_has_flush_cache_ext(drive->id) &&
 	    (drive->capacity64 >= (1UL << 28)))
-		rq->buffer[0] = WIN_FLUSH_CACHE_EXT;
+		args->tfRegister[IDE_COMMAND_OFFSET] = WIN_FLUSH_CACHE_EXT;
+	else
+		args->tfRegister[IDE_COMMAND_OFFSET] = WIN_FLUSH_CACHE;
+
+	args->command_type = IDE_DRIVE_TASK_NO_DATA;
+	args->data_phase = TASKFILE_NO_DATA;
+	args->handler = task_no_data_intr;
 }
 
 /*
@@ -80,7 +77,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 *args = &hwgroup->flush_args;
 
 	/*
 	 * write cache disabled, clear the barrier bit and treat it like
@@ -92,10 +91,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 = args;
+	hwgroup->flush_real_rq = rq;
+
+	ide_init_flush_task(drive, args);
 
 	if (!post) {
 		drive->doing_barrier = 1;
@@ -105,7 +106,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 +163,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 +177,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) {
@@ -221,41 +223,37 @@ 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 *args)
 {
 	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 (ide_id_has_flush_cache_ext(drive->id) &&
+	    (drive->capacity64 >= (1UL << 28)))
+		high = (args->hobRegister[IDE_HCYL_OFFSET] << 16) |
+		       (args->hobRegister[IDE_LCYL_OFFSET] << 8 ) |
+			args->hobRegister[IDE_SECTOR_OFFSET];
+	else
+		high = 0;
+
+	hcyl = args->tfRegister[IDE_HCYL_OFFSET];
+	lcyl = args->tfRegister[IDE_LCYL_OFFSET];
+	sect = args->tfRegister[IDE_SECTOR_OFFSET];
+
+	if (args->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;
 
@@ -302,7 +300,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-series2-export/include/linux/ide.h
===================================================================
--- linux-ide-series2-export.orig/include/linux/ide.h	2005-02-05 19:27:08.104643467 +0900
+++ linux-ide-series2-export/include/linux/ide.h	2005-02-05 19:27:08.737540663 +0900
@@ -930,6 +930,25 @@ 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;
+ */
+	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 *);
@@ -955,8 +974,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_args;
+		/* real_rq for flushing */
+	struct request *flush_real_rq;
 		/* timeout value during long polls */
 	unsigned long poll_timeout;
 		/* queried upon timeouts */
@@ -1201,9 +1224,14 @@ extern ide_startstop_t ide_do_reset (ide
 extern void ide_init_drive_cmd (struct request *rq);
 
 /*
+ * This function initializes @task to WIN_FLUSH_CACHE[_EXT] command.
+ */
+void ide_init_flush_task(ide_drive_t *drive, ide_task_t *args);
+
+/*
  * 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.
@@ -1260,25 +1288,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;
- */
-	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 *);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2.6.11-rc2 07/09] ide: convert REQ_DRIVE_CMD to REQ_DRIVE_TASKFILE
  2005-02-05 10:25 [PATCH REPOST 2.6.11-rc2] ide: driver updates (phase 2) Tejun Heo
                   ` (4 preceding siblings ...)
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 04/09] ide: convert REQ_DRIVE_TASK to REQ_DRIVE_TASKFILE Tejun Heo
@ 2005-02-05 10:28 ` Tejun Heo
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 09/09] ide: remove REQ_DRIVE_CMD handling Tejun Heo
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tejun Heo @ 2005-02-05 10:28 UTC (permalink / raw)
  To: bzolnier, linux-ide, linux-kernel


07_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 <tj@home-tj.org>


Index: linux-ide-series2-export/drivers/ide/ide-disk.c
===================================================================
--- linux-ide-series2-export.orig/drivers/ide/ide-disk.c	2005-02-05 19:27:08.735540988 +0900
+++ linux-ide-series2-export/drivers/ide/ide-disk.c	2005-02-05 19:27:09.428428438 +0900
@@ -741,7 +741,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-series2-export/drivers/ide/ide.c
===================================================================
--- linux-ide-series2-export.orig/drivers/ide/ide.c	2005-02-05 19:26:50.977425527 +0900
+++ linux-ide-series2-export/drivers/ide/ide.c	2005-02-05 19:27:09.429428276 +0900
@@ -1263,9 +1263,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 args;
+	int err;
+
+	memset(&args, 0, sizeof(args));
+	args.tfRegister[IDE_COMMAND_OFFSET]	= WIN_SETFEATURES;
+	args.tfRegister[IDE_FEATURE_OFFSET]	= SETFEATURES_XFER;
+	args.tfRegister[IDE_NSECTOR_OFFSET]	= arg;
+	args.command_type			= IDE_DRIVE_TASK_NO_DATA;
+	args.data_phase				= TASKFILE_NO_DATA;
+	args.handler				= task_no_data_intr;
+
+	err = ide_raw_taskfile(drive, &args, NULL);
 
 	if (!err && arg) {
 		ide_set_xfer_rate(drive, (u8) arg);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2.6.11-rc2 06/09] ide: remove REQ_DRIVE_TASK handling
  2005-02-05 10:25 [PATCH REPOST 2.6.11-rc2] ide: driver updates (phase 2) Tejun Heo
                   ` (2 preceding siblings ...)
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 03/09] ide: ide_diag_taskfile() rq initialization fix Tejun Heo
@ 2005-02-05 10:28 ` Tejun Heo
  2005-02-05 10:43   ` Christoph Hellwig
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 04/09] ide: convert REQ_DRIVE_TASK to REQ_DRIVE_TASKFILE Tejun Heo
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Tejun Heo @ 2005-02-05 10:28 UTC (permalink / raw)
  To: bzolnier, linux-ide, linux-kernel


06_ide_remove_task.patch

	Unused REQ_DRIVE_TASK handling removed.


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-ide-series2-export/drivers/ide/ide-io.c
===================================================================
--- linux-ide-series2-export.orig/drivers/ide/ide-io.c	2005-02-05 19:27:08.736540825 +0900
+++ linux-ide-series2-export/drivers/ide/ide-io.c	2005-02-05 19:27:09.190467092 +0900
@@ -357,20 +357,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)
@@ -557,7 +543,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;
@@ -599,7 +585,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;
@@ -757,33 +743,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)
@@ -894,7 +854,7 @@ static ide_startstop_t start_request (id
 		return startstop;
 	}
 	if (!drive->special.all) {
-		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-series2-export/drivers/ide/ide-lib.c
===================================================================
--- linux-ide-series2-export.orig/drivers/ide/ide-lib.c	2005-02-05 19:26:51.018418868 +0900
+++ linux-ide-series2-export/drivers/ide/ide-lib.c	2005-02-05 19:27:09.191466929 +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-series2-export/include/linux/blkdev.h
===================================================================
--- linux-ide-series2-export.orig/include/linux/blkdev.h	2005-02-05 19:26:51.018418868 +0900
+++ linux-ide-series2-export/include/linux/blkdev.h	2005-02-05 19:27:09.191466929 +0900
@@ -202,7 +202,7 @@ enum rq_flag_bits {
 	__REQ_QUIET,		/* don't worry about errors */
 	__REQ_SPECIAL,		/* driver suplied command */
 	__REQ_DRIVE_CMD,
-	__REQ_DRIVE_TASK,
+	__REQ_DRIVE_TASK,	/* obsolete, unused anymore - tj */
 	__REQ_DRIVE_TASKFILE,
 	__REQ_PREEMPT,		/* set for "ide_preempt" requests */
 	__REQ_PM_SUSPEND,	/* suspend request */

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2.6.11-rc2 09/09] ide: remove REQ_DRIVE_CMD handling
  2005-02-05 10:25 [PATCH REPOST 2.6.11-rc2] ide: driver updates (phase 2) Tejun Heo
                   ` (5 preceding siblings ...)
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 07/09] ide: convert REQ_DRIVE_CMD " Tejun Heo
@ 2005-02-05 10:28 ` Tejun Heo
  2005-02-05 10:43   ` Christoph Hellwig
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 05/09] ide: map ide_task_ioctl() to ide_taskfile_ioctl() Tejun Heo
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 08/09] ide: map ide_cmd_ioctl() " Tejun Heo
  8 siblings, 1 reply; 15+ messages in thread
From: Tejun Heo @ 2005-02-05 10:28 UTC (permalink / raw)
  To: bzolnier, linux-ide, linux-kernel


09_ide_remove_cmd.patch

	Removed unused REQ_DRIVE_CMD handling.


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-ide-series2-export/drivers/ide/ide-io.c
===================================================================
--- linux-ide-series2-export.orig/drivers/ide/ide-io.c	2005-02-05 19:27:09.190467092 +0900
+++ linux-ide-series2-export/drivers/ide/ide-io.c	2005-02-05 19:27:09.866357303 +0900
@@ -347,17 +347,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);
@@ -543,7 +533,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;
@@ -585,7 +575,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;
@@ -595,63 +585,6 @@ ide_startstop_t ide_abort(ide_drive_t *d
 }
 
 /**
- *	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) && DRIVER(drive) != NULL)
-		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;
-}
-
-/**
  *	do_special		-	issue some special commands
  *	@drive: drive the command is for
  *
@@ -706,80 +639,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 *args = rq->special;
+	
+	if (!args) {
+		/*
+		 * 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 = 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);
 }
 
 /**
@@ -854,10 +759,8 @@ static ide_startstop_t start_request (id
 		return startstop;
 	}
 	if (!drive->special.all) {
-		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-series2-export/include/linux/blkdev.h
===================================================================
--- linux-ide-series2-export.orig/include/linux/blkdev.h	2005-02-05 19:27:09.191466929 +0900
+++ linux-ide-series2-export/include/linux/blkdev.h	2005-02-05 19:27:09.867357141 +0900
@@ -201,7 +201,7 @@ 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_CMD,	/* obsolete, unused anymore - tj */
 	__REQ_DRIVE_TASK,	/* obsolete, unused anymore - tj */
 	__REQ_DRIVE_TASKFILE,
 	__REQ_PREEMPT,		/* set for "ide_preempt" requests */

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2.6.11-rc2 05/09] ide: map ide_task_ioctl() to ide_taskfile_ioctl()
  2005-02-05 10:25 [PATCH REPOST 2.6.11-rc2] ide: driver updates (phase 2) Tejun Heo
                   ` (6 preceding siblings ...)
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 09/09] ide: remove REQ_DRIVE_CMD handling Tejun Heo
@ 2005-02-05 10:28 ` Tejun Heo
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 08/09] ide: map ide_cmd_ioctl() " Tejun Heo
  8 siblings, 0 replies; 15+ messages in thread
From: Tejun Heo @ 2005-02-05 10:28 UTC (permalink / raw)
  To: bzolnier, linux-ide, linux-kernel


05_ide_taskfile_task_ioctl.patch

	ide_task_ioctl() modified to map to ide_taskfile_ioctl().
	This is the last user of REQ_DRIVE_TASK.


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-ide-series2-export/drivers/ide/ide-taskfile.c
===================================================================
--- linux-ide-series2-export.orig/drivers/ide/ide-taskfile.c	2005-02-05 19:27:08.533573794 +0900
+++ linux-ide-series2-export/drivers/ide/ide-taskfile.c	2005-02-05 19:27:08.985500385 +0900
@@ -735,32 +735,45 @@ 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;
+	u8 args[7];
+	ide_task_t task;
+	task_ioreg_t *regs = task.tfRegister;
+	int ret;
 
 	if (copy_from_user(args, p, 7))
 		return -EFAULT;
-	err = ide_wait_cmd_task(drive, argbuf);
-	if (copy_to_user(p, argbuf, argsize))
-		err = -EFAULT;
-	return err;
+
+	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;
+
+	ret = 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))
+		ret = -EFAULT;
+
+	return ret;
 }
 
 /*

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2.6.11-rc2 03/09] ide: ide_diag_taskfile() rq initialization fix
  2005-02-05 10:25 [PATCH REPOST 2.6.11-rc2] ide: driver updates (phase 2) Tejun Heo
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 02/09] ide: ide_init_drive_cmd() now defaults to REQ_DRIVE_TASKFILE Tejun Heo
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 01/09] ide: kill unused pkt_task_t Tejun Heo
@ 2005-02-05 10:28 ` Tejun Heo
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 06/09] ide: remove REQ_DRIVE_TASK handling Tejun Heo
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Tejun Heo @ 2005-02-05 10:28 UTC (permalink / raw)
  To: bzolnier, linux-ide, linux-kernel


03_ide_diag_taskfile_use_init_drive_cmd.patch

	In ide_diag_taskfile(), when initializing taskfile rq,
	ref_count wasn't initialized properly.  Modified to use
	ide_init_drive_cmd().  This doesn't really change any behavior
	as the request isn't managed via the block layer.


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-ide-series2-export/drivers/ide/ide-taskfile.c
===================================================================
--- linux-ide-series2-export.orig/drivers/ide/ide-taskfile.c	2005-02-05 19:27:08.308610336 +0900
+++ linux-ide-series2-export/drivers/ide/ide-taskfile.c	2005-02-05 19:27:08.533573794 +0900
@@ -471,8 +471,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;
 
 	/*

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2.6.11-rc2 08/09] ide: map ide_cmd_ioctl() to ide_taskfile_ioctl()
  2005-02-05 10:25 [PATCH REPOST 2.6.11-rc2] ide: driver updates (phase 2) Tejun Heo
                   ` (7 preceding siblings ...)
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 05/09] ide: map ide_task_ioctl() to ide_taskfile_ioctl() Tejun Heo
@ 2005-02-05 10:28 ` Tejun Heo
  8 siblings, 0 replies; 15+ messages in thread
From: Tejun Heo @ 2005-02-05 10:28 UTC (permalink / raw)
  To: bzolnier, linux-ide, linux-kernel


08_ide_taskfile_cmd_ioctl.patch

	ide_cmd_ioctl() converted to use ide_taskfile_ioctl().  This
	is the last user of REQ_DRIVE_CMD.


Signed-off-by: Tejun Heo <tj@home-tj.org>


Index: linux-ide-series2-export/drivers/ide/ide-iops.c
===================================================================
--- linux-ide-series2-export.orig/drivers/ide/ide-iops.c	2005-02-05 19:26:50.937432023 +0900
+++ linux-ide-series2-export/drivers/ide/ide-iops.c	2005-02-05 19:27:09.636394657 +0900
@@ -648,11 +648,11 @@ u8 eighty_ninty_three (ide_drive_t *driv
 
 EXPORT_SYMBOL(eighty_ninty_three);
 
-int ide_ata66_check (ide_drive_t *drive, ide_task_t *args)
+int ide_ata66_check(ide_drive_t *drive, task_ioreg_t *regs)
 {
-	if ((args->tfRegister[IDE_COMMAND_OFFSET] == WIN_SETFEATURES) &&
-	    (args->tfRegister[IDE_SECTOR_OFFSET] > XFER_UDMA_2) &&
-	    (args->tfRegister[IDE_FEATURE_OFFSET] == SETFEATURES_XFER)) {
+	if (regs[IDE_COMMAND_OFFSET] == WIN_SETFEATURES &&
+	    regs[IDE_SECTOR_OFFSET] > XFER_UDMA_2 &&
+	    regs[IDE_FEATURE_OFFSET] == SETFEATURES_XFER) {
 #ifndef CONFIG_IDEDMA_IVB
 		if ((drive->id->hw_config & 0x6000) == 0) {
 #else /* !CONFIG_IDEDMA_IVB */
@@ -678,11 +678,11 @@ int ide_ata66_check (ide_drive_t *drive,
  * 1 : Safe to update drive->id DMA registers.
  * 0 : OOPs not allowed.
  */
-int set_transfer (ide_drive_t *drive, ide_task_t *args)
+int set_transfer(ide_drive_t *drive, task_ioreg_t *regs)
 {
-	if ((args->tfRegister[IDE_COMMAND_OFFSET] == WIN_SETFEATURES) &&
-	    (args->tfRegister[IDE_SECTOR_OFFSET] >= XFER_SW_DMA_0) &&
-	    (args->tfRegister[IDE_FEATURE_OFFSET] == SETFEATURES_XFER) &&
+	if (regs[IDE_COMMAND_OFFSET] == WIN_SETFEATURES &&
+	    regs[IDE_SECTOR_OFFSET] >= XFER_SW_DMA_0 &&
+	    regs[IDE_FEATURE_OFFSET] == SETFEATURES_XFER &&
 	    (drive->id->dma_ultra ||
 	     drive->id->dma_mword ||
 	     drive->id->dma_1word))
Index: linux-ide-series2-export/drivers/ide/ide-taskfile.c
===================================================================
--- linux-ide-series2-export.orig/drivers/ide/ide-taskfile.c	2005-02-05 19:27:08.985500385 +0900
+++ linux-ide-series2-export/drivers/ide/ide-taskfile.c	2005-02-05 19:27:09.637394495 +0900
@@ -660,79 +660,93 @@ 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 io_32bit, xfer_rate = 0, in_size, ret;
 
-	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, regs)) {
 		xfer_rate = args[1];
-		if (ide_ata66_check(drive, &tfargs))
-			goto abort;
+		if (ide_ata66_check(drive, regs))
+			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;
 
-	if (!err && xfer_rate) {
+		if ((buf = kmalloc(in_size, GFP_KERNEL)) == NULL)
+			return -ENOMEM;
+		memset(buf, 0, in_size);	/* paranoia */
+	}
+
+	io_32bit = drive->io_32bit;
+	drive->io_32bit = 0;	/* racy */
+	ret = ide_diag_taskfile(drive, &task, in_size, buf);
+	drive->io_32bit = io_32bit;
+
+	if (!ret && 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))
-		err = -EFAULT;
-	if (argsize > 4)
-		kfree(argbuf);
-	return err;
+
+	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))
+		ret = -EFAULT;
+
+	kfree(buf);
+	return ret;
 }
 
 int ide_task_ioctl(ide_drive_t *drive, unsigned int cmd, unsigned long arg)
Index: linux-ide-series2-export/include/linux/ide.h
===================================================================
--- linux-ide-series2-export.orig/include/linux/ide.h	2005-02-05 19:27:08.737540663 +0900
+++ linux-ide-series2-export/include/linux/ide.h	2005-02-05 19:27:09.638394333 +0900
@@ -1280,14 +1280,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 *);
@@ -1329,10 +1321,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 *, task_ioreg_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 *, task_ioreg_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);

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2.6.11-rc2 06/09] ide: remove REQ_DRIVE_TASK handling
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 06/09] ide: remove REQ_DRIVE_TASK handling Tejun Heo
@ 2005-02-05 10:43   ` Christoph Hellwig
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2005-02-05 10:43 UTC (permalink / raw)
  To: Tejun Heo; +Cc: bzolnier, linux-ide, linux-kernel

>  	__REQ_DRIVE_CMD,
> -	__REQ_DRIVE_TASK,
> +	__REQ_DRIVE_TASK,	/* obsolete, unused anymore - tj */

please kill it completely

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2.6.11-rc2 09/09] ide: remove REQ_DRIVE_CMD handling
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 09/09] ide: remove REQ_DRIVE_CMD handling Tejun Heo
@ 2005-02-05 10:43   ` Christoph Hellwig
  0 siblings, 0 replies; 15+ messages in thread
From: Christoph Hellwig @ 2005-02-05 10:43 UTC (permalink / raw)
  To: Tejun Heo; +Cc: bzolnier, linux-ide, linux-kernel

> -	__REQ_DRIVE_CMD,
> +	__REQ_DRIVE_CMD,	/* obsolete, unused anymore - tj */

dito


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2.6.11-rc2 04/09] ide: convert REQ_DRIVE_TASK to REQ_DRIVE_TASKFILE
       [not found] ` <20050205021556.8FC05132701@htj.dyndns.org>
@ 2005-02-06 18:34   ` Bartlomiej Zolnierkiewicz
  2005-02-07  5:13     ` Tejun Heo
  0 siblings, 1 reply; 15+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2005-02-06 18:34 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-ide, linux-kernel

I put some more thought into this change... details below...

On Sat,  5 Feb 2005 11:15:56 +0900 (KST), Tejun Heo <tj@home-tj.org> wrote:

> @@ -705,24 +705,17 @@ static int idedisk_issue_flush(request_q
>  {
>         ide_drive_t *drive = q->queuedata;
>         struct request *rq;
> +       ide_task_t args;
>         int ret;

ide_task_t task please

> @@ -730,8 +723,9 @@ 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, &args);
> 
> +       rq->special = NULL;     /* just in case */

In what case?

> @@ -55,22 +55,19 @@
>  #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 *args)

ide_task_t *task

> @@ -80,7 +77,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 *args = &hwgroup->flush_args;

ide_task_t *task

> @@ -221,41 +223,37 @@ 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 *args)

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 (ide_id_has_flush_cache_ext(drive->id) &&
> +           (drive->capacity64 >= (1UL << 28)))

Please just use if (drive->addressing), it is simpler and still correct.
Since we are now using ide_task_t 'high' will be 0 when
ide_id_has_flush_cache() == 0 and drive->addressing == 1
(such combination is unlikely but...).  Also thanks to this change
ide_get_error_location() becomes a really *generic* helper and
can be later used by other code.

> @@ -1201,9 +1224,14 @@ extern ide_startstop_t ide_do_reset (ide
>  extern void ide_init_drive_cmd (struct request *rq);
> 
>  /*
> + * This function initializes @task to WIN_FLUSH_CACHE[_EXT] command.
> + */
> +void ide_init_flush_task(ide_drive_t *drive, ide_task_t *args);

comment is wrong and not needed,

void ide_init_flush_task(ide_drive_t *, ide_task_t *);

should be enough


There is one problem left with this change - FLUSH_CACHE_{EXT}
command handling becomes slower for drive's supporting LBA48
(also next patches make ide_{task,cmd}_ioctl() slower).  Why is so?
See do_rw_taskfile(), HOB registers are written/read unconditionally
if (drive->addressing == 1).  This can be fixed by i.e. adding
'unsigned long flags' to ide_task_t and IDE_TASK_LBA48 flag.
BTW this fix is needed also to implement LBA48 optimization for
read/write requests (not writing HOB registers when not needed).

IMHO there are some things worth mentioning in the patch description,
do_rw_taskfile() vs execute_drive_cmd()+ide_cmd() details:
some registers are written now in different order and timeout is bumped
(these changes shouldn't make any harm but I'm paranoid :).

Thanks,
Bartlomiej

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2.6.11-rc2 04/09] ide: convert REQ_DRIVE_TASK to REQ_DRIVE_TASKFILE
  2005-02-05 10:28 ` [PATCH 2.6.11-rc2 04/09] ide: convert REQ_DRIVE_TASK to REQ_DRIVE_TASKFILE Tejun Heo
@ 2005-02-06 19:08   ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 15+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2005-02-06 19:08 UTC (permalink / raw)
  To: Tejun Heo; +Cc: linux-ide, linux-kernel

I forgot about this one...

> @@ -55,22 +55,19 @@
>  #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 *args)
>  {
> -       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(args, 0, sizeof(*args));
> 
>         if (ide_id_has_flush_cache_ext(drive->id) &&
>             (drive->capacity64 >= (1UL << 28)))
> -               rq->buffer[0] = WIN_FLUSH_CACHE_EXT;
> +               args->tfRegister[IDE_COMMAND_OFFSET] = WIN_FLUSH_CACHE_EXT;
> +       else
> +               args->tfRegister[IDE_COMMAND_OFFSET] = WIN_FLUSH_CACHE;
> +
> +       args->command_type = IDE_DRIVE_TASK_NO_DATA;
> +       args->data_phase = TASKFILE_NO_DATA;
> +       args->handler = task_no_data_intr;
>  }

Isn't EXPORT_SYMBOL_{GPL} needed for ide_init_flush_task()?

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 2.6.11-rc2 04/09] ide: convert REQ_DRIVE_TASK to REQ_DRIVE_TASKFILE
  2005-02-06 18:34   ` [PATCH 2.6.11-rc2 04/09] ide: convert REQ_DRIVE_TASK to REQ_DRIVE_TASKFILE Bartlomiej Zolnierkiewicz
@ 2005-02-07  5:13     ` Tejun Heo
  0 siblings, 0 replies; 15+ messages in thread
From: Tejun Heo @ 2005-02-07  5:13 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: Tejun Heo, linux-ide, linux-kernel

Bartlomiej Zolnierkiewicz wrote:
> I put some more thought into this change... details below...
> 
> On Sat,  5 Feb 2005 11:15:56 +0900 (KST), Tejun Heo <tj@home-tj.org> wrote:
> 
> 
>>@@ -705,24 +705,17 @@ static int idedisk_issue_flush(request_q
>> {
>>        ide_drive_t *drive = q->queuedata;
>>        struct request *rq;
>>+       ide_task_t args;
>>        int ret;
> 
> 
> ide_task_t task please
> 

  Okay.

> 
>>@@ -730,8 +723,9 @@ 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, &args);
>>
>>+       rq->special = NULL;     /* just in case */
> 
> 
> In what case?
> 

  As the request is allocated and freed by the generic block layer, I 
was kinda worrying about cases where the request outlives 
blk_put_request() and somehow somebody accesses ->special.  Probably 
unnecessary but dangling pointers pointing stack area really scares me.

> 
>>@@ -55,22 +55,19 @@
>> #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 *args)
> 
> 
> ide_task_t *task
> 
> 
>>@@ -80,7 +77,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 *args = &hwgroup->flush_args;
> 
> 
> ide_task_t *task
> 
> 
>>@@ -221,41 +223,37 @@ 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 *args)
> 
> 
> 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 (ide_id_has_flush_cache_ext(drive->id) &&
>>+           (drive->capacity64 >= (1UL << 28)))
> 
> 
> Please just use if (drive->addressing), it is simpler and still correct.
> Since we are now using ide_task_t 'high' will be 0 when
> ide_id_has_flush_cache() == 0 and drive->addressing == 1
> (such combination is unlikely but...).  Also thanks to this change
> ide_get_error_location() becomes a really *generic* helper and
> can be later used by other code.
> 

  Sure.

> 
>>@@ -1201,9 +1224,14 @@ extern ide_startstop_t ide_do_reset (ide
>> extern void ide_init_drive_cmd (struct request *rq);
>>
>> /*
>>+ * This function initializes @task to WIN_FLUSH_CACHE[_EXT] command.
>>+ */
>>+void ide_init_flush_task(ide_drive_t *drive, ide_task_t *args);
> 
> 
> comment is wrong and not needed,
> 
> void ide_init_flush_task(ide_drive_t *, ide_task_t *);
> 
> should be enough
> 

  Okay.

> 
> There is one problem left with this change - FLUSH_CACHE_{EXT}
> command handling becomes slower for drive's supporting LBA48
> (also next patches make ide_{task,cmd}_ioctl() slower).  Why is so?
> See do_rw_taskfile(), HOB registers are written/read unconditionally
> if (drive->addressing == 1).  This can be fixed by i.e. adding
> 'unsigned long flags' to ide_task_t and IDE_TASK_LBA48 flag.
> BTW this fix is needed also to implement LBA48 optimization for
> read/write requests (not writing HOB registers when not needed).
> 
> IMHO there are some things worth mentioning in the patch description,
> do_rw_taskfile() vs execute_drive_cmd()+ide_cmd() details:
> some registers are written now in different order and timeout is bumped
> (these changes shouldn't make any harm but I'm paranoid :).
> 

  Yeah, sure.

  Thanks.

-- 
tejun


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2005-02-07  5:13 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-02-05 10:25 [PATCH REPOST 2.6.11-rc2] ide: driver updates (phase 2) Tejun Heo
2005-02-05 10:28 ` [PATCH 2.6.11-rc2 02/09] ide: ide_init_drive_cmd() now defaults to REQ_DRIVE_TASKFILE Tejun Heo
2005-02-05 10:28 ` [PATCH 2.6.11-rc2 01/09] ide: kill unused pkt_task_t Tejun Heo
2005-02-05 10:28 ` [PATCH 2.6.11-rc2 03/09] ide: ide_diag_taskfile() rq initialization fix Tejun Heo
2005-02-05 10:28 ` [PATCH 2.6.11-rc2 06/09] ide: remove REQ_DRIVE_TASK handling Tejun Heo
2005-02-05 10:43   ` Christoph Hellwig
2005-02-05 10:28 ` [PATCH 2.6.11-rc2 04/09] ide: convert REQ_DRIVE_TASK to REQ_DRIVE_TASKFILE Tejun Heo
2005-02-06 19:08   ` Bartlomiej Zolnierkiewicz
2005-02-05 10:28 ` [PATCH 2.6.11-rc2 07/09] ide: convert REQ_DRIVE_CMD " Tejun Heo
2005-02-05 10:28 ` [PATCH 2.6.11-rc2 09/09] ide: remove REQ_DRIVE_CMD handling Tejun Heo
2005-02-05 10:43   ` Christoph Hellwig
2005-02-05 10:28 ` [PATCH 2.6.11-rc2 05/09] ide: map ide_task_ioctl() to ide_taskfile_ioctl() Tejun Heo
2005-02-05 10:28 ` [PATCH 2.6.11-rc2 08/09] ide: map ide_cmd_ioctl() " Tejun Heo
  -- strict thread matches above, loose matches on Subject: below --
2005-02-05  2:15 [PATCH 2.6.11-rc2] ide: driver updates (phase 2) Tejun Heo
     [not found] ` <20050205021556.8FC05132701@htj.dyndns.org>
2005-02-06 18:34   ` [PATCH 2.6.11-rc2 04/09] ide: convert REQ_DRIVE_TASK to REQ_DRIVE_TASKFILE Bartlomiej Zolnierkiewicz
2005-02-07  5:13     ` Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).