* [net-next v2 1/9] igc: Add ECN support for TSO
2020-05-14 21:31 [net-next v2 0/9][pull request] 1GbE Intel Wired LAN Driver Updates 2020-05-14 Jeff Kirsher
@ 2020-05-14 21:31 ` Jeff Kirsher
2020-05-14 21:31 ` [net-next v2 2/9] igc: Use netdev log helpers in igc_main.c Jeff Kirsher
` (7 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2020-05-14 21:31 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.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next v2 2/9] igc: Use netdev log helpers in igc_main.c
2020-05-14 21:31 [net-next v2 0/9][pull request] 1GbE Intel Wired LAN Driver Updates 2020-05-14 Jeff Kirsher
2020-05-14 21:31 ` [net-next v2 1/9] igc: Add ECN support for TSO Jeff Kirsher
@ 2020-05-14 21:31 ` Jeff Kirsher
2020-05-14 22:07 ` Heiner Kallweit
2020-05-14 21:31 ` [net-next v2 3/9] igc: add support to eeprom, registers and link self-tests Jeff Kirsher
` (6 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Jeff Kirsher @ 2020-05-14 21:31 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 | 115 ++++++++++------------
1 file changed, 52 insertions(+), 63 deletions(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 7556fcdf1fd7..c829e78c1a9a 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\n");
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\n");
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\n", 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\n");
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\n", 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\n");
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\n",
+ 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\n");
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\n");
return -ENOMEM;
}
@@ -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\n", 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\n",
adapter->link_speed,
adapter->link_duplex == FULL_DUPLEX ?
"Full" : "Half",
@@ -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\n");
}
} else {
- dev_err(&adapter->pdev->dev, "read 1000Base-T Status Reg\n");
+ netdev_err(netdev, "read 1000Base-T Status Reg\n");
}
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\n");
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\n", err);
request_done:
return err;
@@ -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\n");
return -EINVAL;
}
@@ -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\n");
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\n");
}
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\n");
err = -EIO;
goto err_eeprom;
}
@@ -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\n");
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\n");
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\n");
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\n");
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\n");
return -ENOMEM;
}
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [net-next v2 2/9] igc: Use netdev log helpers in igc_main.c
2020-05-14 21:31 ` [net-next v2 2/9] igc: Use netdev log helpers in igc_main.c Jeff Kirsher
@ 2020-05-14 22:07 ` Heiner Kallweit
2020-05-14 22:45 ` Andre Guedes
0 siblings, 1 reply; 15+ messages in thread
From: Heiner Kallweit @ 2020-05-14 22:07 UTC (permalink / raw)
To: Jeff Kirsher, davem; +Cc: Andre Guedes, netdev, nhorman, sassmann, Aaron Brown
On 14.05.2020 23:31, Jeff Kirsher wrote:
> 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 | 115 ++++++++++------------
> 1 file changed, 52 insertions(+), 63 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 7556fcdf1fd7..c829e78c1a9a 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\n");
>
> 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\n");
> 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\n", 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\n");
> 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\n", 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\n");
> 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\n",
> + 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\n");
> 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\n");
> return -ENOMEM;
> }
>
> @@ -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\n", 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\n",
> adapter->link_speed,
> adapter->link_duplex == FULL_DUPLEX ?
> "Full" : "Half",
> @@ -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\n");
> }
> } else {
> - dev_err(&adapter->pdev->dev, "read 1000Base-T Status Reg\n");
> + netdev_err(netdev, "read 1000Base-T Status Reg\n");
> }
> 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\n");
> 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\n", err);
>
> request_done:
> return err;
> @@ -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\n");
> return -EINVAL;
> }
>
> @@ -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\n");
Using the netdev_xxx message functions before register_netdev() doesn't
provide a benefit. You get "(unnamed net_device) (uninitialized)" in
the message instead of a netdev name. Therefore I went the opposite way
for messages in probe() in 22148df0d0bd ("r8169: don't use netif_info
et al before net_device has been registered")
> 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\n");
> }
>
> 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\n");
> err = -EIO;
> goto err_eeprom;
> }
> @@ -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\n");
> 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\n");
> 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\n");
> 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\n");
> 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\n");
> return -ENOMEM;
> }
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [net-next v2 2/9] igc: Use netdev log helpers in igc_main.c
2020-05-14 22:07 ` Heiner Kallweit
@ 2020-05-14 22:45 ` Andre Guedes
0 siblings, 0 replies; 15+ messages in thread
From: Andre Guedes @ 2020-05-14 22:45 UTC (permalink / raw)
To: Heiner Kallweit, Jeff Kirsher, davem
Cc: netdev, nhorman, sassmann, Aaron Brown
Hi Heiner,
Quoting Heiner Kallweit (2020-05-14 15:07:42)
> On 14.05.2020 23:31, Jeff Kirsher wrote:
> > @@ -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\n");
>
> Using the netdev_xxx message functions before register_netdev() doesn't
> provide a benefit. You get "(unnamed net_device) (uninitialized)" in
> the message instead of a netdev name. Therefore I went the opposite way
> for messages in probe() in 22148df0d0bd ("r8169: don't use netif_info
> et al before net_device has been registered")
Agreed. I'm fixing it. Since this log happens in an error condition, I missed
it in my tests. Thank you for pointing that out.
- Andre
^ permalink raw reply [flat|nested] 15+ messages in thread
* [net-next v2 3/9] igc: add support to eeprom, registers and link self-tests
2020-05-14 21:31 [net-next v2 0/9][pull request] 1GbE Intel Wired LAN Driver Updates 2020-05-14 Jeff Kirsher
2020-05-14 21:31 ` [net-next v2 1/9] igc: Add ECN support for TSO Jeff Kirsher
2020-05-14 21:31 ` [net-next v2 2/9] igc: Use netdev log helpers in igc_main.c Jeff Kirsher
@ 2020-05-14 21:31 ` Jeff Kirsher
2020-05-14 21:52 ` Jakub Kicinski
2020-05-14 21:31 ` [net-next v2 4/9] igc: Use netdev log helpers in igc_ethtool.c Jeff Kirsher
` (5 subsequent siblings)
8 siblings, 1 reply; 15+ messages in thread
From: Jeff Kirsher @ 2020-05-14 21:31 UTC (permalink / raw)
To: davem
Cc: Vitaly Lifshits, netdev, nhorman, sassmann, kbuild test robot,
Dan Carpenter, 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 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>
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 | 186 +++++++++++++++++++
drivers/net/ethernet/intel/igc/igc_diag.h | 30 +++
drivers/net/ethernet/intel/igc/igc_ethtool.c | 60 ++++++
drivers/net/ethernet/intel/igc/igc_main.c | 4 +-
drivers/net/ethernet/intel/igc/igc_regs.h | 2 +
7 files changed, 285 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..1c4536105e56
--- /dev/null
+++ b/drivers/net/ethernet/intel/igc/igc_diag.c
@@ -0,0 +1,186 @@
+// 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;
+}
+
+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..600658e33bec
--- /dev/null
+++ b/drivers/net/ethernet/intel/igc/igc_diag.h
@@ -0,0 +1,30 @@
+/* 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_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
diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index 0a8c4a7412a4..9fc250cdf88c 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,64 @@ 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);
+
+ /* loopback and interrupt tests
+ * will be implemented in the future
+ */
+ data[TEST_LOOP] = 0;
+ data[TEST_IRQ] = 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 +1992,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 c829e78c1a9a..bd426a71d76d 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.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [net-next v2 3/9] igc: add support to eeprom, registers and link self-tests
2020-05-14 21:31 ` [net-next v2 3/9] igc: add support to eeprom, registers and link self-tests Jeff Kirsher
@ 2020-05-14 21:52 ` Jakub Kicinski
2020-05-14 22:07 ` David Miller
0 siblings, 1 reply; 15+ messages in thread
From: Jakub Kicinski @ 2020-05-14 21:52 UTC (permalink / raw)
To: Jeff Kirsher
Cc: davem, Vitaly Lifshits, netdev, nhorman, sassmann,
kbuild test robot, Dan Carpenter, Aaron Brown
On Thu, 14 May 2020 14:31:11 -0700 Jeff Kirsher wrote:
> 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..1c4536105e56
> --- /dev/null
> +++ b/drivers/net/ethernet/intel/igc/igc_diag.c
> @@ -0,0 +1,186 @@
> +// 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 },
drivers/net/ethernet/intel/igc/igc_diag.c:7:21: warning: symbol 'reg_test' was not declared. Should it be static?
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [net-next v2 3/9] igc: add support to eeprom, registers and link self-tests
2020-05-14 21:52 ` Jakub Kicinski
@ 2020-05-14 22:07 ` David Miller
2020-05-14 22:22 ` Kirsher, Jeffrey T
0 siblings, 1 reply; 15+ messages in thread
From: David Miller @ 2020-05-14 22:07 UTC (permalink / raw)
To: kuba
Cc: jeffrey.t.kirsher, vitaly.lifshits, netdev, nhorman, sassmann,
lkp, dan.carpenter, aaron.f.brown
From: Jakub Kicinski <kuba@kernel.org>
Date: Thu, 14 May 2020 14:52:19 -0700
> On Thu, 14 May 2020 14:31:11 -0700 Jeff Kirsher wrote:
>> 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..1c4536105e56
>> --- /dev/null
>> +++ b/drivers/net/ethernet/intel/igc/igc_diag.c
>> @@ -0,0 +1,186 @@
>> +// 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 },
>
> drivers/net/ethernet/intel/igc/igc_diag.c:7:21: warning: symbol 'reg_test' was not declared. Should it be static?
Jeff, you might want to start checking this kind of stuff internally
since Jakub is going to catch it within minutes of you posting your
changes :-)))
^ permalink raw reply [flat|nested] 15+ messages in thread* RE: [net-next v2 3/9] igc: add support to eeprom, registers and link self-tests
2020-05-14 22:07 ` David Miller
@ 2020-05-14 22:22 ` Kirsher, Jeffrey T
0 siblings, 0 replies; 15+ messages in thread
From: Kirsher, Jeffrey T @ 2020-05-14 22:22 UTC (permalink / raw)
To: David Miller, kuba
Cc: Lifshits, Vitaly, netdev, nhorman, sassmann, lkp, dan.carpenter,
Brown, Aaron F
> -----Original Message-----
> From: David Miller <davem@davemloft.net>
> Sent: Thursday, May 14, 2020 15:08
> To: kuba@kernel.org
> Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; Lifshits, Vitaly
> <vitaly.lifshits@intel.com>; netdev@vger.kernel.org; nhorman@redhat.com;
> sassmann@redhat.com; lkp <lkp@intel.com>; dan.carpenter@oracle.com;
> Brown, Aaron F <aaron.f.brown@intel.com>
> Subject: Re: [net-next v2 3/9] igc: add support to eeprom, registers and link
> self-tests
>
> From: Jakub Kicinski <kuba@kernel.org>
> Date: Thu, 14 May 2020 14:52:19 -0700
>
> > On Thu, 14 May 2020 14:31:11 -0700 Jeff Kirsher wrote:
> >> 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..1c4536105e56
> >> --- /dev/null
> >> +++ b/drivers/net/ethernet/intel/igc/igc_diag.c
> >> @@ -0,0 +1,186 @@
> >> +// 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 },
> >
> > drivers/net/ethernet/intel/igc/igc_diag.c:7:21: warning: symbol 'reg_test' was
> not declared. Should it be static?
>
> Jeff, you might want to start checking this kind of stuff internally since Jakub is
> going to catch it within minutes of you posting your changes :-)))
[Kirsher, Jeffrey T]
Ugh, I have been compile testing. Since moving to Fedora 32, kernel compiles are now littered with warnings that were not showing up on the older GCC version which is no excuse, I need to work on parsing the logs better.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [net-next v2 4/9] igc: Use netdev log helpers in igc_ethtool.c
2020-05-14 21:31 [net-next v2 0/9][pull request] 1GbE Intel Wired LAN Driver Updates 2020-05-14 Jeff Kirsher
` (2 preceding siblings ...)
2020-05-14 21:31 ` [net-next v2 3/9] igc: add support to eeprom, registers and link self-tests Jeff Kirsher
@ 2020-05-14 21:31 ` Jeff Kirsher
2020-05-14 21:31 ` [net-next v2 5/9] igc: Use netdev log helpers in igc_ptp.c Jeff Kirsher
` (4 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2020-05-14 21:31 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 | 41 ++++++++++----------
1 file changed, 21 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_ethtool.c b/drivers/net/ethernet/intel/igc/igc_ethtool.c
index 9fc250cdf88c..a05d7abee524 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\n");
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\n");
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\n");
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\n");
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\n");
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\n");
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\n");
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\n");
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\n");
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\n");
}
/* 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,13 +1919,13 @@ 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;
@@ -1940,7 +1941,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.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next v2 5/9] igc: Use netdev log helpers in igc_ptp.c
2020-05-14 21:31 [net-next v2 0/9][pull request] 1GbE Intel Wired LAN Driver Updates 2020-05-14 Jeff Kirsher
` (3 preceding siblings ...)
2020-05-14 21:31 ` [net-next v2 4/9] igc: Use netdev log helpers in igc_ethtool.c Jeff Kirsher
@ 2020-05-14 21:31 ` Jeff Kirsher
2020-05-14 21:31 ` [net-next v2 6/9] igc: Use netdev log helpers in igc_dump.c Jeff Kirsher
` (3 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2020-05-14 21:31 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..1bf016398b9d 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\n");
}
}
@@ -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\n");
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\n");
} else if (adapter->ptp_clock) {
- dev_info(&adapter->pdev->dev, "added PHC on %s\n",
- adapter->netdev->name);
+ netdev_info(netdev, "PHC added\n");
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\n");
adapter->ptp_flags &= ~IGC_PTP_ENABLED;
}
}
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next v2 6/9] igc: Use netdev log helpers in igc_dump.c
2020-05-14 21:31 [net-next v2 0/9][pull request] 1GbE Intel Wired LAN Driver Updates 2020-05-14 Jeff Kirsher
` (4 preceding siblings ...)
2020-05-14 21:31 ` [net-next v2 5/9] igc: Use netdev log helpers in igc_ptp.c Jeff Kirsher
@ 2020-05-14 21:31 ` Jeff Kirsher
2020-05-14 21:31 ` [net-next v2 7/9] igc: Use netdev log helpers in igc_base.c Jeff Kirsher
` (2 subsequent siblings)
8 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2020-05-14 21:31 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..4ad32d98f77f 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\n", 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\n", 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\n",
+ 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\n");
+ netdev_info(netdev, "Queue [NTU] [NTC] [bi(ntc)->dma ] leng ntw timestamp\n");
for (n = 0; n < adapter->num_tx_queues; n++) {
struct igc_tx_buffer *buffer_info;
tx_ring = adapter->tx_ring[n];
buffer_info = &tx_ring->tx_buffer_info[tx_ring->next_to_clean];
- 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",
+ 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\n");
/* 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, "------------------------------------\n");
+ netdev_info(netdev, "TX QUEUE INDEX = %d\n",
+ tx_ring->queue_index);
+ netdev_info(netdev, "------------------------------------\n");
+ netdev_info(netdev, "T [desc] [address 63:0 ] [PlPOCIStDDM Ln] [bi->dma ] leng ntw timestamp bi->skb\n");
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\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);
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\n");
+ netdev_info(netdev, "Queue [NTU] [NTC]\n");
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", 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\n");
/* 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, "------------------------------------\n");
+ netdev_info(netdev, "RX QUEUE INDEX = %d\n",
+ rx_ring->queue_index);
+ netdev_info(netdev, "------------------------------------\n");
+ netdev_info(netdev, "R [desc] [ PktBuf A0] [ HeadBuf DD] [bi->dma ] [bi->skb] <-- Adv Rx Read format\n");
+ netdev_info(netdev, "RWB[desc] [PcsmIpSHl PtRs] [vl er S cks ln] ---------------- [bi->skb] <-- Adv Rx Write-Back format\n");
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\n",
+ "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\n",
+ "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\n");
+ netdev_info(adapter->netdev, "Register Name Value\n");
for (reginfo = (struct igc_reg_info *)igc_reg_info_tbl;
reginfo->name; reginfo++) {
igc_regdump(hw, reginfo);
--
2.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next v2 7/9] igc: Use netdev log helpers in igc_base.c
2020-05-14 21:31 [net-next v2 0/9][pull request] 1GbE Intel Wired LAN Driver Updates 2020-05-14 Jeff Kirsher
` (5 preceding siblings ...)
2020-05-14 21:31 ` [net-next v2 6/9] igc: Use netdev log helpers in igc_dump.c Jeff Kirsher
@ 2020-05-14 21:31 ` Jeff Kirsher
2020-05-14 21:31 ` [net-next v2 8/9] igc: Remove unneeded definition Jeff Kirsher
2020-05-14 21:31 ` [net-next v2 9/9] igc: Remove unneeded register Jeff Kirsher
8 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2020-05-14 21:31 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 | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/intel/igc/igc_base.c b/drivers/net/ethernet/intel/igc/igc_base.c
index f7fb18d8d8f5..cc5a6cf531c7 100644
--- a/drivers/net/ethernet/intel/igc/igc_base.c
+++ b/drivers/net/ethernet/intel/igc/igc_base.c
@@ -26,7 +26,7 @@ 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\n");
hw_dbg("Masking off all interrupts\n");
wr32(IGC_IMC, 0xffffffff);
@@ -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\n");
goto out;
}
@@ -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\n");
/* 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.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next v2 8/9] igc: Remove unneeded definition
2020-05-14 21:31 [net-next v2 0/9][pull request] 1GbE Intel Wired LAN Driver Updates 2020-05-14 Jeff Kirsher
` (6 preceding siblings ...)
2020-05-14 21:31 ` [net-next v2 7/9] igc: Use netdev log helpers in igc_base.c Jeff Kirsher
@ 2020-05-14 21:31 ` Jeff Kirsher
2020-05-14 21:31 ` [net-next v2 9/9] igc: Remove unneeded register Jeff Kirsher
8 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2020-05-14 21:31 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.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* [net-next v2 9/9] igc: Remove unneeded register
2020-05-14 21:31 [net-next v2 0/9][pull request] 1GbE Intel Wired LAN Driver Updates 2020-05-14 Jeff Kirsher
` (7 preceding siblings ...)
2020-05-14 21:31 ` [net-next v2 8/9] igc: Remove unneeded definition Jeff Kirsher
@ 2020-05-14 21:31 ` Jeff Kirsher
8 siblings, 0 replies; 15+ messages in thread
From: Jeff Kirsher @ 2020-05-14 21:31 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.26.2
^ permalink raw reply related [flat|nested] 15+ messages in thread