Re: em(4) fix for Intel I218 chip

2014-10-19 Thread Kaspars Bankovskis
spotted a small typo in a comment. 1Gpbs - 1Gbps

On Sun, Oct 12, 2014 at 09:53:28PM +0200, Claudio Jeker wrote:
 This seems to be enough to help em(4) in modern laptops like the X240 to
 no longer generate watchdog timeouts on high throughput.
 This should only affect I218 but tests on different em(4) devices would
 not hurt.
 
 -- 
 :wq Claudio
 
 
 Index: if_em_hw.c
 ===
 RCS file: /cvs/src/sys/dev/pci/if_em_hw.c,v
 retrieving revision 1.80
 diff -u -p -r1.80 if_em_hw.c
 --- if_em_hw.c22 Jul 2014 13:12:11 -  1.80
 +++ if_em_hw.c28 Sep 2014 12:24:45 -
 @@ -163,6 +163,7 @@ int32_t   em_lv_phy_workarounds_ich8lan(s
  int32_t  em_link_stall_workaround_hv(struct em_hw *);
  int32_t  em_k1_gig_workaround_hv(struct em_hw *, boolean_t);
  int32_t  em_k1_workaround_lv(struct em_hw *);
 +int32_t  em_k1_workaround_lpt_lp(struct em_hw *, boolean_t);
  int32_t  em_configure_k1_ich8lan(struct em_hw *, boolean_t);
  void em_gate_hw_phy_config_ich8lan(struct em_hw *, boolean_t);
  int32_t  em_access_phy_wakeup_reg_bm(struct em_hw *, uint32_t,
 @@ -3709,6 +3710,16 @@ em_check_for_link(struct em_hw *hw)
   if (ret_val)
   return ret_val;
   }
 + /* Work-around I218 hang issue */
 + if ((hw-device_id == E1000_DEV_ID_PCH_LPTLP_I218_LM) ||
 + (hw-device_id == E1000_DEV_ID_PCH_LPTLP_I218_V) ||
 + (hw-device_id == E1000_DEV_ID_PCH_I218_LM3) ||
 + (hw-device_id == E1000_DEV_ID_PCH_I218_V3)) {
 + ret_val = em_k1_workaround_lpt_lp(hw,
 + hw-icp__is_link_up);
 + if (ret_val)
 + return ret_val;
 + }
  
   /*
* Check if there was DownShift, must be checked
 @@ -5104,7 +5115,6 @@ em_kumeran_lock_loss_workaround(struct e
* Attempting this while link is negotiating fouled up link stability
*/
   ret_val = em_read_phy_reg(hw, PHY_STATUS, phy_data);
 - ret_val = em_read_phy_reg(hw, PHY_STATUS, phy_data);
  
   if (phy_data  MII_SR_LINK_STATUS) {
   for (cnt = 0; cnt  10; cnt++) {
 @@ -10185,6 +10195,84 @@ em_k1_workaround_lv(struct em_hw *hw)
   
   return E1000_SUCCESS;
  }
 +
 +/**
 + *  em_k1_workaround_lpt_lp - K1 workaround on Lynxpoint-LP
 + *
 + *  When K1 is enabled for 1Gbps, the MAC can miss 2 DMA completion 
 indications
 + *  preventing further DMA write requests.  Workaround the issue by disabling
 + *  the de-assertion of the clock request when in 1Gpbs mode.
 + *  Also, set appropriate Tx re-transmission timeouts for 10 and 100Half link
 + *  speeds in order to avoid Tx hangs.
 + **/
 +int32_t
 +em_k1_workaround_lpt_lp(struct em_hw *hw, boolean_t link)
 +{
 + uint32_t fextnvm6 = E1000_READ_REG(hw, FEXTNVM6);
 + uint32_t status = E1000_READ_REG(hw, STATUS);
 + int32_t ret_val = E1000_SUCCESS;
 + uint16_t reg;
 +
 + if (link  (status  E1000_STATUS_SPEED_1000)) {
 + ret_val = em_read_kmrn_reg(hw, E1000_KMRNCTRLSTA_K1_CONFIG,
 + reg);
 + if (ret_val)
 + return ret_val;
 +
 + ret_val = em_write_kmrn_reg(hw, E1000_KMRNCTRLSTA_K1_CONFIG,
 + reg  ~E1000_KMRNCTRLSTA_K1_ENABLE);
 + if (ret_val)
 + return ret_val;
 +
 + usec_delay(10);
 +
 + E1000_WRITE_REG(hw, FEXTNVM6,
 + fextnvm6 | E1000_FEXTNVM6_REQ_PLL_CLK);
 +
 + ret_val = em_write_kmrn_reg(hw, E1000_KMRNCTRLSTA_K1_CONFIG,
 + reg);
 + } else {
 + /* clear FEXTNVM6 bit 8 on link down or 10/100 */
 + fextnvm6 = ~E1000_FEXTNVM6_REQ_PLL_CLK;
 +
 + if (!link || ((status  E1000_STATUS_SPEED_100) 
 +   (status  E1000_STATUS_FD)))
 + goto update_fextnvm6;
 +
 + ret_val = em_read_phy_reg(hw, I217_INBAND_CTRL, reg);
 + if (ret_val)
 + return ret_val;
 +
 + /* Clear link status transmit timeout */
 + reg = ~I217_INBAND_CTRL_LINK_STAT_TX_TIMEOUT_MASK;
 +
 + if (status  E1000_STATUS_SPEED_100) {
 + /* Set inband Tx timeout to 5x10us for 100Half */
 + reg |= 5  I217_INBAND_CTRL_LINK_STAT_TX_TIMEOUT_SHIFT;
 +
 + /* Do not extend the K1 entry latency for 100Half */
 + fextnvm6 = ~E1000_FEXTNVM6_ENABLE_K1_ENTRY_CONDITION;
 + } else {
 + /* Set inband Tx timeout to 50x10us for 10Full/Half */
 +  

Re: em(4) fix for Intel I218 chip

2014-10-13 Thread Brad Smith

On 12/10/14 3:53 PM, Claudio Jeker wrote:

This seems to be enough to help em(4) in modern laptops like the X240 to
no longer generate watchdog timeouts on high throughput.
This should only affect I218 but tests on different em(4) devices would
not hurt.


Chunk #3 is within the ICH8/IGP3 workaround, that shouldn't be changed
like that. But the rest works for the i218. I can now actually use the
Ethernet interface on my T440s. It was too unreliable under more or
less any load.

--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.



Re: em(4) fix for Intel I218 chip

2014-10-13 Thread Claudio Jeker
On Mon, Oct 13, 2014 at 01:50:45PM -0400, Brad wrote:
 On 12/10/14 3:53 PM, Claudio Jeker wrote:
 This seems to be enough to help em(4) in modern laptops like the X240 to
 no longer generate watchdog timeouts on high throughput.
 This should only affect I218 but tests on different em(4) devices would
 not hurt.
 
 Chunk #3 is within the ICH8/IGP3 workaround, that shouldn't be changed
 like that. But the rest works for the i218. I can now actually use the
 Ethernet interface on my T440s. It was too unreliable under more or
 less any load.

Yeah, but that chunk should be commited no the less. The FreeBSD version
does not have the duplicated line and it seems like a bad merge to me.

-- 
:wq Claudio



Re: em(4) fix for Intel I218 chip

2014-10-13 Thread Brad Smith

On 13/10/14 5:09 PM, Claudio Jeker wrote:

On Mon, Oct 13, 2014 at 01:50:45PM -0400, Brad wrote:

On 12/10/14 3:53 PM, Claudio Jeker wrote:

This seems to be enough to help em(4) in modern laptops like the X240 to
no longer generate watchdog timeouts on high throughput.
This should only affect I218 but tests on different em(4) devices would
not hurt.


Chunk #3 is within the ICH8/IGP3 workaround, that shouldn't be changed
like that. But the rest works for the i218. I can now actually use the
Ethernet interface on my T440s. It was too unreliable under more or
less any load.


Yeah, but that chunk should be commited no the less. The FreeBSD version
does not have the duplicated line and it seems like a bad merge to me.


But that is not how the code is in the FreeBSD driver and does not match
the behavior used everywhere else in the driver as is.

--
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.