* [U-Boot] [PATCH v2] usb: f_mass_storage: Fix compile on x86
@ 2018-04-26 20:47 Bryan O'Donoghue
2018-04-26 21:21 ` Marek Vasut
0 siblings, 1 reply; 4+ messages in thread
From: Bryan O'Donoghue @ 2018-04-26 20:47 UTC (permalink / raw)
To: u-boot
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@nexus-software.ie>
Cc: Lukasz Majewski <lukma@denx.de>
Cc: Marek Vasut <marex@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)
{
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH v2] usb: f_mass_storage: Fix compile on x86
2018-04-26 20:47 [U-Boot] [PATCH v2] usb: f_mass_storage: Fix compile on x86 Bryan O'Donoghue
@ 2018-04-26 21:21 ` Marek Vasut
2018-04-26 21:53 ` Bryan O'Donoghue
0 siblings, 1 reply; 4+ messages in thread
From: Marek Vasut @ 2018-04-26 21:21 UTC (permalink / raw)
To: u-boot
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@nexus-software.ie>
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Marek Vasut <marex@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.
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH v2] usb: f_mass_storage: Fix compile on x86
2018-04-26 21:21 ` Marek Vasut
@ 2018-04-26 21:53 ` Bryan O'Donoghue
2018-04-27 7:36 ` Lukasz Majewski
0 siblings, 1 reply; 4+ messages in thread
From: Bryan O'Donoghue @ 2018-04-26 21:53 UTC (permalink / raw)
To: u-boot
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@nexus-software.ie>
>> Cc: Lukasz Majewski <lukma@denx.de>
>> Cc: Marek Vasut <marex@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@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
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH v2] usb: f_mass_storage: Fix compile on x86
2018-04-26 21:53 ` Bryan O'Donoghue
@ 2018-04-27 7:36 ` Lukasz Majewski
0 siblings, 0 replies; 4+ messages in thread
From: Lukasz Majewski @ 2018-04-27 7:36 UTC (permalink / raw)
To: u-boot
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@nexus-software.ie>
> >> Cc: Lukasz Majewski <lukma@denx.de>
> >> Cc: Marek Vasut <marex@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@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>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-04-27 7:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-26 20:47 [U-Boot] [PATCH v2] usb: f_mass_storage: Fix compile on x86 Bryan O'Donoghue
2018-04-26 21:21 ` Marek Vasut
2018-04-26 21:53 ` Bryan O'Donoghue
2018-04-27 7:36 ` Lukasz Majewski
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.