* [patch] acpi compile fix
@ 2003-04-03 21:05 Andrew Morton
2003-04-04 6:48 ` J Sloan
2003-04-04 12:18 ` Dave Jones
0 siblings, 2 replies; 6+ messages in thread
From: Andrew Morton @ 2003-04-03 21:05 UTC (permalink / raw)
To: Linus Torvalds; +Cc: jjs, Grover, Andrew, linux-kernel
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.
diff -puN drivers/acpi/osl.c~acpi-spinlock-casts drivers/acpi/osl.c
--- 25/drivers/acpi/osl.c~acpi-spinlock-casts Thu Apr 3 13:00:54 2003
+++ 25-akpm/drivers/acpi/osl.c Thu Apr 3 13:01:25 2003
@@ -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();
_
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] acpi compile fix
2003-04-03 21:05 [patch] acpi compile fix Andrew Morton
@ 2003-04-04 6:48 ` J Sloan
2003-04-04 12:18 ` Dave Jones
1 sibling, 0 replies; 6+ messages in thread
From: J Sloan @ 2003-04-04 6:48 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel
Andrew Morton wrote:
>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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] acpi compile fix
2003-04-03 21:05 [patch] acpi compile fix Andrew Morton
2003-04-04 6:48 ` J Sloan
@ 2003-04-04 12:18 ` Dave Jones
2003-04-04 19:57 ` Andrew Morton
1 sibling, 1 reply; 6+ messages in thread
From: Dave Jones @ 2003-04-04 12:18 UTC (permalink / raw)
To: Andrew Morton; +Cc: Linus Torvalds, jjs, Grover, Andrew, linux-kernel
On Thu, Apr 03, 2003 at 01:05:05PM -0800, Andrew Morton wrote:
> diff -puN drivers/acpi/osl.c~acpi-spinlock-casts drivers/acpi/osl.c
> --- 25/drivers/acpi/osl.c~acpi-spinlock-casts Thu Apr 3 13:00:54 2003
> +++ 25-akpm/drivers/acpi/osl.c Thu Apr 3 13:01:25 2003
> @@ -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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [patch] acpi compile fix
2003-04-04 12:18 ` Dave Jones
@ 2003-04-04 19:57 ` Andrew Morton
2003-04-04 20:27 ` Dave Jones
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2003-04-04 19:57 UTC (permalink / raw)
To: Dave Jones; +Cc: torvalds, jjs, andrew.grover, linux-kernel
Dave Jones <davej@codemonkey.org.uk> wrote:
>
> On Thu, Apr 03, 2003 at 01:05:05PM -0800, Andrew Morton wrote:
>
> > diff -puN drivers/acpi/osl.c~acpi-spinlock-casts drivers/acpi/osl.c
> > --- 25/drivers/acpi/osl.c~acpi-spinlock-casts Thu Apr 3 13:00:54 2003
> > +++ 25-akpm/drivers/acpi/osl.c Thu Apr 3 13:01:25 2003
> > @@ -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.
>
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [patch] acpi compile fix
2003-04-04 19:57 ` Andrew Morton
@ 2003-04-04 20:27 ` Dave Jones
0 siblings, 0 replies; 6+ messages in thread
From: Dave Jones @ 2003-04-04 20:27 UTC (permalink / raw)
To: Andrew Morton; +Cc: torvalds, jjs, andrew.grover, linux-kernel
On Fri, Apr 04, 2003 at 11:57:56AM -0800, Andrew Morton wrote:
> 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.
> 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [patch] acpi compile fix
@ 2003-04-04 21:41 Grover, Andrew
0 siblings, 0 replies; 6+ messages in thread
From: Grover, Andrew @ 2003-04-04 21:41 UTC (permalink / raw)
To: Dave Jones, Andrew Morton; +Cc: torvalds, jjs, linux-kernel
> From: Dave Jones [mailto:davej@codemonkey.org.uk]
> 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.
The code that calls osl.c does not know about spinlock_t. Either the
function's definition and declaration don't match, or the other code
needs to know what a spinlock_t is, doesn't it?
> > 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.
I don't see the cast as being particularly onerous. It's just a cookie.
osl.c knows what it actually points to, the rest doesn't.
Regards -- Andy
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2003-04-04 21:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-04-03 21:05 [patch] acpi compile fix Andrew Morton
2003-04-04 6:48 ` J Sloan
2003-04-04 12:18 ` Dave Jones
2003-04-04 19:57 ` Andrew Morton
2003-04-04 20:27 ` Dave Jones
-- strict thread matches above, loose matches on Subject: below --
2003-04-04 21:41 Grover, Andrew
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox