[U-Boot,V4,1/4] ARM: Define change_bit routine

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(-)

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