Re: [PATCH] tg3: Convert u32 flag,flg2,flg3 uses to bitmap

Eric Dumazet
2011-04-21T08:31:36Z

Le mercredi 20 avril 2011 à 23:39 -0700, Joe Perches a écrit :
> Using a bitmap instead of separate u32 flags allows a consistent, simpler
> and more extensible mechanism to determine capabilities.
>
> Convert bitfields to indexes.
> Add tg3_flag, tg3_flag_clear and tg3_flag_set
> Convert the flag & bitmask tests.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> drivers/net/tg3.c | 1262 ++++++++++++++++++++++++++---------------------------
> drivers/net/tg3.h | 164 ++++----
> 2 files changed, 696 insertions(+), 730 deletions(-)

Use an enum ?

Why first value is 1 and not 0 ?

> +#define TG3_FLAG_TAGGED_STATUS 1
> +#define TG3_FLAG_TXD_MBOX_HWBUG 2
> +#define TG3_FLAG_USE_LINKCHG_REG 3
> +#define TG3_FLAG_ERROR_PROCESSED 4
> +#define TG3_FLAG_ENABLE_ASF 5
> +#define TG3_FLAG_ASPM_WORKAROUND 6
> +#define TG3_FLAG_POLL_SERDES 7
> +#define TG3_FLAG_MBOX_WRITE_REORDER 8
> +#define TG3_FLAG_PCIX_TARGET_HWBUG 9
> +#define TG3_FLAG_WOL_SPEED_100MB 10
> +#define TG3_FLAG_WOL_ENABLE 11
> +#define TG3_FLAG_EEPROM_WRITE_PROT 12
> +#define TG3_FLAG_NVRAM 13
> +#define TG3_FLAG_NVRAM_BUFFERED 14
> +#define TG3_FLAG_SUPPORT_MSI 15
> +#define TG3_FLAG_SUPPORT_MSIX 16
> +#define TG3_FLAG_PCIX_MODE 17
> +#define TG3_FLAG_PCI_HIGH_SPEED 18
> +#define TG3_FLAG_PCI_32BIT 19
> +#define TG3_FLAG_SRAM_USE_CONFIG 20
> +#define TG3_FLAG_TX_RECOVERY_PENDING 21
> +#define TG3_FLAG_WOL_CAP 22
> +#define TG3_FLAG_JUMBO_RING_ENABLE 23
> +#define TG3_FLAG_PAUSE_AUTONEG 24
> +#define TG3_FLAG_CPMU_PRESENT 25
> +#define TG3_FLAG_40BIT_DMA_BUG 26
> +#define TG3_FLAG_BROKEN_CHECKSUMS 27
> +#define TG3_FLAG_JUMBO_CAPABLE 28
> +#define TG3_FLAG_CHIP_RESETTING 29
> +#define TG3_FLAG_INIT_COMPLETE 30
> +#define TG3_FLAG_RESTART_TIMER 31
> +#define TG3_FLAG_TSO_BUG 32
> +#define TG3_FLAG_IS_5788 33
> +#define TG3_FLAG_MAX_RXPEND_64 34
> +#define TG3_FLAG_TSO_CAPABLE 35
> +#define TG3_FLAG_PCI_EXPRESS 36
> +#define TG3_FLAG_ASF_NEW_HANDSHAKE 37
> +#define TG3_FLAG_HW_AUTONEG 38
> +#define TG3_FLAG_IS_NIC 39
> +#define TG3_FLAG_FLASH 40
> +#define TG3_FLAG_HW_TSO_1 41
> +#define TG3_FLAG_5705_PLUS 42
> +#define TG3_FLAG_5750_PLUS 43
> +#define TG3_FLAG_HW_TSO_3 44
> +#define TG3_FLAG_USING_MSI 45
> +#define TG3_FLAG_USING_MSIX 46
> +#define TG3_FLAG_ICH_WORKAROUND 47
> +#define TG3_FLAG_5780_CLASS 48
> +#define TG3_FLAG_HW_TSO_2 49
> +#define TG3_FLAG_1SHOT_MSI 50
> +#define TG3_FLAG_NO_FWARE_REPORTED 51
> +#define TG3_FLAG_NO_NVRAM_ADDR_TRANS 52
> +#define TG3_FLAG_ENABLE_APE 53
> +#define TG3_FLAG_PROTECTED_NVRAM 54
> +#define TG3_FLAG_5701_DMA_BUG 55
> +#define TG3_FLAG_USE_PHYLIB 56
> +#define TG3_FLAG_MDIOBUS_INITED 57
> +#define TG3_FLAG_LRG_PROD_RING_CAP 58
> +#define TG3_FLAG_RGMII_INBAND_DISABLE 59
> +#define TG3_FLAG_RGMII_EXT_IBND_RX_EN 60
> +#define TG3_FLAG_RGMII_EXT_IBND_TX_EN 61
> +#define TG3_FLAG_CLKREQ_BUG 62
> +#define TG3_FLAG_5755_PLUS 63
> +#define TG3_FLAG_NO_NVRAM 64
> +#define TG3_FLAG_ENABLE_RSS 65
> +#define TG3_FLAG_ENABLE_TSS 66
> +#define TG3_FLAG_4G_DMA_BNDRY_BUG 67
> +#define TG3_FLAG_40BIT_DMA_LIMIT_BUG 68
> +#define TG3_FLAG_SHORT_DMA_BUG 69
> +#define TG3_FLAG_USE_JUMBO_BDFLAG 70
> +#define TG3_FLAG_L1PLLPD_EN 71
> +#define TG3_FLAG_57765_PLUS 72
> +#define TG3_FLAG_APE_HAS_NCSI 73
> +#define TG3_FLAG_5717_PLUS 74
> +#define TG3_FLAGS 74 /* Set to number of flags */
> +
> struct tg3 {
> /* begin "general, frequently-used members" cacheline section */
>
> @@ -2838,7 +2914,7 @@ struct tg3 {
> /* SMP locking strategy:
> *
> * lock: Held during reset, PHY access, timer, and when
> - * updating tg3_flags and tg3_flags2.
> + * updating tg3_flags.
> *
> * netif_tx_lock: Held during tg3_start_xmit. tg3_tx holds
> * netif_tx_lock when it needs to call
> @@ -2895,95 +2971,13 @@ struct tg3 {
> struct tg3_ethtool_stats estats;
> struct tg3_ethtool_stats estats_prev;
>
> + DECLARE_BITMAP(tg3_flags, TG3_FLAGS);
> +

Also you need to make TG3_FLAGS be (last_flag_value + 1) or you could
miss one long in bitmap. (Not now, but later when new flags are added
and reach a multiple of BITS_PER_BYTE * sizeof(long)