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

[Lukasz Majewski]

Hi Bryan,

> 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...

+1

> 
> ---
> bod
> 




Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180427/7de660a0/attachment.sig>