| Message ID | 20230921121946.3025771-1-yong.liang.choong@linux.intel.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | TSN auto negotiation between 1G and 2.5G | expand |
On Thu, Sep 21, 2023 at 08:19:43PM +0800, Choong Yong Liang wrote: > From: "Tan, Tee Min" <tee.min.tan@linux.intel.com> > > This commit introduces xpcs_sgmii_2500basex_features[] that combine > xpcs_sgmii_features[] and xpcs_2500basex_features[] for Intel mGbE > controller that desire to interchange the speed mode of > 10/100/1000/2500Mbps at runtime. > > Also, we introduce xpcs_config_aneg_c37_sgmii_2500basex() function Clause 37... SGMII? 2500base-X? Technically, clause 37 doesn't cover 2500base-X. > which is called by the xpcs_do_config() with the new AN mode: > DW_SGMII_2500BASEX, and this new function will proceed next-level > calling to perform C37 SGMII AN/2500BASEX configuration based on > the PHY interface updated by PHY driver. > > Signed-off-by: Tan, Tee Min <tee.min.tan@linux.intel.com> > Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com> > --- > drivers/net/pcs/pcs-xpcs.c | 72 ++++++++++++++++++++++++++++++------ > include/linux/pcs/pcs-xpcs.h | 1 + > 2 files changed, 62 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/pcs/pcs-xpcs.c b/drivers/net/pcs/pcs-xpcs.c > index 4dbc21f604f2..60d90191677d 100644 > --- a/drivers/net/pcs/pcs-xpcs.c > +++ b/drivers/net/pcs/pcs-xpcs.c > @@ -104,6 +104,21 @@ static const int xpcs_2500basex_features[] = { > __ETHTOOL_LINK_MODE_MASK_NBITS, > }; > > +static const int xpcs_sgmii_2500basex_features[] = { > + ETHTOOL_LINK_MODE_Pause_BIT, > + ETHTOOL_LINK_MODE_Asym_Pause_BIT, > + ETHTOOL_LINK_MODE_Autoneg_BIT, > + ETHTOOL_LINK_MODE_10baseT_Half_BIT, > + ETHTOOL_LINK_MODE_10baseT_Full_BIT, > + ETHTOOL_LINK_MODE_100baseT_Half_BIT, > + ETHTOOL_LINK_MODE_100baseT_Full_BIT, > + ETHTOOL_LINK_MODE_1000baseT_Half_BIT, > + ETHTOOL_LINK_MODE_1000baseT_Full_BIT, The connected PHY could be one that supports 1000baseX as well. > + ETHTOOL_LINK_MODE_2500baseX_Full_BIT, > + ETHTOOL_LINK_MODE_2500baseT_Full_BIT, > + __ETHTOOL_LINK_MODE_MASK_NBITS, > +}; > + > static const phy_interface_t xpcs_usxgmii_interfaces[] = { > PHY_INTERFACE_MODE_USXGMII, > }; > @@ -133,6 +148,12 @@ static const phy_interface_t xpcs_2500basex_interfaces[] = { > PHY_INTERFACE_MODE_MAX, > }; > > +static const phy_interface_t xpcs_sgmii_2500basex_interfaces[] = { > + PHY_INTERFACE_MODE_SGMII, > + PHY_INTERFACE_MODE_2500BASEX, > + PHY_INTERFACE_MODE_MAX, > +}; > + > enum { > DW_XPCS_USXGMII, > DW_XPCS_10GKR, > @@ -141,6 +162,7 @@ enum { > DW_XPCS_SGMII, > DW_XPCS_1000BASEX, > DW_XPCS_2500BASEX, > + DW_XPCS_SGMII_2500BASEX, > DW_XPCS_INTERFACE_MAX, > }; > > @@ -290,6 +312,7 @@ static int xpcs_soft_reset(struct dw_xpcs *xpcs, > case DW_AN_C37_SGMII: > case DW_2500BASEX: > case DW_AN_C37_1000BASEX: > + case DW_SGMII_2500BASEX: > dev = MDIO_MMD_VEND2; > break; > default: > @@ -748,6 +771,8 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs, > if (xpcs->dev_flag == DW_DEV_TXGBE) > ret |= DW_VR_MII_DIG_CTRL1_PHY_MODE_CTRL; > > + /* Disable 2.5G GMII for SGMII C37 mode */ > + ret &= ~DW_VR_MII_DIG_CTRL1_2G5_EN; Do you know that this is correct for every user of this function? > ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_DIG_CTRL1, ret); > if (ret < 0) > return ret; > @@ -848,6 +873,26 @@ static int xpcs_config_2500basex(struct dw_xpcs *xpcs) > return xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_MMD_CTRL, ret); > } > > +static int xpcs_config_aneg_c37_sgmii_2500basex(struct dw_xpcs *xpcs, > + unsigned int neg_mode, > + phy_interface_t interface) > +{ > + int ret = -EOPNOTSUPP; > + > + switch (interface) { > + case PHY_INTERFACE_MODE_SGMII: > + ret = xpcs_config_aneg_c37_sgmii(xpcs, neg_mode); > + break; > + case PHY_INTERFACE_MODE_2500BASEX: > + ret = xpcs_config_2500basex(xpcs); > + break; > + default: > + break; > + } > + > + return ret; > +} > + > int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface, > const unsigned long *advertising, unsigned int neg_mode) > { > @@ -890,6 +935,12 @@ int xpcs_do_config(struct dw_xpcs *xpcs, phy_interface_t interface, > if (ret) > return ret; > break; > + case DW_SGMII_2500BASEX: > + ret = xpcs_config_aneg_c37_sgmii_2500basex(xpcs, neg_mode, > + interface); > + if (ret) > + return ret; > + break; > default: > return -1; > } > @@ -1114,6 +1165,11 @@ static void xpcs_get_state(struct phylink_pcs *pcs, > } > break; > case DW_AN_C37_SGMII: > + case DW_SGMII_2500BASEX: > + /* 2500BASEX is not supported for in-band AN mode. */ > + if (state->interface == PHY_INTERFACE_MODE_2500BASEX) > + break; > + > ret = xpcs_get_state_c37_sgmii(xpcs, state); > if (ret) { > pr_err("xpcs_get_state_c37_sgmii returned %pe\n", > @@ -1266,23 +1322,17 @@ static const struct xpcs_compat synopsys_xpcs_compat[DW_XPCS_INTERFACE_MAX] = { > .num_interfaces = ARRAY_SIZE(xpcs_10gbaser_interfaces), > .an_mode = DW_10GBASER, > }, > - [DW_XPCS_SGMII] = { > - .supported = xpcs_sgmii_features, > - .interface = xpcs_sgmii_interfaces, > - .num_interfaces = ARRAY_SIZE(xpcs_sgmii_interfaces), > - .an_mode = DW_AN_C37_SGMII, > - }, Doesn't this break SGMII-only support (those using DW_XPCS_SGMII) ? > [DW_XPCS_1000BASEX] = { > .supported = xpcs_1000basex_features, > .interface = xpcs_1000basex_interfaces, > .num_interfaces = ARRAY_SIZE(xpcs_1000basex_interfaces), > .an_mode = DW_AN_C37_1000BASEX, > }, > - [DW_XPCS_2500BASEX] = { > - .supported = xpcs_2500basex_features, > - .interface = xpcs_2500basex_interfaces, > - .num_interfaces = ARRAY_SIZE(xpcs_2500basex_interfaces), > - .an_mode = DW_2500BASEX, Doesn't this break 2500base-X only support (those using DW_XPCS_2500BASEX)? > + [DW_XPCS_SGMII_2500BASEX] = { > + .supported = xpcs_sgmii_2500basex_features, > + .interface = xpcs_sgmii_2500basex_interfaces, > + .num_interfaces = ARRAY_SIZE(xpcs_sgmii_2500basex_features), > + .an_mode = DW_SGMII_2500BASEX, > }, > }; > > diff --git a/include/linux/pcs/pcs-xpcs.h b/include/linux/pcs/pcs-xpcs.h > index da3a6c30f6d2..f075d2fca54a 100644 > --- a/include/linux/pcs/pcs-xpcs.h > +++ b/include/linux/pcs/pcs-xpcs.h > @@ -19,6 +19,7 @@ > #define DW_2500BASEX 3 > #define DW_AN_C37_1000BASEX 4 > #define DW_10GBASER 5 > +#define DW_SGMII_2500BASEX 6 > > /* device vendor OUI */ > #define DW_OUI_WX 0x0018fc80 > -- > 2.25.1 > >
> Auto-negotiation between 10, 100, 1000Mbps will use > in-band auto negotiation. Auto-negotiation between 10/100/1000Mbps and > 2.5Gbps will work as the following proposed flow, the stmmac driver reads > the PHY link status registers then identifies the negotiated speed. I don't think you replied to my comment. in-band is just an optimisation. It in theory allows you to avoid a software path, the PHY driver talking to the MAC driver about the PHY status. As an optimisation, it is optional. Linux has the software path and the MAC driver you are using basically has it implemented. Why use this odd mix of in-band and out of band? It seems the change will be simpler if you just use the out of band method all the time and ignore in-band. Andrew
On 21/9/2023 10:09 pm, Russell King (Oracle) wrote: > On Thu, Sep 21, 2023 at 03:14:59PM +0200, Andrew Lunn wrote: >>> Auto-negotiation between 10, 100, 1000Mbps will use >>> in-band auto negotiation. Auto-negotiation between 10/100/1000Mbps and >>> 2.5Gbps will work as the following proposed flow, the stmmac driver reads >>> the PHY link status registers then identifies the negotiated speed. >> >> I don't think you replied to my comment. >> >> in-band is just an optimisation. It in theory allows you to avoid a >> software path, the PHY driver talking to the MAC driver about the PHY >> status. As an optimisation, it is optional. Linux has the software >> path and the MAC driver you are using basically has it implemented. > > Sorry Andrew, I have to disagree. It isn't always optional - there are > PHYs out there where they won't pass data until the in-band exchange > has completed. If you try to operate out-of-band without the PHY being > told that is the case, and program the MAC/PCS end not to respond to > the in-band frames from the PHY, the PHY will report link up as normal > (since it reports the media side), but no data will flow because the > MAC facing side of the PHY hasn't completed. > > The only exception are PHYs that default to in-band but have an inband > bypass mode also enabled to cover the case where the MAC/PCS doesn't > respond to the inband messages. > Russell is correct, we did set out-of-band for PCS and configured MAC. Due to the PHY not being completed, there will be no data flow through.
Intel platforms’ integrated Gigabit Ethernet controllers support 2.5Gbps mode statically using BIOS programming. In the current implementation, the BIOS menu provides an option to select between 10/100/1000Mbps and 2.5Gbps modes. Based on the selection, the BIOS programs the Phase Lock Loop (PLL) registers. The BIOS also read the TSN lane registers from Flexible I/O Adapter (FIA) block and provided 10/100/1000Mbps/2.5Gbps information to the stmmac driver. But auto-negotiation between 10/100/1000Mbps and 2.5Gbps is not allowed. The new proposal is to support auto-negotiation between 10/100/1000Mbps and 2.5Gbps . Auto-negotiation between 10, 100, 1000Mbps will use in-band auto negotiation. Auto-negotiation between 10/100/1000Mbps and 2.5Gbps will work as the following proposed flow, the stmmac driver reads the PHY link status registers then identifies the negotiated speed. Based on the speed stmmac driver will identify TSN lane registers from FIA then send IPC command to the Power Management controller (PMC) through PMC driver/API. PMC will act as a proxy to programs the PLL registers. changelog: v1 -> v2: - Add static to pmc_lpm_modes declaration - Add cur_link_an_mode to the kernel doc - Combine 2 commits i.e. "stmmac: intel: Separate driver_data of ADL-N from TGL" and "net: stmmac: Add 1G/2.5G auto-negotiation support for ADL-N" into 1 commit. v2 -> v3: - Create `pmc_ipc.c` file for `intel_pmc_ipc()` function and allocate the file in `arch/x86/platform/intel/` directory. - Update phylink's AN mode during phy interface change and not exposing phylink's AN mode into phylib. --- Choong Yong Liang (2): net: phy: update in-band AN mode when changing interface by PHY driver stmmac: intel: Add 1G/2.5G auto-negotiation support for ADL-N David E. Box (1): arch: x86: Add IPC mailbox accessor function and add SoC register access Tan, Tee Min (2): net: pcs: xpcs: combine C37 SGMII AN and 2500BASEX for Intel mGbE controller net: stmmac: enable Intel mGbE 1G/2.5G auto-negotiation support MAINTAINERS | 2 + arch/x86/Kconfig | 9 + arch/x86/platform/intel/Makefile | 1 + arch/x86/platform/intel/pmc_ipc.c | 75 +++++++ drivers/net/ethernet/stmicro/stmmac/Kconfig | 1 + .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 183 +++++++++++++++++- .../net/ethernet/stmicro/stmmac/dwmac-intel.h | 81 ++++++++ .../net/ethernet/stmicro/stmmac/stmmac_main.c | 20 ++ drivers/net/pcs/pcs-xpcs.c | 72 +++++-- drivers/net/phy/phylink.c | 30 ++- include/linux/pcs/pcs-xpcs.h | 1 + .../linux/platform_data/x86/intel_pmc_ipc.h | 34 ++++ include/linux/stmmac.h | 1 + 13 files changed, 493 insertions(+), 17 deletions(-) create mode 100644 arch/x86/platform/intel/pmc_ipc.c create mode 100644 include/linux/platform_data/x86/intel_pmc_ipc.h