resolve ACPI oops on boot

An u8 was casted into an u32, then all 32 bits were set to zero, this causing another variable - in my case, processor flags - to be corrupted.

Dominik

@@ -134,7 +134,7 @@
 acpi_ec_read (
 	struct acpi_ec		*ec,
 	u8			address,
-	u8			*data)
+	u32			*data)
 {
 	acpi_status		status = AE_OK;
 	int			result = 0;

@@ -167,7 +167,7 @@
 		goto end;
 
 
-	acpi_hw_low_level_read(8, (u32*) data, &ec->data_addr, 0);
+	acpi_hw_low_level_read(8, data, &ec->data_addr, 0);
 
 	ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Read [%02x] from address [%02x]\n",
 		*data, address));

@@ -237,7 +237,7 @@
 static int
 acpi_ec_query (
 	struct acpi_ec		*ec,
-	u8			*data)
+	u32			*data)
 {
 	int			result = 0;
 	acpi_status		status = AE_OK;

@@ -269,7 +269,7 @@
 	if (result)
 		goto end;
 	
-	acpi_hw_low_level_read(8, (u32*) data, &ec->data_addr, 0);
+	acpi_hw_low_level_read(8, data, &ec->data_addr, 0);
 	if (!*data)
 		result = -ENODATA;
 

@@ -328,7 +328,7 @@
 {
 	acpi_status		status = AE_OK;
 	struct acpi_ec		*ec = (struct acpi_ec *) data;
-	u8			value = 0;
+	u32			value = 0;
 	unsigned long		flags = 0;
 	struct acpi_ec_query_data *query_data = NULL;
 

@@ -336,7 +336,7 @@
 		return;
 
 	spin_lock_irqsave(&ec->lock, flags);
-	acpi_hw_low_level_read(8, (u32*) &value, &ec->command_addr, 0);
+	acpi_hw_low_level_read(8, &value, &ec->command_addr, 0);
 	spin_unlock_irqrestore(&ec->lock, flags);
 
 	/* TBD: Implement asynch events!

@@ -398,6 +398,7 @@
 {
 	int			result = 0;
 	struct acpi_ec		*ec = NULL;
+	u32          		tmp = 0;
 
 	ACPI_FUNCTION_TRACE("acpi_ec_space_handler");
 

@@ -408,7 +409,8 @@
 
 	switch (function) {
 	case ACPI_READ:
-		result = acpi_ec_read(ec, (u8) address, (u8*) value);
+		result = acpi_ec_read(ec, (u8) address, &tmp);
+		*value = (acpi_integer) tmp;
 		break;
 	case ACPI_WRITE:
 		result = acpi_ec_write(ec, (u8) address, (u8) *value);

But that's not what's happening here, the pointer is being cast, if the object of the pointer is not u32, then casting the pointer doesn't fix the real problem. If the "data" pointer points to a u8, then no casting will make it work right when you save 32 bits into an 8 bit space. If this changes the problem it's because of alignment, perhaps.

You give neither the kernel version nor the architecture, so I can't be sure what's happening or what the compiler might do. I don't find that code in the kernel I have on this machine (2.4.19-pre7-jam6) but that diesn't mean much. The routine in hardware/hwregs.c on my kernel would seem to pass the width correctly, but clearly this is a very different version.

I think the cast is just to avoid the compiler whining about types, the version I have is actually type "(acpi_generic_address*)" not (u32*), I would think the compiler would still complain, but maybe only with -pedantic or whatever.

In any case only the number of bits requested should be written, the problem may have been avoided rather than fixed.

bill davidsen <[email protected]>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

There is the argument "size" to acpi_hw_low_level_read which makes sure that only the right data is being read. Problem is that a sort of "segmentation fault" occurs when such a cast allows writing in different memory than allocated for the value.

kernel version 2.5.2[5,6] or 2.4.-kernels with acpi-patch 20020709,
architecture: all architectures that implement Embedded Controllers.

The casts were probably introduced for that reason. Per se, they are not critical - but if there is any assumption later on that the data structure is indeed of the large size, there is a problem.

Dominik

Perhaps the pointer chould be (void *) everywhere, and then cast to the right size where and when it is used. Clearly casting the pointer to a larger size will result in allignment issues on some machines.

bill davidsen <[email protected]>
  CTO, TMR Associates, Inc
Doing interesting things with little computers since 1979.

Actually, I found this patch accidently since it didn't list the kernel version but it exactly fixed my ACPI oops I was getting on boots of the 2.5.2[6-7] kernels. I posted the relevent oops about a week ago under the subject "2.5.2[6-7] ACPI oops" but I can send it to you if you're interested

  Matthew Harrell                          Any sufficiently advanced technology
  Bit Twiddlers, Inc.                       is indistinguishable from a rigged
  [email protected]                 demo.