[net-next PATCH 0/4] qlge: Add ethtool self-test.

[net-next PATCH 0/4] qlge: Add ethtool self-test.

[Ron Mercer]

Changes for QLGE.

1) Add ethtool selftest.
2) Reduce debug print output.
3) Generic cleanup.

[net-next PATCH 1/4] qlge: Add ethtool self-test.

[Ron Mercer]

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h         |    6 ++
 drivers/net/qlge/qlge_ethtool.c |  112 +++++++++++++++++++++++++++++++++++++++
 drivers/net/qlge/qlge_main.c    |   20 +++++++-
 3 files changed, 137 insertions(+), 1 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index 4b954e1..b9f65e0 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -1580,6 +1580,8 @@ enum {
 	QL_ALLMULTI = 6,
 	QL_PORT_CFG = 7,
 	QL_CAM_RT_SET = 8,
+	QL_SELFTEST = 9,
+	QL_LB_LINK_UP = 10,
 };
 
 /* link_status bit definitions */
@@ -1716,6 +1718,7 @@ struct ql_adapter {
 	struct completion ide_completion;
 	struct nic_operations *nic_ops;
 	u16 device_id;
+	atomic_t lb_count;
 };
 
 /*
@@ -1807,6 +1810,9 @@ int ql_mb_set_port_cfg(struct ql_adapter *qdev);
 int ql_wait_fifo_empty(struct ql_adapter *qdev);
 void ql_gen_reg_dump(struct ql_adapter *qdev,
 			struct ql_reg_dump *mpi_coredump);
+netdev_tx_t ql_lb_send(struct sk_buff *skb, struct net_device *ndev);
+void ql_check_lb_frame(struct ql_adapter *, struct sk_buff *);
+int ql_clean_lb_rx_ring(struct rx_ring *rx_ring, int budget);
 
 #if 1
 #define QL_ALL_DUMP
diff --git a/drivers/net/qlge/qlge_ethtool.c b/drivers/net/qlge/qlge_ethtool.c
index 62c4af0..058fa0a 100644
--- a/drivers/net/qlge/qlge_ethtool.c
+++ b/drivers/net/qlge/qlge_ethtool.c
@@ -36,6 +36,11 @@
 
 #include "qlge.h"
 
+static const char ql_gstrings_test[][ETH_GSTRING_LEN] = {
+	"Loopback test  (offline)"
+};
+#define QLGE_TEST_LEN (sizeof(ql_gstrings_test) / ETH_GSTRING_LEN)
+
 static int ql_update_ring_coalescing(struct ql_adapter *qdev)
 {
 	int i, status = 0;
@@ -251,6 +256,8 @@ static void ql_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
 static int ql_get_sset_count(struct net_device *dev, int sset)
 {
 	switch (sset) {
+	case ETH_SS_TEST:
+		return QLGE_TEST_LEN;
 	case ETH_SS_STATS:
 		return ARRAY_SIZE(ql_stats_str_arr);
 	default:
@@ -429,6 +436,110 @@ static int ql_phys_id(struct net_device *ndev, u32 data)
 	return 0;
 }
 
+static int ql_start_loopback(struct ql_adapter *qdev)
+{
+	if (netif_carrier_ok(qdev->ndev)) {
+		set_bit(QL_LB_LINK_UP, &qdev->flags);
+		netif_carrier_off(qdev->ndev);
+	} else
+		clear_bit(QL_LB_LINK_UP, &qdev->flags);
+	qdev->link_config |= CFG_LOOPBACK_PCS;
+	return ql_mb_set_port_cfg(qdev);
+}
+
+static void ql_stop_loopback(struct ql_adapter *qdev)
+{
+	qdev->link_config &= ~CFG_LOOPBACK_PCS;
+	ql_mb_set_port_cfg(qdev);
+	if (test_bit(QL_LB_LINK_UP, &qdev->flags)) {
+		netif_carrier_on(qdev->ndev);
+		clear_bit(QL_LB_LINK_UP, &qdev->flags);
+	}
+}
+
+static void ql_create_lb_frame(struct sk_buff *skb,
+					unsigned int frame_size)
+{
+	memset(skb->data, 0xFF, frame_size);
+	frame_size &= ~1;
+	memset(&skb->data[frame_size / 2], 0xAA, frame_size / 2 - 1);
+	memset(&skb->data[frame_size / 2 + 10], 0xBE, 1);
+	memset(&skb->data[frame_size / 2 + 12], 0xAF, 1);
+}
+
+void ql_check_lb_frame(struct ql_adapter *qdev,
+					struct sk_buff *skb)
+{
+	unsigned int frame_size = skb->len;
+
+	if ((*(skb->data + 3) == 0xFF) &&
+		(*(skb->data + frame_size / 2 + 10) == 0xBE) &&
+		(*(skb->data + frame_size / 2 + 12) == 0xAF)) {
+			atomic_dec(&qdev->lb_count);
+			return;
+	}
+}
+
+static int ql_run_loopback_test(struct ql_adapter *qdev)
+{
+	int i;
+	netdev_tx_t rc;
+	struct sk_buff *skb;
+	unsigned int size = SMALL_BUF_MAP_SIZE;
+
+	for (i = 0; i < 64; i++) {
+		skb = netdev_alloc_skb(qdev->ndev, size);
+		if (!skb)
+			return -ENOMEM;
+
+		skb->queue_mapping = 0;
+		skb_put(skb, size);
+		ql_create_lb_frame(skb, size);
+		rc = ql_lb_send(skb, qdev->ndev);
+		if (rc != NETDEV_TX_OK)
+			return -EPIPE;
+		atomic_inc(&qdev->lb_count);
+	}
+
+	ql_clean_lb_rx_ring(&qdev->rx_ring[0], 128);
+	return atomic_read(&qdev->lb_count) ? -EIO : 0;
+}
+
+static int ql_loopback_test(struct ql_adapter *qdev, u64 *data)
+{
+	*data = ql_start_loopback(qdev);
+	if (*data)
+		goto out;
+	*data = ql_run_loopback_test(qdev);
+out:
+	ql_stop_loopback(qdev);
+	return *data;
+}
+
+static void ql_self_test(struct net_device *ndev,
+				struct ethtool_test *eth_test, u64 *data)
+{
+	struct ql_adapter *qdev = netdev_priv(ndev);
+
+	if (netif_running(ndev)) {
+		set_bit(QL_SELFTEST, &qdev->flags);
+		if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
+			/* Offline tests */
+			if (ql_loopback_test(qdev, &data[0]))
+				eth_test->flags |= ETH_TEST_FL_FAILED;
+
+		} else {
+			/* Online tests */
+			data[0] = 0;
+		}
+		clear_bit(QL_SELFTEST, &qdev->flags);
+	} else {
+		QPRINTK(qdev, DRV, ERR,
+			"%s: is down, Loopback test will fail.\n", ndev->name);
+		eth_test->flags |= ETH_TEST_FL_FAILED;
+	}
+}
+
 static int ql_get_regs_len(struct net_device *ndev)
 {
 	return sizeof(struct ql_reg_dump);
@@ -575,6 +686,7 @@ const struct ethtool_ops qlge_ethtool_ops = {
 	.set_msglevel = ql_set_msglevel,
 	.get_link = ethtool_op_get_link,
 	.phys_id		 = ql_phys_id,
+	.self_test		 = ql_self_test,
 	.get_pauseparam		 = ql_get_pauseparam,
 	.set_pauseparam		 = ql_set_pauseparam,
 	.get_rx_csum = ql_get_rx_csum,
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index dd0ea02..9e4d3dc 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -1680,6 +1680,13 @@ static void ql_process_mac_rx_intr(struct ql_adapter *qdev,
 		return;
 	}
 
+	/* loopback self test for ethtool */
+	if (test_bit(QL_SELFTEST, &qdev->flags)) {
+		ql_check_lb_frame(qdev, skb);
+		dev_kfree_skb_any(skb);
+		return;
+	}
+
 	prefetch(skb->data);
 	skb->dev = ndev;
 	if (ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_M_MASK) {
@@ -2248,6 +2255,7 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
 	return NETDEV_TX_OK;
 }
 
+
 static void ql_free_shadow_space(struct ql_adapter *qdev)
 {
 	if (qdev->rx_ring_shadow_reg_area) {
@@ -4173,7 +4181,6 @@ err_out:
 	return err;
 }
 
-
 static const struct net_device_ops qlge_netdev_ops = {
 	.ndo_open		= qlge_open,
 	.ndo_stop		= qlge_close,
@@ -4242,10 +4249,21 @@ static int __devinit qlge_probe(struct pci_dev *pdev,
 	}
 	ql_link_off(qdev);
 	ql_display_dev_info(ndev);
+	atomic_set(&qdev->lb_count, 0);
 	cards_found++;
 	return 0;
 }
 
+netdev_tx_t ql_lb_send(struct sk_buff *skb, struct net_device *ndev)
+{
+	return qlge_send(skb, ndev);
+}
+
+int ql_clean_lb_rx_ring(struct rx_ring *rx_ring, int budget)
+{
+	return ql_clean_inbound_rx_ring(rx_ring, budget);
+}
+
 static void __devexit qlge_remove(struct pci_dev *pdev)
 {
 	struct net_device *ndev = pci_get_drvdata(pdev);
-- 
1.6.0.2

[net-next PATCH 2/4] qlge: Change naming on vlan API.

[Ron Mercer]

Change name on vlan_rx_add, kill, register to match other driver API.

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge_main.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 9e4d3dc..01f78a9 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -1985,7 +1985,7 @@ static int ql_napi_poll_msix(struct napi_struct *napi, int budget)
 	return work_done;
 }
 
-static void ql_vlan_rx_register(struct net_device *ndev, struct vlan_group *grp)
+static void qlge_vlan_rx_register(struct net_device *ndev, struct vlan_group *grp)
 {
 	struct ql_adapter *qdev = netdev_priv(ndev);
 
@@ -2001,7 +2001,7 @@ static void ql_vlan_rx_register(struct net_device *ndev, struct vlan_group *grp)
 	}
 }
 
-static void ql_vlan_rx_add_vid(struct net_device *ndev, u16 vid)
+static void qlge_vlan_rx_add_vid(struct net_device *ndev, u16 vid)
 {
 	struct ql_adapter *qdev = netdev_priv(ndev);
 	u32 enable_bit = MAC_ADDR_E;
@@ -2017,7 +2017,7 @@ static void ql_vlan_rx_add_vid(struct net_device *ndev, u16 vid)
 	ql_sem_unlock(qdev, SEM_MAC_ADDR_MASK);
 }
 
-static void ql_vlan_rx_kill_vid(struct net_device *ndev, u16 vid)
+static void qlge_vlan_rx_kill_vid(struct net_device *ndev, u16 vid)
 {
 	struct ql_adapter *qdev = netdev_priv(ndev);
 	u32 enable_bit = 0;
@@ -4191,9 +4191,9 @@ static const struct net_device_ops qlge_netdev_ops = {
 	.ndo_set_mac_address	= qlge_set_mac_address,
 	.ndo_validate_addr	= eth_validate_addr,
 	.ndo_tx_timeout		= qlge_tx_timeout,
-	.ndo_vlan_rx_register	= ql_vlan_rx_register,
-	.ndo_vlan_rx_add_vid	= ql_vlan_rx_add_vid,
-	.ndo_vlan_rx_kill_vid	= ql_vlan_rx_kill_vid,
+	.ndo_vlan_rx_register	= qlge_vlan_rx_register,
+	.ndo_vlan_rx_add_vid	= qlge_vlan_rx_add_vid,
+	.ndo_vlan_rx_kill_vid	= qlge_vlan_rx_kill_vid,
 };
 
 static int __devinit qlge_probe(struct pci_dev *pdev,
-- 
1.6.0.2

[net-next PATCH 3/4] qlge: Reduce debug print output.

[Ron Mercer]

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge.h      |   12 +++++
 drivers/net/qlge/qlge_main.c |  101 ++++++++++++++++++++++-------------------
 drivers/net/qlge/qlge_mpi.c  |   14 +++---
 3 files changed, 73 insertions(+), 54 deletions(-)

diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
index b9f65e0..502c3af 100644
--- a/drivers/net/qlge/qlge.h
+++ b/drivers/net/qlge/qlge.h
@@ -27,6 +27,18 @@
 		dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
 			   "%s: " fmt, __func__, ##args);  \
        } while (0)
+#if 0
+#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)	\
+	do {	\
+		if (!((qdev)->msg_enable & NETIF_MSG_##nlevel))	\
+			;					\
+		else						\
+			dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
+					"%s: " fmt, __func__, ##args);  \
+	} while (0)
+#else
+#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)
+#endif
 
 #define WQ_ADDR_ALIGN	0x3	/* 4 byte alignment */
 
diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 01f78a9..57bbb90 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -359,7 +359,7 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
 			    (addr[2] << 24) | (addr[3] << 16) | (addr[4] << 8) |
 			    (addr[5]);
 
-			QPRINTK(qdev, IFUP, DEBUG,
+			QPRINTK_DBG(qdev, IFUP, DEBUG,
 				"Adding %s address %pM"
 				" at index %d in the CAM.\n",
 				((type ==
@@ -414,7 +414,8 @@ static int ql_set_mac_addr_reg(struct ql_adapter *qdev, u8 *addr, u32 type,
 			 * addressing. It's either MAC_ADDR_E on or off.
 			 * That's bit-27 we're talking about.
 			 */
-			QPRINTK(qdev, IFUP, INFO, "%s VLAN ID %d %s the CAM.\n",
+			QPRINTK_DBG(qdev, IFUP, INFO,
+				"%s VLAN ID %d %s the CAM.\n",
 				(enable_bit ? "Adding" : "Removing"),
 				index, (enable_bit ? "to" : "from"));
 
@@ -522,9 +523,9 @@ static int ql_set_routing_reg(struct ql_adapter *qdev, u32 index, u32 mask,
 	int status = -EINVAL; /* Return error if no mask match. */
 	u32 value = 0;
 
-	QPRINTK(qdev, IFUP, DEBUG,
-		"%s %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s mask %s the routing reg.\n",
-		(enable ? "Adding" : "Removing"),
+	QPRINTK_DBG(qdev, IFUP, DEBUG,
+		"%s %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s mask %s the routing "
+		"reg.\n", (enable ? "Adding" : "Removing"),
 		((index == RT_IDX_ALL_ERR_SLOT) ? "MAC ERROR/ALL ERROR" : ""),
 		((index == RT_IDX_IP_CSUM_ERR_SLOT) ? "IP CSUM ERROR" : ""),
 		((index ==
@@ -959,8 +960,9 @@ static int ql_8012_port_initialize(struct ql_adapter *qdev)
 		/* Another function has the semaphore, so
 		 * wait for the port init bit to come ready.
 		 */
-		QPRINTK(qdev, LINK, INFO,
-			"Another function has the semaphore, so wait for the port init bit to come ready.\n");
+		QPRINTK_DBG(qdev, LINK, INFO,
+			"Another function has the semaphore, so wait for the "
+			"port init bit to come ready.\n");
 		status = ql_wait_reg_rdy(qdev, STS, qdev->port_init, 0);
 		if (status) {
 			QPRINTK(qdev, LINK, CRIT,
@@ -969,7 +971,7 @@ static int ql_8012_port_initialize(struct ql_adapter *qdev)
 		return status;
 	}
 
-	QPRINTK(qdev, LINK, INFO, "Got xgmac semaphore!.\n");
+	QPRINTK_DBG(qdev, LINK, INFO, "Got xgmac semaphore!.\n");
 	/* Set the core reset. */
 	status = ql_read_xgmac_reg(qdev, GLOBAL_CFG, &data);
 	if (status)
@@ -1148,7 +1150,7 @@ static void ql_update_lbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
 
 	while (rx_ring->lbq_free_cnt > 32) {
 		for (i = 0; i < 16; i++) {
-			QPRINTK(qdev, RX_STATUS, DEBUG,
+			QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
 				"lbq: try cleaning clean_idx = %d.\n",
 				clean_idx);
 			lbq_desc = &rx_ring->lbq[clean_idx];
@@ -1181,7 +1183,7 @@ static void ql_update_lbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
 	}
 
 	if (start_idx != clean_idx) {
-		QPRINTK(qdev, RX_STATUS, DEBUG,
+		QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
 			"lbq: updating prod idx = %d.\n",
 			rx_ring->lbq_prod_idx);
 		ql_write_db_reg(rx_ring->lbq_prod_idx,
@@ -1201,7 +1203,7 @@ static void ql_update_sbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
 	while (rx_ring->sbq_free_cnt > 16) {
 		for (i = 0; i < 16; i++) {
 			sbq_desc = &rx_ring->sbq[clean_idx];
-			QPRINTK(qdev, RX_STATUS, DEBUG,
+			QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
 				"sbq: try cleaning clean_idx = %d.\n",
 				clean_idx);
 			if (sbq_desc->p.skb == NULL) {
@@ -1247,7 +1249,7 @@ static void ql_update_sbq(struct ql_adapter *qdev, struct rx_ring *rx_ring)
 	}
 
 	if (start_idx != clean_idx) {
-		QPRINTK(qdev, RX_STATUS, DEBUG,
+		QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
 			"sbq: updating prod idx = %d.\n",
 			rx_ring->sbq_prod_idx);
 		ql_write_db_reg(rx_ring->sbq_prod_idx,
@@ -1281,7 +1283,7 @@ static void ql_unmap_send(struct ql_adapter *qdev,
 			 * then its an OAL.
 			 */
 			if (i == 7) {
-				QPRINTK(qdev, TX_DONE, DEBUG,
+				QPRINTK_DBG(qdev, TX_DONE, DEBUG,
 					"unmapping OAL area.\n");
 			}
 			pci_unmap_single(qdev->pdev,
@@ -1291,8 +1293,8 @@ static void ql_unmap_send(struct ql_adapter *qdev,
 						       maplen),
 					 PCI_DMA_TODEVICE);
 		} else {
-			QPRINTK(qdev, TX_DONE, DEBUG, "unmapping frag %d.\n",
-				i);
+			QPRINTK_DBG(qdev, TX_DONE, DEBUG,
+					"unmapping frag %d.\n",	i);
 			pci_unmap_page(qdev->pdev,
 				       pci_unmap_addr(&tx_ring_desc->map[i],
 						      mapaddr),
@@ -1316,9 +1318,9 @@ static int ql_map_send(struct ql_adapter *qdev,
 	struct tx_buf_desc *tbd = mac_iocb_ptr->tbd;
 	int frag_cnt = skb_shinfo(skb)->nr_frags;
 
-	if (frag_cnt) {
-		QPRINTK(qdev, TX_QUEUED, DEBUG, "frag_cnt = %d.\n", frag_cnt);
-	}
+	if (frag_cnt)
+		QPRINTK_DBG(qdev, TX_QUEUED, DEBUG, "frag_cnt = %d.\n",
+				frag_cnt);
 	/*
 	 * Map the skb buffer first.
 	 */
@@ -1857,7 +1859,7 @@ static int ql_clean_outbound_rx_ring(struct rx_ring *rx_ring)
 	/* While there are entries in the completion queue. */
 	while (prod != rx_ring->cnsmr_idx) {
 
-		QPRINTK(qdev, RX_STATUS, DEBUG,
+		QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
 			"cq_id = %d, prod = %d, cnsmr = %d.\n.", rx_ring->cq_id,
 			prod, rx_ring->cnsmr_idx);
 
@@ -1871,7 +1873,8 @@ static int ql_clean_outbound_rx_ring(struct rx_ring *rx_ring)
 			break;
 		default:
 			QPRINTK(qdev, RX_STATUS, DEBUG,
-				"Hit default case, not handled! dropping the packet, opcode = %x.\n",
+				"Hit default case, not handled!	"
+				"dropping the packet, opcode = %x.\n",
 				net_rsp->opcode);
 		}
 		count++;
@@ -1904,7 +1907,7 @@ static int ql_clean_inbound_rx_ring(struct rx_ring *rx_ring, int budget)
 	/* While there are entries in the completion queue. */
 	while (prod != rx_ring->cnsmr_idx) {
 
-		QPRINTK(qdev, RX_STATUS, DEBUG,
+		QPRINTK_DBG(qdev, RX_STATUS, DEBUG,
 			"cq_id = %d, prod = %d, cnsmr = %d.\n.", rx_ring->cq_id,
 			prod, rx_ring->cnsmr_idx);
 
@@ -1922,11 +1925,10 @@ static int ql_clean_inbound_rx_ring(struct rx_ring *rx_ring, int budget)
 						net_rsp);
 			break;
 		default:
-			{
-				QPRINTK(qdev, RX_STATUS, DEBUG,
-					"Hit default case, not handled! dropping the packet, opcode = %x.\n",
+			QPRINTK(qdev, RX_STATUS, DEBUG,
+				"Hit default case, not handled!	"
+				"dropping the packet, opcode = %x.\n",
 					net_rsp->opcode);
-			}
 		}
 		count++;
 		ql_update_cq(rx_ring);
@@ -1947,7 +1949,7 @@ static int ql_napi_poll_msix(struct napi_struct *napi, int budget)
 	int i, work_done = 0;
 	struct intr_context *ctx = &qdev->intr_context[rx_ring->cq_id];
 
-	QPRINTK(qdev, RX_STATUS, DEBUG, "Enter, NAPI POLL cq_id = %d.\n",
+	QPRINTK_DBG(qdev, RX_STATUS, DEBUG, "Enter, NAPI POLL cq_id = %d.\n",
 		rx_ring->cq_id);
 
 	/* Service the TX rings first.  They start
@@ -1960,7 +1962,7 @@ static int ql_napi_poll_msix(struct napi_struct *napi, int budget)
 		if ((ctx->irq_mask & (1 << trx_ring->cq_id)) &&
 			(ql_read_sh_reg(trx_ring->prod_idx_sh_reg) !=
 					trx_ring->cnsmr_idx)) {
-			QPRINTK(qdev, INTR, DEBUG,
+			QPRINTK_DBG(qdev, INTR, DEBUG,
 				"%s: Servicing TX completion ring %d.\n",
 				__func__, trx_ring->cq_id);
 			ql_clean_outbound_rx_ring(trx_ring);
@@ -1972,7 +1974,7 @@ static int ql_napi_poll_msix(struct napi_struct *napi, int budget)
 	 */
 	if (ql_read_sh_reg(rx_ring->prod_idx_sh_reg) !=
 					rx_ring->cnsmr_idx) {
-		QPRINTK(qdev, INTR, DEBUG,
+		QPRINTK_DBG(qdev, INTR, DEBUG,
 			"%s: Servicing RX completion ring %d.\n",
 			__func__, rx_ring->cq_id);
 		work_done = ql_clean_inbound_rx_ring(rx_ring, budget);
@@ -1991,11 +1993,12 @@ static void qlge_vlan_rx_register(struct net_device *ndev, struct vlan_group *gr
 
 	qdev->vlgrp = grp;
 	if (grp) {
-		QPRINTK(qdev, IFUP, DEBUG, "Turning on VLAN in NIC_RCV_CFG.\n");
+		QPRINTK_DBG(qdev, IFUP, DEBUG,
+					"Turning on VLAN in NIC_RCV_CFG.\n");
 		ql_write32(qdev, NIC_RCV_CFG, NIC_RCV_CFG_VLAN_MASK |
 			   NIC_RCV_CFG_VLAN_MATCH_AND_NON);
 	} else {
-		QPRINTK(qdev, IFUP, DEBUG,
+		QPRINTK_DBG(qdev, IFUP, DEBUG,
 			"Turning off VLAN in NIC_RCV_CFG.\n");
 		ql_write32(qdev, NIC_RCV_CFG, NIC_RCV_CFG_VLAN_MASK);
 	}
@@ -2058,7 +2061,7 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
 
 	spin_lock(&qdev->hw_lock);
 	if (atomic_read(&qdev->intr_context[0].irq_cnt)) {
-		QPRINTK(qdev, INTR, DEBUG, "Shared Interrupt, Not ours!\n");
+		QPRINTK_DBG(qdev, INTR, DEBUG, "Shared Interrupt, Not ours!\n");
 		spin_unlock(&qdev->hw_lock);
 		return IRQ_NONE;
 	}
@@ -2102,8 +2105,7 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
 	 */
 	var = ql_read32(qdev, ISR1);
 	if (var & intr_context->irq_mask) {
-				QPRINTK(qdev, INTR, INFO,
-			"Waking handler for rx_ring[0].\n");
+		QPRINTK_DBG(qdev, INTR, INFO, "Waking handler for rx_ring[0].\n");
 		ql_disable_completion_interrupt(qdev, intr_context->intr);
 					napi_schedule(&rx_ring->napi);
 				work_done++;
@@ -2200,9 +2202,9 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
 		return NETDEV_TX_OK;
 
 	if (unlikely(atomic_read(&tx_ring->tx_count) < 2)) {
-		QPRINTK(qdev, TX_QUEUED, INFO,
-			"%s: shutting down tx queue %d du to lack of resources.\n",
-			__func__, tx_ring_idx);
+		QPRINTK_DBG(qdev, TX_QUEUED, INFO,
+			"shutting down tx queue %d du to lack of resources.\n",
+			tx_ring_idx);
 		netif_stop_subqueue(ndev, tx_ring->wq_id);
 		atomic_inc(&tx_ring->queue_stopped);
 		return NETDEV_TX_BUSY;
@@ -2222,7 +2224,7 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
 	mac_iocb_ptr->frame_len = cpu_to_le16((u16) skb->len);
 
 	if (qdev->vlgrp && vlan_tx_tag_present(skb)) {
-		QPRINTK(qdev, TX_QUEUED, DEBUG, "Adding a vlan tag %d.\n",
+		QPRINTK_DBG(qdev, TX_QUEUED, DEBUG, "Adding a vlan tag %d.\n",
 			vlan_tx_tag_get(skb));
 		mac_iocb_ptr->flags3 |= OB_MAC_IOCB_V;
 		mac_iocb_ptr->vlan_tci = cpu_to_le16(vlan_tx_tag_get(skb));
@@ -2248,7 +2250,7 @@ static netdev_tx_t qlge_send(struct sk_buff *skb, struct net_device *ndev)
 	wmb();
 
 	ql_write_db_reg(tx_ring->prod_idx, tx_ring->prod_idx_db_reg);
-	QPRINTK(qdev, TX_QUEUED, DEBUG, "tx queued, slot %d, len %d\n",
+	QPRINTK_DBG(qdev, TX_QUEUED, DEBUG, "tx queued, slot %d, len %d\n",
 		tx_ring->prod_idx, skb->len);
 
 	atomic_dec(&tx_ring->tx_count);
@@ -2783,10 +2785,10 @@ static int ql_start_rx_ring(struct ql_adapter *qdev, struct rx_ring *rx_ring)
 		cqicb->pkt_delay = cpu_to_le16(qdev->rx_max_coalesced_frames);
 		break;
 	default:
-		QPRINTK(qdev, IFUP, DEBUG, "Invalid rx_ring->type = %d.\n",
+		QPRINTK_DBG(qdev, IFUP, DEBUG, "Invalid rx_ring->type = %d.\n",
 			rx_ring->type);
 	}
-	QPRINTK(qdev, IFUP, DEBUG, "Initializing rx work queue.\n");
+	QPRINTK_DBG(qdev, IFUP, DEBUG, "Initializing rx work queue.\n");
 	err = ql_write_cfg(qdev, cqicb, sizeof(struct cqicb),
 			   CFG_LCQ, rx_ring->cq_id);
 	if (err) {
@@ -2839,7 +2841,7 @@ static int ql_start_tx_ring(struct ql_adapter *qdev, struct tx_ring *tx_ring)
 		QPRINTK(qdev, IFUP, ERR, "Failed to load tx_ring.\n");
 		return err;
 	}
-	QPRINTK(qdev, IFUP, DEBUG, "Successfully loaded WQICB.\n");
+	QPRINTK_DBG(qdev, IFUP, DEBUG, "Successfully loaded WQICB.\n");
 	return err;
 }
 
@@ -3088,11 +3090,11 @@ static void ql_free_irq(struct ql_adapter *qdev)
 			if (test_bit(QL_MSIX_ENABLED, &qdev->flags)) {
 				free_irq(qdev->msi_x_entry[i].vector,
 					 &qdev->rx_ring[i]);
-				QPRINTK(qdev, IFDOWN, DEBUG,
+				QPRINTK_DBG(qdev, IFDOWN, DEBUG,
 					"freeing msix interrupt %d.\n", i);
 			} else {
 				free_irq(qdev->pdev->irq, &qdev->rx_ring[0]);
-				QPRINTK(qdev, IFDOWN, DEBUG,
+				QPRINTK_DBG(qdev, IFDOWN, DEBUG,
 					"freeing msi interrupt %d.\n", i);
 			}
 		}
@@ -3200,14 +3202,14 @@ static int ql_start_rss(struct ql_adapter *qdev)
 	memcpy((void *)&ricb->ipv6_hash_key[0], init_hash_seed, 40);
 	memcpy((void *)&ricb->ipv4_hash_key[0], init_hash_seed, 16);
 
-	QPRINTK(qdev, IFUP, DEBUG, "Initializing RSS.\n");
+	QPRINTK_DBG(qdev, IFUP, DEBUG, "Initializing RSS.\n");
 
 	status = ql_write_cfg(qdev, ricb, sizeof(*ricb), CFG_LR, 0);
 	if (status) {
 		QPRINTK(qdev, IFUP, ERR, "Failed to load RICB.\n");
 		return status;
 	}
-	QPRINTK(qdev, IFUP, DEBUG, "Successfully loaded RICB.\n");
+	QPRINTK_DBG(qdev, IFUP, DEBUG, "Successfully loaded RICB.\n");
 	return status;
 }
 
@@ -3679,7 +3681,7 @@ static int ql_configure_rings(struct ql_adapter *qdev)
 			rx_ring->lbq_size =
 			    rx_ring->lbq_len * sizeof(__le64);
 			rx_ring->lbq_buf_size = (u16)lbq_buf_len;
-			QPRINTK(qdev, IFUP, DEBUG,
+			QPRINTK_DBG(qdev, IFUP, DEBUG,
 				"lbq_buf_size %d, order = %d\n",
 				rx_ring->lbq_buf_size, qdev->lbq_buf_order);
 			rx_ring->sbq_len = NUM_SMALL_BUFFERS;
@@ -3917,8 +3919,13 @@ static int qlge_set_mac_address(struct net_device *ndev, void *p)
 	if (netif_running(ndev))
 		return -EBUSY;
 
-	if (!is_valid_ether_addr(addr->sa_data))
+	if (!is_valid_ether_addr(addr->sa_data)) {
+		QPRINTK_DBG(qdev, DRV, ERR,
+			"Invalid Mac addr %02x:%02x:%02x:%02x:%02x:%02x\n",
+			addr->sa_data[0], addr->sa_data[1], addr->sa_data[2],
+			addr->sa_data[3], addr->sa_data[4], addr->sa_data[5]);
 		return -EADDRNOTAVAIL;
+	}
 	memcpy(ndev->dev_addr, addr->sa_data, ndev->addr_len);
 
 	status = ql_sem_spinlock(qdev, SEM_MAC_ADDR_MASK);
diff --git a/drivers/net/qlge/qlge_mpi.c b/drivers/net/qlge/qlge_mpi.c
index 80b6853..0a26055 100644
--- a/drivers/net/qlge/qlge_mpi.c
+++ b/drivers/net/qlge/qlge_mpi.c
@@ -207,7 +207,7 @@ static void ql_link_up(struct ql_adapter *qdev, struct mbox_params *mbcp)
 	 * to our liking.
 	 */
 	if (!test_bit(QL_PORT_CFG, &qdev->flags)) {
-		QPRINTK(qdev, DRV, ERR, "Queue Port Config Worker!\n");
+		QPRINTK_DBG(qdev, DRV, DEBUG, "Queue Port Config Worker!\n");
 		set_bit(QL_PORT_CFG, &qdev->flags);
 		/* Begin polled mode early so
 		 * we don't get another interrupt
@@ -277,7 +277,7 @@ static int ql_aen_lost(struct ql_adapter *qdev, struct mbox_params *mbcp)
 		int i;
 		QPRINTK(qdev, DRV, ERR, "Lost AEN detected.\n");
 		for (i = 0; i < mbcp->out_count; i++)
-			QPRINTK(qdev, DRV, ERR, "mbox_out[%d] = 0x%.08x.\n",
+			QPRINTK_DBG(qdev, DRV, ERR, "mbox_out[%d] = 0x%.08x.\n",
 					i, mbcp->mbox_out[i]);
 
 	}
@@ -658,7 +658,7 @@ int ql_mb_set_port_cfg(struct ql_adapter *qdev)
 		return status;
 
 	if (mbcp->mbox_out[0] == MB_CMD_STS_INTRMDT) {
-		QPRINTK(qdev, DRV, ERR,
+		QPRINTK_DBG(qdev, DRV, ERR,
 			"Port Config sent, wait for IDC.\n");
 	} else	if (mbcp->mbox_out[0] != MB_CMD_STS_GOOD) {
 		QPRINTK(qdev, DRV, ERR,
@@ -694,7 +694,7 @@ int ql_mb_get_port_cfg(struct ql_adapter *qdev)
 			"Failed Get Port Configuration.\n");
 		status = -EIO;
 	} else	{
-		QPRINTK(qdev, DRV, DEBUG,
+		QPRINTK_DBG(qdev, DRV, DEBUG,
 			"Passed Get Port Configuration.\n");
 		qdev->link_config = mbcp->mbox_out[1];
 		qdev->max_frame_size = mbcp->mbox_out[2];
@@ -898,7 +898,7 @@ int ql_mb_set_mgmnt_traffic_ctl(struct ql_adapter *qdev, u32 control)
 		return status;
 
 	if (mbcp->mbox_out[0] == MB_CMD_STS_INVLD_CMD) {
-		QPRINTK(qdev, DRV, ERR,
+		QPRINTK_DBG(qdev, DRV, ERR,
 			"Command not supported by firmware.\n");
 		status = -EINVAL;
 	} else if (mbcp->mbox_out[0] == MB_CMD_STS_ERR) {
@@ -906,7 +906,7 @@ int ql_mb_set_mgmnt_traffic_ctl(struct ql_adapter *qdev, u32 control)
 		 * already in the state we are trying to
 		 * change it to.
 		 */
-		QPRINTK(qdev, DRV, ERR,
+		QPRINTK_DBG(qdev, DRV, ERR,
 			"Command parameters make no change.\n");
 	}
 	return status;
@@ -937,7 +937,7 @@ static int ql_mb_get_mgmnt_traffic_ctl(struct ql_adapter *qdev, u32 *control)
 	}
 
 	if (mbcp->mbox_out[0] == MB_CMD_STS_INVLD_CMD) {
-		QPRINTK(qdev, DRV, ERR,
+		QPRINTK_DBG(qdev, DRV, ERR,
 			"Command not supported by firmware.\n");
 		status = -EINVAL;
 	} else if (mbcp->mbox_out[0] == MB_CMD_STS_ERR) {
-- 
1.6.0.2

Re: [net-next PATCH 3/4] qlge: Reduce debug print output.

[Joe Perches]

On Fri, 2009-10-30 at 15:13 -0700, Ron Mercer wrote:
> Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
[]
> diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
> index b9f65e0..502c3af 100644
> --- a/drivers/net/qlge/qlge.h
> +++ b/drivers/net/qlge/qlge.h
> @@ -27,6 +27,18 @@
>  		dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
>  			   "%s: " fmt, __func__, ##args);  \
>         } while (0)
> +#if 0
> +#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)	\
> +	do {	\
> +		if (!((qdev)->msg_enable & NETIF_MSG_##nlevel))	\
> +			;					\
> +		else						\
> +			dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
> +					"%s: " fmt, __func__, ##args);  \
> +	} while (0)
> +#else
> +#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)
> +#endif

This uses an inverted test and it doesn't verify the args to
dev_printk when not #defined.

How about:

#ifdef DEBUG
#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)	do {	\
	if ((qdev)->msg_enable & NETIF_MSG_##nlevel)		\
		dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
			   "%s: " fmt, __func__, ##args);	\
} while (0)
#else
#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)	do {	\
	if (0 && (qdev)->msg_enable & NETIF_MSG_##nlevel)	\
		dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
			   "%s: " fmt, __func__, ##args);	\
} while (0)
#endif

Re: [net-next PATCH 3/4] qlge: Reduce debug print output.

[David Miller]

From: Joe Perches <joe@perches.com>
Date: Fri, 30 Oct 2009 15:44:46 -0700

> On Fri, 2009-10-30 at 15:13 -0700, Ron Mercer wrote:
>> Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
> []
>> diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
>> index b9f65e0..502c3af 100644
>> --- a/drivers/net/qlge/qlge.h
>> +++ b/drivers/net/qlge/qlge.h
>> @@ -27,6 +27,18 @@
>>  		dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
>>  			   "%s: " fmt, __func__, ##args);  \
>>         } while (0)
>> +#if 0
>> +#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)	\
>> +	do {	\
>> +		if (!((qdev)->msg_enable & NETIF_MSG_##nlevel))	\
>> +			;					\
>> +		else						\
>> +			dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
>> +					"%s: " fmt, __func__, ##args);  \
>> +	} while (0)
>> +#else
>> +#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)
>> +#endif
> 
> This uses an inverted test and it doesn't verify the args to
> dev_printk when not #defined.
> 
> How about:

I also don't like this kind of change for another reason.

The message levels are pointless if you're going to adhere to them
or not based upon some CPP define.

Either do it, or don't.  Like every other driver does.

If for some reason the default is problematic, adjust the default that
you pass to netif_msg_init() or, alternatively, adjust what level the
debugging messages are assigned to.

We have all of this wonderful, full, infrastructure for message
levelling.  And you can change the setting either at module load time
or via ethtool.  The default is also up to you as well.

And you're going to stick a "#if 0" CPP control in there? :-/

No way, I absolutely won't accept this kind of change, there is no
need for it.  There is more than enough dynamic flexibility, both at
run-time via module option and ethtool message level selections, and
at compile time via the default you can choose hoever you like.

Re: [net-next PATCH 3/4] qlge: Reduce debug print output.

[Ron Mercer]

Dave and Joe,

Thanks for your feedback on the printk macro.  I agree there is good
infrastructure for debug printing for network devices.  I will drop this
patch for now.  The main reason I did it this way was to remove compares from the data path without removing the messages.
I will respin the last patch.

Thanks,
Ron

On Mon, Nov 02, 2009 at 12:03:54AM -0800, David Miller wrote:
> From: Joe Perches <joe@perches.com>
> Date: Fri, 30 Oct 2009 15:44:46 -0700
> 
> > On Fri, 2009-10-30 at 15:13 -0700, Ron Mercer wrote:
> >> Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
> > []
> >> diff --git a/drivers/net/qlge/qlge.h b/drivers/net/qlge/qlge.h
> >> index b9f65e0..502c3af 100644
> >> --- a/drivers/net/qlge/qlge.h
> >> +++ b/drivers/net/qlge/qlge.h
> >> @@ -27,6 +27,18 @@
> >>  		dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
> >>  			   "%s: " fmt, __func__, ##args);  \
> >>         } while (0)
> >> +#if 0
> >> +#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)	\
> >> +	do {	\
> >> +		if (!((qdev)->msg_enable & NETIF_MSG_##nlevel))	\
> >> +			;					\
> >> +		else						\
> >> +			dev_printk(KERN_##klevel, &((qdev)->pdev->dev),	\
> >> +					"%s: " fmt, __func__, ##args);  \
> >> +	} while (0)
> >> +#else
> >> +#define QPRINTK_DBG(qdev, nlevel, klevel, fmt, args...)
> >> +#endif
> > 
> > This uses an inverted test and it doesn't verify the args to
> > dev_printk when not #defined.
> > 
> > How about:
> 
> I also don't like this kind of change for another reason.
> 
> The message levels are pointless if you're going to adhere to them
> or not based upon some CPP define.
> 
> Either do it, or don't.  Like every other driver does.
> 
> If for some reason the default is problematic, adjust the default that
> you pass to netif_msg_init() or, alternatively, adjust what level the
> debugging messages are assigned to.
> 
> We have all of this wonderful, full, infrastructure for message
> levelling.  And you can change the setting either at module load time
> or via ethtool.  The default is also up to you as well.
> 
> And you're going to stick a "#if 0" CPP control in there? :-/
> 
> No way, I absolutely won't accept this kind of change, there is no
> need for it.  There is more than enough dynamic flexibility, both at
> run-time via module option and ethtool message level selections, and
> at compile time via the default you can choose hoever you like.

[net-next PATCH 4/4] qlge: Fix indentations.

[Ron Mercer]

Signed-off-by: Ron Mercer <ron.mercer@qlogic.com>
---
 drivers/net/qlge/qlge_main.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/qlge/qlge_main.c b/drivers/net/qlge/qlge_main.c
index 57bbb90..224fb7c 100644
--- a/drivers/net/qlge/qlge_main.c
+++ b/drivers/net/qlge/qlge_main.c
@@ -2107,9 +2107,9 @@ static irqreturn_t qlge_isr(int irq, void *dev_id)
 	if (var & intr_context->irq_mask) {
 		QPRINTK_DBG(qdev, INTR, INFO, "Waking handler for rx_ring[0].\n");
 		ql_disable_completion_interrupt(qdev, intr_context->intr);
-					napi_schedule(&rx_ring->napi);
-				work_done++;
-			}
+		napi_schedule(&rx_ring->napi);
+		work_done++;
+	}
 	ql_enable_completion_interrupt(qdev, intr_context->intr);
 	return work_done ? IRQ_HANDLED : IRQ_NONE;
 }
-- 
1.6.0.2

Re: [net-next PATCH 0/4] qlge: Add ethtool self-test.

[David Miller]

From: Ron Mercer <ron.mercer@qlogic.com>
Date: Fri, 30 Oct 2009 15:13:32 -0700

> Changes for QLGE.
> 
> 1) Add ethtool selftest.
> 2) Reduce debug print output.
> 3) Generic cleanup.

I've applied patches #1 and #2

Patch #3 needs to be looked into based upon the feedback we
gave you.

Patch #4 needs to be respun based upon whatever happens to
patch #3.

Thanks Ron.