* fix copyright headers to v2 only as suggested by Uwe * drop silly code when not using SMP * fix device/device.c * drop spinlock_t, use struct spinlock instead.
Signed-off-by: Stefan Reinauer stepan@coresystems.de
Index: include/spinlock.h =================================================================== --- include/spinlock.h (revision 427) +++ include/spinlock.h (working copy) @@ -2,11 +2,11 @@ * This file is part of the LinuxBIOS project. * * Copyright (C) 2001 Linux Networx + * Copyright (C) 2007 coresystems GmbH * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. + * the Free Software Foundation; version 2 of the License. * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of @@ -22,23 +22,17 @@ #define SPINLOCK_H
#if defined(CONFIG_SMP) && CONFIG_SMP == 1 + #include <arch/spinlock.h> + #else /* !CONFIG_SMP */
-/* Most GCC versions have a nasty bug with empty initializers */ -#if (__GNUC__ > 2) -typedef struct { } spinlock_t; -#define SPIN_LOCK_UNLOCKED (spinlock_t) { } -#else -typedef struct { int gcc_is_buggy; } spinlock_t; -#define SPIN_LOCK_UNLOCKED (spinlock_t) { 0 } -#endif - #define barrier() do {} while(0) #define spin_is_locked(lock) 0 #define spin_unlock_wait(lock) do {} while(0) #define spin_lock(lock) do {} while(0) #define spin_unlock(lock) do {} while(0) +#define spin_define(lock) /* empty */ #endif
#endif /* SPINLOCK_H */ Index: include/arch/x86/arch/spinlock.h =================================================================== --- include/arch/x86/arch/spinlock.h (revision 427) +++ include/arch/x86/arch/spinlock.h (working copy) @@ -2,11 +2,11 @@ * This file is part of the LinuxBIOS project. * * Copyright (C) 2001 Linux Networx + * Copyright (C) 2007 coresystems GmbH * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by - * the Free Software Foundation; either version 2 of the License, or - * (at your option) any later version. + * the Free Software Foundation; version 2 of the License. * * This program is distributed in the hope that it will be useful, * but WITHOUT ANY WARRANTY; without even the implied warranty of @@ -25,12 +25,12 @@ * Your basic SMP spinlocks, allowing only a single CPU anywhere */
-typedef struct { +struct spinlock { volatile unsigned int lock; -} spinlock_t; +};
-#define SPIN_LOCK_UNLOCKED (spinlock_t) { 1 } +#define SPIN_LOCK_UNLOCKED (struct spinlock) { 1 }
/* * Simple spin lock operations. There are two variants, one clears IRQ's @@ -60,18 +60,21 @@ #define spin_unlock_string \ "movb $1,%0"
-static inline __attribute__((always_inline)) void spin_lock(spinlock_t *lock) +static inline __attribute__((always_inline)) void spin_lock(struct spinlock *lock) { __asm__ __volatile__( spin_lock_string :"=m" (lock->lock) : : "memory"); }
-static inline __attribute__((always_inline)) void spin_unlock(spinlock_t *lock) +static inline __attribute__((always_inline)) void spin_unlock(struct spinlock *lock) { __asm__ __volatile__( spin_unlock_string :"=m" (lock->lock) : : "memory"); }
+#define spin_define(spin) static struct spinlock spin = SPIN_LOCK_UNLOCKED + + #endif /* ARCH_SPINLOCK_H */ Index: device/device.c =================================================================== --- device/device.c (revision 427) +++ device/device.c (working copy) @@ -183,7 +183,9 @@ * @return Pointer to the newly created device structure. * @see device_path */ -// static spinlock_t dev_lock = SPIN_LOCK_UNLOCKED; + +spin_define(dev_lock); + struct device *alloc_dev(struct bus *parent, struct device_path *path, struct device_id *devid) {
Acked-by: Ronald G. Minnich rminnich@gmail.com
A new record!
ron
On 6/30/07, Stefan Reinauer stepan@coresystems.de wrote:
- fix copyright headers to v2 only as suggested by Uwe
- drop silly code when not using SMP
- fix device/device.c
- drop spinlock_t, use struct spinlock instead.
Signed-off-by: Stefan Reinauer stepan@coresystems.de
Index: include/spinlock.h
--- include/spinlock.h (revision 427) +++ include/spinlock.h (working copy) @@ -2,11 +2,11 @@
- This file is part of the LinuxBIOS project.
- Copyright (C) 2001 Linux Networx
- Copyright (C) 2007 coresystems GmbH
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
- the Free Software Foundation; version 2 of the License.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ -22,23 +22,17 @@ #define SPINLOCK_H
#if defined(CONFIG_SMP) && CONFIG_SMP == 1
#include <arch/spinlock.h>
#else /* !CONFIG_SMP */
-/* Most GCC versions have a nasty bug with empty initializers */ -#if (__GNUC__ > 2) -typedef struct { } spinlock_t; -#define SPIN_LOCK_UNLOCKED (spinlock_t) { } -#else -typedef struct { int gcc_is_buggy; } spinlock_t; -#define SPIN_LOCK_UNLOCKED (spinlock_t) { 0 } -#endif
#define barrier() do {} while(0) #define spin_is_locked(lock) 0 #define spin_unlock_wait(lock) do {} while(0) #define spin_lock(lock) do {} while(0) #define spin_unlock(lock) do {} while(0) +#define spin_define(lock) /* empty */ #endif
#endif /* SPINLOCK_H */ Index: include/arch/x86/arch/spinlock.h =================================================================== --- include/arch/x86/arch/spinlock.h (revision 427) +++ include/arch/x86/arch/spinlock.h (working copy) @@ -2,11 +2,11 @@
- This file is part of the LinuxBIOS project.
- Copyright (C) 2001 Linux Networx
- Copyright (C) 2007 coresystems GmbH
- This program is free software; you can redistribute it and/or modify
- it under the terms of the GNU General Public License as published by
- the Free Software Foundation; either version 2 of the License, or
- (at your option) any later version.
- the Free Software Foundation; version 2 of the License.
- This program is distributed in the hope that it will be useful,
- but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ -25,12 +25,12 @@
- Your basic SMP spinlocks, allowing only a single CPU anywhere
*/
-typedef struct { +struct spinlock { volatile unsigned int lock; -} spinlock_t; +};
-#define SPIN_LOCK_UNLOCKED (spinlock_t) { 1 } +#define SPIN_LOCK_UNLOCKED (struct spinlock) { 1 }
/*
- Simple spin lock operations. There are two variants, one clears IRQ's
@@ -60,18 +60,21 @@ #define spin_unlock_string \ "movb $1,%0"
-static inline __attribute__((always_inline)) void spin_lock(spinlock_t *lock) +static inline __attribute__((always_inline)) void spin_lock(struct spinlock *lock) { __asm__ __volatile__( spin_lock_string :"=m" (lock->lock) : : "memory"); }
-static inline __attribute__((always_inline)) void spin_unlock(spinlock_t *lock) +static inline __attribute__((always_inline)) void spin_unlock(struct spinlock *lock) { __asm__ __volatile__( spin_unlock_string :"=m" (lock->lock) : : "memory"); }
+#define spin_define(spin) static struct spinlock spin = SPIN_LOCK_UNLOCKED
#endif /* ARCH_SPINLOCK_H */ Index: device/device.c =================================================================== --- device/device.c (revision 427) +++ device/device.c (working copy) @@ -183,7 +183,9 @@
- @return Pointer to the newly created device structure.
- @see device_path
*/ -// static spinlock_t dev_lock = SPIN_LOCK_UNLOCKED;
+spin_define(dev_lock);
struct device *alloc_dev(struct bus *parent, struct device_path *path, struct device_id *devid) {
-- coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br. Tel.: +49 761 7668825 • Fax: +49 761 7664613 Email: info@coresystems.de • http://www.coresystems.de/ Registergericht: Amtsgericht Freiburg • HRB 7656 Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866
-- linuxbios mailing list linuxbios@linuxbios.org http://www.linuxbios.org/mailman/listinfo/linuxbios
On Sun, Jul 01, 2007 at 12:46:04AM +0200, Stefan Reinauer wrote:
-/* Most GCC versions have a nasty bug with empty initializers */ -#if (__GNUC__ > 2) -typedef struct { } spinlock_t; -#define SPIN_LOCK_UNLOCKED (spinlock_t) { } -#else -typedef struct { int gcc_is_buggy; } spinlock_t; -#define SPIN_LOCK_UNLOCKED (spinlock_t) { 0 } -#endif
ACK. Sending a similar patch was already on my TODO list. We don't care about gcc <= 3, I guess.
#define barrier() do {} while(0) #define spin_is_locked(lock) 0 #define spin_unlock_wait(lock) do {} while(0) #define spin_lock(lock) do {} while(0) #define spin_unlock(lock) do {} while(0) +#define spin_define(lock) /* empty */
Why not this? #define spin_define(lock) do {} while(0)
+#define spin_define(spin) static struct spinlock spin = SPIN_LOCK_UNLOCKED
Uh, that looks like an inline function might be better here.
Uwe.
* Uwe Hermann uwe@hermann-uwe.de [070701 00:57]:
On Sun, Jul 01, 2007 at 12:46:04AM +0200, Stefan Reinauer wrote:
-/* Most GCC versions have a nasty bug with empty initializers */ -#if (__GNUC__ > 2) -typedef struct { } spinlock_t; -#define SPIN_LOCK_UNLOCKED (spinlock_t) { } -#else -typedef struct { int gcc_is_buggy; } spinlock_t; -#define SPIN_LOCK_UNLOCKED (spinlock_t) { 0 } -#endif
ACK. Sending a similar patch was already on my TODO list. We don't care about gcc <= 3, I guess.
Check the code, it will work on pre v3 like a charme. I don't know what those Linux guys must have been smoking, but I better don't try it.
#define spin_unlock(lock) do {} while(0) +#define spin_define(lock) /* empty */
Why not this? #define spin_define(lock) do {} while(0)
Haha. I had this first. But a do .. while construct outside a function scope is nothing gcc (or any other compiler) would handle :-)
+#define spin_define(spin) static struct spinlock spin = SPIN_LOCK_UNLOCKED
Uh, that looks like an inline function might be better here.
Not sure I get what you mean.. This one defines a (file) global variable.
On Sun, Jul 01, 2007 at 01:01:54AM +0200, Stefan Reinauer wrote:
#define spin_unlock(lock) do {} while(0) +#define spin_define(lock) /* empty */
Why not this? #define spin_define(lock) do {} while(0)
Haha. I had this first. But a do .. while construct outside a function scope is nothing gcc (or any other compiler) would handle :-)
Yep.
+#define spin_define(spin) static struct spinlock spin = SPIN_LOCK_UNLOCKED
Uh, that looks like an inline function might be better here.
Not sure I get what you mean.. This one defines a (file) global variable.
OK, I totally misread what the code is supposed to do.
I'm not sure the spin_define() is such a good idea then. It has a nice name and all, but it might be obfuscating things more than necessary. Maybe a simple variable declaration (i.e., drop the #define) would be better?
Uwe.
* Uwe Hermann uwe@hermann-uwe.de [070701 02:14]:
OK, I totally misread what the code is supposed to do.
I'm not sure the spin_define() is such a good idea then. It has a nice name and all, but it might be obfuscating things more than necessary. Maybe a simple variable declaration (i.e., drop the #define) would be better?
I don't see why it would be better, there are no obvious advantages, but some obvious disadvantages. Here our principles start clashing. We can't do this for several reasons:
* The declaration is static, so it has to be surrounded by #if defined(CONFIG_SMP) would waste all the places in the code. This is controverse to our goal to make changes as local as possible. If we don't, we end up with warnings in the code for almost every board
* Since some of us started hating typedefs, we don't have anything to declare anymore. You simply can not rely that a spinlock is represented by struct spinlock on all possible systems. So to get this done right, we would have to go back to spinlock_t, in which case I would vote to also go back to device_t and all the others, so we stay consistent.
Stefan
change spinlock_define to spinlock_declare?
ron
On Mon, Jul 02, 2007 at 12:50:04AM +0200, Stefan Reinauer wrote:
- Uwe Hermann uwe@hermann-uwe.de [070701 02:14]:
OK, I totally misread what the code is supposed to do.
I'm not sure the spin_define() is such a good idea then. It has a nice name and all, but it might be obfuscating things more than necessary. Maybe a simple variable declaration (i.e., drop the #define) would be better?
I don't see why it would be better, there are no obvious advantages, but some obvious disadvantages. Here our principles start clashing. We can't do this for several reasons:
The declaration is static, so it has to be surrounded by #if defined(CONFIG_SMP) would waste all the places in the code. This is controverse to our goal to make changes as local as possible. If we don't, we end up with warnings in the code for almost every board
Since some of us started hating typedefs, we don't have anything to declare anymore. You simply can not rely that a spinlock is represented by struct spinlock on all possible systems. So to get this done right, we would have to go back to spinlock_t, in which case I would vote to also go back to device_t and all the others, so we stay consistent.
Hm, ok forget what I said.
I think we should go back to using spinlock_t (but not device_t!).
As far as I remember the problem with device_t was that it was not just a typedef for a struct, but a typedef for a _pointer_to_a_struct_.
That causes many nasty possibilities for bugs to creep in when you see foo_t and think it's an int or struct where in reality it is actually a _pointer_ to something.
So I think we should revert back to spinlock_t and use the macro, too.
Sorry for the confusion. Uwe.
-
ron minnich -
Stefan Reinauer -
Uwe Hermann