[PATCH iwl-next] igc: demote register and ring dumps to debug

[PATCH iwl-next] igc: demote register and ring dumps to debug

[Rui Salvaterra]

This is debug information, upon which the user is not expected to act. Output as
such. This avoids polluting the dmesg with full register dumps at every link
down.

Signed-off-by: Rui Salvaterra <rsalvaterra@gmail.com>
---

This file hasn't been touched in over four years, it's probably from a time when
the driver was under heavy development (started in 2018). Nevertheless, the
status quo is positively annoying. :)

 drivers/net/ethernet/intel/igc/igc_dump.c | 54 +++++++++++------------
 1 file changed, 27 insertions(+), 27 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_dump.c b/drivers/net/ethernet/intel/igc/igc_dump.c
index c09c95cc5f70..e84d09ca8e67 100644
--- a/drivers/net/ethernet/intel/igc/igc_dump.c
+++ b/drivers/net/ethernet/intel/igc/igc_dump.c
@@ -98,13 +98,13 @@ static void igc_regdump(struct igc_hw *hw, struct igc_reg_info *reginfo)
 			regs[n] = rd32(IGC_TXDCTL(n));
 		break;
 	default:
-		netdev_info(dev, "%-15s %08x\n", reginfo->name,
+		netdev_dbg(dev, "%-15s %08x\n", reginfo->name,
 			    rd32(reginfo->ofs));
 		return;
 	}
 
 	snprintf(rname, 16, "%s%s", reginfo->name, "[0-3]");
-	netdev_info(dev, "%-15s %08x %08x %08x %08x\n", rname, regs[0], regs[1],
+	netdev_dbg(dev, "%-15s %08x %08x %08x %08x\n", rname, regs[0], regs[1],
 		    regs[2], regs[3]);
 }
 
@@ -123,22 +123,22 @@ void igc_rings_dump(struct igc_adapter *adapter)
 	if (!netif_msg_hw(adapter))
 		return;
 
-	netdev_info(netdev, "Device info: state %016lX trans_start %016lX\n",
+	netdev_dbg(netdev, "Device info: state %016lX trans_start %016lX\n",
 		    netdev->state, dev_trans_start(netdev));
 
 	/* Print TX Ring Summary */
 	if (!netif_running(netdev))
 		goto exit;
 
-	netdev_info(netdev, "TX Rings Summary\n");
-	netdev_info(netdev, "Queue [NTU] [NTC] [bi(ntc)->dma  ] leng ntw timestamp\n");
+	netdev_dbg(netdev, "TX Rings Summary\n");
+	netdev_dbg(netdev, "Queue [NTU] [NTC] [bi(ntc)->dma  ] leng ntw timestamp\n");
 	for (n = 0; n < adapter->num_tx_queues; n++) {
 		struct igc_tx_buffer *buffer_info;
 
 		tx_ring = adapter->tx_ring[n];
 		buffer_info = &tx_ring->tx_buffer_info[tx_ring->next_to_clean];
 
-		netdev_info(netdev, "%5d %5X %5X %016llX %04X %p %016llX\n",
+		netdev_dbg(netdev, "%5d %5X %5X %016llX %04X %p %016llX\n",
 			    n, tx_ring->next_to_use, tx_ring->next_to_clean,
 			    (u64)dma_unmap_addr(buffer_info, dma),
 			    dma_unmap_len(buffer_info, len),
@@ -150,7 +150,7 @@ void igc_rings_dump(struct igc_adapter *adapter)
 	if (!netif_msg_tx_done(adapter))
 		goto rx_ring_summary;
 
-	netdev_info(netdev, "TX Rings Dump\n");
+	netdev_dbg(netdev, "TX Rings Dump\n");
 
 	/* Transmit Descriptor Formats
 	 *
@@ -165,11 +165,11 @@ void igc_rings_dump(struct igc_adapter *adapter)
 
 	for (n = 0; n < adapter->num_tx_queues; n++) {
 		tx_ring = adapter->tx_ring[n];
-		netdev_info(netdev, "------------------------------------\n");
-		netdev_info(netdev, "TX QUEUE INDEX = %d\n",
+		netdev_dbg(netdev, "------------------------------------\n");
+		netdev_dbg(netdev, "TX QUEUE INDEX = %d\n",
 			    tx_ring->queue_index);
-		netdev_info(netdev, "------------------------------------\n");
-		netdev_info(netdev, "T [desc]     [address 63:0  ] [PlPOCIStDDM Ln] [bi->dma       ] leng  ntw timestamp        bi->skb\n");
+		netdev_dbg(netdev, "------------------------------------\n");
+		netdev_dbg(netdev, "T [desc]     [address 63:0  ] [PlPOCIStDDM Ln] [bi->dma       ] leng  ntw timestamp        bi->skb\n");
 
 		for (i = 0; tx_ring->desc && (i < tx_ring->count); i++) {
 			const char *next_desc;
@@ -188,7 +188,7 @@ void igc_rings_dump(struct igc_adapter *adapter)
 			else
 				next_desc = "";
 
-			netdev_info(netdev, "T [0x%03X]    %016llX %016llX %016llX %04X  %p %016llX %p%s\n",
+			netdev_dbg(netdev, "T [0x%03X]    %016llX %016llX %016llX %04X  %p %016llX %p%s\n",
 				    i, le64_to_cpu(u0->a),
 				    le64_to_cpu(u0->b),
 				    (u64)dma_unmap_addr(buffer_info, dma),
@@ -198,7 +198,7 @@ void igc_rings_dump(struct igc_adapter *adapter)
 				    buffer_info->skb, next_desc);
 
 			if (netif_msg_pktdata(adapter) && buffer_info->skb)
-				print_hex_dump(KERN_INFO, "",
+				print_hex_dump(KERN_DEBUG, "",
 					       DUMP_PREFIX_ADDRESS,
 					       16, 1, buffer_info->skb->data,
 					       dma_unmap_len(buffer_info, len),
@@ -208,11 +208,11 @@ void igc_rings_dump(struct igc_adapter *adapter)
 
 	/* Print RX Rings Summary */
 rx_ring_summary:
-	netdev_info(netdev, "RX Rings Summary\n");
-	netdev_info(netdev, "Queue [NTU] [NTC]\n");
+	netdev_dbg(netdev, "RX Rings Summary\n");
+	netdev_dbg(netdev, "Queue [NTU] [NTC]\n");
 	for (n = 0; n < adapter->num_rx_queues; n++) {
 		rx_ring = adapter->rx_ring[n];
-		netdev_info(netdev, "%5d %5X %5X\n", n, rx_ring->next_to_use,
+		netdev_dbg(netdev, "%5d %5X %5X\n", n, rx_ring->next_to_use,
 			    rx_ring->next_to_clean);
 	}
 
@@ -220,7 +220,7 @@ void igc_rings_dump(struct igc_adapter *adapter)
 	if (!netif_msg_rx_status(adapter))
 		goto exit;
 
-	netdev_info(netdev, "RX Rings Dump\n");
+	netdev_dbg(netdev, "RX Rings Dump\n");
 
 	/* Advanced Receive Descriptor (Read) Format
 	 *    63                                           1        0
@@ -245,12 +245,12 @@ void igc_rings_dump(struct igc_adapter *adapter)
 
 	for (n = 0; n < adapter->num_rx_queues; n++) {
 		rx_ring = adapter->rx_ring[n];
-		netdev_info(netdev, "------------------------------------\n");
-		netdev_info(netdev, "RX QUEUE INDEX = %d\n",
+		netdev_dbg(netdev, "------------------------------------\n");
+		netdev_dbg(netdev, "RX QUEUE INDEX = %d\n",
 			    rx_ring->queue_index);
-		netdev_info(netdev, "------------------------------------\n");
-		netdev_info(netdev, "R  [desc]      [ PktBuf     A0] [  HeadBuf   DD] [bi->dma       ] [bi->skb] <-- Adv Rx Read format\n");
-		netdev_info(netdev, "RWB[desc]      [PcsmIpSHl PtRs] [vl er S cks ln] ---------------- [bi->skb] <-- Adv Rx Write-Back format\n");
+		netdev_dbg(netdev, "------------------------------------\n");
+		netdev_dbg(netdev, "R  [desc]      [ PktBuf     A0] [  HeadBuf   DD] [bi->dma       ] [bi->skb] <-- Adv Rx Read format\n");
+		netdev_dbg(netdev, "RWB[desc]      [PcsmIpSHl PtRs] [vl er S cks ln] ---------------- [bi->skb] <-- Adv Rx Write-Back format\n");
 
 		for (i = 0; i < rx_ring->count; i++) {
 			const char *next_desc;
@@ -270,13 +270,13 @@ void igc_rings_dump(struct igc_adapter *adapter)
 
 			if (staterr & IGC_RXD_STAT_DD) {
 				/* Descriptor Done */
-				netdev_info(netdev, "%s[0x%03X]     %016llX %016llX ---------------- %s\n",
+				netdev_dbg(netdev, "%s[0x%03X]     %016llX %016llX ---------------- %s\n",
 					    "RWB", i,
 					    le64_to_cpu(u0->a),
 					    le64_to_cpu(u0->b),
 					    next_desc);
 			} else {
-				netdev_info(netdev, "%s[0x%03X]     %016llX %016llX %016llX %s\n",
+				netdev_dbg(netdev, "%s[0x%03X]     %016llX %016llX %016llX %s\n",
 					    "R  ", i,
 					    le64_to_cpu(u0->a),
 					    le64_to_cpu(u0->b),
@@ -285,7 +285,7 @@ void igc_rings_dump(struct igc_adapter *adapter)
 
 				if (netif_msg_pktdata(adapter) &&
 				    buffer_info->dma && buffer_info->page) {
-					print_hex_dump(KERN_INFO, "",
+					print_hex_dump(KERN_DEBUG, "",
 						       DUMP_PREFIX_ADDRESS,
 						       16, 1,
 						       page_address
@@ -309,8 +309,8 @@ void igc_regs_dump(struct igc_adapter *adapter)
 	struct igc_reg_info *reginfo;
 
 	/* Print Registers */
-	netdev_info(adapter->netdev, "Register Dump\n");
-	netdev_info(adapter->netdev, "Register Name   Value\n");
+	netdev_dbg(adapter->netdev, "Register Dump\n");
+	netdev_dbg(adapter->netdev, "Register Name   Value\n");
 	for (reginfo = (struct igc_reg_info *)igc_reg_info_tbl;
 	     reginfo->name; reginfo++) {
 		igc_regdump(hw, reginfo);
-- 
2.49.0

Re: [PATCH iwl-next] igc: demote register and ring dumps to debug

[Simon Horman]

On Mon, Jul 07, 2025 at 10:17:10AM +0100, Rui Salvaterra wrote:
> This is debug information, upon which the user is not expected to act. Output as
> such. This avoids polluting the dmesg with full register dumps at every link
> down.
> 
> Signed-off-by: Rui Salvaterra <rsalvaterra@gmail.com>
> ---
> 
> This file hasn't been touched in over four years, it's probably from a time when
> the driver was under heavy development (started in 2018). Nevertheless, the
> status quo is positively annoying. :)

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [Intel-wired-lan] [PATCH iwl-next] igc: demote register and ring dumps to debug

[Lifshits, Vitaly]

On 7/7/2025 12:17 PM, Rui Salvaterra wrote:
> This is debug information, upon which the user is not expected to act. Output as
> such. This avoids polluting the dmesg with full register dumps at every link
> down.
> 
> Signed-off-by: Rui Salvaterra <rsalvaterra@gmail.com>
> ---
> 
> This file hasn't been touched in over four years, it's probably from a time when
> the driver was under heavy development (started in 2018). Nevertheless, the
> status quo is positively annoying. :)
> 

Hi Rui,

Thanks for this patch.

However, I don't completely agree with you.
I think that the main idea here was to have enough data to pass for the 
developers in a case where any issue happens without enabling debug 
mode. Especially, for those issues that reproduce rarely.

I compared this function to the corresponding functions in ixgbe and igb 
and I see that they are implemented similarly.

Therefore, in my opinion, I think that it is better to leave the prints 
as they are. Or at least, to push similar changes to the other drivers 
as well.