[U-Boot] [PATCH v2] usb: f_mass_storage: Fix compile on x86
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 Cc: Lukasz Majewski Cc: Marek Vasut --- 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) { int mask; unsigned int *a = (unsigned int *) addr; @@ -293,7 +293,7 @@ inline void set_bit(int nr, volatile void *addr) *a |= mask; } -inline void clear_bit(int nr, volatile void *addr) +inline void _clear_bit(int nr, volatile void *addr) { int mask; unsigned int *a = (unsigned int *) addr; @@ -2086,7 +2086,7 @@ static int received_cbw(struct fsg_dev *fsg, struct fsg_buffhd *bh) * we can simply accept and discard any data received * until the next reset. */ wedge_bulk_in_endpoint(fsg); - set_bit(IGNORE_BULK_OUT, &fsg->atomic_bitflags); + _set_bit(IGNORE_BULK_OUT, &fsg->atomic_bitflags); return -EINVAL; } @@ -2250,7 +2250,7 @@ reset: fsg->bulk_out_enabled = 1; common->bulk_out_maxpacket = le16_to_cpu(get_unaligned(&d->wMaxPacketSize)); - clear_bit(IGNORE_BULK_OUT, &fsg->atomic_bitflags); + _clear_bit(IGNORE_BULK_OUT, &fsg->atomic_bitflags); /* Allocate the requests */ for (i = 0; i < FSG_NUM_BUFFERS; ++i) { -- 2.17.0 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] usb: f_mass_storage: Fix compile on x86
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 > Cc: Lukasz Majewski > Cc: Marek Vasut > --- > 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. -- Best regards, Marek Vasut ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] usb: f_mass_storage: Fix compile on x86
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 > >> Cc: Lukasz Majewski > >> Cc: Marek Vasut > >> --- > >> 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 > 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: w...@denx.de pgpCDZOEPONag.pgp Description: OpenPGP digital signature ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] [PATCH v2] usb: f_mass_storage: Fix compile on x86
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 Cc: Lukasz Majewski Cc: Marek Vasut --- 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 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 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot