e1000e: don't inadvertently re-set INTX_DISABLE
Subsystems (2)
PATCH 1/4 e1000e: don't inadvertently re-set INTX_DISABLE
From: Dean Nelson <[email protected]>
Should e1000_test_msi() fail to see an msi interrupt, it attempts to fallback to legacy INTx interrupts. But an error in the code may prevent this from happening correctly.
Before calling e1000_test_msi_interrupt(), e1000_test_msi() disables SERR by clearing the SERR bit from the just read PCI_COMMAND bits as it writes them back out.
Upon return from calling e1000_test_msi_interrupt(), it re-enables SERR by writing out the version of PCI_COMMAND it had previously read.
The problem with this is that e1000_test_msi_interrupt() calls pci_disable_msi(), which eventually ends up in pci_intx(). And because pci_intx() was called with enable set to 1, the INTX_DISABLE bit gets cleared from PCI_COMMAND, which is what we want. But when we get back to e1000_test_msi(), the INTX_DISABLE bit gets inadvertently re-set because of the attempt by e1000_test_msi() to re-enable SERR.
The solution is to have e1000_test_msi() re-read the PCI_COMMAND bits as part of its attempt to re-enable SERR.
During debugging/testing of this issue I found that not all the systems I ran on had the SERR bit set to begin with. And on some of the systems the same could be said for the INTX_DISABLE bit. Needless to say these latter systems didn't have a problem falling back to legacy INTx interrupts with the code as is.
---
drivers/net/e1000e/netdev.c | 13 +++++++++---- 1 files changed, 9 insertions(+), 4 deletions(-)
drivers/net/e1000e/netdev.c +9 -4
@@ -3439,13 +3439,18 @@ static int e1000_test_msi(struct e1000_adapter *adapter)/* disable SERR in case the MSI write causes a master abort */ pci_read_config_word (adapter->pdev,PCI_COMMAND , &pci_cmd); -pci_write_config_word (adapter->pdev,PCI_COMMAND , - pci_cmd & ~PCI_COMMAND_SERR ); +if (pci_cmd &PCI_COMMAND_SERR ) +pci_write_config_word (adapter->pdev,PCI_COMMAND , + pci_cmd & ~PCI_COMMAND_SERR ); err =e1000_test_msi_interrupt (adapter); -/* restore previous setting of command word */ -pci_write_config_word (adapter->pdev,PCI_COMMAND , pci_cmd); +/* re-enable SERR */ +if (pci_cmd &PCI_COMMAND_SERR ) { +pci_read_config_word (adapter->pdev,PCI_COMMAND , &pci_cmd); + pci_cmd |=PCI_COMMAND_SERR ; +pci_write_config_word (adapter->pdev,PCI_COMMAND , pci_cmd); + }/* success ! */ if (!err)
PATCH 2/4 e1000e: suppress compile warnings on certain archs
From: Bruce Allan <[email protected]>
Commit 84f4ee902ad3ee964b7b3a13d5b7cf9c086e9916 causes compile warnings on architectures that have unsigned long long's that are not 64-bit, e.g. ia64.
---
drivers/net/e1000e/netdev.c | 38 +++++++++++++++++++++----------------- 1 files changed, 21 insertions(+), 17 deletions(-)
drivers/net/e1000e/netdev.c +21 -17
@@ -224,10 +224,10 @@ static void e1000e_dump(struct e1000_adapter *adapter) buffer_info = &tx_ring->buffer_info[tx_ring->next_to_clean];printk (KERN_INFO " %5d %5X %5X %016llX %04X %3X %016llX\n" ,0 , tx_ring->next_to_use, tx_ring->next_to_clean, - (u64 )buffer_info->dma, + (unsigned long long )buffer_info->dma, buffer_info->length, buffer_info->next_to_watch, - (u64 )buffer_info->time_stamp); + (unsigned long long )buffer_info->time_stamp);/* Print TX Rings */ if (!netif_msg_tx_done (adapter))
@@ -279,9 +279,11 @@ static void e1000e_dump(struct e1000_adapter *adapter)"%04X %3X %016llX %p" , (!(le64_to_cpu (u0->b) & (1 <<29 )) ?'l' : ((le64_to_cpu (u0->b) & (1 <<20 )) ?'d' :'c' )), i, -le64_to_cpu (u0->a),le64_to_cpu (u0->b), - (u64 )buffer_info->dma, buffer_info->length, - buffer_info->next_to_watch, (u64 )buffer_info->time_stamp, + (unsigned long long )le64_to_cpu (u0->a), + (unsigned long long )le64_to_cpu (u0->b), + (unsigned long long )buffer_info->dma, + buffer_info->length, buffer_info->next_to_watch, + (unsigned long long )buffer_info->time_stamp, buffer_info->skb);if (i == tx_ring->next_to_use && i == tx_ring->next_to_clean)printk (KERN_CONT " NTC/U\n" );
@@ -356,19 +358,19 @@ rx_ring_summary:printk (KERN_INFO "RWB[0x%03X] %016llX " "%016llX %016llX %016llX " "---------------- %p" , i, -le64_to_cpu (u1->a), -le64_to_cpu (u1->b), -le64_to_cpu (u1->c), -le64_to_cpu (u1->d), + (unsigned long long )le64_to_cpu (u1->a), + (unsigned long long )le64_to_cpu (u1->b), + (unsigned long long )le64_to_cpu (u1->c), + (unsigned long long )le64_to_cpu (u1->d), buffer_info->skb); }else {printk (KERN_INFO "R [0x%03X] %016llX " "%016llX %016llX %016llX %016llX %p" , i, -le64_to_cpu (u1->a), -le64_to_cpu (u1->b), -le64_to_cpu (u1->c), -le64_to_cpu (u1->d), - (u64 )buffer_info->dma, + (unsigned long long )le64_to_cpu (u1->a), + (unsigned long long )le64_to_cpu (u1->b), + (unsigned long long )le64_to_cpu (u1->c), + (unsigned long long )le64_to_cpu (u1->d), + (unsigned long long )buffer_info->dma, buffer_info->skb);if (netif_msg_pktdata (adapter))
@@ -405,9 +407,11 @@ rx_ring_summary: buffer_info = &rx_ring->buffer_info[i]; u0 = (struct my_u0 *)rx_desc;printk (KERN_INFO "Rl[0x%03X] %016llX %016llX " -"%016llX %p" , - i,le64_to_cpu (u0->a),le64_to_cpu (u0->b), - (u64 )buffer_info->dma, buffer_info->skb); +"%016llX %p" , i, + (unsigned long long )le64_to_cpu (u0->a), + (unsigned long long )le64_to_cpu (u0->b), + (unsigned long long )buffer_info->dma, + buffer_info->skb);if (i == rx_ring->next_to_use)printk (KERN_CONT " NTU\n" );else if (i == rx_ring->next_to_clean)
Re: [net-next-2.6 PATCH 2/4] e1000e: suppress compile warnings on certain archs
From: Jeff Kirsher <[email protected]>
Date: Tue, 29 Jun 2010 21:12:30 -0700
Quoted 8 lines
From: Bruce Allan <[email protected]>
Commit
84f4ee902ad3ee964b7b3a13d5b7cf9c086e9916causes compile warnings on architectures that have unsigned long long's that are not 64-bit, e.g. ia64.Signed-off-by Bruce Allan <[email protected]>Signed-off-by Jeff Kirsher <[email protected]>
Applied.
PATCH 3/4 e1000e: remove EEE module parameter
In reply to Jeff KirsherFrom: Bruce Allan <[email protected]>
As requested by Dave Miller. A follow-on set of patches will allow for ethtool to enable/disable the feature instead.
---
drivers/net/e1000e/param.c | 28 ---------------------------- 1 files changed, 0 insertions(+), 28 deletions(-)
drivers/net/e1000e/param.c +0 -28
@@ -161,15 +161,6 @@ E1000_PARAM(WriteProtectNVM, "Write-protect NVM [WARNING: disabling this can leaE1000_PARAM (CrcStripping,"Enable CRC Stripping, disable if your BMC needs " \"the CRC" ); -/* -* Enable/disable EEE (a.k.a. IEEE802.3az) -* -* Valid Range: 0, 1 -* -* Default Value: 1 -*/ -E1000_PARAM (EEE ,"Enable/disable on parts that support the feature" ); -struct e1000_option {enum { enable_option, range_option, list_option } type;const char *name;
@@ -486,23 +477,4 @@ void __devinit e1000e_check_options(struct e1000_adapter *adapter) } } } - {/* EEE for parts supporting the feature */ -static const struct e1000_option opt = { - .type = enable_option, - .name ="EEE Support" , - .err ="defaulting to Enabled" , - .def =OPTION_ENABLED - }; - -if (adapter->flags2 &FLAG2_HAS_EEE ) { -/* Currently only supported on 82579 */ -if (num_EEE > bd) { -unsigned int eee =EEE [bd]; -e1000_validate_option (&eee, &opt, adapter); - hw->dev_spec.ich8lan.eee_disable = !eee; - }else { - hw->dev_spec.ich8lan.eee_disable = !opt.def; - } - } - } }
Re: [net-next-2.6 PATCH 3/4] e1000e: remove EEE module parameter
From: Jeff Kirsher <[email protected]>
Date: Tue, 29 Jun 2010 21:12:52 -0700
Quoted 8 lines
From: Bruce Allan <[email protected]>
As requested by Dave Miller. A follow-on set of patches will allow for ethtool to enable/disable the feature instead.
Signed-off-by Bruce Allan <[email protected]>Tested-by Emil Tantilov <[email protected]>Signed-off-by Jeff Kirsher <[email protected]>
Applied.
PATCH 4/4 e1000e: disable EEE support by default
In reply to Jeff KirsherFrom: Bruce Allan <[email protected]>
Based on community feedback, EEE should be disabled by default until the IEEE802.3az specification has been finalized.
---
drivers/net/e1000e/ich8lan.c | 4 ++++ 1 files changed, 4 insertions(+), 0 deletions(-)
drivers/net/e1000e/ich8lan.c +4 -0
@@ -731,6 +731,10 @@ static s32 e1000_get_variants_ich8lan(struct e1000_adapter *adapter) (adapter->hw.phy.type == e1000_phy_igp_3)) adapter->flags |= FLAG_LSC_GIG_SPEED_DROP; +/* Disable EEE by default until IEEE802.3az spec is finalized */ +if (adapter->flags2 &FLAG2_HAS_EEE ) + adapter->hw.dev_spec.ich8lan.eee_disable = true; +return 0 ; }
Re: [net-next-2.6 PATCH 4/4] e1000e: disable EEE support by default
From: Jeff Kirsher <[email protected]>
Date: Tue, 29 Jun 2010 21:13:13 -0700
Quoted 9 lines
From: Bruce Allan <[email protected]>
Based on community feedback, EEE should be disabled by default until the IEEE802.3az specification has been finalized.
Signed-off-by Bruce Allan <[email protected]>Tested-by Emil Tantilov <[email protected]>Signed-off-by Jeff Kirsher <[email protected]>
Applied.
From: Jeff Kirsher <[email protected]>
Date: Tue, 29 Jun 2010 21:12:05 -0700
Quoted 5 lines
From: Dean Nelson <[email protected]>
Should
e1000_test_msi()fail to see an msi interrupt, it attempts to fallback to legacy INTx interrupts. But an error in the code may prevent this from happening correctly.
...
Quoted 9 lines
The solution is to have
e1000_test_msi()re-read thePCI_COMMANDbits as part of its attempt to re-enable SERR.During debugging/testing of this issue I found that not all the systems I ran on had the SERR bit set to begin with. And on some of the systems the same could be said for the
INTX_DISABLEbit. Needless to say these latter systems didn't have a problem falling back to legacy INTx interrupts with the code as is.
Quoted 4 lines
Signed-off-by Dean Nelson <[email protected]>Tested-by Emil Tantilov <[email protected]>Signed-off-by Jeff Kirsher <[email protected]>
Applied.