Re: [PATCH 1/4] Blackfin: arch patch for 2.6.18

[Robin Getz <rgetz@blackfin.uclinux.org>]

Arnd wrote:
> Am Wednesday 27 September 2006 19:19 schrieb Robin Getz:
> > OK - I was just doing the similar thing to what already exists in
> > ./asm-blackfin/system.h
> >
> > #define local_irq_enable() do {         \
> >          __asm__ __volatile__ (          \
> >                  "sti %0;"               \
> >                  ::"d"(irq_flags));      \ } while (0)
> >
> > which could be simplified to:
> >
> > #define local_irq_enable() __asm__ __volatile__ ("sti %0;"
> > ::"d"(irq_flags));
>
> Actually, this one is slightly broken, because of the ';' at the end of 
> the macro (think of "if(x) local_irq_enable(); else somthing_else()").

Ok - the extra ; is a typo in the email - not anything that I was proposing 
as a submission. What you are pointing out is to be _really_ careful when 
doing macros.

I was trying to say was that we are just doing what everyone else seems to 
be doing (which doesn't make it correct or the proper thing to do).

Systems that use macros:
./asm-alpha/system.h:#define local_irq_enable()
./asm-arm26/system.h:#define local_irq_enable()
./asm-arm/system.h:#define local_irq_enable()
./asm-blackfin/system.h:#define local_irq_enable()
./asm-frv/system.h:#define local_irq_enable()
./asm-h8300/system.h:#define local_irq_enable()
./asm-i386/system.h:#define local_irq_enable()
./asm-ia64/system.h:#define local_irq_enable()
./asm-m32r/system.h:#define local_irq_enable()
./asm-m68knommu/system.h:#define local_irq_enable()
./asm-m68k/system.h:#define local_irq_enable()
./asm-parisc/system.h:#define local_irq_enable()
./asm-s390/system.h:#define local_irq_enable()
./asm-sparc64/system.h:#define local_irq_enable()
./asm/system.h:#define local_irq_enable()
./asm-v850/system.h:#define local_irq_enable()
./asm-x86_64/system.h:#define local_irq_enable()
./asm-x86_64/system.h:#define local_irq_enable()

Systems that use static inline:
./asm-m32r/system.h:static inline void local_irq_enable(void)
./asm-sh64/system.h:static __inline__ void local_irq_enable(void)
./asm-sh/system.h:static __inline__ void local_irq_enable(void)
./asm-xtensa/system.h:static inline void local_irq_enable(void)

With the "optimizations" that gcc4 is making with inline being only a 
"suggestion", I think I would prefer to stick with the macro, unless there 
is violent opposition.

As Mike pointed out - we are sheep - we just do what the majority (18/22) 
of other people do.

-Robin
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/