* [U-Boot] [PATCH] usb: f_mass_storage: Fix compile on x86
@ 2018-04-26 14:41 Bryan O'Donoghue
2018-04-26 15:14 ` Marek Vasut
0 siblings, 1 reply; 5+ messages in thread
From: Bryan O'Donoghue @ 2018-04-26 14:41 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
Fix that now by only compiling up the local definition of set_bit and
clear_bit only if not already provided by the environment.
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 | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
index 1ecb92ac6b..2e856af6ed 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -283,6 +283,7 @@ static const char fsg_string_interface[] = "Mass Storage";
struct kref {int x; };
struct completion {int x; };
+#ifndef _I386_BITOPS_H
inline void set_bit(int nr, volatile void *addr)
{
int mask;
@@ -302,6 +303,7 @@ inline void clear_bit(int nr, volatile void *addr)
mask = 1 << (nr & 0x1f);
*a &= ~mask;
}
+#endif /* _I386_BITOPTS_H */
struct fsg_dev;
struct fsg_common;
--
2.17.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] usb: f_mass_storage: Fix compile on x86
2018-04-26 14:41 [U-Boot] [PATCH] usb: f_mass_storage: Fix compile on x86 Bryan O'Donoghue
@ 2018-04-26 15:14 ` Marek Vasut
2018-04-26 16:58 ` Bryan O'Donoghue
0 siblings, 1 reply; 5+ messages in thread
From: Marek Vasut @ 2018-04-26 15:14 UTC (permalink / raw)
To: u-boot
On 04/26/2018 04:41 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
>
> Fix that now by only compiling up the local definition of set_bit and
> clear_bit only if not already provided by the environment.
>
> 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 | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
> index 1ecb92ac6b..2e856af6ed 100644
> --- a/drivers/usb/gadget/f_mass_storage.c
> +++ b/drivers/usb/gadget/f_mass_storage.c
> @@ -283,6 +283,7 @@ static const char fsg_string_interface[] = "Mass Storage";
> struct kref {int x; };
> struct completion {int x; };
>
> +#ifndef _I386_BITOPS_H
> inline void set_bit(int nr, volatile void *addr)
> {
> int mask;
> @@ -302,6 +303,7 @@ inline void clear_bit(int nr, volatile void *addr)
> mask = 1 << (nr & 0x1f);
> *a &= ~mask;
> }
> +#endif /* _I386_BITOPTS_H */
This doesn't look right, generic driver shouldn't contain arch-specific
fixup or ifdef. Can this be somehow abstracted out?
> struct fsg_dev;
> struct fsg_common;
>
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] usb: f_mass_storage: Fix compile on x86
2018-04-26 15:14 ` Marek Vasut
@ 2018-04-26 16:58 ` Bryan O'Donoghue
2018-04-26 17:03 ` Marek Vasut
0 siblings, 1 reply; 5+ messages in thread
From: Bryan O'Donoghue @ 2018-04-26 16:58 UTC (permalink / raw)
To: u-boot
On 26/04/18 16:14, Marek Vasut wrote:
> On 04/26/2018 04:41 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
>>
>> Fix that now by only compiling up the local definition of set_bit and
>> clear_bit only if not already provided by the environment.
>>
>> 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 | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/usb/gadget/f_mass_storage.c b/drivers/usb/gadget/f_mass_storage.c
>> index 1ecb92ac6b..2e856af6ed 100644
>> --- a/drivers/usb/gadget/f_mass_storage.c
>> +++ b/drivers/usb/gadget/f_mass_storage.c
>> @@ -283,6 +283,7 @@ static const char fsg_string_interface[] = "Mass Storage";
>> struct kref {int x; };
>> struct completion {int x; };
>>
>> +#ifndef _I386_BITOPS_H
>> inline void set_bit(int nr, volatile void *addr)
>> {
>> int mask;
>> @@ -302,6 +303,7 @@ inline void clear_bit(int nr, volatile void *addr)
>> mask = 1 << (nr & 0x1f);
>> *a &= ~mask;
>> }
>> +#endif /* _I386_BITOPTS_H */
>
> This doesn't look right, generic driver shouldn't contain arch-specific
> fixup or ifdef. Can this be somehow abstracted out?
>
Hmm.
On a second look - the name of these functions should change not to
conflict with bitops.h - there's some funny bit-shifting going on there..
---
bod
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] usb: f_mass_storage: Fix compile on x86
2018-04-26 16:58 ` Bryan O'Donoghue
@ 2018-04-26 17:03 ` Marek Vasut
2018-04-26 20:12 ` Lukasz Majewski
0 siblings, 1 reply; 5+ messages in thread
From: Marek Vasut @ 2018-04-26 17:03 UTC (permalink / raw)
To: u-boot
On 04/26/2018 06:58 PM, Bryan O'Donoghue wrote:
> On 26/04/18 16:14, Marek Vasut wrote:
>> On 04/26/2018 04:41 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
>>>
>>> Fix that now by only compiling up the local definition of set_bit and
>>> clear_bit only if not already provided by the environment.
>>>
>>> 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 | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/usb/gadget/f_mass_storage.c
>>> b/drivers/usb/gadget/f_mass_storage.c
>>> index 1ecb92ac6b..2e856af6ed 100644
>>> --- a/drivers/usb/gadget/f_mass_storage.c
>>> +++ b/drivers/usb/gadget/f_mass_storage.c
>>> @@ -283,6 +283,7 @@ static const char fsg_string_interface[] = "Mass
>>> Storage";
>>> struct kref {int x; };
>>> struct completion {int x; };
>>> +#ifndef _I386_BITOPS_H
>>> inline void set_bit(int nr, volatile void *addr)
>>> {
>>> int mask;
>>> @@ -302,6 +303,7 @@ inline void clear_bit(int nr, volatile void *addr)
>>> mask = 1 << (nr & 0x1f);
>>> *a &= ~mask;
>>> }
>>> +#endif /* _I386_BITOPTS_H */
>>
>> This doesn't look right, generic driver shouldn't contain arch-specific
>> fixup or ifdef. Can this be somehow abstracted out?
>>
>
> Hmm.
>
> On a second look - the name of these functions should change not to
> conflict with bitops.h - there's some funny bit-shifting going on there..
Better :)
--
Best regards,
Marek Vasut
^ permalink raw reply [flat|nested] 5+ messages in thread* [U-Boot] [PATCH] usb: f_mass_storage: Fix compile on x86
2018-04-26 17:03 ` Marek Vasut
@ 2018-04-26 20:12 ` Lukasz Majewski
0 siblings, 0 replies; 5+ messages in thread
From: Lukasz Majewski @ 2018-04-26 20:12 UTC (permalink / raw)
To: u-boot
Hi,
> On 04/26/2018 06:58 PM, Bryan O'Donoghue wrote:
> > On 26/04/18 16:14, Marek Vasut wrote:
> >> On 04/26/2018 04:41 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
> >>>
> >>> Fix that now by only compiling up the local definition of set_bit
> >>> and clear_bit only if not already provided by the environment.
> >>>
> >>> 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 | 2 ++
> >>> 1 file changed, 2 insertions(+)
> >>>
> >>> diff --git a/drivers/usb/gadget/f_mass_storage.c
> >>> b/drivers/usb/gadget/f_mass_storage.c
> >>> index 1ecb92ac6b..2e856af6ed 100644
> >>> --- a/drivers/usb/gadget/f_mass_storage.c
> >>> +++ b/drivers/usb/gadget/f_mass_storage.c
> >>> @@ -283,6 +283,7 @@ static const char fsg_string_interface[] =
> >>> "Mass Storage";
> >>> struct kref {int x; };
> >>> struct completion {int x; };
> >>> +#ifndef _I386_BITOPS_H
> >>> inline void set_bit(int nr, volatile void *addr)
> >>> {
> >>> int mask;
> >>> @@ -302,6 +303,7 @@ inline void clear_bit(int nr, volatile void
> >>> *addr) mask = 1 << (nr & 0x1f);
> >>> *a &= ~mask;
> >>> }
> >>> +#endif /* _I386_BITOPTS_H */
> >>
> >> This doesn't look right, generic driver shouldn't contain
> >> arch-specific fixup or ifdef. Can this be somehow abstracted out?
> >>
> >
> > Hmm.
> >
> > On a second look - the name of these functions should change not to
> > conflict with bitops.h - there's some funny bit-shifting going on
> > there..
>
> Better :)
>
The best option would be to replace those inlines with functions
already provided in bitops.h - delete local instances and then re-use
ones from the generic header.
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/20180426/3dae2472/attachment.sig>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-04-26 20:12 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-04-26 14:41 [U-Boot] [PATCH] usb: f_mass_storage: Fix compile on x86 Bryan O'Donoghue
2018-04-26 15:14 ` Marek Vasut
2018-04-26 16:58 ` Bryan O'Donoghue
2018-04-26 17:03 ` Marek Vasut
2018-04-26 20:12 ` Lukasz Majewski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox