diff mbox series

[v3,11/11] igc: Add watchdog

Message ID 20180624084528.10419-1-sasha.neftin@intel.com
State RFC
Headers show
Series None | expand

Commit Message

Sasha Neftin June 24, 2018, 8:45 a.m. UTC
Code completion, remove obsolete code
Add watchdog methods

Sasha Neftin (v2):
minor cosmetic changes

Sasha Neftin (v3):
resolve conflict and code optimization

Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
---
 drivers/net/ethernet/intel/igc/e1000_defines.h |  12 ++
 drivers/net/ethernet/intel/igc/e1000_hw.h      |   2 +
 drivers/net/ethernet/intel/igc/igc.h           |  12 ++
 drivers/net/ethernet/intel/igc/igc_main.c      | 235 +++++++++++++++++++++++++
 4 files changed, 261 insertions(+)

Comments

kernel test robot June 24, 2018, 6:13 p.m. UTC | #1
Hi Sasha,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on jkirsher-next-queue/dev-queue]
[also build test WARNING on v4.18-rc2 next-20180622]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sasha-Neftin/igc-Add-skeletal-frame-for-Intel-R-2-5G-Ethernet-Controller-support/20180624-164739
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue.git dev-queue
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   drivers/net/ethernet/intel/igc/igc_main.c:2832:23: sparse: expression using sizeof(void)
   drivers/net/ethernet/intel/igc/igc_main.c:2832:23: sparse: expression using sizeof(void)
   drivers/net/ethernet/intel/igc/igc_main.c:2862:27: sparse: expression using sizeof(void)
   drivers/net/ethernet/intel/igc/igc_main.c:2862:27: sparse: expression using sizeof(void)
   drivers/net/ethernet/intel/igc/igc_main.c:2608:33: sparse: expression using sizeof(void)
   drivers/net/ethernet/intel/igc/igc_main.c:2608:33: sparse: expression using sizeof(void)
   drivers/net/ethernet/intel/igc/igc_main.c:2619:25: sparse: expression using sizeof(void)
   drivers/net/ethernet/intel/igc/igc_main.c:91:6: sparse: symbol 'igc_reset' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:111:6: sparse: symbol 'igc_power_up_link' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:154:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:154:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:154:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:173:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:173:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:173:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:183:6: sparse: symbol 'igc_free_tx_resources' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:214:6: sparse: symbol 'igc_unmap_and_free_tx_resource' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:286:5: sparse: symbol 'igc_setup_tx_resources' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:348:6: sparse: symbol 'igc_clean_rx_ring' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:407:6: sparse: symbol 'igc_free_rx_resources' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:444:5: sparse: symbol 'igc_setup_rx_resources' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:528:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:528:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:528:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:531:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:531:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:531:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:533:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:533:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:533:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:534:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:534:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:534:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:539:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:539:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:539:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:554:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:554:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:554:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:571:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:571:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:571:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:518:6: sparse: symbol 'igc_configure_rx_ring' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:607:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:607:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:607:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:611:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:611:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:611:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:613:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:613:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:613:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:615:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:615:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:615:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:618:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:618:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:618:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:626:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:626:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:626:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:598:6: sparse: symbol 'igc_configure_tx_ring' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:681:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:681:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:681:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:696:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:696:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:696:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:655:6: sparse: symbol 'igc_setup_rctl' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:709:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:709:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:709:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:720:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:720:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:720:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:703:6: sparse: symbol 'igc_setup_tctl' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:981:13: sparse: symbol 'igc_xmit_frame_ring' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:1378:6: sparse: symbol 'igc_alloc_rx_buffers' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:1769:5: sparse: symbol 'igc_up' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:1804:6: sparse: symbol 'igc_update_stats' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:1823:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:1823:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:1823:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:1837:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:1837:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:1837:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:1812:6: sparse: symbol 'igc_down' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:1874:6: sparse: symbol 'igc_reinit_locked' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:2012:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:2012:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:2012:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:2014:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:2014:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:2014:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:2064:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:2064:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:2064:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:2092:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:2092:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:2092:9:    got unsigned char [usertype] *__val
>> drivers/net/ethernet/intel/igc/igc_main.c:2337:6: sparse: symbol 'igc_has_link' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:2551:17: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:2551:17:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:2551:17:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:2553:17: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:2553:17:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:2553:17:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:3447:5: sparse: symbol 'igc_open' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:3481:5: sparse: symbol 'igc_close' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:3546:31: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:3546:31:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:3546:31:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:3649:21: sparse: incorrect type in assignment (different address spaces) @@    expected unsigned char [usertype] *hw_addr @@    got unsigned char [nounsigned char [usertype] *hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:3649:21:    expected unsigned char [usertype] *hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:3649:21:    got unsigned char [noderef] [usertype] <asn:2>*io_addr
   drivers/net/ethernet/intel/igc/igc_main.c:3707:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:3707:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:3707:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:3708:9: sparse: incorrect type in initializer (different address spaces) @@    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr @@    got deref] [usertype] <asn:2>*hw_addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:3708:9:    expected unsigned char [noderef] [usertype] <asn:2>*hw_addr
   drivers/net/ethernet/intel/igc/igc_main.c:3708:9:    got unsigned char [usertype] *__val
   drivers/net/ethernet/intel/igc/igc_main.c:3764:27: sparse: incorrect type in argument 1 (different address spaces) @@    expected void volatile [noderef] <asn:2>*addr @@    got olatile [noderef] <asn:2>*addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:3764:27:    expected void volatile [noderef] <asn:2>*addr
   drivers/net/ethernet/intel/igc/igc_main.c:3764:27:    got unsigned char [usertype] *flash_address
   drivers/net/ethernet/intel/igc/igc_main.c:3810:27: sparse: incorrect type in argument 1 (different address spaces) @@    expected void volatile [noderef] <asn:2>*addr @@    got olatile [noderef] <asn:2>*addr @@
   drivers/net/ethernet/intel/igc/igc_main.c:3810:27:    expected void volatile [noderef] <asn:2>*addr
   drivers/net/ethernet/intel/igc/igc_main.c:3810:27:    got unsigned char [usertype] *flash_address
   drivers/net/ethernet/intel/igc/igc_main.c:3829:6: sparse: symbol 'igc_set_flag_queue_pairs' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:3842:14: sparse: symbol 'igc_get_max_rss_queues' was not declared. Should it be static?
   drivers/net/ethernet/intel/igc/igc_main.c:3857:31: sparse: expression using sizeof(void)
   drivers/net/ethernet/intel/igc/igc_main.c:3857:31: sparse: expression using sizeof(void)
   include/linux/slab.h:631:13: sparse: call with no type!

Please review and possibly fold the followup patch.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot June 25, 2018, 12:30 p.m. UTC | #2
Hi Sasha,

Thank you for the patch! Perhaps something to improve:

url:    https://github.com/0day-ci/linux/commits/Sasha-Neftin/igc-Add-skeletal-frame-for-Intel-R-2-5G-Ethernet-Controller-support/20180624-164739
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jkirsher/next-queue.git dev-queue

smatch warnings:
drivers/net/ethernet/intel/igc/igc_main.c:2464 igc_watchdog_task() error: uninitialized symbol 'phy_data'.

# https://github.com/0day-ci/linux/commit/d32b0b485e39c7c5a2ef4e9484919d6fa6a6733e
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout d32b0b485e39c7c5a2ef4e9484919d6fa6a6733e
vim +/phy_data +2464 drivers/net/ethernet/intel/igc/igc_main.c

3e307bfc Sasha Neftin 2018-06-24  2382  
d32b0b48 Sasha Neftin 2018-06-24  2383  static void igc_watchdog_task(struct work_struct *work)
d32b0b48 Sasha Neftin 2018-06-24  2384  {
d32b0b48 Sasha Neftin 2018-06-24  2385  	struct igc_adapter *adapter = container_of(work,
d32b0b48 Sasha Neftin 2018-06-24  2386  						   struct igc_adapter,
d32b0b48 Sasha Neftin 2018-06-24  2387  						   watchdog_task);
d32b0b48 Sasha Neftin 2018-06-24  2388  	struct e1000_hw *hw = &adapter->hw;
d32b0b48 Sasha Neftin 2018-06-24  2389  	struct e1000_phy_info *phy = &hw->phy;
d32b0b48 Sasha Neftin 2018-06-24  2390  	struct net_device *netdev = adapter->netdev;
d32b0b48 Sasha Neftin 2018-06-24  2391  	u32 link;
d32b0b48 Sasha Neftin 2018-06-24  2392  	int i;
d32b0b48 Sasha Neftin 2018-06-24  2393  	u32 connsw;
d32b0b48 Sasha Neftin 2018-06-24  2394  	u16 phy_data, retry_count = 20;
d32b0b48 Sasha Neftin 2018-06-24  2395  
d32b0b48 Sasha Neftin 2018-06-24  2396  	link = igc_has_link(adapter);
d32b0b48 Sasha Neftin 2018-06-24  2397  
d32b0b48 Sasha Neftin 2018-06-24  2398  	if (adapter->flags & IGC_FLAG_NEED_LINK_UPDATE) {
d32b0b48 Sasha Neftin 2018-06-24  2399  		if (time_after(jiffies, (adapter->link_check_timeout + HZ)))
d32b0b48 Sasha Neftin 2018-06-24  2400  			adapter->flags &= ~IGC_FLAG_NEED_LINK_UPDATE;
d32b0b48 Sasha Neftin 2018-06-24  2401  		else
d32b0b48 Sasha Neftin 2018-06-24  2402  			link = false;
d32b0b48 Sasha Neftin 2018-06-24  2403  	}
d32b0b48 Sasha Neftin 2018-06-24  2404  
d32b0b48 Sasha Neftin 2018-06-24  2405  	/* Force link down if we have fiber to swap to */
d32b0b48 Sasha Neftin 2018-06-24  2406  	if (adapter->flags & IGC_FLAG_MAS_ENABLE) {
d32b0b48 Sasha Neftin 2018-06-24  2407  		if (hw->phy.media_type == e1000_media_type_copper) {
d32b0b48 Sasha Neftin 2018-06-24  2408  			connsw = rd32(E1000_CONNSW);
d32b0b48 Sasha Neftin 2018-06-24  2409  			if (!(connsw & E1000_CONNSW_AUTOSENSE_EN))
d32b0b48 Sasha Neftin 2018-06-24  2410  				link = 0;
d32b0b48 Sasha Neftin 2018-06-24  2411  		}
d32b0b48 Sasha Neftin 2018-06-24  2412  	}
d32b0b48 Sasha Neftin 2018-06-24  2413  	if (link) {
d32b0b48 Sasha Neftin 2018-06-24  2414  		/* Perform a reset if the media type changed. */
d32b0b48 Sasha Neftin 2018-06-24  2415  		if (hw->dev_spec._base.media_changed) {
d32b0b48 Sasha Neftin 2018-06-24  2416  			hw->dev_spec._base.media_changed = false;
d32b0b48 Sasha Neftin 2018-06-24  2417  			adapter->flags |= IGC_FLAG_MEDIA_RESET;
d32b0b48 Sasha Neftin 2018-06-24  2418  			igc_reset(adapter);
d32b0b48 Sasha Neftin 2018-06-24  2419  		}
d32b0b48 Sasha Neftin 2018-06-24  2420  
d32b0b48 Sasha Neftin 2018-06-24  2421  		if (!netif_carrier_ok(netdev)) {
d32b0b48 Sasha Neftin 2018-06-24  2422  			u32 ctrl;
d32b0b48 Sasha Neftin 2018-06-24  2423  
d32b0b48 Sasha Neftin 2018-06-24  2424  			hw->mac.ops.get_speed_and_duplex(hw,
d32b0b48 Sasha Neftin 2018-06-24  2425  							 &adapter->link_speed,
d32b0b48 Sasha Neftin 2018-06-24  2426  							 &adapter->link_duplex);
d32b0b48 Sasha Neftin 2018-06-24  2427  
d32b0b48 Sasha Neftin 2018-06-24  2428  			ctrl = rd32(E1000_CTRL);
d32b0b48 Sasha Neftin 2018-06-24  2429  			/* Links status message must follow this format */
d32b0b48 Sasha Neftin 2018-06-24  2430  			netdev_info(netdev,
d32b0b48 Sasha Neftin 2018-06-24  2431  				    "igc: %s NIC Link is Up %d Mbps %s Duplex, Flow Control: %s\n",
d32b0b48 Sasha Neftin 2018-06-24  2432  				    netdev->name,
d32b0b48 Sasha Neftin 2018-06-24  2433  				    adapter->link_speed,
d32b0b48 Sasha Neftin 2018-06-24  2434  				    adapter->link_duplex == FULL_DUPLEX ?
d32b0b48 Sasha Neftin 2018-06-24  2435  				    "Full" : "Half",
d32b0b48 Sasha Neftin 2018-06-24  2436  				    (ctrl & E1000_CTRL_TFCE) &&
d32b0b48 Sasha Neftin 2018-06-24  2437  				    (ctrl & E1000_CTRL_RFCE) ? "RX/TX" :
d32b0b48 Sasha Neftin 2018-06-24  2438  				    (ctrl & E1000_CTRL_RFCE) ?  "RX" :
d32b0b48 Sasha Neftin 2018-06-24  2439  				    (ctrl & E1000_CTRL_TFCE) ?  "TX" : "None");
d32b0b48 Sasha Neftin 2018-06-24  2440  
d32b0b48 Sasha Neftin 2018-06-24  2441  			/* check if SmartSpeed worked */
d32b0b48 Sasha Neftin 2018-06-24  2442  			igc_check_downshift(hw);
d32b0b48 Sasha Neftin 2018-06-24  2443  			if (phy->speed_downgraded)
d32b0b48 Sasha Neftin 2018-06-24  2444  				netdev_warn(netdev, "Link Speed was downgraded by SmartSpeed\n");
d32b0b48 Sasha Neftin 2018-06-24  2445  
d32b0b48 Sasha Neftin 2018-06-24  2446  			/* adjust timeout factor according to speed/duplex */
d32b0b48 Sasha Neftin 2018-06-24  2447  			adapter->tx_timeout_factor = 1;
d32b0b48 Sasha Neftin 2018-06-24  2448  			switch (adapter->link_speed) {
d32b0b48 Sasha Neftin 2018-06-24  2449  			case SPEED_10:
d32b0b48 Sasha Neftin 2018-06-24  2450  				adapter->tx_timeout_factor = 14;
d32b0b48 Sasha Neftin 2018-06-24  2451  				break;
d32b0b48 Sasha Neftin 2018-06-24  2452  			case SPEED_100:
d32b0b48 Sasha Neftin 2018-06-24  2453  				/* maybe add some timeout factor ? */
d32b0b48 Sasha Neftin 2018-06-24  2454  				break;
d32b0b48 Sasha Neftin 2018-06-24  2455  			}
d32b0b48 Sasha Neftin 2018-06-24  2456  
d32b0b48 Sasha Neftin 2018-06-24  2457  			if (adapter->link_speed != SPEED_1000)
d32b0b48 Sasha Neftin 2018-06-24  2458  				goto no_wait;
d32b0b48 Sasha Neftin 2018-06-24  2459  
d32b0b48 Sasha Neftin 2018-06-24  2460  			/* wait for Remote receiver status OK */
d32b0b48 Sasha Neftin 2018-06-24  2461  retry_read_status:
d32b0b48 Sasha Neftin 2018-06-24  2462  			if (!igc_read_phy_reg(hw, PHY_1000T_STATUS,
                                                                     ^^^^^^^^^^^^^^^^
If the function isn't implemented the this returns success without initializing "phy_data".

d32b0b48 Sasha Neftin 2018-06-24  2463  					      &phy_data)) {
d32b0b48 Sasha Neftin 2018-06-24 @2464  				if (!(phy_data & SR_1000T_REMOTE_RX_STATUS) &&
                                                                              ^^^^^^^^
d32b0b48 Sasha Neftin 2018-06-24  2465  				    retry_count) {
d32b0b48 Sasha Neftin 2018-06-24  2466  					msleep(100);
d32b0b48 Sasha Neftin 2018-06-24  2467  					retry_count--;
d32b0b48 Sasha Neftin 2018-06-24  2468  					goto retry_read_status;
d32b0b48 Sasha Neftin 2018-06-24  2469  				} else if (!retry_count) {
d32b0b48 Sasha Neftin 2018-06-24  2470  					dev_err(&adapter->pdev->dev, "exceed max 2 second\n");
d32b0b48 Sasha Neftin 2018-06-24  2471  				}
d32b0b48 Sasha Neftin 2018-06-24  2472  			} else {
d32b0b48 Sasha Neftin 2018-06-24  2473  				dev_err(&adapter->pdev->dev, "read 1000Base-T Status Reg\n");
d32b0b48 Sasha Neftin 2018-06-24  2474  			}
d32b0b48 Sasha Neftin 2018-06-24  2475  no_wait:
d32b0b48 Sasha Neftin 2018-06-24  2476  			netif_carrier_on(netdev);
d32b0b48 Sasha Neftin 2018-06-24  2477  
d32b0b48 Sasha Neftin 2018-06-24  2478  			/* link state has changed, schedule phy info update */
d32b0b48 Sasha Neftin 2018-06-24  2479  			if (!test_bit(__IGC_DOWN, &adapter->state))
d32b0b48 Sasha Neftin 2018-06-24  2480  				mod_timer(&adapter->phy_info_timer,
d32b0b48 Sasha Neftin 2018-06-24  2481  					  round_jiffies(jiffies + 2 * HZ));
d32b0b48 Sasha Neftin 2018-06-24  2482  		}
d32b0b48 Sasha Neftin 2018-06-24  2483  	} else {
d32b0b48 Sasha Neftin 2018-06-24  2484  		if (netif_carrier_ok(netdev)) {
d32b0b48 Sasha Neftin 2018-06-24  2485  			adapter->link_speed = 0;
d32b0b48 Sasha Neftin 2018-06-24  2486  			adapter->link_duplex = 0;
d32b0b48 Sasha Neftin 2018-06-24  2487  
d32b0b48 Sasha Neftin 2018-06-24  2488  			/* Links status message must follow this format */
d32b0b48 Sasha Neftin 2018-06-24  2489  			netdev_info(netdev, "igc: %s NIC Link is Down\n",
d32b0b48 Sasha Neftin 2018-06-24  2490  				    netdev->name);
d32b0b48 Sasha Neftin 2018-06-24  2491  			netif_carrier_off(netdev);
d32b0b48 Sasha Neftin 2018-06-24  2492  
d32b0b48 Sasha Neftin 2018-06-24  2493  			/* link state has changed, schedule phy info update */
d32b0b48 Sasha Neftin 2018-06-24  2494  			if (!test_bit(__IGC_DOWN, &adapter->state))
d32b0b48 Sasha Neftin 2018-06-24  2495  				mod_timer(&adapter->phy_info_timer,
d32b0b48 Sasha Neftin 2018-06-24  2496  					  round_jiffies(jiffies + 2 * HZ));
d32b0b48 Sasha Neftin 2018-06-24  2497  
d32b0b48 Sasha Neftin 2018-06-24  2498  			/* link is down, time to check for alternate media */
d32b0b48 Sasha Neftin 2018-06-24  2499  			if (adapter->flags & IGC_FLAG_MAS_ENABLE) {
d32b0b48 Sasha Neftin 2018-06-24  2500  				igc_check_swap_media(adapter);
d32b0b48 Sasha Neftin 2018-06-24  2501  				if (adapter->flags & IGC_FLAG_MEDIA_RESET) {
d32b0b48 Sasha Neftin 2018-06-24  2502  					schedule_work(&adapter->reset_task);
d32b0b48 Sasha Neftin 2018-06-24  2503  					/* return immediately */
d32b0b48 Sasha Neftin 2018-06-24  2504  					return;
d32b0b48 Sasha Neftin 2018-06-24  2505  				}
d32b0b48 Sasha Neftin 2018-06-24  2506  			}
d32b0b48 Sasha Neftin 2018-06-24  2507  
d32b0b48 Sasha Neftin 2018-06-24  2508  		/* also check for alternate media here */
d32b0b48 Sasha Neftin 2018-06-24  2509  		} else if (!netif_carrier_ok(netdev) &&
d32b0b48 Sasha Neftin 2018-06-24  2510  			   (adapter->flags & IGC_FLAG_MAS_ENABLE)) {
d32b0b48 Sasha Neftin 2018-06-24  2511  			igc_check_swap_media(adapter);
d32b0b48 Sasha Neftin 2018-06-24  2512  			if (adapter->flags & IGC_FLAG_MEDIA_RESET) {
d32b0b48 Sasha Neftin 2018-06-24  2513  				schedule_work(&adapter->reset_task);
d32b0b48 Sasha Neftin 2018-06-24  2514  				/* return immediately */
d32b0b48 Sasha Neftin 2018-06-24  2515  				return;
d32b0b48 Sasha Neftin 2018-06-24  2516  			}
d32b0b48 Sasha Neftin 2018-06-24  2517  		}
d32b0b48 Sasha Neftin 2018-06-24  2518  	}
d32b0b48 Sasha Neftin 2018-06-24  2519  
d32b0b48 Sasha Neftin 2018-06-24  2520  	spin_lock(&adapter->stats64_lock);
d32b0b48 Sasha Neftin 2018-06-24  2521  	igc_update_stats(adapter);
d32b0b48 Sasha Neftin 2018-06-24  2522  	spin_unlock(&adapter->stats64_lock);
d32b0b48 Sasha Neftin 2018-06-24  2523  
d32b0b48 Sasha Neftin 2018-06-24  2524  	for (i = 0; i < adapter->num_tx_queues; i++) {
d32b0b48 Sasha Neftin 2018-06-24  2525  		struct igc_ring *tx_ring = adapter->tx_ring[i];
d32b0b48 Sasha Neftin 2018-06-24  2526  
d32b0b48 Sasha Neftin 2018-06-24  2527  		if (!netif_carrier_ok(netdev)) {
d32b0b48 Sasha Neftin 2018-06-24  2528  			/* We've lost link, so the controller stops DMA,
d32b0b48 Sasha Neftin 2018-06-24  2529  			 * but we've got queued Tx work that's never going
d32b0b48 Sasha Neftin 2018-06-24  2530  			 * to get done, so reset controller to flush Tx.
d32b0b48 Sasha Neftin 2018-06-24  2531  			 * (Do the reset outside of interrupt context).
d32b0b48 Sasha Neftin 2018-06-24  2532  			 */
d32b0b48 Sasha Neftin 2018-06-24  2533  			if (igc_desc_unused(tx_ring) + 1 < tx_ring->count) {
d32b0b48 Sasha Neftin 2018-06-24  2534  				adapter->tx_timeout_count++;
d32b0b48 Sasha Neftin 2018-06-24  2535  				schedule_work(&adapter->reset_task);
d32b0b48 Sasha Neftin 2018-06-24  2536  				/* return immediately since reset is imminent */
d32b0b48 Sasha Neftin 2018-06-24  2537  				return;
d32b0b48 Sasha Neftin 2018-06-24  2538  			}
d32b0b48 Sasha Neftin 2018-06-24  2539  		}
d32b0b48 Sasha Neftin 2018-06-24  2540  
d32b0b48 Sasha Neftin 2018-06-24  2541  		/* Force detection of hung controller every watchdog period */
d32b0b48 Sasha Neftin 2018-06-24  2542  		set_bit(IGC_RING_FLAG_TX_DETECT_HANG, &tx_ring->flags);
d32b0b48 Sasha Neftin 2018-06-24  2543  	}
d32b0b48 Sasha Neftin 2018-06-24  2544  
d32b0b48 Sasha Neftin 2018-06-24  2545  	/* Cause software interrupt to ensure Rx ring is cleaned */
d32b0b48 Sasha Neftin 2018-06-24  2546  	if (adapter->flags & IGC_FLAG_HAS_MSIX) {
d32b0b48 Sasha Neftin 2018-06-24  2547  		u32 eics = 0;
d32b0b48 Sasha Neftin 2018-06-24  2548  
d32b0b48 Sasha Neftin 2018-06-24  2549  		for (i = 0; i < adapter->num_q_vectors; i++)
d32b0b48 Sasha Neftin 2018-06-24  2550  			eics |= adapter->q_vector[i]->eims_value;
d32b0b48 Sasha Neftin 2018-06-24  2551  		wr32(E1000_EICS, eics);
d32b0b48 Sasha Neftin 2018-06-24  2552  	} else {
d32b0b48 Sasha Neftin 2018-06-24  2553  		wr32(E1000_ICS, E1000_ICS_RXDMT0);
d32b0b48 Sasha Neftin 2018-06-24  2554  	}
d32b0b48 Sasha Neftin 2018-06-24  2555  
d32b0b48 Sasha Neftin 2018-06-24  2556  	/* Reset the timer */
d32b0b48 Sasha Neftin 2018-06-24  2557  	if (!test_bit(__IGC_DOWN, &adapter->state)) {
d32b0b48 Sasha Neftin 2018-06-24  2558  		if (adapter->flags & IGC_FLAG_NEED_LINK_UPDATE)
d32b0b48 Sasha Neftin 2018-06-24  2559  			mod_timer(&adapter->watchdog_timer,
d32b0b48 Sasha Neftin 2018-06-24  2560  				  round_jiffies(jiffies +  HZ));
d32b0b48 Sasha Neftin 2018-06-24  2561  		else
d32b0b48 Sasha Neftin 2018-06-24  2562  			mod_timer(&adapter->watchdog_timer,
d32b0b48 Sasha Neftin 2018-06-24  2563  				  round_jiffies(jiffies + 2 * HZ));
d32b0b48 Sasha Neftin 2018-06-24  2564  	}
d32b0b48 Sasha Neftin 2018-06-24  2565  }
d32b0b48 Sasha Neftin 2018-06-24  2566  

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Shannon Nelson June 28, 2018, 12:56 a.m. UTC | #3
On 6/24/2018 1:45 AM, Sasha Neftin wrote:
> Code completion, remove obsolete code
> Add watchdog methods
> 
> Sasha Neftin (v2):
> minor cosmetic changes
> 
> Sasha Neftin (v3):
> resolve conflict and code optimization
> 
> Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
> ---
>   drivers/net/ethernet/intel/igc/e1000_defines.h |  12 ++
>   drivers/net/ethernet/intel/igc/e1000_hw.h      |   2 +
>   drivers/net/ethernet/intel/igc/igc.h           |  12 ++
>   drivers/net/ethernet/intel/igc/igc_main.c      | 235 +++++++++++++++++++++++++
>   4 files changed, 261 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/igc/e1000_defines.h b/drivers/net/ethernet/intel/igc/e1000_defines.h
> index 24c24c197d6b..5e13a606712b 100644
> --- a/drivers/net/ethernet/intel/igc/e1000_defines.h
> +++ b/drivers/net/ethernet/intel/igc/e1000_defines.h
> @@ -86,6 +86,9 @@
>   #define E1000_CTRL_RFCE		0x08000000  /* Receive Flow Control enable */
>   #define E1000_CTRL_TFCE		0x10000000  /* Transmit flow control enable */
>   
> +#define E1000_CONNSW_AUTOSENSE_CONF	0x2
> +#define E1000_CONNSW_AUTOSENSE_EN	0x1
> +
>   /* PBA constants */
>   #define E1000_PBA_34K			0x0022
>   
> @@ -128,6 +131,10 @@
>   #define CR_1000T_HD_CAPS	0x0100 /* Advertise 1000T HD capability */
>   #define CR_1000T_FD_CAPS	0x0200 /* Advertise 1000T FD capability  */
>   
> +/* 1000BASE-T Status Register */
> +#define SR_1000T_REMOTE_RX_STATUS	0x1000 /* Remote receiver OK */
> +#define SR_1000T_LOCAL_RX_STATUS	0x2000 /* Local receiver OK */
> +
>   /* PHY GPY 211 registers */
>   #define STANDARD_AN_REG_MASK	0x0007 /* MMD */
>   #define ANEG_MULTIGBT_AN_CTRL	0x0020 /* MULTI GBT AN Control Register */
> @@ -270,6 +277,11 @@
>   #define E1000_IMS_RXT0		E1000_ICR_RXT0    /* Rx timer intr */
>   #define E1000_IMS_RXDMT0	E1000_ICR_RXDMT0  /* Rx desc min. threshold */
>   
> +/* Interrupt Cause Set */
> +#define E1000_ICS_LSC		E1000_ICR_LSC       /* Link Status Change */
> +#define E1000_ICS_RXDMT0	E1000_ICR_RXDMT0    /* rx desc min. threshold */
> +#define E1000_ICS_DRSTA		E1000_ICR_DRSTA     /* Device Reset Aserted */
> +
>   #define E1000_ICR_DOUTSYNC	0x10000000 /* NIC DMA out of sync */
>   #define E1000_EITR_CNT_IGNR	0x80000000 /* Don't reset counters on write */
>   #define E1000_IVAR_VALID	0x80
> diff --git a/drivers/net/ethernet/intel/igc/e1000_hw.h b/drivers/net/ethernet/intel/igc/e1000_hw.h
> index d8fc6fa9eda2..df948840df3b 100644
> --- a/drivers/net/ethernet/intel/igc/e1000_hw.h
> +++ b/drivers/net/ethernet/intel/igc/e1000_hw.h
> @@ -229,6 +229,8 @@ struct e1000_dev_spec_base {
>   	bool clear_semaphore_once;
>   	bool module_plugged;
>   	u8 media_port;
> +	bool media_changed;
> +	bool mas_capable;
>   };
>   
>   struct e1000_hw {
> diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
> index 99bc2344b7ff..36685d07c765 100644
> --- a/drivers/net/ethernet/intel/igc/igc.h
> +++ b/drivers/net/ethernet/intel/igc/igc.h
> @@ -33,6 +33,8 @@ extern char igc_driver_version[];
>   #define IGC_FLAG_HAS_MSI		BIT(0)
>   #define IGC_FLAG_QUEUE_PAIRS		BIT(4)
>   #define IGC_FLAG_NEED_LINK_UPDATE	BIT(9)
> +#define IGC_FLAG_MEDIA_RESET		BIT(10)
> +#define IGC_FLAG_MAS_ENABLE		BIT(12)
>   #define IGC_FLAG_HAS_MSIX		BIT(13)
>   #define IGC_FLAG_VLAN_PROMISC		BIT(15)
>   
> @@ -296,6 +298,7 @@ struct igc_adapter {
>   
>   	/* TX */
>   	u16 tx_work_limit;
> +	u32 tx_timeout_count;
>   	int num_tx_queues;
>   	struct igc_ring *tx_ring[IGC_MAX_TX_QUEUES];
>   
> @@ -354,6 +357,7 @@ struct igc_adapter {
>   
>   	struct igc_mac_addr *mac_table;
>   
> +	unsigned long link_check_timeout;
>   	struct e1000_info ei;
>   };
>   
> @@ -423,6 +427,14 @@ static inline unsigned int igc_rx_pg_order(struct igc_ring *ring)
>   	return 0;
>   }
>   
> +static inline s32 igc_read_phy_reg(struct e1000_hw *hw, u32 offset, u16 *data)
> +{
> +	if (hw->phy.ops.read_reg)
> +		return hw->phy.ops.read_reg(hw, offset, data);
> +
> +	return 0;
> +}
> +
>   #define igc_rx_pg_size(_ring) (PAGE_SIZE << igc_rx_pg_order(_ring))
>   
>   #define IGC_TXD_DCMD	(E1000_ADVTXD_DCMD_EOP | E1000_ADVTXD_DCMD_RS)
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 563612910b3f..7fb21af955ec 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -129,6 +129,14 @@ static void igc_power_down_link(struct igc_adapter *adapter)
>   }
>   
>   /**
> + *  Detect and switch function for Media Auto Sense
> + *  @adapter: address of the board private structure
> + **/
> +static void igc_check_swap_media(struct igc_adapter *adapter)
> +{
> +}
> +
> +/**
>    *  igc_release_hw_control - release control of the h/w to f/w
>    *  @adapter: address of board private structure
>    *
> @@ -2323,6 +2331,45 @@ static void igc_free_q_vector(struct igc_adapter *adapter, int v_idx)
>   }
>   
>   /**
> + *  igc_has_link - check shared code for link and determine up/down
> + *  @adapter: pointer to driver private info
> + **/
> +bool igc_has_link(struct igc_adapter *adapter)
> +{
> +	struct e1000_hw *hw = &adapter->hw;
> +	bool link_active = false;
> +
> +	/* get_link_status is set on LSC (link status) interrupt or
> +	 * rx sequence error interrupt.  get_link_status will stay
> +	 * false until the e1000_check_for_link establishes link
> +	 * for copper adapters ONLY
> +	 */
> +	switch (hw->phy.media_type) {
> +	case e1000_media_type_copper:
> +		if (!hw->mac.get_link_status)
> +			return true;
> +		hw->mac.ops.check_for_link(hw);
> +		link_active = !hw->mac.get_link_status;
> +		break;
> +	default:
> +	case e1000_media_type_unknown:
> +		break;
> +	}
> +
> +	if (hw->mac.type == e1000_i225 &&
> +	    hw->phy.id == I225_I_PHY_ID) {
> +		if (!netif_carrier_ok(adapter->netdev)) {
> +			adapter->flags &= ~IGC_FLAG_NEED_LINK_UPDATE;
> +		} else if (!(adapter->flags & IGC_FLAG_NEED_LINK_UPDATE)) {
> +			adapter->flags |= IGC_FLAG_NEED_LINK_UPDATE;
> +			adapter->link_check_timeout = jiffies;
> +		}
> +	}
> +
> +	return link_active;
> +}
> +
> +/**
>    *  igc_watchdog - Timer Call-back
>    *  @data: pointer to adapter cast into an unsigned long
>    **/
> @@ -2333,6 +2380,190 @@ static void igc_watchdog(struct timer_list *t)
>   	schedule_work(&adapter->watchdog_task);
>   }
>   
> +static void igc_watchdog_task(struct work_struct *work)
> +{
> +	struct igc_adapter *adapter = container_of(work,
> +						   struct igc_adapter,
> +						   watchdog_task);
> +	struct e1000_hw *hw = &adapter->hw;
> +	struct e1000_phy_info *phy = &hw->phy;
> +	struct net_device *netdev = adapter->netdev;
> +	u32 link;
> +	int i;
> +	u32 connsw;
> +	u16 phy_data, retry_count = 20;

reverse xmas tree

> +
> +	link = igc_has_link(adapter);
> +
> +	if (adapter->flags & IGC_FLAG_NEED_LINK_UPDATE) {
> +		if (time_after(jiffies, (adapter->link_check_timeout + HZ)))
> +			adapter->flags &= ~IGC_FLAG_NEED_LINK_UPDATE;
> +		else
> +			link = false;
> +	}
> +
> +	/* Force link down if we have fiber to swap to */
> +	if (adapter->flags & IGC_FLAG_MAS_ENABLE) {
> +		if (hw->phy.media_type == e1000_media_type_copper) {
> +			connsw = rd32(E1000_CONNSW);
> +			if (!(connsw & E1000_CONNSW_AUTOSENSE_EN))
> +				link = 0;
> +		}
> +	}
> +	if (link) {
> +		/* Perform a reset if the media type changed. */
> +		if (hw->dev_spec._base.media_changed) {
> +			hw->dev_spec._base.media_changed = false;
> +			adapter->flags |= IGC_FLAG_MEDIA_RESET;
> +			igc_reset(adapter);

Down below is a comment "(Do the reset outside of interrupt context)." 
and a call to schedule_work(&adapter->reset_task);  Should this happen 
here rather than a direct call to reset?

> +		}
> +
> +		if (!netif_carrier_ok(netdev)) {
> +			u32 ctrl;
> +
> +			hw->mac.ops.get_speed_and_duplex(hw,
> +							 &adapter->link_speed,
> +							 &adapter->link_duplex);
> +
> +			ctrl = rd32(E1000_CTRL);
> +			/* Links 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,
> +				    adapter->link_speed,
> +				    adapter->link_duplex == FULL_DUPLEX ?
> +				    "Full" : "Half",
> +				    (ctrl & E1000_CTRL_TFCE) &&
> +				    (ctrl & E1000_CTRL_RFCE) ? "RX/TX" :
> +				    (ctrl & E1000_CTRL_RFCE) ?  "RX" :
> +				    (ctrl & E1000_CTRL_TFCE) ?  "TX" : "None");
> +
> +			/* check if SmartSpeed worked */
> +			igc_check_downshift(hw);
> +			if (phy->speed_downgraded)
> +				netdev_warn(netdev, "Link Speed was downgraded by SmartSpeed\n");
> +
> +			/* adjust timeout factor according to speed/duplex */
> +			adapter->tx_timeout_factor = 1;
> +			switch (adapter->link_speed) {
> +			case SPEED_10:
> +				adapter->tx_timeout_factor = 14;
> +				break;
> +			case SPEED_100:
> +				/* maybe add some timeout factor ? */
> +				break;
> +			}
> +
> +			if (adapter->link_speed != SPEED_1000)
> +				goto no_wait;
> +
> +			/* wait for Remote receiver status OK */
> +retry_read_status:
> +			if (!igc_read_phy_reg(hw, PHY_1000T_STATUS,
> +					      &phy_data)) {
> +				if (!(phy_data & SR_1000T_REMOTE_RX_STATUS) &&
> +				    retry_count) {
> +					msleep(100);
> +					retry_count--;
> +					goto retry_read_status;
> +				} else if (!retry_count) {
> +					dev_err(&adapter->pdev->dev, "exceed max 2 second\n");
> +				}
> +			} else {
> +				dev_err(&adapter->pdev->dev, "read 1000Base-T Status Reg\n");
> +			}
> +no_wait:
> +			netif_carrier_on(netdev);
> +
> +			/* link state has changed, schedule phy info update */
> +			if (!test_bit(__IGC_DOWN, &adapter->state))
> +				mod_timer(&adapter->phy_info_timer,
> +					  round_jiffies(jiffies + 2 * HZ));
> +		}
> +	} else {
> +		if (netif_carrier_ok(netdev)) {
> +			adapter->link_speed = 0;
> +			adapter->link_duplex = 0;
> +
> +			/* Links status message must follow this format */

s/Links/Link/

> +			netdev_info(netdev, "igc: %s NIC Link is Down\n",
> +				    netdev->name);
> +			netif_carrier_off(netdev);
> +
> +			/* link state has changed, schedule phy info update */
> +			if (!test_bit(__IGC_DOWN, &adapter->state))
> +				mod_timer(&adapter->phy_info_timer,
> +					  round_jiffies(jiffies + 2 * HZ));
> +
> +			/* link is down, time to check for alternate media */
> +			if (adapter->flags & IGC_FLAG_MAS_ENABLE) {
> +				igc_check_swap_media(adapter);
> +				if (adapter->flags & IGC_FLAG_MEDIA_RESET) {
> +					schedule_work(&adapter->reset_task);
> +					/* return immediately */
> +					return;
> +				}
> +			}
> +
> +		/* also check for alternate media here */
> +		} else if (!netif_carrier_ok(netdev) &&
> +			   (adapter->flags & IGC_FLAG_MAS_ENABLE)) {
> +			igc_check_swap_media(adapter);
> +			if (adapter->flags & IGC_FLAG_MEDIA_RESET) {
> +				schedule_work(&adapter->reset_task);
> +				/* return immediately */
> +				return;
> +			}
> +		}
> +	}
> +
> +	spin_lock(&adapter->stats64_lock);
> +	igc_update_stats(adapter);
> +	spin_unlock(&adapter->stats64_lock);
> +
> +	for (i = 0; i < adapter->num_tx_queues; i++) {
> +		struct igc_ring *tx_ring = adapter->tx_ring[i];
> +
> +		if (!netif_carrier_ok(netdev)) {
> +			/* We've lost link, so the controller stops DMA,
> +			 * but we've got queued Tx work that's never going
> +			 * to get done, so reset controller to flush Tx.
> +			 * (Do the reset outside of interrupt context).
> +			 */
> +			if (igc_desc_unused(tx_ring) + 1 < tx_ring->count) {
> +				adapter->tx_timeout_count++;
> +				schedule_work(&adapter->reset_task);
> +				/* return immediately since reset is imminent */
> +				return;
> +			}
> +		}
> +
> +		/* Force detection of hung controller every watchdog period */
> +		set_bit(IGC_RING_FLAG_TX_DETECT_HANG, &tx_ring->flags);
> +	}
> +
> +	/* Cause software interrupt to ensure Rx ring is cleaned */
> +	if (adapter->flags & IGC_FLAG_HAS_MSIX) {
> +		u32 eics = 0;
> +
> +		for (i = 0; i < adapter->num_q_vectors; i++)
> +			eics |= adapter->q_vector[i]->eims_value;
> +		wr32(E1000_EICS, eics);
> +	} else {
> +		wr32(E1000_ICS, E1000_ICS_RXDMT0);
> +	}
> +
> +	/* Reset the timer */
> +	if (!test_bit(__IGC_DOWN, &adapter->state)) {
> +		if (adapter->flags & IGC_FLAG_NEED_LINK_UPDATE)
> +			mod_timer(&adapter->watchdog_timer,
> +				  round_jiffies(jiffies +  HZ));
> +		else
> +			mod_timer(&adapter->watchdog_timer,
> +				  round_jiffies(jiffies + 2 * HZ));
> +	}
> +}
> +
>   /**
>    *  igc_update_ring_itr - update the dynamic ITR value based on packet size
>    *  @q_vector: pointer to q_vector
> @@ -3195,6 +3426,8 @@ static int __igc_open(struct net_device *netdev, bool resuming)
>   	/* start the watchdog. */
>   	hw->mac.get_link_status = 1;
>   
> +	schedule_work(&adapter->watchdog_task);
> +
>   	return E1000_SUCCESS;
>   
>   err_set_queues:
> @@ -3477,6 +3710,7 @@ static int igc_probe(struct pci_dev *pdev,
>   	timer_setup(&adapter->watchdog_timer, igc_watchdog, 0);
>   
>   	INIT_WORK(&adapter->reset_task, igc_reset_task);
> +	INIT_WORK(&adapter->watchdog_task, igc_watchdog_task);
>   
>   	/* Initialize link properties that are user-changeable */
>   	adapter->fc_autoneg = true;
> @@ -3562,6 +3796,7 @@ static void igc_remove(struct pci_dev *pdev)
>   	del_timer_sync(&adapter->watchdog_timer);
>   
>   	cancel_work_sync(&adapter->reset_task);
> +	cancel_work_sync(&adapter->watchdog_task);
>   
>   	/* Release control of h/w to f/w.  If f/w is AMT enabled, this
>   	 * would have already happened in close and is redundant.
>
Sasha Neftin July 1, 2018, 8:56 a.m. UTC | #4
On 6/28/2018 03:56, Shannon Nelson wrote:
> On 6/24/2018 1:45 AM, Sasha Neftin wrote:
>> Code completion, remove obsolete code
>> Add watchdog methods
>>
>> Sasha Neftin (v2):
>> minor cosmetic changes
>>
>> Sasha Neftin (v3):
>> resolve conflict and code optimization
>>
>> Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
>> ---
>>   drivers/net/ethernet/intel/igc/e1000_defines.h |  12 ++
>>   drivers/net/ethernet/intel/igc/e1000_hw.h      |   2 +
>>   drivers/net/ethernet/intel/igc/igc.h           |  12 ++
>>   drivers/net/ethernet/intel/igc/igc_main.c      | 235 
>> +++++++++++++++++++++++++
>>   4 files changed, 261 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/igc/e1000_defines.h 
>> b/drivers/net/ethernet/intel/igc/e1000_defines.h
>> index 24c24c197d6b..5e13a606712b 100644
>> --- a/drivers/net/ethernet/intel/igc/e1000_defines.h
>> +++ b/drivers/net/ethernet/intel/igc/e1000_defines.h
>> @@ -86,6 +86,9 @@
>>   #define E1000_CTRL_RFCE        0x08000000  /* Receive Flow Control 
>> enable */
>>   #define E1000_CTRL_TFCE        0x10000000  /* Transmit flow control 
>> enable */
>> +#define E1000_CONNSW_AUTOSENSE_CONF    0x2
>> +#define E1000_CONNSW_AUTOSENSE_EN    0x1
>> +
>>   /* PBA constants */
>>   #define E1000_PBA_34K            0x0022
>> @@ -128,6 +131,10 @@
>>   #define CR_1000T_HD_CAPS    0x0100 /* Advertise 1000T HD capability */
>>   #define CR_1000T_FD_CAPS    0x0200 /* Advertise 1000T FD capability  */
>> +/* 1000BASE-T Status Register */
>> +#define SR_1000T_REMOTE_RX_STATUS    0x1000 /* Remote receiver OK */
>> +#define SR_1000T_LOCAL_RX_STATUS    0x2000 /* Local receiver OK */
>> +
>>   /* PHY GPY 211 registers */
>>   #define STANDARD_AN_REG_MASK    0x0007 /* MMD */
>>   #define ANEG_MULTIGBT_AN_CTRL    0x0020 /* MULTI GBT AN Control 
>> Register */
>> @@ -270,6 +277,11 @@
>>   #define E1000_IMS_RXT0        E1000_ICR_RXT0    /* Rx timer intr */
>>   #define E1000_IMS_RXDMT0    E1000_ICR_RXDMT0  /* Rx desc min. 
>> threshold */
>> +/* Interrupt Cause Set */
>> +#define E1000_ICS_LSC        E1000_ICR_LSC       /* Link Status 
>> Change */
>> +#define E1000_ICS_RXDMT0    E1000_ICR_RXDMT0    /* rx desc min. 
>> threshold */
>> +#define E1000_ICS_DRSTA        E1000_ICR_DRSTA     /* Device Reset 
>> Aserted */
>> +
>>   #define E1000_ICR_DOUTSYNC    0x10000000 /* NIC DMA out of sync */
>>   #define E1000_EITR_CNT_IGNR    0x80000000 /* Don't reset counters on 
>> write */
>>   #define E1000_IVAR_VALID    0x80
>> diff --git a/drivers/net/ethernet/intel/igc/e1000_hw.h 
>> b/drivers/net/ethernet/intel/igc/e1000_hw.h
>> index d8fc6fa9eda2..df948840df3b 100644
>> --- a/drivers/net/ethernet/intel/igc/e1000_hw.h
>> +++ b/drivers/net/ethernet/intel/igc/e1000_hw.h
>> @@ -229,6 +229,8 @@ struct e1000_dev_spec_base {
>>       bool clear_semaphore_once;
>>       bool module_plugged;
>>       u8 media_port;
>> +    bool media_changed;
>> +    bool mas_capable;
>>   };
>>   struct e1000_hw {
>> diff --git a/drivers/net/ethernet/intel/igc/igc.h 
>> b/drivers/net/ethernet/intel/igc/igc.h
>> index 99bc2344b7ff..36685d07c765 100644
>> --- a/drivers/net/ethernet/intel/igc/igc.h
>> +++ b/drivers/net/ethernet/intel/igc/igc.h
>> @@ -33,6 +33,8 @@ extern char igc_driver_version[];
>>   #define IGC_FLAG_HAS_MSI        BIT(0)
>>   #define IGC_FLAG_QUEUE_PAIRS        BIT(4)
>>   #define IGC_FLAG_NEED_LINK_UPDATE    BIT(9)
>> +#define IGC_FLAG_MEDIA_RESET        BIT(10)
>> +#define IGC_FLAG_MAS_ENABLE        BIT(12)
>>   #define IGC_FLAG_HAS_MSIX        BIT(13)
>>   #define IGC_FLAG_VLAN_PROMISC        BIT(15)
>> @@ -296,6 +298,7 @@ struct igc_adapter {
>>       /* TX */
>>       u16 tx_work_limit;
>> +    u32 tx_timeout_count;
>>       int num_tx_queues;
>>       struct igc_ring *tx_ring[IGC_MAX_TX_QUEUES];
>> @@ -354,6 +357,7 @@ struct igc_adapter {
>>       struct igc_mac_addr *mac_table;
>> +    unsigned long link_check_timeout;
>>       struct e1000_info ei;
>>   };
>> @@ -423,6 +427,14 @@ static inline unsigned int igc_rx_pg_order(struct 
>> igc_ring *ring)
>>       return 0;
>>   }
>> +static inline s32 igc_read_phy_reg(struct e1000_hw *hw, u32 offset, 
>> u16 *data)
>> +{
>> +    if (hw->phy.ops.read_reg)
>> +        return hw->phy.ops.read_reg(hw, offset, data);
>> +
>> +    return 0;
>> +}
>> +
>>   #define igc_rx_pg_size(_ring) (PAGE_SIZE << igc_rx_pg_order(_ring))
>>   #define IGC_TXD_DCMD    (E1000_ADVTXD_DCMD_EOP | E1000_ADVTXD_DCMD_RS)
>> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c 
>> b/drivers/net/ethernet/intel/igc/igc_main.c
>> index 563612910b3f..7fb21af955ec 100644
>> --- a/drivers/net/ethernet/intel/igc/igc_main.c
>> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
>> @@ -129,6 +129,14 @@ static void igc_power_down_link(struct 
>> igc_adapter *adapter)
>>   }
>>   /**
>> + *  Detect and switch function for Media Auto Sense
>> + *  @adapter: address of the board private structure
>> + **/
>> +static void igc_check_swap_media(struct igc_adapter *adapter)
>> +{
>> +}
>> +
>> +/**
>>    *  igc_release_hw_control - release control of the h/w to f/w
>>    *  @adapter: address of board private structure
>>    *
>> @@ -2323,6 +2331,45 @@ static void igc_free_q_vector(struct 
>> igc_adapter *adapter, int v_idx)
>>   }
>>   /**
>> + *  igc_has_link - check shared code for link and determine up/down
>> + *  @adapter: pointer to driver private info
>> + **/
>> +bool igc_has_link(struct igc_adapter *adapter)
>> +{
>> +    struct e1000_hw *hw = &adapter->hw;
>> +    bool link_active = false;
>> +
>> +    /* get_link_status is set on LSC (link status) interrupt or
>> +     * rx sequence error interrupt.  get_link_status will stay
>> +     * false until the e1000_check_for_link establishes link
>> +     * for copper adapters ONLY
>> +     */
>> +    switch (hw->phy.media_type) {
>> +    case e1000_media_type_copper:
>> +        if (!hw->mac.get_link_status)
>> +            return true;
>> +        hw->mac.ops.check_for_link(hw);
>> +        link_active = !hw->mac.get_link_status;
>> +        break;
>> +    default:
>> +    case e1000_media_type_unknown:
>> +        break;
>> +    }
>> +
>> +    if (hw->mac.type == e1000_i225 &&
>> +        hw->phy.id == I225_I_PHY_ID) {
>> +        if (!netif_carrier_ok(adapter->netdev)) {
>> +            adapter->flags &= ~IGC_FLAG_NEED_LINK_UPDATE;
>> +        } else if (!(adapter->flags & IGC_FLAG_NEED_LINK_UPDATE)) {
>> +            adapter->flags |= IGC_FLAG_NEED_LINK_UPDATE;
>> +            adapter->link_check_timeout = jiffies;
>> +        }
>> +    }
>> +
>> +    return link_active;
>> +}
>> +
>> +/**
>>    *  igc_watchdog - Timer Call-back
>>    *  @data: pointer to adapter cast into an unsigned long
>>    **/
>> @@ -2333,6 +2380,190 @@ static void igc_watchdog(struct timer_list *t)
>>       schedule_work(&adapter->watchdog_task);
>>   }
>> +static void igc_watchdog_task(struct work_struct *work)
>> +{
>> +    struct igc_adapter *adapter = container_of(work,
>> +                           struct igc_adapter,
>> +                           watchdog_task);
>> +    struct e1000_hw *hw = &adapter->hw;
>> +    struct e1000_phy_info *phy = &hw->phy;
>> +    struct net_device *netdev = adapter->netdev;
>> +    u32 link;
>> +    int i;
>> +    u32 connsw;
>> +    u16 phy_data, retry_count = 20;
> 
> reverse xmas tree
> 
Good. Will be applied to v4.
>> +
>> +    link = igc_has_link(adapter);
>> +
>> +    if (adapter->flags & IGC_FLAG_NEED_LINK_UPDATE) {
>> +        if (time_after(jiffies, (adapter->link_check_timeout + HZ)))
>> +            adapter->flags &= ~IGC_FLAG_NEED_LINK_UPDATE;
>> +        else
>> +            link = false;
>> +    }
>> +
>> +    /* Force link down if we have fiber to swap to */
>> +    if (adapter->flags & IGC_FLAG_MAS_ENABLE) {
>> +        if (hw->phy.media_type == e1000_media_type_copper) {
>> +            connsw = rd32(E1000_CONNSW);
>> +            if (!(connsw & E1000_CONNSW_AUTOSENSE_EN))
>> +                link = 0;
>> +        }
>> +    }
>> +    if (link) {
>> +        /* Perform a reset if the media type changed. */
>> +        if (hw->dev_spec._base.media_changed) {
>> +            hw->dev_spec._base.media_changed = false;
>> +            adapter->flags |= IGC_FLAG_MEDIA_RESET;
>> +            igc_reset(adapter);
> 
> Down below is a comment "(Do the reset outside of interrupt context)." 
> and a call to schedule_work(&adapter->reset_task);  Should this happen 
> here rather than a direct call to reset?
> 
Ah... Good point. I will re check that with our design. In case our 
product won't be support another media type I consider remove this 
condition and optimize code of watchdog.
>> +        }
>> +
>> +        if (!netif_carrier_ok(netdev)) {
>> +            u32 ctrl;
>> +
>> +            hw->mac.ops.get_speed_and_duplex(hw,
>> +                             &adapter->link_speed,
>> +                             &adapter->link_duplex);
>> +
>> +            ctrl = rd32(E1000_CTRL);
>> +            /* Links 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,
>> +                    adapter->link_speed,
>> +                    adapter->link_duplex == FULL_DUPLEX ?
>> +                    "Full" : "Half",
>> +                    (ctrl & E1000_CTRL_TFCE) &&
>> +                    (ctrl & E1000_CTRL_RFCE) ? "RX/TX" :
>> +                    (ctrl & E1000_CTRL_RFCE) ?  "RX" :
>> +                    (ctrl & E1000_CTRL_TFCE) ?  "TX" : "None");
>> +
>> +            /* check if SmartSpeed worked */
>> +            igc_check_downshift(hw);
>> +            if (phy->speed_downgraded)
>> +                netdev_warn(netdev, "Link Speed was downgraded by 
>> SmartSpeed\n");
>> +
>> +            /* adjust timeout factor according to speed/duplex */
>> +            adapter->tx_timeout_factor = 1;
>> +            switch (adapter->link_speed) {
>> +            case SPEED_10:
>> +                adapter->tx_timeout_factor = 14;
>> +                break;
>> +            case SPEED_100:
>> +                /* maybe add some timeout factor ? */
>> +                break;
>> +            }
>> +
>> +            if (adapter->link_speed != SPEED_1000)
>> +                goto no_wait;
>> +
>> +            /* wait for Remote receiver status OK */
>> +retry_read_status:
>> +            if (!igc_read_phy_reg(hw, PHY_1000T_STATUS,
>> +                          &phy_data)) {
>> +                if (!(phy_data & SR_1000T_REMOTE_RX_STATUS) &&
>> +                    retry_count) {
>> +                    msleep(100);
>> +                    retry_count--;
>> +                    goto retry_read_status;
>> +                } else if (!retry_count) {
>> +                    dev_err(&adapter->pdev->dev, "exceed max 2 
>> second\n");
>> +                }
>> +            } else {
>> +                dev_err(&adapter->pdev->dev, "read 1000Base-T Status 
>> Reg\n");
>> +            }
>> +no_wait:
>> +            netif_carrier_on(netdev);
>> +
>> +            /* link state has changed, schedule phy info update */
>> +            if (!test_bit(__IGC_DOWN, &adapter->state))
>> +                mod_timer(&adapter->phy_info_timer,
>> +                      round_jiffies(jiffies + 2 * HZ));
>> +        }
>> +    } else {
>> +        if (netif_carrier_ok(netdev)) {
>> +            adapter->link_speed = 0;
>> +            adapter->link_duplex = 0;
>> +
>> +            /* Links status message must follow this format */
> 
> s/Links/Link/
> 
Good. Will be applied in v4.
>> +            netdev_info(netdev, "igc: %s NIC Link is Down\n",
>> +                    netdev->name);
>> +            netif_carrier_off(netdev);
>> +
>> +            /* link state has changed, schedule phy info update */
>> +            if (!test_bit(__IGC_DOWN, &adapter->state))
>> +                mod_timer(&adapter->phy_info_timer,
>> +                      round_jiffies(jiffies + 2 * HZ));
>> +
>> +            /* link is down, time to check for alternate media */
>> +            if (adapter->flags & IGC_FLAG_MAS_ENABLE) {
>> +                igc_check_swap_media(adapter);
>> +                if (adapter->flags & IGC_FLAG_MEDIA_RESET) {
>> +                    schedule_work(&adapter->reset_task);
>> +                    /* return immediately */
>> +                    return;
>> +                }
>> +            }
>> +
>> +        /* also check for alternate media here */
>> +        } else if (!netif_carrier_ok(netdev) &&
>> +               (adapter->flags & IGC_FLAG_MAS_ENABLE)) {
>> +            igc_check_swap_media(adapter);
>> +            if (adapter->flags & IGC_FLAG_MEDIA_RESET) {
>> +                schedule_work(&adapter->reset_task);
>> +                /* return immediately */
>> +                return;
>> +            }
>> +        }
>> +    }
>> +
>> +    spin_lock(&adapter->stats64_lock);
>> +    igc_update_stats(adapter);
>> +    spin_unlock(&adapter->stats64_lock);
>> +
>> +    for (i = 0; i < adapter->num_tx_queues; i++) {
>> +        struct igc_ring *tx_ring = adapter->tx_ring[i];
>> +
>> +        if (!netif_carrier_ok(netdev)) {
>> +            /* We've lost link, so the controller stops DMA,
>> +             * but we've got queued Tx work that's never going
>> +             * to get done, so reset controller to flush Tx.
>> +             * (Do the reset outside of interrupt context).
>> +             */
>> +            if (igc_desc_unused(tx_ring) + 1 < tx_ring->count) {
>> +                adapter->tx_timeout_count++;
>> +                schedule_work(&adapter->reset_task);
>> +                /* return immediately since reset is imminent */
>> +                return;
>> +            }
>> +        }
>> +
>> +        /* Force detection of hung controller every watchdog period */
>> +        set_bit(IGC_RING_FLAG_TX_DETECT_HANG, &tx_ring->flags);
>> +    }
>> +
>> +    /* Cause software interrupt to ensure Rx ring is cleaned */
>> +    if (adapter->flags & IGC_FLAG_HAS_MSIX) {
>> +        u32 eics = 0;
>> +
>> +        for (i = 0; i < adapter->num_q_vectors; i++)
>> +            eics |= adapter->q_vector[i]->eims_value;
>> +        wr32(E1000_EICS, eics);
>> +    } else {
>> +        wr32(E1000_ICS, E1000_ICS_RXDMT0);
>> +    }
>> +
>> +    /* Reset the timer */
>> +    if (!test_bit(__IGC_DOWN, &adapter->state)) {
>> +        if (adapter->flags & IGC_FLAG_NEED_LINK_UPDATE)
>> +            mod_timer(&adapter->watchdog_timer,
>> +                  round_jiffies(jiffies +  HZ));
>> +        else
>> +            mod_timer(&adapter->watchdog_timer,
>> +                  round_jiffies(jiffies + 2 * HZ));
>> +    }
>> +}
>> +
>>   /**
>>    *  igc_update_ring_itr - update the dynamic ITR value based on 
>> packet size
>>    *  @q_vector: pointer to q_vector
>> @@ -3195,6 +3426,8 @@ static int __igc_open(struct net_device *netdev, 
>> bool resuming)
>>       /* start the watchdog. */
>>       hw->mac.get_link_status = 1;
>> +    schedule_work(&adapter->watchdog_task);
>> +
>>       return E1000_SUCCESS;
>>   err_set_queues:
>> @@ -3477,6 +3710,7 @@ static int igc_probe(struct pci_dev *pdev,
>>       timer_setup(&adapter->watchdog_timer, igc_watchdog, 0);
>>       INIT_WORK(&adapter->reset_task, igc_reset_task);
>> +    INIT_WORK(&adapter->watchdog_task, igc_watchdog_task);
>>       /* Initialize link properties that are user-changeable */
>>       adapter->fc_autoneg = true;
>> @@ -3562,6 +3796,7 @@ static void igc_remove(struct pci_dev *pdev)
>>       del_timer_sync(&adapter->watchdog_timer);
>>       cancel_work_sync(&adapter->reset_task);
>> +    cancel_work_sync(&adapter->watchdog_task);
>>       /* Release control of h/w to f/w.  If f/w is AMT enabled, this
>>        * would have already happened in close and is redundant.
>>
Hello Shannon,
Thanks for your comments.
Thanks,
Sasha
Sasha Neftin July 2, 2018, 7:40 a.m. UTC | #5
On 7/1/2018 11:56, Neftin, Sasha wrote:
> On 6/28/2018 03:56, Shannon Nelson wrote:
>> On 6/24/2018 1:45 AM, Sasha Neftin wrote:
>>> Code completion, remove obsolete code
>>> Add watchdog methods
>>>
>>> Sasha Neftin (v2):
>>> minor cosmetic changes
>>>
>>> Sasha Neftin (v3):
>>> resolve conflict and code optimization
>>>
>>> Signed-off-by: Sasha Neftin <sasha.neftin@intel.com>
>>> ---
>>>   drivers/net/ethernet/intel/igc/e1000_defines.h |  12 ++
>>>   drivers/net/ethernet/intel/igc/e1000_hw.h      |   2 +
>>>   drivers/net/ethernet/intel/igc/igc.h           |  12 ++
>>>   drivers/net/ethernet/intel/igc/igc_main.c      | 235 
>>> +++++++++++++++++++++++++
>>>   4 files changed, 261 insertions(+)
>>>
>>> diff --git a/drivers/net/ethernet/intel/igc/e1000_defines.h 
>>> b/drivers/net/ethernet/intel/igc/e1000_defines.h
>>> index 24c24c197d6b..5e13a606712b 100644
>>> --- a/drivers/net/ethernet/intel/igc/e1000_defines.h
>>> +++ b/drivers/net/ethernet/intel/igc/e1000_defines.h
>>> @@ -86,6 +86,9 @@
>>>   #define E1000_CTRL_RFCE        0x08000000  /* Receive Flow Control 
>>> enable */
>>>   #define E1000_CTRL_TFCE        0x10000000  /* Transmit flow control 
>>> enable */
>>> +#define E1000_CONNSW_AUTOSENSE_CONF    0x2
>>> +#define E1000_CONNSW_AUTOSENSE_EN    0x1
>>> +
>>>   /* PBA constants */
>>>   #define E1000_PBA_34K            0x0022
>>> @@ -128,6 +131,10 @@
>>>   #define CR_1000T_HD_CAPS    0x0100 /* Advertise 1000T HD capability */
>>>   #define CR_1000T_FD_CAPS    0x0200 /* Advertise 1000T FD 
>>> capability  */
>>> +/* 1000BASE-T Status Register */
>>> +#define SR_1000T_REMOTE_RX_STATUS    0x1000 /* Remote receiver OK */
>>> +#define SR_1000T_LOCAL_RX_STATUS    0x2000 /* Local receiver OK */
>>> +
>>>   /* PHY GPY 211 registers */
>>>   #define STANDARD_AN_REG_MASK    0x0007 /* MMD */
>>>   #define ANEG_MULTIGBT_AN_CTRL    0x0020 /* MULTI GBT AN Control 
>>> Register */
>>> @@ -270,6 +277,11 @@
>>>   #define E1000_IMS_RXT0        E1000_ICR_RXT0    /* Rx timer intr */
>>>   #define E1000_IMS_RXDMT0    E1000_ICR_RXDMT0  /* Rx desc min. 
>>> threshold */
>>> +/* Interrupt Cause Set */
>>> +#define E1000_ICS_LSC        E1000_ICR_LSC       /* Link Status 
>>> Change */
>>> +#define E1000_ICS_RXDMT0    E1000_ICR_RXDMT0    /* rx desc min. 
>>> threshold */
>>> +#define E1000_ICS_DRSTA        E1000_ICR_DRSTA     /* Device Reset 
>>> Aserted */
>>> +
>>>   #define E1000_ICR_DOUTSYNC    0x10000000 /* NIC DMA out of sync */
>>>   #define E1000_EITR_CNT_IGNR    0x80000000 /* Don't reset counters 
>>> on write */
>>>   #define E1000_IVAR_VALID    0x80
>>> diff --git a/drivers/net/ethernet/intel/igc/e1000_hw.h 
>>> b/drivers/net/ethernet/intel/igc/e1000_hw.h
>>> index d8fc6fa9eda2..df948840df3b 100644
>>> --- a/drivers/net/ethernet/intel/igc/e1000_hw.h
>>> +++ b/drivers/net/ethernet/intel/igc/e1000_hw.h
>>> @@ -229,6 +229,8 @@ struct e1000_dev_spec_base {
>>>       bool clear_semaphore_once;
>>>       bool module_plugged;
>>>       u8 media_port;
>>> +    bool media_changed;
>>> +    bool mas_capable;
>>>   };
>>>   struct e1000_hw {
>>> diff --git a/drivers/net/ethernet/intel/igc/igc.h 
>>> b/drivers/net/ethernet/intel/igc/igc.h
>>> index 99bc2344b7ff..36685d07c765 100644
>>> --- a/drivers/net/ethernet/intel/igc/igc.h
>>> +++ b/drivers/net/ethernet/intel/igc/igc.h
>>> @@ -33,6 +33,8 @@ extern char igc_driver_version[];
>>>   #define IGC_FLAG_HAS_MSI        BIT(0)
>>>   #define IGC_FLAG_QUEUE_PAIRS        BIT(4)
>>>   #define IGC_FLAG_NEED_LINK_UPDATE    BIT(9)
>>> +#define IGC_FLAG_MEDIA_RESET        BIT(10)
>>> +#define IGC_FLAG_MAS_ENABLE        BIT(12)
>>>   #define IGC_FLAG_HAS_MSIX        BIT(13)
>>>   #define IGC_FLAG_VLAN_PROMISC        BIT(15)
>>> @@ -296,6 +298,7 @@ struct igc_adapter {
>>>       /* TX */
>>>       u16 tx_work_limit;
>>> +    u32 tx_timeout_count;
>>>       int num_tx_queues;
>>>       struct igc_ring *tx_ring[IGC_MAX_TX_QUEUES];
>>> @@ -354,6 +357,7 @@ struct igc_adapter {
>>>       struct igc_mac_addr *mac_table;
>>> +    unsigned long link_check_timeout;
>>>       struct e1000_info ei;
>>>   };
>>> @@ -423,6 +427,14 @@ static inline unsigned int 
>>> igc_rx_pg_order(struct igc_ring *ring)
>>>       return 0;
>>>   }
>>> +static inline s32 igc_read_phy_reg(struct e1000_hw *hw, u32 offset, 
>>> u16 *data)
>>> +{
>>> +    if (hw->phy.ops.read_reg)
>>> +        return hw->phy.ops.read_reg(hw, offset, data);
>>> +
>>> +    return 0;
>>> +}
>>> +
>>>   #define igc_rx_pg_size(_ring) (PAGE_SIZE << igc_rx_pg_order(_ring))
>>>   #define IGC_TXD_DCMD    (E1000_ADVTXD_DCMD_EOP | E1000_ADVTXD_DCMD_RS)
>>> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c 
>>> b/drivers/net/ethernet/intel/igc/igc_main.c
>>> index 563612910b3f..7fb21af955ec 100644
>>> --- a/drivers/net/ethernet/intel/igc/igc_main.c
>>> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
>>> @@ -129,6 +129,14 @@ static void igc_power_down_link(struct 
>>> igc_adapter *adapter)
>>>   }
>>>   /**
>>> + *  Detect and switch function for Media Auto Sense
>>> + *  @adapter: address of the board private structure
>>> + **/
>>> +static void igc_check_swap_media(struct igc_adapter *adapter)
>>> +{
>>> +}
>>> +
>>> +/**
>>>    *  igc_release_hw_control - release control of the h/w to f/w
>>>    *  @adapter: address of board private structure
>>>    *
>>> @@ -2323,6 +2331,45 @@ static void igc_free_q_vector(struct 
>>> igc_adapter *adapter, int v_idx)
>>>   }
>>>   /**
>>> + *  igc_has_link - check shared code for link and determine up/down
>>> + *  @adapter: pointer to driver private info
>>> + **/
>>> +bool igc_has_link(struct igc_adapter *adapter)
>>> +{
>>> +    struct e1000_hw *hw = &adapter->hw;
>>> +    bool link_active = false;
>>> +
>>> +    /* get_link_status is set on LSC (link status) interrupt or
>>> +     * rx sequence error interrupt.  get_link_status will stay
>>> +     * false until the e1000_check_for_link establishes link
>>> +     * for copper adapters ONLY
>>> +     */
>>> +    switch (hw->phy.media_type) {
>>> +    case e1000_media_type_copper:
>>> +        if (!hw->mac.get_link_status)
>>> +            return true;
>>> +        hw->mac.ops.check_for_link(hw);
>>> +        link_active = !hw->mac.get_link_status;
>>> +        break;
>>> +    default:
>>> +    case e1000_media_type_unknown:
>>> +        break;
>>> +    }
>>> +
>>> +    if (hw->mac.type == e1000_i225 &&
>>> +        hw->phy.id == I225_I_PHY_ID) {
>>> +        if (!netif_carrier_ok(adapter->netdev)) {
>>> +            adapter->flags &= ~IGC_FLAG_NEED_LINK_UPDATE;
>>> +        } else if (!(adapter->flags & IGC_FLAG_NEED_LINK_UPDATE)) {
>>> +            adapter->flags |= IGC_FLAG_NEED_LINK_UPDATE;
>>> +            adapter->link_check_timeout = jiffies;
>>> +        }
>>> +    }
>>> +
>>> +    return link_active;
>>> +}
>>> +
>>> +/**
>>>    *  igc_watchdog - Timer Call-back
>>>    *  @data: pointer to adapter cast into an unsigned long
>>>    **/
>>> @@ -2333,6 +2380,190 @@ static void igc_watchdog(struct timer_list *t)
>>>       schedule_work(&adapter->watchdog_task);
>>>   }
>>> +static void igc_watchdog_task(struct work_struct *work)
>>> +{
>>> +    struct igc_adapter *adapter = container_of(work,
>>> +                           struct igc_adapter,
>>> +                           watchdog_task);
>>> +    struct e1000_hw *hw = &adapter->hw;
>>> +    struct e1000_phy_info *phy = &hw->phy;
>>> +    struct net_device *netdev = adapter->netdev;
>>> +    u32 link;
>>> +    int i;
>>> +    u32 connsw;
>>> +    u16 phy_data, retry_count = 20;
>>
>> reverse xmas tree
>>
> Good. Will be applied to v4.
>>> +
>>> +    link = igc_has_link(adapter);
>>> +
>>> +    if (adapter->flags & IGC_FLAG_NEED_LINK_UPDATE) {
>>> +        if (time_after(jiffies, (adapter->link_check_timeout + HZ)))
>>> +            adapter->flags &= ~IGC_FLAG_NEED_LINK_UPDATE;
>>> +        else
>>> +            link = false;
>>> +    }
>>> +
>>> +    /* Force link down if we have fiber to swap to */
>>> +    if (adapter->flags & IGC_FLAG_MAS_ENABLE) {
>>> +        if (hw->phy.media_type == e1000_media_type_copper) {
>>> +            connsw = rd32(E1000_CONNSW);
>>> +            if (!(connsw & E1000_CONNSW_AUTOSENSE_EN))
>>> +                link = 0;
>>> +        }
>>> +    }
>>> +    if (link) {
>>> +        /* Perform a reset if the media type changed. */
>>> +        if (hw->dev_spec._base.media_changed) {
>>> +            hw->dev_spec._base.media_changed = false;
>>> +            adapter->flags |= IGC_FLAG_MEDIA_RESET;
>>> +            igc_reset(adapter);
>>
>> Down below is a comment "(Do the reset outside of interrupt context)." 
>> and a call to schedule_work(&adapter->reset_task);  Should this happen 
>> here rather than a direct call to reset?
>>
> Ah... Good point. I will re check that with our design. In case our 
> product won't be support another media type I consider remove this 
> condition and optimize code of watchdog.This code will be gone in v4.
>>> +        }
>>> +
>>> +        if (!netif_carrier_ok(netdev)) {
>>> +            u32 ctrl;
>>> +
>>> +            hw->mac.ops.get_speed_and_duplex(hw,
>>> +                             &adapter->link_speed,
>>> +                             &adapter->link_duplex);
>>> +
>>> +            ctrl = rd32(E1000_CTRL);
>>> +            /* Links 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,
>>> +                    adapter->link_speed,
>>> +                    adapter->link_duplex == FULL_DUPLEX ?
>>> +                    "Full" : "Half",
>>> +                    (ctrl & E1000_CTRL_TFCE) &&
>>> +                    (ctrl & E1000_CTRL_RFCE) ? "RX/TX" :
>>> +                    (ctrl & E1000_CTRL_RFCE) ?  "RX" :
>>> +                    (ctrl & E1000_CTRL_TFCE) ?  "TX" : "None");
>>> +
>>> +            /* check if SmartSpeed worked */
>>> +            igc_check_downshift(hw);
>>> +            if (phy->speed_downgraded)
>>> +                netdev_warn(netdev, "Link Speed was downgraded by 
>>> SmartSpeed\n");
>>> +
>>> +            /* adjust timeout factor according to speed/duplex */
>>> +            adapter->tx_timeout_factor = 1;
>>> +            switch (adapter->link_speed) {
>>> +            case SPEED_10:
>>> +                adapter->tx_timeout_factor = 14;
>>> +                break;
>>> +            case SPEED_100:
>>> +                /* maybe add some timeout factor ? */
>>> +                break;
>>> +            }
>>> +
>>> +            if (adapter->link_speed != SPEED_1000)
>>> +                goto no_wait;
>>> +
>>> +            /* wait for Remote receiver status OK */
>>> +retry_read_status:
>>> +            if (!igc_read_phy_reg(hw, PHY_1000T_STATUS,
>>> +                          &phy_data)) {
>>> +                if (!(phy_data & SR_1000T_REMOTE_RX_STATUS) &&
>>> +                    retry_count) {
>>> +                    msleep(100);
>>> +                    retry_count--;
>>> +                    goto retry_read_status;
>>> +                } else if (!retry_count) {
>>> +                    dev_err(&adapter->pdev->dev, "exceed max 2 
>>> second\n");
>>> +                }
>>> +            } else {
>>> +                dev_err(&adapter->pdev->dev, "read 1000Base-T Status 
>>> Reg\n");
>>> +            }
>>> +no_wait:
>>> +            netif_carrier_on(netdev);
>>> +
>>> +            /* link state has changed, schedule phy info update */
>>> +            if (!test_bit(__IGC_DOWN, &adapter->state))
>>> +                mod_timer(&adapter->phy_info_timer,
>>> +                      round_jiffies(jiffies + 2 * HZ));
>>> +        }
>>> +    } else {
>>> +        if (netif_carrier_ok(netdev)) {
>>> +            adapter->link_speed = 0;
>>> +            adapter->link_duplex = 0;
>>> +
>>> +            /* Links status message must follow this format */
>>
>> s/Links/Link/
>>
> Good. Will be applied in v4.
>>> +            netdev_info(netdev, "igc: %s NIC Link is Down\n",
>>> +                    netdev->name);
>>> +            netif_carrier_off(netdev);
>>> +
>>> +            /* link state has changed, schedule phy info update */
>>> +            if (!test_bit(__IGC_DOWN, &adapter->state))
>>> +                mod_timer(&adapter->phy_info_timer,
>>> +                      round_jiffies(jiffies + 2 * HZ));
>>> +
>>> +            /* link is down, time to check for alternate media */
>>> +            if (adapter->flags & IGC_FLAG_MAS_ENABLE) {
>>> +                igc_check_swap_media(adapter);
>>> +                if (adapter->flags & IGC_FLAG_MEDIA_RESET) {
>>> +                    schedule_work(&adapter->reset_task);
>>> +                    /* return immediately */
>>> +                    return;
>>> +                }
>>> +            }
>>> +
>>> +        /* also check for alternate media here */
>>> +        } else if (!netif_carrier_ok(netdev) &&
>>> +               (adapter->flags & IGC_FLAG_MAS_ENABLE)) {
>>> +            igc_check_swap_media(adapter);
>>> +            if (adapter->flags & IGC_FLAG_MEDIA_RESET) {
>>> +                schedule_work(&adapter->reset_task);
>>> +                /* return immediately */
>>> +                return;
>>> +            }
>>> +        }
>>> +    }
>>> +
>>> +    spin_lock(&adapter->stats64_lock);
>>> +    igc_update_stats(adapter);
>>> +    spin_unlock(&adapter->stats64_lock);
>>> +
>>> +    for (i = 0; i < adapter->num_tx_queues; i++) {
>>> +        struct igc_ring *tx_ring = adapter->tx_ring[i];
>>> +
>>> +        if (!netif_carrier_ok(netdev)) {
>>> +            /* We've lost link, so the controller stops DMA,
>>> +             * but we've got queued Tx work that's never going
>>> +             * to get done, so reset controller to flush Tx.
>>> +             * (Do the reset outside of interrupt context).
>>> +             */
>>> +            if (igc_desc_unused(tx_ring) + 1 < tx_ring->count) {
>>> +                adapter->tx_timeout_count++;
>>> +                schedule_work(&adapter->reset_task);
>>> +                /* return immediately since reset is imminent */
>>> +                return;
>>> +            }
>>> +        }
>>> +
>>> +        /* Force detection of hung controller every watchdog period */
>>> +        set_bit(IGC_RING_FLAG_TX_DETECT_HANG, &tx_ring->flags);
>>> +    }
>>> +
>>> +    /* Cause software interrupt to ensure Rx ring is cleaned */
>>> +    if (adapter->flags & IGC_FLAG_HAS_MSIX) {
>>> +        u32 eics = 0;
>>> +
>>> +        for (i = 0; i < adapter->num_q_vectors; i++)
>>> +            eics |= adapter->q_vector[i]->eims_value;
>>> +        wr32(E1000_EICS, eics);
>>> +    } else {
>>> +        wr32(E1000_ICS, E1000_ICS_RXDMT0);
>>> +    }
>>> +
>>> +    /* Reset the timer */
>>> +    if (!test_bit(__IGC_DOWN, &adapter->state)) {
>>> +        if (adapter->flags & IGC_FLAG_NEED_LINK_UPDATE)
>>> +            mod_timer(&adapter->watchdog_timer,
>>> +                  round_jiffies(jiffies +  HZ));
>>> +        else
>>> +            mod_timer(&adapter->watchdog_timer,
>>> +                  round_jiffies(jiffies + 2 * HZ));
>>> +    }
>>> +}
>>> +
>>>   /**
>>>    *  igc_update_ring_itr - update the dynamic ITR value based on 
>>> packet size
>>>    *  @q_vector: pointer to q_vector
>>> @@ -3195,6 +3426,8 @@ static int __igc_open(struct net_device 
>>> *netdev, bool resuming)
>>>       /* start the watchdog. */
>>>       hw->mac.get_link_status = 1;
>>> +    schedule_work(&adapter->watchdog_task);
>>> +
>>>       return E1000_SUCCESS;
>>>   err_set_queues:
>>> @@ -3477,6 +3710,7 @@ static int igc_probe(struct pci_dev *pdev,
>>>       timer_setup(&adapter->watchdog_timer, igc_watchdog, 0);
>>>       INIT_WORK(&adapter->reset_task, igc_reset_task);
>>> +    INIT_WORK(&adapter->watchdog_task, igc_watchdog_task);
>>>       /* Initialize link properties that are user-changeable */
>>>       adapter->fc_autoneg = true;
>>> @@ -3562,6 +3796,7 @@ static void igc_remove(struct pci_dev *pdev)
>>>       del_timer_sync(&adapter->watchdog_timer);
>>>       cancel_work_sync(&adapter->reset_task);
>>> +    cancel_work_sync(&adapter->watchdog_task);
>>>       /* Release control of h/w to f/w.  If f/w is AMT enabled, this
>>>        * would have already happened in close and is redundant.
>>>
> Hello Shannon,
> Thanks for your comments.
> Thanks,
> Sasha
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@osuosl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/igc/e1000_defines.h b/drivers/net/ethernet/intel/igc/e1000_defines.h
index 24c24c197d6b..5e13a606712b 100644
--- a/drivers/net/ethernet/intel/igc/e1000_defines.h
+++ b/drivers/net/ethernet/intel/igc/e1000_defines.h
@@ -86,6 +86,9 @@ 
 #define E1000_CTRL_RFCE		0x08000000  /* Receive Flow Control enable */
 #define E1000_CTRL_TFCE		0x10000000  /* Transmit flow control enable */
 
+#define E1000_CONNSW_AUTOSENSE_CONF	0x2
+#define E1000_CONNSW_AUTOSENSE_EN	0x1
+
 /* PBA constants */
 #define E1000_PBA_34K			0x0022
 
@@ -128,6 +131,10 @@ 
 #define CR_1000T_HD_CAPS	0x0100 /* Advertise 1000T HD capability */
 #define CR_1000T_FD_CAPS	0x0200 /* Advertise 1000T FD capability  */
 
+/* 1000BASE-T Status Register */
+#define SR_1000T_REMOTE_RX_STATUS	0x1000 /* Remote receiver OK */
+#define SR_1000T_LOCAL_RX_STATUS	0x2000 /* Local receiver OK */
+
 /* PHY GPY 211 registers */
 #define STANDARD_AN_REG_MASK	0x0007 /* MMD */
 #define ANEG_MULTIGBT_AN_CTRL	0x0020 /* MULTI GBT AN Control Register */
@@ -270,6 +277,11 @@ 
 #define E1000_IMS_RXT0		E1000_ICR_RXT0    /* Rx timer intr */
 #define E1000_IMS_RXDMT0	E1000_ICR_RXDMT0  /* Rx desc min. threshold */
 
+/* Interrupt Cause Set */
+#define E1000_ICS_LSC		E1000_ICR_LSC       /* Link Status Change */
+#define E1000_ICS_RXDMT0	E1000_ICR_RXDMT0    /* rx desc min. threshold */
+#define E1000_ICS_DRSTA		E1000_ICR_DRSTA     /* Device Reset Aserted */
+
 #define E1000_ICR_DOUTSYNC	0x10000000 /* NIC DMA out of sync */
 #define E1000_EITR_CNT_IGNR	0x80000000 /* Don't reset counters on write */
 #define E1000_IVAR_VALID	0x80
diff --git a/drivers/net/ethernet/intel/igc/e1000_hw.h b/drivers/net/ethernet/intel/igc/e1000_hw.h
index d8fc6fa9eda2..df948840df3b 100644
--- a/drivers/net/ethernet/intel/igc/e1000_hw.h
+++ b/drivers/net/ethernet/intel/igc/e1000_hw.h
@@ -229,6 +229,8 @@  struct e1000_dev_spec_base {
 	bool clear_semaphore_once;
 	bool module_plugged;
 	u8 media_port;
+	bool media_changed;
+	bool mas_capable;
 };
 
 struct e1000_hw {
diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index 99bc2344b7ff..36685d07c765 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -33,6 +33,8 @@  extern char igc_driver_version[];
 #define IGC_FLAG_HAS_MSI		BIT(0)
 #define IGC_FLAG_QUEUE_PAIRS		BIT(4)
 #define IGC_FLAG_NEED_LINK_UPDATE	BIT(9)
+#define IGC_FLAG_MEDIA_RESET		BIT(10)
+#define IGC_FLAG_MAS_ENABLE		BIT(12)
 #define IGC_FLAG_HAS_MSIX		BIT(13)
 #define IGC_FLAG_VLAN_PROMISC		BIT(15)
 
@@ -296,6 +298,7 @@  struct igc_adapter {
 
 	/* TX */
 	u16 tx_work_limit;
+	u32 tx_timeout_count;
 	int num_tx_queues;
 	struct igc_ring *tx_ring[IGC_MAX_TX_QUEUES];
 
@@ -354,6 +357,7 @@  struct igc_adapter {
 
 	struct igc_mac_addr *mac_table;
 
+	unsigned long link_check_timeout;
 	struct e1000_info ei;
 };
 
@@ -423,6 +427,14 @@  static inline unsigned int igc_rx_pg_order(struct igc_ring *ring)
 	return 0;
 }
 
+static inline s32 igc_read_phy_reg(struct e1000_hw *hw, u32 offset, u16 *data)
+{
+	if (hw->phy.ops.read_reg)
+		return hw->phy.ops.read_reg(hw, offset, data);
+
+	return 0;
+}
+
 #define igc_rx_pg_size(_ring) (PAGE_SIZE << igc_rx_pg_order(_ring))
 
 #define IGC_TXD_DCMD	(E1000_ADVTXD_DCMD_EOP | E1000_ADVTXD_DCMD_RS)
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 563612910b3f..7fb21af955ec 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -129,6 +129,14 @@  static void igc_power_down_link(struct igc_adapter *adapter)
 }
 
 /**
+ *  Detect and switch function for Media Auto Sense
+ *  @adapter: address of the board private structure
+ **/
+static void igc_check_swap_media(struct igc_adapter *adapter)
+{
+}
+
+/**
  *  igc_release_hw_control - release control of the h/w to f/w
  *  @adapter: address of board private structure
  *
@@ -2323,6 +2331,45 @@  static void igc_free_q_vector(struct igc_adapter *adapter, int v_idx)
 }
 
 /**
+ *  igc_has_link - check shared code for link and determine up/down
+ *  @adapter: pointer to driver private info
+ **/
+bool igc_has_link(struct igc_adapter *adapter)
+{
+	struct e1000_hw *hw = &adapter->hw;
+	bool link_active = false;
+
+	/* get_link_status is set on LSC (link status) interrupt or
+	 * rx sequence error interrupt.  get_link_status will stay
+	 * false until the e1000_check_for_link establishes link
+	 * for copper adapters ONLY
+	 */
+	switch (hw->phy.media_type) {
+	case e1000_media_type_copper:
+		if (!hw->mac.get_link_status)
+			return true;
+		hw->mac.ops.check_for_link(hw);
+		link_active = !hw->mac.get_link_status;
+		break;
+	default:
+	case e1000_media_type_unknown:
+		break;
+	}
+
+	if (hw->mac.type == e1000_i225 &&
+	    hw->phy.id == I225_I_PHY_ID) {
+		if (!netif_carrier_ok(adapter->netdev)) {
+			adapter->flags &= ~IGC_FLAG_NEED_LINK_UPDATE;
+		} else if (!(adapter->flags & IGC_FLAG_NEED_LINK_UPDATE)) {
+			adapter->flags |= IGC_FLAG_NEED_LINK_UPDATE;
+			adapter->link_check_timeout = jiffies;
+		}
+	}
+
+	return link_active;
+}
+
+/**
  *  igc_watchdog - Timer Call-back
  *  @data: pointer to adapter cast into an unsigned long
  **/
@@ -2333,6 +2380,190 @@  static void igc_watchdog(struct timer_list *t)
 	schedule_work(&adapter->watchdog_task);
 }
 
+static void igc_watchdog_task(struct work_struct *work)
+{
+	struct igc_adapter *adapter = container_of(work,
+						   struct igc_adapter,
+						   watchdog_task);
+	struct e1000_hw *hw = &adapter->hw;
+	struct e1000_phy_info *phy = &hw->phy;
+	struct net_device *netdev = adapter->netdev;
+	u32 link;
+	int i;
+	u32 connsw;
+	u16 phy_data, retry_count = 20;
+
+	link = igc_has_link(adapter);
+
+	if (adapter->flags & IGC_FLAG_NEED_LINK_UPDATE) {
+		if (time_after(jiffies, (adapter->link_check_timeout + HZ)))
+			adapter->flags &= ~IGC_FLAG_NEED_LINK_UPDATE;
+		else
+			link = false;
+	}
+
+	/* Force link down if we have fiber to swap to */
+	if (adapter->flags & IGC_FLAG_MAS_ENABLE) {
+		if (hw->phy.media_type == e1000_media_type_copper) {
+			connsw = rd32(E1000_CONNSW);
+			if (!(connsw & E1000_CONNSW_AUTOSENSE_EN))
+				link = 0;
+		}
+	}
+	if (link) {
+		/* Perform a reset if the media type changed. */
+		if (hw->dev_spec._base.media_changed) {
+			hw->dev_spec._base.media_changed = false;
+			adapter->flags |= IGC_FLAG_MEDIA_RESET;
+			igc_reset(adapter);
+		}
+
+		if (!netif_carrier_ok(netdev)) {
+			u32 ctrl;
+
+			hw->mac.ops.get_speed_and_duplex(hw,
+							 &adapter->link_speed,
+							 &adapter->link_duplex);
+
+			ctrl = rd32(E1000_CTRL);
+			/* Links 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,
+				    adapter->link_speed,
+				    adapter->link_duplex == FULL_DUPLEX ?
+				    "Full" : "Half",
+				    (ctrl & E1000_CTRL_TFCE) &&
+				    (ctrl & E1000_CTRL_RFCE) ? "RX/TX" :
+				    (ctrl & E1000_CTRL_RFCE) ?  "RX" :
+				    (ctrl & E1000_CTRL_TFCE) ?  "TX" : "None");
+
+			/* check if SmartSpeed worked */
+			igc_check_downshift(hw);
+			if (phy->speed_downgraded)
+				netdev_warn(netdev, "Link Speed was downgraded by SmartSpeed\n");
+
+			/* adjust timeout factor according to speed/duplex */
+			adapter->tx_timeout_factor = 1;
+			switch (adapter->link_speed) {
+			case SPEED_10:
+				adapter->tx_timeout_factor = 14;
+				break;
+			case SPEED_100:
+				/* maybe add some timeout factor ? */
+				break;
+			}
+
+			if (adapter->link_speed != SPEED_1000)
+				goto no_wait;
+
+			/* wait for Remote receiver status OK */
+retry_read_status:
+			if (!igc_read_phy_reg(hw, PHY_1000T_STATUS,
+					      &phy_data)) {
+				if (!(phy_data & SR_1000T_REMOTE_RX_STATUS) &&
+				    retry_count) {
+					msleep(100);
+					retry_count--;
+					goto retry_read_status;
+				} else if (!retry_count) {
+					dev_err(&adapter->pdev->dev, "exceed max 2 second\n");
+				}
+			} else {
+				dev_err(&adapter->pdev->dev, "read 1000Base-T Status Reg\n");
+			}
+no_wait:
+			netif_carrier_on(netdev);
+
+			/* link state has changed, schedule phy info update */
+			if (!test_bit(__IGC_DOWN, &adapter->state))
+				mod_timer(&adapter->phy_info_timer,
+					  round_jiffies(jiffies + 2 * HZ));
+		}
+	} else {
+		if (netif_carrier_ok(netdev)) {
+			adapter->link_speed = 0;
+			adapter->link_duplex = 0;
+
+			/* Links status message must follow this format */
+			netdev_info(netdev, "igc: %s NIC Link is Down\n",
+				    netdev->name);
+			netif_carrier_off(netdev);
+
+			/* link state has changed, schedule phy info update */
+			if (!test_bit(__IGC_DOWN, &adapter->state))
+				mod_timer(&adapter->phy_info_timer,
+					  round_jiffies(jiffies + 2 * HZ));
+
+			/* link is down, time to check for alternate media */
+			if (adapter->flags & IGC_FLAG_MAS_ENABLE) {
+				igc_check_swap_media(adapter);
+				if (adapter->flags & IGC_FLAG_MEDIA_RESET) {
+					schedule_work(&adapter->reset_task);
+					/* return immediately */
+					return;
+				}
+			}
+
+		/* also check for alternate media here */
+		} else if (!netif_carrier_ok(netdev) &&
+			   (adapter->flags & IGC_FLAG_MAS_ENABLE)) {
+			igc_check_swap_media(adapter);
+			if (adapter->flags & IGC_FLAG_MEDIA_RESET) {
+				schedule_work(&adapter->reset_task);
+				/* return immediately */
+				return;
+			}
+		}
+	}
+
+	spin_lock(&adapter->stats64_lock);
+	igc_update_stats(adapter);
+	spin_unlock(&adapter->stats64_lock);
+
+	for (i = 0; i < adapter->num_tx_queues; i++) {
+		struct igc_ring *tx_ring = adapter->tx_ring[i];
+
+		if (!netif_carrier_ok(netdev)) {
+			/* We've lost link, so the controller stops DMA,
+			 * but we've got queued Tx work that's never going
+			 * to get done, so reset controller to flush Tx.
+			 * (Do the reset outside of interrupt context).
+			 */
+			if (igc_desc_unused(tx_ring) + 1 < tx_ring->count) {
+				adapter->tx_timeout_count++;
+				schedule_work(&adapter->reset_task);
+				/* return immediately since reset is imminent */
+				return;
+			}
+		}
+
+		/* Force detection of hung controller every watchdog period */
+		set_bit(IGC_RING_FLAG_TX_DETECT_HANG, &tx_ring->flags);
+	}
+
+	/* Cause software interrupt to ensure Rx ring is cleaned */
+	if (adapter->flags & IGC_FLAG_HAS_MSIX) {
+		u32 eics = 0;
+
+		for (i = 0; i < adapter->num_q_vectors; i++)
+			eics |= adapter->q_vector[i]->eims_value;
+		wr32(E1000_EICS, eics);
+	} else {
+		wr32(E1000_ICS, E1000_ICS_RXDMT0);
+	}
+
+	/* Reset the timer */
+	if (!test_bit(__IGC_DOWN, &adapter->state)) {
+		if (adapter->flags & IGC_FLAG_NEED_LINK_UPDATE)
+			mod_timer(&adapter->watchdog_timer,
+				  round_jiffies(jiffies +  HZ));
+		else
+			mod_timer(&adapter->watchdog_timer,
+				  round_jiffies(jiffies + 2 * HZ));
+	}
+}
+
 /**
  *  igc_update_ring_itr - update the dynamic ITR value based on packet size
  *  @q_vector: pointer to q_vector
@@ -3195,6 +3426,8 @@  static int __igc_open(struct net_device *netdev, bool resuming)
 	/* start the watchdog. */
 	hw->mac.get_link_status = 1;
 
+	schedule_work(&adapter->watchdog_task);
+
 	return E1000_SUCCESS;
 
 err_set_queues:
@@ -3477,6 +3710,7 @@  static int igc_probe(struct pci_dev *pdev,
 	timer_setup(&adapter->watchdog_timer, igc_watchdog, 0);
 
 	INIT_WORK(&adapter->reset_task, igc_reset_task);
+	INIT_WORK(&adapter->watchdog_task, igc_watchdog_task);
 
 	/* Initialize link properties that are user-changeable */
 	adapter->fc_autoneg = true;
@@ -3562,6 +3796,7 @@  static void igc_remove(struct pci_dev *pdev)
 	del_timer_sync(&adapter->watchdog_timer);
 
 	cancel_work_sync(&adapter->reset_task);
+	cancel_work_sync(&adapter->watchdog_task);
 
 	/* Release control of h/w to f/w.  If f/w is AMT enabled, this
 	 * would have already happened in close and is redundant.