[U-Boot] [PATCH v3 8/9] usb: f_mass_storage: Fix set_bit and clear_bit usage

2018-04-30 Thread Bryan O'Donoghue 
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

Looking at the provenance of the current u-boot code and the git change
history in the kernel, it looks like we have a local copy of set_bit and
clear_bit as a hold-over from porting the Linux driver into u-boot.

These days __set_bit and __clear_bit are optionally provided by an arch and
can be used as inputs to generic_bit_set and generic_bit_clear.

This patch switches over to generic_set_bit and generic_clear_bit to
accommodate.

Tested on i.MX WaRP7 and Intel Edison

Signed-off-by: Bryan O'Donoghue 
Cc: Lukasz Majewski 
Cc: Marek Vasut 
---
 drivers/usb/gadget/f_mass_storage.c | 25 +++--
 1 file changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/gadget/f_mass_storage.c 
b/drivers/usb/gadget/f_mass_storage.c
index 1ecb92ac6b..209932df45 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -252,6 +252,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -283,26 +284,6 @@ 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)
-{
-   int mask;
-   unsigned int *a = (unsigned int *) addr;
-
-   a += nr >> 5;
-   mask = 1 << (nr & 0x1f);
-   *a |= mask;
-}
-
-inline void clear_bit(int nr, volatile void *addr)
-{
-   int mask;
-   unsigned int *a = (unsigned int *) addr;
-
-   a += nr >> 5;
-   mask = 1 << (nr & 0x1f);
-   *a &= ~mask;
-}
-
 struct fsg_dev;
 struct fsg_common;
 
@@ -2086,7 +2067,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, >atomic_bitflags);
+   generic_set_bit(IGNORE_BULK_OUT, >atomic_bitflags);
return -EINVAL;
}
 
@@ -2250,7 +2231,7 @@ reset:
fsg->bulk_out_enabled = 1;
common->bulk_out_maxpacket =
le16_to_cpu(get_unaligned(>wMaxPacketSize));
-   clear_bit(IGNORE_BULK_OUT, >atomic_bitflags);
+   generic_clear_bit(IGNORE_BULK_OUT, >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 v3 8/9] usb: f_mass_storage: Fix set_bit and clear_bit usage

2018-04-30 Thread Bryan O'Donoghue 

On 27/04/18 17:20, Lukasz Majewski wrote:

Hi Bryan,


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

Looking at the provenance of the current u-boot code and the git
change history in the kernel, it looks like we have a local copy of
set_bit and clear_bit as a hold-over from porting the Linux driver
into u-boot.

These days __set_bit and __clear_bit are optionally provided by an
arch and can be used as inputs to generic_bit_set and
generic_bit_clear.

This patch switches over to generic_set_bit and generic_clear_bit to
accommodate.

Tested on i.MX WaRP7 and Intel Edison


Just I'm a bit puzzled:

This is v3 8/9, followed by 9/9 (v1)?


I sent out two previous versions of this patch as a standalone, if you 
recall.



And after some time you sent v2 8/9 and v2 9/9.

I suppose that the latter ones are correct?

Also it is very handy to have some kind of change log in patches - i.e.:


Cover letter contains, and I believe best practice is to keep the 
changelog in the cover-letter - not the patch itself.


##
V2:
- Fix commit log nds2 -> nds32
- Fix copy/paste error resulting in double colon "arch : : text"

V1:
Following on from a discussion with Marek and Lukasz re: a namespace
collision with set_bit and clear_bit in f_mass_storage, I noticed some
inconsistencies in the definition and usage of PLATFORM__SET_BIT and
PLATFORM__CLEAR_BIT as well as a similar use of __set_bit in the composite
USB gadget driver.

__set_bit is lock-prefixed on x86 whereas set_bit is not and the analog
driver in upstream Linux does set_bit() not __set_bit().

This series addresses all of those inconsistencies.

There are some usages of __set_bit() but those are in SoC specific GPIO
code-paths and therefore don't really need to change IMO.
##

Anyway I'll resend this set as a v3 since Bin Meng needs a change to the 
log text anyway.


---
bod
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [U-Boot] [PATCH v3 8/9] usb: f_mass_storage: Fix set_bit and clear_bit usage

2018-04-27 Thread Lukasz Majewski 
Hi Bryan,

> 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
> 
> Looking at the provenance of the current u-boot code and the git
> change history in the kernel, it looks like we have a local copy of
> set_bit and clear_bit as a hold-over from porting the Linux driver
> into u-boot.
> 
> These days __set_bit and __clear_bit are optionally provided by an
> arch and can be used as inputs to generic_bit_set and
> generic_bit_clear.
> 
> This patch switches over to generic_set_bit and generic_clear_bit to
> accommodate.
> 
> Tested on i.MX WaRP7 and Intel Edison

Just I'm a bit puzzled:

This is v3 8/9, followed by 9/9 (v1)?

And after some time you sent v2 8/9 and v2 9/9.

I suppose that the latter ones are correct?

Also it is very handy to have some kind of change log in patches - i.e.:

Changes for v2:
- Switching to generic_set|clear bit functions
- ..


The patch seems OK, but I need to test them on my machines.

> 
> Signed-off-by: Bryan O'Donoghue 
> Cc: Lukasz Majewski 
> Cc: Marek Vasut 
> ---
>  drivers/usb/gadget/f_mass_storage.c | 25 +++--
>  1 file changed, 3 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/usb/gadget/f_mass_storage.c
> b/drivers/usb/gadget/f_mass_storage.c index 1ecb92ac6b..af42136e1d
> 100644 --- a/drivers/usb/gadget/f_mass_storage.c
> +++ b/drivers/usb/gadget/f_mass_storage.c
> @@ -252,6 +252,7 @@
>  #include 
>  
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -283,26 +284,6 @@ 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)
> -{
> - int mask;
> - unsigned int *a = (unsigned int *) addr;
> -
> - a += nr >> 5;
> - mask = 1 << (nr & 0x1f);
> - *a |= mask;
> -}
> -
> -inline void clear_bit(int nr, volatile void *addr)
> -{
> - int mask;
> - unsigned int *a = (unsigned int *) addr;
> -
> - a += nr >> 5;
> - mask = 1 << (nr & 0x1f);
> - *a &= ~mask;
> -}
> -
>  struct fsg_dev;
>  struct fsg_common;
>  
> @@ -2086,7 +2067,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, >atomic_bitflags);
> + generic_set_bit(IGNORE_BULK_OUT,
> >atomic_bitflags); return -EINVAL;
>   }
>  
> @@ -2250,7 +2231,7 @@ reset:
>   fsg->bulk_out_enabled = 1;
>   common->bulk_out_maxpacket =
>   le16_to_cpu(get_unaligned(>wMaxPacketSize));
> - clear_bit(IGNORE_BULK_OUT, >atomic_bitflags);
> + generic_clear_bit(IGNORE_BULK_OUT, >atomic_bitflags);
>  
>   /* Allocate the requests */
>   for (i = 0; i < FSG_NUM_BUFFERS; ++i) {




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


pgp0Xkw4Ii89w.pgp
Description: OpenPGP digital signature
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

[U-Boot] [PATCH v3 8/9] usb: f_mass_storage: Fix set_bit and clear_bit usage

2018-04-27 Thread Bryan O'Donoghue 
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

Looking at the provenance of the current u-boot code and the git change
history in the kernel, it looks like we have a local copy of set_bit and
clear_bit as a hold-over from porting the Linux driver into u-boot.

These days __set_bit and __clear_bit are optionally provided by an arch and
can be used as inputs to generic_bit_set and generic_bit_clear.

This patch switches over to generic_set_bit and generic_clear_bit to
accommodate.

Tested on i.MX WaRP7 and Intel Edison

Signed-off-by: Bryan O'Donoghue 
Cc: Lukasz Majewski 
Cc: Marek Vasut 
---
 drivers/usb/gadget/f_mass_storage.c | 25 +++--
 1 file changed, 3 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/gadget/f_mass_storage.c 
b/drivers/usb/gadget/f_mass_storage.c
index 1ecb92ac6b..af42136e1d 100644
--- a/drivers/usb/gadget/f_mass_storage.c
+++ b/drivers/usb/gadget/f_mass_storage.c
@@ -252,6 +252,7 @@
 #include 
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -283,26 +284,6 @@ 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)
-{
-   int mask;
-   unsigned int *a = (unsigned int *) addr;
-
-   a += nr >> 5;
-   mask = 1 << (nr & 0x1f);
-   *a |= mask;
-}
-
-inline void clear_bit(int nr, volatile void *addr)
-{
-   int mask;
-   unsigned int *a = (unsigned int *) addr;
-
-   a += nr >> 5;
-   mask = 1 << (nr & 0x1f);
-   *a &= ~mask;
-}
-
 struct fsg_dev;
 struct fsg_common;
 
@@ -2086,7 +2067,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, >atomic_bitflags);
+   generic_set_bit(IGNORE_BULK_OUT, >atomic_bitflags);
return -EINVAL;
}
 
@@ -2250,7 +2231,7 @@ reset:
fsg->bulk_out_enabled = 1;
common->bulk_out_maxpacket =
le16_to_cpu(get_unaligned(>wMaxPacketSize));
-   clear_bit(IGNORE_BULK_OUT, >atomic_bitflags);
+   generic_clear_bit(IGNORE_BULK_OUT, >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