Re: [PATCH 2.6.11-rc2 07/29] ide: ide_reg_valid_t endian fix

[Tejun Heo]

> 07_ide_reg_valid_t_endian_fix.patch
> 
> 	ide_reg_valid_t contains bitfield flags but doesn't reverse
> 	bit orders using __*_ENDIAN_BITFIELD macros.  And constants
> 	for ide_reg_valid_t, IDE_{TASKFILE|HOB}_STD_{IN|OUT}_FLAGS,
> 	are defined as byte values which are correct only on
> 	little-endian machines.  This patch defines reversed constants
> 	and .h byte union structure to make things correct on big
> 	endian machines.  The only code which uses above macros is in
> 	flagged_taskfile() and the code is currently unused, so this
> 	patch doesn't change any behavior.  (The code will get used in
> 	later patches.)


Signed-off-by: Tejun Heo <tj@xxxxxxxxxxx>


Index: linux-ide-export/drivers/ide/ide-taskfile.c
===================================================================
--- linux-ide-export.orig/drivers/ide/ide-taskfile.c	2005-02-02 10:27:15.930195526 +0900
+++ linux-ide-export/drivers/ide/ide-taskfile.c	2005-02-02 10:28:03.518474705 +0900
@@ -794,15 +794,15 @@ ide_startstop_t flagged_taskfile (ide_dr
 	 *	write and read the hob registers (sector,nsector,lcyl,hcyl)
 	 */
 	if (task->tf_out_flags.all == 0) {
-		task->tf_out_flags.all = IDE_TASKFILE_STD_OUT_FLAGS;
+		task->tf_out_flags.h.taskfile = IDE_TASKFILE_STD_OUT_FLAGS;
 		if (drive->addressing == 1)
-			task->tf_out_flags.all |= (IDE_HOB_STD_OUT_FLAGS << 8);
+			task->tf_out_flags.h.hob = IDE_HOB_STD_OUT_FLAGS;
         }
 
 	if (task->tf_in_flags.all == 0) {
-		task->tf_in_flags.all = IDE_TASKFILE_STD_IN_FLAGS;
+		task->tf_in_flags.h.taskfile = IDE_TASKFILE_STD_IN_FLAGS;
 		if (drive->addressing == 1)
-			task->tf_in_flags.all |= (IDE_HOB_STD_IN_FLAGS  << 8);
+			task->tf_in_flags.h.hob = IDE_HOB_STD_IN_FLAGS;
         }
 
 	/* ALL Command Block Executions SHALL clear nIEN, unless otherwise */
Index: linux-ide-export/include/linux/hdreg.h
===================================================================
--- linux-ide-export.orig/include/linux/hdreg.h	2005-02-02 10:27:15.931195364 +0900
+++ linux-ide-export/include/linux/hdreg.h	2005-02-02 10:28:03.519474543 +0900
@@ -1,6 +1,8 @@
 #ifndef _LINUX_HDREG_H
 #define _LINUX_HDREG_H
 
+#include <asm/byteorder.h>
+
 #ifdef __KERNEL__
 #include <linux/ata.h>
 
@@ -79,11 +81,26 @@
 
 /*
  * Define standard taskfile in/out register
+ *
+ * ide_reg_valid_t should have been defined conditionally using
+ * __*_ENDIAN_BITFIELD macros.  But ide_reg_valid_t and the following
+ * macros are already out in the wild.  Defining reversed constants on
+ * big endian machines and adding .h.{taskfile|hob} to ide_reg_valid_t
+ * seem to be the least disrupting solution. - tj
  */
+#if defined(__LITTLE_ENDIAN_BITFIELD)
 #define IDE_TASKFILE_STD_OUT_FLAGS	0xFE
 #define IDE_TASKFILE_STD_IN_FLAGS	0xFE
 #define IDE_HOB_STD_OUT_FLAGS		0x3C
 #define IDE_HOB_STD_IN_FLAGS		0x3C
+#elif defined(__BIG_ENDIAN_BITFIELD)
+#define IDE_TASKFILE_STD_OUT_FLAGS	0x7F
+#define IDE_TASKFILE_STD_IN_FLAGS	0x7F
+#define IDE_HOB_STD_OUT_FLAGS		0x3C
+#define IDE_HOB_STD_IN_FLAGS		0x3C
+#else
+#error "Please fix <asm/byteorder.h>"
+#endif
 
 typedef unsigned char task_ioreg_t;
 typedef unsigned long sata_ioreg_t;
@@ -91,6 +108,10 @@ typedef unsigned long sata_ioreg_t;
 typedef union ide_reg_valid_s {
 	unsigned all				: 16;
 	struct {
+		unsigned taskfile		: 8;
+		unsigned hob			: 8;
+	} h;
+	struct {
 		unsigned data			: 1;
 		unsigned error_feature		: 1;
 		unsigned sector			: 1;
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/