[PATCH v2 1/4] staging: qlge: fix trailing */ in block comment

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v2 1/4] staging: qlge: fix trailing */ in block comment
       [not found] <20200627145857.15926-1-coiby.xu@gmail.com>
@ 2020-06-27 14:58 ` Coiby Xu
  2020-06-27 14:58 ` [PATCH v2 2/4] staging: qlge: fix else after return or break Coiby Xu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Coiby Xu @ 2020-06-27 14:58 UTC (permalink / raw)
  To: devel
  Cc: joe, dan.carpenter, gregkh, Manish Chopra,
	supporter:QLOGIC QLGE 10Gb ETHERNET DRIVER,
	open list:QLOGIC QLGE 10Gb ETHERNET DRIVER, open list

Remove trailing "*/" in block comments.

Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 drivers/staging/qlge/qlge_main.c |  3 ++-
 drivers/staging/qlge/qlge_mpi.c  | 10 ++++++----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index 1650de13842f..aaecf2b0f9a1 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -3244,7 +3244,8 @@ static void ql_set_irq_mask(struct ql_adapter *qdev, struct intr_context *ctx)
 		 */
 		ctx->irq_mask = (1 << qdev->rx_ring[vect].cq_id);
 		/* Add the TX ring(s) serviced by this vector
-		 * to the mask. */
+		 * to the mask.
+		 */
 		for (j = 0; j < tx_rings_per_vector; j++) {
 			ctx->irq_mask |=
 			(1 << qdev->rx_ring[qdev->rss_ring_count +
diff --git a/drivers/staging/qlge/qlge_mpi.c b/drivers/staging/qlge/qlge_mpi.c
index 60c08d9cc034..3bb08d290525 100644
--- a/drivers/staging/qlge/qlge_mpi.c
+++ b/drivers/staging/qlge/qlge_mpi.c
@@ -389,7 +389,8 @@ static void ql_init_fw_done(struct ql_adapter *qdev, struct mbox_params *mbcp)
  *  This can get called iteratively from the mpi_work thread
  *  when events arrive via an interrupt.
  *  It also gets called when a mailbox command is polling for
- *  it's completion. */
+ *  it's completion.
+ */
 static int ql_mpi_handler(struct ql_adapter *qdev, struct mbox_params *mbcp)
 {
 	int status;
@@ -520,7 +521,7 @@ static int ql_mpi_handler(struct ql_adapter *qdev, struct mbox_params *mbcp)
 	 * changed when a mailbox command is waiting
 	 * for a response and an AEN arrives and
 	 * is handled.
-	 * */
+	 */
 	mbcp->out_count = orig_count;
 	return status;
 }
@@ -555,7 +556,8 @@ static int ql_mailbox_command(struct ql_adapter *qdev, struct mbox_params *mbcp)
 	 * here because some AEN might arrive while
 	 * we're waiting for the mailbox command to
 	 * complete. If more than 5 seconds expire we can
-	 * assume something is wrong. */
+	 * assume something is wrong.
+	 */
 	count = jiffies + HZ * MAILBOX_TIMEOUT;
 	do {
 		/* Wait for the interrupt to come in. */
@@ -1178,7 +1180,7 @@ void ql_mpi_idc_work(struct work_struct *work)
 		/* Signal the resulting link up AEN
 		 * that the frame routing and mac addr
 		 * needs to be set.
-		 * */
+		 */
 		set_bit(QL_CAM_RT_SET, &qdev->flags);
 		/* Do ACK if required */
 		if (timeout) {
--
2.27.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 2/4] staging: qlge: fix else after return or break
       [not found] <20200627145857.15926-1-coiby.xu@gmail.com>
  2020-06-27 14:58 ` [PATCH v2 1/4] staging: qlge: fix trailing */ in block comment Coiby Xu
@ 2020-06-27 14:58 ` Coiby Xu
  2020-06-27 14:58 ` [PATCH v2 3/4] staging: qlge: fix ql_sem_unlock Coiby Xu
  2020-06-27 14:58 ` [PATCH v2 4/4] staging: qlge: replace pr_err with netdev_err Coiby Xu
  3 siblings, 0 replies; 7+ messages in thread
From: Coiby Xu @ 2020-06-27 14:58 UTC (permalink / raw)
  To: devel
  Cc: joe, dan.carpenter, gregkh, Manish Chopra,
	supporter:QLOGIC QLGE 10Gb ETHERNET DRIVER,
	open list:QLOGIC QLGE 10Gb ETHERNET DRIVER, open list

Remove unnecessary elses after return or break.

Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 drivers/staging/qlge/qlge_dbg.c  | 23 ++++++++++-------------
 drivers/staging/qlge/qlge_main.c |  8 ++++----
 drivers/staging/qlge/qlge_mpi.c  |  4 ++--
 3 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
index 058889687907..87433510a224 100644
--- a/drivers/staging/qlge/qlge_dbg.c
+++ b/drivers/staging/qlge/qlge_dbg.c
@@ -1391,12 +1391,11 @@ static void ql_dump_cam_entries(struct ql_adapter *qdev)
 			pr_err("%s: Failed read of mac index register\n",
 			       __func__);
 			return;
-		} else {
-			if (value[0])
-				pr_err("%s: CAM index %d CAM Lookup Lower = 0x%.08x:%.08x, Output = 0x%.08x\n",
-				       qdev->ndev->name, i, value[1], value[0],
-				       value[2]);
 		}
+		if (value[0])
+			pr_err("%s: CAM index %d CAM Lookup Lower = 0x%.08x:%.08x, Output = 0x%.08x\n",
+			       qdev->ndev->name, i, value[1], value[0],
+			       value[2]);
 	}
 	for (i = 0; i < 32; i++) {
 		if (ql_get_mac_addr_reg
@@ -1404,11 +1403,10 @@ static void ql_dump_cam_entries(struct ql_adapter *qdev)
 			pr_err("%s: Failed read of mac index register\n",
 			       __func__);
 			return;
-		} else {
-			if (value[0])
-				pr_err("%s: MCAST index %d CAM Lookup Lower = 0x%.08x:%.08x\n",
-				       qdev->ndev->name, i, value[1], value[0]);
 		}
+		if (value[0])
+			pr_err("%s: MCAST index %d CAM Lookup Lower = 0x%.08x:%.08x\n",
+			       qdev->ndev->name, i, value[1], value[0]);
 	}
 	ql_sem_unlock(qdev, SEM_MAC_ADDR_MASK);
 }
@@ -1427,11 +1425,10 @@ void ql_dump_routing_entries(struct ql_adapter *qdev)
 			pr_err("%s: Failed read of routing index register\n",
 			       __func__);
 			return;
-		} else {
-			if (value)
-				pr_err("%s: Routing Mask %d = 0x%.08x\n",
-				       qdev->ndev->name, i, value);
 		}
+		if (value)
+			pr_err("%s: Routing Mask %d = 0x%.08x\n",
+			       qdev->ndev->name, i, value);
 	}
 	ql_sem_unlock(qdev, SEM_RT_IDX_MASK);
 }
diff --git a/drivers/staging/qlge/qlge_main.c b/drivers/staging/qlge/qlge_main.c
index aaecf2b0f9a1..0054c454506b 100644
--- a/drivers/staging/qlge/qlge_main.c
+++ b/drivers/staging/qlge/qlge_main.c
@@ -3778,10 +3778,10 @@ static int ql_wol(struct ql_adapter *qdev)
 				  "Failed to set magic packet on %s.\n",
 				  qdev->ndev->name);
 			return status;
-		} else
-			netif_info(qdev, drv, qdev->ndev,
-				   "Enabled magic packet successfully on %s.\n",
-				   qdev->ndev->name);
+		}
+		netif_info(qdev, drv, qdev->ndev,
+			   "Enabled magic packet successfully on %s.\n",
+			   qdev->ndev->name);

 		wol |= MB_WOL_MAGIC_PKT;
 	}
diff --git a/drivers/staging/qlge/qlge_mpi.c b/drivers/staging/qlge/qlge_mpi.c
index 3bb08d290525..fa178fc642a6 100644
--- a/drivers/staging/qlge/qlge_mpi.c
+++ b/drivers/staging/qlge/qlge_mpi.c
@@ -276,8 +276,8 @@ static void ql_link_up(struct ql_adapter *qdev, struct mbox_params *mbcp)
 			netif_err(qdev, ifup, qdev->ndev,
 				  "Failed to init CAM/Routing tables.\n");
 			return;
-		} else
-			clear_bit(QL_CAM_RT_SET, &qdev->flags);
+		}
+		clear_bit(QL_CAM_RT_SET, &qdev->flags);
 	}

 	/* Queue up a worker to check the frame
--
2.27.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 3/4] staging: qlge: fix ql_sem_unlock
       [not found] <20200627145857.15926-1-coiby.xu@gmail.com>
  2020-06-27 14:58 ` [PATCH v2 1/4] staging: qlge: fix trailing */ in block comment Coiby Xu
  2020-06-27 14:58 ` [PATCH v2 2/4] staging: qlge: fix else after return or break Coiby Xu
@ 2020-06-27 14:58 ` Coiby Xu
  2020-06-27 14:58 ` [PATCH v2 4/4] staging: qlge: replace pr_err with netdev_err Coiby Xu
  3 siblings, 0 replies; 7+ messages in thread
From: Coiby Xu @ 2020-06-27 14:58 UTC (permalink / raw)
  To: devel
  Cc: joe, dan.carpenter, gregkh, Manish Chopra,
	supporter:QLOGIC QLGE 10Gb ETHERNET DRIVER,
	open list:QLOGIC QLGE 10Gb ETHERNET DRIVER, open list

Some functions return without releasing the lock. Replace return with
break.

Suggested-by Dan Carpenter <dan.carpenter@oracle.com>.

Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 drivers/staging/qlge/qlge_dbg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
index 87433510a224..63e965966ced 100644
--- a/drivers/staging/qlge/qlge_dbg.c
+++ b/drivers/staging/qlge/qlge_dbg.c
@@ -1390,7 +1390,7 @@ static void ql_dump_cam_entries(struct ql_adapter *qdev)
 		if (ql_get_mac_addr_reg(qdev, MAC_ADDR_TYPE_CAM_MAC, i, value)) {
 			pr_err("%s: Failed read of mac index register\n",
 			       __func__);
-			return;
+			break;
 		}
 		if (value[0])
 			pr_err("%s: CAM index %d CAM Lookup Lower = 0x%.08x:%.08x, Output = 0x%.08x\n",
@@ -1402,7 +1402,7 @@ static void ql_dump_cam_entries(struct ql_adapter *qdev)
 		    (qdev, MAC_ADDR_TYPE_MULTI_MAC, i, value)) {
 			pr_err("%s: Failed read of mac index register\n",
 			       __func__);
-			return;
+			break;
 		}
 		if (value[0])
 			pr_err("%s: MCAST index %d CAM Lookup Lower = 0x%.08x:%.08x\n",
@@ -1424,7 +1424,7 @@ void ql_dump_routing_entries(struct ql_adapter *qdev)
 		if (ql_get_routing_reg(qdev, i, &value)) {
 			pr_err("%s: Failed read of routing index register\n",
 			       __func__);
-			return;
+			break;
 		}
 		if (value)
 			pr_err("%s: Routing Mask %d = 0x%.08x\n",
--
2.27.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 4/4] staging: qlge: replace pr_err with netdev_err
       [not found] <20200627145857.15926-1-coiby.xu@gmail.com>
                   ` (2 preceding siblings ...)
  2020-06-27 14:58 ` [PATCH v2 3/4] staging: qlge: fix ql_sem_unlock Coiby Xu
@ 2020-06-27 14:58 ` Coiby Xu
  2020-06-29  5:30   ` Benjamin Poirier
  3 siblings, 1 reply; 7+ messages in thread
From: Coiby Xu @ 2020-06-27 14:58 UTC (permalink / raw)
  To: devel
  Cc: joe, dan.carpenter, gregkh, Manish Chopra,
	supporter:QLOGIC QLGE 10Gb ETHERNET DRIVER,
	open list:QLOGIC QLGE 10Gb ETHERNET DRIVER, open list

Replace all pr_errs with netdev_err.

Suggested-by: Joe Perches <joe@perches.com>
Signed-off-by: Coiby Xu <coiby.xu@gmail.com>
---
 drivers/staging/qlge/qlge_dbg.c | 568 ++++++++++++++++----------------
 1 file changed, 289 insertions(+), 279 deletions(-)

diff --git a/drivers/staging/qlge/qlge_dbg.c b/drivers/staging/qlge/qlge_dbg.c
index 63e965966ced..32fbd30a6a2e 100644
--- a/drivers/staging/qlge/qlge_dbg.c
+++ b/drivers/staging/qlge/qlge_dbg.c
@@ -647,7 +647,7 @@ static void ql_get_mac_protocol_registers(struct ql_adapter *qdev, u32 *buf)
 			max_offset = MAC_ADDR_MAX_MGMT_TU_DP_WCOUNT;
 			break;
 		default:
-			pr_err("Bad type!!! 0x%08x\n", type);
+			netdev_err(qdev->ndev, "Bad type!!! 0x%08x\n", type);
 			max_index = 0;
 			max_offset = 0;
 			break;
@@ -1335,9 +1335,8 @@ static void ql_dump_intr_states(struct ql_adapter *qdev)
 	for (i = 0; i < qdev->intr_count; i++) {
 		ql_write32(qdev, INTR_EN, qdev->intr_context[i].intr_read_mask);
 		value = ql_read32(qdev, INTR_EN);
-		pr_err("%s: Interrupt %d is %s\n",
-		       qdev->ndev->name, i,
-		       (value & INTR_EN_EN ? "enabled" : "disabled"));
+		netdev_err(qdev->ndev, "Interrupt %d is %s\n", i,
+			   (value & INTR_EN_EN ? "enabled" : "disabled"));
 	}
 }

@@ -1345,13 +1344,14 @@ static void ql_dump_intr_states(struct ql_adapter *qdev)
 do {								\
 	u32 data;						\
 	ql_read_xgmac_reg(qdev, reg, &data);			\
-	pr_err("%s: %s = 0x%.08x\n", qdev->ndev->name, #reg, data); \
+	netdev_err(qdev->ndev, "%s = 0x%.08x\n", #reg, data); \
 } while (0)

 void ql_dump_xgmac_control_regs(struct ql_adapter *qdev)
 {
 	if (ql_sem_spinlock(qdev, qdev->xg_sem_mask)) {
-		pr_err("%s: Couldn't get xgmac sem\n", __func__);
+		netdev_err(qdev->ndev, "%s: Couldn't get xgmac sem\n",
+			   __func__);
 		return;
 	}
 	DUMP_XGMAC(qdev, PAUSE_SRC_LO);
@@ -1388,25 +1388,28 @@ static void ql_dump_cam_entries(struct ql_adapter *qdev)
 		return;
 	for (i = 0; i < 4; i++) {
 		if (ql_get_mac_addr_reg(qdev, MAC_ADDR_TYPE_CAM_MAC, i, value)) {
-			pr_err("%s: Failed read of mac index register\n",
-			       __func__);
+			netdev_err(qdev->ndev,
+				   "%s: Failed read of mac index register\n",
+				   __func__);
 			break;
 		}
 		if (value[0])
-			pr_err("%s: CAM index %d CAM Lookup Lower = 0x%.08x:%.08x, Output = 0x%.08x\n",
-			       qdev->ndev->name, i, value[1], value[0],
-			       value[2]);
+			netdev_err(qdev->ndev,
+				   "CAM index %d CAM Lookup Lower = 0x%.08x:%.08x, Output = 0x%.08x\n",
+				   i, value[1], value[0], value[2]);
 	}
 	for (i = 0; i < 32; i++) {
 		if (ql_get_mac_addr_reg
 		    (qdev, MAC_ADDR_TYPE_MULTI_MAC, i, value)) {
-			pr_err("%s: Failed read of mac index register\n",
-			       __func__);
+			netdev_err(qdev->ndev,
+				   "%s: Failed read of mac index register\n",
+				   __func__);
 			break;
 		}
 		if (value[0])
-			pr_err("%s: MCAST index %d CAM Lookup Lower = 0x%.08x:%.08x\n",
-			       qdev->ndev->name, i, value[1], value[0]);
+			netdev_err(qdev->ndev,
+				   "MCAST index %d CAM Lookup Lower = 0x%.08x:%.08x\n",
+				   i, value[1], value[0]);
 	}
 	ql_sem_unlock(qdev, SEM_MAC_ADDR_MASK);
 }
@@ -1422,23 +1425,25 @@ void ql_dump_routing_entries(struct ql_adapter *qdev)
 	for (i = 0; i < 16; i++) {
 		value = 0;
 		if (ql_get_routing_reg(qdev, i, &value)) {
-			pr_err("%s: Failed read of routing index register\n",
-			       __func__);
+			netdev_err(qdev->ndev,
+				   "%s: Failed read of routing index register\n",
+				   __func__);
 			break;
 		}
 		if (value)
-			pr_err("%s: Routing Mask %d = 0x%.08x\n",
-			       qdev->ndev->name, i, value);
+			netdev_err(qdev->ndev,
+				   "%s: Routing Mask %d = 0x%.08x\n",
+				   i, value);
 	}
 	ql_sem_unlock(qdev, SEM_RT_IDX_MASK);
 }

 #define DUMP_REG(qdev, reg)			\
-	pr_err("%-32s= 0x%x\n", #reg, ql_read32(qdev, reg))
+	netdev_err(qdev->ndev, "%-32s= 0x%x\n", #reg, ql_read32(qdev, reg))

 void ql_dump_regs(struct ql_adapter *qdev)
 {
-	pr_err("reg dump for function #%d\n", qdev->func);
+	netdev_err(qdev->ndev, "reg dump for function #%d\n", qdev->func);
 	DUMP_REG(qdev, SYS);
 	DUMP_REG(qdev, RST_FO);
 	DUMP_REG(qdev, FSC);
@@ -1503,11 +1508,12 @@ void ql_dump_regs(struct ql_adapter *qdev)
 #ifdef QL_STAT_DUMP

 #define DUMP_STAT(qdev, stat)	\
-	pr_err("%s = %ld\n", #stat, (unsigned long)(qdev)->nic_stats.stat)
+	netdev_err(qdev->ndev, "%s = %ld\n", #stat,  \
+		   (unsigned long)(qdev)->nic_stats.stat)

 void ql_dump_stat(struct ql_adapter *qdev)
 {
-	pr_err("%s: Enter\n", __func__);
+	netdev_err(qdev->ndev, "%s: Enter\n", __func__);
 	DUMP_STAT(qdev, tx_pkts);
 	DUMP_STAT(qdev, tx_bytes);
 	DUMP_STAT(qdev, tx_mcast_pkts);
@@ -1556,11 +1562,12 @@ void ql_dump_stat(struct ql_adapter *qdev)
 #ifdef QL_DEV_DUMP

 #define DUMP_QDEV_FIELD(qdev, type, field)		\
-	pr_err("qdev->%-24s = " type "\n", #field, (qdev)->field)
+	netdev_err(qdev->ndev, "qdev->%-24s = " type "\n", #field, (qdev)->field)
 #define DUMP_QDEV_DMA_FIELD(qdev, field)		\
-	pr_err("qdev->%-24s = %llx\n", #field, (unsigned long long)qdev->field)
+	netdev_err(qdev->ndev, "qdev->%-24s = %llx\n", #field, \
+		   (unsigned long long)qdev->field)
 #define DUMP_QDEV_ARRAY(qdev, type, array, index, field) \
-	pr_err("%s[%d].%s = " type "\n",		 \
+	netdev_err(qdev->ndev, "%s[%d].%s = " type "\n",		 \
 	       #array, index, #field, (qdev)->array[index].field)
 void ql_dump_qdev(struct ql_adapter *qdev)
 {
@@ -1611,99 +1618,100 @@ void ql_dump_qdev(struct ql_adapter *qdev)
 #ifdef QL_CB_DUMP
 void ql_dump_wqicb(struct wqicb *wqicb)
 {
-	pr_err("Dumping wqicb stuff...\n");
-	pr_err("wqicb->len = 0x%x\n", le16_to_cpu(wqicb->len));
-	pr_err("wqicb->flags = %x\n", le16_to_cpu(wqicb->flags));
-	pr_err("wqicb->cq_id_rss = %d\n",
-	       le16_to_cpu(wqicb->cq_id_rss));
-	pr_err("wqicb->rid = 0x%x\n", le16_to_cpu(wqicb->rid));
-	pr_err("wqicb->wq_addr = 0x%llx\n",
-	       (unsigned long long)le64_to_cpu(wqicb->addr));
-	pr_err("wqicb->wq_cnsmr_idx_addr = 0x%llx\n",
-	       (unsigned long long)le64_to_cpu(wqicb->cnsmr_idx_addr));
+	netdev_err(qdev->ndev, "Dumping wqicb stuff...\n");
+	netdev_err(qdev->ndev, "wqicb->len = 0x%x\n", le16_to_cpu(wqicb->len));
+	netdev_err(qdev->ndev, "wqicb->flags = %x\n",
+		   le16_to_cpu(wqicb->flags));
+	netdev_err(qdev->ndev, "wqicb->cq_id_rss = %d\n",
+		   le16_to_cpu(wqicb->cq_id_rss));
+	netdev_err(qdev->ndev, "wqicb->rid = 0x%x\n", le16_to_cpu(wqicb->rid));
+	netdev_err(qdev->ndev, "wqicb->wq_addr = 0x%llx\n",
+		   (unsigned long long)le64_to_cpu(wqicb->addr));
+	netdev_err(qdev->ndev, "wqicb->wq_cnsmr_idx_addr = 0x%llx\n",
+		   (unsigned long long)le64_to_cpu(wqicb->cnsmr_idx_addr));
 }

 void ql_dump_tx_ring(struct tx_ring *tx_ring)
 {
 	if (!tx_ring)
 		return;
-	pr_err("===================== Dumping tx_ring %d ===============\n",
-	       tx_ring->wq_id);
-	pr_err("tx_ring->base = %p\n", tx_ring->wq_base);
-	pr_err("tx_ring->base_dma = 0x%llx\n",
-	       (unsigned long long)tx_ring->wq_base_dma);
-	pr_err("tx_ring->cnsmr_idx_sh_reg, addr = 0x%p, value = %d\n",
-	       tx_ring->cnsmr_idx_sh_reg,
-	       tx_ring->cnsmr_idx_sh_reg
+	netdev_err(qdev->ndev, "===================== Dumping tx_ring %d ===============\n",
+		   tx_ring->wq_id);
+	netdev_err(qdev->ndev, "tx_ring->base = %p\n", tx_ring->wq_base);
+	netdev_err(qdev->ndev, "tx_ring->base_dma = 0x%llx\n",
+		   (unsigned long long)tx_ring->wq_base_dma);
+	netdev_err(qdev->ndev, "tx_ring->cnsmr_idx_sh_reg, addr = 0x%p, value = %d\n",
+		   tx_ring->cnsmr_idx_sh_reg,
+		   tx_ring->cnsmr_idx_sh_reg
 			? ql_read_sh_reg(tx_ring->cnsmr_idx_sh_reg) : 0);
-	pr_err("tx_ring->size = %d\n", tx_ring->wq_size);
-	pr_err("tx_ring->len = %d\n", tx_ring->wq_len);
-	pr_err("tx_ring->prod_idx_db_reg = %p\n", tx_ring->prod_idx_db_reg);
-	pr_err("tx_ring->valid_db_reg = %p\n", tx_ring->valid_db_reg);
-	pr_err("tx_ring->prod_idx = %d\n", tx_ring->prod_idx);
-	pr_err("tx_ring->cq_id = %d\n", tx_ring->cq_id);
-	pr_err("tx_ring->wq_id = %d\n", tx_ring->wq_id);
-	pr_err("tx_ring->q = %p\n", tx_ring->q);
-	pr_err("tx_ring->tx_count = %d\n", atomic_read(&tx_ring->tx_count));
+	netdev_err(qdev->ndev, "tx_ring->size = %d\n", tx_ring->wq_size);
+	netdev_err(qdev->ndev, "tx_ring->len = %d\n", tx_ring->wq_len);
+	netdev_err(qdev->ndev, "tx_ring->prod_idx_db_reg = %p\n", tx_ring->prod_idx_db_reg);
+	netdev_err(qdev->ndev, "tx_ring->valid_db_reg = %p\n", tx_ring->valid_db_reg);
+	netdev_err(qdev->ndev, "tx_ring->prod_idx = %d\n", tx_ring->prod_idx);
+	netdev_err(qdev->ndev, "tx_ring->cq_id = %d\n", tx_ring->cq_id);
+	netdev_err(qdev->ndev, "tx_ring->wq_id = %d\n", tx_ring->wq_id);
+	netdev_err(qdev->ndev, "tx_ring->q = %p\n", tx_ring->q);
+	netdev_err(qdev->ndev, "tx_ring->tx_count = %d\n", atomic_read(&tx_ring->tx_count));
 }

 void ql_dump_ricb(struct ricb *ricb)
 {
 	int i;

-	pr_err("===================== Dumping ricb ===============\n");
-	pr_err("Dumping ricb stuff...\n");
-
-	pr_err("ricb->base_cq = %d\n", ricb->base_cq & 0x1f);
-	pr_err("ricb->flags = %s%s%s%s%s%s%s%s%s\n",
-	       ricb->base_cq & RSS_L4K ? "RSS_L4K " : "",
-	       ricb->flags & RSS_L6K ? "RSS_L6K " : "",
-	       ricb->flags & RSS_LI ? "RSS_LI " : "",
-	       ricb->flags & RSS_LB ? "RSS_LB " : "",
-	       ricb->flags & RSS_LM ? "RSS_LM " : "",
-	       ricb->flags & RSS_RI4 ? "RSS_RI4 " : "",
-	       ricb->flags & RSS_RT4 ? "RSS_RT4 " : "",
-	       ricb->flags & RSS_RI6 ? "RSS_RI6 " : "",
-	       ricb->flags & RSS_RT6 ? "RSS_RT6 " : "");
-	pr_err("ricb->mask = 0x%.04x\n", le16_to_cpu(ricb->mask));
+	netdev_err(qdev->ndev, "===================== Dumping ricb ===============\n");
+	netdev_err(qdev->ndev, "Dumping ricb stuff...\n");
+
+	netdev_err(qdev->ndev, "ricb->base_cq = %d\n", ricb->base_cq & 0x1f);
+	netdev_err(qdev->ndev, "ricb->flags = %s%s%s%s%s%s%s%s%s\n",
+		   ricb->base_cq & RSS_L4K ? "RSS_L4K " : "",
+		   ricb->flags & RSS_L6K ? "RSS_L6K " : "",
+		   ricb->flags & RSS_LI ? "RSS_LI " : "",
+		   ricb->flags & RSS_LB ? "RSS_LB " : "",
+		   ricb->flags & RSS_LM ? "RSS_LM " : "",
+		   ricb->flags & RSS_RI4 ? "RSS_RI4 " : "",
+		   ricb->flags & RSS_RT4 ? "RSS_RT4 " : "",
+		   ricb->flags & RSS_RI6 ? "RSS_RI6 " : "",
+		   ricb->flags & RSS_RT6 ? "RSS_RT6 " : "");
+	netdev_err(qdev->ndev, "ricb->mask = 0x%.04x\n", le16_to_cpu(ricb->mask));
 	for (i = 0; i < 16; i++)
-		pr_err("ricb->hash_cq_id[%d] = 0x%.08x\n", i,
-		       le32_to_cpu(ricb->hash_cq_id[i]));
+		netdev_err(qdev->ndev, "ricb->hash_cq_id[%d] = 0x%.08x\n", i,
+			   le32_to_cpu(ricb->hash_cq_id[i]));
 	for (i = 0; i < 10; i++)
-		pr_err("ricb->ipv6_hash_key[%d] = 0x%.08x\n", i,
-		       le32_to_cpu(ricb->ipv6_hash_key[i]));
+		netdev_err(qdev->ndev, "ricb->ipv6_hash_key[%d] = 0x%.08x\n", i,
+			   le32_to_cpu(ricb->ipv6_hash_key[i]));
 	for (i = 0; i < 4; i++)
-		pr_err("ricb->ipv4_hash_key[%d] = 0x%.08x\n", i,
-		       le32_to_cpu(ricb->ipv4_hash_key[i]));
+		netdev_err(qdev->ndev, "ricb->ipv4_hash_key[%d] = 0x%.08x\n", i,
+			   le32_to_cpu(ricb->ipv4_hash_key[i]));
 }

 void ql_dump_cqicb(struct cqicb *cqicb)
 {
-	pr_err("Dumping cqicb stuff...\n");
-
-	pr_err("cqicb->msix_vect = %d\n", cqicb->msix_vect);
-	pr_err("cqicb->flags = %x\n", cqicb->flags);
-	pr_err("cqicb->len = %d\n", le16_to_cpu(cqicb->len));
-	pr_err("cqicb->addr = 0x%llx\n",
-	       (unsigned long long)le64_to_cpu(cqicb->addr));
-	pr_err("cqicb->prod_idx_addr = 0x%llx\n",
-	       (unsigned long long)le64_to_cpu(cqicb->prod_idx_addr));
-	pr_err("cqicb->pkt_delay = 0x%.04x\n",
-	       le16_to_cpu(cqicb->pkt_delay));
-	pr_err("cqicb->irq_delay = 0x%.04x\n",
-	       le16_to_cpu(cqicb->irq_delay));
-	pr_err("cqicb->lbq_addr = 0x%llx\n",
-	       (unsigned long long)le64_to_cpu(cqicb->lbq_addr));
-	pr_err("cqicb->lbq_buf_size = 0x%.04x\n",
-	       le16_to_cpu(cqicb->lbq_buf_size));
-	pr_err("cqicb->lbq_len = 0x%.04x\n",
-	       le16_to_cpu(cqicb->lbq_len));
-	pr_err("cqicb->sbq_addr = 0x%llx\n",
-	       (unsigned long long)le64_to_cpu(cqicb->sbq_addr));
-	pr_err("cqicb->sbq_buf_size = 0x%.04x\n",
-	       le16_to_cpu(cqicb->sbq_buf_size));
-	pr_err("cqicb->sbq_len = 0x%.04x\n",
-	       le16_to_cpu(cqicb->sbq_len));
+	netdev_err(qdev->ndev, "Dumping cqicb stuff...\n");
+
+	netdev_err(qdev->ndev, "cqicb->msix_vect = %d\n", cqicb->msix_vect);
+	netdev_err(qdev->ndev, "cqicb->flags = %x\n", cqicb->flags);
+	netdev_err(qdev->ndev, "cqicb->len = %d\n", le16_to_cpu(cqicb->len));
+	netdev_err(qdev->ndev, "cqicb->addr = 0x%llx\n",
+		   (unsigned long long)le64_to_cpu(cqicb->addr));
+	netdev_err(qdev->ndev, "cqicb->prod_idx_addr = 0x%llx\n",
+		   (unsigned long long)le64_to_cpu(cqicb->prod_idx_addr));
+	netdev_err(qdev->ndev, "cqicb->pkt_delay = 0x%.04x\n",
+		   le16_to_cpu(cqicb->pkt_delay));
+	netdev_err(qdev->ndev, "cqicb->irq_delay = 0x%.04x\n",
+		   le16_to_cpu(cqicb->irq_delay));
+	netdev_err(qdev->ndev, "cqicb->lbq_addr = 0x%llx\n",
+		   (unsigned long long)le64_to_cpu(cqicb->lbq_addr));
+	netdev_err(qdev->ndev, "cqicb->lbq_buf_size = 0x%.04x\n",
+		   le16_to_cpu(cqicb->lbq_buf_size));
+	netdev_err(qdev->ndev, "cqicb->lbq_len = 0x%.04x\n",
+		   le16_to_cpu(cqicb->lbq_len));
+	netdev_err(qdev->ndev, "cqicb->sbq_addr = 0x%llx\n",
+		   (unsigned long long)le64_to_cpu(cqicb->sbq_addr));
+	netdev_err(qdev->ndev, "cqicb->sbq_buf_size = 0x%.04x\n",
+		   le16_to_cpu(cqicb->sbq_buf_size));
+	netdev_err(qdev->ndev, "cqicb->sbq_len = 0x%.04x\n",
+		   le16_to_cpu(cqicb->sbq_len));
 }

 static const char *qlge_rx_ring_type_name(struct rx_ring *rx_ring)
@@ -1720,71 +1728,73 @@ void ql_dump_rx_ring(struct rx_ring *rx_ring)
 {
 	if (!rx_ring)
 		return;
-	pr_err("===================== Dumping rx_ring %d ===============\n",
-	       rx_ring->cq_id);
-	pr_err("Dumping rx_ring %d, type = %s\n", rx_ring->cq_id,
-	       qlge_rx_ring_type_name(rx_ring));
-	pr_err("rx_ring->cqicb = %p\n", &rx_ring->cqicb);
-	pr_err("rx_ring->cq_base = %p\n", rx_ring->cq_base);
-	pr_err("rx_ring->cq_base_dma = %llx\n",
-	       (unsigned long long)rx_ring->cq_base_dma);
-	pr_err("rx_ring->cq_size = %d\n", rx_ring->cq_size);
-	pr_err("rx_ring->cq_len = %d\n", rx_ring->cq_len);
-	pr_err("rx_ring->prod_idx_sh_reg, addr = 0x%p, value = %d\n",
-	       rx_ring->prod_idx_sh_reg,
-	       rx_ring->prod_idx_sh_reg
-			? ql_read_sh_reg(rx_ring->prod_idx_sh_reg) : 0);
-	pr_err("rx_ring->prod_idx_sh_reg_dma = %llx\n",
-	       (unsigned long long)rx_ring->prod_idx_sh_reg_dma);
-	pr_err("rx_ring->cnsmr_idx_db_reg = %p\n",
-	       rx_ring->cnsmr_idx_db_reg);
-	pr_err("rx_ring->cnsmr_idx = %d\n", rx_ring->cnsmr_idx);
-	pr_err("rx_ring->curr_entry = %p\n", rx_ring->curr_entry);
-	pr_err("rx_ring->valid_db_reg = %p\n", rx_ring->valid_db_reg);
-
-	pr_err("rx_ring->lbq.base = %p\n", rx_ring->lbq.base);
-	pr_err("rx_ring->lbq.base_dma = %llx\n",
-	       (unsigned long long)rx_ring->lbq.base_dma);
-	pr_err("rx_ring->lbq.base_indirect = %p\n",
-	       rx_ring->lbq.base_indirect);
-	pr_err("rx_ring->lbq.base_indirect_dma = %llx\n",
-	       (unsigned long long)rx_ring->lbq.base_indirect_dma);
-	pr_err("rx_ring->lbq = %p\n", rx_ring->lbq.queue);
-	pr_err("rx_ring->lbq.prod_idx_db_reg = %p\n",
-	       rx_ring->lbq.prod_idx_db_reg);
-	pr_err("rx_ring->lbq.next_to_use = %d\n", rx_ring->lbq.next_to_use);
-	pr_err("rx_ring->lbq.next_to_clean = %d\n", rx_ring->lbq.next_to_clean);
-
-	pr_err("rx_ring->sbq.base = %p\n", rx_ring->sbq.base);
-	pr_err("rx_ring->sbq.base_dma = %llx\n",
-	       (unsigned long long)rx_ring->sbq.base_dma);
-	pr_err("rx_ring->sbq.base_indirect = %p\n",
-	       rx_ring->sbq.base_indirect);
-	pr_err("rx_ring->sbq.base_indirect_dma = %llx\n",
-	       (unsigned long long)rx_ring->sbq.base_indirect_dma);
-	pr_err("rx_ring->sbq = %p\n", rx_ring->sbq.queue);
-	pr_err("rx_ring->sbq.prod_idx_db_reg addr = %p\n",
-	       rx_ring->sbq.prod_idx_db_reg);
-	pr_err("rx_ring->sbq.next_to_use = %d\n", rx_ring->sbq.next_to_use);
-	pr_err("rx_ring->sbq.next_to_clean = %d\n", rx_ring->sbq.next_to_clean);
-	pr_err("rx_ring->cq_id = %d\n", rx_ring->cq_id);
-	pr_err("rx_ring->irq = %d\n", rx_ring->irq);
-	pr_err("rx_ring->cpu = %d\n", rx_ring->cpu);
-	pr_err("rx_ring->qdev = %p\n", rx_ring->qdev);
+	netdev_err(qdev->ndev,
+		   "===================== Dumping rx_ring %d ===============\n",
+		   rx_ring->cq_id);
+	netdev_err(qdev->ndev,
+		   "Dumping rx_ring %d, type = %s\n", rx_ring->cq_id,
+		   qlge_rx_ring_type_name(rx_ring));
+	netdev_err(qdev->ndev, "rx_ring->cqicb = %p\n", &rx_ring->cqicb);
+	netdev_err(qdev->ndev, "rx_ring->cq_base = %p\n", rx_ring->cq_base);
+	netdev_err(qdev->ndev, "rx_ring->cq_base_dma = %llx\n",
+		   (unsigned long long)rx_ring->cq_base_dma);
+	netdev_err(qdev->ndev, "rx_ring->cq_size = %d\n", rx_ring->cq_size);
+	netdev_err(qdev->ndev, "rx_ring->cq_len = %d\n", rx_ring->cq_len);
+	netdev_err(qdev->ndev,
+		   "rx_ring->prod_idx_sh_reg, addr = 0x%p, value = %d\n",
+		   rx_ring->prod_idx_sh_reg,
+		   rx_ring->prod_idx_sh_reg ? ql_read_sh_reg(rx_ring->prod_idx_sh_reg) : 0);
+	netdev_err(qdev->ndev, "rx_ring->prod_idx_sh_reg_dma = %llx\n",
+		   (unsigned long long)rx_ring->prod_idx_sh_reg_dma);
+	netdev_err(qdev->ndev, "rx_ring->cnsmr_idx_db_reg = %p\n",
+		   rx_ring->cnsmr_idx_db_reg);
+	netdev_err(qdev->ndev, "rx_ring->cnsmr_idx = %d\n", rx_ring->cnsmr_idx);
+	netdev_err(qdev->ndev, "rx_ring->curr_entry = %p\n", rx_ring->curr_entry);
+	netdev_err(qdev->ndev, "rx_ring->valid_db_reg = %p\n", rx_ring->valid_db_reg);
+
+	netdev_err(qdev->ndev, "rx_ring->lbq.base = %p\n", rx_ring->lbq.base);
+	netdev_err(qdev->ndev, "rx_ring->lbq.base_dma = %llx\n",
+		   (unsigned long long)rx_ring->lbq.base_dma);
+	netdev_err(qdev->ndev, "rx_ring->lbq.base_indirect = %p\n",
+		   rx_ring->lbq.base_indirect);
+	netdev_err(qdev->ndev, "rx_ring->lbq.base_indirect_dma = %llx\n",
+		   (unsigned long long)rx_ring->lbq.base_indirect_dma);
+	netdev_err(qdev->ndev, "rx_ring->lbq = %p\n", rx_ring->lbq.queue);
+	netdev_err(qdev->ndev, "rx_ring->lbq.prod_idx_db_reg = %p\n",
+		   rx_ring->lbq.prod_idx_db_reg);
+	netdev_err(qdev->ndev, "rx_ring->lbq.next_to_use = %d\n", rx_ring->lbq.next_to_use);
+	netdev_err(qdev->ndev, "rx_ring->lbq.next_to_clean = %d\n", rx_ring->lbq.next_to_clean);
+
+	netdev_err(qdev->ndev, "rx_ring->sbq.base = %p\n", rx_ring->sbq.base);
+	netdev_err(qdev->ndev, "rx_ring->sbq.base_dma = %llx\n",
+		   (unsigned long long)rx_ring->sbq.base_dma);
+	netdev_err(qdev->ndev, "rx_ring->sbq.base_indirect = %p\n",
+		   rx_ring->sbq.base_indirect);
+	netdev_err(qdev->ndev, "rx_ring->sbq.base_indirect_dma = %llx\n",
+		   (unsigned long long)rx_ring->sbq.base_indirect_dma);
+	netdev_err(qdev->ndev, "rx_ring->sbq = %p\n", rx_ring->sbq.queue);
+	netdev_err(qdev->ndev, "rx_ring->sbq.prod_idx_db_reg addr = %p\n",
+		   rx_ring->sbq.prod_idx_db_reg);
+	netdev_err(qdev->ndev, "rx_ring->sbq.next_to_use = %d\n", rx_ring->sbq.next_to_use);
+	netdev_err(qdev->ndev, "rx_ring->sbq.next_to_clean = %d\n", rx_ring->sbq.next_to_clean);
+	netdev_err(qdev->ndev, "rx_ring->cq_id = %d\n", rx_ring->cq_id);
+	netdev_err(qdev->ndev, "rx_ring->irq = %d\n", rx_ring->irq);
+	netdev_err(qdev->ndev, "rx_ring->cpu = %d\n", rx_ring->cpu);
+	netdev_err(qdev->ndev, "rx_ring->qdev = %p\n", rx_ring->qdev);
 }

 void ql_dump_hw_cb(struct ql_adapter *qdev, int size, u32 bit, u16 q_id)
 {
 	void *ptr;

-	pr_err("%s: Enter\n", __func__);
+	netdev_err(qdev->ndev, "%s: Enter\n", __func__);

 	ptr = kmalloc(size, GFP_ATOMIC);
 	if (!ptr)
 		return;

 	if (ql_write_cfg(qdev, ptr, size, bit, q_id)) {
-		pr_err("%s: Failed to upload control block!\n", __func__);
+		netdev_err(qdev->ndev, "%s: Failed to upload control block!\n", __func__);
 		goto fail_it;
 	}
 	switch (bit) {
@@ -1798,7 +1808,7 @@ void ql_dump_hw_cb(struct ql_adapter *qdev, int size, u32 bit, u16 q_id)
 		ql_dump_ricb((struct ricb *)ptr);
 		break;
 	default:
-		pr_err("%s: Invalid bit value = %x\n", __func__, bit);
+		netdev_err(qdev->ndev, "%s: Invalid bit value = %x\n", __func__, bit);
 		break;
 	}
 fail_it:
@@ -1809,29 +1819,29 @@ void ql_dump_hw_cb(struct ql_adapter *qdev, int size, u32 bit, u16 q_id)
 #ifdef QL_OB_DUMP
 void ql_dump_tx_desc(struct tx_buf_desc *tbd)
 {
-	pr_err("tbd->addr  = 0x%llx\n",
-	       le64_to_cpu((u64) tbd->addr));
-	pr_err("tbd->len   = %d\n",
-	       le32_to_cpu(tbd->len & TX_DESC_LEN_MASK));
-	pr_err("tbd->flags = %s %s\n",
-	       tbd->len & TX_DESC_C ? "C" : ".",
-	       tbd->len & TX_DESC_E ? "E" : ".");
+	netdev_err(qdev->ndev, "tbd->addr  = 0x%llx\n",
+		   le64_to_cpu((u64)tbd->addr));
+	netdev_err(qdev->ndev, "tbd->len   = %d\n",
+		   le32_to_cpu(tbd->len & TX_DESC_LEN_MASK));
+	netdev_err(qdev->ndev, "tbd->flags = %s %s\n",
+		   tbd->len & TX_DESC_C ? "C" : ".",
+		   tbd->len & TX_DESC_E ? "E" : ".");
 	tbd++;
-	pr_err("tbd->addr  = 0x%llx\n",
-	       le64_to_cpu((u64) tbd->addr));
-	pr_err("tbd->len   = %d\n",
-	       le32_to_cpu(tbd->len & TX_DESC_LEN_MASK));
-	pr_err("tbd->flags = %s %s\n",
-	       tbd->len & TX_DESC_C ? "C" : ".",
-	       tbd->len & TX_DESC_E ? "E" : ".");
+	netdev_err(qdev->ndev, "tbd->addr  = 0x%llx\n",
+		   le64_to_cpu((u64)tbd->addr));
+	netdev_err(qdev->ndev, "tbd->len   = %d\n",
+		   le32_to_cpu(tbd->len & TX_DESC_LEN_MASK));
+	netdev_err(qdev->ndev, "tbd->flags = %s %s\n",
+		   tbd->len & TX_DESC_C ? "C" : ".",
+		   tbd->len & TX_DESC_E ? "E" : ".");
 	tbd++;
-	pr_err("tbd->addr  = 0x%llx\n",
-	       le64_to_cpu((u64) tbd->addr));
-	pr_err("tbd->len   = %d\n",
-	       le32_to_cpu(tbd->len & TX_DESC_LEN_MASK));
-	pr_err("tbd->flags = %s %s\n",
-	       tbd->len & TX_DESC_C ? "C" : ".",
-	       tbd->len & TX_DESC_E ? "E" : ".");
+	netdev_err(qdev->ndev, "tbd->addr  = 0x%llx\n",
+		   le64_to_cpu((u64)tbd->addr));
+	netdev_err(qdev->ndev, "tbd->len   = %d\n",
+		   le32_to_cpu(tbd->len & TX_DESC_LEN_MASK));
+	netdev_err(qdev->ndev, "tbd->flags = %s %s\n",
+		   tbd->len & TX_DESC_C ? "C" : ".",
+		   tbd->len & TX_DESC_E ? "E" : ".");
 }

 void ql_dump_ob_mac_iocb(struct ob_mac_iocb_req *ob_mac_iocb)
@@ -1841,39 +1851,39 @@ void ql_dump_ob_mac_iocb(struct ob_mac_iocb_req *ob_mac_iocb)
 	struct tx_buf_desc *tbd;
 	u16 frame_len;

-	pr_err("%s\n", __func__);
-	pr_err("opcode         = %s\n",
-	       (ob_mac_iocb->opcode == OPCODE_OB_MAC_IOCB) ? "MAC" : "TSO");
-	pr_err("flags1          = %s %s %s %s %s\n",
-	       ob_mac_tso_iocb->flags1 & OB_MAC_TSO_IOCB_OI ? "OI" : "",
-	       ob_mac_tso_iocb->flags1 & OB_MAC_TSO_IOCB_I ? "I" : "",
-	       ob_mac_tso_iocb->flags1 & OB_MAC_TSO_IOCB_D ? "D" : "",
-	       ob_mac_tso_iocb->flags1 & OB_MAC_TSO_IOCB_IP4 ? "IP4" : "",
-	       ob_mac_tso_iocb->flags1 & OB_MAC_TSO_IOCB_IP6 ? "IP6" : "");
-	pr_err("flags2          = %s %s %s\n",
-	       ob_mac_tso_iocb->flags2 & OB_MAC_TSO_IOCB_LSO ? "LSO" : "",
-	       ob_mac_tso_iocb->flags2 & OB_MAC_TSO_IOCB_UC ? "UC" : "",
-	       ob_mac_tso_iocb->flags2 & OB_MAC_TSO_IOCB_TC ? "TC" : "");
-	pr_err("flags3          = %s %s %s\n",
-	       ob_mac_tso_iocb->flags3 & OB_MAC_TSO_IOCB_IC ? "IC" : "",
-	       ob_mac_tso_iocb->flags3 & OB_MAC_TSO_IOCB_DFP ? "DFP" : "",
-	       ob_mac_tso_iocb->flags3 & OB_MAC_TSO_IOCB_V ? "V" : "");
-	pr_err("tid = %x\n", ob_mac_iocb->tid);
-	pr_err("txq_idx = %d\n", ob_mac_iocb->txq_idx);
-	pr_err("vlan_tci      = %x\n", ob_mac_tso_iocb->vlan_tci);
+	netdev_err(qdev->ndev, "%s\n", __func__);
+	netdev_err(qdev->ndev, "opcode         = %s\n",
+		   (ob_mac_iocb->opcode == OPCODE_OB_MAC_IOCB) ? "MAC" : "TSO");
+	netdev_err(qdev->ndev, "flags1          = %s %s %s %s %s\n",
+		   ob_mac_tso_iocb->flags1 & OB_MAC_TSO_IOCB_OI ? "OI" : "",
+		   ob_mac_tso_iocb->flags1 & OB_MAC_TSO_IOCB_I ? "I" : "",
+		   ob_mac_tso_iocb->flags1 & OB_MAC_TSO_IOCB_D ? "D" : "",
+		   ob_mac_tso_iocb->flags1 & OB_MAC_TSO_IOCB_IP4 ? "IP4" : "",
+		   ob_mac_tso_iocb->flags1 & OB_MAC_TSO_IOCB_IP6 ? "IP6" : "");
+	netdev_err(qdev->ndev, "flags2          = %s %s %s\n",
+		   ob_mac_tso_iocb->flags2 & OB_MAC_TSO_IOCB_LSO ? "LSO" : "",
+		   ob_mac_tso_iocb->flags2 & OB_MAC_TSO_IOCB_UC ? "UC" : "",
+		   ob_mac_tso_iocb->flags2 & OB_MAC_TSO_IOCB_TC ? "TC" : "");
+	netdev_err(qdev->ndev, "flags3          = %s %s %s\n",
+		   ob_mac_tso_iocb->flags3 & OB_MAC_TSO_IOCB_IC ? "IC" : "",
+		   ob_mac_tso_iocb->flags3 & OB_MAC_TSO_IOCB_DFP ? "DFP" : "",
+		   ob_mac_tso_iocb->flags3 & OB_MAC_TSO_IOCB_V ? "V" : "");
+	netdev_err(qdev->ndev, "tid = %x\n", ob_mac_iocb->tid);
+	netdev_err(qdev->ndev, "txq_idx = %d\n", ob_mac_iocb->txq_idx);
+	netdev_err(qdev->ndev, "vlan_tci      = %x\n", ob_mac_tso_iocb->vlan_tci);
 	if (ob_mac_iocb->opcode == OPCODE_OB_MAC_TSO_IOCB) {
-		pr_err("frame_len      = %d\n",
-		       le32_to_cpu(ob_mac_tso_iocb->frame_len));
-		pr_err("mss      = %d\n",
-		       le16_to_cpu(ob_mac_tso_iocb->mss));
-		pr_err("prot_hdr_len   = %d\n",
-		       le16_to_cpu(ob_mac_tso_iocb->total_hdrs_len));
-		pr_err("hdr_offset     = 0x%.04x\n",
-		       le16_to_cpu(ob_mac_tso_iocb->net_trans_offset));
+		netdev_err(qdev->ndev, "frame_len      = %d\n",
+			   le32_to_cpu(ob_mac_tso_iocb->frame_len));
+		netdev_err(qdev->ndev, "mss      = %d\n",
+			   le16_to_cpu(ob_mac_tso_iocb->mss));
+		netdev_err(qdev->ndev, "prot_hdr_len   = %d\n",
+			   le16_to_cpu(ob_mac_tso_iocb->total_hdrs_len));
+		netdev_err(qdev->ndev, "hdr_offset     = 0x%.04x\n",
+			   le16_to_cpu(ob_mac_tso_iocb->net_trans_offset));
 		frame_len = le32_to_cpu(ob_mac_tso_iocb->frame_len);
 	} else {
-		pr_err("frame_len      = %d\n",
-		       le16_to_cpu(ob_mac_iocb->frame_len));
+		netdev_err(qdev->ndev, "frame_len      = %d\n",
+			   le16_to_cpu(ob_mac_iocb->frame_len));
 		frame_len = le16_to_cpu(ob_mac_iocb->frame_len);
 	}
 	tbd = &ob_mac_iocb->tbd[0];
@@ -1882,98 +1892,98 @@ void ql_dump_ob_mac_iocb(struct ob_mac_iocb_req *ob_mac_iocb)

 void ql_dump_ob_mac_rsp(struct ob_mac_iocb_rsp *ob_mac_rsp)
 {
-	pr_err("%s\n", __func__);
-	pr_err("opcode         = %d\n", ob_mac_rsp->opcode);
-	pr_err("flags          = %s %s %s %s %s %s %s\n",
-	       ob_mac_rsp->flags1 & OB_MAC_IOCB_RSP_OI ? "OI" : ".",
-	       ob_mac_rsp->flags1 & OB_MAC_IOCB_RSP_I ? "I" : ".",
-	       ob_mac_rsp->flags1 & OB_MAC_IOCB_RSP_E ? "E" : ".",
-	       ob_mac_rsp->flags1 & OB_MAC_IOCB_RSP_S ? "S" : ".",
-	       ob_mac_rsp->flags1 & OB_MAC_IOCB_RSP_L ? "L" : ".",
-	       ob_mac_rsp->flags1 & OB_MAC_IOCB_RSP_P ? "P" : ".",
-	       ob_mac_rsp->flags2 & OB_MAC_IOCB_RSP_B ? "B" : ".");
-	pr_err("tid = %x\n", ob_mac_rsp->tid);
+	netdev_err(qdev->ndev, "%s\n", __func__);
+	netdev_err(qdev->ndev, "opcode         = %d\n", ob_mac_rsp->opcode);
+	netdev_err(qdev->ndev, "flags          = %s %s %s %s %s %s %s\n",
+		   ob_mac_rsp->flags1 & OB_MAC_IOCB_RSP_OI ?
+			"OI" : ".", ob_mac_rsp->flags1 & OB_MAC_IOCB_RSP_I ? "I" : ".",
+		   ob_mac_rsp->flags1 & OB_MAC_IOCB_RSP_E ? "E" : ".",
+		   ob_mac_rsp->flags1 & OB_MAC_IOCB_RSP_S ? "S" : ".",
+		   ob_mac_rsp->flags1 & OB_MAC_IOCB_RSP_L ? "L" : ".",
+		   ob_mac_rsp->flags1 & OB_MAC_IOCB_RSP_P ? "P" : ".",
+		   ob_mac_rsp->flags2 & OB_MAC_IOCB_RSP_B ? "B" : ".");
+	netdev_err(qdev->ndev, "tid = %x\n", ob_mac_rsp->tid);
 }
 #endif

 #ifdef QL_IB_DUMP
 void ql_dump_ib_mac_rsp(struct ib_mac_iocb_rsp *ib_mac_rsp)
 {
-	pr_err("%s\n", __func__);
-	pr_err("opcode         = 0x%x\n", ib_mac_rsp->opcode);
-	pr_err("flags1 = %s%s%s%s%s%s\n",
-	       ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_OI ? "OI " : "",
-	       ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_I ? "I " : "",
-	       ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_TE ? "TE " : "",
-	       ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_NU ? "NU " : "",
-	       ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_IE ? "IE " : "",
-	       ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_B ? "B " : "");
+	netdev_err(qdev->ndev, "%s\n", __func__);
+	netdev_err(qdev->ndev, "opcode         = 0x%x\n", ib_mac_rsp->opcode);
+	netdev_err(qdev->ndev, "flags1 = %s%s%s%s%s%s\n",
+		   ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_OI ? "OI " : "",
+		   ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_I ? "I " : "",
+		   ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_TE ? "TE " : "",
+		   ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_NU ? "NU " : "",
+		   ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_IE ? "IE " : "",
+		   ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_B ? "B " : "");

 	if (ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_M_MASK)
-		pr_err("%s%s%s Multicast\n",
-		       (ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_M_MASK) ==
-		       IB_MAC_IOCB_RSP_M_HASH ? "Hash" : "",
-		       (ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_M_MASK) ==
-		       IB_MAC_IOCB_RSP_M_REG ? "Registered" : "",
-		       (ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_M_MASK) ==
-		       IB_MAC_IOCB_RSP_M_PROM ? "Promiscuous" : "");
-
-	pr_err("flags2 = %s%s%s%s%s\n",
-	       (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_P) ? "P " : "",
-	       (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_V) ? "V " : "",
-	       (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_U) ? "U " : "",
-	       (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_T) ? "T " : "",
-	       (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_FO) ? "FO " : "");
+		netdev_err(qdev->ndev, "%s%s%s Multicast\n",
+			   (ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_M_MASK) ==
+			   IB_MAC_IOCB_RSP_M_HASH ? "Hash" : "",
+			   (ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_M_MASK) ==
+			   IB_MAC_IOCB_RSP_M_REG ? "Registered" : "",
+			   (ib_mac_rsp->flags1 & IB_MAC_IOCB_RSP_M_MASK) ==
+			   IB_MAC_IOCB_RSP_M_PROM ? "Promiscuous" : "");
+
+	netdev_err(qdev->ndev, "flags2 = %s%s%s%s%s\n",
+		   (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_P) ? "P " : "",
+		   (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_V) ? "V " : "",
+		   (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_U) ? "U " : "",
+		   (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_T) ? "T " : "",
+		   (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_FO) ? "FO " : "");

 	if (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK)
-		pr_err("%s%s%s%s%s error\n",
-		       (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) ==
-		       IB_MAC_IOCB_RSP_ERR_OVERSIZE ? "oversize" : "",
-		       (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) ==
-		       IB_MAC_IOCB_RSP_ERR_UNDERSIZE ? "undersize" : "",
-		       (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) ==
-		       IB_MAC_IOCB_RSP_ERR_PREAMBLE ? "preamble" : "",
-		       (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) ==
-		       IB_MAC_IOCB_RSP_ERR_FRAME_LEN ? "frame length" : "",
-		       (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) ==
-		       IB_MAC_IOCB_RSP_ERR_CRC ? "CRC" : "");
-
-	pr_err("flags3 = %s%s\n",
-	       ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_DS ? "DS " : "",
-	       ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_DL ? "DL " : "");
+		netdev_err(qdev->ndev, "%s%s%s%s%s error\n",
+			   (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) ==
+			   IB_MAC_IOCB_RSP_ERR_OVERSIZE ? "oversize" : "",
+			   (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) ==
+			   IB_MAC_IOCB_RSP_ERR_UNDERSIZE ? "undersize" : "",
+			   (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) ==
+			   IB_MAC_IOCB_RSP_ERR_PREAMBLE ? "preamble" : "",
+			   (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) ==
+			   IB_MAC_IOCB_RSP_ERR_FRAME_LEN ? "frame length" : "",
+			   (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_ERR_MASK) ==
+			   IB_MAC_IOCB_RSP_ERR_CRC ? "CRC" : "");
+
+	netdev_err(qdev->ndev, "flags3 = %s%s\n",
+		   ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_DS ? "DS " : "",
+		   ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_DL ? "DL " : "");

 	if (ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_RSS_MASK)
-		pr_err("RSS flags = %s%s%s%s\n",
-		       ((ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_RSS_MASK) ==
-			IB_MAC_IOCB_RSP_M_IPV4) ? "IPv4 RSS" : "",
-		       ((ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_RSS_MASK) ==
-			IB_MAC_IOCB_RSP_M_IPV6) ? "IPv6 RSS " : "",
-		       ((ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_RSS_MASK) ==
-			IB_MAC_IOCB_RSP_M_TCP_V4) ? "TCP/IPv4 RSS" : "",
-		       ((ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_RSS_MASK) ==
-			IB_MAC_IOCB_RSP_M_TCP_V6) ? "TCP/IPv6 RSS" : "");
-
-	pr_err("data_len	= %d\n",
-	       le32_to_cpu(ib_mac_rsp->data_len));
-	pr_err("data_addr    = 0x%llx\n",
-	       (unsigned long long)le64_to_cpu(ib_mac_rsp->data_addr));
+		netdev_err(qdev->ndev, "RSS flags = %s%s%s%s\n",
+			   ((ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_RSS_MASK) ==
+			    IB_MAC_IOCB_RSP_M_IPV4) ? "IPv4 RSS" : "",
+			   ((ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_RSS_MASK) ==
+			    IB_MAC_IOCB_RSP_M_IPV6) ? "IPv6 RSS " : "",
+			   ((ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_RSS_MASK) ==
+			    IB_MAC_IOCB_RSP_M_TCP_V4) ? "TCP/IPv4 RSS" : "",
+			   ((ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_RSS_MASK) ==
+			    IB_MAC_IOCB_RSP_M_TCP_V6) ? "TCP/IPv6 RSS" : "");
+
+	netdev_err(qdev->ndev, "data_len	= %d\n",
+		   le32_to_cpu(ib_mac_rsp->data_len));
+	netdev_err(qdev->ndev, "data_addr    = 0x%llx\n",
+		   (unsigned long long)le64_to_cpu(ib_mac_rsp->data_addr));
 	if (ib_mac_rsp->flags3 & IB_MAC_IOCB_RSP_RSS_MASK)
-		pr_err("rss    = %x\n",
-		       le32_to_cpu(ib_mac_rsp->rss));
+		netdev_err(qdev->ndev, "rss    = %x\n",
+			   le32_to_cpu(ib_mac_rsp->rss));
 	if (ib_mac_rsp->flags2 & IB_MAC_IOCB_RSP_V)
-		pr_err("vlan_id    = %x\n",
-		       le16_to_cpu(ib_mac_rsp->vlan_id));
+		netdev_err(qdev->ndev, "vlan_id    = %x\n",
+			   le16_to_cpu(ib_mac_rsp->vlan_id));

-	pr_err("flags4 = %s%s%s\n",
-	       ib_mac_rsp->flags4 & IB_MAC_IOCB_RSP_HV ? "HV " : "",
-		ib_mac_rsp->flags4 & IB_MAC_IOCB_RSP_HS ? "HS " : "",
-		ib_mac_rsp->flags4 & IB_MAC_IOCB_RSP_HL ? "HL " : "");
+	netdev_err(qdev->ndev, "flags4 = %s%s%s\n",
+		   ib_mac_rsp->flags4 & IB_MAC_IOCB_RSP_HV ? "HV " : "",
+		   ib_mac_rsp->flags4 & IB_MAC_IOCB_RSP_HS ? "HS " : "",
+		   ib_mac_rsp->flags4 & IB_MAC_IOCB_RSP_HL ? "HL " : "");

 	if (ib_mac_rsp->flags4 & IB_MAC_IOCB_RSP_HV) {
-		pr_err("hdr length	= %d\n",
-		       le32_to_cpu(ib_mac_rsp->hdr_len));
-		pr_err("hdr addr    = 0x%llx\n",
-		       (unsigned long long)le64_to_cpu(ib_mac_rsp->hdr_addr));
+		netdev_err(qdev->ndev, "hdr length	= %d\n",
+			   le32_to_cpu(ib_mac_rsp->hdr_len));
+		netdev_err(qdev->ndev, "hdr addr    = 0x%llx\n",
+			   (unsigned long long)le64_to_cpu(ib_mac_rsp->hdr_addr));
 	}
 }
 #endif
--
2.27.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 4/4] staging: qlge: replace pr_err with netdev_err
  2020-06-27 14:58 ` [PATCH v2 4/4] staging: qlge: replace pr_err with netdev_err Coiby Xu
@ 2020-06-29  5:30   ` Benjamin Poirier
  2020-06-29 17:43     ` Coiby Xu
  0 siblings, 1 reply; 7+ messages in thread
From: Benjamin Poirier @ 2020-06-29  5:30 UTC (permalink / raw)
  To: Coiby Xu
  Cc: devel, joe, dan.carpenter, gregkh, Manish Chopra,
	supporter:QLOGIC QLGE 10Gb ETHERNET DRIVER,
	open list:QLOGIC QLGE 10Gb ETHERNET DRIVER, open list

On 2020-06-27 22:58 +0800, Coiby Xu wrote:
[...]
>  void ql_dump_qdev(struct ql_adapter *qdev)
>  {
> @@ -1611,99 +1618,100 @@ void ql_dump_qdev(struct ql_adapter *qdev)
>  #ifdef QL_CB_DUMP
>  void ql_dump_wqicb(struct wqicb *wqicb)
>  {
> -	pr_err("Dumping wqicb stuff...\n");
> -	pr_err("wqicb->len = 0x%x\n", le16_to_cpu(wqicb->len));
> -	pr_err("wqicb->flags = %x\n", le16_to_cpu(wqicb->flags));
> -	pr_err("wqicb->cq_id_rss = %d\n",
> -	       le16_to_cpu(wqicb->cq_id_rss));
> -	pr_err("wqicb->rid = 0x%x\n", le16_to_cpu(wqicb->rid));
> -	pr_err("wqicb->wq_addr = 0x%llx\n",
> -	       (unsigned long long)le64_to_cpu(wqicb->addr));
> -	pr_err("wqicb->wq_cnsmr_idx_addr = 0x%llx\n",
> -	       (unsigned long long)le64_to_cpu(wqicb->cnsmr_idx_addr));
> +	netdev_err(qdev->ndev, "Dumping wqicb stuff...\n");

drivers/staging/qlge/qlge_dbg.c:1621:13: error: ‘qdev’ undeclared (first use in this function); did you mean ‘cdev’?
 1621 |  netdev_err(qdev->ndev, "Dumping wqicb stuff...\n");
      |             ^~~~
      |             cdev

[...]
and many more like that

Anyways, qlge_dbg.h is a dumpster. It has hundreds of lines of code
bitrotting away in ifdef land. See this comment from David Miller on the
topic of ifdef'ed debugging code:
https://lore.kernel.org/netdev/20200303.145916.1506066510928020193.davem@davemloft.net/

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 4/4] staging: qlge: replace pr_err with netdev_err
  2020-06-29  5:30   ` Benjamin Poirier
@ 2020-06-29 17:43     ` Coiby Xu
  2020-06-30  4:41       ` Benjamin Poirier
  0 siblings, 1 reply; 7+ messages in thread
From: Coiby Xu @ 2020-06-29 17:43 UTC (permalink / raw)
  To: Benjamin Poirier
  Cc: devel, joe, dan.carpenter, gregkh, Manish Chopra,
	supporter:QLOGIC QLGE 10Gb ETHERNET DRIVER,
	open list:QLOGIC QLGE 10Gb ETHERNET DRIVER, open list

On Mon, Jun 29, 2020 at 02:30:04PM +0900, Benjamin Poirier wrote:
>On 2020-06-27 22:58 +0800, Coiby Xu wrote:
>[...]
>>  void ql_dump_qdev(struct ql_adapter *qdev)
>>  {
>> @@ -1611,99 +1618,100 @@ void ql_dump_qdev(struct ql_adapter *qdev)
>>  #ifdef QL_CB_DUMP
>>  void ql_dump_wqicb(struct wqicb *wqicb)
>>  {
>> -	pr_err("Dumping wqicb stuff...\n");
>> -	pr_err("wqicb->len = 0x%x\n", le16_to_cpu(wqicb->len));
>> -	pr_err("wqicb->flags = %x\n", le16_to_cpu(wqicb->flags));
>> -	pr_err("wqicb->cq_id_rss = %d\n",
>> -	       le16_to_cpu(wqicb->cq_id_rss));
>> -	pr_err("wqicb->rid = 0x%x\n", le16_to_cpu(wqicb->rid));
>> -	pr_err("wqicb->wq_addr = 0x%llx\n",
>> -	       (unsigned long long)le64_to_cpu(wqicb->addr));
>> -	pr_err("wqicb->wq_cnsmr_idx_addr = 0x%llx\n",
>> -	       (unsigned long long)le64_to_cpu(wqicb->cnsmr_idx_addr));
>> +	netdev_err(qdev->ndev, "Dumping wqicb stuff...\n");
>
>drivers/staging/qlge/qlge_dbg.c:1621:13: error: ‘qdev’ undeclared (first use in this function); did you mean ‘cdev’?
> 1621 |  netdev_err(qdev->ndev, "Dumping wqicb stuff...\n");
>      |             ^~~~
>      |             cdev
>
>[...]
>and many more like that
>
>Anyways, qlge_dbg.h is a dumpster. It has hundreds of lines of code
>bitrotting away in ifdef land. See this comment from David Miller on the
>topic of ifdef'ed debugging code:
>https://lore.kernel.org/netdev/20200303.145916.1506066510928020193.davem@davemloft.net/

Thank you for spotting this problem! This issue could be fixed by
passing qdev to ql_dump_wqicb. Or are you suggesting we completely
remove qlge_dbg since it's only for the purpose of debugging the driver
by the developer?

Btw, I'm curious how you make this compiling error occur. Do you manually trigger
it via "QL_CB_DUMP=1 make M=drivers/staging/qlge" or use some automate
tools?

--
Best regards,
Coiby

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 4/4] staging: qlge: replace pr_err with netdev_err
  2020-06-29 17:43     ` Coiby Xu
@ 2020-06-30  4:41       ` Benjamin Poirier
  0 siblings, 0 replies; 7+ messages in thread
From: Benjamin Poirier @ 2020-06-30  4:41 UTC (permalink / raw)
  To: Coiby Xu
  Cc: devel, joe, dan.carpenter, gregkh, Manish Chopra,
	supporter:QLOGIC QLGE 10Gb ETHERNET DRIVER,
	open list:QLOGIC QLGE 10Gb ETHERNET DRIVER, open list

On 2020-06-30 01:43 +0800, Coiby Xu wrote:
> On Mon, Jun 29, 2020 at 02:30:04PM +0900, Benjamin Poirier wrote:
> > On 2020-06-27 22:58 +0800, Coiby Xu wrote:
> > [...]
> > >  void ql_dump_qdev(struct ql_adapter *qdev)
> > >  {
> > > @@ -1611,99 +1618,100 @@ void ql_dump_qdev(struct ql_adapter *qdev)
> > >  #ifdef QL_CB_DUMP
> > >  void ql_dump_wqicb(struct wqicb *wqicb)
> > >  {
> > > -	pr_err("Dumping wqicb stuff...\n");
> > > -	pr_err("wqicb->len = 0x%x\n", le16_to_cpu(wqicb->len));
> > > -	pr_err("wqicb->flags = %x\n", le16_to_cpu(wqicb->flags));
> > > -	pr_err("wqicb->cq_id_rss = %d\n",
> > > -	       le16_to_cpu(wqicb->cq_id_rss));
> > > -	pr_err("wqicb->rid = 0x%x\n", le16_to_cpu(wqicb->rid));
> > > -	pr_err("wqicb->wq_addr = 0x%llx\n",
> > > -	       (unsigned long long)le64_to_cpu(wqicb->addr));
> > > -	pr_err("wqicb->wq_cnsmr_idx_addr = 0x%llx\n",
> > > -	       (unsigned long long)le64_to_cpu(wqicb->cnsmr_idx_addr));
> > > +	netdev_err(qdev->ndev, "Dumping wqicb stuff...\n");
> > 
> > drivers/staging/qlge/qlge_dbg.c:1621:13: error: ‘qdev’ undeclared (first use in this function); did you mean ‘cdev’?
> > 1621 |  netdev_err(qdev->ndev, "Dumping wqicb stuff...\n");
> >      |             ^~~~
> >      |             cdev
> > 
> > [...]
> > and many more like that
> > 
> > Anyways, qlge_dbg.h is a dumpster. It has hundreds of lines of code
> > bitrotting away in ifdef land. See this comment from David Miller on the
> > topic of ifdef'ed debugging code:
> > https://lore.kernel.org/netdev/20200303.145916.1506066510928020193.davem@davemloft.net/
> 
> Thank you for spotting this problem! This issue could be fixed by
> passing qdev to ql_dump_wqicb. Or are you suggesting we completely
> remove qlge_dbg since it's only for the purpose of debugging the driver
> by the developer?

At 2000 lines, qlge_dbg.c alone is larger than some entire ethernet
drivers. Most of what it does is dump kernel data structures or pci
memory mapped registers to dmesg. There are better facilities for that.
My thinking is not simply to delete qlge_dbg.c but to replace it, making
sure that most of the same information is still available. For data
structures, crash or drgn can be used; possibly with a script for the
latter which formats the data. For pci registers, they should be
included in the ethtool register dump and a patch added to ethtool to
pretty print them. That's what other drivers like e1000e do. For the
"coredump", devlink health can be used.

The qlge_force_coredump module option should also be removed. At the
moment, calling the ethtool register dump function with that option
enabled does a hexdump of a 176k struct to dmesg. That's shameful.

> 
> Btw, I'm curious how you make this compiling error occur. Do you manually trigger
> it via "QL_CB_DUMP=1 make M=drivers/staging/qlge" or use some automate
> tools?

I just uncommented the defines in qlge.h

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-06-30  4:42 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <20200627145857.15926-1-coiby.xu@gmail.com>
2020-06-27 14:58 ` [PATCH v2 1/4] staging: qlge: fix trailing */ in block comment Coiby Xu
2020-06-27 14:58 ` [PATCH v2 2/4] staging: qlge: fix else after return or break Coiby Xu
2020-06-27 14:58 ` [PATCH v2 3/4] staging: qlge: fix ql_sem_unlock Coiby Xu
2020-06-27 14:58 ` [PATCH v2 4/4] staging: qlge: replace pr_err with netdev_err Coiby Xu
2020-06-29  5:30   ` Benjamin Poirier
2020-06-29 17:43     ` Coiby Xu
2020-06-30  4:41       ` Benjamin Poirier

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).