[v3,33/41] hw/net: Use the BYTE-based definitions

Message ID 20180415234307.28132-34-f4bug@amsat.org
State New
Headers show
Series
  • hw: Use the BYTE-based definitions when useful
Related show

Commit Message

Philippe Mathieu-Daudé April 15, 2018, 11:42 p.m.
It eases code review, unit is explicit.

Patch generated using:

  $ git grep -E '(1024|2048|4096|8192|(<<|>>).?(10|20|30))' hw/ include/hw/

and modified manually.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 include/hw/net/allwinner_emac.h | 5 +++--
 hw/net/e1000e.c                 | 7 ++++---
 hw/net/e1000x_common.c          | 3 ++-
 hw/net/eepro100.c               | 7 +++----
 4 files changed, 12 insertions(+), 10 deletions(-)

Comments

Stefan Weil April 16, 2018, 5:23 a.m. | #1
Am 16.04.2018 um 01:42 schrieb Philippe Mathieu-Daudé:
> It eases code review, unit is explicit.
> 
> Patch generated using:
> 
>   $ git grep -E '(1024|2048|4096|8192|(<<|>>).?(10|20|30))' hw/ include/hw/
> 
> and modified manually.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  include/hw/net/allwinner_emac.h | 5 +++--
>  hw/net/e1000e.c                 | 7 ++++---
>  hw/net/e1000x_common.c          | 3 ++-
>  hw/net/eepro100.c               | 7 +++----
>  4 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/include/hw/net/allwinner_emac.h b/include/hw/net/allwinner_emac.h
> index 4cc8aab7ec..4b53b6485c 100644
> --- a/include/hw/net/allwinner_emac.h
> +++ b/include/hw/net/allwinner_emac.h
> @@ -23,6 +23,7 @@
>  #ifndef ALLWINNER_EMAC_H
>  #define ALLWINNER_EMAC_H
>  
> +#include "qemu/units.h"
>  #include "net/net.h"
>  #include "qemu/fifo8.h"
>  #include "hw/net/mii.h"
> @@ -125,8 +126,8 @@
>  #define EMAC_INT_RX         (1 << 8)
>  
>  /* Due to lack of specifications, size of fifos is chosen arbitrarily */
> -#define TX_FIFO_SIZE        (4 * 1024)
> -#define RX_FIFO_SIZE        (32 * 1024)
> +#define TX_FIFO_SIZE        (4 * K_BYTE)
> +#define RX_FIFO_SIZE        (32 * K_BYTE)
>  
>  #define NUM_TX_FIFOS        2
>  #define RX_HDR_SIZE         8
> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
> index 16a9417a85..101efe7c83 100644
> --- a/hw/net/e1000e.c
> +++ b/hw/net/e1000e.c
> @@ -34,6 +34,7 @@
>  */
>  
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "net/net.h"
>  #include "net/tap.h"
>  #include "qemu/range.h"
> @@ -81,10 +82,10 @@ typedef struct E1000EState {
>  #define E1000E_IO_IDX       2
>  #define E1000E_MSIX_IDX     3
>  
> -#define E1000E_MMIO_SIZE    (128 * 1024)
> -#define E1000E_FLASH_SIZE   (128 * 1024)
> +#define E1000E_MMIO_SIZE    (128 * K_BYTE)
> +#define E1000E_FLASH_SIZE   (128 * K_BYTE)
>  #define E1000E_IO_SIZE      (32)
> -#define E1000E_MSIX_SIZE    (16 * 1024)
> +#define E1000E_MSIX_SIZE    (16 * K_BYTE)
>  
>  #define E1000E_MSIX_TABLE   (0x0000)
>  #define E1000E_MSIX_PBA     (0x2000)
> diff --git a/hw/net/e1000x_common.c b/hw/net/e1000x_common.c
> index eb0e097137..58c8db77e9 100644
> --- a/hw/net/e1000x_common.c
> +++ b/hw/net/e1000x_common.c
> @@ -23,6 +23,7 @@
>  */
>  
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "hw/hw.h"
>  #include "hw/pci/pci.h"
>  #include "net/net.h"
> @@ -111,7 +112,7 @@ bool e1000x_is_oversized(uint32_t *mac, size_t size)
>      static const int maximum_ethernet_vlan_size = 1522;
>      /* this is the size past which hardware will
>         drop packets when setting LPE=1 */
> -    static const int maximum_ethernet_lpe_size = 16384;
> +    static const int maximum_ethernet_lpe_size = 16 * K_BYTE;
>  
>      if ((size > maximum_ethernet_lpe_size ||
>          (size > maximum_ethernet_vlan_size
> diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
> index a07a63247e..a02e2b55e8 100644
> --- a/hw/net/eepro100.c
> +++ b/hw/net/eepro100.c
> @@ -41,6 +41,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> +#include "qemu/units.h"
>  #include "hw/hw.h"
>  #include "hw/pci/pci.h"
>  #include "net/net.h"
> @@ -60,8 +61,6 @@
>   * changed to pad short packets itself. */
>  #define CONFIG_PAD_RECEIVED_FRAMES
>  
> -#define KiB 1024
> -
>  /* Debug EEPRO100 card. */
>  #if 0
>  # define DEBUG_EEPRO100
> @@ -104,9 +103,9 @@
>  /* Use 64 word EEPROM. TODO: could be a runtime option. */
>  #define EEPROM_SIZE     64
>  
> -#define PCI_MEM_SIZE            (4 * KiB)
> +#define PCI_MEM_SIZE            (4 * K_BYTE)
>  #define PCI_IO_SIZE             64
> -#define PCI_FLASH_SIZE          (128 * KiB)
> +#define PCI_FLASH_SIZE          (128 * K_BYTE)
>  
>  #define BITS(n, m) (((0xffffffffU << (31 - n)) >> (31 - n + m)) << m)


Technically this is fine, therefore

Reviewed-by: Stefan Weil <sw@weilnetz.de>

Practically I'd prefer replacing all K_BYTE by KiB, because that is the
standard name with a precise definition:
https://en.wikipedia.org/wiki/Kibibyte.

Stefan
Philippe Mathieu-Daudé April 17, 2018, 4:09 p.m. | #2
Hi Stefan,

On 04/16/2018 02:23 AM, Stefan Weil wrote:
> Am 16.04.2018 um 01:42 schrieb Philippe Mathieu-Daudé:
>> It eases code review, unit is explicit.
>>
>> Patch generated using:
>>
>>   $ git grep -E '(1024|2048|4096|8192|(<<|>>).?(10|20|30))' hw/ include/hw/
>>
>> and modified manually.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  include/hw/net/allwinner_emac.h | 5 +++--
>>  hw/net/e1000e.c                 | 7 ++++---
>>  hw/net/e1000x_common.c          | 3 ++-
>>  hw/net/eepro100.c               | 7 +++----
>>  4 files changed, 12 insertions(+), 10 deletions(-)
>>
>> diff --git a/include/hw/net/allwinner_emac.h b/include/hw/net/allwinner_emac.h
>> index 4cc8aab7ec..4b53b6485c 100644
>> --- a/include/hw/net/allwinner_emac.h
>> +++ b/include/hw/net/allwinner_emac.h
>> @@ -23,6 +23,7 @@
>>  #ifndef ALLWINNER_EMAC_H
>>  #define ALLWINNER_EMAC_H
>>  
>> +#include "qemu/units.h"
>>  #include "net/net.h"
>>  #include "qemu/fifo8.h"
>>  #include "hw/net/mii.h"
>> @@ -125,8 +126,8 @@
>>  #define EMAC_INT_RX         (1 << 8)
>>  
>>  /* Due to lack of specifications, size of fifos is chosen arbitrarily */
>> -#define TX_FIFO_SIZE        (4 * 1024)
>> -#define RX_FIFO_SIZE        (32 * 1024)
>> +#define TX_FIFO_SIZE        (4 * K_BYTE)
>> +#define RX_FIFO_SIZE        (32 * K_BYTE)
>>  
>>  #define NUM_TX_FIFOS        2
>>  #define RX_HDR_SIZE         8
>> diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
>> index 16a9417a85..101efe7c83 100644
>> --- a/hw/net/e1000e.c
>> +++ b/hw/net/e1000e.c
>> @@ -34,6 +34,7 @@
>>  */
>>  
>>  #include "qemu/osdep.h"
>> +#include "qemu/units.h"
>>  #include "net/net.h"
>>  #include "net/tap.h"
>>  #include "qemu/range.h"
>> @@ -81,10 +82,10 @@ typedef struct E1000EState {
>>  #define E1000E_IO_IDX       2
>>  #define E1000E_MSIX_IDX     3
>>  
>> -#define E1000E_MMIO_SIZE    (128 * 1024)
>> -#define E1000E_FLASH_SIZE   (128 * 1024)
>> +#define E1000E_MMIO_SIZE    (128 * K_BYTE)
>> +#define E1000E_FLASH_SIZE   (128 * K_BYTE)
>>  #define E1000E_IO_SIZE      (32)
>> -#define E1000E_MSIX_SIZE    (16 * 1024)
>> +#define E1000E_MSIX_SIZE    (16 * K_BYTE)
>>  
>>  #define E1000E_MSIX_TABLE   (0x0000)
>>  #define E1000E_MSIX_PBA     (0x2000)
>> diff --git a/hw/net/e1000x_common.c b/hw/net/e1000x_common.c
>> index eb0e097137..58c8db77e9 100644
>> --- a/hw/net/e1000x_common.c
>> +++ b/hw/net/e1000x_common.c
>> @@ -23,6 +23,7 @@
>>  */
>>  
>>  #include "qemu/osdep.h"
>> +#include "qemu/units.h"
>>  #include "hw/hw.h"
>>  #include "hw/pci/pci.h"
>>  #include "net/net.h"
>> @@ -111,7 +112,7 @@ bool e1000x_is_oversized(uint32_t *mac, size_t size)
>>      static const int maximum_ethernet_vlan_size = 1522;
>>      /* this is the size past which hardware will
>>         drop packets when setting LPE=1 */
>> -    static const int maximum_ethernet_lpe_size = 16384;
>> +    static const int maximum_ethernet_lpe_size = 16 * K_BYTE;
>>  
>>      if ((size > maximum_ethernet_lpe_size ||
>>          (size > maximum_ethernet_vlan_size
>> diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
>> index a07a63247e..a02e2b55e8 100644
>> --- a/hw/net/eepro100.c
>> +++ b/hw/net/eepro100.c
>> @@ -41,6 +41,7 @@
>>   */
>>  
>>  #include "qemu/osdep.h"
>> +#include "qemu/units.h"
>>  #include "hw/hw.h"
>>  #include "hw/pci/pci.h"
>>  #include "net/net.h"
>> @@ -60,8 +61,6 @@
>>   * changed to pad short packets itself. */
>>  #define CONFIG_PAD_RECEIVED_FRAMES
>>  
>> -#define KiB 1024
>> -
>>  /* Debug EEPRO100 card. */
>>  #if 0
>>  # define DEBUG_EEPRO100
>> @@ -104,9 +103,9 @@
>>  /* Use 64 word EEPROM. TODO: could be a runtime option. */
>>  #define EEPROM_SIZE     64
>>  
>> -#define PCI_MEM_SIZE            (4 * KiB)
>> +#define PCI_MEM_SIZE            (4 * K_BYTE)
>>  #define PCI_IO_SIZE             64
>> -#define PCI_FLASH_SIZE          (128 * KiB)
>> +#define PCI_FLASH_SIZE          (128 * K_BYTE)
>>  
>>  #define BITS(n, m) (((0xffffffffU << (31 - n)) >> (31 - n + m)) << m)
> 
> 
> Technically this is fine, therefore
> 
> Reviewed-by: Stefan Weil <sw@weilnetz.de>
> 
> Practically I'd prefer replacing all K_BYTE by KiB, because that is the
> standard name with a precise definition:
> https://en.wikipedia.org/wiki/Kibibyte.

"1 kibibyte is 1024 bytes. The unit symbol for the kibibyte is KiB."

I like your suggestion :) Mostly because this shortens the code and
keeps it readable (while respecting the SI).

If the other maintainers agree with this change, I'm OK to do the
cleanup. I couldn't find a rule requiring #defines be all capital letter
in CODING_STYLE, is this acceptable to have a lower case "i"?

Regards,

Phil.

Patch

diff --git a/include/hw/net/allwinner_emac.h b/include/hw/net/allwinner_emac.h
index 4cc8aab7ec..4b53b6485c 100644
--- a/include/hw/net/allwinner_emac.h
+++ b/include/hw/net/allwinner_emac.h
@@ -23,6 +23,7 @@ 
 #ifndef ALLWINNER_EMAC_H
 #define ALLWINNER_EMAC_H
 
+#include "qemu/units.h"
 #include "net/net.h"
 #include "qemu/fifo8.h"
 #include "hw/net/mii.h"
@@ -125,8 +126,8 @@ 
 #define EMAC_INT_RX         (1 << 8)
 
 /* Due to lack of specifications, size of fifos is chosen arbitrarily */
-#define TX_FIFO_SIZE        (4 * 1024)
-#define RX_FIFO_SIZE        (32 * 1024)
+#define TX_FIFO_SIZE        (4 * K_BYTE)
+#define RX_FIFO_SIZE        (32 * K_BYTE)
 
 #define NUM_TX_FIFOS        2
 #define RX_HDR_SIZE         8
diff --git a/hw/net/e1000e.c b/hw/net/e1000e.c
index 16a9417a85..101efe7c83 100644
--- a/hw/net/e1000e.c
+++ b/hw/net/e1000e.c
@@ -34,6 +34,7 @@ 
 */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
 #include "net/net.h"
 #include "net/tap.h"
 #include "qemu/range.h"
@@ -81,10 +82,10 @@  typedef struct E1000EState {
 #define E1000E_IO_IDX       2
 #define E1000E_MSIX_IDX     3
 
-#define E1000E_MMIO_SIZE    (128 * 1024)
-#define E1000E_FLASH_SIZE   (128 * 1024)
+#define E1000E_MMIO_SIZE    (128 * K_BYTE)
+#define E1000E_FLASH_SIZE   (128 * K_BYTE)
 #define E1000E_IO_SIZE      (32)
-#define E1000E_MSIX_SIZE    (16 * 1024)
+#define E1000E_MSIX_SIZE    (16 * K_BYTE)
 
 #define E1000E_MSIX_TABLE   (0x0000)
 #define E1000E_MSIX_PBA     (0x2000)
diff --git a/hw/net/e1000x_common.c b/hw/net/e1000x_common.c
index eb0e097137..58c8db77e9 100644
--- a/hw/net/e1000x_common.c
+++ b/hw/net/e1000x_common.c
@@ -23,6 +23,7 @@ 
 */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
 #include "hw/hw.h"
 #include "hw/pci/pci.h"
 #include "net/net.h"
@@ -111,7 +112,7 @@  bool e1000x_is_oversized(uint32_t *mac, size_t size)
     static const int maximum_ethernet_vlan_size = 1522;
     /* this is the size past which hardware will
        drop packets when setting LPE=1 */
-    static const int maximum_ethernet_lpe_size = 16384;
+    static const int maximum_ethernet_lpe_size = 16 * K_BYTE;
 
     if ((size > maximum_ethernet_lpe_size ||
         (size > maximum_ethernet_vlan_size
diff --git a/hw/net/eepro100.c b/hw/net/eepro100.c
index a07a63247e..a02e2b55e8 100644
--- a/hw/net/eepro100.c
+++ b/hw/net/eepro100.c
@@ -41,6 +41,7 @@ 
  */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
 #include "hw/hw.h"
 #include "hw/pci/pci.h"
 #include "net/net.h"
@@ -60,8 +61,6 @@ 
  * changed to pad short packets itself. */
 #define CONFIG_PAD_RECEIVED_FRAMES
 
-#define KiB 1024
-
 /* Debug EEPRO100 card. */
 #if 0
 # define DEBUG_EEPRO100
@@ -104,9 +103,9 @@ 
 /* Use 64 word EEPROM. TODO: could be a runtime option. */
 #define EEPROM_SIZE     64
 
-#define PCI_MEM_SIZE            (4 * KiB)
+#define PCI_MEM_SIZE            (4 * K_BYTE)
 #define PCI_IO_SIZE             64
-#define PCI_FLASH_SIZE          (128 * KiB)
+#define PCI_FLASH_SIZE          (128 * K_BYTE)
 
 #define BITS(n, m) (((0xffffffffU << (31 - n)) >> (31 - n + m)) << m)