| Message ID | E1csRQT-0004rM-Tq@rmk-PC.armlinux.org.uk |
|---|---|
| State | RFC, archived |
| Delegated to: | David Miller |
| Headers | show |
On 03/27/2017 02:59 AM, Russell King wrote: > As EEE is able to work in SGMII mode as well, add it to the list of > permissable EEE modes that phy_init_eee() will accept. This is > necessary so that EEE can work with an 88E1512 connected in SGMII mode. As you mention in your cover letter, we should probably reverse this test and make it reject modes where EEE has no chance of being supported at all. Other than that: Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
On Mon, Mar 27, 2017 at 09:47:31AM -0700, Florian Fainelli wrote: > On 03/27/2017 02:59 AM, Russell King wrote: > > As EEE is able to work in SGMII mode as well, add it to the list of > > permissable EEE modes that phy_init_eee() will accept. This is > > necessary so that EEE can work with an 88E1512 connected in SGMII mode. > > As you mention in your cover letter, we should probably reverse this > test and make it reject modes where EEE has no chance of being supported > at all. Want me to re-spin? Any thought on which interface modes we should explicitly exclude?
On 03/27/2017 10:00 AM, Russell King - ARM Linux wrote: > On Mon, Mar 27, 2017 at 09:47:31AM -0700, Florian Fainelli wrote: >> On 03/27/2017 02:59 AM, Russell King wrote: >>> As EEE is able to work in SGMII mode as well, add it to the list of >>> permissable EEE modes that phy_init_eee() will accept. This is >>> necessary so that EEE can work with an 88E1512 connected in SGMII mode. >> >> As you mention in your cover letter, we should probably reverse this >> test and make it reject modes where EEE has no chance of being supported >> at all. > > Want me to re-spin? Any thought on which interface modes we should > explicitly exclude? It actually sounds like we should just kill the check entirely, it does not appear that any of the interface mode would not fundamentally be able to support EEE, because the "lowest" mode we support is MII, and even there it's quite possible to support EEE.
On Mon, Mar 27, 2017 at 11:03:12AM -0700, Florian Fainelli wrote: > On 03/27/2017 10:00 AM, Russell King - ARM Linux wrote: > > On Mon, Mar 27, 2017 at 09:47:31AM -0700, Florian Fainelli wrote: > >> On 03/27/2017 02:59 AM, Russell King wrote: > >>> As EEE is able to work in SGMII mode as well, add it to the list of > >>> permissable EEE modes that phy_init_eee() will accept. This is > >>> necessary so that EEE can work with an 88E1512 connected in SGMII mode. > >> > >> As you mention in your cover letter, we should probably reverse this > >> test and make it reject modes where EEE has no chance of being supported > >> at all. > > > > Want me to re-spin? Any thought on which interface modes we should > > explicitly exclude? > > It actually sounds like we should just kill the check entirely, it does > not appear that any of the interface mode would not fundamentally be > able to support EEE, because the "lowest" mode we support is MII, and > even there it's quite possible to support EEE. Right, so it looks like the test reduces down to just: if (phydev->duplex == DUPLEX_FULL) { agreed?
On 03/27/2017 12:47 PM, Russell King - ARM Linux wrote: > On Mon, Mar 27, 2017 at 11:03:12AM -0700, Florian Fainelli wrote: >> On 03/27/2017 10:00 AM, Russell King - ARM Linux wrote: >>> On Mon, Mar 27, 2017 at 09:47:31AM -0700, Florian Fainelli wrote: >>>> On 03/27/2017 02:59 AM, Russell King wrote: >>>>> As EEE is able to work in SGMII mode as well, add it to the list of >>>>> permissable EEE modes that phy_init_eee() will accept. This is >>>>> necessary so that EEE can work with an 88E1512 connected in SGMII mode. >>>> >>>> As you mention in your cover letter, we should probably reverse this >>>> test and make it reject modes where EEE has no chance of being supported >>>> at all. >>> >>> Want me to re-spin? Any thought on which interface modes we should >>> explicitly exclude? >> >> It actually sounds like we should just kill the check entirely, it does >> not appear that any of the interface mode would not fundamentally be >> able to support EEE, because the "lowest" mode we support is MII, and >> even there it's quite possible to support EEE. > > Right, so it looks like the test reduces down to just: > > if (phydev->duplex == DUPLEX_FULL) { > > agreed? Yes indeed. Thanks!
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index 345251f21699..3529cc8bec10 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -1215,6 +1215,7 @@ int phy_init_eee(struct phy_device *phydev, bool clk_stop_enable) if ((phydev->duplex == DUPLEX_FULL) && ((phydev->interface == PHY_INTERFACE_MODE_MII) || (phydev->interface == PHY_INTERFACE_MODE_GMII) || + phydev->interface == PHY_INTERFACE_MODE_SGMII || phy_interface_is_rgmii(phydev) || phy_is_internal(phydev))) { int eee_lp, eee_cap, eee_adv;
As EEE is able to work in SGMII mode as well, add it to the list of permissable EEE modes that phy_init_eee() will accept. This is necessary so that EEE can work with an 88E1512 connected in SGMII mode. Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk> --- drivers/net/phy/phy.c | 1 + 1 file changed, 1 insertion(+)