From b7fccf026afa546fe7227b1b5c5d307805762e4b Mon Sep 17 00:00:00 2001 From: kevlo Date: Wed, 11 May 2022 06:14:15 +0000 Subject: [PATCH] Use hardware routine for PHY reset and always accept the PHY that's present. The previously used software reset routine wasn't sufficient to reset the PHY if the bootloader hadn't left the device in an initialized state. From FreeBSD. Bug reported and the fix tested by daniel@ --- sys/dev/pci/if_igc.c | 4 +-- sys/dev/pci/igc_api.h | 3 +-- sys/dev/pci/igc_hw.h | 16 +----------- sys/dev/pci/igc_i225.c | 17 ++++-------- sys/dev/pci/igc_phy.c | 59 ++++++++++++++++++++++++++++-------------- sys/dev/pci/igc_phy.h | 3 +-- 6 files changed, 50 insertions(+), 52 deletions(-) diff --git a/sys/dev/pci/if_igc.c b/sys/dev/pci/if_igc.c index 6d7363f44c1..b6377fb5a09 100644 --- a/sys/dev/pci/if_igc.c +++ b/sys/dev/pci/if_igc.c @@ -1,4 +1,4 @@ -/* $OpenBSD: if_igc.c,v 1.7 2022/04/06 18:59:29 naddy Exp $ */ +/* $OpenBSD: if_igc.c,v 1.8 2022/05/11 06:14:15 kevlo Exp $ */ /*- * SPDX-License-Identifier: BSD-2-Clause * @@ -725,7 +725,7 @@ igc_dma_malloc(struct igc_softc *sc, bus_size_t size, struct igc_dma_alloc *dma) 1, &dma->dma_nseg, BUS_DMA_NOWAIT)) goto destroy; if (bus_dmamem_map(dma->dma_tag, &dma->dma_seg, dma->dma_nseg, size, - &dma->dma_vaddr, BUS_DMA_NOWAIT)) + &dma->dma_vaddr, BUS_DMA_NOWAIT | BUS_DMA_COHERENT)) goto free; if (bus_dmamap_load(dma->dma_tag, dma->dma_map, dma->dma_vaddr, size, NULL, BUS_DMA_NOWAIT)) diff --git a/sys/dev/pci/igc_api.h b/sys/dev/pci/igc_api.h index 5cf1d122a11..dee8557c7cb 100644 --- a/sys/dev/pci/igc_api.h +++ b/sys/dev/pci/igc_api.h @@ -1,4 +1,4 @@ -/* $OpenBSD: igc_api.h,v 1.1 2021/10/31 14:52:57 patrick Exp $ */ +/* $OpenBSD: igc_api.h,v 1.2 2022/05/11 06:14:15 kevlo Exp $ */ /*- * Copyright 2021 Intel Corp * Copyright 2021 Rubicon Communications, LLC (Netgate) @@ -40,7 +40,6 @@ int igc_get_cable_length(struct igc_hw *); int igc_validate_mdi_setting(struct igc_hw *); int igc_get_phy_info(struct igc_hw *); int igc_phy_hw_reset(struct igc_hw *); -int igc_phy_commit(struct igc_hw *); void igc_power_up_phy(struct igc_hw *); void igc_power_down_phy(struct igc_hw *); int igc_read_mac_addr(struct igc_hw *); diff --git a/sys/dev/pci/igc_hw.h b/sys/dev/pci/igc_hw.h index 4224afb4945..c1ad011133c 100644 --- a/sys/dev/pci/igc_hw.h +++ b/sys/dev/pci/igc_hw.h @@ -1,4 +1,4 @@ -/* $OpenBSD: igc_hw.h,v 1.1 2021/10/31 14:52:57 patrick Exp $ */ +/* $OpenBSD: igc_hw.h,v 1.2 2022/05/11 06:14:15 kevlo Exp $ */ /*- * Copyright 2021 Intel Corp * Copyright 2021 Rubicon Communications, LLC (Netgate) @@ -50,19 +50,6 @@ #include #include -#if 0 -/* Enable/disable debugging statements in shared code */ -#define DBG 0 - -#define DEBUGOUT(...) \ - do { if (DBG) printf(__VA_ARGS__); } while (0) -#define DEBUGOUT1(...) DEBUGOUT(__VA_ARGS__) -#define DEBUGOUT2(...) DEBUGOUT(__VA_ARGS__) -#define DEBUGOUT3(...) DEBUGOUT(__VA_ARGS__) -#define DEBUGOUT7(...) DEBUGOUT(__VA_ARGS__) -#define DEBUGFUNC(F) DEBUGOUT(F "\n") -#endif - struct igc_hw; #define IGC_FUNC_1 1 @@ -244,7 +231,6 @@ struct igc_phy_operations { int (*init_params)(struct igc_hw *); int (*acquire)(struct igc_hw *); int (*check_reset_block)(struct igc_hw *); - int (*commit)(struct igc_hw *); int (*force_speed_duplex)(struct igc_hw *); int (*get_info)(struct igc_hw *); int (*set_page)(struct igc_hw *, uint16_t); diff --git a/sys/dev/pci/igc_i225.c b/sys/dev/pci/igc_i225.c index cf22c7428a7..2f192e5cdba 100644 --- a/sys/dev/pci/igc_i225.c +++ b/sys/dev/pci/igc_i225.c @@ -1,4 +1,4 @@ -/* $OpenBSD: igc_i225.c,v 1.2 2022/01/28 07:11:14 guenther Exp $ */ +/* $OpenBSD: igc_i225.c,v 1.3 2022/05/11 06:14:15 kevlo Exp $ */ /*- * Copyright 2021 Intel Corp * Copyright 2021 Rubicon Communications, LLC (Netgate) @@ -133,7 +133,6 @@ int igc_init_phy_params_i225(struct igc_hw *hw) { struct igc_phy_info *phy = &hw->phy; - uint32_t ctrl_ext; int ret_val = IGC_SUCCESS; DEBUGFUNC("igc_init_phy_params_i225"); @@ -149,10 +148,10 @@ igc_init_phy_params_i225(struct igc_hw *hw) phy->reset_delay_us = 100; phy->ops.acquire = igc_acquire_phy_base; phy->ops.check_reset_block = igc_check_reset_block_generic; - phy->ops.commit = igc_phy_sw_reset_generic; phy->ops.release = igc_release_phy_base; - - ctrl_ext = IGC_READ_REG(hw, IGC_CTRL_EXT); + phy->ops.reset = igc_phy_hw_reset_generic; + phy->ops.read_reg = igc_read_phy_reg_gpy; + phy->ops.write_reg = igc_write_phy_reg_gpy; /* Make sure the PHY is in a good state. Several people have reported * firmware leaving the PHY's page select register set to something @@ -163,22 +162,16 @@ igc_init_phy_params_i225(struct igc_hw *hw) if (ret_val) goto out; - IGC_WRITE_REG(hw, IGC_CTRL_EXT, ctrl_ext); - phy->ops.read_reg = igc_read_phy_reg_gpy; - phy->ops.write_reg = igc_write_phy_reg_gpy; - ret_val = igc_get_phy_id(hw); /* Verify phy id and set remaining function pointers */ switch (phy->id) { case I225_I_PHY_ID: + default: phy->type = igc_phy_i225; phy->ops.set_d0_lplu_state = igc_set_d0_lplu_state_i225; phy->ops.set_d3_lplu_state = igc_set_d3_lplu_state_i225; /* TODO - complete with GPY PHY information */ break; - default: - ret_val = -IGC_ERR_PHY; - goto out; } out: diff --git a/sys/dev/pci/igc_phy.c b/sys/dev/pci/igc_phy.c index 7b7d3209087..ba3efe62457 100644 --- a/sys/dev/pci/igc_phy.c +++ b/sys/dev/pci/igc_phy.c @@ -1,4 +1,4 @@ -/* $OpenBSD: igc_phy.c,v 1.1 2021/10/31 14:52:57 patrick Exp $ */ +/* $OpenBSD: igc_phy.c,v 1.2 2022/05/11 06:14:15 kevlo Exp $ */ /*- * Copyright 2021 Intel Corp * Copyright 2021 Rubicon Communications, LLC (Netgate) @@ -23,7 +23,6 @@ igc_init_phy_ops_generic(struct igc_hw *hw) phy->ops.init_params = igc_null_ops_generic; phy->ops.acquire = igc_null_ops_generic; phy->ops.check_reset_block = igc_null_ops_generic; - phy->ops.commit = igc_null_ops_generic; phy->ops.force_speed_duplex = igc_null_ops_generic; phy->ops.get_info = igc_null_ops_generic; phy->ops.set_page = igc_null_set_page; @@ -149,7 +148,7 @@ igc_get_phy_id(struct igc_hw *hw) return ret_val; phy->id = (uint32_t)(phy_id << 16); - DELAY(20); + DELAY(200); ret_val = phy->ops.read_reg(hw, PHY_ID2, &phy_id); if (ret_val) return ret_val; @@ -157,7 +156,6 @@ igc_get_phy_id(struct igc_hw *hw) phy->id |= (uint32_t)(phy_id & PHY_REVISION_MASK); phy->revision = (uint32_t)(phy_id & ~PHY_REVISION_MASK); - return IGC_SUCCESS; } @@ -311,7 +309,7 @@ igc_phy_setup_autoneg(struct igc_hw *hw) if ((phy->autoneg_mask & ADVERTISE_2500_FULL) && hw->phy.id == I225_I_PHY_ID) { - /* Read the MULTI GBT AN Control Register - reg 7.32 */ + /* Read the MULTI GBT AN Control Register - reg 7.32 */ ret_val = phy->ops.read_reg(hw, (STANDARD_AN_REG_MASK << MMD_DEVADDR_SHIFT) | ANEG_MULTIGBT_AN_CTRL, &aneg_multigbt_an_ctrl); @@ -689,33 +687,56 @@ igc_phy_has_link_generic(struct igc_hw *hw, uint32_t iterations, } /** - * igc_phy_sw_reset_generic - PHY software reset + * igc_phy_hw_reset_generic - PHY hardware reset * @hw: pointer to the HW structure * - * Does a software reset of the PHY by reading the PHY control register and - * setting/write the control register reset bit to the PHY. + * Verify the reset block is not blocking us from resetting. Acquire + * semaphore (if necessary) and read/set/write the device control reset + * bit in the PHY. Wait the appropriate delay time for the device to + * reset and release the semaphore (if necessary). **/ int -igc_phy_sw_reset_generic(struct igc_hw *hw) +igc_phy_hw_reset_generic(struct igc_hw *hw) { - uint16_t phy_ctrl; + struct igc_phy_info *phy = &hw->phy; + uint32_t ctrl, timeout = 10000, phpm = 0; int ret_val; - DEBUGFUNC("igc_phy_sw_reset_generic"); + DEBUGFUNC("igc_phy_hw_reset_generic"); - if (!hw->phy.ops.read_reg) - return IGC_SUCCESS; + if (phy->ops.check_reset_block) { + ret_val = phy->ops.check_reset_block(hw); + if (ret_val) + return IGC_SUCCESS; + } - ret_val = hw->phy.ops.read_reg(hw, PHY_CONTROL, &phy_ctrl); + ret_val = phy->ops.acquire(hw); if (ret_val) return ret_val; - phy_ctrl |= MII_CR_RESET; - ret_val = hw->phy.ops.write_reg(hw, PHY_CONTROL, phy_ctrl); - if (ret_val) - return ret_val; + phpm = IGC_READ_REG(hw, IGC_I225_PHPM); + + ctrl = IGC_READ_REG(hw, IGC_CTRL); + IGC_WRITE_REG(hw, IGC_CTRL, ctrl | IGC_CTRL_PHY_RST); + IGC_WRITE_FLUSH(hw); + + DELAY(phy->reset_delay_us); + + IGC_WRITE_REG(hw, IGC_CTRL, ctrl); + IGC_WRITE_FLUSH(hw); + + DELAY(150); + + do { + phpm = IGC_READ_REG(hw, IGC_I225_PHPM); + timeout--; + DELAY(1); + } while (!(phpm & IGC_I225_PHPM_RST_COMPL) && timeout); + + if (!timeout) + DEBUGOUT("Timeout expired after a phy reset\n"); - DELAY(1); + phy->ops.release(hw); return ret_val; } diff --git a/sys/dev/pci/igc_phy.h b/sys/dev/pci/igc_phy.h index ded2589d690..9dafd927fe4 100644 --- a/sys/dev/pci/igc_phy.h +++ b/sys/dev/pci/igc_phy.h @@ -1,4 +1,4 @@ -/* $OpenBSD: igc_phy.h,v 1.1 2021/10/31 14:52:57 patrick Exp $ */ +/* $OpenBSD: igc_phy.h,v 1.2 2022/05/11 06:14:15 kevlo Exp $ */ /*- * Copyright 2021 Intel Corp * Copyright 2021 Rubicon Communications, LLC (Netgate) @@ -19,7 +19,6 @@ int igc_null_set_page(struct igc_hw *, uint16_t); int igc_check_downshift_generic(struct igc_hw *); int igc_check_reset_block_generic(struct igc_hw *); int igc_get_phy_id(struct igc_hw *); -int igc_phy_sw_reset_generic(struct igc_hw *); int igc_phy_hw_reset_generic(struct igc_hw *); int igc_phy_reset_dsp_generic(struct igc_hw *); int igc_set_d3_lplu_state_generic(struct igc_hw *, bool); -- 2.20.1