| Message ID | 20211012180415.3454346-1-Shyam-sundar.S-k@amd.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,1/2] net: amd-xgbe: Toggle PLL settings during rate change | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Single patches do not need cover letters |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | success | CCed 4 of 4 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 64 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | No static functions without inline keyword in header files |
On 10/12/21 1:04 PM, Shyam Sundar S K wrote: > For each rate change command submission, the FW has to do phy > power off sequence internally. For this to happen correctly, the > PLL re-initialization control setting has to be turned off before > sending mailbox commands and re-enabled once the command submission > is complete. > > Co-developed-by: Sudheesh Mavila <sudheesh.mavila@amd.com> > Signed-off-by: Sudheesh Mavila <sudheesh.mavila@amd.com> > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> With the minor change below... Acked-by: Tom Lendacky <thomas.lendacky@amd.com> > --- > v2: add a missing Co-developed-by tag > > drivers/net/ethernet/amd/xgbe/xgbe-common.h | 8 ++++++++ > drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c | 20 +++++++++++++++++++- > 2 files changed, 27 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h b/drivers/net/ethernet/amd/xgbe/xgbe-common.h > index b2cd3bdba9f8..3ac396cf94e0 100644 > --- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h > +++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h > @@ -1331,6 +1331,10 @@ > #define MDIO_VEND2_PMA_CDR_CONTROL 0x8056 > #endif > > +#ifndef MDIO_VEND2_PMA_MISC_CTRL0 > +#define MDIO_VEND2_PMA_MISC_CTRL0 0x8090 > +#endif > + > #ifndef MDIO_CTRL1_SPEED1G > #define MDIO_CTRL1_SPEED1G (MDIO_CTRL1_SPEED10G & ~BMCR_SPEED100) > #endif > @@ -1389,6 +1393,10 @@ > #define XGBE_PMA_RX_RST_0_RESET_ON 0x10 > #define XGBE_PMA_RX_RST_0_RESET_OFF 0x00 > > +#define XGBE_PMA_PLL_CTRL_MASK BIT(15) > +#define XGBE_PMA_PLL_CTRL_SET BIT(15) > +#define XGBE_PMA_PLL_CTRL_CLEAR 0x0000 > + > /* Bit setting and getting macros > * The get macro will extract the current bit field value from within > * the variable > diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c > index 18e48b3bc402..4465af9b72cf 100644 > --- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c > +++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c > @@ -1977,12 +1977,26 @@ 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_SET > + : XGBE_PMA_PLL_CTRL_CLEAR); Please line the ":" up with the "?" above it. Thanks, Tom > + > + /* Wait for command to complete */ > + usleep_range(100, 200); > +} > + > static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata, > unsigned int cmd, unsigned int sub_cmd) > { > unsigned int s0 = 0; > unsigned int wait; > > + /* Clear the PLL so that it helps in power down sequence */ > + xgbe_phy_pll_ctrl(pdata, false); > + > /* Log if a previous command did not complete */ > if (XP_IOREAD_BITS(pdata, XP_DRIVER_INT_RO, STATUS)) { > netif_dbg(pdata, link, pdata->netdev, > @@ -2003,7 +2017,7 @@ static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata, > wait = XGBE_RATECHANGE_COUNT; > while (wait--) { > if (!XP_IOREAD_BITS(pdata, XP_DRIVER_INT_RO, STATUS)) > - return; > + goto reenable_pll; > > usleep_range(1000, 2000); > } > @@ -2013,6 +2027,10 @@ static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata, > > /* Reset on error */ > xgbe_phy_rx_reset(pdata); > + > +reenable_pll: > + /* Re-enable the PLL control */ > + xgbe_phy_pll_ctrl(pdata, true); > } > > static void xgbe_phy_rrc(struct xgbe_prv_data *pdata) >
On Tue, 12 Oct 2021 13:13:21 -0500 Tom Lendacky wrote: > On 10/12/21 1:04 PM, Shyam Sundar S K wrote: > > For each rate change command submission, the FW has to do phy > > power off sequence internally. For this to happen correctly, the > > PLL re-initialization control setting has to be turned off before > > sending mailbox commands and re-enabled once the command submission > > is complete. > > > > Co-developed-by: Sudheesh Mavila <sudheesh.mavila@amd.com> > > Signed-off-by: Sudheesh Mavila <sudheesh.mavila@amd.com> > > Signed-off-by: Shyam Sundar S K <Shyam-sundar.S-k@amd.com> > > With the minor change below... For both patches - any more info on the impact? Will the link not come up? Driver lock up? ..? It's unclear whether this is a fix for an issue which was always there, new FW/HW/platform. Please include information which will allow us to answer those questions in the commit messages or cover letter for the series. If it's a fix it will need Fixes tags (seems likely, even if it's a new FW that surfaced the issue we probably still want the change to go to stable.) Thanks!
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-common.h b/drivers/net/ethernet/amd/xgbe/xgbe-common.h index b2cd3bdba9f8..3ac396cf94e0 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-common.h +++ b/drivers/net/ethernet/amd/xgbe/xgbe-common.h @@ -1331,6 +1331,10 @@ #define MDIO_VEND2_PMA_CDR_CONTROL 0x8056 #endif +#ifndef MDIO_VEND2_PMA_MISC_CTRL0 +#define MDIO_VEND2_PMA_MISC_CTRL0 0x8090 +#endif + #ifndef MDIO_CTRL1_SPEED1G #define MDIO_CTRL1_SPEED1G (MDIO_CTRL1_SPEED10G & ~BMCR_SPEED100) #endif @@ -1389,6 +1393,10 @@ #define XGBE_PMA_RX_RST_0_RESET_ON 0x10 #define XGBE_PMA_RX_RST_0_RESET_OFF 0x00 +#define XGBE_PMA_PLL_CTRL_MASK BIT(15) +#define XGBE_PMA_PLL_CTRL_SET BIT(15) +#define XGBE_PMA_PLL_CTRL_CLEAR 0x0000 + /* Bit setting and getting macros * The get macro will extract the current bit field value from within * the variable diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c index 18e48b3bc402..4465af9b72cf 100644 --- a/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c +++ b/drivers/net/ethernet/amd/xgbe/xgbe-phy-v2.c @@ -1977,12 +1977,26 @@ 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_SET + : XGBE_PMA_PLL_CTRL_CLEAR); + + /* Wait for command to complete */ + usleep_range(100, 200); +} + static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata, unsigned int cmd, unsigned int sub_cmd) { unsigned int s0 = 0; unsigned int wait; + /* Clear the PLL so that it helps in power down sequence */ + xgbe_phy_pll_ctrl(pdata, false); + /* Log if a previous command did not complete */ if (XP_IOREAD_BITS(pdata, XP_DRIVER_INT_RO, STATUS)) { netif_dbg(pdata, link, pdata->netdev, @@ -2003,7 +2017,7 @@ static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata, wait = XGBE_RATECHANGE_COUNT; while (wait--) { if (!XP_IOREAD_BITS(pdata, XP_DRIVER_INT_RO, STATUS)) - return; + goto reenable_pll; usleep_range(1000, 2000); } @@ -2013,6 +2027,10 @@ static void xgbe_phy_perform_ratechange(struct xgbe_prv_data *pdata, /* Reset on error */ xgbe_phy_rx_reset(pdata); + +reenable_pll: + /* Re-enable the PLL control */ + xgbe_phy_pll_ctrl(pdata, true); } static void xgbe_phy_rrc(struct xgbe_prv_data *pdata)