Skip to contentSkip to conversation

ide: add struct ide_taskfile

PATCH In review Bartlomiej Zolnierkiewicz 2 participants 5 messages Oct 8, 2007 – Oct 25, 2007 libata

lore
Share & tools
lore

Bartlomiej Zolnierkiewicz

author
#0 lore raw

PATCH 6/12 ide: add struct ide_taskfile

* Don't set write-only ide_task_t.hobRegister[6] and ide_task_t.hobRegister[7] in idedisk_set_max_address_ext().

* Add struct ide_taskfile and use it in ide_task_t instead of tfRegister[] and hobRegister[].

* Remove no longer needed IDE_CONTROL_OFFSET_HOB define.

* Add #ifndef/#endif __KERNEL__ around definitions of {task,hob}_struct_t.

While at it:

* Use ATA_LBA define for LBA bit (0x40) as suggested by Tejun Heo.

There should be no functionality changes caused by this patch.

Cc Tejun Heo <[email protected]>
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ide/ide-disk.c     |  156 ++++++++++++++++++++++++---------------------
 drivers/ide/ide-io.c       |   80 +++++++++++++----------
 drivers/ide/ide-iops.c     |   12 +--
 drivers/ide/ide-lib.c      |    3
 drivers/ide/ide-acpi.c     |    8 ---
 drivers/ide/ide-disk.c     |  118 ++++++++++++++++++++-------------------------
 drivers/ide/ide-io.c       |   60 +++++++++++-----------
 drivers/ide/ide-iops.c     |   12 ++--
 drivers/ide/ide-lib.c      |    3 -
 drivers/ide/ide-taskfile.c |   99 +++++++++++++++++--------------------
 include/linux/hdreg.h      |    4 +
 include/linux/ide.h        |   36 +++++++++----
 8 files changed, 169 insertions(+), 171 deletions(-)
drivers/ide/ide-acpi.c +1 -7
Index: b/drivers/ide/ide-acpi.c
===================================================================
@@ -354,13 +354,7 @@ static int taskfile_load_raw(ide_drive_t
 	args.handler      = &task_no_data_intr;
 
 	/* convert gtf to IDE Taskfile */
-	args.tfRegister[1] = gtf->tfa[0];	/* 0x1f1 */
-	args.tfRegister[2] = gtf->tfa[1];	/* 0x1f2 */
-	args.tfRegister[3] = gtf->tfa[2];	/* 0x1f3 */
-	args.tfRegister[4] = gtf->tfa[3];	/* 0x1f4 */
-	args.tfRegister[5] = gtf->tfa[4];	/* 0x1f5 */
-	args.tfRegister[6] = gtf->tfa[5];	/* 0x1f6 */
-	args.tfRegister[7] = gtf->tfa[6];	/* 0x1f7 */
+	memcpy(&args.tf_array[7], &gtf->tfa, 7);
 
 	if (ide_noacpitfs) {
 		DEBPRINT("_GTF execution disabled\n");
drivers/ide/ide-disk.c +55 -63

230 lines not on this page · raw message

drivers/ide/ide-io.c +31 -29

109 lines not on this page · raw message

drivers/ide/ide-iops.c +6 -6
Index: b/drivers/ide/ide-iops.c
===================================================================
@@ -635,9 +635,9 @@ no_80w:
 
 int ide_ata66_check (ide_drive_t *drive, ide_task_t *args)
 {
-	if ((args->tfRegister[IDE_COMMAND_OFFSET] == WIN_SETFEATURES) &&
-	    (args->tfRegister[IDE_SECTOR_OFFSET] > XFER_UDMA_2) &&
-	    (args->tfRegister[IDE_FEATURE_OFFSET] == SETFEATURES_XFER)) {
+	if (args->tf.command == WIN_SETFEATURES &&
+	    args->tf.lbal > XFER_UDMA_2 &&
+	    args->tf.feature == SETFEATURES_XFER) {
 		if (eighty_ninty_three(drive) == 0) {
 			printk(KERN_WARNING "%s: UDMA speeds >UDMA33 cannot "
 					    "be set\n", drive->name);

@@ -655,9 +655,9 @@ int ide_ata66_check (ide_drive_t *drive,
  */
 int set_transfer (ide_drive_t *drive, ide_task_t *args)
 {
-	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 (args->tf.command == WIN_SETFEATURES &&
+	    args->tf.lbal >= XFER_SW_DMA_0 &&
+	    args->tf.feature == SETFEATURES_XFER &&
 	    (drive->id->dma_ultra ||
 	     drive->id->dma_mword ||
 	     drive->id->dma_1word))
drivers/ide/ide-lib.c +1 -2
Index: b/drivers/ide/ide-lib.c
===================================================================
@@ -465,8 +465,7 @@ static void ide_dump_opcode(ide_drive_t 
 	} else if (rq->cmd_type == REQ_TYPE_ATA_TASKFILE) {
 		ide_task_t *args = rq->special;
 		if (args) {
-			task_struct_t *tf = (task_struct_t *) args->tfRegister;
-			opcode = tf->command;
+			opcode = args->tf.command;
 			found = 1;
 		}
 	}
drivers/ide/ide-taskfile.c +46 -53

226 lines not on this page · raw message

include/linux/hdreg.h +3 -1
Index: b/include/linux/hdreg.h
===================================================================
@@ -117,7 +117,7 @@ typedef union ide_reg_valid_s {
 
 typedef struct ide_task_request_s {
 	__u8		io_ports[8];
-	__u8		hob_ports[8];
+	__u8		hob_ports[8]; /* bytes 6 and 7 are unused */
 	ide_reg_valid_t	out_flags;
 	ide_reg_valid_t	in_flags;
 	int		data_phase;

@@ -139,6 +139,7 @@ struct hd_drive_cmd_hdr {
 	__u8 sector_count;
 };
 
+#ifndef __KERNEL__
 typedef struct hd_drive_task_hdr {
 	__u8 data;
 	__u8 feature;

@@ -160,6 +161,7 @@ typedef struct hd_drive_hob_hdr {
 	__u8 device_head;
 	__u8 control;
 } hob_struct_t;
+#endif
 
 #define TASKFILE_INVALID		0x7fff
 #define TASKFILE_48			0x8000
include/linux/ide.h +26 -10
Index: b/include/linux/ide.h
===================================================================
@@ -103,8 +103,6 @@ typedef unsigned char	byte;	/* used ever
 #define IDE_FEATURE_OFFSET	IDE_ERROR_OFFSET
 #define IDE_COMMAND_OFFSET	IDE_STATUS_OFFSET
 
-#define IDE_CONTROL_OFFSET_HOB	(7)
-
 #define IDE_DATA_REG		(HWIF(drive)->io_ports[IDE_DATA_OFFSET])
 #define IDE_ERROR_REG		(HWIF(drive)->io_ports[IDE_ERROR_OFFSET])
 #define IDE_NSECTOR_REG		(HWIF(drive)->io_ports[IDE_NSECTOR_OFFSET])

@@ -1072,15 +1070,33 @@ extern void ide_end_drive_cmd(ide_drive_
  */
 extern int ide_wait_cmd(ide_drive_t *, u8, u8, u8, u8, u8 *);
 
+struct ide_taskfile {
+	u8	hob_data;	/*  0: high data byte (for TASKFILE IOCTL) */
+
+	u8	hob_feature;	/*  1-5: additional data to support LBA48 */
+	u8	hob_nsect;
+	u8	hob_lbal;
+	u8	hob_lbam;
+	u8	hob_lbah;
+
+	u8	data;		/*  6: low data byte (for TASKFILE IOCTL) */
+
+	u8	feature;	/*  7: feature */
+	u8	nsect;		/*  8: number of sectors */
+	u8	lbal;		/*  9: LBA low */
+	u8	lbam;		/* 10: LBA mid */
+	u8	lbah;		/* 11: LBA high */
+
+	u8	device;		/* 12: device select */
+
+	u8	command;	/* 13: command */
+};
+
 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;
- */
-	u8			tfRegister[8];
-	u8			hobRegister[8];
+	union {
+		struct ide_taskfile	tf;
+		u8			tf_array[14];
+	};
 	ide_reg_valid_t		tf_out_flags;
 	ide_reg_valid_t		tf_in_flags;
 	int			data_phase;
#1 lore raw
Quoted 10 lines · Bartlomiej Zolnierkiewicz

* Don't set write-only ide_task_t.hobRegister[6] and ide_task_t.hobRegister[7] in idedisk_set_max_address_ext().

* Add struct ide_taskfile and use it in ide_task_t instead of tfRegister[] and hobRegister[].

* Remove no longer needed IDE_CONTROL_OFFSET_HOB define.

* Add #ifndef/#endif __KERNEL__ around definitions of {task,hob}_struct_t.

Quoted 7 lines

While at it:

* Use ATA_LBA define for LBA bit (0x40) as suggested by Tejun Heo.

There should be no functionality changes caused by this patch.

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
Nearly-acked-by Sergei Shtylyov <[email protected]>
Quoted 4 lines

Index: b/drivers/ide/ide-disk.c
===================================================================

drivers/ide/ide-disk.c +0 -0

[...]

drivers/ide/ide-disk.c

Quoted 2 lines
drivers/ide/ide-disk.c +2 -0
+	if ((tf->command & 0x01) == 0) {
+		u32 high, low;

Isn't newline needed after declarations?

Quoted 5 lines
drivers/ide/ide-disk.c +2 -0
+		high = (tf->hob_lbah << 16) | (tf->hob_lbam << 8) | tf->hob_lbal;
+		low  = (tf->lbah << 16) | (tf->lbam << 8) | tf->lbal;
 		addr = ((__u64)high << 24) | low;
 		addr++;	/* since the return value is (maxlba - 1), we add 1 */
 	}

[...]

Quoted 1 line
drivers/ide/ide-disk.c +0 -0
@@ -422,33 +417,29 @@ static unsigned long idedisk_set_max_add

[...]

drivers/ide/ide-disk.c static unsigned long idedisk_read_native · line 377

Quoted 10 lines
drivers/ide/ide-disk.c +2 -7
 	/* if OK, compute maximum address value */
-	if ((args.tfRegister[IDE_STATUS_OFFSET] & 0x01) == 0) {
-		u32 high = (args.hobRegister[IDE_HCYL_OFFSET] << 16) |
-			   (args.hobRegister[IDE_LCYL_OFFSET] <<  8) |
-			    args.hobRegister[IDE_SECTOR_OFFSET];
-		u32 low  = ((args.tfRegister[IDE_HCYL_OFFSET])<<16) |
-			   ((args.tfRegister[IDE_LCYL_OFFSET])<<8) |
-			    (args.tfRegister[IDE_SECTOR_OFFSET]);
+	if ((tf->command & 0x01) == 0) {
+		u32 high, low;

Again missing newline...

Quoted 30 lines
drivers/ide/ide-disk.c +8 -4
+		high = (tf->hob_lbah << 16) | (tf->hob_lbam << 8) | tf->hob_lbal;
+		low  = (tf->lbah << 16) | (tf->lbam << 8) | tf->lbal;
 		addr_set = ((__u64)high << 24) | low;
 		addr_set++;
 	}

@@ -582,12 +573,13 @@ static sector_t idedisk_capacity (ide_dr
 static int smart_enable(ide_drive_t *drive)
 {
 	ide_task_t args;
+	struct ide_taskfile *tf = &args.tf;
 
 	memset(&args, 0, sizeof(ide_task_t));
-	args.tfRegister[IDE_FEATURE_OFFSET]	= SMART_ENABLE;
-	args.tfRegister[IDE_LCYL_OFFSET]	= SMART_LCYL_PASS;
-	args.tfRegister[IDE_HCYL_OFFSET]	= SMART_HCYL_PASS;
-	args.tfRegister[IDE_COMMAND_OFFSET]	= WIN_SMART;
+	tf->feature = SMART_ENABLE;
+	tf->lbam    = SMART_LCYL_PASS;
+	tf->lbah    = SMART_HCYL_PASS;
+	tf->command = WIN_SMART;
 	args.command_type			= IDE_DRIVE_TASK_NO_DATA;
 	args.handler				= &task_no_data_intr;
 	return ide_raw_taskfile(drive, &args, NULL);

@@ -596,13 +588,14 @@ static int smart_enable(ide_drive_t *dri
 static int get_smart_data(ide_drive_t *drive, u8 *buf, u8 sub_cmd)
 {
 	ide_task_t args;
+	struct ide_taskfile *tf = &args.tf;
 
 	memset(&args, 0, sizeof(ide_task_t));

drivers/ide/ide-disk.c static int smart_enable(ide_drive_t *dri · line 598

Quoted 10 lines
drivers/ide/ide-disk.c +5 -5
-	args.tfRegister[IDE_FEATURE_OFFSET]	= sub_cmd;
-	args.tfRegister[IDE_NSECTOR_OFFSET]	= 0x01;
-	args.tfRegister[IDE_LCYL_OFFSET]	= SMART_LCYL_PASS;
-	args.tfRegister[IDE_HCYL_OFFSET]	= SMART_HCYL_PASS;
-	args.tfRegister[IDE_COMMAND_OFFSET]	= WIN_SMART;
+	tf->feature = sub_cmd;
+	tf->nsect   = 0x01;
+	tf->lbam    = SMART_LCYL_PASS;
+	tf->lbah    = SMART_HCYL_PASS;
+	tf->command = WIN_SMART;

I guess the code above and below were menat to have the same = indentation...

drivers/ide/ide-disk.c static int write_cache(ide_drive_t *driv · line 806

Quoted 13 lines
drivers/ide/ide-disk.c +2 -2
 	args.command_type			= IDE_DRIVE_TASK_IN;
 	args.data_phase				= TASKFILE_IN;
 	args.handler				= &task_in_intr;

@@ -808,9 +801,9 @@ static int write_cache(ide_drive_t *driv
 
 	if (ide_id_has_flush_cache(drive->id)) {
 		memset(&args, 0, sizeof(ide_task_t));
-		args.tfRegister[IDE_FEATURE_OFFSET]	= (arg) ?
+		args.tf.feature = arg ?
 			SETFEATURES_EN_WCACHE : SETFEATURES_DIS_WCACHE;
-		args.tfRegister[IDE_COMMAND_OFFSET]	= WIN_SETFEATURES;
+		args.tf.command = WIN_SETFEATURES;

Same here...

drivers/ide/ide-disk.c static int do_idedisk_flushcache (ide_dr · line 827

Quoted 12 lines
drivers/ide/ide-disk.c +2 -2
 		args.command_type		= IDE_DRIVE_TASK_NO_DATA;
 		args.handler			= &task_no_data_intr;
 		err = ide_raw_taskfile(drive, &args, NULL);

@@ -829,9 +822,9 @@ static int do_idedisk_flushcache (ide_dr
 
 	memset(&args, 0, sizeof(ide_task_t));
 	if (ide_id_has_flush_cache_ext(drive->id))
-		args.tfRegister[IDE_COMMAND_OFFSET]	= WIN_FLUSH_CACHE_EXT;
+		args.tf.command = WIN_FLUSH_CACHE_EXT;
 	else
-		args.tfRegister[IDE_COMMAND_OFFSET]	= WIN_FLUSH_CACHE;
+		args.tf.command = WIN_FLUSH_CACHE;

... and here...

Quoted 4 lines
drivers/ide/ide-disk.c +0 -0
 	args.command_type			= IDE_DRIVE_TASK_NO_DATA;
 	args.handler				= &task_no_data_intr;
 	return ide_raw_taskfile(drive, &args, NULL);

drivers/ide/ide-disk.c static int set_acoustic (ide_drive_t *dr · line 843

Quoted 10 lines
drivers/ide/ide-disk.c +3 -4
@@ -845,10 +838,9 @@ static int set_acoustic (ide_drive_t *dr
 		return -EINVAL;
 
 	memset(&args, 0, sizeof(ide_task_t));
-	args.tfRegister[IDE_FEATURE_OFFSET]	= (arg) ? SETFEATURES_EN_AAM :
-							  SETFEATURES_DIS_AAM;
-	args.tfRegister[IDE_NSECTOR_OFFSET]	= arg;
-	args.tfRegister[IDE_COMMAND_OFFSET]	= WIN_SETFEATURES;
+	args.tf.feature = arg ? SETFEATURES_EN_AAM : SETFEATURES_DIS_AAM;
+	args.tf.nsect   = arg;
+	args.tf.command = WIN_SETFEATURES;

... and here...

drivers/ide/ide-disk.c static int idedisk_open(struct inode *in · line 1110

Quoted 9 lines
drivers/ide/ide-disk.c +1 -1
 	args.command_type = IDE_DRIVE_TASK_NO_DATA;
 	args.handler	  = &task_no_data_intr;
 	ide_raw_taskfile(drive, &args, NULL);

@@ -1115,7 +1107,7 @@ static int idedisk_open(struct inode *in
 	if (drive->removable && idkp->openers == 1) {
 		ide_task_t args;
 		memset(&args, 0, sizeof(ide_task_t));
-		args.tfRegister[IDE_COMMAND_OFFSET] = WIN_DOORLOCK;
+		args.tf.command = WIN_DOORLOCK;

... and here...

drivers/ide/ide-disk.c static int idedisk_release(struct inode · line 1137

Quoted 9 lines
drivers/ide/ide-disk.c +1 -1
 		args.command_type = IDE_DRIVE_TASK_NO_DATA;
 		args.handler	  = &task_no_data_intr;
 		check_disk_change(inode->i_bdev);

@@ -1142,7 +1134,7 @@ static int idedisk_release(struct inode 
 	if (drive->removable && idkp->openers == 1) {
 		ide_task_t args;
 		memset(&args, 0, sizeof(ide_task_t));
-		args.tfRegister[IDE_COMMAND_OFFSET] = WIN_DOORUNLOCK;
+		args.tf.command = WIN_DOORUNLOCK;

... and here...

Quoted 13 lines
drivers/ide/ide-disk.c +0 -0
 		args.command_type = IDE_DRIVE_TASK_NO_DATA;
 		args.handler	  = &task_no_data_intr;
 		if (drive->doorlocking && ide_raw_taskfile(drive, &args, NULL))

Index: b/drivers/ide/ide-io.c
===================================================================

drivers/ide/ide-io.c +1 -1
@@ -189,15 +189,15 @@ static ide_startstop_t ide_start_power_s
 			return ide_stopped;
 		}
 		if (ide_id_has_flush_cache_ext(drive->id))
-			args->tfRegister[IDE_COMMAND_OFFSET] = WIN_FLUSH_CACHE_EXT;
+			args->tf.command = WIN_FLUSH_CACHE_EXT;

drivers/ide/ide-io.c static ide_startstop_t ide_start_power_s · line 200

Quoted 10 lines
drivers/ide/ide-io.c +2 -2
 		else
-			args->tfRegister[IDE_COMMAND_OFFSET] = WIN_FLUSH_CACHE;
+			args->tf.command = WIN_FLUSH_CACHE;
 		args->command_type = IDE_DRIVE_TASK_NO_DATA;
 		args->handler	   = &task_no_data_intr;
 		return do_rw_taskfile(drive, args);
 
 	case idedisk_pm_standby:	/* Suspend step 2 (standby) */
-		args->tfRegister[IDE_COMMAND_OFFSET] = WIN_STANDBYNOW1;
+		args->tf.command = WIN_STANDBYNOW1;

... and here...

Quoted 3 lines
drivers/ide/ide-io.c +0 -0
 		args->command_type = IDE_DRIVE_TASK_NO_DATA;
 		args->handler	   = &task_no_data_intr;
 		return do_rw_taskfile(drive, args);

[...]

drivers/ide/ide-io.c void ide_end_drive_cmd (ide_drive_t *dri · line 393

Quoted 11 lines
drivers/ide/ide-io.c +3 -3
@@ -387,28 +387,30 @@ void ide_end_drive_cmd (ide_drive_t *dri
 			rq->errors = !OK_STAT(stat,READY_STAT,BAD_STAT);
 
 		if (args) {
+			struct ide_taskfile *tf = &args->tf;
+
 			if (args->tf_in_flags.b.data) {
-				u16 data				= hwif->INW(IDE_DATA_REG);
-				args->tfRegister[IDE_DATA_OFFSET]	= (data) & 0xFF;
-				args->hobRegister[IDE_DATA_OFFSET]	= (data >> 8) & 0xFF;
+				u16 data = hwif->INW(IDE_DATA_REG);

Again, no newline after declaration block.

[...]

Quoted 5 lines
drivers/ide/ide-io.c +0 -1
@@ -707,28 +709,28 @@ static ide_startstop_t drive_cmd_intr (i
 
 static void ide_init_specify_cmd(ide_drive_t *drive, ide_task_t *task)
 {
-	task->tfRegister[IDE_NSECTOR_OFFSET] = drive->sect;

drivers/ide/ide-io.c static ide_startstop_t drive_cmd_intr (i · line 716

Quoted 10 lines
drivers/ide/ide-io.c +5 -5
-	task->tfRegister[IDE_SECTOR_OFFSET]  = drive->sect;
-	task->tfRegister[IDE_LCYL_OFFSET]    = drive->cyl;
-	task->tfRegister[IDE_HCYL_OFFSET]    = drive->cyl>>8;
-	task->tfRegister[IDE_SELECT_OFFSET]  = ((drive->head-1)|drive->select.all)&0xBF;
-	task->tfRegister[IDE_COMMAND_OFFSET] = WIN_SPECIFY;
+	task->tf.nsect   = drive->sect;
+	task->tf.lbal    = drive->sect;
+	task->tf.lbam    = drive->cyl;
+	task->tf.lbah    = drive->cyl >> 8;
+	task->tf.device  = ((drive->head - 1) | drive->select.all) & 0xBF;

Well, if you started using ATA_LBA, s/0xBF/~ATA_LBA.

Quoted 1 line
drivers/ide/ide-io.c +1 -0
+	task->tf.command = WIN_SPECIFY;

[...]

Quoted 4 lines

Index: b/drivers/ide/ide-taskfile.c
===================================================================

drivers/ide/ide-taskfile.c +0 -0

[...]

Quoted 25 lines
drivers/ide/ide-taskfile.c +6 -7
 	/* (ks) send hob registers first */
 	if (task->tf_out_flags.b.nsector_hob)
-		hwif->OUTB(hobfile->sector_count, IDE_NSECTOR_REG);
+		hwif->OUTB(tf->hob_nsect, IDE_NSECTOR_REG);
 	if (task->tf_out_flags.b.sector_hob)
-		hwif->OUTB(hobfile->sector_number, IDE_SECTOR_REG);
+		hwif->OUTB(tf->hob_lbal, IDE_SECTOR_REG);
 	if (task->tf_out_flags.b.lcyl_hob)
-		hwif->OUTB(hobfile->low_cylinder, IDE_LCYL_REG);
+		hwif->OUTB(tf->hob_lbam, IDE_LCYL_REG);
 	if (task->tf_out_flags.b.hcyl_hob)
-		hwif->OUTB(hobfile->high_cylinder, IDE_HCYL_REG);
+		hwif->OUTB(tf->hob_lbah, IDE_HCYL_REG);
 
 	/* (ks) Send now the standard registers */
 	if (task->tf_out_flags.b.error_feature)
-		hwif->OUTB(taskfile->feature, IDE_FEATURE_REG);
+		hwif->OUTB(tf->feature, IDE_FEATURE_REG);
 	/* refers to number of sectors to transfer */
 	if (task->tf_out_flags.b.nsector)
-		hwif->OUTB(taskfile->sector_count, IDE_NSECTOR_REG);
+		hwif->OUTB(tf->nsect, IDE_NSECTOR_REG);
 	/* refers to sector offset or start sector */
 	if (task->tf_out_flags.b.sector)
-		hwif->OUTB(taskfile->sector_number, IDE_SECTOR_REG);

drivers/ide/ide-taskfile.c ide_startstop_t flagged_taskfile (ide_dr · line 811

Quoted 10 lines
drivers/ide/ide-taskfile.c +3 -2
+		hwif->OUTB(tf->lbal, IDE_SECTOR_REG);
 	if (task->tf_out_flags.b.lcyl)
-		hwif->OUTB(taskfile->low_cylinder, IDE_LCYL_REG);
+		hwif->OUTB(tf->lbam, IDE_LCYL_REG);
 	if (task->tf_out_flags.b.hcyl)
-		hwif->OUTB(taskfile->high_cylinder, IDE_HCYL_REG);
+		hwif->OUTB(tf->lbah, IDE_HCYL_REG);
 
         /*
 	 * (ks) In the flagged taskfile approch, we will use all specified

Well, maybe it time to fix typo in approAch? :-)

Quoted 17 lines

Index: b/include/linux/ide.h
===================================================================

include/linux/ide.h +0 -2
@@ -103,8 +103,6 @@ typedef unsigned char	byte;	/* used ever
 #define IDE_FEATURE_OFFSET	IDE_ERROR_OFFSET
 #define IDE_COMMAND_OFFSET	IDE_STATUS_OFFSET
 
-#define IDE_CONTROL_OFFSET_HOB	(7)
-
 #define IDE_DATA_REG		(HWIF(drive)->io_ports[IDE_DATA_OFFSET])
 #define IDE_ERROR_REG		(HWIF(drive)->io_ports[IDE_ERROR_OFFSET])
 #define IDE_NSECTOR_REG		(HWIF(drive)->io_ports[IDE_NSECTOR_OFFSET])

@@ -1072,15 +1070,33 @@ extern void ide_end_drive_cmd(ide_drive_
  */
 extern int ide_wait_cmd(ide_drive_t *, u8, u8, u8, u8, u8 *);

include/linux/ide.h extern void ide_end_drive_cmd(ide_drive_ · line 1082

Quoted 10 lines
include/linux/ide.h +10 -0
+struct ide_taskfile {
+	u8	hob_data;	/*  0: high data byte (for TASKFILE IOCTL) */
+
+	u8	hob_feature;	/*  1-5: additional data to support LBA48 */
+	u8	hob_nsect;
+	u8	hob_lbal;
+	u8	hob_lbam;
+	u8	hob_lbah;
+
+	u8	data;		/*  6: low data byte (for TASKFILE IOCTL) */

I wonder who needs to read/write the data reg. (even with HOB)? :-O

MBR, Sergei

author
#2 lore raw
Quoted 29 lines · Sergei Shtylyov
Quoted 10 lines · Bartlomiej Zolnierkiewicz

* Don't set write-only ide_task_t.hobRegister[6] and ide_task_t.hobRegister[7] in idedisk_set_max_address_ext().

* Add struct ide_taskfile and use it in ide_task_t instead of tfRegister[] and hobRegister[].

* Remove no longer needed IDE_CONTROL_OFFSET_HOB define.

* Add #ifndef/#endif __KERNEL__ around definitions of {task,hob}_struct_t.

Quoted 7 lines

While at it:

* Use ATA_LBA define for LBA bit (0x40) as suggested by Tejun Heo.

There should be no functionality changes caused by this patch.

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
Nearly-acked-by Sergei Shtylyov <[email protected]>
Quoted 4 lines

Index: b/drivers/ide/ide-disk.c
===================================================================

drivers/ide/ide-disk.c +0 -0

[...]

Quoted 4 lines

drivers/ide/ide-disk.c

Quoted 2 lines
drivers/ide/ide-disk.c +2 -0
+	if ((tf->command & 0x01) == 0) {
+		u32 high, low;

Isn't newline needed after declarations?

Well, it is not stricly needed, this code should compile and work without it.

;)

fixed (FWIW the original code also lacked newline)

Quoted 10 lines
Quoted 5 lines
drivers/ide/ide-disk.c +2 -0
+		high = (tf->hob_lbah << 16) | (tf->hob_lbam << 8) | tf->hob_lbal;
+		low  = (tf->lbah << 16) | (tf->lbam << 8) | tf->lbal;
 		addr = ((__u64)high << 24) | low;
 		addr++;	/* since the return value is (maxlba - 1), we add 1 */
 	}

[...]

Quoted 1 line
drivers/ide/ide-disk.c +0 -0
@@ -422,33 +417,29 @@ static unsigned long idedisk_set_max_add

[...]

Quoted 12 lines

drivers/ide/ide-disk.c

Quoted 10 lines
drivers/ide/ide-disk.c +2 -7
 	/* if OK, compute maximum address value */
-	if ((args.tfRegister[IDE_STATUS_OFFSET] & 0x01) == 0) {
-		u32 high = (args.hobRegister[IDE_HCYL_OFFSET] << 16) |
-			   (args.hobRegister[IDE_LCYL_OFFSET] <<  8) |
-			    args.hobRegister[IDE_SECTOR_OFFSET];
-		u32 low  = ((args.tfRegister[IDE_HCYL_OFFSET])<<16) |
-			   ((args.tfRegister[IDE_LCYL_OFFSET])<<8) |
-			    (args.tfRegister[IDE_SECTOR_OFFSET]);
+	if ((tf->command & 0x01) == 0) {
+		u32 high, low;

Again missing newline...

fixed

Quoted 5 lines
Quoted 5 lines
drivers/ide/ide-disk.c +2 -0
+		high = (tf->hob_lbah << 16) | (tf->hob_lbam << 8) | tf->hob_lbal;
+		low  = (tf->lbah << 16) | (tf->lbam << 8) | tf->lbal;
 		addr_set = ((__u64)high << 24) | low;
 		addr_set++;
 	}

[...]

Quoted 6 lines
Quoted 6 lines
drivers/ide/ide-disk.c +1 -0
@@ -596,13 +588,14 @@ static int smart_enable(ide_drive_t *dri
 static int get_smart_data(ide_drive_t *drive, u8 *buf, u8 sub_cmd)
 {
 	ide_task_t args;
+	struct ide_taskfile *tf = &args.tf;
Quoted 17 lines

drivers/ide/ide-disk.c static int smart_enable(ide_drive_t *dri · line 598

Quoted 11 lines
drivers/ide/ide-disk.c +5 -5
 	memset(&args, 0, sizeof(ide_task_t));
-	args.tfRegister[IDE_FEATURE_OFFSET]	= sub_cmd;
-	args.tfRegister[IDE_NSECTOR_OFFSET]	= 0x01;
-	args.tfRegister[IDE_LCYL_OFFSET]	= SMART_LCYL_PASS;
-	args.tfRegister[IDE_HCYL_OFFSET]	= SMART_HCYL_PASS;
-	args.tfRegister[IDE_COMMAND_OFFSET]	= WIN_SMART;
+	tf->feature = sub_cmd;
+	tf->nsect   = 0x01;
+	tf->lbam    = SMART_LCYL_PASS;
+	tf->lbah    = SMART_HCYL_PASS;
+	tf->command = WIN_SMART;

I guess the code above and below were menat to have the same = indentation...

Quoted 3 lines
drivers/ide/ide-disk.c +0 -0
 	args.command_type			= IDE_DRIVE_TASK_IN;
 	args.data_phase				= TASKFILE_IN;
 	args.handler				= &task_in_intr;

This is to distinguish struct ide_taskfile initialization from ide_task_t initialization. It would be also just a pointless noise since patch #11/12 ("ide: add ide_no_data_taskfile() helper") cleanups such places (with the only exception being this one because it uses IDE_DRIVE_TASK_IN but some of the future patches should address it as well).

[...]

Quoted 17 lines
Quoted 17 lines

Index: b/drivers/ide/ide-io.c
===================================================================

drivers/ide/ide-io.c +2 -2
@@ -189,15 +189,15 @@ static ide_startstop_t ide_start_power_s
 			return ide_stopped;
 		}
 		if (ide_id_has_flush_cache_ext(drive->id))
-			args->tfRegister[IDE_COMMAND_OFFSET] = WIN_FLUSH_CACHE_EXT;
+			args->tf.command = WIN_FLUSH_CACHE_EXT;
 		else
-			args->tfRegister[IDE_COMMAND_OFFSET] = WIN_FLUSH_CACHE;
+			args->tf.command = WIN_FLUSH_CACHE;
 		args->command_type = IDE_DRIVE_TASK_NO_DATA;
 		args->handler	   = &task_no_data_intr;
 		return do_rw_taskfile(drive, args);
Quoted 5 lines

drivers/ide/ide-io.c static ide_startstop_t ide_start_power_s · line 200

Quoted 3 lines
drivers/ide/ide-io.c +1 -1
 	case idedisk_pm_standby:	/* Suspend step 2 (standby) */
-		args->tfRegister[IDE_COMMAND_OFFSET] = WIN_STANDBYNOW1;
+		args->tf.command = WIN_STANDBYNOW1;

    ... and here...

ditto but this one is cleaned up by patch #12/12 ("ide: use do_rw_taskfile() in flagged_taskfile()")

Quoted 7 lines
Quoted 3 lines
drivers/ide/ide-io.c +0 -0
 		args->command_type = IDE_DRIVE_TASK_NO_DATA;
 		args->handler	   = &task_no_data_intr;
 		return do_rw_taskfile(drive, args);

[...]

Quoted 3 lines
drivers/ide/ide-io.c +0 -0
@@ -387,28 +387,30 @@ void ide_end_drive_cmd (ide_drive_t *dri
 			rq->errors = !OK_STAT(stat,READY_STAT,BAD_STAT);
Quoted 10 lines

drivers/ide/ide-io.c void ide_end_drive_cmd (ide_drive_t *dri · line 393

Quoted 8 lines
drivers/ide/ide-io.c +3 -3
 		if (args) {
+			struct ide_taskfile *tf = &args->tf;
+
 			if (args->tf_in_flags.b.data) {
-				u16 data				= hwif->INW(IDE_DATA_REG);
-				args->tfRegister[IDE_DATA_OFFSET]	= (data) & 0xFF;
-				args->hobRegister[IDE_DATA_OFFSET]	= (data >> 8) & 0xFF;
+				u16 data = hwif->INW(IDE_DATA_REG);

Again, no newline after declaration block.

fixed

Quoted 4 lines

[...]

Quoted 2 lines
drivers/ide/ide-io.c +0 -0
@@ -707,28 +709,28 @@ static ide_startstop_t drive_cmd_intr (i
Quoted 15 lines

drivers/ide/ide-io.c static ide_startstop_t drive_cmd_intr (i · line 716

Quoted 13 lines
drivers/ide/ide-io.c +5 -6
 static void ide_init_specify_cmd(ide_drive_t *drive, ide_task_t *task)
 {
-	task->tfRegister[IDE_NSECTOR_OFFSET] = drive->sect;
-	task->tfRegister[IDE_SECTOR_OFFSET]  = drive->sect;
-	task->tfRegister[IDE_LCYL_OFFSET]    = drive->cyl;
-	task->tfRegister[IDE_HCYL_OFFSET]    = drive->cyl>>8;
-	task->tfRegister[IDE_SELECT_OFFSET]  = ((drive->head-1)|drive->select.all)&0xBF;
-	task->tfRegister[IDE_COMMAND_OFFSET] = WIN_SPECIFY;
+	task->tf.nsect   = drive->sect;
+	task->tf.lbal    = drive->sect;
+	task->tf.lbam    = drive->cyl;
+	task->tf.lbah    = drive->cyl >> 8;
+	task->tf.device  = ((drive->head - 1) | drive->select.all) & 0xBF;

    Well, if you started using ATA_LBA, s/0xBF/~ATA_LBA.

thanks, fixed

Quoted 42 lines
Quoted 1 line
drivers/ide/ide-io.c +1 -0
+	task->tf.command = WIN_SPECIFY;

[...]

Quoted 4 lines

Index: b/drivers/ide/ide-taskfile.c
===================================================================

drivers/ide/ide-taskfile.c +0 -0

[...]

Quoted 33 lines
drivers/ide/ide-taskfile.c +9 -9
 	/* (ks) send hob registers first */
 	if (task->tf_out_flags.b.nsector_hob)
-		hwif->OUTB(hobfile->sector_count, IDE_NSECTOR_REG);
+		hwif->OUTB(tf->hob_nsect, IDE_NSECTOR_REG);
 	if (task->tf_out_flags.b.sector_hob)
-		hwif->OUTB(hobfile->sector_number, IDE_SECTOR_REG);
+		hwif->OUTB(tf->hob_lbal, IDE_SECTOR_REG);
 	if (task->tf_out_flags.b.lcyl_hob)
-		hwif->OUTB(hobfile->low_cylinder, IDE_LCYL_REG);
+		hwif->OUTB(tf->hob_lbam, IDE_LCYL_REG);
 	if (task->tf_out_flags.b.hcyl_hob)
-		hwif->OUTB(hobfile->high_cylinder, IDE_HCYL_REG);
+		hwif->OUTB(tf->hob_lbah, IDE_HCYL_REG);
 
 	/* (ks) Send now the standard registers */
 	if (task->tf_out_flags.b.error_feature)
-		hwif->OUTB(taskfile->feature, IDE_FEATURE_REG);
+		hwif->OUTB(tf->feature, IDE_FEATURE_REG);
 	/* refers to number of sectors to transfer */
 	if (task->tf_out_flags.b.nsector)
-		hwif->OUTB(taskfile->sector_count, IDE_NSECTOR_REG);
+		hwif->OUTB(tf->nsect, IDE_NSECTOR_REG);
 	/* refers to sector offset or start sector */
 	if (task->tf_out_flags.b.sector)
-		hwif->OUTB(taskfile->sector_number, IDE_SECTOR_REG);
+		hwif->OUTB(tf->lbal, IDE_SECTOR_REG);
 	if (task->tf_out_flags.b.lcyl)
-		hwif->OUTB(taskfile->low_cylinder, IDE_LCYL_REG);
+		hwif->OUTB(tf->lbam, IDE_LCYL_REG);
 	if (task->tf_out_flags.b.hcyl)
-		hwif->OUTB(taskfile->high_cylinder, IDE_HCYL_REG);
+		hwif->OUTB(tf->lbah, IDE_HCYL_REG);
Quoted 4 lines
Quoted 2 lines
         /*

* (ks) In the flagged taskfile approch, we will use all specified

Well, maybe it time to fix typo in approAch? :-)

fixed in patch #12/12 which just removes this comment along with the majority of flagged_taskfile()'s content :)

Quoted 17 lines
Quoted 17 lines

Index: b/include/linux/ide.h
===================================================================

include/linux/ide.h +0 -2
@@ -103,8 +103,6 @@ typedef unsigned char	byte;	/* used ever
 #define IDE_FEATURE_OFFSET	IDE_ERROR_OFFSET
 #define IDE_COMMAND_OFFSET	IDE_STATUS_OFFSET
 
-#define IDE_CONTROL_OFFSET_HOB	(7)
-
 #define IDE_DATA_REG		(HWIF(drive)->io_ports[IDE_DATA_OFFSET])
 #define IDE_ERROR_REG		(HWIF(drive)->io_ports[IDE_ERROR_OFFSET])
 #define IDE_NSECTOR_REG		(HWIF(drive)->io_ports[IDE_NSECTOR_OFFSET])

@@ -1072,15 +1070,33 @@ extern void ide_end_drive_cmd(ide_drive_
  */
 extern int ide_wait_cmd(ide_drive_t *, u8, u8, u8, u8, u8 *);
Quoted 12 lines

include/linux/ide.h extern void ide_end_drive_cmd(ide_drive_ · line 1082

Quoted 10 lines
include/linux/ide.h +10 -0
+struct ide_taskfile {
+	u8	hob_data;	/*  0: high data byte (for TASKFILE IOCTL) */
+
+	u8	hob_feature;	/*  1-5: additional data to support LBA48 */
+	u8	hob_nsect;
+	u8	hob_lbal;
+	u8	hob_lbam;
+	u8	hob_lbah;
+
+	u8	data;		/*  6: low data byte (for TASKFILE IOCTL) */

I wonder who needs to read/write the data reg. (even with HOB)? :-O

HDIO_DRIVE_TASKFILE ioctl, unfortunately this is all that is known :(

New revision of the patch (it also uses the suggestion about using unnamed unions for error/feature and status/command, thanks!):

[PATCH] ide: add struct ide_taskfile (take 2)

* Don't set write-only ide_task_t.hobRegister[6] and ide_task_t.hobRegister[7] in idedisk_set_max_address_ext().

* Add struct ide_taskfile and use it in ide_task_t instead of tfRegister[] and hobRegister[].

* Remove no longer needed IDE_CONTROL_OFFSET_HOB define.

* Add #ifndef/#endif __KERNEL__ around definitions of {task,hob}_struct_t.

While at it:

* Use ATA_LBA define for LBA bit (0x40) as suggested by Tejun Heo.

v2:
* Add missing newlines. (Noticed by Sergei)

* Use ~ATA_LBA instead of 0xBF. (Noticed by Sergei)

* Use unnamed unions for error/feature and status/command. (Suggested by Sergei).

There should be no functionality changes caused by this patch.

Cc Tejun Heo <[email protected]>
Cc Sergei Shtylyov <[email protected]>
Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
---
 drivers/ide/ide-disk.c     |  156 ++++++++++++++++++++++++---------------------
 drivers/ide/ide-io.c       |   80 +++++++++++++----------
 drivers/ide/ide-iops.c     |   12 +--
 drivers/ide/ide-lib.c      |    3
 drivers/ide/ide-acpi.c     |    8 ---
 drivers/ide/ide-disk.c     |  120 +++++++++++++++++++++------------------------
 drivers/ide/ide-io.c       |   61 ++++++++++++----------
 drivers/ide/ide-iops.c     |   12 ++--
 drivers/ide/ide-lib.c      |    3 -
 drivers/ide/ide-taskfile.c |   99 +++++++++++++++++--------------------
 include/linux/hdreg.h      |    4 +
 include/linux/ide.h        |   43 ++++++++++++----
 8 files changed, 179 insertions(+), 171 deletions(-)
drivers/ide/ide-acpi.c +1 -7
Index: b/drivers/ide/ide-acpi.c
===================================================================
@@ -354,13 +354,7 @@ static int taskfile_load_raw(ide_drive_t
 	args.handler      = &task_no_data_intr;
 
 	/* convert gtf to IDE Taskfile */
-	args.tfRegister[1] = gtf->tfa[0];	/* 0x1f1 */
-	args.tfRegister[2] = gtf->tfa[1];	/* 0x1f2 */
-	args.tfRegister[3] = gtf->tfa[2];	/* 0x1f3 */
-	args.tfRegister[4] = gtf->tfa[3];	/* 0x1f4 */
-	args.tfRegister[5] = gtf->tfa[4];	/* 0x1f5 */
-	args.tfRegister[6] = gtf->tfa[5];	/* 0x1f6 */
-	args.tfRegister[7] = gtf->tfa[6];	/* 0x1f7 */
+	memcpy(&args.tf_array[7], &gtf->tfa, 7);
 
 	if (ide_noacpitfs) {
 		DEBPRINT("_GTF execution disabled\n");
drivers/ide/ide-disk.c +57 -63

232 lines not on this page · raw message

drivers/ide/ide-io.c +32 -29

110 lines not on this page · raw message

drivers/ide/ide-iops.c +6 -6
Index: b/drivers/ide/ide-iops.c
===================================================================
@@ -638,9 +638,9 @@ no_80w:
 
 int ide_ata66_check (ide_drive_t *drive, ide_task_t *args)
 {
-	if ((args->tfRegister[IDE_COMMAND_OFFSET] == WIN_SETFEATURES) &&
-	    (args->tfRegister[IDE_SECTOR_OFFSET] > XFER_UDMA_2) &&
-	    (args->tfRegister[IDE_FEATURE_OFFSET] == SETFEATURES_XFER)) {
+	if (args->tf.command == WIN_SETFEATURES &&
+	    args->tf.lbal > XFER_UDMA_2 &&
+	    args->tf.feature == SETFEATURES_XFER) {
 		if (eighty_ninty_three(drive) == 0) {
 			printk(KERN_WARNING "%s: UDMA speeds >UDMA33 cannot "
 					    "be set\n", drive->name);

@@ -658,9 +658,9 @@ int ide_ata66_check (ide_drive_t *drive,
  */
 int set_transfer (ide_drive_t *drive, ide_task_t *args)
 {
-	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 (args->tf.command == WIN_SETFEATURES &&
+	    args->tf.lbal >= XFER_SW_DMA_0 &&
+	    args->tf.feature == SETFEATURES_XFER &&
 	    (drive->id->dma_ultra ||
 	     drive->id->dma_mword ||
 	     drive->id->dma_1word))
drivers/ide/ide-lib.c +1 -2
Index: b/drivers/ide/ide-lib.c
===================================================================
@@ -465,8 +465,7 @@ static void ide_dump_opcode(ide_drive_t 
 	} else if (rq->cmd_type == REQ_TYPE_ATA_TASKFILE) {
 		ide_task_t *args = rq->special;
 		if (args) {
-			task_struct_t *tf = (task_struct_t *) args->tfRegister;
-			opcode = tf->command;
+			opcode = args->tf.command;
 			found = 1;
 		}
 	}
drivers/ide/ide-taskfile.c +46 -53

226 lines not on this page · raw message

include/linux/hdreg.h +3 -1
Index: b/include/linux/hdreg.h
===================================================================
@@ -117,7 +117,7 @@ typedef union ide_reg_valid_s {
 
 typedef struct ide_task_request_s {
 	__u8		io_ports[8];
-	__u8		hob_ports[8];
+	__u8		hob_ports[8]; /* bytes 6 and 7 are unused */
 	ide_reg_valid_t	out_flags;
 	ide_reg_valid_t	in_flags;
 	int		data_phase;

@@ -139,6 +139,7 @@ struct hd_drive_cmd_hdr {
 	__u8 sector_count;
 };
 
+#ifndef __KERNEL__
 typedef struct hd_drive_task_hdr {
 	__u8 data;
 	__u8 feature;

@@ -160,6 +161,7 @@ typedef struct hd_drive_hob_hdr {
 	__u8 device_head;
 	__u8 control;
 } hob_struct_t;
+#endif
 
 #define TASKFILE_INVALID		0x7fff
 #define TASKFILE_48			0x8000
include/linux/ide.h +33 -10

56 lines not on this page · raw message

Acked-by
#3 lore raw
Quoted 12 lines · Bartlomiej Zolnierkiewicz
Quoted 11 lines
Quoted 4 lines

Index: b/drivers/ide/ide-disk.c
===================================================================

drivers/ide/ide-disk.c +0 -0

[...]

drivers/ide/ide-disk.c

Quoted 2 lines
drivers/ide/ide-disk.c +2 -0
+	if ((tf->command & 0x01) == 0) {
+		u32 high, low;

Isn't newline needed after declarations?

Quoted 5 lines

Well, it is not stricly needed, this code should compile and work without it.

;)

fixed (FWIW the original code also lacked newline)

Yeah, I saw but that's hardly worth imitating. :-)

[...]

Quoted 22 lines

New revision of the patch (it also uses the suggestion about using unnamed unions for error/feature and status/command, thanks!):

[PATCH] ide: add struct ide_taskfile (take 2)

* Don't set write-only ide_task_t.hobRegister[6] and ide_task_t.hobRegister[7] in idedisk_set_max_address_ext().

* Add struct ide_taskfile and use it in ide_task_t instead of tfRegister[] and hobRegister[].

* Remove no longer needed IDE_CONTROL_OFFSET_HOB define.

* Add #ifndef/#endif __KERNEL__ around definitions of {task,hob}_struct_t.

While at it:

* Use ATA_LBA define for LBA bit (0x40) as suggested by Tejun Heo.

v2:
* Add missing newlines. (Noticed by Sergei)

Quoted 8 lines

* Use ~ATA_LBA instead of 0xBF. (Noticed by Sergei)

* Use unnamed unions for error/feature and status/command. (Suggested by Sergei).

There should be no functionality changes caused by this patch.

Signed-off-by Bartlomiej Zolnierkiewicz <[email protected]>
Acked-by Sergei Shtylyov <[email protected]>
Quoted 10 lines
---
 drivers/ide/ide-disk.c     |  156 ++++++++++++++++++++++++---------------------
 drivers/ide/ide-io.c       |   80 +++++++++++++----------
 drivers/ide/ide-iops.c     |   12 +--
 drivers/ide/ide-lib.c      |    3
 drivers/ide/ide-acpi.c     |    8 ---
 drivers/ide/ide-disk.c     |  120 +++++++++++++++++++++------------------------
 drivers/ide/ide-io.c       |   61 ++++++++++++----------
 drivers/ide/ide-iops.c     |   12 ++--
 drivers/ide/ide-lib.c      |    3 -

Don't know how but ide-disk.c, ide-io[ps].c and ide-lib.c appear twice in
the diffstat output... 8-)

MBR, Sergei

#4 lore raw
In reply to Bartlomiej Zolnierkiewicz

Hello.

Quoted 8 lines · Bartlomiej Zolnierkiewicz

* Don't set write-only ide_task_t.hobRegister[6] and ide_task_t.hobRegister[7] in idedisk_set_max_address_ext().

* Add struct ide_taskfile and use it in ide_task_t instead of tfRegister[] and hobRegister[].

* Remove no longer needed IDE_CONTROL_OFFSET_HOB define.

Quoted 7 lines

* Add #ifndef/#endif __KERNEL__ around definitions of {task,hob}_struct_t.

While at it:

* Use ATA_LBA define for LBA bit (0x40) as suggested by Tejun Heo.

There should be no functionality changes caused by this patch.

There's some idea too...

Quoted 4 lines

Index: b/include/linux/ide.h
===================================================================

include/linux/ide.h +0 -0

[...]

Quoted 6 lines
include/linux/ide.h +2 -0
@@ -1072,15 +1070,33 @@ extern void ide_end_drive_cmd(ide_drive_
  */
 extern int ide_wait_cmd(ide_drive_t *, u8, u8, u8, u8, u8 *);
 
+struct ide_taskfile {
+	u8	hob_data;	/*  0: high data byte (for TASKFILE IOCTL) */

include/linux/ide.h extern void ide_end_drive_cmd(ide_drive_ · line 1084

Quoted 10 lines
include/linux/ide.h +10 -0
+
+	u8	hob_feature;	/*  1-5: additional data to support LBA48 */
+	u8	hob_nsect;
+	u8	hob_lbal;
+	u8	hob_lbam;
+	u8	hob_lbah;
+
+	u8	data;		/*  6: low data byte (for TASKFILE IOCTL) */
+
+	u8	feature;

Why not use unnamed union like this:

	union {				/*  7: feature */
		u8	error;		/*  read: error   */
		u8	feature;	/* write: feature */
	};

include/linux/ide.h extern void ide_end_drive_cmd(ide_drive_ · line 1092

Quoted 1 line
include/linux/ide.h +1 -0
+	u8	command;	/* 13: command */

	union {				/* 13 */
		u8	status;		/*  read: status  */
		u8	command;	/* write: command */
	};

This should qualify some places where you have to check for command
register for the ERR bit, etc...

include/linux/ide.h extern void ide_end_drive_cmd(ide_drive_ · line 1086

Quoted 1 line
include/linux/ide.h +1 -0
+};

MBR, Sergei