All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage
@ 2018-04-30 14:56 Bryan O'Donoghue
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 1/9] x86: Define PLATFORM__SET_BIT for generic_set_bit() Bryan O'Donoghue
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: Bryan O'Donoghue @ 2018-04-30 14:56 UTC (permalink / raw)
  To: u-boot

v3:
- Use linux/bitops.h instead of asm/bitops.h
  - checkpatch.pl

- Updated commit logs to make intended usage of __set_bit() and __clear_bit()
  clearer with respect to generic_set_bit() - Bin Meng

- Added Patch #8 changelong to cover-letter - Lukasz Majewski
  Initial patch fixes compile error for x86 by ifdeffing the local
  set_bit() clear_bit()

  Second patch renames to _set_bit() and _clear_bit() respectively

  Third patch - on further discussion we agree to use generic_set_bit()
  and generic_clear_bit().

  Using generic_set_bit() and generic_clear_bit() prompts me to do a
  general tidy-up of set_bit() and clear_bit() for USB gadget and to
  ensure where architectures provide __set_bit() and/or __clear_bit() that
  said functions are appropriately stitched into generic_set_bit() and
  generic_clear_bit() respectively.

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.


Bryan O'Donoghue (9):
  x86: Define PLATFORM__SET_BIT for generic_set_bit()
  riscv: Define PLATFORM__SET_BIT for generic_set_bit()
  riscv: Define PLATFORM__CLEAR_BIT for generic_clear_bit()
  nios2: Define PLATFORM__SET_BIT for generic_set_bit()
  nios2: Define PLATFORM__CLEAR_BIT for generic_clear_bit()
  nds32: Define PLATFORM__SET_BIT for generic_set_bit()
  nds32: Define PLATFORM__CLEAR_BIT for generic_clear_bit()
  usb: f_mass_storage: Fix set_bit and clear_bit usage
  usb: composite convert __set_bit to generic_set_bit

 arch/nds32/include/asm/bitops.h            |  4 ++++
 arch/nios2/include/asm/bitops/non-atomic.h |  4 ++++
 arch/riscv/include/asm/bitops.h            |  4 ++++
 arch/x86/include/asm/bitops.h              |  2 ++
 drivers/usb/gadget/composite.c             |  2 +-
 drivers/usb/gadget/f_mass_storage.c        | 25 +++-------------------
 6 files changed, 18 insertions(+), 23 deletions(-)

-- 
2.17.0

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v3 1/9] x86: Define PLATFORM__SET_BIT for generic_set_bit()
  2018-04-30 14:56 [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage Bryan O'Donoghue
@ 2018-04-30 14:56 ` Bryan O'Donoghue
  2018-05-16 12:49   ` [U-Boot] [U-Boot, v3, " Tom Rini
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 2/9] riscv: " Bryan O'Donoghue
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Bryan O'Donoghue @ 2018-04-30 14:56 UTC (permalink / raw)
  To: u-boot

x86 bitops.h provides a __set_bit() but does not define PLATFORM__SET_BIT
as a result generic_set_bit() is used instead of the architecturally
provided __set_bit().

This patch defines PLATFORM__SET_BIT which means that __set_bit() in x86
bitops.h will be called whenever generic_set_bit() is called - as opposed
to the default cross-platform generic_set_bit().

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Cc: Simon Glass <sjg@chromium.org>
Cc: Bin Meng <bmeng.cn@gmail.com>
---
 arch/x86/include/asm/bitops.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/x86/include/asm/bitops.h b/arch/x86/include/asm/bitops.h
index f97dc66439..196fcf9d3f 100644
--- a/arch/x86/include/asm/bitops.h
+++ b/arch/x86/include/asm/bitops.h
@@ -61,6 +61,8 @@ static __inline__ void __set_bit(int nr, volatile void * addr)
 		:"Ir" (nr));
 }
 
+#define PLATFORM__SET_BIT
+
 /**
  * clear_bit - Clears a bit in memory
  * @nr: Bit to clear
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v3 2/9] riscv: Define PLATFORM__SET_BIT for generic_set_bit()
  2018-04-30 14:56 [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage Bryan O'Donoghue
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 1/9] x86: Define PLATFORM__SET_BIT for generic_set_bit() Bryan O'Donoghue
@ 2018-04-30 14:56 ` Bryan O'Donoghue
  2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 3/9] riscv: Define PLATFORM__CLEAR_BIT for generic_clear_bit() Bryan O'Donoghue
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Bryan O'Donoghue @ 2018-04-30 14:56 UTC (permalink / raw)
  To: u-boot

riscv bitops.h provides a __set_bit() but does not define PLATFORM__SET_BIT
as a result generic_set_bit() is used instead of the architecturally
provided __set_bit().

This patch defines PLATFORM__SET_BIT which means that __set_bit() in x86
bitops.h will be called whenever generic_set_bit() is called - as opposed
to the default cross-platform generic_set_bit().

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Cc: Rick Chen <rick@andestech.com>
Cc: Greentime Hu <green.hu@gmail.com>
---
 arch/riscv/include/asm/bitops.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/include/asm/bitops.h b/arch/riscv/include/asm/bitops.h
index 55d420fdfb..0149e5c696 100644
--- a/arch/riscv/include/asm/bitops.h
+++ b/arch/riscv/include/asm/bitops.h
@@ -42,6 +42,8 @@ static inline void __set_bit(int nr, void *addr)
 	*a |= mask;
 }
 
+#define PLATFORM__SET_BIT
+
 static inline void __clear_bit(int nr, void *addr)
 {
 	int *a = (int *)addr;
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v3 3/9] riscv: Define PLATFORM__CLEAR_BIT for generic_clear_bit()
  2018-04-30 14:56 [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage Bryan O'Donoghue
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 1/9] x86: Define PLATFORM__SET_BIT for generic_set_bit() Bryan O'Donoghue
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 2/9] riscv: " Bryan O'Donoghue
@ 2018-04-30 14:56 ` Bryan O'Donoghue
  2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 4/9] nios2: Define PLATFORM__SET_BIT for generic_set_bit() Bryan O'Donoghue
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Bryan O'Donoghue @ 2018-04-30 14:56 UTC (permalink / raw)
  To: u-boot

riscv bitops.h provides a __clear_bit() but does not define
PLATFORM__CLEAR_BIT as a result generic_clear_bit() is used instead of the
architecturally provided __clear_bit().

This patch defines PLATFORM__CLEAR_BIT which means that __clear_bit() in
riscv bitops.h will be called whenever generic_clear_bit() is called - as
opposed to the default cross-platform generic_clear_bit().

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Cc: Rick Chen <rick@andestech.com>
Cc: Greentime Hu <green.hu@gmail.com>
---
 arch/riscv/include/asm/bitops.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/riscv/include/asm/bitops.h b/arch/riscv/include/asm/bitops.h
index 0149e5c696..536629bbec 100644
--- a/arch/riscv/include/asm/bitops.h
+++ b/arch/riscv/include/asm/bitops.h
@@ -54,6 +54,8 @@ static inline void __clear_bit(int nr, void *addr)
 	*a &= ~mask;
 }
 
+#define PLATFORM__CLEAR_BIT
+
 static inline void __change_bit(int nr, void *addr)
 {
 	int mask;
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v3 4/9] nios2: Define PLATFORM__SET_BIT for generic_set_bit()
  2018-04-30 14:56 [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage Bryan O'Donoghue
                   ` (2 preceding siblings ...)
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 3/9] riscv: Define PLATFORM__CLEAR_BIT for generic_clear_bit() Bryan O'Donoghue
@ 2018-04-30 14:56 ` Bryan O'Donoghue
  2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 5/9] nios2: Define PLATFORM__CLEAR_BIT for generic_clear_bit() Bryan O'Donoghue
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Bryan O'Donoghue @ 2018-04-30 14:56 UTC (permalink / raw)
  To: u-boot

nios2 bitops.h provides a __set_bit() but does not define PLATFORM__SET_BIT
as a result generic_set_bit() is used instead of the architecturally
provided __set_bit().

This patch defines PLATFORM__SET_BIT which means that __set_bit() in nios2
bitops.h will be called whenever generic_set_bit() is called - as opposed
to the default cross-platform generic_set_bit().

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Cc: Thomas Chou <thomas@wytron.com.tw>
---
 arch/nios2/include/asm/bitops/non-atomic.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/nios2/include/asm/bitops/non-atomic.h b/arch/nios2/include/asm/bitops/non-atomic.h
index 697cc2b7e0..9dd9d923e1 100644
--- a/arch/nios2/include/asm/bitops/non-atomic.h
+++ b/arch/nios2/include/asm/bitops/non-atomic.h
@@ -20,6 +20,8 @@ static inline void __set_bit(int nr, volatile unsigned long *addr)
 	*p  |= mask;
 }
 
+#define PLATFORM__SET_BIT
+
 static inline void __clear_bit(int nr, volatile unsigned long *addr)
 {
 	unsigned long mask = BIT_MASK(nr);
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v3 5/9] nios2: Define PLATFORM__CLEAR_BIT for generic_clear_bit()
  2018-04-30 14:56 [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage Bryan O'Donoghue
                   ` (3 preceding siblings ...)
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 4/9] nios2: Define PLATFORM__SET_BIT for generic_set_bit() Bryan O'Donoghue
@ 2018-04-30 14:56 ` Bryan O'Donoghue
  2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 6/9] nds32: Define PLATFORM__SET_BIT for generic_set_bit() Bryan O'Donoghue
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Bryan O'Donoghue @ 2018-04-30 14:56 UTC (permalink / raw)
  To: u-boot

nios2 bitops.h provides a __clear_bit() but does not define
PLATFORM__CLEAR_BIT as a result generic_clear_bit() is used instead of the
architecturally provided __clear_bit().

This patch defines PLATFORM__CLEAR_BIT which means that __clear_bit() in
nios2 bitops.h will be called whenever generic_clear_bit() is called - as
opposed to the default cross-platform generic_clear_bit().

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Cc: Thomas Chou <thomas@wytron.com.tw>
---
 arch/nios2/include/asm/bitops/non-atomic.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/nios2/include/asm/bitops/non-atomic.h b/arch/nios2/include/asm/bitops/non-atomic.h
index 9dd9d923e1..f746819b43 100644
--- a/arch/nios2/include/asm/bitops/non-atomic.h
+++ b/arch/nios2/include/asm/bitops/non-atomic.h
@@ -30,6 +30,8 @@ static inline void __clear_bit(int nr, volatile unsigned long *addr)
 	*p &= ~mask;
 }
 
+#define PLATFORM__CLEAR_BIT
+
 /**
  * __change_bit - Toggle a bit in memory
  * @nr: the bit to change
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v3 6/9] nds32: Define PLATFORM__SET_BIT for generic_set_bit()
  2018-04-30 14:56 [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage Bryan O'Donoghue
                   ` (4 preceding siblings ...)
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 5/9] nios2: Define PLATFORM__CLEAR_BIT for generic_clear_bit() Bryan O'Donoghue
@ 2018-04-30 14:56 ` Bryan O'Donoghue
  2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 7/9] nds32: Define PLATFORM__CLEAR_BIT for generic_clear_bit() Bryan O'Donoghue
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Bryan O'Donoghue @ 2018-04-30 14:56 UTC (permalink / raw)
  To: u-boot

nds32 bitops.h provides a __set_bit() but does not define PLATFORM__SET_BIT
as a result generic_set_bit() is used instead of the architecturally
provided __set_bit().

This patch defines PLATFORM__SET_BIT which means that __set_bit() in nds32
bitops.h will be called whenever generic_set_bit() is called - as opposed
to the default cross-platform generic_set_bit().

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Cc: Macpaul Lin <macpaul@andestech.com>
---
 arch/nds32/include/asm/bitops.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/nds32/include/asm/bitops.h b/arch/nds32/include/asm/bitops.h
index 7ee37c37bc..ecd0032664 100644
--- a/arch/nds32/include/asm/bitops.h
+++ b/arch/nds32/include/asm/bitops.h
@@ -44,6 +44,8 @@ static inline void __set_bit(int nr, void *addr)
 	*a |= mask;
 }
 
+#define PLATFORM__SET_BIT
+
 extern void clear_bit(int nr, void *addr);
 
 static inline void __clear_bit(int nr, void *addr)
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v3 7/9] nds32: Define PLATFORM__CLEAR_BIT for generic_clear_bit()
  2018-04-30 14:56 [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage Bryan O'Donoghue
                   ` (5 preceding siblings ...)
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 6/9] nds32: Define PLATFORM__SET_BIT for generic_set_bit() Bryan O'Donoghue
@ 2018-04-30 14:56 ` Bryan O'Donoghue
  2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 8/9] usb: f_mass_storage: Fix set_bit and clear_bit usage Bryan O'Donoghue
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Bryan O'Donoghue @ 2018-04-30 14:56 UTC (permalink / raw)
  To: u-boot

nds2 bitops.h provides a __clear_bit() but does not define
PLATFORM__CLEAR_BIT as a result generic_clear_bit() is used instead of the
architecturally provided __clear_bit().

This patch defines PLATFORM__CLEAR_BIT which means that __clear_bit() in
nds32 bitops.h will be called whenever generic_clear_bit() is called - as
opposed to the default cross-platform generic_clear_bit().

Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
Cc: Macpaul Lin <macpaul@andestech.com>
---
 arch/nds32/include/asm/bitops.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/nds32/include/asm/bitops.h b/arch/nds32/include/asm/bitops.h
index ecd0032664..f1cdcf3e65 100644
--- a/arch/nds32/include/asm/bitops.h
+++ b/arch/nds32/include/asm/bitops.h
@@ -61,6 +61,8 @@ static inline void __clear_bit(int nr, void *addr)
 	local_irq_restore(flags);
 }
 
+#define PLATFORM__CLEAR_BIT
+
 extern void change_bit(int nr, void *addr);
 
 static inline void __change_bit(int nr, void *addr)
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v3 8/9] usb: f_mass_storage: Fix set_bit and clear_bit usage
  2018-04-30 14:56 [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage Bryan O'Donoghue
                   ` (6 preceding siblings ...)
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 7/9] nds32: Define PLATFORM__CLEAR_BIT for generic_clear_bit() Bryan O'Donoghue
@ 2018-04-30 14:56 ` Bryan O'Donoghue
  2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 9/9] usb: composite convert __set_bit to generic_set_bit Bryan O'Donoghue
  2018-05-10 22:26 ` [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage Tom Rini
  9 siblings, 1 reply; 20+ messages in thread
From: Bryan O'Donoghue @ 2018-04-30 14:56 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

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 <pure.logic@nexus-software.ie>
Cc: Lukasz Majewski <lukma@denx.de>
Cc: Marek Vasut <marex@denx.de>
---
 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 <usb_mass_storage.h>
 
 #include <asm/unaligned.h>
+#include <linux/bitops.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/gadget.h>
 #include <linux/usb/composite.h>
@@ -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, &fsg->atomic_bitflags);
+		generic_set_bit(IGNORE_BULK_OUT, &fsg->atomic_bitflags);
 		return -EINVAL;
 	}
 
@@ -2250,7 +2231,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);
+	generic_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] 20+ messages in thread

* [U-Boot] [PATCH v3 9/9] usb: composite convert __set_bit to generic_set_bit
  2018-04-30 14:56 [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage Bryan O'Donoghue
                   ` (7 preceding siblings ...)
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 8/9] usb: f_mass_storage: Fix set_bit and clear_bit usage Bryan O'Donoghue
@ 2018-04-30 14:56 ` Bryan O'Donoghue
  2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
  2018-05-10 22:26 ` [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage Tom Rini
  9 siblings, 1 reply; 20+ messages in thread
From: Bryan O'Donoghue @ 2018-04-30 14:56 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

To address that situation we discussed on the list moving to
genetic_set_bit() instead.

Doing a quick grep for similar situations in drivers/usb shows that the
composite device is using __set_bit().

This patch switches over to generic_set_bit to maintain consistency between
the two gadget drivers.

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/composite.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index a87639def9..9229d9ee30 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -379,7 +379,7 @@ static int set_config(struct usb_composite_dev *cdev,
 			ep = (struct usb_endpoint_descriptor *)*descriptors;
 			addr = ((ep->bEndpointAddress & 0x80) >> 3)
 			     |	(ep->bEndpointAddress & 0x0f);
-			__set_bit(addr, f->endpoints);
+			generic_set_bit(addr, f->endpoints);
 		}
 
 		result = f->set_alt(f, tmp, 0);
-- 
2.17.0

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage
  2018-04-30 14:56 [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage Bryan O'Donoghue
                   ` (8 preceding siblings ...)
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 9/9] usb: composite convert __set_bit to generic_set_bit Bryan O'Donoghue
@ 2018-05-10 22:26 ` Tom Rini
  9 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2018-05-10 22:26 UTC (permalink / raw)
  To: u-boot

On Mon, Apr 30, 2018 at 03:56:01PM +0100, Bryan O'Donoghue wrote:

> v3:
> - Use linux/bitops.h instead of asm/bitops.h
>   - checkpatch.pl
> 
> - Updated commit logs to make intended usage of __set_bit() and __clear_bit()
>   clearer with respect to generic_set_bit() - Bin Meng
> 
> - Added Patch #8 changelong to cover-letter - Lukasz Majewski
>   Initial patch fixes compile error for x86 by ifdeffing the local
>   set_bit() clear_bit()
> 
>   Second patch renames to _set_bit() and _clear_bit() respectively
> 
>   Third patch - on further discussion we agree to use generic_set_bit()
>   and generic_clear_bit().
> 
>   Using generic_set_bit() and generic_clear_bit() prompts me to do a
>   general tidy-up of set_bit() and clear_bit() for USB gadget and to
>   ensure where architectures provide __set_bit() and/or __clear_bit() that
>   said functions are appropriately stitched into generic_set_bit() and
>   generic_clear_bit() respectively.
> 
> 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.
> 
> 
> Bryan O'Donoghue (9):
>   x86: Define PLATFORM__SET_BIT for generic_set_bit()
>   riscv: Define PLATFORM__SET_BIT for generic_set_bit()
>   riscv: Define PLATFORM__CLEAR_BIT for generic_clear_bit()
>   nios2: Define PLATFORM__SET_BIT for generic_set_bit()
>   nios2: Define PLATFORM__CLEAR_BIT for generic_clear_bit()
>   nds32: Define PLATFORM__SET_BIT for generic_set_bit()
>   nds32: Define PLATFORM__CLEAR_BIT for generic_clear_bit()
>   usb: f_mass_storage: Fix set_bit and clear_bit usage
>   usb: composite convert __set_bit to generic_set_bit
> 
>  arch/nds32/include/asm/bitops.h            |  4 ++++
>  arch/nios2/include/asm/bitops/non-atomic.h |  4 ++++
>  arch/riscv/include/asm/bitops.h            |  4 ++++
>  arch/x86/include/asm/bitops.h              |  2 ++
>  drivers/usb/gadget/composite.c             |  2 +-
>  drivers/usb/gadget/f_mass_storage.c        | 25 +++-------------------
>  6 files changed, 18 insertions(+), 23 deletions(-)

This seems right, but I see a whole lot of CC and no Acks/Reviews, so
I'm going to wait a bit more before grabbing it.  It would be great to
see a travis-ci link before I try myself, thanks!

> 
> -- 
> 2.17.0
> 
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180510/c034a7fb/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [U-Boot, v3, 1/9] x86: Define PLATFORM__SET_BIT for generic_set_bit()
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 1/9] x86: Define PLATFORM__SET_BIT for generic_set_bit() Bryan O'Donoghue
@ 2018-05-16 12:49   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2018-05-16 12:49 UTC (permalink / raw)
  To: u-boot

On Mon, Apr 30, 2018 at 03:56:02PM +0100, Bryan O'Donoghue wrote:

> x86 bitops.h provides a __set_bit() but does not define PLATFORM__SET_BIT
> as a result generic_set_bit() is used instead of the architecturally
> provided __set_bit().
> 
> This patch defines PLATFORM__SET_BIT which means that __set_bit() in x86
> bitops.h will be called whenever generic_set_bit() is called - as opposed
> to the default cross-platform generic_set_bit().
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> Cc: Simon Glass <sjg@chromium.org>
> Cc: Bin Meng <bmeng.cn@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180516/f97e59e6/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [U-Boot, v3, 2/9] riscv: Define PLATFORM__SET_BIT for generic_set_bit()
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 2/9] riscv: " Bryan O'Donoghue
@ 2018-05-16 12:50   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2018-05-16 12:50 UTC (permalink / raw)
  To: u-boot

On Mon, Apr 30, 2018 at 03:56:03PM +0100, Bryan O'Donoghue wrote:

> riscv bitops.h provides a __set_bit() but does not define PLATFORM__SET_BIT
> as a result generic_set_bit() is used instead of the architecturally
> provided __set_bit().
> 
> This patch defines PLATFORM__SET_BIT which means that __set_bit() in x86
> bitops.h will be called whenever generic_set_bit() is called - as opposed
> to the default cross-platform generic_set_bit().
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> Cc: Rick Chen <rick@andestech.com>
> Cc: Greentime Hu <green.hu@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180516/b5c18187/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [U-Boot, v3, 3/9] riscv: Define PLATFORM__CLEAR_BIT for generic_clear_bit()
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 3/9] riscv: Define PLATFORM__CLEAR_BIT for generic_clear_bit() Bryan O'Donoghue
@ 2018-05-16 12:50   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2018-05-16 12:50 UTC (permalink / raw)
  To: u-boot

On Mon, Apr 30, 2018 at 03:56:04PM +0100, Bryan O'Donoghue wrote:

> riscv bitops.h provides a __clear_bit() but does not define
> PLATFORM__CLEAR_BIT as a result generic_clear_bit() is used instead of the
> architecturally provided __clear_bit().
> 
> This patch defines PLATFORM__CLEAR_BIT which means that __clear_bit() in
> riscv bitops.h will be called whenever generic_clear_bit() is called - as
> opposed to the default cross-platform generic_clear_bit().
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> Cc: Rick Chen <rick@andestech.com>
> Cc: Greentime Hu <green.hu@gmail.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180516/9d741c95/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [U-Boot, v3, 4/9] nios2: Define PLATFORM__SET_BIT for generic_set_bit()
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 4/9] nios2: Define PLATFORM__SET_BIT for generic_set_bit() Bryan O'Donoghue
@ 2018-05-16 12:50   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2018-05-16 12:50 UTC (permalink / raw)
  To: u-boot

On Mon, Apr 30, 2018 at 03:56:05PM +0100, Bryan O'Donoghue wrote:

> nios2 bitops.h provides a __set_bit() but does not define PLATFORM__SET_BIT
> as a result generic_set_bit() is used instead of the architecturally
> provided __set_bit().
> 
> This patch defines PLATFORM__SET_BIT which means that __set_bit() in nios2
> bitops.h will be called whenever generic_set_bit() is called - as opposed
> to the default cross-platform generic_set_bit().
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> Cc: Thomas Chou <thomas@wytron.com.tw>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180516/c17a7f9e/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [U-Boot, v3, 5/9] nios2: Define PLATFORM__CLEAR_BIT for generic_clear_bit()
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 5/9] nios2: Define PLATFORM__CLEAR_BIT for generic_clear_bit() Bryan O'Donoghue
@ 2018-05-16 12:50   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2018-05-16 12:50 UTC (permalink / raw)
  To: u-boot

On Mon, Apr 30, 2018 at 03:56:06PM +0100, Bryan O'Donoghue wrote:

> nios2 bitops.h provides a __clear_bit() but does not define
> PLATFORM__CLEAR_BIT as a result generic_clear_bit() is used instead of the
> architecturally provided __clear_bit().
> 
> This patch defines PLATFORM__CLEAR_BIT which means that __clear_bit() in
> nios2 bitops.h will be called whenever generic_clear_bit() is called - as
> opposed to the default cross-platform generic_clear_bit().
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> Cc: Thomas Chou <thomas@wytron.com.tw>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180516/bdf3de16/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [U-Boot, v3, 6/9] nds32: Define PLATFORM__SET_BIT for generic_set_bit()
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 6/9] nds32: Define PLATFORM__SET_BIT for generic_set_bit() Bryan O'Donoghue
@ 2018-05-16 12:50   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2018-05-16 12:50 UTC (permalink / raw)
  To: u-boot

On Mon, Apr 30, 2018 at 03:56:07PM +0100, Bryan O'Donoghue wrote:

> nds32 bitops.h provides a __set_bit() but does not define PLATFORM__SET_BIT
> as a result generic_set_bit() is used instead of the architecturally
> provided __set_bit().
> 
> This patch defines PLATFORM__SET_BIT which means that __set_bit() in nds32
> bitops.h will be called whenever generic_set_bit() is called - as opposed
> to the default cross-platform generic_set_bit().
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> Cc: Macpaul Lin <macpaul@andestech.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180516/4ff9e6fc/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [U-Boot, v3, 7/9] nds32: Define PLATFORM__CLEAR_BIT for generic_clear_bit()
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 7/9] nds32: Define PLATFORM__CLEAR_BIT for generic_clear_bit() Bryan O'Donoghue
@ 2018-05-16 12:50   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2018-05-16 12:50 UTC (permalink / raw)
  To: u-boot

On Mon, Apr 30, 2018 at 03:56:08PM +0100, Bryan O'Donoghue wrote:

> nds2 bitops.h provides a __clear_bit() but does not define
> PLATFORM__CLEAR_BIT as a result generic_clear_bit() is used instead of the
> architecturally provided __clear_bit().
> 
> This patch defines PLATFORM__CLEAR_BIT which means that __clear_bit() in
> nds32 bitops.h will be called whenever generic_clear_bit() is called - as
> opposed to the default cross-platform generic_clear_bit().
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> Cc: Macpaul Lin <macpaul@andestech.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180516/26c16bc4/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [U-Boot, v3, 8/9] usb: f_mass_storage: Fix set_bit and clear_bit usage
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 8/9] usb: f_mass_storage: Fix set_bit and clear_bit usage Bryan O'Donoghue
@ 2018-05-16 12:50   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2018-05-16 12:50 UTC (permalink / raw)
  To: u-boot

On Mon, Apr 30, 2018 at 03:56:09PM +0100, 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
> 
> 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 <pure.logic@nexus-software.ie>
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Marek Vasut <marex@denx.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180516/8932109e/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [U-Boot] [U-Boot, v3, 9/9] usb: composite convert __set_bit to generic_set_bit
  2018-04-30 14:56 ` [U-Boot] [PATCH v3 9/9] usb: composite convert __set_bit to generic_set_bit Bryan O'Donoghue
@ 2018-05-16 12:50   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2018-05-16 12:50 UTC (permalink / raw)
  To: u-boot

On Mon, Apr 30, 2018 at 03:56:10PM +0100, 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
> 
> To address that situation we discussed on the list moving to
> genetic_set_bit() instead.
> 
> Doing a quick grep for similar situations in drivers/usb shows that the
> composite device is using __set_bit().
> 
> This patch switches over to generic_set_bit to maintain consistency between
> the two gadget drivers.
> 
> Signed-off-by: Bryan O'Donoghue <pure.logic@nexus-software.ie>
> Cc: Lukasz Majewski <lukma@denx.de>
> Cc: Marek Vasut <marex@denx.de>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180516/42c836b8/attachment.sig>

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2018-05-16 12:50 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-30 14:56 [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage Bryan O'Donoghue
2018-04-30 14:56 ` [U-Boot] [PATCH v3 1/9] x86: Define PLATFORM__SET_BIT for generic_set_bit() Bryan O'Donoghue
2018-05-16 12:49   ` [U-Boot] [U-Boot, v3, " Tom Rini
2018-04-30 14:56 ` [U-Boot] [PATCH v3 2/9] riscv: " Bryan O'Donoghue
2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
2018-04-30 14:56 ` [U-Boot] [PATCH v3 3/9] riscv: Define PLATFORM__CLEAR_BIT for generic_clear_bit() Bryan O'Donoghue
2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
2018-04-30 14:56 ` [U-Boot] [PATCH v3 4/9] nios2: Define PLATFORM__SET_BIT for generic_set_bit() Bryan O'Donoghue
2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
2018-04-30 14:56 ` [U-Boot] [PATCH v3 5/9] nios2: Define PLATFORM__CLEAR_BIT for generic_clear_bit() Bryan O'Donoghue
2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
2018-04-30 14:56 ` [U-Boot] [PATCH v3 6/9] nds32: Define PLATFORM__SET_BIT for generic_set_bit() Bryan O'Donoghue
2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
2018-04-30 14:56 ` [U-Boot] [PATCH v3 7/9] nds32: Define PLATFORM__CLEAR_BIT for generic_clear_bit() Bryan O'Donoghue
2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
2018-04-30 14:56 ` [U-Boot] [PATCH v3 8/9] usb: f_mass_storage: Fix set_bit and clear_bit usage Bryan O'Donoghue
2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
2018-04-30 14:56 ` [U-Boot] [PATCH v3 9/9] usb: composite convert __set_bit to generic_set_bit Bryan O'Donoghue
2018-05-16 12:50   ` [U-Boot] [U-Boot, v3, " Tom Rini
2018-05-10 22:26 ` [U-Boot] [PATCH v3 0/9] Fixup set_bit/clear_bit definition and usage Tom Rini

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.