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.
Choong Yong Liang (1):
net: stmmac: Add 1G/2.5G auto-negotiation support for ADL-N
David E. Box (1):
platform/x86: intel_pmc_core: Add IPC mailbox accessor function and
add SoC register access
Michael Sit Wei Hong (1):
stmmac: intel: Separate driver_data of ADL-N from TGL
Tan, Tee Min (3):
net: pcs: xpcs: combine C37 SGMII AN and 2500BASEX for Intel mGbE
controller
net: phy: update in-band AN mode when changing interface by PHY driver
net: stmmac: enable Intel mGbE 1G/2.5G auto-negotiation support
MAINTAINERS | 1 +
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/marvell10g.c | 6 +
drivers/net/phy/phylink.c | 3 +
drivers/platform/x86/intel/pmc/adl.c | 2 +-
drivers/platform/x86/intel/pmc/cnp.c | 2 +-
drivers/platform/x86/intel/pmc/core.c | 63 +++++-
drivers/platform/x86/intel/pmc/icl.c | 2 +-
drivers/platform/x86/intel/pmc/mtl.c | 2 +-
drivers/platform/x86/intel/pmc/spt.c | 2 +-
drivers/platform/x86/intel/pmc/tgl.c | 2 +-
.../core.h => include/linux/intel_pmc_core.h | 27 ++-
include/linux/pcs/pcs-xpcs.h | 1 +
include/linux/phy.h | 2 +
include/linux/stmmac.h | 1 +
19 files changed, 449 insertions(+), 24 deletions(-)
rename drivers/platform/x86/intel/pmc/core.h => include/linux/intel_pmc_core.h (95%)
--
2.25.1
From: "Tan, Tee Min" <[email protected]>
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
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 <[email protected]>
Signed-off-by: Choong Yong Liang <[email protected]>
---
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 e4e59aa9faf7..cd1dc09c422f 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,
+ 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,
};
@@ -267,6 +289,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:
@@ -712,6 +735,8 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs, unsigned int mode)
else
ret &= ~DW_VR_MII_DIG_CTRL1_MAC_AUTO_SW;
+ /* Disable 2.5G GMII for SGMII C37 mode */
+ ret &= ~DW_VR_MII_DIG_CTRL1_2G5_EN;
ret = xpcs_write(xpcs, MDIO_MMD_VEND2, DW_VR_MII_DIG_CTRL1, ret);
if (ret < 0)
return ret;
@@ -807,6 +832,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 mode,
+ phy_interface_t interface)
+{
+ int ret = -EOPNOTSUPP;
+
+ switch (interface) {
+ case PHY_INTERFACE_MODE_SGMII:
+ ret = xpcs_config_aneg_c37_sgmii(xpcs, 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,
unsigned int mode, const unsigned long *advertising)
{
@@ -843,6 +888,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, mode,
+ interface);
+ if (ret)
+ return ret;
+ break;
default:
return -1;
}
@@ -1028,6 +1079,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",
@@ -1180,23 +1236,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,
- },
[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,
+ [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 ec8175b847cc..3e2265ca9e22 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
struct xpcs_id;
--
2.25.1
From: Michael Sit Wei Hong <[email protected]>
Separates the driver_data of ADL-N from TGL as a preparation to handle
the differences. No functional changes in this commit.
Signed-off-by: Michael Sit Wei Hong <[email protected]>
Signed-off-by: Choong Yong Liang <[email protected]>
---
.../net/ethernet/stmicro/stmmac/dwmac-intel.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
index 36c91dcb7a78..1ffa03451d26 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
@@ -962,6 +962,21 @@ static int adls_sgmii_phy1_data(struct pci_dev *pdev,
static struct stmmac_pci_info adls_sgmii1g_phy1_info = {
.setup = adls_sgmii_phy1_data,
};
+
+static int adln_sgmii_phy0_data(struct pci_dev *pdev,
+ struct plat_stmmacenet_data *plat)
+{
+ plat->bus_id = 1;
+ plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
+ plat->serdes_powerup = intel_serdes_powerup;
+ plat->serdes_powerdown = intel_serdes_powerdown;
+ return tgl_common_data(pdev, plat);
+}
+
+static struct stmmac_pci_info adln_sgmii1g_phy0_info = {
+ .setup = adln_sgmii_phy0_data,
+};
+
static const struct stmmac_pci_func_data galileo_stmmac_func_data[] = {
{
.func = 6,
@@ -1344,7 +1359,7 @@ static const struct pci_device_id intel_eth_pci_id_table[] = {
{ PCI_DEVICE_DATA(INTEL, TGLH_SGMII1G_1, &tgl_sgmii1g_phy1_info) },
{ PCI_DEVICE_DATA(INTEL, ADLS_SGMII1G_0, &adls_sgmii1g_phy0_info) },
{ PCI_DEVICE_DATA(INTEL, ADLS_SGMII1G_1, &adls_sgmii1g_phy1_info) },
- { PCI_DEVICE_DATA(INTEL, ADLN_SGMII1G, &tgl_sgmii1g_phy0_info) },
+ { PCI_DEVICE_DATA(INTEL, ADLN_SGMII1G, &adln_sgmii1g_phy0_info) },
{ PCI_DEVICE_DATA(INTEL, RPLP_SGMII1G, &tgl_sgmii1g_phy0_info) },
{}
};
--
2.25.1
Add modphy register lane to have 1G/2.5G auto-negotiation support for
ADL-N.
Signed-off-by: Choong Yong Liang <[email protected]>
---
.../net/ethernet/stmicro/stmmac/dwmac-intel.c | 34 ++++++++++++++++++-
.../net/ethernet/stmicro/stmmac/dwmac-intel.h | 2 ++
2 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
index 1ffa03451d26..024f436b276e 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
@@ -963,14 +963,46 @@ static struct stmmac_pci_info adls_sgmii1g_phy1_info = {
.setup = adls_sgmii_phy1_data,
};
+static int adln_common_data(struct pci_dev *pdev,
+ struct plat_stmmacenet_data *plat)
+{
+ struct intel_priv_data *intel_priv = plat->bsp_priv;
+
+ plat->rx_queues_to_use = 6;
+ plat->tx_queues_to_use = 4;
+ plat->clk_ptp_rate = 204800000;
+
+ plat->safety_feat_cfg->tsoee = 1;
+ plat->safety_feat_cfg->mrxpee = 0;
+ plat->safety_feat_cfg->mestee = 1;
+ plat->safety_feat_cfg->mrxee = 1;
+ plat->safety_feat_cfg->mtxee = 1;
+ plat->safety_feat_cfg->epsi = 0;
+ plat->safety_feat_cfg->edpp = 0;
+ plat->safety_feat_cfg->prtyen = 0;
+ plat->safety_feat_cfg->tmouten = 0;
+
+ intel_priv->tsn_lane_registers = adln_tsn_lane_registers;
+ intel_priv->max_tsn_lane_registers = ARRAY_SIZE(adln_tsn_lane_registers);
+
+ return intel_mgbe_common_data(pdev, plat);
+}
+
static int adln_sgmii_phy0_data(struct pci_dev *pdev,
struct plat_stmmacenet_data *plat)
{
+ struct intel_priv_data *intel_priv = plat->bsp_priv;
+
plat->bus_id = 1;
plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
+ plat->max_speed = SPEED_2500;
plat->serdes_powerup = intel_serdes_powerup;
plat->serdes_powerdown = intel_serdes_powerdown;
- return tgl_common_data(pdev, plat);
+ plat->config_serdes = intel_config_serdes;
+
+ intel_priv->pid_modphy = PID_MODPHY1;
+
+ return adln_common_data(pdev, plat);
}
static struct stmmac_pci_info adln_sgmii1g_phy0_info = {
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
index 75a336cf8af1..349c160c17b3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
@@ -124,8 +124,10 @@ static const struct pmc_serdes_regs pid_modphy1_2p5g_regs[] = {
{}
};
+static const int adln_tsn_lane_registers[] = {6};
static const int ehl_tsn_lane_registers[] = {7, 8, 9, 10, 11};
#else
+static const int adln_tsn_lane_registers[] = {};
static const int ehl_tsn_lane_registers[] = {};
#endif /* CONFIG_INTEL_PMC_CORE */
--
2.25.1
From: "Tan, Tee Min" <[email protected]>
Add cur_link_an_mode into phy_device struct for PHY drivers to
communicate the in-band AN mode setting with phylink framework.
As there is a mechanism in PHY drivers to switch the PHY interface
between SGMII and 2500BaseX according to link speed. In this case,
the in-band AN mode should be switching based on the PHY interface
as well, if the PHY interface has been changed/updated by PHY driver.
For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN
back for SGMII mode (10/100/1000Mbps).
Signed-off-by: Tan, Tee Min <[email protected]>
Signed-off-by: Choong Yong Liang <[email protected]>
---
drivers/net/phy/marvell10g.c | 6 ++++++
drivers/net/phy/phylink.c | 3 +++
include/linux/phy.h | 2 ++
3 files changed, 11 insertions(+)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 55d9d7acc32e..5cf26d02841d 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -30,6 +30,7 @@
#include <linux/phy.h>
#include <linux/sfp.h>
#include <linux/netdevice.h>
+#include <linux/phylink.h>
#define MV_PHY_ALASKA_NBT_QUIRK_MASK 0xfffffffe
#define MV_PHY_ALASKA_NBT_QUIRK_REV (MARVELL_PHY_ID_88X3310 | 0xa)
@@ -939,6 +940,9 @@ static void mv3310_update_interface(struct phy_device *phydev)
* xaui / rxaui modes according to the speed.
* Florian suggests setting phydev->interface to communicate this to the
* MAC. Only do this if we are already in one of the above modes.
+ * In-band Auto-negotiation is not supported in 2500BASE-X.
+ * Setting phydev->cur_link_an_mode to communicate this to the
+ * phylink framework.
*/
switch (phydev->speed) {
case SPEED_10000:
@@ -949,11 +953,13 @@ static void mv3310_update_interface(struct phy_device *phydev)
break;
case SPEED_2500:
phydev->interface = PHY_INTERFACE_MODE_2500BASEX;
+ phydev->cur_link_an_mode = MLO_AN_PHY;
break;
case SPEED_1000:
case SPEED_100:
case SPEED_10:
phydev->interface = PHY_INTERFACE_MODE_SGMII;
+ phydev->cur_link_an_mode = MLO_AN_INBAND;
break;
default:
break;
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 97c15e1f81de..d5783ea95eee 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1654,6 +1654,7 @@ static void phylink_phy_change(struct phy_device *phydev, bool up)
pl->phy_state.pause |= MLO_PAUSE_RX;
pl->phy_state.interface = phydev->interface;
pl->phy_state.link = up;
+ pl->cur_link_an_mode = phydev->cur_link_an_mode;
mutex_unlock(&pl->state_mutex);
phylink_run_resolve(pl);
@@ -1758,6 +1759,8 @@ static int phylink_bringup_phy(struct phylink *pl, struct phy_device *phy,
if (pl->config->mac_managed_pm)
phy->mac_managed_pm = true;
+ pl->phydev->cur_link_an_mode = pl->cur_link_an_mode;
+
return 0;
}
diff --git a/include/linux/phy.h b/include/linux/phy.h
index 11c1e91563d4..c685b526e307 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -756,6 +756,8 @@ struct phy_device {
/* MACsec management functions */
const struct macsec_ops *macsec_ops;
#endif
+ /* For communicate the AN mode setting with phylink framework. */
+ u8 cur_link_an_mode;
};
/* Generic phy_device::dev_flags */
--
2.25.1
On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote:
> From: "Tan, Tee Min" <[email protected]>
>
> Add cur_link_an_mode into phy_device struct for PHY drivers to
> communicate the in-band AN mode setting with phylink framework.
>
> As there is a mechanism in PHY drivers to switch the PHY interface
> between SGMII and 2500BaseX according to link speed. In this case,
> the in-band AN mode should be switching based on the PHY interface
> as well, if the PHY interface has been changed/updated by PHY driver.
>
> For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN
> back for SGMII mode (10/100/1000Mbps).
>
> Signed-off-by: Tan, Tee Min <[email protected]>
> Signed-off-by: Choong Yong Liang <[email protected]>
...
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 11c1e91563d4..c685b526e307 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -756,6 +756,8 @@ struct phy_device {
> /* MACsec management functions */
> const struct macsec_ops *macsec_ops;
> #endif
> + /* For communicate the AN mode setting with phylink framework. */
> + u8 cur_link_an_mode;
> };
Hi Choong Yong Liang,
Please consider adding cur_link_an_mode to the kernel doc
for struct phy_device - which is above the definition of struct phy_device.
On Thu, Jun 22, 2023 at 04:43:51PM +0200, Simon Horman wrote:
> On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote:
> > From: "Tan, Tee Min" <[email protected]>
> >
> > Add cur_link_an_mode into phy_device struct for PHY drivers to
> > communicate the in-band AN mode setting with phylink framework.
> >
> > As there is a mechanism in PHY drivers to switch the PHY interface
> > between SGMII and 2500BaseX according to link speed. In this case,
> > the in-band AN mode should be switching based on the PHY interface
> > as well, if the PHY interface has been changed/updated by PHY driver.
> >
> > For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN
> > back for SGMII mode (10/100/1000Mbps).
> >
> > Signed-off-by: Tan, Tee Min <[email protected]>
> > Signed-off-by: Choong Yong Liang <[email protected]>
>
> ...
>
> > diff --git a/include/linux/phy.h b/include/linux/phy.h
> > index 11c1e91563d4..c685b526e307 100644
> > --- a/include/linux/phy.h
> > +++ b/include/linux/phy.h
> > @@ -756,6 +756,8 @@ struct phy_device {
> > /* MACsec management functions */
> > const struct macsec_ops *macsec_ops;
> > #endif
> > + /* For communicate the AN mode setting with phylink framework. */
> > + u8 cur_link_an_mode;
> > };
>
> Hi Choong Yong Liang,
>
> Please consider adding cur_link_an_mode to the kernel doc
> for struct phy_device - which is above the definition of struct phy_device.
This looks like it's grabbing something from phylink and stuffing it
into phylib. However, I have no idea, because I don't seem to have
received the original patches. I'm guessing the reason is:
2023-06-22 05:21:24 1qCBoy-0003ji-G9 H=mga03.intel.com
[134.134.136.65]:57703 I=[78.32.30.218]:25
X=TLS1.2:ECDHE_SECP521R1__RSA_SHA512__AES_256_GCM:256
F=<[email protected]> rejected after DATA: unqualified
address not permitted: failing address in "Cc:" header is: Tan
Which I suspect came from:
Tan, Tee Min <[email protected]>
and someone doesn't realise that a "," in the display-name part of
an address *must* be quoted, otherwise "," is taken to be a separator
in the address list.
Consequently, it has now become:
[email protected], Tee Min <[email protected]>,
It should have been:
"Tan, Tee Min" <[email protected]>
with the double-quotes.
Please do not review this series further, but instead, please can the
author repost it forthwith with correct conformant headers so that a
proper review can be undertaken by all?
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Thu, Jun 22, 2023 at 04:06:31PM +0100, Russell King (Oracle) wrote:
> On Thu, Jun 22, 2023 at 04:43:51PM +0200, Simon Horman wrote:
> > On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote:
> > > From: "Tan, Tee Min" <[email protected]>
> > >
> > > Add cur_link_an_mode into phy_device struct for PHY drivers to
> > > communicate the in-band AN mode setting with phylink framework.
> > >
> > > As there is a mechanism in PHY drivers to switch the PHY interface
> > > between SGMII and 2500BaseX according to link speed. In this case,
> > > the in-band AN mode should be switching based on the PHY interface
> > > as well, if the PHY interface has been changed/updated by PHY driver.
> > >
> > > For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN
> > > back for SGMII mode (10/100/1000Mbps).
> > >
> > > Signed-off-by: Tan, Tee Min <[email protected]>
> > > Signed-off-by: Choong Yong Liang <[email protected]>
> >
> > ...
> >
> > > diff --git a/include/linux/phy.h b/include/linux/phy.h
> > > index 11c1e91563d4..c685b526e307 100644
> > > --- a/include/linux/phy.h
> > > +++ b/include/linux/phy.h
> > > @@ -756,6 +756,8 @@ struct phy_device {
> > > /* MACsec management functions */
> > > const struct macsec_ops *macsec_ops;
> > > #endif
> > > + /* For communicate the AN mode setting with phylink framework. */
> > > + u8 cur_link_an_mode;
> > > };
> >
> > Hi Choong Yong Liang,
> >
> > Please consider adding cur_link_an_mode to the kernel doc
> > for struct phy_device - which is above the definition of struct phy_device.
>
> This looks like it's grabbing something from phylink and stuffing it
> into phylib. However, I have no idea, because I don't seem to have
> received the original patches. I'm guessing the reason is:
>
> 2023-06-22 05:21:24 1qCBoy-0003ji-G9 H=mga03.intel.com
> [134.134.136.65]:57703 I=[78.32.30.218]:25
> X=TLS1.2:ECDHE_SECP521R1__RSA_SHA512__AES_256_GCM:256
> F=<[email protected]> rejected after DATA: unqualified
> address not permitted: failing address in "Cc:" header is: Tan
>
> Which I suspect came from:
>
> Tan, Tee Min <[email protected]>
>
> and someone doesn't realise that a "," in the display-name part of
> an address *must* be quoted, otherwise "," is taken to be a separator
> in the address list.
>
> Consequently, it has now become:
>
> [email protected], Tee Min <[email protected]>,
>
> It should have been:
>
> "Tan, Tee Min" <[email protected]>
>
> with the double-quotes.
>
> Please do not review this series further, but instead, please can the
> author repost it forthwith with correct conformant headers so that a
> proper review can be undertaken by all?
Hi Russell,
Sorry for not noticing this myself.
I agree that we should wait for a properly formed post as you suggest.
Hi Simon,
I will update it in v2. Thank you.
On 22/6/2023 10:43 pm, Simon Horman wrote:
> On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote:
>> From: "Tan, Tee Min" <[email protected]>
>>
>> Add cur_link_an_mode into phy_device struct for PHY drivers to
>> communicate the in-band AN mode setting with phylink framework.
>>
>> As there is a mechanism in PHY drivers to switch the PHY interface
>> between SGMII and 2500BaseX according to link speed. In this case,
>> the in-band AN mode should be switching based on the PHY interface
>> as well, if the PHY interface has been changed/updated by PHY driver.
>>
>> For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN
>> back for SGMII mode (10/100/1000Mbps).
>>
>> Signed-off-by: Tan, Tee Min <[email protected]>
>> Signed-off-by: Choong Yong Liang <[email protected]>
>
> ...
>
>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>> index 11c1e91563d4..c685b526e307 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -756,6 +756,8 @@ struct phy_device {
>> /* MACsec management functions */
>> const struct macsec_ops *macsec_ops;
>> #endif
>> + /* For communicate the AN mode setting with phylink framework. */
>> + u8 cur_link_an_mode;
>> };
>
> Hi Choong Yong Liang,
>
> Please consider adding cur_link_an_mode to the kernel doc
> for struct phy_device - which is above the definition of struct phy_device.
Hi Russell,
Thank you for pointing that out.
I will fix it and send out version 2.
On 22/6/2023 11:06 pm, Russell King (Oracle) wrote:
> On Thu, Jun 22, 2023 at 04:43:51PM +0200, Simon Horman wrote:
>> On Thu, Jun 22, 2023 at 12:19:02PM +0800, Choong Yong Liang wrote:
>>> From: "Tan, Tee Min" <[email protected]>
>>>
>>> Add cur_link_an_mode into phy_device struct for PHY drivers to
>>> communicate the in-band AN mode setting with phylink framework.
>>>
>>> As there is a mechanism in PHY drivers to switch the PHY interface
>>> between SGMII and 2500BaseX according to link speed. In this case,
>>> the in-band AN mode should be switching based on the PHY interface
>>> as well, if the PHY interface has been changed/updated by PHY driver.
>>>
>>> For e.g., disable in-band AN in 2500BaseX mode, or enable in-band AN
>>> back for SGMII mode (10/100/1000Mbps).
>>>
>>> Signed-off-by: Tan, Tee Min <[email protected]>
>>> Signed-off-by: Choong Yong Liang <[email protected]>
>>
>> ...
>>
>>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>>> index 11c1e91563d4..c685b526e307 100644
>>> --- a/include/linux/phy.h
>>> +++ b/include/linux/phy.h
>>> @@ -756,6 +756,8 @@ struct phy_device {
>>> /* MACsec management functions */
>>> const struct macsec_ops *macsec_ops;
>>> #endif
>>> + /* For communicate the AN mode setting with phylink framework. */
>>> + u8 cur_link_an_mode;
>>> };
>>
>> Hi Choong Yong Liang,
>>
>> Please consider adding cur_link_an_mode to the kernel doc
>> for struct phy_device - which is above the definition of struct phy_device.
>
> This looks like it's grabbing something from phylink and stuffing it
> into phylib. However, I have no idea, because I don't seem to have
> received the original patches. I'm guessing the reason is:
>
> 2023-06-22 05:21:24 1qCBoy-0003ji-G9 H=mga03.intel.com
> [134.134.136.65]:57703 I=[78.32.30.218]:25
> X=TLS1.2:ECDHE_SECP521R1__RSA_SHA512__AES_256_GCM:256
> F=<[email protected]> rejected after DATA: unqualified
> address not permitted: failing address in "Cc:" header is: Tan
>
> Which I suspect came from:
>
> Tan, Tee Min <[email protected]>
>
> and someone doesn't realise that a "," in the display-name part of
> an address *must* be quoted, otherwise "," is taken to be a separator
> in the address list.
>
> Consequently, it has now become:
>
> [email protected], Tee Min <[email protected]>,
>
> It should have been:
>
> "Tan, Tee Min" <[email protected]>
>
> with the double-quotes.
>
> Please do not review this series further, but instead, please can the
> author repost it forthwith with correct conformant headers so that a
> proper review can be undertaken by all?
>
> Thanks.
>