[net-next PATCH 1/5] be2net: remove type argument of be_cmd_mac_addr_query()

[net-next PATCH 1/5] be2net: remove type argument of be_cmd_mac_addr_query()

[Sathya Perla]

All invocations of this routine use the same type value.

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.c |    4 ++--
 drivers/net/ethernet/emulex/benet/be_cmds.h |    2 +-
 drivers/net/ethernet/emulex/benet/be_main.c |   18 ++++++------------
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 701b3e9..6fbfb20 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -717,7 +717,7 @@ int be_cmd_eq_create(struct be_adapter *adapter,
 
 /* Use MCC */
 int be_cmd_mac_addr_query(struct be_adapter *adapter, u8 *mac_addr,
-			u8 type, bool permanent, u32 if_handle, u32 pmac_id)
+			  bool permanent, u32 if_handle, u32 pmac_id)
 {
 	struct be_mcc_wrb *wrb;
 	struct be_cmd_req_mac_query *req;
@@ -734,7 +734,7 @@ int be_cmd_mac_addr_query(struct be_adapter *adapter, u8 *mac_addr,
 
 	be_wrb_cmd_hdr_prepare(&req->hdr, CMD_SUBSYSTEM_COMMON,
 		OPCODE_COMMON_NTWK_MAC_QUERY, sizeof(*req), wrb, NULL);
-	req->type = type;
+	req->type = MAC_ADDRESS_TYPE_NETWORK;
 	if (permanent) {
 		req->permanent = 1;
 	} else {
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.h b/drivers/net/ethernet/emulex/benet/be_cmds.h
index 250f19b..1f5b839 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.h
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.h
@@ -1687,7 +1687,7 @@ struct be_cmd_req_set_ext_fat_caps {
 extern int be_pci_fnum_get(struct be_adapter *adapter);
 extern int be_fw_wait_ready(struct be_adapter *adapter);
 extern int be_cmd_mac_addr_query(struct be_adapter *adapter, u8 *mac_addr,
-			u8 type, bool permanent, u32 if_handle, u32 pmac_id);
+				 bool permanent, u32 if_handle, u32 pmac_id);
 extern int be_cmd_pmac_add(struct be_adapter *adapter, u8 *mac_addr,
 			u32 if_id, u32 *pmac_id, u32 domain);
 extern int be_cmd_pmac_del(struct be_adapter *adapter, u32 if_id,
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 84379f4..fa17430 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -241,9 +241,8 @@ static int be_mac_addr_set(struct net_device *netdev, void *p)
 	if (!is_valid_ether_addr(addr->sa_data))
 		return -EADDRNOTAVAIL;
 
-	status = be_cmd_mac_addr_query(adapter, current_mac,
-				MAC_ADDRESS_TYPE_NETWORK, false,
-				adapter->if_handle, 0);
+	status = be_cmd_mac_addr_query(adapter, current_mac, false,
+				       adapter->if_handle, 0);
 	if (status)
 		goto err;
 
@@ -2693,21 +2692,16 @@ static int be_get_mac_addr(struct be_adapter *adapter, u8 *mac, u32 if_handle,
 		status = be_cmd_get_mac_from_list(adapter, mac,
 						  active_mac, pmac_id, 0);
 		if (*active_mac) {
-			status = be_cmd_mac_addr_query(adapter, mac,
-						       MAC_ADDRESS_TYPE_NETWORK,
-						       false, if_handle,
-						       *pmac_id);
+			status = be_cmd_mac_addr_query(adapter, mac, false,
+						       if_handle, *pmac_id);
 		}
 	} else if (be_physfn(adapter)) {
 		/* For BE3, for PF get permanent MAC */
-		status = be_cmd_mac_addr_query(adapter, mac,
-					       MAC_ADDRESS_TYPE_NETWORK, true,
-					       0, 0);
+		status = be_cmd_mac_addr_query(adapter, mac, true, 0, 0);
 		*active_mac = false;
 	} else {
 		/* For BE3, for VF get soft MAC assigned by PF*/
-		status = be_cmd_mac_addr_query(adapter, mac,
-					       MAC_ADDRESS_TYPE_NETWORK, false,
+		status = be_cmd_mac_addr_query(adapter, mac, false,
 					       if_handle, 0);
 		*active_mac = true;
 	}
-- 
1.7.4

[net-next PATCH 2/5] be2net: fix wrong handling of be_setup() failure in be_probe()

[Sathya Perla]

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_main.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index fa17430..b712091 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -3889,7 +3889,7 @@ static int __devinit be_probe(struct pci_dev *pdev,
 
 	status = be_setup(adapter);
 	if (status)
-		goto msix_disable;
+		goto stats_clean;
 
 	be_netdev_init(netdev);
 	status = register_netdev(netdev);
@@ -3910,8 +3910,6 @@ static int __devinit be_probe(struct pci_dev *pdev,
 
 unsetup:
 	be_clear(adapter);
-msix_disable:
-	be_msix_disable(adapter);
 stats_clean:
 	be_stats_cleanup(adapter);
 ctrl_clean:
-- 
1.7.4

[net-next PATCH 3/5] be2net: cleanup code related to be_link_status_query()

[Sathya Perla]

1) link_status_query() is always called to query the link-speed (speed
after applying qos). When there is no qos setting, link-speed is derived from
port-speed. Do all this inside this routine and hide this from the callers.

2) adpater->phy.forced_port_speed is not being set anywhere after being
initialized. Get rid of this variable.

3) Ignore async link_speed notifications till the initial value has been
fetched from FW.

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be.h         |    1 -
 drivers/net/ethernet/emulex/benet/be_cmds.c    |   46 ++++++++++++++-----
 drivers/net/ethernet/emulex/benet/be_cmds.h    |    4 +-
 drivers/net/ethernet/emulex/benet/be_ethtool.c |   57 +++++-------------------
 drivers/net/ethernet/emulex/benet/be_main.c    |    4 +-
 5 files changed, 48 insertions(+), 64 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index 5b622993..cf4c05b 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -337,7 +337,6 @@ struct phy_info {
 	u16 auto_speeds_supported;
 	u16 fixed_speeds_supported;
 	int link_speed;
-	int forced_port_speed;
 	u32 dac_cable_len;
 	u32 advertising;
 	u32 supported;
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 6fbfb20..46a19af 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -165,14 +165,13 @@ static void be_async_grp5_cos_priority_process(struct be_adapter *adapter,
 	}
 }
 
-/* Grp5 QOS Speed evt */
+/* Grp5 QOS Speed evt: qos_link_speed is in units of 10 Mbps */
 static void be_async_grp5_qos_speed_process(struct be_adapter *adapter,
 		struct be_async_event_grp5_qos_link_speed *evt)
 {
-	if (evt->physical_port == adapter->port_num) {
-		/* qos_link_speed is in units of 10 Mbps */
-		adapter->phy.link_speed = evt->qos_link_speed * 10;
-	}
+	if (adapter->phy.link_speed >= 0 &&
+	    evt->physical_port == adapter->port_num)
+		adapter->phy.link_speed = le16_to_cpu(evt->qos_link_speed) * 10;
 }
 
 /*Grp5 PVID evt*/
@@ -1326,9 +1325,28 @@ err:
 	return status;
 }
 
-/* Uses synchronous mcc */
-int be_cmd_link_status_query(struct be_adapter *adapter, u8 *mac_speed,
-			     u16 *link_speed, u8 *link_status, u32 dom)
+static int be_mac_to_link_speed(int mac_speed)
+{
+	switch (mac_speed) {
+	case PHY_LINK_SPEED_ZERO:
+		return 0;
+	case PHY_LINK_SPEED_10MBPS:
+		return 10;
+	case PHY_LINK_SPEED_100MBPS:
+		return 100;
+	case PHY_LINK_SPEED_1GBPS:
+		return 1000;
+	case PHY_LINK_SPEED_10GBPS:
+		return 10000;
+	}
+	return 0;
+}
+
+/* Uses synchronous mcc
+ * Returns link_speed in Mbps
+ */
+int be_cmd_link_status_query(struct be_adapter *adapter, u16 *link_speed,
+			     u8 *link_status, u32 dom)
 {
 	struct be_mcc_wrb *wrb;
 	struct be_cmd_req_link_status *req;
@@ -1357,11 +1375,13 @@ int be_cmd_link_status_query(struct be_adapter *adapter, u8 *mac_speed,
 	status = be_mcc_notify_wait(adapter);
 	if (!status) {
 		struct be_cmd_resp_link_status *resp = embedded_payload(wrb);
-		if (resp->mac_speed != PHY_LINK_SPEED_ZERO) {
-			if (link_speed)
-				*link_speed = le16_to_cpu(resp->link_speed);
-			if (mac_speed)
-				*mac_speed = resp->mac_speed;
+		if (link_speed) {
+			*link_speed = resp->link_speed ?
+				      le16_to_cpu(resp->link_speed) * 10 :
+				      be_mac_to_link_speed(resp->mac_speed);
+
+			if (!resp->logical_link_status)
+				*link_speed = 0;
 		}
 		if (link_status)
 			*link_status = resp->logical_link_status;
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.h b/drivers/net/ethernet/emulex/benet/be_cmds.h
index 1f5b839..0936e21 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.h
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.h
@@ -1714,8 +1714,8 @@ extern int be_cmd_q_destroy(struct be_adapter *adapter, struct be_queue_info *q,
 			int type);
 extern int be_cmd_rxq_destroy(struct be_adapter *adapter,
 			struct be_queue_info *q);
-extern int be_cmd_link_status_query(struct be_adapter *adapter, u8 *mac_speed,
-				    u16 *link_speed, u8 *link_status, u32 dom);
+extern int be_cmd_link_status_query(struct be_adapter *adapter, u16 *link_speed,
+				    u8 *link_status, u32 dom);
 extern int be_cmd_reset(struct be_adapter *adapter);
 extern int be_cmd_get_stats(struct be_adapter *adapter,
 			struct be_dma_mem *nonemb_cmd);
diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index c0e7006..8e6fb0b 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -512,28 +512,6 @@ static u32 convert_to_et_setting(u32 if_type, u32 if_speeds)
 	return val;
 }
 
-static int convert_to_et_speed(u32 be_speed)
-{
-	int et_speed = SPEED_10000;
-
-	switch (be_speed) {
-	case PHY_LINK_SPEED_10MBPS:
-		et_speed = SPEED_10;
-		break;
-	case PHY_LINK_SPEED_100MBPS:
-		et_speed = SPEED_100;
-		break;
-	case PHY_LINK_SPEED_1GBPS:
-		et_speed = SPEED_1000;
-		break;
-	case PHY_LINK_SPEED_10GBPS:
-		et_speed = SPEED_10000;
-		break;
-	}
-
-	return et_speed;
-}
-
 bool be_pause_supported(struct be_adapter *adapter)
 {
 	return (adapter->phy.interface_type == PHY_TYPE_SFP_PLUS_10GB ||
@@ -544,27 +522,16 @@ bool be_pause_supported(struct be_adapter *adapter)
 static int be_get_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)
 {
 	struct be_adapter *adapter = netdev_priv(netdev);
-	u8 port_speed = 0;
-	u16 link_speed = 0;
 	u8 link_status;
-	u32 et_speed = 0;
+	u16 link_speed = 0;
 	int status;
 
-	if (adapter->phy.link_speed < 0 || !(netdev->flags & IFF_UP)) {
-		if (adapter->phy.forced_port_speed < 0) {
-			status = be_cmd_link_status_query(adapter, &port_speed,
-						&link_speed, &link_status, 0);
-			if (!status)
-				be_link_status_update(adapter, link_status);
-			if (link_speed)
-				et_speed = link_speed * 10;
-			else if (link_status)
-				et_speed = convert_to_et_speed(port_speed);
-		} else {
-			et_speed = adapter->phy.forced_port_speed;
-		}
-
-		ethtool_cmd_speed_set(ecmd, et_speed);
+	if (adapter->phy.link_speed < 0) {
+		status = be_cmd_link_status_query(adapter, &link_speed,
+						  &link_status, 0);
+		if (!status)
+			be_link_status_update(adapter, link_status);
+		ethtool_cmd_speed_set(ecmd, link_speed);
 
 		status = be_cmd_get_phy_info(adapter);
 		if (status)
@@ -773,8 +740,8 @@ static void
 be_self_test(struct net_device *netdev, struct ethtool_test *test, u64 *data)
 {
 	struct be_adapter *adapter = netdev_priv(netdev);
-	u8 mac_speed = 0;
-	u16 qos_link_speed = 0;
+	int status;
+	u8 link_status = 0;
 
 	memset(data, 0, sizeof(u64) * ETHTOOL_TESTS_NUM);
 
@@ -798,11 +765,11 @@ be_self_test(struct net_device *netdev, struct ethtool_test *test, u64 *data)
 		test->flags |= ETH_TEST_FL_FAILED;
 	}
 
-	if (be_cmd_link_status_query(adapter, &mac_speed,
-				     &qos_link_speed, NULL, 0) != 0) {
+	status = be_cmd_link_status_query(adapter, NULL, &link_status, 0);
+	if (status) {
 		test->flags |= ETH_TEST_FL_FAILED;
 		data[4] = -1;
-	} else if (!mac_speed) {
+	} else if (!link_status) {
 		test->flags |= ETH_TEST_FL_FAILED;
 		data[4] = 1;
 	}
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index b712091..4855dd6 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -2440,8 +2440,7 @@ static int be_open(struct net_device *netdev)
 		be_eq_notify(adapter, eqo->q.id, true, false, 0);
 	}
 
-	status = be_cmd_link_status_query(adapter, NULL, NULL,
-					  &link_status, 0);
+	status = be_cmd_link_status_query(adapter, NULL, &link_status, 0);
 	if (!status)
 		be_link_status_update(adapter, link_status);
 
@@ -2670,7 +2669,6 @@ static void be_setup_init(struct be_adapter *adapter)
 	adapter->be3_native = false;
 	adapter->promiscuous = false;
 	adapter->eq_next_idx = 0;
-	adapter->phy.forced_port_speed = -1;
 }
 
 static int be_get_mac_addr(struct be_adapter *adapter, u8 *mac, u32 if_handle,
-- 
1.7.4

[net-next PATCH 4/5] be2net: get rid of AMAP_SET/GET macros in TX path

[Sathya Perla]

The AMAP macros are used in be2net for setting and parsing bits in HW
descriptors. The macros do this by calculating the mask & offset of each
field from the AMAP structure definition.
In the TX patch, replace the usage of these macros with code to explicitly
shift & mask each field. Doing this reduces instructions and improves
readability.

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_hw.h   |   62 ++++++++-------------------
 drivers/net/ethernet/emulex/benet/be_main.c |   58 ++++++++++---------------
 2 files changed, 41 insertions(+), 79 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_hw.h b/drivers/net/ethernet/emulex/benet/be_hw.h
index b755f70..32a84ab 100644
--- a/drivers/net/ethernet/emulex/benet/be_hw.h
+++ b/drivers/net/ethernet/emulex/benet/be_hw.h
@@ -273,56 +273,30 @@ struct be_eth_wrb {
 	u32 frag_len;		/* dword 3: bits 0 - 15 */
 } __packed;
 
-/* Pseudo amap definition for eth_hdr_wrb in which each bit of the
- * actual structure is defined as a byte : used to calculate
- * offset/shift/mask of each field */
-struct amap_eth_hdr_wrb {
-	u8 rsvd0[32];		/* dword 0 */
-	u8 rsvd1[32];		/* dword 1 */
-	u8 complete;		/* dword 2 */
-	u8 event;
-	u8 crc;
-	u8 forward;
-	u8 lso6;
-	u8 mgmt;
-	u8 ipcs;
-	u8 udpcs;
-	u8 tcpcs;
-	u8 lso;
-	u8 vlan;
-	u8 gso[2];
-	u8 num_wrb[5];
-	u8 lso_mss[14];
-	u8 len[16];		/* dword 3 */
-	u8 vlan_tag[16];
-} __packed;
+#define TX_HDR_WRB_COMPL		1		/* word 2 */
+#define TX_HDR_WRB_EVT			(1 << 1)	/* word 2 */
+#define TX_HDR_WRB_CRC			(1 << 2)	/* word 2 */
+#define TX_HDR_WRB_LSO6			(1 << 4)	/* word 2 */
+#define TX_HDR_WRB_IPCS			(1 << 6)	/* word 2 */
+#define TX_HDR_WRB_UDPCS		(1 << 7)	/* word 2 */
+#define TX_HDR_WRB_TCPCS		(1 << 8)	/* word 2 */
+#define TX_HDR_WRB_LSO			(1 << 9)	/* word 2 */
+#define TX_HDR_WRB_VLAN			(1 << 10)	/* word 2 */
+#define TX_HDR_WRB_NUM_SHIFT		13		/* word 2: bits 13:17 */
+#define TX_HDR_WRB_NUM_MASK		0x1F		/* word 2: bits 13:17 */
+#define TX_HDR_WRB_MSS_SHIFT		18		/* word 2: bits 18:31 */
+#define TX_HDR_WRB_MSS_MASK		0x3FFF		/* word 2: bits 18:31 */
+#define TX_HDR_WRB_LEN_MASK		0xFFFF		/* word 3: bits 0:15 */
+#define TX_HDR_WRB_VLAN_TCI_SHIFT	16		/* word 3: bits 16:31 */
+#define TX_HDR_WRB_VLAN_TCI_MASK	0xFFFF		/* word 3: bits 16:31 */
 
 struct be_eth_hdr_wrb {
 	u32 dw[4];
 };
 
 /* TX Compl Queue Descriptor */
-
-/* Pseudo amap definition for eth_tx_compl in which each bit of the
- * actual structure is defined as a byte: used to calculate
- * offset/shift/mask of each field */
-struct amap_eth_tx_compl {
-	u8 wrb_index[16];	/* dword 0 */
-	u8 ct[2]; 		/* dword 0 */
-	u8 port[2];		/* dword 0 */
-	u8 rsvd0[8];		/* dword 0 */
-	u8 status[4];		/* dword 0 */
-	u8 user_bytes[16];	/* dword 1 */
-	u8 nwh_bytes[8];	/* dword 1 */
-	u8 lso;			/* dword 1 */
-	u8 cast_enc[2];		/* dword 1 */
-	u8 rsvd1[5];		/* dword 1 */
-	u8 rsvd2[32];		/* dword 2 */
-	u8 pkts[16];		/* dword 3 */
-	u8 ringid[11];		/* dword 3 */
-	u8 hash_val[4];		/* dword 3 */
-	u8 valid;		/* dword 3 */
-} __packed;
+#define TX_COMPL_WRB_IDX_MASK		0xFFFF		/* word 0: bits 0:15 */
+#define TX_COMPL_VALID			(1 << 31)	/* word 3 */
 
 struct be_eth_tx_compl {
 	u32 dw[4];
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 4855dd6..c74906d 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -585,45 +585,34 @@ static int be_vlan_tag_chk(struct be_adapter *adapter, struct sk_buff *skb)
 static void wrb_fill_hdr(struct be_adapter *adapter, struct be_eth_hdr_wrb *hdr,
 		struct sk_buff *skb, u32 wrb_cnt, u32 len)
 {
-	u16 vlan_tag;
-
-	memset(hdr, 0, sizeof(*hdr));
-
-	AMAP_SET_BITS(struct amap_eth_hdr_wrb, crc, hdr, 1);
+	u32 dw2 = 0, dw3 = 0;
 
 	if (skb_is_gso(skb)) {
-		AMAP_SET_BITS(struct amap_eth_hdr_wrb, lso, hdr, 1);
-		AMAP_SET_BITS(struct amap_eth_hdr_wrb, lso_mss,
-			hdr, skb_shinfo(skb)->gso_size);
+		dw2 |= TX_HDR_WRB_LSO;
+		dw2 |= (skb_shinfo(skb)->gso_size & TX_HDR_WRB_MSS_MASK) <<
+			TX_HDR_WRB_MSS_SHIFT;
 		if (skb_is_gso_v6(skb) && !lancer_chip(adapter))
-			AMAP_SET_BITS(struct amap_eth_hdr_wrb, lso6, hdr, 1);
-		if (lancer_chip(adapter) && adapter->sli_family  ==
-							LANCER_A0_SLI_FAMILY) {
-			AMAP_SET_BITS(struct amap_eth_hdr_wrb, ipcs, hdr, 1);
-			if (is_tcp_pkt(skb))
-				AMAP_SET_BITS(struct amap_eth_hdr_wrb,
-								tcpcs, hdr, 1);
-			else if (is_udp_pkt(skb))
-				AMAP_SET_BITS(struct amap_eth_hdr_wrb,
-								udpcs, hdr, 1);
-		}
+			dw2 |= TX_HDR_WRB_LSO6;
 	} else if (skb->ip_summed == CHECKSUM_PARTIAL) {
 		if (is_tcp_pkt(skb))
-			AMAP_SET_BITS(struct amap_eth_hdr_wrb, tcpcs, hdr, 1);
+			dw2 |= TX_HDR_WRB_TCPCS;
 		else if (is_udp_pkt(skb))
-			AMAP_SET_BITS(struct amap_eth_hdr_wrb, udpcs, hdr, 1);
+			dw2 |= TX_HDR_WRB_UDPCS;
 	}
 
 	if (vlan_tx_tag_present(skb)) {
-		AMAP_SET_BITS(struct amap_eth_hdr_wrb, vlan, hdr, 1);
-		vlan_tag = be_get_tx_vlan_tag(adapter, skb);
-		AMAP_SET_BITS(struct amap_eth_hdr_wrb, vlan_tag, hdr, vlan_tag);
+		dw2 |= TX_HDR_WRB_VLAN;
+		dw3 = (be_get_tx_vlan_tag(adapter, skb) & 0xFFFF) <<
+				TX_HDR_WRB_VLAN_TCI_SHIFT;
 	}
+	dw2 |= TX_HDR_WRB_CRC | TX_HDR_WRB_EVT | TX_HDR_WRB_COMPL |
+		(wrb_cnt & TX_HDR_WRB_NUM_MASK) << TX_HDR_WRB_NUM_SHIFT;
+	dw3 |= len & TX_HDR_WRB_LEN_MASK;
 
-	AMAP_SET_BITS(struct amap_eth_hdr_wrb, event, hdr, 1);
-	AMAP_SET_BITS(struct amap_eth_hdr_wrb, complete, hdr, 1);
-	AMAP_SET_BITS(struct amap_eth_hdr_wrb, num_wrb, hdr, wrb_cnt);
-	AMAP_SET_BITS(struct amap_eth_hdr_wrb, len, hdr, len);
+	hdr->dw[2] = dw2;
+	hdr->dw[3] = dw3;
+	hdr->dw[0] = 0;
+	hdr->dw[1] = 0;
 }
 
 static void unmap_tx_frag(struct device *dev, struct be_eth_wrb *wrb,
@@ -1554,13 +1543,14 @@ static struct be_eth_tx_compl *be_tx_compl_get(struct be_queue_info *tx_cq)
 {
 	struct be_eth_tx_compl *txcp = queue_tail_node(tx_cq);
 
-	if (txcp->dw[offsetof(struct amap_eth_tx_compl, valid) / 32] == 0)
+	/* valid bit is bit 31 of dw[3] */
+	if (!txcp->dw[3])
 		return NULL;
 
 	rmb();
 	be_dws_le_to_cpu(txcp, sizeof(*txcp));
 
-	txcp->dw[offsetof(struct amap_eth_tx_compl, valid) / 32] = 0;
+	txcp->dw[3] = 0;
 
 	queue_tail_inc(tx_cq);
 	return txcp;
@@ -1686,9 +1676,7 @@ static void be_tx_compl_clean(struct be_adapter *adapter)
 		for_all_tx_queues(adapter, txo, i) {
 			txq = &txo->q;
 			while ((txcp = be_tx_compl_get(&txo->cq))) {
-				end_idx =
-					AMAP_GET_BITS(struct amap_eth_tx_compl,
-						      wrb_index, txcp);
+				end_idx = txcp->dw[0] & TX_COMPL_WRB_IDX_MASK;
 				num_wrbs += be_tx_compl_process(adapter, txo,
 								end_idx);
 				cmpl++;
@@ -2040,8 +2028,8 @@ static bool be_process_tx(struct be_adapter *adapter, struct be_tx_obj *txo,
 		if (!txcp)
 			break;
 		num_wrbs += be_tx_compl_process(adapter, txo,
-				AMAP_GET_BITS(struct amap_eth_tx_compl,
-					wrb_index, txcp));
+						txcp->dw[0] &
+						TX_COMPL_WRB_IDX_MASK);
 	}
 
 	if (work_done) {
-- 
1.7.4

Re: [net-next PATCH 4/5] be2net: get rid of AMAP_SET/GET macros in TX path

[David Miller]

From: Sathya Perla <sathya.perla@emulex.com>
Date: Thu, 27 Sep 2012 12:02:47 +0530

> The AMAP macros are used in be2net for setting and parsing bits in HW
> descriptors. The macros do this by calculating the mask & offset of each
> field from the AMAP structure definition.
> In the TX patch, replace the usage of these macros with code to explicitly
> shift & mask each field. Doing this reduces instructions and improves
> readability.
> 
> Signed-off-by: Sathya Perla <sathya.perla@emulex.com>

Now you have endianness bugs.  The previous code worked with 8-bit
struct members and as such was endian neutral.

Now you're working with words, so you thus have to take endianness
into consideration.

The readability aspect is also extremely questionable, here's why.
The old code accessed struct members with _NAMES_ which described what
the values are and what they do.

This new code puts values into opaque "word" array members.  That's
about as crappy as it comes wrt. readability.  What in the world
does word[0] do?  I can't tell from it's name.  Yet with the existing
"struct amap_eth_hdr_wrb" there is none of that kind of confusion.

So don't pretend this new code even looks better, it looks like opaque
garbage to me.

Just admit this is an optimization that broke things on the endianness
you did not test.

be2net patches have been of a very low quality lately.  So I suggest
you improve things or else your submissions will be processed with an
extremely low priority.

Thanks.

RE: [net-next PATCH 4/5] be2net: get rid of AMAP_SET/GET macros in TX path

[Perla, Sathya]

>-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net]
>
>Now you have endianness bugs.  The previous code worked with 8-bit
>struct members and as such was endian neutral.
>
>Now you're working with words, so you thus have to take endianness
>into consideration.

Dave,
endianness is handled even in this patch. The call to wrb_fill_hdr()
is followed by be_dws_cpu_to_le() to handle this.

The old code did not set the 8-bit members in the amap_eth_wrb_hdr struct.
The TX hdr wrb structure is actually 4 words (32-bit * 4) long. Each word
has some fields with different bit sizes. In the old code a *separate* psuedo
structure -- called the amap_eth_wrb_hdr -- was defined, wherein a field
of size x bits in the actual TX-hrd-wrb was defined as *x bytes* long. This was
done to be able to calculate the mask and shift values of each bit-field without having to define
them manually.

There has been feedback in the community earlier that this method of calculating
bit-field masks and shifts is unusual and to be consistent with other drivers' code we
manually define the mask and shift values for each field.

>
>The readability aspect is also extremely questionable, here's why.
>The old code accessed struct members with _NAMES_ which described what
>the values are and what they do.
>
>This new code puts values into opaque "word" array members.  That's
>about as crappy as it comes wrt. readability.  What in the world
>does word[0] do?  I can't tell from it's name.  Yet with the existing
>"struct amap_eth_hdr_wrb" there is none of that kind of confusion.

The word[2] of TX-hdr-wrb consists for various 1-bit fields.
I could call this word "flags". But, word[3] consists of unrelated fields like
wrb-num (5bits),  lso-mss-len(14 bits) etc . So, I just used dw[3].

I could rename the current definition of TX-hdr-wrb from
struct be_eth_hdr_wrb {
	u32 dw[4];
};

to

struct be_eth_hdr_wrb {
	u32 rsvd0;
	u32 rsvd0;
	/* compl, evt, crc & cksum bits */
	u32 flags;
	/* num-wrb, lso-mss, vlan-tci etc */
	u32 info;
};

Pls let me know if this addresses your concerns...

thanks,
-Sathya

Re: [net-next PATCH 4/5] be2net: get rid of AMAP_SET/GET macros in TX path

[David Miller]

From: "Perla, Sathya" <Sathya.Perla@Emulex.Com>
Date: Fri, 28 Sep 2012 05:47:25 +0000

> endianness is handled even in this patch. The call to wrb_fill_hdr()
> is followed by be_dws_cpu_to_le() to handle this.

That swap_dws() thing is the most inefficient thing I've ever seen.

Instead of being able to benefit from compile time optimizations
such as byte swaps of constants, you do everything hidden from the
compiler so nothing gets optimized.

The aspect you are changing is the least of your problems in this
area.

RE: [net-next PATCH 4/5] be2net: get rid of AMAP_SET/GET macros in TX path

[Perla, Sathya]

>-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net]
>
>> endianness is handled even in this patch. The call to wrb_fill_hdr()
>> is followed by be_dws_cpu_to_le() to handle this.
>
>That swap_dws() thing is the most inefficient thing I've ever seen.
>
>Instead of being able to benefit from compile time optimizations
>such as byte swaps of constants, you do everything hidden from the
>compiler so nothing gets optimized.

I'd like to clarify that swap_dws() was being used/needed even in the old code. 
The AMAP_SET_BITS() macros (that this patch removes) did nothing to take care of endian byte-swapping.
They are just calculating the mask/shift values of each bit-field and setting the value
in host-endian.

But, as you are convinced that this patch doesn't provide much value, I'll just drop it and 
re-send the patch set....

Re: [net-next PATCH 4/5] be2net: get rid of AMAP_SET/GET macros in TX path

[David Miller]

From: "Perla, Sathya" <Sathya.Perla@Emulex.Com>
Date: Fri, 28 Sep 2012 07:05:27 +0000

>>-----Original Message-----
>>From: David Miller [mailto:davem@davemloft.net]
>>
>>> endianness is handled even in this patch. The call to wrb_fill_hdr()
>>> is followed by be_dws_cpu_to_le() to handle this.
>>
>>That swap_dws() thing is the most inefficient thing I've ever seen.
>>
>>Instead of being able to benefit from compile time optimizations
>>such as byte swaps of constants, you do everything hidden from the
>>compiler so nothing gets optimized.
> 
> I'd like to clarify that swap_dws() was being used/needed even in
> the old code.

I fully understand this.

[net-next PATCH 5/5] be2net: fixup log messages

[Sathya Perla]

Added and modified a few log messages mostly in probe path.

Signed-off-by: Sathya Perla <sathya.perla@emulex.com>
---
 drivers/net/ethernet/emulex/benet/be_cmds.c |    3 ++
 drivers/net/ethernet/emulex/benet/be_main.c |   34 ++++++++++++++++++++++----
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 46a19af..af60bb2 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -2425,6 +2425,9 @@ int be_cmd_req_native_mode(struct be_adapter *adapter)
 		struct be_cmd_resp_set_func_cap *resp = embedded_payload(wrb);
 		adapter->be3_native = le32_to_cpu(resp->cap_flags) &
 					CAPABILITY_BE3_NATIVE_ERX_API;
+		if (!adapter->be3_native)
+			dev_warn(&adapter->pdev->dev,
+				 "adapter not in advanced mode\n");
 	}
 err:
 	mutex_unlock(&adapter->mbox_lock);
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index c74906d..b7f9bf7 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -1884,6 +1884,8 @@ static int be_tx_qs_create(struct be_adapter *adapter)
 			return status;
 	}
 
+	dev_info(&adapter->pdev->dev, "created %d TX queue(s)\n",
+		 adapter->num_tx_qs);
 	return 0;
 }
 
@@ -1934,10 +1936,9 @@ static int be_rx_cqs_create(struct be_adapter *adapter)
 			return rc;
 	}
 
-	if (adapter->num_rx_qs != MAX_RX_QS)
-		dev_info(&adapter->pdev->dev,
-			"Created only %d receive queues\n", adapter->num_rx_qs);
-
+	dev_info(&adapter->pdev->dev,
+		 "created %d RSS queue(s) and 1 default RX queue\n",
+		 adapter->num_rx_qs - 1);
 	return 0;
 }
 
@@ -2175,6 +2176,7 @@ static void be_msix_enable(struct be_adapter *adapter)
 {
 #define BE_MIN_MSIX_VECTORS		1
 	int i, status, num_vec, num_roce_vec = 0;
+	struct device *dev = &adapter->pdev->dev;
 
 	/* If RSS queues are not used, need a vec for default RX Q */
 	num_vec = min(be_num_rss_want(adapter), num_online_cpus());
@@ -2199,6 +2201,8 @@ static void be_msix_enable(struct be_adapter *adapter)
 				num_vec) == 0)
 			goto done;
 	}
+
+	dev_warn(dev, "MSIx enable failed\n");
 	return;
 done:
 	if (be_roce_supported(adapter)) {
@@ -2212,6 +2216,7 @@ done:
 		}
 	} else
 		adapter->num_msix_vec = num_vec;
+	dev_info(dev, "enabled %d MSI-x vector(s)\n", adapter->num_msix_vec);
 	return;
 }
 
@@ -3785,6 +3790,23 @@ static bool be_reset_required(struct be_adapter *adapter)
 	return be_find_vfs(adapter, ENABLED) > 0 ? false : true;
 }
 
+static char *mc_name(struct be_adapter *adapter)
+{
+	if (adapter->function_mode & FLEX10_MODE)
+		return "FLEX10";
+	else if (adapter->function_mode & VNIC_MODE)
+		return "vNIC";
+	else if (adapter->function_mode & UMC_ENABLED)
+		return "UMC";
+	else
+		return "";
+}
+
+static inline char *func_name(struct be_adapter *adapter)
+{
+	return be_physfn(adapter) ? "PF" : "VF";
+}
+
 static int __devinit be_probe(struct pci_dev *pdev,
 			const struct pci_device_id *pdev_id)
 {
@@ -3889,8 +3911,8 @@ static int __devinit be_probe(struct pci_dev *pdev,
 
 	be_cmd_query_port_name(adapter, &port_name);
 
-	dev_info(&pdev->dev, "%s: %s port %c\n", netdev->name, nic_name(pdev),
-		 port_name);
+	dev_info(&pdev->dev, "%s: %s %s port %c\n", nic_name(pdev),
+		 func_name(adapter), mc_name(adapter), port_name);
 
 	return 0;
 
-- 
1.7.4