[U-Boot] [PATCH v2] usb: f_mass_storage: Fix compile on x86

[Bryan O'Donoghue]

On 26/04/18 22:21, Marek Vasut wrote:
> On 04/26/2018 10:47 PM, Bryan O'Donoghue wrote:
>> Compiling the f_mass_storage driver for an x86 target results in a
>> compilation error as set_bit and clear_bit are provided by bitops.h
>>
>> The local version of set_bit and clear_bit are doing some IP-block specific
>> bit-twiddling so we don't actually want to accidentally pick up the
>> bitops.h version of set_bit and clear bit.
>>
>> This patch renames to _set_bit() and _clear_bit() respectively to avoid the
>> namespace collision.
>>
>> Signed-off-by: Bryan O'Donoghue <pure.logic at nexus-software.ie>
>> Cc: Lukasz Majewski <lukma at denx.de>
>> Cc: Marek Vasut <marex at denx.de>
>> ---
>>   drivers/usb/gadget/f_mass_storage.c | 8 ++++----
>>   1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
>> index 1ecb92ac6b..524c43419f 100644
>> --- a/drivers/usb/gadget/f_mass_storage.c
>> +++ b/drivers/usb/gadget/f_mass_storage.c
>> @@ -283,7 +283,7 @@ static const char fsg_string_interface[] = "Mass Storage";
>>   struct kref {int x; };
>>   struct completion {int x; };
>>   
>> -inline void set_bit(int nr, volatile void *addr)
>> +inline void _set_bit(int nr, volatile void *addr)
> 
> Do not name functions with _ prefix. Also, I think the suggestion from
> Lukasz was better.
> 

Funny - I had assumed we were seting the bit like this for a good reason 
but...

This code seems to be something that just crept in along the way

inline void set_bit(int nr, volatile void *addr)
{
         int     mask;
         unsigned int *a = (unsigned int *) addr;

         a += nr >> 5;
         mask = 1 << (nr & 0x1f);
         *a |= mask;
}

but on Linux the upstream driver actually does arch-specific 
set_bit/clear_bit

Looking at it a bit more.. I'm not quite sure how/why we are setting the 
bit like that

git blame drivers/usb/gadget/f_mass_storage.c

b4d36f6809f (Piotr Wilczek          2013-03-05 12:10:16 +0100  286) 
inline void set_bit(int nr, volatile void *addr)

The f_mass_storage.c source file from v2.6.36 Linux kernel.

     commit 8876f5e7d3b2a320777dd4f6f5301d474c97a06c
     Author: Michal Nazarewicz <m.nazarewicz at samsung.com>
     Date:   Mon Jun 21 13:57:09 2010 +0200


linux$ git checkout 8876f5e7d3b2a320777dd4f6f5301d474c97a06c -- 
drivers/usb/gadget/f_mass_storage.c

Doesn't have a local copy of set_bit/clear_bit at all, which is 
confusing - I guess bitops.h in 2013 didn't have set_bit/clear_bit..

But yeah - I guess we really should be using arch-specific 
get_bit/set_bit after all...

---
bod