Message ID | cbe6d45427797fa5f4f8ef493006a5d7676e935d.1337320291.git.amit.virdi@st.com |
---|---|
State | Changes Requested |
Delegated to: | Wolfgang Denk |
Headers | show |
On 05/18/2012 01:00 AM, Amit Virdi wrote: > From: Vipin KUMAR <vipin.kumar@st.com> > > change_bit routine is left implementation dependent until now. > This routine, which is basically a wrapper over __change_bit, is now defined for > arm platforms in asm-arm/bitops.h > > The Flexible Static memory controller driver, placed in > mtd/nand/fsmc_nand.c needs this routine. FSMC is a memory controller > peripheral from ST. The new driver implements the NAND interface part > of the peripheral. > > Signed-off-by: Vipin Kumar <vipin.kumar@st.com> > Signed-off-by: Amit Virdi <amit.virdi@st.com> > --- > arch/arm/include/asm/bitops.h | 7 +++++-- > 1 files changed, 5 insertions(+), 2 deletions(-) > > diff --git a/arch/arm/include/asm/bitops.h b/arch/arm/include/asm/bitops.h > index 879e20e..7fe9c6d 100644 > --- a/arch/arm/include/asm/bitops.h > +++ b/arch/arm/include/asm/bitops.h > @@ -29,8 +29,6 @@ extern void set_bit(int nr, volatile void * addr); > > extern void clear_bit(int nr, volatile void * addr); > > -extern void change_bit(int nr, volatile void * addr); > - > static inline void __change_bit(int nr, volatile void *addr) > { > unsigned long mask = BIT_MASK(nr); > @@ -39,6 +37,11 @@ static inline void __change_bit(int nr, volatile void *addr) > *p ^= mask; > } > > +static inline void change_bit(int nr, volatile void *addr) > +{ > + __change_bit(nr, addr); > +} > + > static inline int __test_and_set_bit(int nr, volatile void *addr) > { > unsigned long mask = BIT_MASK(nr); While they're not used much, U-Boot does have interrupt support (at least on powerpc -- not sure about ARM), so this should be atomic against interrupts. The NAND driver should be using __change_bit() instead. -Scott
>> >> extern void clear_bit(int nr, volatile void * addr); >> >> -extern void change_bit(int nr, volatile void * addr); >> - >> static inline void __change_bit(int nr, volatile void *addr) >> { >> unsigned long mask = BIT_MASK(nr); >> @@ -39,6 +37,11 @@ static inline void __change_bit(int nr, volatile void *addr) >> *p ^= mask; >> } >> >> +static inline void change_bit(int nr, volatile void *addr) >> +{ >> + __change_bit(nr, addr); >> +} >> + >> static inline int __test_and_set_bit(int nr, volatile void *addr) >> { >> unsigned long mask = BIT_MASK(nr); > > While they're not used much, U-Boot does have interrupt support (at > least on powerpc -- not sure about ARM), so this should be atomic > against interrupts. > > The NAND driver should be using __change_bit() instead. > Alright, I shall be soon sending V5 that drops this patch and uses __change_bit directly. Thanks Amit Virdi
Dear Amit Virdi, In message <4FBB6712.3040005@st.com> you wrote: > > Alright, I shall be soon sending V5 that drops this patch and uses > __change_bit directly. It would be a much better investment in time if you come up with patches that help to _avoid_ usign this horrible function, and eventually removig it from U-Boot. change_bit() is inherently not portable and actual behaviour is largely undefined, so please do NOT use this function in any new code. Best regards, Wolfgang Denk
Hi Wolfgang, On 5/22/2012 9:21 PM, Wolfgang Denk wrote: > Dear Amit Virdi, > > In message<4FBB6712.3040005@st.com> you wrote: >> >> Alright, I shall be soon sending V5 that drops this patch and uses >> __change_bit directly. > > It would be a much better investment in time if you come up with > patches that help to _avoid_ usign this horrible function, and > eventually removig it from U-Boot. > > change_bit() is inherently not portable and actual behaviour is > largely undefined, so please do NOT use this function in any new code. > Ok. I have understood your point. I would resend V5 excluding any call to __change_bit. Thanks Amit Virdi
diff --git a/arch/arm/include/asm/bitops.h b/arch/arm/include/asm/bitops.h index 879e20e..7fe9c6d 100644 --- a/arch/arm/include/asm/bitops.h +++ b/arch/arm/include/asm/bitops.h @@ -29,8 +29,6 @@ extern void set_bit(int nr, volatile void * addr); extern void clear_bit(int nr, volatile void * addr); -extern void change_bit(int nr, volatile void * addr); - static inline void __change_bit(int nr, volatile void *addr) { unsigned long mask = BIT_MASK(nr); @@ -39,6 +37,11 @@ static inline void __change_bit(int nr, volatile void *addr) *p ^= mask; } +static inline void change_bit(int nr, volatile void *addr) +{ + __change_bit(nr, addr); +} + static inline int __test_and_set_bit(int nr, volatile void *addr) { unsigned long mask = BIT_MASK(nr);