lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <8a7c989d-1541-bb69-89b6-e18398ecaed4@amd.com> Date: Thu, 6 Oct 2022 22:53:53 +0530 From: Raju Rangoju <Raju.Rangoju@....com> To: Tom Lendacky <thomas.lendacky@....com>, Shyam-sundar.S-k@....com, davem@...emloft.net, kuba@...nel.org Cc: netdev@...r.kernel.org, rrangoju@....com Subject: Re: [PATCH net 2/3] amd-xgbe: enable PLL_CTL for fixed PHY modes only Hi Tom, On 10/6/2022 8:17 PM, Tom Lendacky wrote: > On 10/6/22 08:54, Raju Rangoju wrote: >> PLL control setting(HW RRCM) is needed only in fixed PHY configuration >> to fix the peer-peer issues. Without the PLL control setting, the link >> up takes longer time in a fixed phy configuration. >> >> Driver implements SW RRCM for Autoneg On configuration, hence PLL >> control setting (HW RRCM) is not needed for AN On configuration, and >> can be skipped. >> >> Also, PLL re-initialization is not needed for PHY Power Off and RRCM >> commands. Otherwise, they lead to mailbox errors. Added the changes >> accordingly. >> >> Fixes: daf182d360e5 ("net: amd-xgbe: Toggle PLL settings during rate >> change") >> Signed-off-by: Raju Rangoju <Raju.Rangoju@....com> >> --- >> drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 21 +++++++++++++-------- >> drivers/net/ethernet/amd/xgbe/xgbe.h | 10 ++++++++++ >> 2 files changed, 23 insertions(+), 8 deletions(-) >> >> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c >> b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c >> index 19b943eba560..23fbd89a29df 100644 >> --- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c >> +++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c >> @@ -1979,13 +1979,16 @@ static void xgbe_phy_rx_reset(struct >> xgbe_prv_data *pdata) >> static void xgbe_phy_pll_ctrl(struct xgbe_prv_data *pdata, bool enable) >> { >> - XMDIO_WRITE_BITS(pdata, MDIO_MMD_PMAPMD, MDIO_VEND2_PMA_MISC_CTRL0, >> - XGBE_PMA_PLL_CTRL_MASK, >> - enable ? XGBE_PMA_PLL_CTRL_ENABLE >> - : XGBE_PMA_PLL_CTRL_DISABLE); >> + /* PLL_CTRL feature needs to be enabled for fixed PHY modes >> (Non-Autoneg) only */ >> + if (pdata->phy.autoneg == AUTONEG_DISABLE) { >> + XMDIO_WRITE_BITS(pdata, MDIO_MMD_PMAPMD, >> MDIO_VEND2_PMA_MISC_CTRL0, >> + XGBE_PMA_PLL_CTRL_MASK, >> + enable ? XGBE_PMA_PLL_CTRL_ENABLE >> + : XGBE_PMA_PLL_CTRL_DISABLE); >> - /* Wait for command to complete */ >> - usleep_range(100, 200); >> + /* Wait for command to complete */ >> + usleep_range(100, 200); >> + } > > Rather than indent all this, just add an if that returns at the beginning > of the function, e.g.: > > /* PLL_CTRL feature needs to be enabled for fixed PHY modes > (Non-Autoneg) only */ > if (pdata->phy.autoneg != AUTONEG_DISABLE) > return; Sure, I'll add the above changes. > > Now a general question... is this going to force Autoneg ON to end up > always going through the RRC path now where it may not have before? In > other words, there's now an auto-negotiation delay? As per the databook, Receiver power state change (HW RRCM) is not allowed during the CL73 / CL72. FW is already implementing SW RRCM for Autoneg ON, and no additional changes are needed in driver for AN ON path. However, HW RRCM(PLL_CTRL) is needed in Fixed PHY configs. > >> } >> static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata, >> @@ -2029,8 +2032,10 @@ static void xgbe_phy_perform_ratechange(struct >> xgbe_prv_data *pdata, >> xgbe_phy_rx_reset(pdata); >> reenable_pll: >> - /* Enable PLL re-initialization */ >> - xgbe_phy_pll_ctrl(pdata, true); >> + /* Enable PLL re-initialization, not needed for PHY Power Off cmd */ > > Comment should also include the RRC command... > >> + if (cmd != XGBE_MAILBOX_CMD_POWER_OFF && >> + cmd != XGBE_MAILBOX_CMD_RRCM) >> + xgbe_phy_pll_ctrl(pdata, true); >> } >> static void xgbe_phy_rrc(struct xgbe_prv_data *pdata) >> diff --git a/drivers/net/ethernet/amd/xgbe/xgbe.h >> b/drivers/net/ethernet/amd/xgbe/xgbe.h >> index 49d23abce73d..c7865681790c 100644 >> --- a/drivers/net/ethernet/amd/xgbe/xgbe.h >> +++ b/drivers/net/ethernet/amd/xgbe/xgbe.h >> @@ -611,6 +611,16 @@ enum xgbe_mdio_mode { >> XGBE_MDIO_MODE_CL45, >> }; >> +enum XGBE_MAILBOX_CMD { >> + XGBE_MAILBOX_CMD_POWER_OFF = 0, >> + XGBE_MAILBOX_CMD_SET_1G = 1, >> + XGBE_MAILBOX_CMD_SET_2_5G = 2, >> + XGBE_MAILBOX_CMD_SET_10G_SFI = 3, >> + XGBE_MAILBOX_CMD_SET_10G_KR = 4, >> + XGBE_MAILBOX_CMD_RRCM = 5, >> + XGBE_MAILBOX_CMD_UNKNOWN = 6 >> +}; > > If you're going to add an enum for the commands, then you should apply > them everywhere. Creating this enum and updating all the command locations > should be a pre-patch to this patch. > > Thanks, > Tom > >> + >> struct xgbe_phy { >> struct ethtool_link_ksettings lks;
Powered by blists - more mailing lists