mbox series

[U-Boot,v3,0/9] Fixup set_bit/clear_bit definition and usage

Message ID 20180430145610.18250-1-pure.logic@nexus-software.ie
Headers show
Series Fixup set_bit/clear_bit definition and usage | expand

Message

Bryan O'Donoghue April 30, 2018, 2:56 p.m. UTC
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(-)

Comments

Tom Rini May 10, 2018, 10:26 p.m. UTC | #1
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@lists.denx.de
> https://lists.denx.de/listinfo/u-boot