acpi compile fix

PATCH In review Andrew Morton 3 participants 5 messages Apr 3, 2003 – Apr 4, 2003 ACPI

Share & tools

Andrew Morton

author

2003-04-03 20:54 UTC #0 lore raw

PATCH 1/1 acpi compile fix

ACPI is performing a spin_lock() on a `void *'. That's OK when spin_lock is implemented via an inline function. But when it is implemented via macros (eg, with spinlock debugging enabled) we get:

drivers/acpi/osl.c:739: warning: dereferencing void *' pointer drivers/acpi/osl.c:739: request for member owner' in something not a structure or union

So cast it to the right type.

drivers/acpi/osl.c renamed from drivers/acpi/osl.c~acpi-spinlock-casts +2 -2

diff -puN drivers/acpi/osl.c~acpi-spinlock-casts drivers/acpi/osl.c
@@ -736,7 +736,7 @@ acpi_os_acquire_lock (
 	if (flags & ACPI_NOT_ISR)
 		ACPI_DISABLE_IRQS();
 
-	spin_lock(handle);
+	spin_lock((spinlock_t *)handle);
 
 	return_VOID;
 }

@@ -755,7 +755,7 @@ acpi_os_release_lock (
 	ACPI_DEBUG_PRINT ((ACPI_DB_MUTEX, "Releasing spinlock[%p] from %s level\n", handle,
 		((flags & ACPI_NOT_ISR) ? "non-interrupt" : "interrupt")));
 
-	spin_unlock(handle);
+	spin_unlock((spinlock_t *)handle);
 
 	if (flags & ACPI_NOT_ISR)
 		ACPI_ENABLE_IRQS();

J Sloan

2003-04-04 06:37 UTC #1 lore raw

Quoted 9 lines · Andrew Morton

ACPI is performing a spin_lock() on a `void *'. That's OK when spin_lock is implemented via an inline function. But when it is implemented via macros (eg, with spinlock debugging enabled) we get:
drivers/acpi/osl.c:739: warning: dereferencing void *' pointer drivers/acpi/osl.c:739: request for member owner' in something not a structure or union
So cast it to the right type.

Yep 2.5.66 is happy with my config now
and is running nicely - thanks for the fix!

Joe

Dave Jones

2003-04-04 12:10 UTC #2 lore raw

In reply to Andrew Morton

drivers/acpi/osl.c acpi_os_acquire_lock ( · line 739

Quoted 9 lines · Andrew Morton

diff -puN drivers/acpi/osl.c~acpi-spinlock-casts drivers/acpi/osl.c
drivers/acpi/osl.c renamed from drivers/acpi/osl.c~acpi-spinlock-casts +1 -1
@@ -736,7 +736,7 @@ acpi_os_acquire_lock (
 	if (flags & ACPI_NOT_ISR)
 		ACPI_DISABLE_IRQS();
 
-	spin_lock(handle);
+	spin_lock((spinlock_t *)handle);

Is there a reason these functions can't just have their arguments changed to take a spinlock_t* instead of an acpi_handle ? That cast looks really fugly IMO.

Dave

Andrew Morton

author

2003-04-04 19:47 UTC #3 lore raw

Quoted 10 lines · Dave Jones

Quoted 7 lines · Andrew Morton
diff -puN drivers/acpi/osl.c~acpi-spinlock-casts drivers/acpi/osl.c
drivers/acpi/osl.c renamed from drivers/acpi/osl.c~acpi-spinlock-casts +0 -0
@@ -736,7 +736,7 @@ acpi_os_acquire_lock (
 	if (flags & ACPI_NOT_ISR)
 		ACPI_DISABLE_IRQS();
 

Quoted 7 lines

drivers/acpi/osl.c acpi_os_acquire_lock ( · line 739
Quoted 2 lines
drivers/acpi/osl.c renamed from drivers/acpi/osl.c~acpi-spinlock-casts +1 -1
-	spin_lock(handle);
+	spin_lock((spinlock_t *)handle);
Is there a reason these functions can't just have their arguments changed to take a spinlock_t* instead of an acpi_handle ? That cast looks really fugly IMO.

I think acpi_handle_t is "an opaque type specific to the OS on which the APCI code happens to be running".

It is presently `void *', implicitly pointing at a spinlock_t.

If the above guesses (I'd prefer not to look) are correct then

	struct acpi_handle_t {
		spinlock_t lock;
	};

would make a ton more sense.

Dave Jones

2003-04-04 20:16 UTC #4 lore raw

Quoted 2 lines · Andrew Morton

I think acpi_handle_t is "an opaque type specific to the OS on which the APCI code happens to be running".

I don't see how putting a spinlock_t cast in the code is any more portable between OS's than spinlock_t as a function parameter.

Quoted 6 lines

If the above guesses (I'd prefer not to look) are correct then
	struct acpi_handle_t {
		spinlock_t lock;
	};
would make a ton more sense.

That would solve the portability argument in my eyes if that is indeed the case here. It's still ugly, but it at least kills the problem in a slightly more tasteful way.

Dave