diff mbox series

[for-6.1?] bitops.h: revert db1ffc32dd ("qemu/bitops.h: add bitrev8 implementation")

Message ID 20210725110557.3007-1-mark.cave-ayland@ilande.co.uk
State New
Headers show
Series [for-6.1?] bitops.h: revert db1ffc32dd ("qemu/bitops.h: add bitrev8 implementation") | expand

Commit Message

Mark Cave-Ayland July 25, 2021, 11:05 a.m. UTC
Commit db1ffc32dd ("qemu/bitops.h: add bitrev8 implementation") introduced a
bitrev8() function to reverse the bit ordering required for storing the MAC
address in the q800 PROM.

This function is not required since QEMU implements its own revbit8() function
which does exactly the same thing. Remove the extraneous bitrev8() function and
switch its only caller in hw/m68k/q800.c to use revbit8() instead.

Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
---
 hw/m68k/q800.c        |  2 +-
 include/qemu/bitops.h | 22 ----------------------
 2 files changed, 1 insertion(+), 23 deletions(-)

---
I picked this up reading the loongarch thread where I realised that QEMU
already has a revbit8() function - I was searching for bitrev8() before
deciding that this needed to be added since this was the name of the equivalent
function in Linux.

I think this is a good candidate for 6.1 still because a) it only has 1 caller
which is easy for me to test and b) it prevents anyone else coming along and
accidentally using it later.

MCA.

Comments

Richard Henderson July 25, 2021, 11:59 a.m. UTC | #1
On 7/25/21 1:05 AM, Mark Cave-Ayland wrote:
> Commit db1ffc32dd ("qemu/bitops.h: add bitrev8 implementation") introduced a
> bitrev8() function to reverse the bit ordering required for storing the MAC
> address in the q800 PROM.
> 
> This function is not required since QEMU implements its own revbit8() function
> which does exactly the same thing. Remove the extraneous bitrev8() function and
> switch its only caller in hw/m68k/q800.c to use revbit8() instead.
> 
> Signed-off-by: Mark Cave-Ayland<mark.cave-ayland@ilande.co.uk>
> ---
>   hw/m68k/q800.c        |  2 +-
>   include/qemu/bitops.h | 22 ----------------------
>   2 files changed, 1 insertion(+), 23 deletions(-)
> 
> ---
> I picked this up reading the loongarch thread where I realised that QEMU
> already has a revbit8() function - I was searching for bitrev8() before
> deciding that this needed to be added since this was the name of the equivalent
> function in Linux.
> 
> I think this is a good candidate for 6.1 still because a) it only has 1 caller
> which is easy for me to test and b) it prevents anyone else coming along and
> accidentally using it later.
> 
> MCA.

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~
Philippe Mathieu-Daudé July 25, 2021, 9:23 p.m. UTC | #2
On 7/25/21 1:05 PM, Mark Cave-Ayland wrote:
> Commit db1ffc32dd ("qemu/bitops.h: add bitrev8 implementation") introduced a
> bitrev8() function to reverse the bit ordering required for storing the MAC
> address in the q800 PROM.
> 
> This function is not required since QEMU implements its own revbit8() function
> which does exactly the same thing. Remove the extraneous bitrev8() function and
> switch its only caller in hw/m68k/q800.c to use revbit8() instead.
> 
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/m68k/q800.c        |  2 +-
>  include/qemu/bitops.h | 22 ----------------------
>  2 files changed, 1 insertion(+), 23 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Richard Henderson July 26, 2021, 4:57 p.m. UTC | #3
On 7/25/21 1:05 AM, Mark Cave-Ayland wrote:
> Commit db1ffc32dd ("qemu/bitops.h: add bitrev8 implementation") introduced a
> bitrev8() function to reverse the bit ordering required for storing the MAC
> address in the q800 PROM.
> 
> This function is not required since QEMU implements its own revbit8() function
> which does exactly the same thing. Remove the extraneous bitrev8() function and
> switch its only caller in hw/m68k/q800.c to use revbit8() instead.
> 
> Signed-off-by: Mark Cave-Ayland<mark.cave-ayland@ilande.co.uk>
> ---
>   hw/m68k/q800.c        |  2 +-
>   include/qemu/bitops.h | 22 ----------------------
>   2 files changed, 1 insertion(+), 23 deletions(-)
> 
> ---
> I picked this up reading the loongarch thread where I realised that QEMU
> already has a revbit8() function - I was searching for bitrev8() before
> deciding that this needed to be added since this was the name of the equivalent
> function in Linux.
> 
> I think this is a good candidate for 6.1 still because a) it only has 1 caller
> which is easy for me to test and b) it prevents anyone else coming along and
> accidentally using it later.
> 
> MCA.

Queued for 6.1.


r~
diff mbox series

Patch

diff --git a/hw/m68k/q800.c b/hw/m68k/q800.c
index 6817c8b5d1..ac0a13060b 100644
--- a/hw/m68k/q800.c
+++ b/hw/m68k/q800.c
@@ -334,7 +334,7 @@  static void q800_init(MachineState *machine)
     prom = memory_region_get_ram_ptr(dp8393x_prom);
     checksum = 0;
     for (i = 0; i < 6; i++) {
-        prom[i] = bitrev8(nd_table[0].macaddr.a[i]);
+        prom[i] = revbit8(nd_table[0].macaddr.a[i]);
         checksum ^= prom[i];
     }
     prom[7] = 0xff - checksum;
diff --git a/include/qemu/bitops.h b/include/qemu/bitops.h
index 110c56e099..03213ce952 100644
--- a/include/qemu/bitops.h
+++ b/include/qemu/bitops.h
@@ -618,26 +618,4 @@  static inline uint64_t half_unshuffle64(uint64_t x)
     return x;
 }
 
-/**
- * bitrev8:
- * @x: 8-bit value to be reversed
- *
- * Given an input value with bits::
- *
- *   ABCDEFGH
- *
- * return the value with its bits reversed from left to right::
- *
- *   HGFEDCBA
- *
- * Returns: the bit-reversed value.
- */
-static inline uint8_t bitrev8(uint8_t x)
-{
-    x = ((x >> 1) & 0x55) | ((x << 1) & 0xaa);
-    x = ((x >> 2) & 0x33) | ((x << 2) & 0xcc);
-    x = (x >> 4) | (x << 4) ;
-    return x;
-}
-
 #endif