2023-08-04 11:08:53

by Choong Yong Liang

[permalink] [raw]
Subject: [PATCH net-next v2 0/5] TSN auto negotiation between 1G and 2.5G

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:
- Created intel_pmc_core.h in include/linux/platform_data/x86/ and
export the desired functionality.
- Add cur_link_an_mode to the kernel doc
- Update cfg_link_an_mode value during phy driver changed
- 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.


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.
---
Choong Yong Liang (1):
stmmac: intel: 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

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 | 4 +
drivers/platform/x86/intel/pmc/core.c | 60 ++++++
include/linux/pcs/pcs-xpcs.h | 1 +
include/linux/phy.h | 3 +
.../linux/platform_data/x86/intel_pmc_core.h | 41 ++++
include/linux/stmmac.h | 1 +
13 files changed, 458 insertions(+), 16 deletions(-)
create mode 100644 include/linux/platform_data/x86/intel_pmc_core.h

--
2.25.1



2023-08-04 11:12:29

by Choong Yong Liang

[permalink] [raw]
Subject: [PATCH net-next v2 2/5] net: pcs: xpcs: combine C37 SGMII AN and 2500BASEX for Intel mGbE controller

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 44b037646865..b35767f65d62 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:
@@ -713,6 +736,8 @@ static int xpcs_config_aneg_c37_sgmii(struct dw_xpcs *xpcs,
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;
@@ -808,6 +833,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)
{
@@ -844,6 +889,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;
}
@@ -1030,6 +1081,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",
@@ -1182,23 +1238,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 ff99cf7a5d0d..a1625e1c5875 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


2023-08-04 12:26:38

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/5] TSN auto negotiation between 1G and 2.5G

On Fri, Aug 04, 2023 at 04:45:22PM +0800, Choong Yong Liang wrote:
> 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.

Have you considered using out of band for all link modes? You might
end up with a cleaner architecture, and not need any phylink/phylib
hacks.

Andrew

2023-08-10 10:41:31

by Choong Yong Liang

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/5] TSN auto negotiation between 1G and 2.5G



On 4/8/2023 8:04 pm, Andrew Lunn wrote:
> On Fri, Aug 04, 2023 at 04:45:22PM +0800, Choong Yong Liang wrote:
>> 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.
>
> Have you considered using out of band for all link modes? You might
> end up with a cleaner architecture, and not need any phylink/phylib
> hacks.
>
> Andrew
Hi Andrew,

Thank you for your feedback.
I will study the feasibility of the out-of-band (OOB) approach.

2023-09-21 19:09:49

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/5] TSN auto negotiation between 1G and 2.5G

On Thu, Sep 21, 2023 at 03:21:00PM +0200, Andrew Lunn wrote:
> > Hi Andrew,
> >
> > After conducting a comprehensive study, it seems that implementing
> > out-of-band for all link modes might not be feasible. I may have missed some
> > key aspects during my analysis.
> >
> > Would you be open to sharing a high-level idea of how we could potentially
> > make this feasible? Your insights would be greatly appreciated.
>
> stmmac_mac_link_up() gets passed interface, speed and duplex. That
> tells you what the PHY has negotiated. Is there anything else you need
> to know?

The problem is... the stmmac driver is utter bollocks - that information
is *not* passed to the BSP. Instead, stmmac parse and store information
such as the PHY interface mode at initialisation time. BSPs also re-
parse and store e.g. the PHY interface mode at initialisation time.
The driver ignores what it gets from phylink.

The driver is basically utter crap. That's an area I _had_ patches to
clean up. I no longer do. stmmac is crap crap crap and will stay crap
until they become more receptive to patches to fix it, even if the
patches are not 100% to their liking but are in fact correct. Maybe
if I ever decide to touch that driver in the future. Which I doubt
given my recent experience.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

2023-09-21 19:58:04

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/5] TSN auto negotiation between 1G and 2.5G

On Thu, Sep 21, 2023 at 04:41:20PM +0200, Andrew Lunn wrote:
> On Thu, Sep 21, 2023 at 03:12:19PM +0100, Russell King (Oracle) wrote:
> > On Thu, Sep 21, 2023 at 03:21:00PM +0200, Andrew Lunn wrote:
> > > > Hi Andrew,
> > > >
> > > > After conducting a comprehensive study, it seems that implementing
> > > > out-of-band for all link modes might not be feasible. I may have missed some
> > > > key aspects during my analysis.
> > > >
> > > > Would you be open to sharing a high-level idea of how we could potentially
> > > > make this feasible? Your insights would be greatly appreciated.
> > >
> > > stmmac_mac_link_up() gets passed interface, speed and duplex. That
> > > tells you what the PHY has negotiated. Is there anything else you need
> > > to know?
> >
> > The problem is... the stmmac driver is utter bollocks - that information
> > is *not* passed to the BSP. Instead, stmmac parse and store information
> > such as the PHY interface mode at initialisation time. BSPs also re-
> > parse and store e.g. the PHY interface mode at initialisation time.
> > The driver ignores what it gets from phylink.
> >
> > The driver is basically utter crap. That's an area I _had_ patches to
> > clean up. I no longer do. stmmac is crap crap crap and will stay crap
> > until they become more receptive to patches to fix it, even if the
> > patches are not 100% to their liking but are in fact correct. Maybe
> > if I ever decide to touch that driver in the future. Which I doubt
> > given my recent experience.
>
> Hi Russell
>
> You pointed out the current proposal will break stuff. Do you see a
> way forward for this patchset which does not first involve actually
> cleaning up of this driver?

As I said in one of my replies, it would really help if the author can
provide a table showing what is attempting to be achieved here. With
that, we should be able to work out exactly what is required, what
needs to change in stmmac, etc.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

2023-09-21 21:15:48

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/5] TSN auto negotiation between 1G and 2.5G

On Thu, Sep 21, 2023 at 08:25:05PM +0800, Choong Yong Liang wrote:
>
>
> On 4/8/2023 8:04 pm, Andrew Lunn wrote:
> > On Fri, Aug 04, 2023 at 04:45:22PM +0800, Choong Yong Liang wrote:
> > > 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.
> >
> > Have you considered using out of band for all link modes? You might
> > end up with a cleaner architecture, and not need any phylink/phylib
> > hacks.
> >
> > Andrew
> Hi Andrew,
>
> After conducting a comprehensive study, it seems that implementing
> out-of-band for all link modes might not be feasible. I may have missed some
> key aspects during my analysis.

You need to provide details of why you think it's not feasible, because
you're making those reading your message have to guess.

We _do_ have cases where this is already supported. The DM7052 SFP
module for example has a BCM84881 PHY on board that has no in-band
support, so always has to use out-of-band. This module supports 10G,
5G, 2.5G, 1G, 100M and 10M speeds. It switches its interface between
10G, 2500base-X and SGMII mode. It's been supported in Linux for a
while with MAC/PCS that implement phylink _correctly_.

I wouldn't call stmmac a proper phylink implementation, especially
when it comes to switching between different interfaces.

My attempt at starting to clean up the stmmac code was thwarted by
niggly review comments (over whether %u or %d should be used to print
a _signed integer_ that stmmac stupidly implicitly casts to an unsigned
integer. That lead me to decide that stmmac was beyond being cleaned
up, so I junked the large patch set of improvements that I had - along
with multiple issues that I had found in the driver.

Someone else needs to sort stmmac out, and I suspect that may be a
pre-requisit for your changes so that stmmac operates _correctly_ with
phylink.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

2023-09-21 21:24:28

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/5] TSN auto negotiation between 1G and 2.5G

> Hi Andrew,
>
> After conducting a comprehensive study, it seems that implementing
> out-of-band for all link modes might not be feasible. I may have missed some
> key aspects during my analysis.
>
> Would you be open to sharing a high-level idea of how we could potentially
> make this feasible? Your insights would be greatly appreciated.

stmmac_mac_link_up() gets passed interface, speed and duplex. That
tells you what the PHY has negotiated. Is there anything else you need
to know?

Andrew

2023-09-22 04:51:30

by Andrew Lunn

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/5] TSN auto negotiation between 1G and 2.5G

On Thu, Sep 21, 2023 at 03:12:19PM +0100, Russell King (Oracle) wrote:
> On Thu, Sep 21, 2023 at 03:21:00PM +0200, Andrew Lunn wrote:
> > > Hi Andrew,
> > >
> > > After conducting a comprehensive study, it seems that implementing
> > > out-of-band for all link modes might not be feasible. I may have missed some
> > > key aspects during my analysis.
> > >
> > > Would you be open to sharing a high-level idea of how we could potentially
> > > make this feasible? Your insights would be greatly appreciated.
> >
> > stmmac_mac_link_up() gets passed interface, speed and duplex. That
> > tells you what the PHY has negotiated. Is there anything else you need
> > to know?
>
> The problem is... the stmmac driver is utter bollocks - that information
> is *not* passed to the BSP. Instead, stmmac parse and store information
> such as the PHY interface mode at initialisation time. BSPs also re-
> parse and store e.g. the PHY interface mode at initialisation time.
> The driver ignores what it gets from phylink.
>
> The driver is basically utter crap. That's an area I _had_ patches to
> clean up. I no longer do. stmmac is crap crap crap and will stay crap
> until they become more receptive to patches to fix it, even if the
> patches are not 100% to their liking but are in fact correct. Maybe
> if I ever decide to touch that driver in the future. Which I doubt
> given my recent experience.

Hi Russell

You pointed out the current proposal will break stuff. Do you see a
way forward for this patchset which does not first involve actually
cleaning up of this driver?

Andrew

2023-09-22 06:48:24

by Choong Yong Liang

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/5] TSN auto negotiation between 1G and 2.5G



On 4/8/2023 8:04 pm, Andrew Lunn wrote:
> On Fri, Aug 04, 2023 at 04:45:22PM +0800, Choong Yong Liang wrote:
>> 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.
>
> Have you considered using out of band for all link modes? You might
> end up with a cleaner architecture, and not need any phylink/phylib
> hacks.
>
> Andrew
Hi Andrew,

After conducting a comprehensive study, it seems that implementing
out-of-band for all link modes might not be feasible. I may have missed
some key aspects during my analysis.

Would you be open to sharing a high-level idea of how we could potentially
make this feasible? Your insights would be greatly appreciated.

By the way, I've submitted a new design that not exposing phylink's AN mode
into phylib. Please help review it to determine if it is acceptable.

2024-01-29 19:20:21

by Choong Yong Liang

[permalink] [raw]
Subject: Re: [PATCH net-next v2 0/5] TSN auto negotiation between 1G and 2.5G



On 21/9/2023 10:55 pm, Russell King (Oracle) wrote:
> On Thu, Sep 21, 2023 at 04:41:20PM +0200, Andrew Lunn wrote:
>> On Thu, Sep 21, 2023 at 03:12:19PM +0100, Russell King (Oracle) wrote:
>>> On Thu, Sep 21, 2023 at 03:21:00PM +0200, Andrew Lunn wrote:
>>>>> Hi Andrew,
>>>>>
>>>>> After conducting a comprehensive study, it seems that implementing
>>>>> out-of-band for all link modes might not be feasible. I may have missed some
>>>>> key aspects during my analysis.
>>>>>
>>>>> Would you be open to sharing a high-level idea of how we could potentially
>>>>> make this feasible? Your insights would be greatly appreciated.
>>>>
>>>> stmmac_mac_link_up() gets passed interface, speed and duplex. That
>>>> tells you what the PHY has negotiated. Is there anything else you need
>>>> to know?
>>>
>>> The problem is... the stmmac driver is utter bollocks - that information
>>> is *not* passed to the BSP. Instead, stmmac parse and store information
>>> such as the PHY interface mode at initialisation time. BSPs also re-
>>> parse and store e.g. the PHY interface mode at initialisation time.
>>> The driver ignores what it gets from phylink.
>>>
>>> The driver is basically utter crap. That's an area I _had_ patches to
>>> clean up. I no longer do. stmmac is crap crap crap and will stay crap
>>> until they become more receptive to patches to fix it, even if the
>>> patches are not 100% to their liking but are in fact correct. Maybe
>>> if I ever decide to touch that driver in the future. Which I doubt
>>> given my recent experience.
>>
>> Hi Russell
>>
>> You pointed out the current proposal will break stuff. Do you see a
>> way forward for this patchset which does not first involve actually
>> cleaning up of this driver?
>
> As I said in one of my replies, it would really help if the author can
> provide a table showing what is attempting to be achieved here. With
> that, we should be able to work out exactly what is required, what
> needs to change in stmmac, etc.
>

Thank you, Russell and Andrew for the comments.

What I'm trying to achieve here is to enable interface mode switching
between 2500basex and SGMII interfaces for Intel platforms.

I did based on the DM7052 SFP module and BCM84881 PHY to make the necessary
changes in the new version of my patch series.

In the new patch series, the 'allow_switch_interface' flag was introduced,
based on the 'allow_switch_interface' flag, the interface mode is
configured to PHY_INTERFACE_MODE_NA within the 'phylink_validate_phy'
function. This setting allows all ethtool link modes that are supported and
advertised will be published. Then interface mode switching occurs based on
the selection of different link modes.

During the interface mode switching, the code will go through the
`phylink_major_config` function and based on the interface mode to select
PCS and PCS negotiation mode to configure PCS. Then, the MAC driver will
perform SerDes configuration according to the interface mode.

I did rewrite the description for the new patch series, hoping that it is
clear to describe the whole intention of the changes.