On 12/02/2016 09:48 AM, Florian Fainelli wrote: >>> Peppe, any thoughts on this? >> >> I share what you say. >> >> In sum, the EEE management inside the stmmac is: >> >> - the driver looks at own HW cap register if EEE is supported >> >> (indeed the user could keep disable EEE if bugged on some HW >> + Alex, Fabrice: we had some patches for this to propose where we >> called the phy_ethtool_set_eee to disable feature at phy >> level >> >> - then the stmmac asks PHY layer to understand if transceiver and >> partners are EEE capable. >> >> - If all matches the EEE is actually initialized. >> >> the logic above should be respected when use ethtool, hmm, I will >> check the stmmac_ethtool_op_set_eee asap. >> >> Hoping this is useful > > This is definitively useful, the only part that I am struggling to > understand in phy_init_eee() is this: > > eee_adv = phy_read_mmd_indirect(phydev, MDIO_AN_EEE_ADV, > MDIO_MMD_AN); > if (eee_adv <= 0) > goto eee_exit_err; > > if we are not already advertising EEE in the PHY's MMIO_MMD_AN page, by > the time we call phy_init_eee(), then we cannot complete the EEE > configuration at the PHY level, and presumably we should abort the EEE > configuration at the MAC level. > > While this condition makes sense if e.g: you are re-negotiating the link > with your partner for instance and if EEE was already advertised, the > very first time this function is called, it seems to be like we should > skip the check, because phy_init_eee() should actually tell us if, as a > result of a successful check, we should be setting EEE as something we > advertise? > > Do you remember what was the logic behind this check when you added it?
Peppe, can you remember why phy_init_eee() was written in a way that you need to have already locally advertised EEE for the function to successfully return? Thank you! -- Florian
