netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20
@ 2020-04-20 23:43 Jeff Kirsher
  2020-04-20 23:43 ` [net-next 01/13] igc: Add ECN support for TSO Jeff Kirsher
                   ` (12 more replies)
  0 siblings, 13 replies; 20+ messages in thread
From: Jeff Kirsher @ 2020-04-20 23:43 UTC (permalink / raw)
  To: davem; +Cc: Jeff Kirsher, netdev, nhorman, sassmann

This series contains updates to igc driver only.

Sasha adds ECN support for TSO by adding the NETIF_F_TSO_ECN flag, which
aligns with other Intel drivers.  Also cleaned up defines that are not
supported or used in the igc driver.

Andre does most of the changes with updating the log messages for igc
driver.

Vitaly adds support for interrupt, EEPROM, register and link ethtool
self-tests.

The following are changes since commit 82ebc889091a488b4dd95e682b3c3b889a50713c:
  qed: use true,false for bool variables
and are available in the git repository at:
  git://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue 1GbE

Andre Guedes (9):
  igc: Use netdev log helpers in igc_main.c
  igc: Use netdev log helpers in igc_ethtool.c
  igc: Use netdev log helpers in igc_ptp.c
  igc: Use netdev log helpers in igc_dump.c
  igc: Use netdev log helpers in igc_base.c
  igc: Remove '\n' from log strings in igc_i225.c
  igc: Remove '\n' from log strings in igc_mac.c
  igc: Remove '\n' from log messages in igc_nvm.c
  igc: Remove '\n' from log strings in igc_phy.c

Sasha Neftin (3):
  igc: Add ECN support for TSO
  igc: Remove unneeded definition
  igc: Remove unneeded register

Vitaly Lifshits (1):
  igc: add support to interrupt, eeprom, registers and link self-tests

 drivers/net/ethernet/intel/igc/Makefile      |   2 +-
 drivers/net/ethernet/intel/igc/igc.h         |   4 +
 drivers/net/ethernet/intel/igc/igc_base.c    |  16 +-
 drivers/net/ethernet/intel/igc/igc_defines.h |   1 -
 drivers/net/ethernet/intel/igc/igc_diag.c    | 336 +++++++++++++++++++
 drivers/net/ethernet/intel/igc/igc_diag.h    |  37 ++
 drivers/net/ethernet/intel/igc/igc_dump.c    | 109 +++---
 drivers/net/ethernet/intel/igc/igc_ethtool.c |  96 +++++-
 drivers/net/ethernet/intel/igc/igc_i225.c    |  22 +-
 drivers/net/ethernet/intel/igc/igc_mac.c     |  42 +--
 drivers/net/ethernet/intel/igc/igc_main.c    | 134 ++++----
 drivers/net/ethernet/intel/igc/igc_nvm.c     |  12 +-
 drivers/net/ethernet/intel/igc/igc_phy.c     |  52 +--
 drivers/net/ethernet/intel/igc/igc_ptp.c     |  12 +-
 drivers/net/ethernet/intel/igc/igc_regs.h    |   3 +-
 15 files changed, 653 insertions(+), 225 deletions(-)
 create mode 100644 drivers/net/ethernet/intel/igc/igc_diag.c
 create mode 100644 drivers/net/ethernet/intel/igc/igc_diag.h

-- 
2.25.3


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

* [net-next 01/13] igc: Add ECN support for TSO
  2020-04-20 23:43 [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20 Jeff Kirsher
@ 2020-04-20 23:43 ` Jeff Kirsher
  2020-04-20 23:43 ` [net-next 02/13] igc: Use netdev log helpers in igc_main.c Jeff Kirsher
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Kirsher @ 2020-04-20 23:43 UTC (permalink / raw)
  To: davem; +Cc: Sasha Neftin, netdev, nhorman, sassmann, Aaron Brown, Jeff Kirsher

From: Sasha Neftin <sasha.neftin@intel.com>

Align with other Intel drivers and add ECN support for TSO.

Add NETIF_F_TSO_ECN flag

Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 9d5f8287c704..7556fcdf1fd7 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -4838,6 +4838,7 @@ static int igc_probe(struct pci_dev *pdev,
 	netdev->features |= NETIF_F_SG;
 	netdev->features |= NETIF_F_TSO;
 	netdev->features |= NETIF_F_TSO6;
+	netdev->features |= NETIF_F_TSO_ECN;
 	netdev->features |= NETIF_F_RXCSUM;
 	netdev->features |= NETIF_F_HW_CSUM;
 	netdev->features |= NETIF_F_SCTP_CRC;
-- 
2.25.3


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

* [net-next 02/13] igc: Use netdev log helpers in igc_main.c
  2020-04-20 23:43 [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20 Jeff Kirsher
  2020-04-20 23:43 ` [net-next 01/13] igc: Add ECN support for TSO Jeff Kirsher
@ 2020-04-20 23:43 ` Jeff Kirsher
  2020-04-21 17:39   ` Jakub Kicinski
  2020-04-21 19:26   ` David Miller
  2020-04-20 23:43 ` [net-next 03/13] igc: add support to interrupt, eeprom, registers and link self-tests Jeff Kirsher
                   ` (10 subsequent siblings)
  12 siblings, 2 replies; 20+ messages in thread
From: Jeff Kirsher @ 2020-04-20 23:43 UTC (permalink / raw)
  To: davem; +Cc: Andre Guedes, netdev, nhorman, sassmann, Aaron Brown, Jeff Kirsher

From: Andre Guedes <andre.guedes@intel.com>

In igc_main.c we print log messages using both dev_* and netdev_*
helpers, generating inconsistent output. Since this is a network device
driver, we should preferably use netdev_* helpers because they append
the interface name to the message, helping making sense out of the logs.

This patch converts all dev_* calls to netdev_*. There is only two
exceptions: one in igc_probe (net_device has not been allocated yet),
and another one in igc_init_module (module initialization). It also
takes this opportunity to improve some messages and remove the '\n'
character at the end of messages since it is automatically added to by
netdev_* log helpers.

Signed-off-by: Andre Guedes <andre.guedes@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 129 ++++++++++------------
 1 file changed, 59 insertions(+), 70 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 7556fcdf1fd7..c67d141def1d 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -76,7 +76,7 @@ static void igc_power_down_link(struct igc_adapter *adapter)
 
 void igc_reset(struct igc_adapter *adapter)
 {
-	struct pci_dev *pdev = adapter->pdev;
+	struct net_device *dev = adapter->netdev;
 	struct igc_hw *hw = &adapter->hw;
 	struct igc_fc_info *fc = &hw->fc;
 	u32 pba, hwm;
@@ -103,7 +103,7 @@ void igc_reset(struct igc_adapter *adapter)
 	hw->mac.ops.reset_hw(hw);
 
 	if (hw->mac.ops.init_hw(hw))
-		dev_err(&pdev->dev, "Hardware Error\n");
+		netdev_err(dev, "Error on hardware initialization");
 
 	if (!netif_running(adapter->netdev))
 		igc_power_down_link(adapter);
@@ -288,6 +288,7 @@ static void igc_clean_all_tx_rings(struct igc_adapter *adapter)
  */
 int igc_setup_tx_resources(struct igc_ring *tx_ring)
 {
+	struct net_device *ndev = tx_ring->netdev;
 	struct device *dev = tx_ring->dev;
 	int size = 0;
 
@@ -313,8 +314,7 @@ int igc_setup_tx_resources(struct igc_ring *tx_ring)
 
 err:
 	vfree(tx_ring->tx_buffer_info);
-	dev_err(dev,
-		"Unable to allocate memory for the transmit descriptor ring\n");
+	netdev_err(ndev, "Unable to allocate memory for tx descriptor ring");
 	return -ENOMEM;
 }
 
@@ -326,14 +326,13 @@ int igc_setup_tx_resources(struct igc_ring *tx_ring)
  */
 static int igc_setup_all_tx_resources(struct igc_adapter *adapter)
 {
-	struct pci_dev *pdev = adapter->pdev;
+	struct net_device *dev = adapter->netdev;
 	int i, err = 0;
 
 	for (i = 0; i < adapter->num_tx_queues; i++) {
 		err = igc_setup_tx_resources(adapter->tx_ring[i]);
 		if (err) {
-			dev_err(&pdev->dev,
-				"Allocation for Tx Queue %u failed\n", i);
+			netdev_err(dev, "Error on tx queue %u setup", i);
 			for (i--; i >= 0; i--)
 				igc_free_tx_resources(adapter->tx_ring[i]);
 			break;
@@ -444,6 +443,7 @@ static void igc_free_all_rx_resources(struct igc_adapter *adapter)
  */
 int igc_setup_rx_resources(struct igc_ring *rx_ring)
 {
+	struct net_device *ndev = rx_ring->netdev;
 	struct device *dev = rx_ring->dev;
 	int size, desc_len;
 
@@ -473,8 +473,7 @@ int igc_setup_rx_resources(struct igc_ring *rx_ring)
 err:
 	vfree(rx_ring->rx_buffer_info);
 	rx_ring->rx_buffer_info = NULL;
-	dev_err(dev,
-		"Unable to allocate memory for the receive descriptor ring\n");
+	netdev_err(ndev, "Unable to allocate memory for rx descriptor ring");
 	return -ENOMEM;
 }
 
@@ -487,14 +486,13 @@ int igc_setup_rx_resources(struct igc_ring *rx_ring)
  */
 static int igc_setup_all_rx_resources(struct igc_adapter *adapter)
 {
-	struct pci_dev *pdev = adapter->pdev;
+	struct net_device *dev = adapter->netdev;
 	int i, err = 0;
 
 	for (i = 0; i < adapter->num_rx_queues; i++) {
 		err = igc_setup_rx_resources(adapter->rx_ring[i]);
 		if (err) {
-			dev_err(&pdev->dev,
-				"Allocation for Rx Queue %u failed\n", i);
+			netdev_err(dev, "Error on rx queue %u setup", i);
 			for (i--; i >= 0; i--)
 				igc_free_rx_resources(adapter->rx_ring[i]);
 			break;
@@ -1196,7 +1194,7 @@ static int igc_tx_map(struct igc_ring *tx_ring,
 
 	return 0;
 dma_error:
-	dev_err(tx_ring->dev, "TX DMA map failed\n");
+	netdev_err(tx_ring->netdev, "TX DMA map failed");
 	tx_buffer = &tx_ring->tx_buffer_info[i];
 
 	/* clear dma mappings for failed tx_buffer_info map */
@@ -1459,8 +1457,8 @@ static void igc_rx_checksum(struct igc_ring *ring,
 				      IGC_RXD_STAT_UDPCS))
 		skb->ip_summed = CHECKSUM_UNNECESSARY;
 
-	dev_dbg(ring->dev, "cksum success: bits %08X\n",
-		le32_to_cpu(rx_desc->wb.upper.status_error));
+	netdev_dbg(ring->netdev, "cksum success: bits %08X",
+		   le32_to_cpu(rx_desc->wb.upper.status_error));
 }
 
 static inline void igc_rx_hash(struct igc_ring *ring,
@@ -2122,27 +2120,27 @@ static bool igc_clean_tx_irq(struct igc_q_vector *q_vector, int napi_budget)
 		    (adapter->tx_timeout_factor * HZ)) &&
 		    !(rd32(IGC_STATUS) & IGC_STATUS_TXOFF)) {
 			/* detected Tx unit hang */
-			dev_err(tx_ring->dev,
-				"Detected Tx Unit Hang\n"
-				"  Tx Queue             <%d>\n"
-				"  TDH                  <%x>\n"
-				"  TDT                  <%x>\n"
-				"  next_to_use          <%x>\n"
-				"  next_to_clean        <%x>\n"
-				"buffer_info[next_to_clean]\n"
-				"  time_stamp           <%lx>\n"
-				"  next_to_watch        <%p>\n"
-				"  jiffies              <%lx>\n"
-				"  desc.status          <%x>\n",
-				tx_ring->queue_index,
-				rd32(IGC_TDH(tx_ring->reg_idx)),
-				readl(tx_ring->tail),
-				tx_ring->next_to_use,
-				tx_ring->next_to_clean,
-				tx_buffer->time_stamp,
-				tx_buffer->next_to_watch,
-				jiffies,
-				tx_buffer->next_to_watch->wb.status);
+			netdev_err(tx_ring->netdev,
+				   "Detected Tx Unit Hang\n"
+				   "  Tx Queue             <%d>\n"
+				   "  TDH                  <%x>\n"
+				   "  TDT                  <%x>\n"
+				   "  next_to_use          <%x>\n"
+				   "  next_to_clean        <%x>\n"
+				   "buffer_info[next_to_clean]\n"
+				   "  time_stamp           <%lx>\n"
+				   "  next_to_watch        <%p>\n"
+				   "  jiffies              <%lx>\n"
+				   "  desc.status          <%x>\n",
+				   tx_ring->queue_index,
+				   rd32(IGC_TDH(tx_ring->reg_idx)),
+				   readl(tx_ring->tail),
+				   tx_ring->next_to_use,
+				   tx_ring->next_to_clean,
+				   tx_buffer->time_stamp,
+				   tx_buffer->next_to_watch,
+				   jiffies,
+				   tx_buffer->next_to_watch->wb.status);
 			netif_stop_subqueue(tx_ring->netdev,
 					    tx_ring->queue_index);
 
@@ -3238,14 +3236,14 @@ static int igc_alloc_q_vectors(struct igc_adapter *adapter)
  */
 static int igc_init_interrupt_scheme(struct igc_adapter *adapter, bool msix)
 {
-	struct pci_dev *pdev = adapter->pdev;
+	struct net_device *dev = adapter->netdev;
 	int err = 0;
 
 	igc_set_interrupt_capability(adapter, msix);
 
 	err = igc_alloc_q_vectors(adapter);
 	if (err) {
-		dev_err(&pdev->dev, "Unable to allocate memory for vectors\n");
+		netdev_err(dev, "Unable to allocate memory for vectors");
 		goto err_alloc_q_vectors;
 	}
 
@@ -3305,7 +3303,7 @@ static int igc_sw_init(struct igc_adapter *adapter)
 
 	/* This call may decrease the number of queues */
 	if (igc_init_interrupt_scheme(adapter, true)) {
-		dev_err(&pdev->dev, "Unable to allocate memory for queues\n");
+		netdev_err(netdev, "Unable to allocate memory for queues");
 		return -ENOMEM;
 	}
 
@@ -3619,7 +3617,7 @@ static void igc_reset_task(struct work_struct *work)
 
 	igc_rings_dump(adapter);
 	igc_regs_dump(adapter);
-	netdev_err(adapter->netdev, "Reset adapter\n");
+	netdev_err(adapter->netdev, "Reset adapter");
 	igc_reinit_locked(adapter);
 }
 
@@ -3648,8 +3646,7 @@ static int igc_change_mtu(struct net_device *netdev, int new_mtu)
 	if (netif_running(netdev))
 		igc_down(adapter);
 
-	netdev_dbg(netdev, "changing MTU from %d to %d\n",
-		   netdev->mtu, new_mtu);
+	netdev_dbg(netdev, "changing MTU from %d to %d", netdev->mtu, new_mtu);
 	netdev->mtu = new_mtu;
 
 	if (netif_running(netdev))
@@ -4006,8 +4003,7 @@ static void igc_watchdog_task(struct work_struct *work)
 			ctrl = rd32(IGC_CTRL);
 			/* Link status message must follow this format */
 			netdev_info(netdev,
-				    "igc: %s NIC Link is Up %d Mbps %s Duplex, Flow Control: %s\n",
-				    netdev->name,
+				    "NIC Link is Up %d Mbps %s Duplex, Flow Control: %s",
 				    adapter->link_speed,
 				    adapter->link_duplex == FULL_DUPLEX ?
 				    "Full" : "Half",
@@ -4019,7 +4015,7 @@ static void igc_watchdog_task(struct work_struct *work)
 			/* check if SmartSpeed worked */
 			igc_check_downshift(hw);
 			if (phy->speed_downgraded)
-				netdev_warn(netdev, "Link Speed was downgraded by SmartSpeed\n");
+				netdev_warn(netdev, "Link Speed was downgraded by SmartSpeed");
 
 			/* adjust timeout factor according to speed/duplex */
 			adapter->tx_timeout_factor = 1;
@@ -4045,10 +4041,10 @@ static void igc_watchdog_task(struct work_struct *work)
 					retry_count--;
 					goto retry_read_status;
 				} else if (!retry_count) {
-					dev_err(&adapter->pdev->dev, "exceed max 2 second\n");
+					netdev_err(netdev, "exceed max 2 second");
 				}
 			} else {
-				dev_err(&adapter->pdev->dev, "read 1000Base-T Status Reg\n");
+				netdev_err(netdev, "read 1000Base-T Status Reg");
 			}
 no_wait:
 			netif_carrier_on(netdev);
@@ -4064,8 +4060,7 @@ static void igc_watchdog_task(struct work_struct *work)
 			adapter->link_duplex = 0;
 
 			/* Links status message must follow this format */
-			netdev_info(netdev, "igc: %s NIC Link is Down\n",
-				    netdev->name);
+			netdev_info(netdev, "NIC Link is Down");
 			netif_carrier_off(netdev);
 
 			/* link state has changed, schedule phy info update */
@@ -4283,8 +4278,7 @@ static int igc_request_irq(struct igc_adapter *adapter)
 			  netdev->name, adapter);
 
 	if (err)
-		dev_err(&pdev->dev, "Error %d getting interrupt\n",
-			err);
+		netdev_err(netdev, "Error %d getting interrupt", err);
 
 request_done:
 	return err;
@@ -4676,7 +4670,7 @@ u32 igc_rd32(struct igc_hw *hw, u32 reg)
 
 		hw->hw_addr = NULL;
 		netif_device_detach(netdev);
-		netdev_err(netdev, "PCIe link lost, device now detached\n");
+		netdev_err(netdev, "PCIe link lost, device now detached");
 		WARN(pci_device_is_present(igc->pdev),
 		     "igc: Failed to read reg 0x%x!\n", reg);
 	}
@@ -4686,7 +4680,6 @@ u32 igc_rd32(struct igc_hw *hw, u32 reg)
 
 int igc_set_spd_dplx(struct igc_adapter *adapter, u32 spd, u8 dplx)
 {
-	struct pci_dev *pdev = adapter->pdev;
 	struct igc_mac_info *mac = &adapter->hw.mac;
 
 	mac->autoneg = 0;
@@ -4731,7 +4724,7 @@ int igc_set_spd_dplx(struct igc_adapter *adapter, u32 spd, u8 dplx)
 	return 0;
 
 err_inval:
-	dev_err(&pdev->dev, "Unsupported Speed/Duplex configuration\n");
+	netdev_err(adapter->netdev, "Unsupported Speed/Duplex configuration");
 	return -EINVAL;
 }
 
@@ -4767,7 +4760,7 @@ static int igc_probe(struct pci_dev *pdev,
 		err = dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32));
 		if (err) {
 			dev_err(&pdev->dev,
-				"No usable DMA configuration, aborting\n");
+				"No usable DMA configuration, aborting");
 			goto err_dma;
 		}
 	}
@@ -4877,8 +4870,7 @@ static int igc_probe(struct pci_dev *pdev,
 
 	if (igc_get_flash_presence_i225(hw)) {
 		if (hw->nvm.ops.validate(hw) < 0) {
-			dev_err(&pdev->dev,
-				"The NVM Checksum Is Not Valid\n");
+			netdev_err(netdev, "The NVM Checksum Is Not Valid");
 			err = -EIO;
 			goto err_eeprom;
 		}
@@ -4887,13 +4879,13 @@ static int igc_probe(struct pci_dev *pdev,
 	if (eth_platform_get_mac_address(&pdev->dev, hw->mac.addr)) {
 		/* copy the MAC address out of the NVM */
 		if (hw->mac.ops.read_mac_addr(hw))
-			dev_err(&pdev->dev, "NVM Read Error\n");
+			netdev_err(netdev, "NVM Read Error");
 	}
 
 	memcpy(netdev->dev_addr, hw->mac.addr, netdev->addr_len);
 
 	if (!is_valid_ether_addr(netdev->dev_addr)) {
-		dev_err(&pdev->dev, "Invalid MAC Address\n");
+		netdev_err(netdev, "Invalid MAC Address");
 		err = -EIO;
 		goto err_eeprom;
 	}
@@ -4950,7 +4942,7 @@ static int igc_probe(struct pci_dev *pdev,
 
 	/* print pcie link status and MAC address */
 	pcie_print_link_status(pdev);
-	netdev_info(netdev, "MAC: %pM\n", netdev->dev_addr);
+	netdev_info(netdev, "MAC: %pM", netdev->dev_addr);
 
 	dev_pm_set_driver_flags(&pdev->dev, DPM_FLAG_NEVER_SKIP);
 
@@ -5141,8 +5133,7 @@ static int __maybe_unused igc_resume(struct device *dev)
 		return -ENODEV;
 	err = pci_enable_device_mem(pdev);
 	if (err) {
-		dev_err(&pdev->dev,
-			"igc: Cannot enable PCI device from suspend\n");
+		netdev_err(netdev, "Cannot enable PCI device from suspend");
 		return err;
 	}
 	pci_set_master(pdev);
@@ -5151,7 +5142,7 @@ static int __maybe_unused igc_resume(struct device *dev)
 	pci_enable_wake(pdev, PCI_D3cold, 0);
 
 	if (igc_init_interrupt_scheme(adapter, true)) {
-		dev_err(&pdev->dev, "Unable to allocate memory for queues\n");
+		netdev_err(netdev, "Unable to allocate memory for queues");
 		return -ENOMEM;
 	}
 
@@ -5255,8 +5246,7 @@ static pci_ers_result_t igc_io_slot_reset(struct pci_dev *pdev)
 	pci_ers_result_t result;
 
 	if (pci_enable_device_mem(pdev)) {
-		dev_err(&pdev->dev,
-			"Could not re-enable PCI device after reset.\n");
+		netdev_err(netdev, "Could not re-enable PCI device after reset");
 		result = PCI_ERS_RESULT_DISCONNECT;
 	} else {
 		pci_set_master(pdev);
@@ -5295,7 +5285,7 @@ static void igc_io_resume(struct pci_dev *pdev)
 	rtnl_lock();
 	if (netif_running(netdev)) {
 		if (igc_open(netdev)) {
-			dev_err(&pdev->dev, "igc_open failed after reset\n");
+			netdev_err(netdev, "igc_open failed after reset");
 			return;
 		}
 	}
@@ -5342,7 +5332,6 @@ static struct pci_driver igc_driver = {
 int igc_reinit_queues(struct igc_adapter *adapter)
 {
 	struct net_device *netdev = adapter->netdev;
-	struct pci_dev *pdev = adapter->pdev;
 	int err = 0;
 
 	if (netif_running(netdev))
@@ -5351,7 +5340,7 @@ int igc_reinit_queues(struct igc_adapter *adapter)
 	igc_reset_interrupt_capability(adapter);
 
 	if (igc_init_interrupt_scheme(adapter, true)) {
-		dev_err(&pdev->dev, "Unable to allocate memory for queues\n");
+		netdev_err(netdev, "Unable to allocate memory for queues");
 		return -ENOMEM;
 	}
 
@@ -5384,10 +5373,10 @@ static int __init igc_init_module(void)
 {
 	int ret;
 
-	pr_info("%s - version %s\n",
+	pr_info("%s - version %s",
 		igc_driver_string, igc_driver_version);
 
-	pr_info("%s\n", igc_copyright);
+	pr_info("%s", igc_copyright);
 
 	ret = pci_register_driver(&igc_driver);
 	return ret;
-- 
2.25.3


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

* [net-next 03/13] igc: add support to interrupt, eeprom, registers and link self-tests
  2020-04-20 23:43 [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20 Jeff Kirsher
  2020-04-20 23:43 ` [net-next 01/13] igc: Add ECN support for TSO Jeff Kirsher
  2020-04-20 23:43 ` [net-next 02/13] igc: Use netdev log helpers in igc_main.c Jeff Kirsher
@ 2020-04-20 23:43 ` Jeff Kirsher
  2020-04-21 17:48   ` Jakub Kicinski
  2020-04-20 23:43 ` [net-next 04/13] igc: Use netdev log helpers in igc_ethtool.c Jeff Kirsher
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 20+ messages in thread
From: Jeff Kirsher @ 2020-04-20 23:43 UTC (permalink / raw)
  To: davem
  Cc: Vitaly Lifshits, netdev, nhorman, sassmann, kbuild test robot,
	Dan Carpenter, Andre Guedes, Aaron Brown, Jeff Kirsher

From: Vitaly Lifshits <vitaly.lifshits@intel.com>

Introduced igc_diag.c and igc_diag.h, these files have the
diagnostics functionality of igc driver. For the time being
these files are being used by ethtool self-test callbacks.
Which mean that interrupt, eeprom, registers and link self-tests for
ethtool were implemented.

Signed-off-by: Vitaly Lifshits <vitaly.lifshits@intel.com>
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Acked-by: Andre Guedes <andre.guedes@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/Makefile      |   2 +-
 drivers/net/ethernet/intel/igc/igc.h         |   4 +
 drivers/net/ethernet/intel/igc/igc_diag.c    | 336 +++++++++++++++++++
 drivers/net/ethernet/intel/igc/igc_diag.h    |  37 ++
 drivers/net/ethernet/intel/igc/igc_ethtool.c |  63 ++++
 drivers/net/ethernet/intel/igc/igc_main.c    |   4 +-
 drivers/net/ethernet/intel/igc/igc_regs.h    |   2 +
 7 files changed, 445 insertions(+), 3 deletions(-)
 create mode 100644 drivers/net/ethernet/intel/igc/igc_diag.c
 create mode 100644 drivers/net/ethernet/intel/igc/igc_diag.h

diff --git a/drivers/net/ethernet/intel/igc/Makefile b/drivers/net/ethernet/intel/igc/Makefile
index 3652f211f351..1c3051db9085 100644
--- a/drivers/net/ethernet/intel/igc/Makefile
+++ b/drivers/net/ethernet/intel/igc/Makefile
@@ -8,4 +8,4 @@
 obj-$(CONFIG_IGC) += igc.o
 
 igc-objs := igc_main.o igc_mac.o igc_i225.o igc_base.o igc_nvm.o igc_phy.o \
-igc_ethtool.o igc_ptp.o igc_dump.o igc_tsn.o
+igc_diag.o igc_ethtool.o igc_ptp.o igc_dump.o igc_tsn.o
diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index 8ddc39482a8e..661dc8875f3f 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -198,6 +198,8 @@ struct igc_adapter {
 	unsigned long link_check_timeout;
 	struct igc_info ei;
 
+	u32 test_icr;
+
 	struct ptp_clock *ptp_clock;
 	struct ptp_clock_info ptp_caps;
 	struct work_struct ptp_tx_work;
@@ -215,6 +217,8 @@ struct igc_adapter {
 
 void igc_up(struct igc_adapter *adapter);
 void igc_down(struct igc_adapter *adapter);
+int igc_open(struct net_device *netdev);
+int igc_close(struct net_device *netdev);
 int igc_setup_tx_resources(struct igc_ring *ring);
 int igc_setup_rx_resources(struct igc_ring *ring);
 void igc_free_tx_resources(struct igc_ring *ring);
diff --git a/drivers/net/ethernet/intel/igc/igc_diag.c b/drivers/net/ethernet/intel/igc/igc_diag.c
new file mode 100644
index 000000000000..4197ceac5d28
--- /dev/null
+++ b/drivers/net/ethernet/intel/igc/igc_diag.c
@@ -0,0 +1,336 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c)  2020 Intel Corporation */
+
+#include "igc.h"
+#include "igc_diag.h"
+
+struct igc_reg_test reg_test[] = {
+	{ IGC_FCAL,	1,	PATTERN_TEST,	0xFFFFFFFF,	0xFFFFFFFF },
+	{ IGC_FCAH,	1,	PATTERN_TEST,	0x0000FFFF,	0xFFFFFFFF },
+	{ IGC_FCT,	1,	PATTERN_TEST,	0x0000FFFF,	0xFFFFFFFF },
+	{ IGC_RDBAH(0),	4,	PATTERN_TEST,	0xFFFFFFFF,	0xFFFFFFFF },
+	{ IGC_RDBAL(0),	4,	PATTERN_TEST,	0xFFFFFF80,	0xFFFFFF80 },
+	{ IGC_RDLEN(0),	4,	PATTERN_TEST,	0x000FFF80,	0x000FFFFF },
+	{ IGC_RDT(0),	4,	PATTERN_TEST,	0x0000FFFF,	0x0000FFFF },
+	{ IGC_FCRTH,	1,	PATTERN_TEST,	0x0003FFF0,	0x0003FFF0 },
+	{ IGC_FCTTV,	1,	PATTERN_TEST,	0x0000FFFF,	0x0000FFFF },
+	{ IGC_TIPG,	1,	PATTERN_TEST,	0x3FFFFFFF,	0x3FFFFFFF },
+	{ IGC_TDBAH(0),	4,	PATTERN_TEST,	0xFFFFFFFF,	0xFFFFFFFF },
+	{ IGC_TDBAL(0),	4,	PATTERN_TEST,	0xFFFFFF80,	0xFFFFFF80 },
+	{ IGC_TDLEN(0),	4,	PATTERN_TEST,	0x000FFF80,	0x000FFFFF },
+	{ IGC_TDT(0),	4,	PATTERN_TEST,	0x0000FFFF,	0x0000FFFF },
+	{ IGC_RCTL,	1,	SET_READ_TEST,	0xFFFFFFFF,	0x00000000 },
+	{ IGC_RCTL,	1,	SET_READ_TEST,	0x04CFB2FE,	0x003FFFFB },
+	{ IGC_RCTL,	1,	SET_READ_TEST,	0x04CFB2FE,	0xFFFFFFFF },
+	{ IGC_TCTL,	1,	SET_READ_TEST,	0xFFFFFFFF,	0x00000000 },
+	{ IGC_RA,	16,	TABLE64_TEST_LO,
+						0xFFFFFFFF,	0xFFFFFFFF },
+	{ IGC_RA,	16,	TABLE64_TEST_HI,
+						0x900FFFFF,	0xFFFFFFFF },
+	{ IGC_MTA,	128,	TABLE32_TEST,
+						0xFFFFFFFF,	0xFFFFFFFF },
+	{ 0, 0, 0, 0}
+};
+
+static bool reg_pattern_test(struct igc_adapter *adapter, u64 *data, int reg,
+			     u32 mask, u32 write)
+{
+	struct igc_hw *hw = &adapter->hw;
+	u32 pat, val, before;
+	static const u32 test_pattern[] = {
+		0x5A5A5A5A, 0xA5A5A5A5, 0x00000000, 0xFFFFFFFF
+	};
+
+	for (pat = 0; pat < ARRAY_SIZE(test_pattern); pat++) {
+		before = rd32(reg);
+		wr32(reg, test_pattern[pat] & write);
+		val = rd32(reg);
+		if (val != (test_pattern[pat] & write & mask)) {
+			netdev_err(adapter->netdev,
+				   "pattern test reg %04X failed: got 0x%08X expected 0x%08X",
+				   reg, val, test_pattern[pat] & write & mask);
+			*data = reg;
+			wr32(reg, before);
+			return false;
+		}
+		wr32(reg, before);
+	}
+	return true;
+}
+
+static bool reg_set_and_check(struct igc_adapter *adapter, u64 *data, int reg,
+			      u32 mask, u32 write)
+{
+	struct igc_hw *hw = &adapter->hw;
+	u32 val, before;
+
+	before = rd32(reg);
+	wr32(reg, write & mask);
+	val = rd32(reg);
+	if ((write & mask) != (val & mask)) {
+		netdev_err(adapter->netdev,
+			   "set/check reg %04X test failed: got 0x%08X expected 0x%08X",
+			   reg, (val & mask), (write & mask));
+		*data = reg;
+		wr32(reg, before);
+		return false;
+	}
+	wr32(reg, before);
+	return true;
+}
+
+bool igc_reg_test(struct igc_adapter *adapter, u64 *data)
+{
+	struct igc_reg_test *test = reg_test;
+	struct igc_hw *hw = &adapter->hw;
+	u32 value, before, after;
+	u32 i, toggle, b = false;
+
+	/* Because the status register is such a special case,
+	 * we handle it separately from the rest of the register
+	 * tests.  Some bits are read-only, some toggle, and some
+	 * are writeable.
+	 */
+	toggle = 0x6800D3;
+	before = rd32(IGC_STATUS);
+	value = before & toggle;
+	wr32(IGC_STATUS, toggle);
+	after = rd32(IGC_STATUS) & toggle;
+	if (value != after) {
+		netdev_err(adapter->netdev,
+			   "failed STATUS register test got: 0x%08X expected: 0x%08X",
+			   after, value);
+		*data = 1;
+		return false;
+	}
+	/* restore previous status */
+	wr32(IGC_STATUS, before);
+
+	/* Perform the remainder of the register test, looping through
+	 * the test table until we either fail or reach the null entry.
+	 */
+	while (test->reg) {
+		for (i = 0; i < test->array_len; i++) {
+			switch (test->test_type) {
+			case PATTERN_TEST:
+				b = reg_pattern_test(adapter, data,
+						     test->reg + (i * 0x40),
+						     test->mask,
+						     test->write);
+				break;
+			case SET_READ_TEST:
+				b = reg_set_and_check(adapter, data,
+						      test->reg + (i * 0x40),
+						      test->mask,
+						      test->write);
+				break;
+			case TABLE64_TEST_LO:
+				b = reg_pattern_test(adapter, data,
+						     test->reg + (i * 8),
+						     test->mask,
+						     test->write);
+				break;
+			case TABLE64_TEST_HI:
+				b = reg_pattern_test(adapter, data,
+						     test->reg + 4 + (i * 8),
+						     test->mask,
+						     test->write);
+				break;
+			case TABLE32_TEST:
+				b = reg_pattern_test(adapter, data,
+						     test->reg + (i * 4),
+						     test->mask,
+						     test->write);
+				break;
+			}
+			if (!b)
+				return false;
+		}
+		test++;
+	}
+	*data = 0;
+	return true;
+}
+
+bool igc_eeprom_test(struct igc_adapter *adapter, u64 *data)
+{
+	struct igc_hw *hw = &adapter->hw;
+
+	*data = 0;
+
+	if (hw->nvm.ops.validate(hw) != IGC_SUCCESS) {
+		*data = 1;
+		return false;
+	}
+
+	return true;
+}
+
+static irqreturn_t igc_test_intr(int irq, void *data)
+{
+	struct igc_adapter *adapter = (struct igc_adapter *)data;
+	struct igc_hw *hw = &adapter->hw;
+
+	adapter->test_icr |= rd32(IGC_ICR);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t igc_test_intr_msix(int irq, void *data)
+{
+	struct igc_adapter *adapter = (struct igc_adapter *)data;
+	struct igc_hw *hw = &adapter->hw;
+
+	adapter->test_icr |= rd32(IGC_EICR);
+
+	return IRQ_HANDLED;
+}
+
+bool igc_intr_test(struct igc_adapter *adapter, u64 *data)
+{
+	struct igc_hw *hw = &adapter->hw;
+	struct net_device *netdev = adapter->netdev;
+	u32 mask, ics_mask = IGC_ICS_MASK_OTHER, i = 0, shared_int = true;
+	u32 irq = adapter->pdev->irq;
+
+	*data = 0;
+
+	/* Hook up test interrupt handler just for this test */
+	if (adapter->msix_entries) {
+		if (request_irq(adapter->msix_entries[0].vector,
+				&igc_test_intr_msix, 0,
+				netdev->name, adapter)) {
+			*data = 1;
+			return false;
+		}
+		ics_mask = IGC_ICS_MASK_MSIX;
+	} else if (adapter->flags & IGC_FLAG_HAS_MSI) {
+		shared_int = false;
+		if (request_irq(irq,
+				igc_test_intr, 0, netdev->name, adapter)) {
+			*data = 1;
+			return false;
+		}
+	} else if (!request_irq(irq, igc_test_intr, IRQF_PROBE_SHARED,
+				netdev->name, adapter)) {
+		shared_int = false;
+	} else if (request_irq(irq, &igc_test_intr, IRQF_SHARED,
+		 netdev->name, adapter)) {
+		*data = 1;
+		return false;
+	}
+	netdev_info(adapter->netdev, "testing %s interrupt",
+		    (shared_int ? "shared" : "unshared"));
+
+	/* Disable all the interrupts */
+	wr32(IGC_IMC, ~0);
+	wr32(IGC_EIMC, ~0);
+	wrfl();
+	usleep_range(10000, 20000);
+
+	/* Test each interrupt */
+	for (; i < 31; i++) {
+		/* Interrupt to test */
+		mask = BIT(i);
+
+		if (!(mask & ics_mask))
+			continue;
+
+		if (!shared_int) {
+			/* Disable the interrupt to be reported in
+			 * the cause register and then force the same
+			 * interrupt and see if one gets posted.  If
+			 * an interrupt was posted to the bus, the
+			 * test failed.
+			 */
+			adapter->test_icr = 0;
+
+			/* Flush any pending interrupts */
+			wr32(IGC_ICR, ~0);
+
+			wr32(IGC_IMC, mask);
+			wr32(IGC_ICS, mask);
+			wrfl();
+			usleep_range(10000, 20000);
+
+			if (adapter->test_icr & mask) {
+				*data = 3;
+				break;
+			}
+		}
+
+		/* Enable the interrupt to be reported in
+		 * the cause register and then force the same
+		 * interrupt and see if one gets posted.  If
+		 * an interrupt was not posted to the bus, the
+		 * test failed.
+		 */
+		adapter->test_icr = 0;
+
+		wr32(IGC_EIMS, mask);
+		wr32(IGC_EICS, mask);
+		wrfl();
+		usleep_range(10000, 20000);
+
+		if (!(adapter->test_icr & mask)) {
+			*data = 4;
+			break;
+		}
+
+		if (!shared_int) {
+			/* Disable the other interrupts to be reported in
+			 * the cause register and then force the other
+			 * interrupts and see if any get posted.  If
+			 * an interrupt was posted to the bus, the
+			 * test failed.
+			 */
+			adapter->test_icr = 0;
+
+			/* Flush any pending interrupts */
+			wr32(IGC_ICR, ~0);
+
+			wr32(IGC_IMC, ~mask);
+			wr32(IGC_ICS, ~mask);
+			wrfl();
+			usleep_range(10000, 20000);
+
+			if (adapter->test_icr & mask) {
+				*data = 5;
+				break;
+			}
+		}
+	}
+
+	/* Disable all the interrupts */
+	wr32(IGC_EIMC, ~0);
+	wr32(IGC_IMC, ~0);
+	wrfl();
+	usleep_range(10000, 20000);
+
+	/* Unhook test interrupt handler */
+	if (adapter->msix_entries)
+		free_irq(adapter->msix_entries[0].vector, adapter);
+	else
+		free_irq(irq, adapter);
+
+	return true;
+}
+
+bool igc_link_test(struct igc_adapter *adapter, u64 *data)
+{
+	bool link_up;
+
+	*data = 0;
+
+	/* add delay to give enough time for autonegotioation to finish */
+	if (adapter->hw.mac.autoneg)
+		ssleep(5);
+
+	link_up = igc_has_link(adapter);
+	if (!link_up) {
+		*data = 1;
+		return false;
+	}
+
+	return true;
+}
diff --git a/drivers/net/ethernet/intel/igc/igc_diag.h b/drivers/net/ethernet/intel/igc/igc_diag.h
new file mode 100644
index 000000000000..3cffaad01d50
--- /dev/null
+++ b/drivers/net/ethernet/intel/igc/igc_diag.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* Copyright (c)  2020 Intel Corporation */
+
+bool igc_reg_test(struct igc_adapter *adapter, u64 *data);
+bool igc_eeprom_test(struct igc_adapter *adapter, u64 *data);
+bool igc_intr_test(struct igc_adapter *adapter, u64 *data);
+bool igc_link_test(struct igc_adapter *adapter, u64 *data);
+
+struct igc_reg_test {
+	u16 reg;
+	u8 array_len;
+	u8 test_type;
+	u32 mask;
+	u32 write;
+};
+
+/* In the hardware, registers are laid out either singly, in arrays
+ * spaced 0x40 bytes apart, or in contiguous tables.  We assume
+ * most tests take place on arrays or single registers (handled
+ * as a single-element array) and special-case the tables.
+ * Table tests are always pattern tests.
+ *
+ * We also make provision for some required setup steps by specifying
+ * registers to be written without any read-back testing.
+ */
+
+#define PATTERN_TEST	1
+#define SET_READ_TEST	2
+#define TABLE32_TEST	3
+#define TABLE64_TEST_LO	4
+#define TABLE64_TEST_HI	5
+
+/* For interrupt test we are using different registers
+ * and masks for msi-x interrupts and the other methods
+ */
+#define IGC_ICS_MASK_OTHER	0x774CFED5
+#define IGC_ICS_MASK_MSIX	0xF
diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index 0a8c4a7412a4..c14196663ebb 100644
--- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
+++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
@@ -6,6 +6,7 @@
 #include <linux/pm_runtime.h>
 
 #include "igc.h"
+#include "igc_diag.h"
 
 /* forward declaration */
 struct igc_stats {
@@ -1896,6 +1897,67 @@ static int igc_set_link_ksettings(struct net_device *netdev,
 	return 0;
 }
 
+static void igc_diag_test(struct net_device *netdev,
+			  struct ethtool_test *eth_test, u64 *data)
+{
+	struct igc_adapter *adapter = netdev_priv(netdev);
+	bool if_running = netif_running(netdev);
+
+	if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
+		netdev_info(adapter->netdev, "offline testing starting");
+		set_bit(__IGC_TESTING, &adapter->state);
+
+		/* Link test performed before hardware reset so autoneg doesn't
+		 * interfere with test result
+		 */
+		if (!igc_link_test(adapter, &data[TEST_LINK]))
+			eth_test->flags |= ETH_TEST_FL_FAILED;
+
+		if (if_running)
+			igc_close(netdev);
+		else
+			igc_reset(adapter);
+
+		netdev_info(adapter->netdev, "register testing starting");
+		if (!igc_reg_test(adapter, &data[TEST_REG]))
+			eth_test->flags |= ETH_TEST_FL_FAILED;
+
+		igc_reset(adapter);
+
+		netdev_info(adapter->netdev, "eeprom testing starting");
+		if (!igc_eeprom_test(adapter, &data[TEST_EEP]))
+			eth_test->flags |= ETH_TEST_FL_FAILED;
+
+		igc_reset(adapter);
+
+		netdev_info(adapter->netdev, "interrupt testing starting");
+		if (!igc_intr_test(adapter, &data[TEST_IRQ]))
+			eth_test->flags |= ETH_TEST_FL_FAILED;
+
+		igc_reset(adapter);
+
+		/* loopback test will be implemented in the future */
+		data[TEST_LOOP] = 0;
+
+		clear_bit(__IGC_TESTING, &adapter->state);
+		if (if_running)
+			igc_open(netdev);
+	} else {
+		netdev_info(adapter->netdev, "online testing starting");
+
+		/* register, eeprom, intr and loopback tests not run online */
+		data[TEST_REG] = 0;
+		data[TEST_EEP] = 0;
+		data[TEST_IRQ] = 0;
+		data[TEST_LOOP] = 0;
+
+		if (!igc_link_test(adapter, &data[TEST_LINK]))
+			eth_test->flags |= ETH_TEST_FL_FAILED;
+	}
+
+	msleep_interruptible(4 * 1000);
+}
+
 static const struct ethtool_ops igc_ethtool_ops = {
 	.supported_coalesce_params = ETHTOOL_COALESCE_USECS,
 	.get_drvinfo		= igc_get_drvinfo,
@@ -1933,6 +1995,7 @@ static const struct ethtool_ops igc_ethtool_ops = {
 	.complete		= igc_ethtool_complete,
 	.get_link_ksettings	= igc_get_link_ksettings,
 	.set_link_ksettings	= igc_set_link_ksettings,
+	.self_test		= igc_diag_test,
 };
 
 void igc_set_ethtool_ops(struct net_device *netdev)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index c67d141def1d..ecf074093a42 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -4380,7 +4380,7 @@ static int __igc_open(struct net_device *netdev, bool resuming)
 	return err;
 }
 
-static int igc_open(struct net_device *netdev)
+int igc_open(struct net_device *netdev)
 {
 	return __igc_open(netdev, false);
 }
@@ -4422,7 +4422,7 @@ static int __igc_close(struct net_device *netdev, bool suspending)
 	return 0;
 }
 
-static int igc_close(struct net_device *netdev)
+int igc_close(struct net_device *netdev)
 {
 	if (netif_device_present(netdev) || netdev->dismantle)
 		return __igc_close(netdev, false);
diff --git a/drivers/net/ethernet/intel/igc/igc_regs.h b/drivers/net/ethernet/intel/igc/igc_regs.h
index 6093cde2351c..633545977a65 100644
--- a/drivers/net/ethernet/intel/igc/igc_regs.h
+++ b/drivers/net/ethernet/intel/igc/igc_regs.h
@@ -49,6 +49,7 @@
 #define IGC_FACTPS		0x05B30
 
 /* Interrupt Register Description */
+#define IGC_EICR		0x01580  /* Ext. Interrupt Cause read - W0 */
 #define IGC_EICS		0x01520  /* Ext. Interrupt Cause Set - W0 */
 #define IGC_EIMS		0x01524  /* Ext. Interrupt Mask Set/Read - RW */
 #define IGC_EIMC		0x01528  /* Ext. Interrupt Mask Clear - WO */
@@ -119,6 +120,7 @@
 #define IGC_RLPML		0x05004  /* Rx Long Packet Max Length */
 #define IGC_RFCTL		0x05008  /* Receive Filter Control*/
 #define IGC_MTA			0x05200  /* Multicast Table Array - RW Array */
+#define IGC_RA			0x05400  /* Receive Address - RW Array */
 #define IGC_UTA			0x0A000  /* Unicast Table Array - RW */
 #define IGC_RAL(_n)		(0x05400 + ((_n) * 0x08))
 #define IGC_RAH(_n)		(0x05404 + ((_n) * 0x08))
-- 
2.25.3


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

* [net-next 04/13] igc: Use netdev log helpers in igc_ethtool.c
  2020-04-20 23:43 [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20 Jeff Kirsher
                   ` (2 preceding siblings ...)
  2020-04-20 23:43 ` [net-next 03/13] igc: add support to interrupt, eeprom, registers and link self-tests Jeff Kirsher
@ 2020-04-20 23:43 ` Jeff Kirsher
  2020-04-20 23:43 ` [net-next 05/13] igc: Use netdev log helpers in igc_ptp.c Jeff Kirsher
                   ` (8 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Kirsher @ 2020-04-20 23:43 UTC (permalink / raw)
  To: davem; +Cc: Andre Guedes, netdev, nhorman, sassmann, Aaron Brown, Jeff Kirsher

From: Andre Guedes <andre.guedes@intel.com>

In igc_ethtool.c we print log messages using dev_* helpers, generating
inconsistent output with the rest of the driver. Since this is a network
device driver, we should preferably use netdev_* helpers because they
append the interface name to the message, helping making sense the of
the logs.

This patch converts all dev_* calls to netdev_*. It also takes this
opportunity to remove the '\n' character at the end of messages since it
is automatically added by netdev_* log helpers.

Signed-off-by: Andre Guedes <andre.guedes@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_ethtool.c | 43 ++++++++++----------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index c14196663ebb..be6b1cbff926 100644
--- a/drivers/net/ethernet/intel/igc/igc_ethtool.c
+++ b/drivers/net/ethernet/intel/igc/igc_ethtool.c
@@ -1155,8 +1155,8 @@ static int igc_set_rss_hash_opt(struct igc_adapter *adapter,
 
 		if ((flags & UDP_RSS_FLAGS) &&
 		    !(adapter->flags & UDP_RSS_FLAGS))
-			dev_err(&adapter->pdev->dev,
-				"enabling UDP RSS: fragmented packets may arrive out of order to the stack above\n");
+			netdev_err(adapter->netdev,
+				   "Enabling UDP RSS: fragmented packets may arrive out of order to the stack above");
 
 		adapter->flags = flags;
 
@@ -1195,7 +1195,8 @@ static int igc_rxnfc_write_etype_filter(struct igc_adapter *adapter,
 			break;
 	}
 	if (i == MAX_ETYPE_FILTER) {
-		dev_err(&adapter->pdev->dev, "ethtool -N: etype filters are all used.\n");
+		netdev_err(adapter->netdev,
+			   "ethtool -N: etype filters are all used");
 		return -EINVAL;
 	}
 
@@ -1236,7 +1237,8 @@ static int igc_rxnfc_write_vlan_prio_filter(struct igc_adapter *adapter,
 	/* check whether this vlan prio is already set */
 	if (vlapqf & IGC_VLAPQF_P_VALID(vlan_priority) &&
 	    queue_index != input->action) {
-		dev_err(&adapter->pdev->dev, "ethtool rxnfc set vlan prio filter failed.\n");
+		netdev_err(adapter->netdev,
+			   "ethtool rxnfc set vlan prio filter failed");
 		return -EEXIST;
 	}
 
@@ -1255,8 +1257,8 @@ int igc_add_filter(struct igc_adapter *adapter, struct igc_nfc_filter *input)
 
 	if (hw->mac.type == igc_i225 &&
 	    !(input->filter.match_flags & ~IGC_FILTER_FLAG_SRC_MAC_ADDR)) {
-		dev_err(&adapter->pdev->dev,
-			"i225 doesn't support flow classification rules specifying only source addresses.\n");
+		netdev_err(adapter->netdev,
+			   "i225 doesn't support flow classification rules specifying only source addresses");
 		return -EOPNOTSUPP;
 	}
 
@@ -1404,13 +1406,14 @@ static int igc_add_ethtool_nfc_entry(struct igc_adapter *adapter,
 	 */
 	if (fsp->ring_cookie == RX_CLS_FLOW_DISC ||
 	    fsp->ring_cookie >= adapter->num_rx_queues) {
-		dev_err(&adapter->pdev->dev, "ethtool -N: The specified action is invalid\n");
+		netdev_err(netdev,
+			   "ethtool -N: The specified action is invalid");
 		return -EINVAL;
 	}
 
 	/* Don't allow indexes to exist outside of available space */
 	if (fsp->location >= IGC_MAX_RXNFC_FILTERS) {
-		dev_err(&adapter->pdev->dev, "Location out of range\n");
+		netdev_err(netdev, "Location out of range");
 		return -EINVAL;
 	}
 
@@ -1458,8 +1461,8 @@ static int igc_add_ethtool_nfc_entry(struct igc_adapter *adapter,
 		if (!memcmp(&input->filter, &rule->filter,
 			    sizeof(input->filter))) {
 			err = -EEXIST;
-			dev_err(&adapter->pdev->dev,
-				"ethtool: this filter is already set\n");
+			netdev_err(netdev,
+				   "ethtool: this filter is already set");
 			goto err_out_w_lock;
 		}
 	}
@@ -1832,6 +1835,7 @@ static int igc_set_link_ksettings(struct net_device *netdev,
 				  const struct ethtool_link_ksettings *cmd)
 {
 	struct igc_adapter *adapter = netdev_priv(netdev);
+	struct net_device *dev = adapter->netdev;
 	struct igc_hw *hw = &adapter->hw;
 	u32 advertising;
 
@@ -1839,8 +1843,7 @@ static int igc_set_link_ksettings(struct net_device *netdev,
 	 * cannot be changed
 	 */
 	if (igc_check_reset_block(hw)) {
-		dev_err(&adapter->pdev->dev,
-			"Cannot change link characteristics when reset is active.\n");
+		netdev_err(dev, "Cannot change link characteristics when reset is active");
 		return -EINVAL;
 	}
 
@@ -1851,7 +1854,7 @@ static int igc_set_link_ksettings(struct net_device *netdev,
 	if (cmd->base.eth_tp_mdix_ctrl) {
 		if (cmd->base.eth_tp_mdix_ctrl != ETH_TP_MDI_AUTO &&
 		    cmd->base.autoneg != AUTONEG_ENABLE) {
-			dev_err(&adapter->pdev->dev, "forcing MDI/MDI-X state is not supported when link speed and/or duplex are forced\n");
+			netdev_err(dev, "Forcing MDI/MDI-X state is not supported when link speed and/or duplex are forced");
 			return -EINVAL;
 		}
 	}
@@ -1868,9 +1871,7 @@ static int igc_set_link_ksettings(struct net_device *netdev,
 		if (adapter->fc_autoneg)
 			hw->fc.requested_mode = igc_fc_default;
 	} else {
-		/* calling this overrides forced MDI setting */
-		dev_info(&adapter->pdev->dev,
-			 "Force mode currently not supported\n");
+		netdev_info(dev, "Force mode currently not supported");
 	}
 
 	/* MDI-X => 2; MDI => 1; Auto => 3 */
@@ -1904,7 +1905,7 @@ static void igc_diag_test(struct net_device *netdev,
 	bool if_running = netif_running(netdev);
 
 	if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
-		netdev_info(adapter->netdev, "offline testing starting");
+		netdev_info(adapter->netdev, "Offline testing starting");
 		set_bit(__IGC_TESTING, &adapter->state);
 
 		/* Link test performed before hardware reset so autoneg doesn't
@@ -1918,19 +1919,19 @@ static void igc_diag_test(struct net_device *netdev,
 		else
 			igc_reset(adapter);
 
-		netdev_info(adapter->netdev, "register testing starting");
+		netdev_info(adapter->netdev, "Register testing starting");
 		if (!igc_reg_test(adapter, &data[TEST_REG]))
 			eth_test->flags |= ETH_TEST_FL_FAILED;
 
 		igc_reset(adapter);
 
-		netdev_info(adapter->netdev, "eeprom testing starting");
+		netdev_info(adapter->netdev, "EEPROM testing starting");
 		if (!igc_eeprom_test(adapter, &data[TEST_EEP]))
 			eth_test->flags |= ETH_TEST_FL_FAILED;
 
 		igc_reset(adapter);
 
-		netdev_info(adapter->netdev, "interrupt testing starting");
+		netdev_info(adapter->netdev, "Interrupt testing starting");
 		if (!igc_intr_test(adapter, &data[TEST_IRQ]))
 			eth_test->flags |= ETH_TEST_FL_FAILED;
 
@@ -1943,7 +1944,7 @@ static void igc_diag_test(struct net_device *netdev,
 		if (if_running)
 			igc_open(netdev);
 	} else {
-		netdev_info(adapter->netdev, "online testing starting");
+		netdev_info(adapter->netdev, "Online testing starting");
 
 		/* register, eeprom, intr and loopback tests not run online */
 		data[TEST_REG] = 0;
-- 
2.25.3


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

* [net-next 05/13] igc: Use netdev log helpers in igc_ptp.c
  2020-04-20 23:43 [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20 Jeff Kirsher
                   ` (3 preceding siblings ...)
  2020-04-20 23:43 ` [net-next 04/13] igc: Use netdev log helpers in igc_ethtool.c Jeff Kirsher
@ 2020-04-20 23:43 ` Jeff Kirsher
  2020-04-20 23:43 ` [net-next 06/13] igc: Use netdev log helpers in igc_dump.c Jeff Kirsher
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Kirsher @ 2020-04-20 23:43 UTC (permalink / raw)
  To: davem; +Cc: Andre Guedes, netdev, nhorman, sassmann, Aaron Brown, Jeff Kirsher

From: Andre Guedes <andre.guedes@intel.com>

In igc_ptp.c we print log messages using dev_* helpers, generating
inconsistent output with the rest of the driver. Since this is a network
device driver, we should preferably use netdev_* helpers because they
append the interface name to the message, helping making sense out of
the logs.

This patch converts all dev_* calls to netdev_*. It also takes this
opportunity to remove the '\n' character at the end of messages since it
is automatically added by netdev_* log helpers.

Signed-off-by: Andre Guedes <andre.guedes@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_ptp.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_ptp.c b/drivers/net/ethernet/intel/igc/igc_ptp.c
index f99c514ad0f4..c292aaf7c79d 100644
--- a/drivers/net/ethernet/intel/igc/igc_ptp.c
+++ b/drivers/net/ethernet/intel/igc/igc_ptp.c
@@ -466,7 +466,7 @@ void igc_ptp_tx_hang(struct igc_adapter *adapter)
 		 * interrupt
 		 */
 		rd32(IGC_TXSTMPH);
-		dev_warn(&adapter->pdev->dev, "clearing Tx timestamp hang\n");
+		netdev_warn(adapter->netdev, "Clearing Tx timestamp hang");
 	}
 }
 
@@ -529,7 +529,7 @@ static void igc_ptp_tx_work(struct work_struct *work)
 		 * interrupt
 		 */
 		rd32(IGC_TXSTMPH);
-		dev_warn(&adapter->pdev->dev, "clearing Tx timestamp hang\n");
+		netdev_warn(adapter->netdev, "Clearing Tx timestamp hang");
 		return;
 	}
 
@@ -626,10 +626,9 @@ void igc_ptp_init(struct igc_adapter *adapter)
 						&adapter->pdev->dev);
 	if (IS_ERR(adapter->ptp_clock)) {
 		adapter->ptp_clock = NULL;
-		dev_err(&adapter->pdev->dev, "ptp_clock_register failed\n");
+		netdev_err(netdev, "ptp_clock_register failed");
 	} else if (adapter->ptp_clock) {
-		dev_info(&adapter->pdev->dev, "added PHC on %s\n",
-			 adapter->netdev->name);
+		netdev_info(netdev, "PHC added");
 		adapter->ptp_flags |= IGC_PTP_ENABLED;
 	}
 }
@@ -666,8 +665,7 @@ void igc_ptp_stop(struct igc_adapter *adapter)
 
 	if (adapter->ptp_clock) {
 		ptp_clock_unregister(adapter->ptp_clock);
-		dev_info(&adapter->pdev->dev, "removed PHC on %s\n",
-			 adapter->netdev->name);
+		netdev_info(adapter->netdev, "PHC removed");
 		adapter->ptp_flags &= ~IGC_PTP_ENABLED;
 	}
 }
-- 
2.25.3


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

* [net-next 06/13] igc: Use netdev log helpers in igc_dump.c
  2020-04-20 23:43 [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20 Jeff Kirsher
                   ` (4 preceding siblings ...)
  2020-04-20 23:43 ` [net-next 05/13] igc: Use netdev log helpers in igc_ptp.c Jeff Kirsher
@ 2020-04-20 23:43 ` Jeff Kirsher
  2020-04-20 23:43 ` [net-next 07/13] igc: Use netdev log helpers in igc_base.c Jeff Kirsher
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Kirsher @ 2020-04-20 23:43 UTC (permalink / raw)
  To: davem; +Cc: Andre Guedes, netdev, nhorman, sassmann, Aaron Brown, Jeff Kirsher

From: Andre Guedes <andre.guedes@intel.com>

In igc_dump.c we print log messages using dev_* and pr_* helpers,
generating inconsistent output with the rest of the driver. Since this
is a network device driver, we should preferably use netdev_* helpers
because they append the interface name to the message, helping making
sense out of the logs.

This patch converts all dev_* and pr_* calls to netdev_*. It also takes
this opportunity to remove the '\n' character at the end of messages
since is it automatically added by netdev_* log helpers.

Quick note about igc_rings_dump(): This function is always called with
valid adapter->netdev so there is not need to check it.

Signed-off-by: Andre Guedes <andre.guedes@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_dump.c | 109 +++++++++++-----------
 1 file changed, 54 insertions(+), 55 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_dump.c b/drivers/net/ethernet/intel/igc/igc_dump.c
index 657ab50ae296..d6990d63c8bb 100644
--- a/drivers/net/ethernet/intel/igc/igc_dump.c
+++ b/drivers/net/ethernet/intel/igc/igc_dump.c
@@ -47,6 +47,7 @@ static const struct igc_reg_info igc_reg_info_tbl[] = {
 /* igc_regdump - register printout routine */
 static void igc_regdump(struct igc_hw *hw, struct igc_reg_info *reginfo)
 {
+	struct net_device *dev = igc_get_hw_dev(hw);
 	int n = 0;
 	char rname[16];
 	u32 regs[8];
@@ -101,13 +102,14 @@ static void igc_regdump(struct igc_hw *hw, struct igc_reg_info *reginfo)
 			regs[n] = rd32(IGC_TXDCTL(n));
 		break;
 	default:
-		pr_info("%-15s %08x\n", reginfo->name, rd32(reginfo->ofs));
+		netdev_info(dev, "%-15s %08x", reginfo->name,
+			    rd32(reginfo->ofs));
 		return;
 	}
 
 	snprintf(rname, 16, "%s%s", reginfo->name, "[0-3]");
-	pr_info("%-15s %08x %08x %08x %08x\n", rname, regs[0], regs[1],
-		regs[2], regs[3]);
+	netdev_info(dev, "%-15s %08x %08x %08x %08x", rname, regs[0], regs[1],
+		    regs[2], regs[3]);
 }
 
 /* igc_rings_dump - Tx-rings and Rx-rings */
@@ -125,39 +127,34 @@ void igc_rings_dump(struct igc_adapter *adapter)
 	if (!netif_msg_hw(adapter))
 		return;
 
-	/* Print netdevice Info */
-	if (netdev) {
-		dev_info(&adapter->pdev->dev, "Net device Info\n");
-		pr_info("Device Name     state            trans_start\n");
-		pr_info("%-15s %016lX %016lX\n", netdev->name,
-			netdev->state, dev_trans_start(netdev));
-	}
+	netdev_info(netdev, "Device info: state %016lX trans_start %016lX",
+		    netdev->state, dev_trans_start(netdev));
 
 	/* Print TX Ring Summary */
-	if (!netdev || !netif_running(netdev))
+	if (!netif_running(netdev))
 		goto exit;
 
-	dev_info(&adapter->pdev->dev, "TX Rings Summary\n");
-	pr_info("Queue [NTU] [NTC] [bi(ntc)->dma  ] leng ntw timestamp\n");
+	netdev_info(netdev, "TX Rings Summary");
+	netdev_info(netdev, "Queue [NTU] [NTC] [bi(ntc)->dma  ] leng ntw timestamp");
 	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];
 
-		pr_info(" %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),
-			buffer_info->next_to_watch,
-			(u64)buffer_info->time_stamp);
+		netdev_info(netdev, "%5d %5X %5X %016llX %04X %p %016llX",
+			    n, tx_ring->next_to_use, tx_ring->next_to_clean,
+			    (u64)dma_unmap_addr(buffer_info, dma),
+			    dma_unmap_len(buffer_info, len),
+			    buffer_info->next_to_watch,
+			    (u64)buffer_info->time_stamp);
 	}
 
 	/* Print TX Rings */
 	if (!netif_msg_tx_done(adapter))
 		goto rx_ring_summary;
 
-	dev_info(&adapter->pdev->dev, "TX Rings Dump\n");
+	netdev_info(netdev, "TX Rings Dump");
 
 	/* Transmit Descriptor Formats
 	 *
@@ -172,10 +169,11 @@ void igc_rings_dump(struct igc_adapter *adapter)
 
 	for (n = 0; n < adapter->num_tx_queues; n++) {
 		tx_ring = adapter->tx_ring[n];
-		pr_info("------------------------------------\n");
-		pr_info("TX QUEUE INDEX = %d\n", tx_ring->queue_index);
-		pr_info("------------------------------------\n");
-		pr_info("T [desc]     [address 63:0  ] [PlPOCIStDDM Ln] [bi->dma       ] leng  ntw timestamp        bi->skb\n");
+		netdev_info(netdev, "------------------------------------");
+		netdev_info(netdev, "TX QUEUE INDEX = %d",
+			    tx_ring->queue_index);
+		netdev_info(netdev, "------------------------------------");
+		netdev_info(netdev, "T [desc]     [address 63:0  ] [PlPOCIStDDM Ln] [bi->dma       ] leng  ntw timestamp        bi->skb");
 
 		for (i = 0; tx_ring->desc && (i < tx_ring->count); i++) {
 			const char *next_desc;
@@ -194,14 +192,14 @@ void igc_rings_dump(struct igc_adapter *adapter)
 			else
 				next_desc = "";
 
-			pr_info("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),
-				dma_unmap_len(buffer_info, len),
-				buffer_info->next_to_watch,
-				(u64)buffer_info->time_stamp,
-				buffer_info->skb, next_desc);
+			netdev_info(netdev, "T [0x%03X]    %016llX %016llX %016llX %04X  %p %016llX %p%s",
+				    i, le64_to_cpu(u0->a),
+				    le64_to_cpu(u0->b),
+				    (u64)dma_unmap_addr(buffer_info, dma),
+				    dma_unmap_len(buffer_info, len),
+				    buffer_info->next_to_watch,
+				    (u64)buffer_info->time_stamp,
+				    buffer_info->skb, next_desc);
 
 			if (netif_msg_pktdata(adapter) && buffer_info->skb)
 				print_hex_dump(KERN_INFO, "",
@@ -214,19 +212,19 @@ void igc_rings_dump(struct igc_adapter *adapter)
 
 	/* Print RX Rings Summary */
 rx_ring_summary:
-	dev_info(&adapter->pdev->dev, "RX Rings Summary\n");
-	pr_info("Queue [NTU] [NTC]\n");
+	netdev_info(netdev, "RX Rings Summary");
+	netdev_info(netdev, "Queue [NTU] [NTC]");
 	for (n = 0; n < adapter->num_rx_queues; n++) {
 		rx_ring = adapter->rx_ring[n];
-		pr_info(" %5d %5X %5X\n",
-			n, rx_ring->next_to_use, rx_ring->next_to_clean);
+		netdev_info(netdev, "%5d %5X %5X", n, rx_ring->next_to_use,
+			    rx_ring->next_to_clean);
 	}
 
 	/* Print RX Rings */
 	if (!netif_msg_rx_status(adapter))
 		goto exit;
 
-	dev_info(&adapter->pdev->dev, "RX Rings Dump\n");
+	netdev_info(netdev, "RX Rings Dump");
 
 	/* Advanced Receive Descriptor (Read) Format
 	 *    63                                           1        0
@@ -251,11 +249,12 @@ void igc_rings_dump(struct igc_adapter *adapter)
 
 	for (n = 0; n < adapter->num_rx_queues; n++) {
 		rx_ring = adapter->rx_ring[n];
-		pr_info("------------------------------------\n");
-		pr_info("RX QUEUE INDEX = %d\n", rx_ring->queue_index);
-		pr_info("------------------------------------\n");
-		pr_info("R  [desc]      [ PktBuf     A0] [  HeadBuf   DD] [bi->dma       ] [bi->skb] <-- Adv Rx Read format\n");
-		pr_info("RWB[desc]      [PcsmIpSHl PtRs] [vl er S cks ln] ---------------- [bi->skb] <-- Adv Rx Write-Back format\n");
+		netdev_info(netdev, "------------------------------------");
+		netdev_info(netdev, "RX QUEUE INDEX = %d",
+			    rx_ring->queue_index);
+		netdev_info(netdev, "------------------------------------");
+		netdev_info(netdev, "R  [desc]      [ PktBuf     A0] [  HeadBuf   DD] [bi->dma       ] [bi->skb] <-- Adv Rx Read format");
+		netdev_info(netdev, "RWB[desc]      [PcsmIpSHl PtRs] [vl er S cks ln] ---------------- [bi->skb] <-- Adv Rx Write-Back format");
 
 		for (i = 0; i < rx_ring->count; i++) {
 			const char *next_desc;
@@ -275,18 +274,18 @@ void igc_rings_dump(struct igc_adapter *adapter)
 
 			if (staterr & IGC_RXD_STAT_DD) {
 				/* Descriptor Done */
-				pr_info("%s[0x%03X]     %016llX %016llX ---------------- %s\n",
-					"RWB", i,
-					le64_to_cpu(u0->a),
-					le64_to_cpu(u0->b),
-					next_desc);
+				netdev_info(netdev, "%s[0x%03X]     %016llX %016llX ---------------- %s",
+					    "RWB", i,
+					    le64_to_cpu(u0->a),
+					    le64_to_cpu(u0->b),
+					    next_desc);
 			} else {
-				pr_info("%s[0x%03X]     %016llX %016llX %016llX %s\n",
-					"R  ", i,
-					le64_to_cpu(u0->a),
-					le64_to_cpu(u0->b),
-					(u64)buffer_info->dma,
-					next_desc);
+				netdev_info(netdev, "%s[0x%03X]     %016llX %016llX %016llX %s",
+					    "R  ", i,
+					    le64_to_cpu(u0->a),
+					    le64_to_cpu(u0->b),
+					    (u64)buffer_info->dma,
+					    next_desc);
 
 				if (netif_msg_pktdata(adapter) &&
 				    buffer_info->dma && buffer_info->page) {
@@ -314,8 +313,8 @@ void igc_regs_dump(struct igc_adapter *adapter)
 	struct igc_reg_info *reginfo;
 
 	/* Print Registers */
-	dev_info(&adapter->pdev->dev, "Register Dump\n");
-	pr_info(" Register Name   Value\n");
+	netdev_info(adapter->netdev, "Register Dump");
+	netdev_info(adapter->netdev, "Register Name   Value");
 	for (reginfo = (struct igc_reg_info *)igc_reg_info_tbl;
 	     reginfo->name; reginfo++) {
 		igc_regdump(hw, reginfo);
-- 
2.25.3


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

* [net-next 07/13] igc: Use netdev log helpers in igc_base.c
  2020-04-20 23:43 [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20 Jeff Kirsher
                   ` (5 preceding siblings ...)
  2020-04-20 23:43 ` [net-next 06/13] igc: Use netdev log helpers in igc_dump.c Jeff Kirsher
@ 2020-04-20 23:43 ` Jeff Kirsher
  2020-04-20 23:43 ` [net-next 08/13] igc: Remove '\n' from log strings in igc_i225.c Jeff Kirsher
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Kirsher @ 2020-04-20 23:43 UTC (permalink / raw)
  To: davem; +Cc: Andre Guedes, netdev, nhorman, sassmann, Aaron Brown, Jeff Kirsher

From: Andre Guedes <andre.guedes@intel.com>

This patch coverts one pr_debug() call to hw_dbg() in order to keep log
output aligned with the rest of the driver. hw_dbg() is actually a macro
defined in igc_hw.h that expands to netdev_dbg().

It also takes this opportunity to remove the '\n' character at the end
of messages since it is automatically added to by netdev_dbg().

Signed-off-by: Andre Guedes <andre.guedes@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_base.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_base.c b/drivers/net/ethernet/intel/igc/igc_base.c
index f7fb18d8d8f5..9722db75064f 100644
--- a/drivers/net/ethernet/intel/igc/igc_base.c
+++ b/drivers/net/ethernet/intel/igc/igc_base.c
@@ -26,9 +26,9 @@ static s32 igc_reset_hw_base(struct igc_hw *hw)
 	 */
 	ret_val = igc_disable_pcie_master(hw);
 	if (ret_val)
-		hw_dbg("PCI-E Master disable polling has failed.\n");
+		hw_dbg("PCI-E Master disable polling has failed");
 
-	hw_dbg("Masking off all interrupts\n");
+	hw_dbg("Masking off all interrupts");
 	wr32(IGC_IMC, 0xffffffff);
 
 	wr32(IGC_RCTL, 0);
@@ -39,7 +39,7 @@ static s32 igc_reset_hw_base(struct igc_hw *hw)
 
 	ctrl = rd32(IGC_CTRL);
 
-	hw_dbg("Issuing a global reset to MAC\n");
+	hw_dbg("Issuing a global reset to MAC");
 	wr32(IGC_CTRL, ctrl | IGC_CTRL_DEV_RST);
 
 	ret_val = igc_get_auto_rd_done(hw);
@@ -48,7 +48,7 @@ static s32 igc_reset_hw_base(struct igc_hw *hw)
 		 * return with an error. This can happen in situations
 		 * where there is no eeprom and prevents getting link.
 		 */
-		hw_dbg("Auto Read Done did not complete\n");
+		hw_dbg("Auto Read Done did not complete");
 	}
 
 	/* Clear any pending interrupt events. */
@@ -177,7 +177,7 @@ static s32 igc_init_phy_params_base(struct igc_hw *hw)
 	 */
 	ret_val = hw->phy.ops.reset(hw);
 	if (ret_val) {
-		hw_dbg("Error resetting the PHY.\n");
+		hw_dbg("Error resetting the PHY");
 		goto out;
 	}
 
@@ -292,12 +292,12 @@ static s32 igc_init_hw_base(struct igc_hw *hw)
 	igc_init_rx_addrs(hw, rar_count);
 
 	/* Zero out the Multicast HASH table */
-	hw_dbg("Zeroing the MTA\n");
+	hw_dbg("Zeroing the MTA");
 	for (i = 0; i < mac->mta_reg_count; i++)
 		array_wr32(IGC_MTA, i, 0);
 
 	/* Zero out the Unicast HASH table */
-	hw_dbg("Zeroing the UTA\n");
+	hw_dbg("Zeroing the UTA");
 	for (i = 0; i < mac->uta_reg_count; i++)
 		array_wr32(IGC_UTA, i, 0);
 
@@ -367,7 +367,7 @@ void igc_rx_fifo_flush_base(struct igc_hw *hw)
 	}
 
 	if (ms_wait == 10)
-		pr_debug("Queue disable timed out after 10ms\n");
+		hw_dbg("Queue disable timed out after 10ms");
 
 	/* Clear RLPML, RCTL.SBP, RFCTL.LEF, and set RCTL.LPE so that all
 	 * incoming packets are rejected.  Set enable and wait 2ms so that
-- 
2.25.3


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

* [net-next 08/13] igc: Remove '\n' from log strings in igc_i225.c
  2020-04-20 23:43 [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20 Jeff Kirsher
                   ` (6 preceding siblings ...)
  2020-04-20 23:43 ` [net-next 07/13] igc: Use netdev log helpers in igc_base.c Jeff Kirsher
@ 2020-04-20 23:43 ` Jeff Kirsher
  2020-04-20 23:43 ` [net-next 09/13] igc: Remove '\n' from log strings in igc_mac.c Jeff Kirsher
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Kirsher @ 2020-04-20 23:43 UTC (permalink / raw)
  To: davem; +Cc: Andre Guedes, netdev, nhorman, sassmann, Aaron Brown, Jeff Kirsher

From: Andre Guedes <andre.guedes@intel.com>

To keep log strings in igc_i225.c consistent with the rest of the driver
code, this patch removes the '\n' character at the end. The newline
character is automatically added by netdev_dbg() so there is no changes
in the output.

Note: hw_dbg() is a macro that expands to netdev_dbg().

Signed-off-by: Andre Guedes <andre.guedes@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_i225.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_i225.c b/drivers/net/ethernet/intel/igc/igc_i225.c
index c25f555aaf82..a5856ae87a7e 100644
--- a/drivers/net/ethernet/intel/igc/igc_i225.c
+++ b/drivers/net/ethernet/intel/igc/igc_i225.c
@@ -71,7 +71,7 @@ static s32 igc_get_hw_semaphore_i225(struct igc_hw *hw)
 
 		/* If we do not have the semaphore here, we have to give up. */
 		if (i == timeout) {
-			hw_dbg("Driver can't access device - SMBI bit is set.\n");
+			hw_dbg("Driver can't access device - SMBI bit is set");
 			return -IGC_ERR_NVM;
 		}
 	}
@@ -91,7 +91,7 @@ static s32 igc_get_hw_semaphore_i225(struct igc_hw *hw)
 	if (i == timeout) {
 		/* Release semaphores */
 		igc_put_hw_semaphore(hw);
-		hw_dbg("Driver can't access the NVM\n");
+		hw_dbg("Driver can't access the NVM");
 		return -IGC_ERR_NVM;
 	}
 
@@ -131,7 +131,7 @@ s32 igc_acquire_swfw_sync_i225(struct igc_hw *hw, u16 mask)
 	}
 
 	if (i == timeout) {
-		hw_dbg("Driver can't access resource, SW_FW_SYNC timeout.\n");
+		hw_dbg("Driver can't access resource, SW_FW_SYNC timeout");
 		ret_val = -IGC_ERR_SWFW_SYNC;
 		goto out;
 	}
@@ -228,7 +228,7 @@ static s32 igc_write_nvm_srwr(struct igc_hw *hw, u16 offset, u16 words,
 	 */
 	if (offset >= nvm->word_size || (words > (nvm->word_size - offset)) ||
 	    words == 0) {
-		hw_dbg("nvm parameter(s) out of bounds\n");
+		hw_dbg("nvm parameter(s) out of bounds");
 		ret_val = -IGC_ERR_NVM;
 		goto out;
 	}
@@ -250,7 +250,7 @@ static s32 igc_write_nvm_srwr(struct igc_hw *hw, u16 offset, u16 words,
 		}
 
 		if (ret_val) {
-			hw_dbg("Shadow RAM write EEWR timed out\n");
+			hw_dbg("Shadow RAM write EEWR timed out");
 			break;
 		}
 	}
@@ -369,7 +369,7 @@ static s32 igc_update_flash_i225(struct igc_hw *hw)
 
 	ret_val = igc_pool_flash_update_done_i225(hw);
 	if (ret_val == -IGC_ERR_NVM) {
-		hw_dbg("Flash update time out\n");
+		hw_dbg("Flash update time out");
 		goto out;
 	}
 
@@ -378,9 +378,9 @@ static s32 igc_update_flash_i225(struct igc_hw *hw)
 
 	ret_val = igc_pool_flash_update_done_i225(hw);
 	if (ret_val)
-		hw_dbg("Flash update time out\n");
+		hw_dbg("Flash update time out");
 	else
-		hw_dbg("Flash update complete\n");
+		hw_dbg("Flash update complete");
 
 out:
 	return ret_val;
@@ -406,7 +406,7 @@ static s32 igc_update_nvm_checksum_i225(struct igc_hw *hw)
 	 */
 	ret_val = igc_read_nvm_eerd(hw, 0, 1, &nvm_data);
 	if (ret_val) {
-		hw_dbg("EEPROM read failed\n");
+		hw_dbg("EEPROM read failed");
 		goto out;
 	}
 
@@ -423,7 +423,7 @@ static s32 igc_update_nvm_checksum_i225(struct igc_hw *hw)
 		ret_val = igc_read_nvm_eerd(hw, i, 1, &nvm_data);
 		if (ret_val) {
 			hw->nvm.ops.release(hw);
-			hw_dbg("NVM Read Error while updating checksum.\n");
+			hw_dbg("NVM Read Error while updating checksum");
 			goto out;
 		}
 		checksum += nvm_data;
@@ -433,7 +433,7 @@ static s32 igc_update_nvm_checksum_i225(struct igc_hw *hw)
 				     &checksum);
 	if (ret_val) {
 		hw->nvm.ops.release(hw);
-		hw_dbg("NVM Write Error while updating checksum.\n");
+		hw_dbg("NVM Write Error while updating checksum");
 		goto out;
 	}
 
-- 
2.25.3


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

* [net-next 09/13] igc: Remove '\n' from log strings in igc_mac.c
  2020-04-20 23:43 [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20 Jeff Kirsher
                   ` (7 preceding siblings ...)
  2020-04-20 23:43 ` [net-next 08/13] igc: Remove '\n' from log strings in igc_i225.c Jeff Kirsher
@ 2020-04-20 23:43 ` Jeff Kirsher
  2020-04-20 23:43 ` [net-next 10/13] igc: Remove '\n' from log messages in igc_nvm.c Jeff Kirsher
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Kirsher @ 2020-04-20 23:43 UTC (permalink / raw)
  To: davem; +Cc: Andre Guedes, netdev, nhorman, sassmann, Aaron Brown, Jeff Kirsher

From: Andre Guedes <andre.guedes@intel.com>

To keep log strings in igc_mac.c consistent with the rest of the driver
code, this patch removes the '\n' character at the end. The newline
character is automatically added by netdev_dbg() so there is no changes
in the output.

Note: hw_dbg() is a macro that expands to netdev_dbg().

Signed-off-by: Andre Guedes <andre.guedes@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_mac.c | 42 ++++++++++++------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_mac.c b/drivers/net/ethernet/intel/igc/igc_mac.c
index 12aa6b5fcb5d..2cd52b5c203d 100644
--- a/drivers/net/ethernet/intel/igc/igc_mac.c
+++ b/drivers/net/ethernet/intel/igc/igc_mac.c
@@ -37,7 +37,7 @@ s32 igc_disable_pcie_master(struct igc_hw *hw)
 	}
 
 	if (!timeout) {
-		hw_dbg("Master requests are pending.\n");
+		hw_dbg("Master requests are pending");
 		ret_val = -IGC_ERR_MASTER_REQUESTS_PENDING;
 		goto out;
 	}
@@ -61,12 +61,12 @@ void igc_init_rx_addrs(struct igc_hw *hw, u16 rar_count)
 	u32 i;
 
 	/* Setup the receive address */
-	hw_dbg("Programming MAC Address into RAR[0]\n");
+	hw_dbg("Programming MAC Address into RAR[0]");
 
 	hw->mac.ops.rar_set(hw, hw->mac.addr, 0);
 
 	/* Zero out the other (rar_entry_count - 1) receive addresses */
-	hw_dbg("Clearing RAR[1-%u]\n", rar_count - 1);
+	hw_dbg("Clearing RAR[1-%u]", rar_count - 1);
 	for (i = 1; i < rar_count; i++)
 		hw->mac.ops.rar_set(hw, mac_addr, i);
 }
@@ -138,7 +138,7 @@ s32 igc_setup_link(struct igc_hw *hw)
 	 */
 	hw->fc.current_mode = hw->fc.requested_mode;
 
-	hw_dbg("After fix-ups FlowControl is now = %x\n", hw->fc.current_mode);
+	hw_dbg("After fix-ups FlowControl is now = %x", hw->fc.current_mode);
 
 	/* Call the necessary media_type subroutine to configure the link. */
 	ret_val = hw->mac.ops.setup_physical_interface(hw);
@@ -150,7 +150,7 @@ s32 igc_setup_link(struct igc_hw *hw)
 	 * control is disabled, because it does not hurt anything to
 	 * initialize these registers.
 	 */
-	hw_dbg("Initializing the Flow Control address, type and timer regs\n");
+	hw_dbg("Initializing the Flow Control address, type and timer regs");
 	wr32(IGC_FCT, FLOW_CONTROL_TYPE);
 	wr32(IGC_FCAH, FLOW_CONTROL_ADDRESS_HIGH);
 	wr32(IGC_FCAL, FLOW_CONTROL_ADDRESS_LOW);
@@ -197,7 +197,7 @@ s32 igc_force_mac_fc(struct igc_hw *hw)
 	 *      3:  Both Rx and TX flow control (symmetric) is enabled.
 	 *  other:  No other values should be possible at this point.
 	 */
-	hw_dbg("hw->fc.current_mode = %u\n", hw->fc.current_mode);
+	hw_dbg("hw->fc.current_mode = %u", hw->fc.current_mode);
 
 	switch (hw->fc.current_mode) {
 	case igc_fc_none:
@@ -215,7 +215,7 @@ s32 igc_force_mac_fc(struct igc_hw *hw)
 		ctrl |= (IGC_CTRL_TFCE | IGC_CTRL_RFCE);
 		break;
 	default:
-		hw_dbg("Flow control param set incorrectly\n");
+		hw_dbg("Flow control param set incorrectly");
 		ret_val = -IGC_ERR_CONFIG;
 		goto out;
 	}
@@ -419,7 +419,7 @@ s32 igc_check_for_copper_link(struct igc_hw *hw)
 	 */
 	ret_val = igc_config_fc_after_link_up(hw);
 	if (ret_val)
-		hw_dbg("Error configuring flow control\n");
+		hw_dbg("Error configuring flow control");
 
 out:
 	return ret_val;
@@ -473,7 +473,7 @@ s32 igc_config_fc_after_link_up(struct igc_hw *hw)
 	}
 
 	if (ret_val) {
-		hw_dbg("Error forcing flow control settings\n");
+		hw_dbg("Error forcing flow control settings");
 		goto out;
 	}
 
@@ -497,7 +497,7 @@ s32 igc_config_fc_after_link_up(struct igc_hw *hw)
 			goto out;
 
 		if (!(mii_status_reg & MII_SR_AUTONEG_COMPLETE)) {
-			hw_dbg("Copper PHY and Auto Neg has not completed.\n");
+			hw_dbg("Copper PHY and Auto Neg has not completed");
 			goto out;
 		}
 
@@ -558,10 +558,10 @@ s32 igc_config_fc_after_link_up(struct igc_hw *hw)
 			 */
 			if (hw->fc.requested_mode == igc_fc_full) {
 				hw->fc.current_mode = igc_fc_full;
-				hw_dbg("Flow Control = FULL.\n");
+				hw_dbg("Flow Control = FULL");
 			} else {
 				hw->fc.current_mode = igc_fc_rx_pause;
-				hw_dbg("Flow Control = RX PAUSE frames only.\n");
+				hw_dbg("Flow Control = RX PAUSE frames only");
 			}
 		}
 
@@ -577,7 +577,7 @@ s32 igc_config_fc_after_link_up(struct igc_hw *hw)
 			 (mii_nway_lp_ability_reg & NWAY_LPAR_PAUSE) &&
 			 (mii_nway_lp_ability_reg & NWAY_LPAR_ASM_DIR)) {
 			hw->fc.current_mode = igc_fc_tx_pause;
-			hw_dbg("Flow Control = TX PAUSE frames only.\n");
+			hw_dbg("Flow Control = TX PAUSE frames only");
 		}
 		/* For transmitting PAUSE frames ONLY.
 		 *
@@ -591,7 +591,7 @@ s32 igc_config_fc_after_link_up(struct igc_hw *hw)
 			 !(mii_nway_lp_ability_reg & NWAY_LPAR_PAUSE) &&
 			 (mii_nway_lp_ability_reg & NWAY_LPAR_ASM_DIR)) {
 			hw->fc.current_mode = igc_fc_rx_pause;
-			hw_dbg("Flow Control = RX PAUSE frames only.\n");
+			hw_dbg("Flow Control = RX PAUSE frames only");
 		}
 		/* Per the IEEE spec, at this point flow control should be
 		 * disabled.  However, we want to consider that we could
@@ -617,10 +617,10 @@ s32 igc_config_fc_after_link_up(struct igc_hw *hw)
 			 (hw->fc.requested_mode == igc_fc_tx_pause) ||
 			 (hw->fc.strict_ieee)) {
 			hw->fc.current_mode = igc_fc_none;
-			hw_dbg("Flow Control = NONE.\n");
+			hw_dbg("Flow Control = NONE");
 		} else {
 			hw->fc.current_mode = igc_fc_rx_pause;
-			hw_dbg("Flow Control = RX PAUSE frames only.\n");
+			hw_dbg("Flow Control = RX PAUSE frames only");
 		}
 
 		/* Now we need to do one last check...  If we auto-
@@ -629,7 +629,7 @@ s32 igc_config_fc_after_link_up(struct igc_hw *hw)
 		 */
 		ret_val = hw->mac.ops.get_speed_and_duplex(hw, &speed, &duplex);
 		if (ret_val) {
-			hw_dbg("Error getting link speed and duplex\n");
+			hw_dbg("Error getting link speed and duplex");
 			goto out;
 		}
 
@@ -641,7 +641,7 @@ s32 igc_config_fc_after_link_up(struct igc_hw *hw)
 		 */
 		ret_val = igc_force_mac_fc(hw);
 		if (ret_val) {
-			hw_dbg("Error forcing flow control settings\n");
+			hw_dbg("Error forcing flow control settings");
 			goto out;
 		}
 	}
@@ -669,7 +669,7 @@ s32 igc_get_auto_rd_done(struct igc_hw *hw)
 	}
 
 	if (i == AUTO_READ_DONE_TIMEOUT) {
-		hw_dbg("Auto read by HW from NVM has not completed.\n");
+		hw_dbg("Auto read by HW from NVM has not completed");
 		ret_val = -IGC_ERR_RESET;
 		goto out;
 	}
@@ -716,10 +716,10 @@ s32 igc_get_speed_and_duplex_copper(struct igc_hw *hw, u16 *speed,
 
 	if (status & IGC_STATUS_FD) {
 		*duplex = FULL_DUPLEX;
-		hw_dbg("Full Duplex\n");
+		hw_dbg("Full Duplex");
 	} else {
 		*duplex = HALF_DUPLEX;
-		hw_dbg("Half Duplex\n");
+		hw_dbg("Half Duplex");
 	}
 
 	return 0;
-- 
2.25.3


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

* [net-next 10/13] igc: Remove '\n' from log messages in igc_nvm.c
  2020-04-20 23:43 [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20 Jeff Kirsher
                   ` (8 preceding siblings ...)
  2020-04-20 23:43 ` [net-next 09/13] igc: Remove '\n' from log strings in igc_mac.c Jeff Kirsher
@ 2020-04-20 23:43 ` Jeff Kirsher
  2020-04-20 23:43 ` [net-next 11/13] igc: Remove '\n' from log strings in igc_phy.c Jeff Kirsher
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Kirsher @ 2020-04-20 23:43 UTC (permalink / raw)
  To: davem; +Cc: Andre Guedes, netdev, nhorman, sassmann, Aaron Brown, Jeff Kirsher

From: Andre Guedes <andre.guedes@intel.com>

To keep log strings in igc_nvm.c consistent with the rest of the driver
code, this patch removes the '\n' character at the end. The newline
character is automatically added by netdev_dbg() so there is no changes
in the output.

Note: hw_dbg() is a macro that expands to netdev_dbg().

Signed-off-by: Andre Guedes <andre.guedes@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_nvm.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_nvm.c b/drivers/net/ethernet/intel/igc/igc_nvm.c
index 58f81aba0144..ac01330a8232 100644
--- a/drivers/net/ethernet/intel/igc/igc_nvm.c
+++ b/drivers/net/ethernet/intel/igc/igc_nvm.c
@@ -63,7 +63,7 @@ s32 igc_acquire_nvm(struct igc_hw *hw)
 	if (!timeout) {
 		eecd &= ~IGC_EECD_REQ;
 		wr32(IGC_EECD, eecd);
-		hw_dbg("Could not acquire NVM grant\n");
+		hw_dbg("Could not acquire NVM grant");
 		ret_val = -IGC_ERR_NVM;
 	}
 
@@ -105,7 +105,7 @@ s32 igc_read_nvm_eerd(struct igc_hw *hw, u16 offset, u16 words, u16 *data)
 	 */
 	if (offset >= nvm->word_size || (words > (nvm->word_size - offset)) ||
 	    words == 0) {
-		hw_dbg("nvm parameter(s) out of bounds\n");
+		hw_dbg("nvm parameter(s) out of bounds");
 		ret_val = -IGC_ERR_NVM;
 		goto out;
 	}
@@ -167,14 +167,14 @@ s32 igc_validate_nvm_checksum(struct igc_hw *hw)
 	for (i = 0; i < (NVM_CHECKSUM_REG + 1); i++) {
 		ret_val = hw->nvm.ops.read(hw, i, 1, &nvm_data);
 		if (ret_val) {
-			hw_dbg("NVM Read Error\n");
+			hw_dbg("NVM Read Error");
 			goto out;
 		}
 		checksum += nvm_data;
 	}
 
 	if (checksum != (u16)NVM_SUM) {
-		hw_dbg("NVM Checksum Invalid\n");
+		hw_dbg("NVM Checksum Invalid");
 		ret_val = -IGC_ERR_NVM;
 		goto out;
 	}
@@ -200,7 +200,7 @@ s32 igc_update_nvm_checksum(struct igc_hw *hw)
 	for (i = 0; i < NVM_CHECKSUM_REG; i++) {
 		ret_val = hw->nvm.ops.read(hw, i, 1, &nvm_data);
 		if (ret_val) {
-			hw_dbg("NVM Read Error while updating checksum.\n");
+			hw_dbg("NVM Read Error while updating checksum");
 			goto out;
 		}
 		checksum += nvm_data;
@@ -208,7 +208,7 @@ s32 igc_update_nvm_checksum(struct igc_hw *hw)
 	checksum = (u16)NVM_SUM - checksum;
 	ret_val = hw->nvm.ops.write(hw, NVM_CHECKSUM_REG, 1, &checksum);
 	if (ret_val)
-		hw_dbg("NVM Write Error while updating checksum.\n");
+		hw_dbg("NVM Write Error while updating checksum");
 
 out:
 	return ret_val;
-- 
2.25.3


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

* [net-next 11/13] igc: Remove '\n' from log strings in igc_phy.c
  2020-04-20 23:43 [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20 Jeff Kirsher
                   ` (9 preceding siblings ...)
  2020-04-20 23:43 ` [net-next 10/13] igc: Remove '\n' from log messages in igc_nvm.c Jeff Kirsher
@ 2020-04-20 23:43 ` Jeff Kirsher
  2020-04-20 23:43 ` [net-next 12/13] igc: Remove unneeded definition Jeff Kirsher
  2020-04-20 23:43 ` [net-next 13/13] igc: Remove unneeded register Jeff Kirsher
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Kirsher @ 2020-04-20 23:43 UTC (permalink / raw)
  To: davem; +Cc: Andre Guedes, netdev, nhorman, sassmann, Aaron Brown, Jeff Kirsher

From: Andre Guedes <andre.guedes@intel.com>

To keep log strings in igc_phy.c consistent with the rest of the driver
code, this patch removes the '\n' character at the end. The newline
character is automatically added by netdev_dbg() so there is no changes
in the output.

Note: hw_dbg() is a macro that expands to netdev_dbg().

Signed-off-by: Andre Guedes <andre.guedes@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_phy.c | 52 ++++++++++++------------
 1 file changed, 26 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_phy.c b/drivers/net/ethernet/intel/igc/igc_phy.c
index 8e1799508edc..a11c2e45d894 100644
--- a/drivers/net/ethernet/intel/igc/igc_phy.c
+++ b/drivers/net/ethernet/intel/igc/igc_phy.c
@@ -207,7 +207,7 @@ s32 igc_phy_hw_reset(struct igc_hw *hw)
 	} while (!(phpm & IGC_PHY_RST_COMP) && timeout);
 
 	if (!timeout)
-		hw_dbg("Timeout is expired after a phy reset\n");
+		hw_dbg("Timeout is expired after a phy reset");
 
 	usleep_range(100, 150);
 
@@ -278,49 +278,49 @@ static s32 igc_phy_setup_autoneg(struct igc_hw *hw)
 				 NWAY_AR_10T_HD_CAPS);
 	mii_1000t_ctrl_reg &= ~(CR_1000T_HD_CAPS | CR_1000T_FD_CAPS);
 
-	hw_dbg("autoneg_advertised %x\n", phy->autoneg_advertised);
+	hw_dbg("autoneg_advertised %x", phy->autoneg_advertised);
 
 	/* Do we want to advertise 10 Mb Half Duplex? */
 	if (phy->autoneg_advertised & ADVERTISE_10_HALF) {
-		hw_dbg("Advertise 10mb Half duplex\n");
+		hw_dbg("Advertise 10mb Half duplex");
 		mii_autoneg_adv_reg |= NWAY_AR_10T_HD_CAPS;
 	}
 
 	/* Do we want to advertise 10 Mb Full Duplex? */
 	if (phy->autoneg_advertised & ADVERTISE_10_FULL) {
-		hw_dbg("Advertise 10mb Full duplex\n");
+		hw_dbg("Advertise 10mb Full duplex");
 		mii_autoneg_adv_reg |= NWAY_AR_10T_FD_CAPS;
 	}
 
 	/* Do we want to advertise 100 Mb Half Duplex? */
 	if (phy->autoneg_advertised & ADVERTISE_100_HALF) {
-		hw_dbg("Advertise 100mb Half duplex\n");
+		hw_dbg("Advertise 100mb Half duplex");
 		mii_autoneg_adv_reg |= NWAY_AR_100TX_HD_CAPS;
 	}
 
 	/* Do we want to advertise 100 Mb Full Duplex? */
 	if (phy->autoneg_advertised & ADVERTISE_100_FULL) {
-		hw_dbg("Advertise 100mb Full duplex\n");
+		hw_dbg("Advertise 100mb Full duplex");
 		mii_autoneg_adv_reg |= NWAY_AR_100TX_FD_CAPS;
 	}
 
 	/* We do not allow the Phy to advertise 1000 Mb Half Duplex */
 	if (phy->autoneg_advertised & ADVERTISE_1000_HALF)
-		hw_dbg("Advertise 1000mb Half duplex request denied!\n");
+		hw_dbg("Advertise 1000mb Half duplex request denied");
 
 	/* Do we want to advertise 1000 Mb Full Duplex? */
 	if (phy->autoneg_advertised & ADVERTISE_1000_FULL) {
-		hw_dbg("Advertise 1000mb Full duplex\n");
+		hw_dbg("Advertise 1000mb Full duplex");
 		mii_1000t_ctrl_reg |= CR_1000T_FD_CAPS;
 	}
 
 	/* We do not allow the Phy to advertise 2500 Mb Half Duplex */
 	if (phy->autoneg_advertised & ADVERTISE_2500_HALF)
-		hw_dbg("Advertise 2500mb Half duplex request denied!\n");
+		hw_dbg("Advertise 2500mb Half duplex request denied");
 
 	/* Do we want to advertise 2500 Mb Full Duplex? */
 	if (phy->autoneg_advertised & ADVERTISE_2500_FULL) {
-		hw_dbg("Advertise 2500mb Full duplex\n");
+		hw_dbg("Advertise 2500mb Full duplex");
 		aneg_multigbt_an_ctrl |= CR_2500T_FD_CAPS;
 	} else {
 		aneg_multigbt_an_ctrl &= ~CR_2500T_FD_CAPS;
@@ -376,7 +376,7 @@ static s32 igc_phy_setup_autoneg(struct igc_hw *hw)
 		mii_autoneg_adv_reg |= (NWAY_AR_ASM_DIR | NWAY_AR_PAUSE);
 		break;
 	default:
-		hw_dbg("Flow control param set incorrectly\n");
+		hw_dbg("Flow control param set incorrectly");
 		return -IGC_ERR_CONFIG;
 	}
 
@@ -384,7 +384,7 @@ static s32 igc_phy_setup_autoneg(struct igc_hw *hw)
 	if (ret_val)
 		return ret_val;
 
-	hw_dbg("Auto-Neg Advertising %x\n", mii_autoneg_adv_reg);
+	hw_dbg("Auto-Neg Advertising %x", mii_autoneg_adv_reg);
 
 	if (phy->autoneg_mask & ADVERTISE_1000_FULL)
 		ret_val = phy->ops.write_reg(hw, PHY_1000T_CTRL,
@@ -458,13 +458,13 @@ static s32 igc_copper_link_autoneg(struct igc_hw *hw)
 	if (phy->autoneg_advertised == 0)
 		phy->autoneg_advertised = phy->autoneg_mask;
 
-	hw_dbg("Reconfiguring auto-neg advertisement params\n");
+	hw_dbg("Reconfiguring auto-neg advertisement params");
 	ret_val = igc_phy_setup_autoneg(hw);
 	if (ret_val) {
-		hw_dbg("Error Setting up Auto-Negotiation\n");
+		hw_dbg("Error Setting up Auto-Negotiation");
 		goto out;
 	}
-	hw_dbg("Restarting Auto-Neg\n");
+	hw_dbg("Restarting Auto-Neg");
 
 	/* Restart auto-negotiation by setting the Auto Neg Enable bit and
 	 * the Auto Neg Restart bit in the PHY control register.
@@ -484,7 +484,7 @@ static s32 igc_copper_link_autoneg(struct igc_hw *hw)
 	if (phy->autoneg_wait_to_complete) {
 		ret_val = igc_wait_autoneg(hw);
 		if (ret_val) {
-			hw_dbg("Error while waiting for autoneg to complete\n");
+			hw_dbg("Error while waiting for autoneg to complete");
 			goto out;
 		}
 	}
@@ -520,10 +520,10 @@ s32 igc_setup_copper_link(struct igc_hw *hw)
 		/* PHY will be set to 10H, 10F, 100H or 100F
 		 * depending on user settings.
 		 */
-		hw_dbg("Forcing Speed and Duplex\n");
+		hw_dbg("Forcing Speed and Duplex");
 		ret_val = hw->phy.ops.force_speed_duplex(hw);
 		if (ret_val) {
-			hw_dbg("Error Forcing Speed and Duplex\n");
+			hw_dbg("Error Forcing Speed and Duplex");
 			goto out;
 		}
 	}
@@ -536,11 +536,11 @@ s32 igc_setup_copper_link(struct igc_hw *hw)
 		goto out;
 
 	if (link) {
-		hw_dbg("Valid link established!!!\n");
+		hw_dbg("Valid link established");
 		igc_config_collision_dist(hw);
 		ret_val = igc_config_fc_after_link_up(hw);
 	} else {
-		hw_dbg("Unable to establish link!!!\n");
+		hw_dbg("Unable to establish link");
 	}
 
 out:
@@ -563,7 +563,7 @@ static s32 igc_read_phy_reg_mdic(struct igc_hw *hw, u32 offset, u16 *data)
 	s32 ret_val = 0;
 
 	if (offset > MAX_PHY_REG_ADDRESS) {
-		hw_dbg("PHY Address %d is out of range\n", offset);
+		hw_dbg("PHY Address %d is out of range", offset);
 		ret_val = -IGC_ERR_PARAM;
 		goto out;
 	}
@@ -589,12 +589,12 @@ static s32 igc_read_phy_reg_mdic(struct igc_hw *hw, u32 offset, u16 *data)
 			break;
 	}
 	if (!(mdic & IGC_MDIC_READY)) {
-		hw_dbg("MDI Read did not complete\n");
+		hw_dbg("MDI Read did not complete");
 		ret_val = -IGC_ERR_PHY;
 		goto out;
 	}
 	if (mdic & IGC_MDIC_ERROR) {
-		hw_dbg("MDI Error\n");
+		hw_dbg("MDI Error");
 		ret_val = -IGC_ERR_PHY;
 		goto out;
 	}
@@ -619,7 +619,7 @@ static s32 igc_write_phy_reg_mdic(struct igc_hw *hw, u32 offset, u16 data)
 	s32 ret_val = 0;
 
 	if (offset > MAX_PHY_REG_ADDRESS) {
-		hw_dbg("PHY Address %d is out of range\n", offset);
+		hw_dbg("PHY Address %d is out of range", offset);
 		ret_val = -IGC_ERR_PARAM;
 		goto out;
 	}
@@ -646,12 +646,12 @@ static s32 igc_write_phy_reg_mdic(struct igc_hw *hw, u32 offset, u16 data)
 			break;
 	}
 	if (!(mdic & IGC_MDIC_READY)) {
-		hw_dbg("MDI Write did not complete\n");
+		hw_dbg("MDI Write did not complete");
 		ret_val = -IGC_ERR_PHY;
 		goto out;
 	}
 	if (mdic & IGC_MDIC_ERROR) {
-		hw_dbg("MDI Error\n");
+		hw_dbg("MDI Error");
 		ret_val = -IGC_ERR_PHY;
 		goto out;
 	}
-- 
2.25.3


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

* [net-next 12/13] igc: Remove unneeded definition
  2020-04-20 23:43 [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20 Jeff Kirsher
                   ` (10 preceding siblings ...)
  2020-04-20 23:43 ` [net-next 11/13] igc: Remove '\n' from log strings in igc_phy.c Jeff Kirsher
@ 2020-04-20 23:43 ` Jeff Kirsher
  2020-04-20 23:43 ` [net-next 13/13] igc: Remove unneeded register Jeff Kirsher
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Kirsher @ 2020-04-20 23:43 UTC (permalink / raw)
  To: davem; +Cc: Sasha Neftin, netdev, nhorman, sassmann, Aaron Brown, Jeff Kirsher

From: Sasha Neftin <sasha.neftin@intel.com>

PHY_FORCE_LIMIT definition not in use and could be removed
i225 parts support auto negotiation mechanism

Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_defines.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_defines.h b/drivers/net/ethernet/intel/igc/igc_defines.h
index af0c03d77a39..54a7941bdb48 100644
--- a/drivers/net/ethernet/intel/igc/igc_defines.h
+++ b/drivers/net/ethernet/intel/igc/igc_defines.h
@@ -47,7 +47,6 @@
 /* Loop limit on how long we wait for auto-negotiation to complete */
 #define COPPER_LINK_UP_LIMIT		10
 #define PHY_AUTO_NEG_LIMIT		45
-#define PHY_FORCE_LIMIT			20
 
 /* Number of 100 microseconds we wait for PCI Express master disable */
 #define MASTER_DISABLE_TIMEOUT		800
-- 
2.25.3


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

* [net-next 13/13] igc: Remove unneeded register
  2020-04-20 23:43 [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20 Jeff Kirsher
                   ` (11 preceding siblings ...)
  2020-04-20 23:43 ` [net-next 12/13] igc: Remove unneeded definition Jeff Kirsher
@ 2020-04-20 23:43 ` Jeff Kirsher
  12 siblings, 0 replies; 20+ messages in thread
From: Jeff Kirsher @ 2020-04-20 23:43 UTC (permalink / raw)
  To: davem; +Cc: Sasha Neftin, netdev, nhorman, sassmann, Aaron Brown, Jeff Kirsher

From: Sasha Neftin <sasha.neftin@intel.com>

Flow control status register not applicable for i225 parts
so clean up the unneeded define.

Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
Tested-by: Aaron Brown <aaron.f.brown@intel.com>
Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
---
 drivers/net/ethernet/intel/igc/igc_regs.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_regs.h b/drivers/net/ethernet/intel/igc/igc_regs.h
index 633545977a65..5a6110e211fd 100644
--- a/drivers/net/ethernet/intel/igc/igc_regs.h
+++ b/drivers/net/ethernet/intel/igc/igc_regs.h
@@ -35,7 +35,6 @@
 #define IGC_FCRTL		0x02160  /* FC Receive Threshold Low - RW */
 #define IGC_FCRTH		0x02168  /* FC Receive Threshold High - RW */
 #define IGC_FCRTV		0x02460  /* FC Refresh Timer Value - RW */
-#define IGC_FCSTS		0x02464  /* FC Status - RO */
 
 /* PCIe Register Description */
 #define IGC_GCR			0x05B00  /* PCIe control- RW */
-- 
2.25.3


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

* Re: [net-next 02/13] igc: Use netdev log helpers in igc_main.c
  2020-04-20 23:43 ` [net-next 02/13] igc: Use netdev log helpers in igc_main.c Jeff Kirsher
@ 2020-04-21 17:39   ` Jakub Kicinski
  2020-04-21 19:26   ` David Miller
  1 sibling, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2020-04-21 17:39 UTC (permalink / raw)
  To: Jeff Kirsher; +Cc: davem, Andre Guedes, netdev, nhorman, sassmann, Aaron Brown

On Mon, 20 Apr 2020 16:43:02 -0700 Jeff Kirsher wrote:
> It also
> takes this opportunity to improve some messages and remove the '\n'
> character at the end of messages since it is automatically added to by
> netdev_* log helpers.

Can you point me to the place that's done?

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

* Re: [net-next 03/13] igc: add support to interrupt, eeprom, registers and link self-tests
  2020-04-20 23:43 ` [net-next 03/13] igc: add support to interrupt, eeprom, registers and link self-tests Jeff Kirsher
@ 2020-04-21 17:48   ` Jakub Kicinski
  0 siblings, 0 replies; 20+ messages in thread
From: Jakub Kicinski @ 2020-04-21 17:48 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: davem, Vitaly Lifshits, netdev, nhorman, sassmann,
	kbuild test robot, Dan Carpenter, Andre Guedes, Aaron Brown,
	Andrew Lunn

On Mon, 20 Apr 2020 16:43:03 -0700 Jeff Kirsher wrote:
> +bool igc_intr_test(struct igc_adapter *adapter, u64 *data)
> +{
> +	struct igc_hw *hw = &adapter->hw;
> +	struct net_device *netdev = adapter->netdev;
> +	u32 mask, ics_mask = IGC_ICS_MASK_OTHER, i = 0, shared_int = true;
> +	u32 irq = adapter->pdev->irq;
> +
> +	*data = 0;
> +
> +	/* Hook up test interrupt handler just for this test */
> +	if (adapter->msix_entries) {
> +		if (request_irq(adapter->msix_entries[0].vector,
> +				&igc_test_intr_msix, 0,
> +				netdev->name, adapter)) {
> +			*data = 1;
> +			return false;
> +		}
> +		ics_mask = IGC_ICS_MASK_MSIX;
> +	} else if (adapter->flags & IGC_FLAG_HAS_MSI) {
> +		shared_int = false;
> +		if (request_irq(irq,
> +				igc_test_intr, 0, netdev->name, adapter)) {
> +			*data = 1;
> +			return false;
> +		}
> +	} else if (!request_irq(irq, igc_test_intr, IRQF_PROBE_SHARED,
> +				netdev->name, adapter)) {
> +		shared_int = false;
> +	} else if (request_irq(irq, &igc_test_intr, IRQF_SHARED,
> +		 netdev->name, adapter)) {
> +		*data = 1;
> +		return false;
> +	}

What's the meaning of shared_int here? Looks like MSI-Ss are shared but
not MSIs? Could you perhaps add a comment or rename so it's clear it's
not IRQF_SHARED we're talking about?

> +static void igc_diag_test(struct net_device *netdev,
> +			  struct ethtool_test *eth_test, u64 *data)
> +{
> +	struct igc_adapter *adapter = netdev_priv(netdev);
> +	bool if_running = netif_running(netdev);
> +
> +	if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
> +		netdev_info(adapter->netdev, "offline testing starting");
> +		set_bit(__IGC_TESTING, &adapter->state);

> +	} else {
> +		netdev_info(adapter->netdev, "online testing starting");

I'm no expert on self-tests but this looks like a strange condition for
a bitfield. If only on bit is set we do offline, if no bit is set, or
offline and something else we do online?

Perhaps:

if (flags & OFFLINE) {
 ...
} else {
 ...
}

Or

if (flags == OFFLINE) {
 ...
} else if (flags == 0) {
 ...
}

Rather than the mix of the two?

> +		/* register, eeprom, intr and loopback tests not run online */
> +		data[TEST_REG] = 0;
> +		data[TEST_EEP] = 0;
> +		data[TEST_IRQ] = 0;
> +		data[TEST_LOOP] = 0;
> +
> +		if (!igc_link_test(adapter, &data[TEST_LINK]))
> +			eth_test->flags |= ETH_TEST_FL_FAILED;
> +	}
> +
> +	msleep_interruptible(4 * 1000);

Why?

> +}
> +

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

* Re: [net-next 02/13] igc: Use netdev log helpers in igc_main.c
  2020-04-20 23:43 ` [net-next 02/13] igc: Use netdev log helpers in igc_main.c Jeff Kirsher
  2020-04-21 17:39   ` Jakub Kicinski
@ 2020-04-21 19:26   ` David Miller
  2020-04-21 21:09     ` Andre Guedes
  1 sibling, 1 reply; 20+ messages in thread
From: David Miller @ 2020-04-21 19:26 UTC (permalink / raw)
  To: jeffrey.t.kirsher; +Cc: andre.guedes, netdev, nhorman, sassmann, aaron.f.brown

From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Mon, 20 Apr 2020 16:43:02 -0700

> It also takes this opportunity to improve some messages and remove
> the '\n' character at the end of messages since it is automatically
> added to by netdev_* log helpers.

Where does this happen?  I can't find it.

I'm tossing this series, you have to explain where this happens
because I see several developers who can't figure this out at all.

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

* Re: [net-next 02/13] igc: Use netdev log helpers in igc_main.c
  2020-04-21 19:26   ` David Miller
@ 2020-04-21 21:09     ` Andre Guedes
  2020-04-21 22:32       ` David Miller
  0 siblings, 1 reply; 20+ messages in thread
From: Andre Guedes @ 2020-04-21 21:09 UTC (permalink / raw)
  To: David Miller, jeffrey.t.kirsher, kuba
  Cc: andre.guedes, netdev, nhorman, sassmann, aaron.f.brown

Hi David/Jakub,

Quoting David Miller (2020-04-21 12:26:10)
> From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Date: Mon, 20 Apr 2020 16:43:02 -0700
> 
> > It also takes this opportunity to improve some messages and remove
> > the '\n' character at the end of messages since it is automatically
> > added to by netdev_* log helpers.
> 
> Where does this happen?  I can't find it.

A while ago, I came across this LWN article [1] that says printk() begins a
new line in the output unless the KERN_CONT flag is used.

This is probably buried deep down in printk() implementation so that's why it
is hard to find. From a quick look, I think the trick is done in
vprintk_store() and log_output() in kernel/printk/printk.c.

Anyways, I can tell you the driver log messages look exactly the same with or
without the ending '\n'.

> I'm tossing this series, you have to explain where this happens
> because I see several developers who can't figure this out at all.

Hope that clarifies and this series is still good to be merged.

Regards,

Andre

[1] https://lwn.net/Articles/732420/

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

* Re: [net-next 02/13] igc: Use netdev log helpers in igc_main.c
  2020-04-21 21:09     ` Andre Guedes
@ 2020-04-21 22:32       ` David Miller
  2020-04-21 22:48         ` Andre Guedes
  0 siblings, 1 reply; 20+ messages in thread
From: David Miller @ 2020-04-21 22:32 UTC (permalink / raw)
  To: andre.guedes
  Cc: jeffrey.t.kirsher, kuba, andre.guedes, netdev, nhorman, sassmann,
	aaron.f.brown

From: Andre Guedes <andre.guedes@linux.intel.com>
Date: Tue, 21 Apr 2020 14:09:45 -0700

> Quoting David Miller (2020-04-21 12:26:10)
>> I'm tossing this series, you have to explain where this happens
>> because I see several developers who can't figure this out at all.
> 
> Hope that clarifies and this series is still good to be merged.

First of all, the commits make it look like specifically like the
netdev printk facilities do the newline thing.

But overall this looks like just busy work, and I've seen commits
that add missing newlines.

I don't think these changes are valuable nor wise.

Please kill these newline removal changes, thank you.

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

* Re: [net-next 02/13] igc: Use netdev log helpers in igc_main.c
  2020-04-21 22:32       ` David Miller
@ 2020-04-21 22:48         ` Andre Guedes
  0 siblings, 0 replies; 20+ messages in thread
From: Andre Guedes @ 2020-04-21 22:48 UTC (permalink / raw)
  To: David Miller
  Cc: jeffrey.t.kirsher, kuba, andre.guedes, netdev, nhorman, sassmann,
	aaron.f.brown

Quoting David Miller (2020-04-21 15:32:21)
> Please kill these newline removal changes, thank you.

OK, I'm working on that. Thanks.

- Andre

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

end of thread, other threads:[~2020-04-21 22:48 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-20 23:43 [net-next 00/13][pull request] 1GbE Intel Wired LAN Driver Updates 2020-04-20 Jeff Kirsher
2020-04-20 23:43 ` [net-next 01/13] igc: Add ECN support for TSO Jeff Kirsher
2020-04-20 23:43 ` [net-next 02/13] igc: Use netdev log helpers in igc_main.c Jeff Kirsher
2020-04-21 17:39   ` Jakub Kicinski
2020-04-21 19:26   ` David Miller
2020-04-21 21:09     ` Andre Guedes
2020-04-21 22:32       ` David Miller
2020-04-21 22:48         ` Andre Guedes
2020-04-20 23:43 ` [net-next 03/13] igc: add support to interrupt, eeprom, registers and link self-tests Jeff Kirsher
2020-04-21 17:48   ` Jakub Kicinski
2020-04-20 23:43 ` [net-next 04/13] igc: Use netdev log helpers in igc_ethtool.c Jeff Kirsher
2020-04-20 23:43 ` [net-next 05/13] igc: Use netdev log helpers in igc_ptp.c Jeff Kirsher
2020-04-20 23:43 ` [net-next 06/13] igc: Use netdev log helpers in igc_dump.c Jeff Kirsher
2020-04-20 23:43 ` [net-next 07/13] igc: Use netdev log helpers in igc_base.c Jeff Kirsher
2020-04-20 23:43 ` [net-next 08/13] igc: Remove '\n' from log strings in igc_i225.c Jeff Kirsher
2020-04-20 23:43 ` [net-next 09/13] igc: Remove '\n' from log strings in igc_mac.c Jeff Kirsher
2020-04-20 23:43 ` [net-next 10/13] igc: Remove '\n' from log messages in igc_nvm.c Jeff Kirsher
2020-04-20 23:43 ` [net-next 11/13] igc: Remove '\n' from log strings in igc_phy.c Jeff Kirsher
2020-04-20 23:43 ` [net-next 12/13] igc: Remove unneeded definition Jeff Kirsher
2020-04-20 23:43 ` [net-next 13/13] igc: Remove unneeded register Jeff Kirsher

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).