On Wed, Jan 25, 2023 at 03:17:48PM +0100, Moritz Buhl wrote: > > Dear tech, > > Looking into various reports on errors with mini routers that use > igc(4), I found the following diff in FreeBSD: > https://github.com/freebsd/freebsd-src/commit/29d7f1ff579579711dd5a3325480728b8ed45f8c > > I additionally deleted the igc_set_d0_lplu_state_i225 and > igc_set_d3_lplu_state_i225 functions since they are no longer used. > > I am not convinced that this diff really fixes any of the issues, > however I do appreciate feedback.
I225 devices do have only one phy vendor, Linux also removes PHY ID checking: https://github.com/torvalds/linux/commit/7c496de538eebd8212dc2a3c9a468386b264d0d4 > Ok? ok kevlo@ > mbuhl > > Index: dev/pci/igc_i225.c > =================================================================== > RCS file: /cvs/src/sys/dev/pci/igc_i225.c,v > retrieving revision 1.3 > diff -u -p -r1.3 igc_i225.c > --- dev/pci/igc_i225.c 11 May 2022 06:14:15 -0000 1.3 > +++ dev/pci/igc_i225.c 10 Jan 2023 09:26:04 -0000 > @@ -163,16 +163,7 @@ igc_init_phy_params_i225(struct igc_hw * > goto out; > > 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; > - } > + phy->type = igc_phy_i225; > > out: > return ret_val; > @@ -1104,66 +1095,6 @@ igc_init_hw_i225(struct igc_hw *hw) > > ret_val = igc_init_hw_base(hw); > return ret_val; > -} > - > -/* > - * igc_set_d0_lplu_state_i225 - Set Low-Power-Link-Up (LPLU) D0 state > - * @hw: pointer to the HW structure > - * @active: true to enable LPLU, false to disable > - * > - * Note: since I225 does not actually support LPLU, this function > - * simply enables/disables 1G and 2.5G speeds in D0. > - */ > -int > -igc_set_d0_lplu_state_i225(struct igc_hw *hw, bool active) > -{ > - uint32_t data; > - > - DEBUGFUNC("igc_set_d0_lplu_state_i225"); > - > - data = IGC_READ_REG(hw, IGC_I225_PHPM); > - > - if (active) { > - data |= IGC_I225_PHPM_DIS_1000; > - data |= IGC_I225_PHPM_DIS_2500; > - } else { > - data &= ~IGC_I225_PHPM_DIS_1000; > - data &= ~IGC_I225_PHPM_DIS_2500; > - } > - > - IGC_WRITE_REG(hw, IGC_I225_PHPM, data); > - return IGC_SUCCESS; > -} > - > -/* > - * igc_set_d3_lplu_state_i225 - Set Low-Power-Link-Up (LPLU) D3 state > - * @hw: pointer to the HW structure > - * @active: true to enable LPLU, false to disable > - * > - * Note: since I225 does not actually support LPLU, this function > - * simply enables/disables 100M, 1G and 2.5G speeds in D3. > - */ > -int > -igc_set_d3_lplu_state_i225(struct igc_hw *hw, bool active) > -{ > - uint32_t data; > - > - DEBUGFUNC("igc_set_d3_lplu_state_i225"); > - > - data = IGC_READ_REG(hw, IGC_I225_PHPM); > - > - if (active) { > - data |= IGC_I225_PHPM_DIS_100_D3; > - data |= IGC_I225_PHPM_DIS_1000_D3; > - data |= IGC_I225_PHPM_DIS_2500_D3; > - } else { > - data &= ~IGC_I225_PHPM_DIS_100_D3; > - data &= ~IGC_I225_PHPM_DIS_1000_D3; > - data &= ~IGC_I225_PHPM_DIS_2500_D3; > - } > - > - IGC_WRITE_REG(hw, IGC_I225_PHPM, data); > - return IGC_SUCCESS; > } > > /** > Index: dev/pci/igc_i225.h > =================================================================== > RCS file: /cvs/src/sys/dev/pci/igc_i225.h,v > retrieving revision 1.1 > diff -u -p -r1.1 igc_i225.h > --- dev/pci/igc_i225.h 31 Oct 2021 14:52:57 -0000 1.1 > +++ dev/pci/igc_i225.h 10 Jan 2023 09:37:40 -0000 > @@ -27,8 +27,6 @@ void igc_release_swfw_sync_i225(struct i > int igc_set_ltr_i225(struct igc_hw *, bool); > int igc_init_hw_i225(struct igc_hw *); > int igc_setup_copper_link_i225(struct igc_hw *); > -int igc_set_d0_lplu_state_i225(struct igc_hw *, bool); > -int igc_set_d3_lplu_state_i225(struct igc_hw *, bool); > int igc_set_eee_i225(struct igc_hw *, bool, bool, bool); > > #define ID_LED_DEFAULT_I225 \ > Index: dev/pci/igc_phy.c > =================================================================== > RCS file: /cvs/src/sys/dev/pci/igc_phy.c,v > retrieving revision 1.2 > diff -u -p -r1.2 igc_phy.c > --- dev/pci/igc_phy.c 11 May 2022 06:14:15 -0000 1.2 > +++ dev/pci/igc_phy.c 10 Jan 2023 09:34:11 -0000 > @@ -307,8 +307,7 @@ igc_phy_setup_autoneg(struct igc_hw *hw) > return ret_val; > } > > - if ((phy->autoneg_mask & ADVERTISE_2500_FULL) && > - hw->phy.id == I225_I_PHY_ID) { > + if (phy->autoneg_mask & ADVERTISE_2500_FULL) { > /* 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, > @@ -443,8 +442,7 @@ igc_phy_setup_autoneg(struct igc_hw *hw) > ret_val = phy->ops.write_reg(hw, PHY_1000T_CTRL, > mii_1000t_ctrl_reg); > > - if ((phy->autoneg_mask & ADVERTISE_2500_FULL) && > - hw->phy.id == I225_I_PHY_ID) > + if (phy->autoneg_mask & ADVERTISE_2500_FULL) > ret_val = phy->ops.write_reg(hw, > (STANDARD_AN_REG_MASK << MMD_DEVADDR_SHIFT) | > ANEG_MULTIGBT_AN_CTRL, aneg_multigbt_an_ctrl); >
