[PATCH 6/6] ibmveth: Remove use of bitfields

2007-08-17 Thread Brian King

Removes the use of bitfields from the ibmveth driver. This results
in slightly smaller object code.

Signed-off-by: Brian King [EMAIL PROTECTED]
---

 linux-2.6-bjking1/drivers/net/ibmveth.c |   90 
 linux-2.6-bjking1/drivers/net/ibmveth.h |   56 ---
 2 files changed, 68 insertions(+), 78 deletions(-)

diff -puN drivers/net/ibmveth.h~ibmveth_nobitfields drivers/net/ibmveth.h
--- linux-2.6/drivers/net/ibmveth.h~ibmveth_nobitfields 2007-08-09 
15:15:27.0 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.h 2007-08-09 15:15:27.0 
-0500
@@ -39,6 +39,12 @@
 #define IbmVethMcastRemoveFilter 0x2UL
 #define IbmVethMcastClearFilterTable 0x3UL
 
+#define IBMVETH_ILLAN_PADDED_PKT_CSUM  0x2000ULL
+#define IBMVETH_ILLAN_TRUNK_PRI_MASK   0x0F00ULL
+#define IBMVETH_ILLAN_IPV6_TCP_CSUM0x0004ULL
+#define IBMVETH_ILLAN_IPV4_TCP_CSUM0x0002ULL
+#define IBMVETH_ILLAN_ACTIVE_TRUNK 0x0001ULL
+
 /* hcall macros */
 #define h_register_logical_lan(ua, buflst, rxq, fltlst, mac) \
   plpar_hcall_norets(H_REGISTER_LOGICAL_LAN, ua, buflst, rxq, fltlst, mac)
@@ -150,13 +156,13 @@ struct ibmveth_adapter {
 };
 
 struct ibmveth_buf_desc_fields {
-u32 valid : 1;
-u32 toggle : 1;
-u32 reserved : 4;
-u32 no_csum : 1;
-u32 csum_good : 1;
-u32 length : 24;
-u32 address;
+   u32 flags_len;
+#define IBMVETH_BUF_VALID  0x8000
+#define IBMVETH_BUF_TOGGLE 0x4000
+#define IBMVETH_BUF_NO_CSUM0x0200
+#define IBMVETH_BUF_CSUM_GOOD  0x0100
+#define IBMVETH_BUF_LEN_MASK   0x00FF
+   u32 address;
 };
 
 union ibmveth_buf_desc {
@@ -164,33 +170,17 @@ union ibmveth_buf_desc {
 struct ibmveth_buf_desc_fields fields;
 };
 
-struct ibmveth_illan_attributes_fields {
-   u32 reserved;
-   u32 reserved2 : 18;
-   u32 csum_offload_padded_pkt_support : 1;
-   u32 reserved3 : 1;
-   u32 trunk_priority : 4;
-   u32 reserved4 : 5;
-   u32 tcp_csum_offload_ipv6 : 1;
-   u32 tcp_csum_offload_ipv4 : 1;
-   u32 active_trunk : 1;
-};
-
-union ibmveth_illan_attributes {
-   u64 desc;
-   struct ibmveth_illan_attributes_fields fields;
-};
-
 struct ibmveth_rx_q_entry {
-u16 toggle : 1;
-u16 valid : 1;
-u16 reserved : 4;
-u16 no_csum : 1;
-u16 csum_good : 1;
-u16 reserved2 : 8;
-u16 offset;
-u32 length;
-u64 correlator;
+   u32 flags_off;
+#define IBMVETH_RXQ_TOGGLE 0x8000
+#define IBMVETH_RXQ_TOGGLE_SHIFT   31
+#define IBMVETH_RXQ_VALID  0x4000
+#define IBMVETH_RXQ_NO_CSUM0x0200
+#define IBMVETH_RXQ_CSUM_GOOD  0x0100
+#define IBMVETH_RXQ_OFF_MASK   0x
+
+   u32 length;
+   u64 correlator;
 };
 
 #endif /* _IBMVETH_H */
diff -puN drivers/net/ibmveth.c~ibmveth_nobitfields drivers/net/ibmveth.c
--- linux-2.6/drivers/net/ibmveth.c~ibmveth_nobitfields 2007-08-09 
15:15:27.0 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-08-09 15:15:27.0 
-0500
@@ -131,19 +131,29 @@ struct ibmveth_stat ibmveth_stats[] = {
 };
 
 /* simple methods of getting data from the current rxq entry */
+static inline u32 ibmveth_rxq_flags(struct ibmveth_adapter *adapter)
+{
+   return adapter-rx_queue.queue_addr[adapter-rx_queue.index].flags_off;
+}
+
+static inline int ibmveth_rxq_toggle(struct ibmveth_adapter *adapter)
+{
+   return (ibmveth_rxq_flags(adapter)  IBMVETH_RXQ_TOGGLE)  
IBMVETH_RXQ_TOGGLE_SHIFT;
+}
+
 static inline int ibmveth_rxq_pending_buffer(struct ibmveth_adapter *adapter)
 {
-   return (adapter-rx_queue.queue_addr[adapter-rx_queue.index].toggle == 
adapter-rx_queue.toggle);
+   return (ibmveth_rxq_toggle(adapter) == adapter-rx_queue.toggle);
 }
 
 static inline int ibmveth_rxq_buffer_valid(struct ibmveth_adapter *adapter)
 {
-   return (adapter-rx_queue.queue_addr[adapter-rx_queue.index].valid);
+   return (ibmveth_rxq_flags(adapter)  IBMVETH_RXQ_VALID);
 }
 
 static inline int ibmveth_rxq_frame_offset(struct ibmveth_adapter *adapter)
 {
-   return (adapter-rx_queue.queue_addr[adapter-rx_queue.index].offset);
+   return (ibmveth_rxq_flags(adapter)  IBMVETH_RXQ_OFF_MASK);
 }
 
 static inline int ibmveth_rxq_frame_length(struct ibmveth_adapter *adapter)
@@ -153,7 +163,7 @@ static inline int ibmveth_rxq_frame_leng
 
 static inline int ibmveth_rxq_csum_good(struct ibmveth_adapter *adapter)
 {
-   return 
(adapter-rx_queue.queue_addr[adapter-rx_queue.index].csum_good);
+   return (ibmveth_rxq_flags(adapter)  IBMVETH_RXQ_CSUM_GOOD);
 }
 
 /* setup the initial settings for a buffer pool */
@@ -253,9 +263,7 @@ static void ibmveth_replenish_buffer_poo
correlator = ((u64)pool-index  32) | index;
*(u64*)skb-data = correlator;
 
-   desc.desc = 0;
-   desc.fields.valid = 1;

[PATCH 6/6] ibmveth: Remove use of bitfields

2007-08-10 Thread Brian King

Removes the use of bitfields from the ibmveth driver. This results
in slightly smaller object code.

Signed-off-by: Brian King [EMAIL PROTECTED]
---

 linux-2.6-bjking1/drivers/net/ibmveth.c |   90 
 linux-2.6-bjking1/drivers/net/ibmveth.h |   56 ---
 2 files changed, 68 insertions(+), 78 deletions(-)

diff -puN drivers/net/ibmveth.h~ibmveth_nobitfields drivers/net/ibmveth.h
--- linux-2.6/drivers/net/ibmveth.h~ibmveth_nobitfields 2007-08-09 
15:15:27.0 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.h 2007-08-09 15:15:27.0 
-0500
@@ -39,6 +39,12 @@
 #define IbmVethMcastRemoveFilter 0x2UL
 #define IbmVethMcastClearFilterTable 0x3UL
 
+#define IBMVETH_ILLAN_PADDED_PKT_CSUM  0x2000ULL
+#define IBMVETH_ILLAN_TRUNK_PRI_MASK   0x0F00ULL
+#define IBMVETH_ILLAN_IPV6_TCP_CSUM0x0004ULL
+#define IBMVETH_ILLAN_IPV4_TCP_CSUM0x0002ULL
+#define IBMVETH_ILLAN_ACTIVE_TRUNK 0x0001ULL
+
 /* hcall macros */
 #define h_register_logical_lan(ua, buflst, rxq, fltlst, mac) \
   plpar_hcall_norets(H_REGISTER_LOGICAL_LAN, ua, buflst, rxq, fltlst, mac)
@@ -150,13 +156,13 @@ struct ibmveth_adapter {
 };
 
 struct ibmveth_buf_desc_fields {
-u32 valid : 1;
-u32 toggle : 1;
-u32 reserved : 4;
-u32 no_csum : 1;
-u32 csum_good : 1;
-u32 length : 24;
-u32 address;
+   u32 flags_len;
+#define IBMVETH_BUF_VALID  0x8000
+#define IBMVETH_BUF_TOGGLE 0x4000
+#define IBMVETH_BUF_NO_CSUM0x0200
+#define IBMVETH_BUF_CSUM_GOOD  0x0100
+#define IBMVETH_BUF_LEN_MASK   0x00FF
+   u32 address;
 };
 
 union ibmveth_buf_desc {
@@ -164,33 +170,17 @@ union ibmveth_buf_desc {
 struct ibmveth_buf_desc_fields fields;
 };
 
-struct ibmveth_illan_attributes_fields {
-   u32 reserved;
-   u32 reserved2 : 18;
-   u32 csum_offload_padded_pkt_support : 1;
-   u32 reserved3 : 1;
-   u32 trunk_priority : 4;
-   u32 reserved4 : 5;
-   u32 tcp_csum_offload_ipv6 : 1;
-   u32 tcp_csum_offload_ipv4 : 1;
-   u32 active_trunk : 1;
-};
-
-union ibmveth_illan_attributes {
-   u64 desc;
-   struct ibmveth_illan_attributes_fields fields;
-};
-
 struct ibmveth_rx_q_entry {
-u16 toggle : 1;
-u16 valid : 1;
-u16 reserved : 4;
-u16 no_csum : 1;
-u16 csum_good : 1;
-u16 reserved2 : 8;
-u16 offset;
-u32 length;
-u64 correlator;
+   u32 flags_off;
+#define IBMVETH_RXQ_TOGGLE 0x8000
+#define IBMVETH_RXQ_TOGGLE_SHIFT   31
+#define IBMVETH_RXQ_VALID  0x4000
+#define IBMVETH_RXQ_NO_CSUM0x0200
+#define IBMVETH_RXQ_CSUM_GOOD  0x0100
+#define IBMVETH_RXQ_OFF_MASK   0x
+
+   u32 length;
+   u64 correlator;
 };
 
 #endif /* _IBMVETH_H */
diff -puN drivers/net/ibmveth.c~ibmveth_nobitfields drivers/net/ibmveth.c
--- linux-2.6/drivers/net/ibmveth.c~ibmveth_nobitfields 2007-08-09 
15:15:27.0 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-08-09 15:15:27.0 
-0500
@@ -131,19 +131,29 @@ struct ibmveth_stat ibmveth_stats[] = {
 };
 
 /* simple methods of getting data from the current rxq entry */
+static inline u32 ibmveth_rxq_flags(struct ibmveth_adapter *adapter)
+{
+   return adapter-rx_queue.queue_addr[adapter-rx_queue.index].flags_off;
+}
+
+static inline int ibmveth_rxq_toggle(struct ibmveth_adapter *adapter)
+{
+   return (ibmveth_rxq_flags(adapter)  IBMVETH_RXQ_TOGGLE)  
IBMVETH_RXQ_TOGGLE_SHIFT;
+}
+
 static inline int ibmveth_rxq_pending_buffer(struct ibmveth_adapter *adapter)
 {
-   return (adapter-rx_queue.queue_addr[adapter-rx_queue.index].toggle == 
adapter-rx_queue.toggle);
+   return (ibmveth_rxq_toggle(adapter) == adapter-rx_queue.toggle);
 }
 
 static inline int ibmveth_rxq_buffer_valid(struct ibmveth_adapter *adapter)
 {
-   return (adapter-rx_queue.queue_addr[adapter-rx_queue.index].valid);
+   return (ibmveth_rxq_flags(adapter)  IBMVETH_RXQ_VALID);
 }
 
 static inline int ibmveth_rxq_frame_offset(struct ibmveth_adapter *adapter)
 {
-   return (adapter-rx_queue.queue_addr[adapter-rx_queue.index].offset);
+   return (ibmveth_rxq_flags(adapter)  IBMVETH_RXQ_OFF_MASK);
 }
 
 static inline int ibmveth_rxq_frame_length(struct ibmveth_adapter *adapter)
@@ -153,7 +163,7 @@ static inline int ibmveth_rxq_frame_leng
 
 static inline int ibmveth_rxq_csum_good(struct ibmveth_adapter *adapter)
 {
-   return 
(adapter-rx_queue.queue_addr[adapter-rx_queue.index].csum_good);
+   return (ibmveth_rxq_flags(adapter)  IBMVETH_RXQ_CSUM_GOOD);
 }
 
 /* setup the initial settings for a buffer pool */
@@ -253,9 +263,7 @@ static void ibmveth_replenish_buffer_poo
correlator = ((u64)pool-index  32) | index;
*(u64*)skb-data = correlator;
 
-   desc.desc = 0;
-   desc.fields.valid = 1;

Re: [PATCH 6/6] ibmveth: Remove use of bitfields

2007-08-07 Thread Jeff Garzik

Brian King wrote:

Removes the use of bitfields from the ibmveth driver. This results
in slightly smaller object code.

Signed-off-by: Brian King [EMAIL PROTECTED]
---

 linux-2.6-bjking1/drivers/net/ibmveth.c |   90 
 linux-2.6-bjking1/drivers/net/ibmveth.h |   56 ---
 2 files changed, 68 insertions(+), 78 deletions(-)


strong ACK :)

Though I also encourage you to avoid #defines for named constants, in 
favor of


enum {
IBMVETH_BUF_VALID   = (1U  31),
IBMVETH_BUF_TOGGLE  = (1U  30),
IBMVETH_BUF_NO_CSUM = (1U  25),
IBMVETH_BUF_CSUM_GOOD   = (1U  24),
IBMVETH_BUF_LEN_MASK= 0x00FF,
};

This illustrates:

1) The 1  n notation is FAR easier to read and compare with data 
sheets.  You're just adding to the trouble by requiring the reviewer's 
brain to convert hex numbers to bits, even if most engineers can do this 
in their sleep.


2) The named constants are available to the C compiler, which is more 
friendly to debuggers.  It also supplies type information to the C compiler.


3) Similar to #2, wading through C pre-processor output is much easier 
when the symbols don't disappear.


These are recommendations, not requirements, but I've found these 
techniques superior to cpp in many other drivers.


Jeff


-
To unsubscribe from this list: send the line unsubscribe netdev in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 6/6] ibmveth: Remove use of bitfields

2007-08-06 Thread Brian King

Removes the use of bitfields from the ibmveth driver. This results
in slightly smaller object code.

Signed-off-by: Brian King [EMAIL PROTECTED]
---

 linux-2.6-bjking1/drivers/net/ibmveth.c |   90 
 linux-2.6-bjking1/drivers/net/ibmveth.h |   56 ---
 2 files changed, 68 insertions(+), 78 deletions(-)

diff -puN drivers/net/ibmveth.h~ibmveth_nobitfields drivers/net/ibmveth.h
--- linux-2.6/drivers/net/ibmveth.h~ibmveth_nobitfields 2007-08-06 
14:11:11.0 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.h 2007-08-06 14:11:11.0 
-0500
@@ -39,6 +39,12 @@
 #define IbmVethMcastRemoveFilter 0x2UL
 #define IbmVethMcastClearFilterTable 0x3UL
 
+#define IBMVETH_ILLAN_PADDED_PKT_CSUM  0x2000ULL
+#define IBMVETH_ILLAN_TRUNK_PRI_MASK   0x0F00ULL
+#define IBMVETH_ILLAN_IPV6_TCP_CSUM0x0004ULL
+#define IBMVETH_ILLAN_IPV4_TCP_CSUM0x0002ULL
+#define IBMVETH_ILLAN_ACTIVE_TRUNK 0x0001ULL
+
 /* hcall macros */
 #define h_register_logical_lan(ua, buflst, rxq, fltlst, mac) \
   plpar_hcall_norets(H_REGISTER_LOGICAL_LAN, ua, buflst, rxq, fltlst, mac)
@@ -150,13 +156,13 @@ struct ibmveth_adapter {
 };
 
 struct ibmveth_buf_desc_fields {
-u32 valid : 1;
-u32 toggle : 1;
-u32 reserved : 4;
-u32 no_csum : 1;
-u32 csum_good : 1;
-u32 length : 24;
-u32 address;
+   u32 flags_len;
+#define IBMVETH_BUF_VALID  0x8000
+#define IBMVETH_BUF_TOGGLE 0x4000
+#define IBMVETH_BUF_NO_CSUM0x0200
+#define IBMVETH_BUF_CSUM_GOOD  0x0100
+#define IBMVETH_BUF_LEN_MASK   0x00FF
+   u32 address;
 };
 
 union ibmveth_buf_desc {
@@ -164,33 +170,17 @@ union ibmveth_buf_desc {
 struct ibmveth_buf_desc_fields fields;
 };
 
-struct ibmveth_illan_attributes_fields {
-   u32 reserved;
-   u32 reserved2 : 18;
-   u32 csum_offload_padded_pkt_support : 1;
-   u32 reserved3 : 1;
-   u32 trunk_priority : 4;
-   u32 reserved4 : 5;
-   u32 tcp_csum_offload_ipv6 : 1;
-   u32 tcp_csum_offload_ipv4 : 1;
-   u32 active_trunk : 1;
-};
-
-union ibmveth_illan_attributes {
-   u64 desc;
-   struct ibmveth_illan_attributes_fields fields;
-};
-
 struct ibmveth_rx_q_entry {
-u16 toggle : 1;
-u16 valid : 1;
-u16 reserved : 4;
-u16 no_csum : 1;
-u16 csum_good : 1;
-u16 reserved2 : 8;
-u16 offset;
-u32 length;
-u64 correlator;
+   u32 flags_off;
+#define IBMVETH_RXQ_TOGGLE 0x8000
+#define IBMVETH_RXQ_TOGGLE_SHIFT   31
+#define IBMVETH_RXQ_VALID  0x4000
+#define IBMVETH_RXQ_NO_CSUM0x0200
+#define IBMVETH_RXQ_CSUM_GOOD  0x0100
+#define IBMVETH_RXQ_OFF_MASK   0x
+
+   u32 length;
+   u64 correlator;
 };
 
 #endif /* _IBMVETH_H */
diff -puN drivers/net/ibmveth.c~ibmveth_nobitfields drivers/net/ibmveth.c
--- linux-2.6/drivers/net/ibmveth.c~ibmveth_nobitfields 2007-08-06 
14:11:11.0 -0500
+++ linux-2.6-bjking1/drivers/net/ibmveth.c 2007-08-06 14:11:11.0 
-0500
@@ -131,19 +131,29 @@ struct ibmveth_stat ibmveth_stats[] = {
 };
 
 /* simple methods of getting data from the current rxq entry */
+static inline u32 ibmveth_rxq_flags(struct ibmveth_adapter *adapter)
+{
+   return adapter-rx_queue.queue_addr[adapter-rx_queue.index].flags_off;
+}
+
+static inline int ibmveth_rxq_toggle(struct ibmveth_adapter *adapter)
+{
+   return (ibmveth_rxq_flags(adapter)  IBMVETH_RXQ_TOGGLE)  
IBMVETH_RXQ_TOGGLE_SHIFT;
+}
+
 static inline int ibmveth_rxq_pending_buffer(struct ibmveth_adapter *adapter)
 {
-   return (adapter-rx_queue.queue_addr[adapter-rx_queue.index].toggle == 
adapter-rx_queue.toggle);
+   return (ibmveth_rxq_toggle(adapter) == adapter-rx_queue.toggle);
 }
 
 static inline int ibmveth_rxq_buffer_valid(struct ibmveth_adapter *adapter)
 {
-   return (adapter-rx_queue.queue_addr[adapter-rx_queue.index].valid);
+   return (ibmveth_rxq_flags(adapter)  IBMVETH_RXQ_VALID);
 }
 
 static inline int ibmveth_rxq_frame_offset(struct ibmveth_adapter *adapter)
 {
-   return (adapter-rx_queue.queue_addr[adapter-rx_queue.index].offset);
+   return (ibmveth_rxq_flags(adapter)  IBMVETH_RXQ_OFF_MASK);
 }
 
 static inline int ibmveth_rxq_frame_length(struct ibmveth_adapter *adapter)
@@ -153,7 +163,7 @@ static inline int ibmveth_rxq_frame_leng
 
 static inline int ibmveth_rxq_csum_good(struct ibmveth_adapter *adapter)
 {
-   return 
(adapter-rx_queue.queue_addr[adapter-rx_queue.index].csum_good);
+   return (ibmveth_rxq_flags(adapter)  IBMVETH_RXQ_CSUM_GOOD);
 }
 
 /* setup the initial settings for a buffer pool */
@@ -253,9 +263,7 @@ static void ibmveth_replenish_buffer_poo
correlator = ((u64)pool-index  32) | index;
*(u64*)skb-data = correlator;
 
-   desc.desc = 0;
-   desc.fields.valid = 1;