| Message ID | 20230628124326.55732-2-ruppala@nvidia.com |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | [1/4] net: phy: aquantia: Enable Tx/Rx pause frame support in aquantia PHY | expand |
On Wed, Jun 28, 2023 at 06:13:24PM +0530, Revanth Kumar Uppala wrote: > Enable MAC controlled energy efficient ethernet (EEE) so that MAC can > keep the PHY in EEE sleep mode when link utilization is low to reduce > energy consumption. This needs more explanation. Is this 'SmartEEE', in that the PHY is doing EEE without the SoC MAC being involved? Ideally, you should only do SmartEEE, if the SoC MAC is dumb and does not have EEE itself. I guess if you are doing rate adaptation, or MACSEC in the PHY, then you might be forced to use SmartEEE since the SoC MAC is somewhat decoupled from the PHY. At the moment, we don't have a good story for SmartEEE. It should be configured in the same way as normal EEE, ethtool --set-eee etc. I've got a rewrite of normal EEE in the works. Once that is merged i hope SmartEEE will be next. Andrew
> -----Original Message----- > From: Andrew Lunn <andrew@lunn.ch> > Sent: Wednesday, June 28, 2023 7:24 PM > To: Revanth Kumar Uppala <ruppala@nvidia.com> > Cc: linux@armlinux.org.uk; hkallweit1@gmail.com; netdev@vger.kernel.org; > linux-tegra@vger.kernel.org; Narayan Reddy <narayanr@nvidia.com> > Subject: Re: [PATCH 2/4] net: phy: aquantia: Enable MAC Controlled EEE > > External email: Use caution opening links or attachments > > > On Wed, Jun 28, 2023 at 06:13:24PM +0530, Revanth Kumar Uppala wrote: > > Enable MAC controlled energy efficient ethernet (EEE) so that MAC can > > keep the PHY in EEE sleep mode when link utilization is low to reduce > > energy consumption. > > This needs more explanation. Is this 'SmartEEE', in that the PHY is doing EEE > without the SoC MAC being involved? No, this is not Smart EEE. > > Ideally, you should only do SmartEEE, if the SoC MAC is dumb and does not have > EEE itself. I guess if you are doing rate adaptation, or MACSEC in the PHY, then > you might be forced to use SmartEEE since the SoC MAC is somewhat decoupled > from the PHY. > > At the moment, we don't have a good story for SmartEEE. It should be > configured in the same way as normal EEE, ethtool --set-eee etc. I've got a > rewrite of normal EEE in the works. Once that is merged i hope SmartEEE will be > next. "ethtool --set-eee" is a dynamic way of enabling normal EEE and here we are doing the same normal EEE but configuring it by default in aqr107_config_init() instead of doing it dynamically. So, is there any concern for this? Thanks, Revanth Uppala > > Andrew
On Mon, Jul 24, 2023 at 11:29:28AM +0000, Revanth Kumar Uppala wrote: > > Ideally, you should only do SmartEEE, if the SoC MAC is dumb and does not have > > EEE itself. I guess if you are doing rate adaptation, or MACSEC in the PHY, then > > you might be forced to use SmartEEE since the SoC MAC is somewhat decoupled > > from the PHY. > > > > At the moment, we don't have a good story for SmartEEE. It should be > > configured in the same way as normal EEE, ethtool --set-eee etc. I've got a > > rewrite of normal EEE in the works. Once that is merged i hope SmartEEE will be > > next. > "ethtool --set-eee" is a dynamic way of enabling normal EEE and here we are doing the same normal EEE but configuring it by default in aqr107_config_init() instead of doing it dynamically. So, setting the MAC_CNTRL_EEE bits is just enabling the standard IEEE paths in the PHY to allow the IEEE defined EEE architecture to work? If that's all its doing, I wonder why they aren't set by default... seems rather strange.
diff --git a/drivers/net/phy/aquantia_main.c b/drivers/net/phy/aquantia_main.c index c99b9d066463..faca2a0b1d49 100644 --- a/drivers/net/phy/aquantia_main.c +++ b/drivers/net/phy/aquantia_main.c @@ -28,6 +28,9 @@ #define MDIO_AN_ADVT 0x0010 +#define VEND1_SEC_INGRESS_CNTRL_REG1 0x7001 +#define MAC_CNTRL_EEE (BIT(8) | BIT(12)) + #define MDIO_PHYXS_VEND_IF_STATUS 0xe812 #define MDIO_PHYXS_VEND_IF_STATUS_TYPE_MASK GENMASK(7, 3) #define MDIO_PHYXS_VEND_IF_STATUS_TYPE_KR 0 @@ -596,6 +599,12 @@ static int aqr107_config_init(struct phy_device *phydev) if (ret < 0) return ret; + /* Enable MAC Controlled EEE */ + ret = phy_write_mmd(phydev, MDIO_MMD_VEND1, + VEND1_SEC_INGRESS_CNTRL_REG1, MAC_CNTRL_EEE); + if (ret < 0) + return ret; + return aqr107_set_downshift(phydev, MDIO_AN_VEND_PROV_DOWNSHIFT_DFLT); }