[PATCH net-next v3 0/5] TSN auto negotiation between 1G and 2.5G

[PATCH net-next v3 0/5] TSN auto negotiation between 1G and 2.5G

[Choong Yong Liang]

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

-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next v3 1/5] arch: x86: Add IPC mailbox accessor function and add SoC register access

[Choong Yong Liang]

From: "David E. Box" <david.e.box@linux.intel.com>

- Exports intel_pmc_ipc() for host access to the PMC IPC mailbox
- Add support to use IPC command allows host to access SoC registers
through PMC firmware that are otherwise inaccessible to the host due to
security policies.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Signed-off-by: Chao Qin <chao.qin@intel.com>
Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>
---
 MAINTAINERS                                   |  2 +
 arch/x86/Kconfig                              |  9 +++
 arch/x86/platform/intel/Makefile              |  1 +
 arch/x86/platform/intel/pmc_ipc.c             | 75 +++++++++++++++++++
 .../linux/platform_data/x86/intel_pmc_ipc.h   | 34 +++++++++
 5 files changed, 121 insertions(+)
 create mode 100644 arch/x86/platform/intel/pmc_ipc.c
 create mode 100644 include/linux/platform_data/x86/intel_pmc_ipc.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 3dde038545d8..f608668fdfa6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10730,8 +10730,10 @@ M:	Rajneesh Bhardwaj <irenic.rajneesh@gmail.com>
 M:	David E Box <david.e.box@intel.com>
 L:	platform-driver-x86@vger.kernel.org
 S:	Maintained
+F:	arch/x86/platform/intel/pmc_ipc.c
 F:	Documentation/ABI/testing/sysfs-platform-intel-pmc
 F:	drivers/platform/x86/intel/pmc/
+F:	linux/platform_data/x86/intel_pmc_ipc.h
 
 INTEL PMIC GPIO DRIVERS
 M:	Andy Shevchenko <andy@kernel.org>
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 982b777eadc7..31fc8ece2b46 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -666,6 +666,15 @@ config X86_AMD_PLATFORM_DEVICE
 	  I2C and UART depend on COMMON_CLK to set clock. GPIO driver is
 	  implemented under PINCTRL subsystem.
 
+config INTEL_PMC_IPC
+	tristate "Intel Core SoC Power Management Controller IPC mailbox"
+	depends on ACPI
+	help
+	  This option enables sideband register access support for Intel SoC
+	  power management controller IPC mailbox.
+
+	  If you don't require the option or are in doubt, say N.
+
 config IOSF_MBI
 	tristate "Intel SoC IOSF Sideband support for SoC platforms"
 	depends on PCI
diff --git a/arch/x86/platform/intel/Makefile b/arch/x86/platform/intel/Makefile
index dbee3b00f9d0..470fc68de6ba 100644
--- a/arch/x86/platform/intel/Makefile
+++ b/arch/x86/platform/intel/Makefile
@@ -1,2 +1,3 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_IOSF_MBI)			+= iosf_mbi.o
+obj-$(CONFIG_INTEL_PMC_IPC)		+= pmc_ipc.o
\ No newline at end of file
diff --git a/arch/x86/platform/intel/pmc_ipc.c b/arch/x86/platform/intel/pmc_ipc.c
new file mode 100644
index 000000000000..a96234982710
--- /dev/null
+++ b/arch/x86/platform/intel/pmc_ipc.c
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Intel Core SoC Power Management Controller IPC mailbox
+ *
+ * Copyright (c) 2023, Intel Corporation.
+ * All Rights Reserved.
+ *
+ * Authors: Choong Yong Liang <yong.liang.choong@linux.intel.com>
+ *          David E. Box <david.e.box@linux.intel.com>
+ */
+#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/platform_data/x86/intel_pmc_ipc.h>
+
+#define PMC_IPCS_PARAM_COUNT           7
+
+int intel_pmc_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf)
+{
+	struct acpi_buffer buffer = { ACPI_ALLOCATE_BUFFER, NULL };
+	union acpi_object params[PMC_IPCS_PARAM_COUNT] = {
+		{.type = ACPI_TYPE_INTEGER,},
+		{.type = ACPI_TYPE_INTEGER,},
+		{.type = ACPI_TYPE_INTEGER,},
+		{.type = ACPI_TYPE_INTEGER,},
+		{.type = ACPI_TYPE_INTEGER,},
+		{.type = ACPI_TYPE_INTEGER,},
+		{.type = ACPI_TYPE_INTEGER,},
+	};
+	struct acpi_object_list arg_list = { PMC_IPCS_PARAM_COUNT, params };
+	union acpi_object *obj;
+	int status;
+
+	if (!ipc_cmd || !rbuf)
+		return -EINVAL;
+
+	/*
+	 * 0: IPC Command
+	 * 1: IPC Sub Command
+	 * 2: Size
+	 * 3-6: Write Buffer for offset
+	 */
+	params[0].integer.value = ipc_cmd->cmd;
+	params[1].integer.value = ipc_cmd->sub_cmd;
+	params[2].integer.value = ipc_cmd->size;
+	params[3].integer.value = ipc_cmd->wbuf[0];
+	params[4].integer.value = ipc_cmd->wbuf[1];
+	params[5].integer.value = ipc_cmd->wbuf[2];
+	params[6].integer.value = ipc_cmd->wbuf[3];
+
+	status = acpi_evaluate_object(NULL, "\\IPCS", &arg_list, &buffer);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	obj = buffer.pointer;
+	/* Check if the number of elements in package is 5 */
+	if (obj && obj->type == ACPI_TYPE_PACKAGE && obj->package.count == 5) {
+		const union acpi_object *objs = obj->package.elements;
+
+		if ((u8)objs[0].integer.value != 0)
+			return -EINVAL;
+
+		rbuf[0] = objs[1].integer.value;
+		rbuf[1] = objs[2].integer.value;
+		rbuf[2] = objs[3].integer.value;
+		rbuf[3] = objs[4].integer.value;
+	} else {
+		return -EINVAL;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(intel_pmc_ipc);
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("Intel PMC IPC Mailbox accessor");
diff --git a/include/linux/platform_data/x86/intel_pmc_ipc.h b/include/linux/platform_data/x86/intel_pmc_ipc.h
new file mode 100644
index 000000000000..25ba57b8a7ea
--- /dev/null
+++ b/include/linux/platform_data/x86/intel_pmc_ipc.h
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Intel Core SoC Power Management Controller Header File
+ *
+ * Copyright (c) 2023, Intel Corporation.
+ * All Rights Reserved.
+ *
+ * Authors: Choong Yong Liang <yong.liang.choong@linux.intel.com>
+ *          David E. Box <david.e.box@linux.intel.com>
+ */
+#ifndef INTEL_PMC_IPC_H
+#define INTEL_PMC_IPC_H
+
+#define IPC_SOC_REGISTER_ACCESS			0xAA
+#define IPC_SOC_SUB_CMD_READ			0x00
+#define IPC_SOC_SUB_CMD_WRITE			0x01
+
+struct pmc_ipc_cmd {
+	u32 cmd;
+	u32 sub_cmd;
+	u32 size;
+	u32 wbuf[4];
+};
+
+/**
+ * intel_pmc_core_ipc() - PMC IPC Mailbox accessor
+ * @ipc_cmd:  struct pmc_ipc_cmd prepared with input to send
+ * @rbuf:     Allocated u32[4] array for returned IPC data
+ *
+ * Return: 0 on success. Non-zero on mailbox error
+ */
+int intel_pmc_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf);
+
+#endif /* INTEL_PMC_IPC_H */
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v3 1/5] arch: x86: Add IPC mailbox accessor function and add SoC register access

[Simon Horman]

On Thu, Sep 21, 2023 at 08:19:42PM +0800, Choong Yong Liang wrote:
> From: "David E. Box" <david.e.box@linux.intel.com>
> 
> - Exports intel_pmc_ipc() for host access to the PMC IPC mailbox
> - Add support to use IPC command allows host to access SoC registers
> through PMC firmware that are otherwise inaccessible to the host due to
> security policies.
> 
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> Signed-off-by: Chao Qin <chao.qin@intel.com>
> Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>

...

> diff --git a/include/linux/platform_data/x86/intel_pmc_ipc.h b/include/linux/platform_data/x86/intel_pmc_ipc.h
> new file mode 100644
> index 000000000000..25ba57b8a7ea
> --- /dev/null
> +++ b/include/linux/platform_data/x86/intel_pmc_ipc.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Intel Core SoC Power Management Controller Header File
> + *
> + * Copyright (c) 2023, Intel Corporation.
> + * All Rights Reserved.
> + *
> + * Authors: Choong Yong Liang <yong.liang.choong@linux.intel.com>
> + *          David E. Box <david.e.box@linux.intel.com>
> + */
> +#ifndef INTEL_PMC_IPC_H
> +#define INTEL_PMC_IPC_H
> +
> +#define IPC_SOC_REGISTER_ACCESS			0xAA
> +#define IPC_SOC_SUB_CMD_READ			0x00
> +#define IPC_SOC_SUB_CMD_WRITE			0x01
> +
> +struct pmc_ipc_cmd {
> +	u32 cmd;
> +	u32 sub_cmd;
> +	u32 size;
> +	u32 wbuf[4];
> +};
> +
> +/**
> + * intel_pmc_core_ipc() - PMC IPC Mailbox accessor

nit: intel_pmc_ipc()

> + * @ipc_cmd:  struct pmc_ipc_cmd prepared with input to send
> + * @rbuf:     Allocated u32[4] array for returned IPC data
> + *
> + * Return: 0 on success. Non-zero on mailbox error
> + */
> +int intel_pmc_ipc(struct pmc_ipc_cmd *ipc_cmd, u32 *rbuf);
> +
> +#endif /* INTEL_PMC_IPC_H */
> -- 
> 2.25.1
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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

[Choong Yong Liang]

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
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,
+	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;
 	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,
-	},
 	[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 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


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v3 2/5] net: pcs: xpcs: combine C37 SGMII AN and 2500BASEX for Intel mGbE controller

[Russell King (Oracle)]

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
> 
> 

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v3 2/5] net: pcs: xpcs: combine C37 SGMII AN and 2500BASEX for Intel mGbE controller

[Serge Semin]

Hi Choong

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
> 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.

Why do you even need all of those changes? Please thoroughly justify
because ... (see below)

> 
> 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,
> +	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,
> +};
> +

... these are just a combination of the
xpcs_sgmii_features[]/xpcs_sgmii_interfaces[] and
xpcs_2500basex_features[]/xpcs_2500basex_interfaces[] data which are
already supported by the generic DW XPCS code. All of these features
and interfaces are checked in the xpcs_create() method and then get to
be combined in the framework of the xpcs_validate() and
xpcs_get_interfaces() functions. And ...

>  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;

* This is the only specific change in this patch. But it can be
* applied independently from the rest of the changes. Although I agree
* with Russel, it must be double checked since may cause regressions
* on the other platforms.

>  	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;
> +}
> +

... this is just a copy of the code which is already available in xpcs_do_config():

<        compat = xpcs_find_compat(xpcs->id, interface);
<        if (!compat)
<                return -ENODEV;
<
<        switch (compat->an_mode) {
< ...
<        case DW_AN_C37_SGMII:
<                ret = xpcs_config_aneg_c37_sgmii(xpcs, neg_mode);
<                if (ret)
<                        return ret;
<                break;
< ...
<        case DW_2500BASEX:
<                ret = xpcs_config_2500basex(xpcs);
<                if (ret)
<                        return ret;
<                break;

So based on the passed interface xpcs_find_compat() will find a proper
compat descriptor, which an_mode field will be then utilized to call
the respective config method. Thus, unless I miss something, basically
you won't need any of the changes below and the most of the changes
above reducing the patch to just a few lines.

-Serge(y)

>  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,
> -	},
>  	[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 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
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v3 2/5] net: pcs: xpcs: combine C37 SGMII AN and 2500BASEX for Intel mGbE controller

[Choong Yong Liang]

On 26/9/2023 6:51 pm, Serge Semin wrote:
> Hi Choong
> 
> 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
>> 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.
> 
> Why do you even need all of those changes? Please thoroughly justify
> because ... (see below)
> 
>>
>> 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,
>> +	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,
>> +};
>> +
> 
> ... these are just a combination of the
> xpcs_sgmii_features[]/xpcs_sgmii_interfaces[] and
> xpcs_2500basex_features[]/xpcs_2500basex_interfaces[] data which are
> already supported by the generic DW XPCS code. All of these features
> and interfaces are checked in the xpcs_create() method and then get to
> be combined in the framework of the xpcs_validate() and
> xpcs_get_interfaces() functions. And ...
> 
>>   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;
> 
> * This is the only specific change in this patch. But it can be
> * applied independently from the rest of the changes. Although I agree
> * with Russel, it must be double checked since may cause regressions
> * on the other platforms.
> 
>>   	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;
>> +}
>> +
> 
> ... this is just a copy of the code which is already available in xpcs_do_config():
> 
> <        compat = xpcs_find_compat(xpcs->id, interface);
> <        if (!compat)
> <                return -ENODEV;
> <
> <        switch (compat->an_mode) {
> < ...
> <        case DW_AN_C37_SGMII:
> <                ret = xpcs_config_aneg_c37_sgmii(xpcs, neg_mode);
> <                if (ret)
> <                        return ret;
> <                break;
> < ...
> <        case DW_2500BASEX:
> <                ret = xpcs_config_2500basex(xpcs);
> <                if (ret)
> <                        return ret;
> <                break;
> 
> So based on the passed interface xpcs_find_compat() will find a proper
> compat descriptor, which an_mode field will be then utilized to call
> the respective config method. Thus, unless I miss something, basically
> you won't need any of the changes below and the most of the changes
> above reducing the patch to just a few lines.
> 
> -Serge(y)
> 
>>   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,
>> -	},
>>   	[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 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
>>
>>
Hi Serge and Russell

This patch does not serve the purpose correctly, I did remove this patch 
and handle it properly in the new patch series.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next v3 3/5] net: phy: update in-band AN mode when changing interface by PHY driver

[Choong Yong Liang]

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: Choong Yong Liang <yong.liang.choong@linux.intel.com>
---
 drivers/net/phy/phylink.c | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 0d7354955d62..5811c8086149 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1723,6 +1723,34 @@ bool phylink_expects_phy(struct phylink *pl)
 }
 EXPORT_SYMBOL_GPL(phylink_expects_phy);
 
+/**
+ * phylink_interface_change() - update both cfg_link_an_mode and
+ * cur_link_an_mode when there is a change in the interface.
+ * @phydev: pointer to &struct phy_device
+ *
+ * When the PHY interface switches between SGMII and 2500BaseX in
+ * accordance with the link speed, the in-band AN mode should also switch
+ * based on the PHY interface
+ */
+static void phylink_interface_change(struct phy_device *phydev)
+{
+	struct phylink *pl = phydev->phylink;
+
+	if (pl->phy_state.interface != phydev->interface) {
+		/* Fallback to the correct AN mode. */
+		if (phy_interface_mode_is_8023z(phydev->interface) &&
+		    pl->cfg_link_an_mode == MLO_AN_INBAND) {
+			pl->cfg_link_an_mode = MLO_AN_PHY;
+			pl->cur_link_an_mode = MLO_AN_PHY;
+		} else if (pl->config->ovr_an_inband) {
+			pl->cfg_link_an_mode = MLO_AN_INBAND;
+			pl->cur_link_an_mode = MLO_AN_INBAND;
+		}
+
+		pl->phy_state.interface = phydev->interface;
+	}
+}
+
 static void phylink_phy_change(struct phy_device *phydev, bool up)
 {
 	struct phylink *pl = phydev->phylink;
@@ -1739,7 +1767,7 @@ static void phylink_phy_change(struct phy_device *phydev, bool up)
 		pl->phy_state.pause |= MLO_PAUSE_TX;
 	if (rx_pause)
 		pl->phy_state.pause |= MLO_PAUSE_RX;
-	pl->phy_state.interface = phydev->interface;
+	phylink_interface_change(phydev);
 	pl->phy_state.link = up;
 	mutex_unlock(&pl->state_mutex);
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v3 3/5] net: phy: update in-band AN mode when changing interface by PHY driver

[Russell King (Oracle)]

On Thu, Sep 21, 2023 at 08:19:44PM +0800, Choong Yong Liang wrote:
> 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: Choong Yong Liang <yong.liang.choong@linux.intel.com>

This approach is *going* to break existing setups, sorry.

> +/**
> + * phylink_interface_change() - update both cfg_link_an_mode and
> + * cur_link_an_mode when there is a change in the interface.
> + * @phydev: pointer to &struct phy_device
> + *
> + * When the PHY interface switches between SGMII and 2500BaseX in
> + * accordance with the link speed, the in-band AN mode should also switch
> + * based on the PHY interface
> + */
> +static void phylink_interface_change(struct phy_device *phydev)
> +{
> +	struct phylink *pl = phydev->phylink;
> +
> +	if (pl->phy_state.interface != phydev->interface) {
> +		/* Fallback to the correct AN mode. */
> +		if (phy_interface_mode_is_8023z(phydev->interface) &&
> +		    pl->cfg_link_an_mode == MLO_AN_INBAND) {
> +			pl->cfg_link_an_mode = MLO_AN_PHY;
> +			pl->cur_link_an_mode = MLO_AN_PHY;

1. Why are you changing both cfg_link_an_mode (configured link AN mode)
and cur_link_an_mode (current link AN mode) ?

The "configured" link AN mode is supposed to be whatever was configured
at phylink creation time, and it's never supposed to change. The
"current" link AN mode can change, but changing that must be followed
by a major reconfiguration to ensure everything is correctly setup.
That will happen only because the change to the current link AN mode
can only happen when pl->phy_state.interface has changed, and the
change of pl->phy_state.interface triggers the reconfiguration.

2. You force this behaviour on everyone, so now everyone with a SFP
module that operates in 802.3z mode will be switched out of inband mode
whether they want that or not. This is likely to cause some breakage.

> +		} else if (pl->config->ovr_an_inband) {
> +			pl->cfg_link_an_mode = MLO_AN_INBAND;
> +			pl->cur_link_an_mode = MLO_AN_INBAND;

Here you force inband when not 802.3z mode and ovr_an_inband is set.
There are SFP modules that do *not* support in-band at all, and this
will break these modules when combined with a driver that sets
ovr_an_inband. So more breakage.

Please enumerate the PHY interface modes that you are trying to support
with this patch set, and indicate whether you want in-band for that
mode or not, and where the restriction for whether in-band can be used
comes from (PHY, PCS or MAC) so that it's possible to better understand
what you're trying to achieve.

Thanks.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v3 3/5] net: phy: update in-band AN mode when changing interface by PHY driver

[Choong Yong Liang]

On 21/9/2023 10:04 pm, Russell King (Oracle) wrote:
> On Thu, Sep 21, 2023 at 08:19:44PM +0800, Choong Yong Liang wrote:
>> 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: Choong Yong Liang <yong.liang.choong@linux.intel.com>
> 
> This approach is *going* to break existing setups, sorry.
> 
>> +/**
>> + * phylink_interface_change() - update both cfg_link_an_mode and
>> + * cur_link_an_mode when there is a change in the interface.
>> + * @phydev: pointer to &struct phy_device
>> + *
>> + * When the PHY interface switches between SGMII and 2500BaseX in
>> + * accordance with the link speed, the in-band AN mode should also switch
>> + * based on the PHY interface
>> + */
>> +static void phylink_interface_change(struct phy_device *phydev)
>> +{
>> +	struct phylink *pl = phydev->phylink;
>> +
>> +	if (pl->phy_state.interface != phydev->interface) {
>> +		/* Fallback to the correct AN mode. */
>> +		if (phy_interface_mode_is_8023z(phydev->interface) &&
>> +		    pl->cfg_link_an_mode == MLO_AN_INBAND) {
>> +			pl->cfg_link_an_mode = MLO_AN_PHY;
>> +			pl->cur_link_an_mode = MLO_AN_PHY;
> 
> 1. Why are you changing both cfg_link_an_mode (configured link AN mode)
> and cur_link_an_mode (current link AN mode) ?
> 
> The "configured" link AN mode is supposed to be whatever was configured
> at phylink creation time, and it's never supposed to change. The
> "current" link AN mode can change, but changing that must be followed
> by a major reconfiguration to ensure everything is correctly setup.
> That will happen only because the change to the current link AN mode
> can only happen when pl->phy_state.interface has changed, and the
> change of pl->phy_state.interface triggers the reconfiguration.
> 
During the phylink_create, the cfg_link_an_mode will be set to MLO_AN_INBAND.

Then we switch from the SGMII interface to 2500BASEX interface. When we 
perform 'ifconfig eth0 down' and 'ifconfig eth0 up' then we will not able 
to bring up the PHY due to the phylink_attach_phy function. It is not 
expect to have MLO_AN_INBAND with PHY_INTERFACE_MODE_2500BASEX interface.

static int phylink_attach_phy(struct phylink *pl, struct phy_device *phy,
			      phy_interface_t interface)
{
	if (WARN_ON(pl->cfg_link_an_mode == MLO_AN_FIXED ||
		    (pl->cfg_link_an_mode == MLO_AN_INBAND &&
		     phy_interface_mode_is_8023z(interface) && !pl->sfp_bus)))
		return -EINVAL;

	if (pl->phydev)
		return -EBUSY;

	return phy_attach_direct(pl->netdev, phy, 0, interface);
}

I did change different ways to handle it in the new patch series.
So it should not impact much on the existing phylink framework.

> 2. You force this behaviour on everyone, so now everyone with a SFP
> module that operates in 802.3z mode will be switched out of inband mode
> whether they want that or not. This is likely to cause some breakage.
> 
>> +		} else if (pl->config->ovr_an_inband) {
>> +			pl->cfg_link_an_mode = MLO_AN_INBAND;
>> +			pl->cur_link_an_mode = MLO_AN_INBAND;
> 
> Here you force inband when not 802.3z mode and ovr_an_inband is set.
> There are SFP modules that do *not* support in-band at all, and this
> will break these modules when combined with a driver that sets
> ovr_an_inband. So more breakage.
> 
> Please enumerate the PHY interface modes that you are trying to support
> with this patch set, and indicate whether you want in-band for that
> mode or not, and where the restriction for whether in-band can be used
> comes from (PHY, PCS or MAC) so that it's possible to better understand
> what you're trying to achieve.
> 
> Thanks.
> 
Thank you for pointing out the bug that will impact everyone. Actually 
cur_link_an_mode is just required for PCS, I did handle it that only intel 
platforms will get the PCS negotiation mode for the PCS.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next v3 4/5] net: stmmac: enable Intel mGbE 1G/2.5G auto-negotiation support

[Choong Yong Liang]

From: "Tan, Tee Min" <tee.min.tan@linux.intel.com>

Initially, Intel mGbE was only able to configure the overclocking of 2.5
times clock rate to enable 2.5Gbps in the BIOS during boot time. Kernel
driver had no access to modify the clock rate for 1G/2.5G mode at runtime.

Now, this patch enables the runtime 1G/2.5G auto-negotiation support to
gets rid of the dependency on BIOS to change the 1G/2.5G clock rate.

This patch adds several new functions below:-
- intel_tsn_interface_is_available(): This new function reads FIA lane
  ownership registers and common lane registers through IPC commands
  to know which lane the mGbE port is assigned to.
- stmmac_mac_prepare(): To obtain the latest PHY interface from phylink
  during initialization and call intel_config_serdes() to proceed with
  SERDES configuration.
- intel_config_serdes(): To configure the SERDES based on the assigned
  lane and latest PHY interface, it sends IPC command to the PMC through
  PMC driver/API. The PMC acts as a proxy for R/W on behalf of the driver.
- intel_set_reg_access(): Set the register access to the available TSN
  interface.

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/ethernet/stmicro/stmmac/Kconfig   |   1 +
 .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 134 +++++++++++++++++-
 .../net/ethernet/stmicro/stmmac/dwmac-intel.h |  79 +++++++++++
 .../net/ethernet/stmicro/stmmac/stmmac_main.c |  20 +++
 include/linux/stmmac.h                        |   1 +
 5 files changed, 231 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index a2b9e289aa36..4340efd9bd50 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -273,6 +273,7 @@ config DWMAC_INTEL
 	default X86
 	depends on X86 && STMMAC_ETH && PCI
 	depends on COMMON_CLK
+	select INTEL_PMC_IPC
 	help
 	  This selects the Intel platform specific bus support for the
 	  stmmac driver. This driver is used for Intel Quark/EHL/TGL.
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
index a3a249c63598..a211f42914a2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
@@ -5,6 +5,7 @@
 #include <linux/clk-provider.h>
 #include <linux/pci.h>
 #include <linux/dmi.h>
+#include <linux/platform_data/x86/intel_pmc_ipc.h>
 #include "dwmac-intel.h"
 #include "dwmac4.h"
 #include "stmmac.h"
@@ -14,6 +15,9 @@ struct intel_priv_data {
 	int mdio_adhoc_addr;	/* mdio address for serdes & etc */
 	unsigned long crossts_adj;
 	bool is_pse;
+	const int *tsn_lane_registers;
+	int max_tsn_lane_registers;
+	int pid_modphy;
 };
 
 /* This struct is used to associate PCI Function of MAC controller on a board,
@@ -93,7 +97,7 @@ static int intel_serdes_powerup(struct net_device *ndev, void *priv_data)
 	data &= ~SERDES_RATE_MASK;
 	data &= ~SERDES_PCLK_MASK;
 
-	if (priv->plat->max_speed == 2500)
+	if (priv->plat->phy_interface == PHY_INTERFACE_MODE_2500BASEX)
 		data |= SERDES_RATE_PCIE_GEN2 << SERDES_RATE_PCIE_SHIFT |
 			SERDES_PCLK_37p5MHZ << SERDES_PCLK_SHIFT;
 	else
@@ -414,6 +418,106 @@ static void intel_mgbe_pse_crossts_adj(struct intel_priv_data *intel_priv,
 	}
 }
 
+#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
+static bool intel_tsn_interface_is_available(struct net_device *ndev,
+					     struct intel_priv_data *intel_priv)
+{
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	struct pmc_ipc_cmd tmp = {0};
+	u32 rbuf[4] = {0};
+	int ret, i, j;
+
+	if (priv->plat->serdes_powerup) {
+		tmp.cmd = IPC_SOC_REGISTER_ACCESS;
+		tmp.sub_cmd = IPC_SOC_SUB_CMD_READ;
+
+		for (i = 0; i < 5; i++) {
+			tmp.wbuf[0] = R_PCH_FIA_15_PCR_LOS1_REG_BASE + i;
+
+			ret = intel_pmc_ipc(&tmp, rbuf);
+			if (ret < 0) {
+				netdev_info(priv->dev,
+					    "Failed to read from PMC.\n");
+				return false;
+			}
+
+			for (j = 0; j <= intel_priv->max_tsn_lane_registers; j++)
+				if ((rbuf[0] >>
+				    (4 * (intel_priv->tsn_lane_registers[j] % 8)) &
+				     B_PCH_FIA_PCR_L0O) == 0xB)
+					return true;
+		}
+	}
+	return false;
+}
+
+static int intel_set_reg_access(const struct pmc_serdes_regs *regs, int max_regs)
+{
+	int ret = 0, i;
+
+	for (i = 0; i < max_regs; i++) {
+		struct pmc_ipc_cmd tmp = {0};
+		u32 buf[4] = {0};
+
+		tmp.cmd = IPC_SOC_REGISTER_ACCESS;
+		tmp.sub_cmd = IPC_SOC_SUB_CMD_WRITE;
+		tmp.wbuf[0] = (u32)regs[i].index;
+		tmp.wbuf[1] = regs[i].val;
+
+		ret = intel_pmc_ipc(&tmp, buf);
+		if (ret < 0)
+			return ret;
+	}
+
+	return ret;
+}
+
+static int intel_config_serdes(struct net_device *ndev, void *intel_data)
+{
+	struct intel_priv_data *intel_priv = intel_data;
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	int ret = 0;
+
+	if (!intel_tsn_interface_is_available(ndev, intel_priv)) {
+		netdev_info(priv->dev,
+			    "No TSN interface available to set the registers.\n");
+		goto pmc_read_error;
+	}
+
+	if (intel_priv->pid_modphy == PID_MODPHY1) {
+		if (priv->plat->phy_interface == PHY_INTERFACE_MODE_2500BASEX) {
+			ret = intel_set_reg_access(pid_modphy1_2p5g_regs,
+						   ARRAY_SIZE(pid_modphy1_2p5g_regs));
+		} else {
+			ret = intel_set_reg_access(pid_modphy1_1g_regs,
+						   ARRAY_SIZE(pid_modphy1_1g_regs));
+		}
+	} else {
+		if (priv->plat->phy_interface == PHY_INTERFACE_MODE_2500BASEX) {
+			ret = intel_set_reg_access(pid_modphy3_2p5g_regs,
+						   ARRAY_SIZE(pid_modphy3_2p5g_regs));
+		} else {
+			ret = intel_set_reg_access(pid_modphy3_1g_regs,
+						   ARRAY_SIZE(pid_modphy3_1g_regs));
+		}
+	}
+
+	if (ret < 0)
+		goto pmc_read_error;
+
+pmc_read_error:
+	intel_serdes_powerdown(ndev, intel_priv);
+	intel_serdes_powerup(ndev, intel_priv);
+
+	return ret;
+}
+#else
+static int intel_config_serdes(struct net_device *ndev, void *intel_data)
+{
+	return -EOPNOTSUPP;
+}
+#endif /* CONFIG_INTEL_PMC_IPC */
+
 static void common_default_data(struct plat_stmmacenet_data *plat)
 {
 	plat->clk_csr = 2;	/* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
@@ -624,6 +728,8 @@ static int intel_mgbe_common_data(struct pci_dev *pdev,
 static int ehl_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 = 8;
 	plat->tx_queues_to_use = 8;
 	plat->flags |= STMMAC_FLAG_USE_PHY_WOL;
@@ -639,20 +745,28 @@ static int ehl_common_data(struct pci_dev *pdev,
 	plat->safety_feat_cfg->prtyen = 0;
 	plat->safety_feat_cfg->tmouten = 0;
 
+	intel_priv->tsn_lane_registers = ehl_tsn_lane_registers;
+	intel_priv->max_tsn_lane_registers = ARRAY_SIZE(ehl_tsn_lane_registers);
+
 	return intel_mgbe_common_data(pdev, plat);
 }
 
 static int ehl_sgmii_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->speed_mode_2500 = intel_speed_mode_2500;
+	plat->max_speed = SPEED_2500;
 	plat->serdes_powerup = intel_serdes_powerup;
 	plat->serdes_powerdown = intel_serdes_powerdown;
+	plat->config_serdes = intel_config_serdes;
 
 	plat->clk_ptp_rate = 204800000;
 
+	intel_priv->pid_modphy = PID_MODPHY3;
+
 	return ehl_common_data(pdev, plat);
 }
 
@@ -705,10 +819,16 @@ static struct stmmac_pci_info ehl_pse0_rgmii1g_info = {
 static int ehl_pse0_sgmii1g_data(struct pci_dev *pdev,
 				 struct plat_stmmacenet_data *plat)
 {
+	struct intel_priv_data *intel_priv = plat->bsp_priv;
+
 	plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
-	plat->speed_mode_2500 = intel_speed_mode_2500;
+	plat->max_speed = SPEED_2500;
 	plat->serdes_powerup = intel_serdes_powerup;
 	plat->serdes_powerdown = intel_serdes_powerdown;
+	plat->config_serdes = intel_config_serdes;
+
+	intel_priv->pid_modphy = PID_MODPHY1;
+
 	return ehl_pse0_common_data(pdev, plat);
 }
 
@@ -746,10 +866,16 @@ static struct stmmac_pci_info ehl_pse1_rgmii1g_info = {
 static int ehl_pse1_sgmii1g_data(struct pci_dev *pdev,
 				 struct plat_stmmacenet_data *plat)
 {
+	struct intel_priv_data *intel_priv = plat->bsp_priv;
+
 	plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
-	plat->speed_mode_2500 = intel_speed_mode_2500;
+	plat->max_speed = SPEED_2500;
 	plat->serdes_powerup = intel_serdes_powerup;
 	plat->serdes_powerdown = intel_serdes_powerdown;
+	plat->config_serdes = intel_config_serdes;
+
+	intel_priv->pid_modphy = PID_MODPHY1;
+
 	return ehl_pse1_common_data(pdev, plat);
 }
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
index 0a37987478c1..093eed977ab0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
@@ -50,4 +50,83 @@
 #define PCH_PTP_CLK_FREQ_19_2MHZ	(GMAC_GPO0)
 #define PCH_PTP_CLK_FREQ_200MHZ		(0)
 
+#define	PID_MODPHY1 0xAA
+#define	PID_MODPHY3 0xA8
+
+#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
+struct pmc_serdes_regs {
+	u8 index;
+	u32 val;
+};
+
+/* Modphy Register index */
+#define R_PCH_FIA_15_PCR_LOS1_REG_BASE			8
+#define R_PCH_FIA_15_PCR_LOS2_REG_BASE			9
+#define R_PCH_FIA_15_PCR_LOS3_REG_BASE			10
+#define R_PCH_FIA_15_PCR_LOS4_REG_BASE			11
+#define R_PCH_FIA_15_PCR_LOS5_REG_BASE			12
+#define B_PCH_FIA_PCR_L0O				GENMASK(3, 0)
+#define PID_MODPHY1_B_MODPHY_PCR_LCPLL_DWORD0		13
+#define PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD2		14
+#define PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD7		15
+#define PID_MODPHY1_N_MODPHY_PCR_LPPLL_DWORD10		16
+#define PID_MODPHY1_N_MODPHY_PCR_CMN_ANA_DWORD30	17
+#define PID_MODPHY3_B_MODPHY_PCR_LCPLL_DWORD0		18
+#define PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD2		19
+#define PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD7		20
+#define PID_MODPHY3_N_MODPHY_PCR_LPPLL_DWORD10		21
+#define PID_MODPHY3_N_MODPHY_PCR_CMN_ANA_DWORD30	22
+
+#define B_MODPHY_PCR_LCPLL_DWORD0_1G		0x46AAAA41
+#define N_MODPHY_PCR_LCPLL_DWORD2_1G		0x00000139
+#define N_MODPHY_PCR_LCPLL_DWORD7_1G		0x002A0003
+#define N_MODPHY_PCR_LPPLL_DWORD10_1G		0x00170008
+#define N_MODPHY_PCR_CMN_ANA_DWORD30_1G		0x0000D4AC
+#define B_MODPHY_PCR_LCPLL_DWORD0_2P5G		0x58555551
+#define N_MODPHY_PCR_LCPLL_DWORD2_2P5G		0x0000012D
+#define N_MODPHY_PCR_LCPLL_DWORD7_2P5G		0x001F0003
+#define N_MODPHY_PCR_LPPLL_DWORD10_2P5G		0x00170008
+#define N_MODPHY_PCR_CMN_ANA_DWORD30_2P5G	0x8200ACAC
+
+static const struct pmc_serdes_regs pid_modphy3_1g_regs[] = {
+	{ PID_MODPHY3_B_MODPHY_PCR_LCPLL_DWORD0,	B_MODPHY_PCR_LCPLL_DWORD0_1G },
+	{ PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD2,	N_MODPHY_PCR_LCPLL_DWORD2_1G },
+	{ PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD7,	N_MODPHY_PCR_LCPLL_DWORD7_1G },
+	{ PID_MODPHY3_N_MODPHY_PCR_LPPLL_DWORD10,	N_MODPHY_PCR_LPPLL_DWORD10_1G },
+	{ PID_MODPHY3_N_MODPHY_PCR_CMN_ANA_DWORD30,	N_MODPHY_PCR_CMN_ANA_DWORD30_1G },
+	{}
+};
+
+static const struct pmc_serdes_regs pid_modphy3_2p5g_regs[] = {
+	{ PID_MODPHY3_B_MODPHY_PCR_LCPLL_DWORD0,	B_MODPHY_PCR_LCPLL_DWORD0_2P5G },
+	{ PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD2,	N_MODPHY_PCR_LCPLL_DWORD2_2P5G },
+	{ PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD7,	N_MODPHY_PCR_LCPLL_DWORD7_2P5G },
+	{ PID_MODPHY3_N_MODPHY_PCR_LPPLL_DWORD10,	N_MODPHY_PCR_LPPLL_DWORD10_2P5G },
+	{ PID_MODPHY3_N_MODPHY_PCR_CMN_ANA_DWORD30,	N_MODPHY_PCR_CMN_ANA_DWORD30_2P5G },
+	{}
+};
+
+static const struct pmc_serdes_regs pid_modphy1_1g_regs[] = {
+	{ PID_MODPHY1_B_MODPHY_PCR_LCPLL_DWORD0,	B_MODPHY_PCR_LCPLL_DWORD0_1G },
+	{ PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD2,	N_MODPHY_PCR_LCPLL_DWORD2_1G },
+	{ PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD7,	N_MODPHY_PCR_LCPLL_DWORD7_1G },
+	{ PID_MODPHY1_N_MODPHY_PCR_LPPLL_DWORD10,	N_MODPHY_PCR_LPPLL_DWORD10_1G },
+	{ PID_MODPHY1_N_MODPHY_PCR_CMN_ANA_DWORD30,	N_MODPHY_PCR_CMN_ANA_DWORD30_1G },
+	{}
+};
+
+static const struct pmc_serdes_regs pid_modphy1_2p5g_regs[] = {
+	{ PID_MODPHY1_B_MODPHY_PCR_LCPLL_DWORD0,	B_MODPHY_PCR_LCPLL_DWORD0_2P5G },
+	{ PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD2,	N_MODPHY_PCR_LCPLL_DWORD2_2P5G },
+	{ PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD7,	N_MODPHY_PCR_LCPLL_DWORD7_2P5G },
+	{ PID_MODPHY1_N_MODPHY_PCR_LPPLL_DWORD10,	N_MODPHY_PCR_LPPLL_DWORD10_2P5G },
+	{ PID_MODPHY1_N_MODPHY_PCR_CMN_ANA_DWORD30,	N_MODPHY_PCR_CMN_ANA_DWORD30_2P5G },
+	{}
+};
+
+static const int ehl_tsn_lane_registers[] = {7, 8, 9, 10, 11};
+#else
+static const int ehl_tsn_lane_registers[] = {};
+#endif /* CONFIG_INTEL_PMC_IPC */
+
 #endif /* __DWMAC_INTEL_H__ */
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 9201ed778ebc..75765cf52cd1 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1103,11 +1103,31 @@ static void stmmac_mac_link_up(struct phylink_config *config,
 		stmmac_hwtstamp_correct_latency(priv, priv);
 }
 
+#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
+static int stmmac_mac_prepare(struct phylink_config *config, unsigned int mode,
+			      phy_interface_t interface)
+{
+	struct net_device *ndev = to_net_dev(config->dev);
+	struct stmmac_priv *priv = netdev_priv(ndev);
+	int ret = 0;
+
+	priv->plat->phy_interface = interface;
+
+	if (priv->plat->config_serdes)
+		ret = priv->plat->config_serdes(ndev, priv->plat->bsp_priv);
+
+	return ret;
+}
+#endif
+
 static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
 	.mac_select_pcs = stmmac_mac_select_pcs,
 	.mac_config = stmmac_mac_config,
 	.mac_link_down = stmmac_mac_link_down,
 	.mac_link_up = stmmac_mac_link_up,
+#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
+	.mac_prepare = stmmac_mac_prepare,
+#endif
 };
 
 /**
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index c0079a7574ae..aa7d4d96391c 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -275,6 +275,7 @@ struct plat_stmmacenet_data {
 	int (*serdes_powerup)(struct net_device *ndev, void *priv);
 	void (*serdes_powerdown)(struct net_device *ndev, void *priv);
 	void (*speed_mode_2500)(struct net_device *ndev, void *priv);
+	int (*config_serdes)(struct net_device *ndev, void *priv);
 	void (*ptp_clk_freq_config)(struct stmmac_priv *priv);
 	int (*init)(struct platform_device *pdev, void *priv);
 	void (*exit)(struct platform_device *pdev, void *priv);
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v3 4/5] net: stmmac: enable Intel mGbE 1G/2.5G auto-negotiation support

[Russell King (Oracle)]

On Thu, Sep 21, 2023 at 08:19:45PM +0800, Choong Yong Liang wrote:
> +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)

There shouldn't be any need to make this conditional.

> +static int stmmac_mac_prepare(struct phylink_config *config, unsigned int mode,
> +			      phy_interface_t interface)
> +{
> +	struct net_device *ndev = to_net_dev(config->dev);
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	int ret = 0;
> +
> +	priv->plat->phy_interface = interface;
> +
> +	if (priv->plat->config_serdes)
> +		ret = priv->plat->config_serdes(ndev, priv->plat->bsp_priv);

Please call this "phylink_mac_prepare" and pass the parameters that
phylink passes you to this function, so we don't end up at a later
date with people needing to extend this function to do other stuff,
thus repeating mistakes from earlier.

This is what has led to some very yucky code in all those
"fix_mac_speed" implementations, with duplicated data in the BSPs
to get the PHY mode and store it separately, etc.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v3 4/5] net: stmmac: enable Intel mGbE 1G/2.5G auto-negotiation support

[Serge Semin]

On Thu, Sep 21, 2023 at 08:19:45PM +0800, Choong Yong Liang wrote:
> From: "Tan, Tee Min" <tee.min.tan@linux.intel.com>
> 
> Initially, Intel mGbE was only able to configure the overclocking of 2.5
> times clock rate to enable 2.5Gbps in the BIOS during boot time. Kernel
> driver had no access to modify the clock rate for 1G/2.5G mode at runtime.
> 
> Now, this patch enables the runtime 1G/2.5G auto-negotiation support to
> gets rid of the dependency on BIOS to change the 1G/2.5G clock rate.
> 
> This patch adds several new functions below:-
> - intel_tsn_interface_is_available(): This new function reads FIA lane
>   ownership registers and common lane registers through IPC commands
>   to know which lane the mGbE port is assigned to.
> - stmmac_mac_prepare(): To obtain the latest PHY interface from phylink
>   during initialization and call intel_config_serdes() to proceed with
>   SERDES configuration.
> - intel_config_serdes(): To configure the SERDES based on the assigned
>   lane and latest PHY interface, it sends IPC command to the PMC through
>   PMC driver/API. The PMC acts as a proxy for R/W on behalf of the driver.
> - intel_set_reg_access(): Set the register access to the available TSN
>   interface.
> 
> 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/ethernet/stmicro/stmmac/Kconfig   |   1 +
>  .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 134 +++++++++++++++++-
>  .../net/ethernet/stmicro/stmmac/dwmac-intel.h |  79 +++++++++++
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c |  20 +++
>  include/linux/stmmac.h                        |   1 +
>  5 files changed, 231 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> index a2b9e289aa36..4340efd9bd50 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> @@ -273,6 +273,7 @@ config DWMAC_INTEL
>  	default X86
>  	depends on X86 && STMMAC_ETH && PCI
>  	depends on COMMON_CLK
> +	select INTEL_PMC_IPC
>  	help
>  	  This selects the Intel platform specific bus support for the
>  	  stmmac driver. This driver is used for Intel Quark/EHL/TGL.
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> index a3a249c63598..a211f42914a2 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
> @@ -5,6 +5,7 @@
>  #include <linux/clk-provider.h>
>  #include <linux/pci.h>
>  #include <linux/dmi.h>
> +#include <linux/platform_data/x86/intel_pmc_ipc.h>
>  #include "dwmac-intel.h"
>  #include "dwmac4.h"
>  #include "stmmac.h"
> @@ -14,6 +15,9 @@ struct intel_priv_data {
>  	int mdio_adhoc_addr;	/* mdio address for serdes & etc */
>  	unsigned long crossts_adj;
>  	bool is_pse;
> +	const int *tsn_lane_registers;
> +	int max_tsn_lane_registers;
> +	int pid_modphy;
>  };
>  
>  /* This struct is used to associate PCI Function of MAC controller on a board,
> @@ -93,7 +97,7 @@ static int intel_serdes_powerup(struct net_device *ndev, void *priv_data)
>  	data &= ~SERDES_RATE_MASK;
>  	data &= ~SERDES_PCLK_MASK;
>  
> -	if (priv->plat->max_speed == 2500)
> +	if (priv->plat->phy_interface == PHY_INTERFACE_MODE_2500BASEX)
>  		data |= SERDES_RATE_PCIE_GEN2 << SERDES_RATE_PCIE_SHIFT |
>  			SERDES_PCLK_37p5MHZ << SERDES_PCLK_SHIFT;
>  	else
> @@ -414,6 +418,106 @@ static void intel_mgbe_pse_crossts_adj(struct intel_priv_data *intel_priv,
>  	}
>  }
>  
> +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
> +static bool intel_tsn_interface_is_available(struct net_device *ndev,
> +					     struct intel_priv_data *intel_priv)
> +{
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	struct pmc_ipc_cmd tmp = {0};
> +	u32 rbuf[4] = {0};
> +	int ret, i, j;
> +
> +	if (priv->plat->serdes_powerup) {
> +		tmp.cmd = IPC_SOC_REGISTER_ACCESS;
> +		tmp.sub_cmd = IPC_SOC_SUB_CMD_READ;
> +
> +		for (i = 0; i < 5; i++) {
> +			tmp.wbuf[0] = R_PCH_FIA_15_PCR_LOS1_REG_BASE + i;
> +
> +			ret = intel_pmc_ipc(&tmp, rbuf);
> +			if (ret < 0) {
> +				netdev_info(priv->dev,
> +					    "Failed to read from PMC.\n");
> +				return false;
> +			}
> +
> +			for (j = 0; j <= intel_priv->max_tsn_lane_registers; j++)
> +				if ((rbuf[0] >>
> +				    (4 * (intel_priv->tsn_lane_registers[j] % 8)) &
> +				     B_PCH_FIA_PCR_L0O) == 0xB)
> +					return true;
> +		}
> +	}
> +	return false;
> +}
> +
> +static int intel_set_reg_access(const struct pmc_serdes_regs *regs, int max_regs)
> +{
> +	int ret = 0, i;
> +
> +	for (i = 0; i < max_regs; i++) {
> +		struct pmc_ipc_cmd tmp = {0};
> +		u32 buf[4] = {0};
> +
> +		tmp.cmd = IPC_SOC_REGISTER_ACCESS;
> +		tmp.sub_cmd = IPC_SOC_SUB_CMD_WRITE;
> +		tmp.wbuf[0] = (u32)regs[i].index;
> +		tmp.wbuf[1] = regs[i].val;
> +
> +		ret = intel_pmc_ipc(&tmp, buf);
> +		if (ret < 0)
> +			return ret;
> +	}
> +
> +	return ret;
> +}
> +
> +static int intel_config_serdes(struct net_device *ndev, void *intel_data)
> +{
> +	struct intel_priv_data *intel_priv = intel_data;
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	int ret = 0;
> +
> +	if (!intel_tsn_interface_is_available(ndev, intel_priv)) {
> +		netdev_info(priv->dev,
> +			    "No TSN interface available to set the registers.\n");
> +		goto pmc_read_error;
> +	}
> +
> +	if (intel_priv->pid_modphy == PID_MODPHY1) {
> +		if (priv->plat->phy_interface == PHY_INTERFACE_MODE_2500BASEX) {
> +			ret = intel_set_reg_access(pid_modphy1_2p5g_regs,
> +						   ARRAY_SIZE(pid_modphy1_2p5g_regs));
> +		} else {
> +			ret = intel_set_reg_access(pid_modphy1_1g_regs,
> +						   ARRAY_SIZE(pid_modphy1_1g_regs));
> +		}
> +	} else {
> +		if (priv->plat->phy_interface == PHY_INTERFACE_MODE_2500BASEX) {
> +			ret = intel_set_reg_access(pid_modphy3_2p5g_regs,
> +						   ARRAY_SIZE(pid_modphy3_2p5g_regs));
> +		} else {
> +			ret = intel_set_reg_access(pid_modphy3_1g_regs,
> +						   ARRAY_SIZE(pid_modphy3_1g_regs));
> +		}
> +	}
> +
> +	if (ret < 0)
> +		goto pmc_read_error;
> +
> +pmc_read_error:
> +	intel_serdes_powerdown(ndev, intel_priv);
> +	intel_serdes_powerup(ndev, intel_priv);
> +
> +	return ret;
> +}
> +#else
> +static int intel_config_serdes(struct net_device *ndev, void *intel_data)
> +{
> +	return -EOPNOTSUPP;
> +}
> +#endif /* CONFIG_INTEL_PMC_IPC */
> +
>  static void common_default_data(struct plat_stmmacenet_data *plat)
>  {
>  	plat->clk_csr = 2;	/* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
> @@ -624,6 +728,8 @@ static int intel_mgbe_common_data(struct pci_dev *pdev,
>  static int ehl_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 = 8;
>  	plat->tx_queues_to_use = 8;
>  	plat->flags |= STMMAC_FLAG_USE_PHY_WOL;
> @@ -639,20 +745,28 @@ static int ehl_common_data(struct pci_dev *pdev,
>  	plat->safety_feat_cfg->prtyen = 0;
>  	plat->safety_feat_cfg->tmouten = 0;
>  
> +	intel_priv->tsn_lane_registers = ehl_tsn_lane_registers;
> +	intel_priv->max_tsn_lane_registers = ARRAY_SIZE(ehl_tsn_lane_registers);
> +
>  	return intel_mgbe_common_data(pdev, plat);
>  }
>  
>  static int ehl_sgmii_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->speed_mode_2500 = intel_speed_mode_2500;
> +	plat->max_speed = SPEED_2500;
>  	plat->serdes_powerup = intel_serdes_powerup;
>  	plat->serdes_powerdown = intel_serdes_powerdown;
> +	plat->config_serdes = intel_config_serdes;
>  
>  	plat->clk_ptp_rate = 204800000;
>  
> +	intel_priv->pid_modphy = PID_MODPHY3;
> +
>  	return ehl_common_data(pdev, plat);
>  }
>  
> @@ -705,10 +819,16 @@ static struct stmmac_pci_info ehl_pse0_rgmii1g_info = {
>  static int ehl_pse0_sgmii1g_data(struct pci_dev *pdev,
>  				 struct plat_stmmacenet_data *plat)
>  {
> +	struct intel_priv_data *intel_priv = plat->bsp_priv;
> +
>  	plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
> -	plat->speed_mode_2500 = intel_speed_mode_2500;
> +	plat->max_speed = SPEED_2500;
>  	plat->serdes_powerup = intel_serdes_powerup;
>  	plat->serdes_powerdown = intel_serdes_powerdown;
> +	plat->config_serdes = intel_config_serdes;
> +
> +	intel_priv->pid_modphy = PID_MODPHY1;
> +
>  	return ehl_pse0_common_data(pdev, plat);
>  }
>  
> @@ -746,10 +866,16 @@ static struct stmmac_pci_info ehl_pse1_rgmii1g_info = {
>  static int ehl_pse1_sgmii1g_data(struct pci_dev *pdev,
>  				 struct plat_stmmacenet_data *plat)
>  {
> +	struct intel_priv_data *intel_priv = plat->bsp_priv;
> +
>  	plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
> -	plat->speed_mode_2500 = intel_speed_mode_2500;
> +	plat->max_speed = SPEED_2500;
>  	plat->serdes_powerup = intel_serdes_powerup;
>  	plat->serdes_powerdown = intel_serdes_powerdown;
> +	plat->config_serdes = intel_config_serdes;
> +
> +	intel_priv->pid_modphy = PID_MODPHY1;
> +
>  	return ehl_pse1_common_data(pdev, plat);
>  }
>  
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
> index 0a37987478c1..093eed977ab0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
> @@ -50,4 +50,83 @@
>  #define PCH_PTP_CLK_FREQ_19_2MHZ	(GMAC_GPO0)
>  #define PCH_PTP_CLK_FREQ_200MHZ		(0)
>  
> +#define	PID_MODPHY1 0xAA
> +#define	PID_MODPHY3 0xA8
> +
> +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
> +struct pmc_serdes_regs {
> +	u8 index;
> +	u32 val;
> +};
> +
> +/* Modphy Register index */
> +#define R_PCH_FIA_15_PCR_LOS1_REG_BASE			8
> +#define R_PCH_FIA_15_PCR_LOS2_REG_BASE			9
> +#define R_PCH_FIA_15_PCR_LOS3_REG_BASE			10
> +#define R_PCH_FIA_15_PCR_LOS4_REG_BASE			11
> +#define R_PCH_FIA_15_PCR_LOS5_REG_BASE			12
> +#define B_PCH_FIA_PCR_L0O				GENMASK(3, 0)
> +#define PID_MODPHY1_B_MODPHY_PCR_LCPLL_DWORD0		13
> +#define PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD2		14
> +#define PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD7		15
> +#define PID_MODPHY1_N_MODPHY_PCR_LPPLL_DWORD10		16
> +#define PID_MODPHY1_N_MODPHY_PCR_CMN_ANA_DWORD30	17
> +#define PID_MODPHY3_B_MODPHY_PCR_LCPLL_DWORD0		18
> +#define PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD2		19
> +#define PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD7		20
> +#define PID_MODPHY3_N_MODPHY_PCR_LPPLL_DWORD10		21
> +#define PID_MODPHY3_N_MODPHY_PCR_CMN_ANA_DWORD30	22
> +
> +#define B_MODPHY_PCR_LCPLL_DWORD0_1G		0x46AAAA41
> +#define N_MODPHY_PCR_LCPLL_DWORD2_1G		0x00000139
> +#define N_MODPHY_PCR_LCPLL_DWORD7_1G		0x002A0003
> +#define N_MODPHY_PCR_LPPLL_DWORD10_1G		0x00170008
> +#define N_MODPHY_PCR_CMN_ANA_DWORD30_1G		0x0000D4AC
> +#define B_MODPHY_PCR_LCPLL_DWORD0_2P5G		0x58555551
> +#define N_MODPHY_PCR_LCPLL_DWORD2_2P5G		0x0000012D
> +#define N_MODPHY_PCR_LCPLL_DWORD7_2P5G		0x001F0003
> +#define N_MODPHY_PCR_LPPLL_DWORD10_2P5G		0x00170008
> +#define N_MODPHY_PCR_CMN_ANA_DWORD30_2P5G	0x8200ACAC
> +
> +static const struct pmc_serdes_regs pid_modphy3_1g_regs[] = {
> +	{ PID_MODPHY3_B_MODPHY_PCR_LCPLL_DWORD0,	B_MODPHY_PCR_LCPLL_DWORD0_1G },
> +	{ PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD2,	N_MODPHY_PCR_LCPLL_DWORD2_1G },
> +	{ PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD7,	N_MODPHY_PCR_LCPLL_DWORD7_1G },
> +	{ PID_MODPHY3_N_MODPHY_PCR_LPPLL_DWORD10,	N_MODPHY_PCR_LPPLL_DWORD10_1G },
> +	{ PID_MODPHY3_N_MODPHY_PCR_CMN_ANA_DWORD30,	N_MODPHY_PCR_CMN_ANA_DWORD30_1G },
> +	{}
> +};
> +
> +static const struct pmc_serdes_regs pid_modphy3_2p5g_regs[] = {
> +	{ PID_MODPHY3_B_MODPHY_PCR_LCPLL_DWORD0,	B_MODPHY_PCR_LCPLL_DWORD0_2P5G },
> +	{ PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD2,	N_MODPHY_PCR_LCPLL_DWORD2_2P5G },
> +	{ PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD7,	N_MODPHY_PCR_LCPLL_DWORD7_2P5G },
> +	{ PID_MODPHY3_N_MODPHY_PCR_LPPLL_DWORD10,	N_MODPHY_PCR_LPPLL_DWORD10_2P5G },
> +	{ PID_MODPHY3_N_MODPHY_PCR_CMN_ANA_DWORD30,	N_MODPHY_PCR_CMN_ANA_DWORD30_2P5G },
> +	{}
> +};
> +
> +static const struct pmc_serdes_regs pid_modphy1_1g_regs[] = {
> +	{ PID_MODPHY1_B_MODPHY_PCR_LCPLL_DWORD0,	B_MODPHY_PCR_LCPLL_DWORD0_1G },
> +	{ PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD2,	N_MODPHY_PCR_LCPLL_DWORD2_1G },
> +	{ PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD7,	N_MODPHY_PCR_LCPLL_DWORD7_1G },
> +	{ PID_MODPHY1_N_MODPHY_PCR_LPPLL_DWORD10,	N_MODPHY_PCR_LPPLL_DWORD10_1G },
> +	{ PID_MODPHY1_N_MODPHY_PCR_CMN_ANA_DWORD30,	N_MODPHY_PCR_CMN_ANA_DWORD30_1G },
> +	{}
> +};
> +
> +static const struct pmc_serdes_regs pid_modphy1_2p5g_regs[] = {
> +	{ PID_MODPHY1_B_MODPHY_PCR_LCPLL_DWORD0,	B_MODPHY_PCR_LCPLL_DWORD0_2P5G },
> +	{ PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD2,	N_MODPHY_PCR_LCPLL_DWORD2_2P5G },
> +	{ PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD7,	N_MODPHY_PCR_LCPLL_DWORD7_2P5G },
> +	{ PID_MODPHY1_N_MODPHY_PCR_LPPLL_DWORD10,	N_MODPHY_PCR_LPPLL_DWORD10_2P5G },
> +	{ PID_MODPHY1_N_MODPHY_PCR_CMN_ANA_DWORD30,	N_MODPHY_PCR_CMN_ANA_DWORD30_2P5G },
> +	{}
> +};
> +
> +static const int ehl_tsn_lane_registers[] = {7, 8, 9, 10, 11};
> +#else
> +static const int ehl_tsn_lane_registers[] = {};
> +#endif /* CONFIG_INTEL_PMC_IPC */
> +
>  #endif /* __DWMAC_INTEL_H__ */
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 9201ed778ebc..75765cf52cd1 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -1103,11 +1103,31 @@ static void stmmac_mac_link_up(struct phylink_config *config,
>  		stmmac_hwtstamp_correct_latency(priv, priv);
>  }
>  

> +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
> +static int stmmac_mac_prepare(struct phylink_config *config, unsigned int mode,
> +			      phy_interface_t interface)
> +{
> +	struct net_device *ndev = to_net_dev(config->dev);
> +	struct stmmac_priv *priv = netdev_priv(ndev);
> +	int ret = 0;
> +
> +	priv->plat->phy_interface = interface;
> +
> +	if (priv->plat->config_serdes)
> +		ret = priv->plat->config_serdes(ndev, priv->plat->bsp_priv);
> +
> +	return ret;
> +}
> +#endif
> +
>  static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
>  	.mac_select_pcs = stmmac_mac_select_pcs,
>  	.mac_config = stmmac_mac_config,
>  	.mac_link_down = stmmac_mac_link_down,
>  	.mac_link_up = stmmac_mac_link_up,
> +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
> +	.mac_prepare = stmmac_mac_prepare,
> +#endif

Please no for the platform-specific ifdef's in the generic code.
STMMAC driver is already overwhelmed with clumsy solutions. Let's not
add another one.

-Serge(y)

>  };
>  
>  /**
> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> index c0079a7574ae..aa7d4d96391c 100644
> --- a/include/linux/stmmac.h
> +++ b/include/linux/stmmac.h
> @@ -275,6 +275,7 @@ struct plat_stmmacenet_data {
>  	int (*serdes_powerup)(struct net_device *ndev, void *priv);
>  	void (*serdes_powerdown)(struct net_device *ndev, void *priv);
>  	void (*speed_mode_2500)(struct net_device *ndev, void *priv);
> +	int (*config_serdes)(struct net_device *ndev, void *priv);
>  	void (*ptp_clk_freq_config)(struct stmmac_priv *priv);
>  	int (*init)(struct platform_device *pdev, void *priv);
>  	void (*exit)(struct platform_device *pdev, void *priv);
> -- 
> 2.25.1
> 
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v3 4/5] net: stmmac: enable Intel mGbE 1G/2.5G auto-negotiation support

[Choong Yong Liang]

On 26/9/2023 6:55 pm, Serge Semin wrote:
> On Thu, Sep 21, 2023 at 08:19:45PM +0800, Choong Yong Liang wrote:
>> From: "Tan, Tee Min" <tee.min.tan@linux.intel.com>
>>
>> Initially, Intel mGbE was only able to configure the overclocking of 2.5
>> times clock rate to enable 2.5Gbps in the BIOS during boot time. Kernel
>> driver had no access to modify the clock rate for 1G/2.5G mode at runtime.
>>
>> Now, this patch enables the runtime 1G/2.5G auto-negotiation support to
>> gets rid of the dependency on BIOS to change the 1G/2.5G clock rate.
>>
>> This patch adds several new functions below:-
>> - intel_tsn_interface_is_available(): This new function reads FIA lane
>>    ownership registers and common lane registers through IPC commands
>>    to know which lane the mGbE port is assigned to.
>> - stmmac_mac_prepare(): To obtain the latest PHY interface from phylink
>>    during initialization and call intel_config_serdes() to proceed with
>>    SERDES configuration.
>> - intel_config_serdes(): To configure the SERDES based on the assigned
>>    lane and latest PHY interface, it sends IPC command to the PMC through
>>    PMC driver/API. The PMC acts as a proxy for R/W on behalf of the driver.
>> - intel_set_reg_access(): Set the register access to the available TSN
>>    interface.
>>
>> 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/ethernet/stmicro/stmmac/Kconfig   |   1 +
>>   .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 134 +++++++++++++++++-
>>   .../net/ethernet/stmicro/stmmac/dwmac-intel.h |  79 +++++++++++
>>   .../net/ethernet/stmicro/stmmac/stmmac_main.c |  20 +++
>>   include/linux/stmmac.h                        |   1 +
>>   5 files changed, 231 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> index a2b9e289aa36..4340efd9bd50 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
>> @@ -273,6 +273,7 @@ config DWMAC_INTEL
>>   	default X86
>>   	depends on X86 && STMMAC_ETH && PCI
>>   	depends on COMMON_CLK
>> +	select INTEL_PMC_IPC
>>   	help
>>   	  This selects the Intel platform specific bus support for the
>>   	  stmmac driver. This driver is used for Intel Quark/EHL/TGL.
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
>> index a3a249c63598..a211f42914a2 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
>> @@ -5,6 +5,7 @@
>>   #include <linux/clk-provider.h>
>>   #include <linux/pci.h>
>>   #include <linux/dmi.h>
>> +#include <linux/platform_data/x86/intel_pmc_ipc.h>
>>   #include "dwmac-intel.h"
>>   #include "dwmac4.h"
>>   #include "stmmac.h"
>> @@ -14,6 +15,9 @@ struct intel_priv_data {
>>   	int mdio_adhoc_addr;	/* mdio address for serdes & etc */
>>   	unsigned long crossts_adj;
>>   	bool is_pse;
>> +	const int *tsn_lane_registers;
>> +	int max_tsn_lane_registers;
>> +	int pid_modphy;
>>   };
>>   
>>   /* This struct is used to associate PCI Function of MAC controller on a board,
>> @@ -93,7 +97,7 @@ static int intel_serdes_powerup(struct net_device *ndev, void *priv_data)
>>   	data &= ~SERDES_RATE_MASK;
>>   	data &= ~SERDES_PCLK_MASK;
>>   
>> -	if (priv->plat->max_speed == 2500)
>> +	if (priv->plat->phy_interface == PHY_INTERFACE_MODE_2500BASEX)
>>   		data |= SERDES_RATE_PCIE_GEN2 << SERDES_RATE_PCIE_SHIFT |
>>   			SERDES_PCLK_37p5MHZ << SERDES_PCLK_SHIFT;
>>   	else
>> @@ -414,6 +418,106 @@ static void intel_mgbe_pse_crossts_adj(struct intel_priv_data *intel_priv,
>>   	}
>>   }
>>   
>> +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
>> +static bool intel_tsn_interface_is_available(struct net_device *ndev,
>> +					     struct intel_priv_data *intel_priv)
>> +{
>> +	struct stmmac_priv *priv = netdev_priv(ndev);
>> +	struct pmc_ipc_cmd tmp = {0};
>> +	u32 rbuf[4] = {0};
>> +	int ret, i, j;
>> +
>> +	if (priv->plat->serdes_powerup) {
>> +		tmp.cmd = IPC_SOC_REGISTER_ACCESS;
>> +		tmp.sub_cmd = IPC_SOC_SUB_CMD_READ;
>> +
>> +		for (i = 0; i < 5; i++) {
>> +			tmp.wbuf[0] = R_PCH_FIA_15_PCR_LOS1_REG_BASE + i;
>> +
>> +			ret = intel_pmc_ipc(&tmp, rbuf);
>> +			if (ret < 0) {
>> +				netdev_info(priv->dev,
>> +					    "Failed to read from PMC.\n");
>> +				return false;
>> +			}
>> +
>> +			for (j = 0; j <= intel_priv->max_tsn_lane_registers; j++)
>> +				if ((rbuf[0] >>
>> +				    (4 * (intel_priv->tsn_lane_registers[j] % 8)) &
>> +				     B_PCH_FIA_PCR_L0O) == 0xB)
>> +					return true;
>> +		}
>> +	}
>> +	return false;
>> +}
>> +
>> +static int intel_set_reg_access(const struct pmc_serdes_regs *regs, int max_regs)
>> +{
>> +	int ret = 0, i;
>> +
>> +	for (i = 0; i < max_regs; i++) {
>> +		struct pmc_ipc_cmd tmp = {0};
>> +		u32 buf[4] = {0};
>> +
>> +		tmp.cmd = IPC_SOC_REGISTER_ACCESS;
>> +		tmp.sub_cmd = IPC_SOC_SUB_CMD_WRITE;
>> +		tmp.wbuf[0] = (u32)regs[i].index;
>> +		tmp.wbuf[1] = regs[i].val;
>> +
>> +		ret = intel_pmc_ipc(&tmp, buf);
>> +		if (ret < 0)
>> +			return ret;
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +static int intel_config_serdes(struct net_device *ndev, void *intel_data)
>> +{
>> +	struct intel_priv_data *intel_priv = intel_data;
>> +	struct stmmac_priv *priv = netdev_priv(ndev);
>> +	int ret = 0;
>> +
>> +	if (!intel_tsn_interface_is_available(ndev, intel_priv)) {
>> +		netdev_info(priv->dev,
>> +			    "No TSN interface available to set the registers.\n");
>> +		goto pmc_read_error;
>> +	}
>> +
>> +	if (intel_priv->pid_modphy == PID_MODPHY1) {
>> +		if (priv->plat->phy_interface == PHY_INTERFACE_MODE_2500BASEX) {
>> +			ret = intel_set_reg_access(pid_modphy1_2p5g_regs,
>> +						   ARRAY_SIZE(pid_modphy1_2p5g_regs));
>> +		} else {
>> +			ret = intel_set_reg_access(pid_modphy1_1g_regs,
>> +						   ARRAY_SIZE(pid_modphy1_1g_regs));
>> +		}
>> +	} else {
>> +		if (priv->plat->phy_interface == PHY_INTERFACE_MODE_2500BASEX) {
>> +			ret = intel_set_reg_access(pid_modphy3_2p5g_regs,
>> +						   ARRAY_SIZE(pid_modphy3_2p5g_regs));
>> +		} else {
>> +			ret = intel_set_reg_access(pid_modphy3_1g_regs,
>> +						   ARRAY_SIZE(pid_modphy3_1g_regs));
>> +		}
>> +	}
>> +
>> +	if (ret < 0)
>> +		goto pmc_read_error;
>> +
>> +pmc_read_error:
>> +	intel_serdes_powerdown(ndev, intel_priv);
>> +	intel_serdes_powerup(ndev, intel_priv);
>> +
>> +	return ret;
>> +}
>> +#else
>> +static int intel_config_serdes(struct net_device *ndev, void *intel_data)
>> +{
>> +	return -EOPNOTSUPP;
>> +}
>> +#endif /* CONFIG_INTEL_PMC_IPC */
>> +
>>   static void common_default_data(struct plat_stmmacenet_data *plat)
>>   {
>>   	plat->clk_csr = 2;	/* clk_csr_i = 20-35MHz & MDC = clk_csr_i/16 */
>> @@ -624,6 +728,8 @@ static int intel_mgbe_common_data(struct pci_dev *pdev,
>>   static int ehl_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 = 8;
>>   	plat->tx_queues_to_use = 8;
>>   	plat->flags |= STMMAC_FLAG_USE_PHY_WOL;
>> @@ -639,20 +745,28 @@ static int ehl_common_data(struct pci_dev *pdev,
>>   	plat->safety_feat_cfg->prtyen = 0;
>>   	plat->safety_feat_cfg->tmouten = 0;
>>   
>> +	intel_priv->tsn_lane_registers = ehl_tsn_lane_registers;
>> +	intel_priv->max_tsn_lane_registers = ARRAY_SIZE(ehl_tsn_lane_registers);
>> +
>>   	return intel_mgbe_common_data(pdev, plat);
>>   }
>>   
>>   static int ehl_sgmii_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->speed_mode_2500 = intel_speed_mode_2500;
>> +	plat->max_speed = SPEED_2500;
>>   	plat->serdes_powerup = intel_serdes_powerup;
>>   	plat->serdes_powerdown = intel_serdes_powerdown;
>> +	plat->config_serdes = intel_config_serdes;
>>   
>>   	plat->clk_ptp_rate = 204800000;
>>   
>> +	intel_priv->pid_modphy = PID_MODPHY3;
>> +
>>   	return ehl_common_data(pdev, plat);
>>   }
>>   
>> @@ -705,10 +819,16 @@ static struct stmmac_pci_info ehl_pse0_rgmii1g_info = {
>>   static int ehl_pse0_sgmii1g_data(struct pci_dev *pdev,
>>   				 struct plat_stmmacenet_data *plat)
>>   {
>> +	struct intel_priv_data *intel_priv = plat->bsp_priv;
>> +
>>   	plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
>> -	plat->speed_mode_2500 = intel_speed_mode_2500;
>> +	plat->max_speed = SPEED_2500;
>>   	plat->serdes_powerup = intel_serdes_powerup;
>>   	plat->serdes_powerdown = intel_serdes_powerdown;
>> +	plat->config_serdes = intel_config_serdes;
>> +
>> +	intel_priv->pid_modphy = PID_MODPHY1;
>> +
>>   	return ehl_pse0_common_data(pdev, plat);
>>   }
>>   
>> @@ -746,10 +866,16 @@ static struct stmmac_pci_info ehl_pse1_rgmii1g_info = {
>>   static int ehl_pse1_sgmii1g_data(struct pci_dev *pdev,
>>   				 struct plat_stmmacenet_data *plat)
>>   {
>> +	struct intel_priv_data *intel_priv = plat->bsp_priv;
>> +
>>   	plat->phy_interface = PHY_INTERFACE_MODE_SGMII;
>> -	plat->speed_mode_2500 = intel_speed_mode_2500;
>> +	plat->max_speed = SPEED_2500;
>>   	plat->serdes_powerup = intel_serdes_powerup;
>>   	plat->serdes_powerdown = intel_serdes_powerdown;
>> +	plat->config_serdes = intel_config_serdes;
>> +
>> +	intel_priv->pid_modphy = PID_MODPHY1;
>> +
>>   	return ehl_pse1_common_data(pdev, plat);
>>   }
>>   
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
>> index 0a37987478c1..093eed977ab0 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
>> @@ -50,4 +50,83 @@
>>   #define PCH_PTP_CLK_FREQ_19_2MHZ	(GMAC_GPO0)
>>   #define PCH_PTP_CLK_FREQ_200MHZ		(0)
>>   
>> +#define	PID_MODPHY1 0xAA
>> +#define	PID_MODPHY3 0xA8
>> +
>> +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
>> +struct pmc_serdes_regs {
>> +	u8 index;
>> +	u32 val;
>> +};
>> +
>> +/* Modphy Register index */
>> +#define R_PCH_FIA_15_PCR_LOS1_REG_BASE			8
>> +#define R_PCH_FIA_15_PCR_LOS2_REG_BASE			9
>> +#define R_PCH_FIA_15_PCR_LOS3_REG_BASE			10
>> +#define R_PCH_FIA_15_PCR_LOS4_REG_BASE			11
>> +#define R_PCH_FIA_15_PCR_LOS5_REG_BASE			12
>> +#define B_PCH_FIA_PCR_L0O				GENMASK(3, 0)
>> +#define PID_MODPHY1_B_MODPHY_PCR_LCPLL_DWORD0		13
>> +#define PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD2		14
>> +#define PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD7		15
>> +#define PID_MODPHY1_N_MODPHY_PCR_LPPLL_DWORD10		16
>> +#define PID_MODPHY1_N_MODPHY_PCR_CMN_ANA_DWORD30	17
>> +#define PID_MODPHY3_B_MODPHY_PCR_LCPLL_DWORD0		18
>> +#define PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD2		19
>> +#define PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD7		20
>> +#define PID_MODPHY3_N_MODPHY_PCR_LPPLL_DWORD10		21
>> +#define PID_MODPHY3_N_MODPHY_PCR_CMN_ANA_DWORD30	22
>> +
>> +#define B_MODPHY_PCR_LCPLL_DWORD0_1G		0x46AAAA41
>> +#define N_MODPHY_PCR_LCPLL_DWORD2_1G		0x00000139
>> +#define N_MODPHY_PCR_LCPLL_DWORD7_1G		0x002A0003
>> +#define N_MODPHY_PCR_LPPLL_DWORD10_1G		0x00170008
>> +#define N_MODPHY_PCR_CMN_ANA_DWORD30_1G		0x0000D4AC
>> +#define B_MODPHY_PCR_LCPLL_DWORD0_2P5G		0x58555551
>> +#define N_MODPHY_PCR_LCPLL_DWORD2_2P5G		0x0000012D
>> +#define N_MODPHY_PCR_LCPLL_DWORD7_2P5G		0x001F0003
>> +#define N_MODPHY_PCR_LPPLL_DWORD10_2P5G		0x00170008
>> +#define N_MODPHY_PCR_CMN_ANA_DWORD30_2P5G	0x8200ACAC
>> +
>> +static const struct pmc_serdes_regs pid_modphy3_1g_regs[] = {
>> +	{ PID_MODPHY3_B_MODPHY_PCR_LCPLL_DWORD0,	B_MODPHY_PCR_LCPLL_DWORD0_1G },
>> +	{ PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD2,	N_MODPHY_PCR_LCPLL_DWORD2_1G },
>> +	{ PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD7,	N_MODPHY_PCR_LCPLL_DWORD7_1G },
>> +	{ PID_MODPHY3_N_MODPHY_PCR_LPPLL_DWORD10,	N_MODPHY_PCR_LPPLL_DWORD10_1G },
>> +	{ PID_MODPHY3_N_MODPHY_PCR_CMN_ANA_DWORD30,	N_MODPHY_PCR_CMN_ANA_DWORD30_1G },
>> +	{}
>> +};
>> +
>> +static const struct pmc_serdes_regs pid_modphy3_2p5g_regs[] = {
>> +	{ PID_MODPHY3_B_MODPHY_PCR_LCPLL_DWORD0,	B_MODPHY_PCR_LCPLL_DWORD0_2P5G },
>> +	{ PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD2,	N_MODPHY_PCR_LCPLL_DWORD2_2P5G },
>> +	{ PID_MODPHY3_N_MODPHY_PCR_LCPLL_DWORD7,	N_MODPHY_PCR_LCPLL_DWORD7_2P5G },
>> +	{ PID_MODPHY3_N_MODPHY_PCR_LPPLL_DWORD10,	N_MODPHY_PCR_LPPLL_DWORD10_2P5G },
>> +	{ PID_MODPHY3_N_MODPHY_PCR_CMN_ANA_DWORD30,	N_MODPHY_PCR_CMN_ANA_DWORD30_2P5G },
>> +	{}
>> +};
>> +
>> +static const struct pmc_serdes_regs pid_modphy1_1g_regs[] = {
>> +	{ PID_MODPHY1_B_MODPHY_PCR_LCPLL_DWORD0,	B_MODPHY_PCR_LCPLL_DWORD0_1G },
>> +	{ PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD2,	N_MODPHY_PCR_LCPLL_DWORD2_1G },
>> +	{ PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD7,	N_MODPHY_PCR_LCPLL_DWORD7_1G },
>> +	{ PID_MODPHY1_N_MODPHY_PCR_LPPLL_DWORD10,	N_MODPHY_PCR_LPPLL_DWORD10_1G },
>> +	{ PID_MODPHY1_N_MODPHY_PCR_CMN_ANA_DWORD30,	N_MODPHY_PCR_CMN_ANA_DWORD30_1G },
>> +	{}
>> +};
>> +
>> +static const struct pmc_serdes_regs pid_modphy1_2p5g_regs[] = {
>> +	{ PID_MODPHY1_B_MODPHY_PCR_LCPLL_DWORD0,	B_MODPHY_PCR_LCPLL_DWORD0_2P5G },
>> +	{ PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD2,	N_MODPHY_PCR_LCPLL_DWORD2_2P5G },
>> +	{ PID_MODPHY1_N_MODPHY_PCR_LCPLL_DWORD7,	N_MODPHY_PCR_LCPLL_DWORD7_2P5G },
>> +	{ PID_MODPHY1_N_MODPHY_PCR_LPPLL_DWORD10,	N_MODPHY_PCR_LPPLL_DWORD10_2P5G },
>> +	{ PID_MODPHY1_N_MODPHY_PCR_CMN_ANA_DWORD30,	N_MODPHY_PCR_CMN_ANA_DWORD30_2P5G },
>> +	{}
>> +};
>> +
>> +static const int ehl_tsn_lane_registers[] = {7, 8, 9, 10, 11};
>> +#else
>> +static const int ehl_tsn_lane_registers[] = {};
>> +#endif /* CONFIG_INTEL_PMC_IPC */
>> +
>>   #endif /* __DWMAC_INTEL_H__ */
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> index 9201ed778ebc..75765cf52cd1 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
>> @@ -1103,11 +1103,31 @@ static void stmmac_mac_link_up(struct phylink_config *config,
>>   		stmmac_hwtstamp_correct_latency(priv, priv);
>>   }
>>   
> 
>> +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
>> +static int stmmac_mac_prepare(struct phylink_config *config, unsigned int mode,
>> +			      phy_interface_t interface)
>> +{
>> +	struct net_device *ndev = to_net_dev(config->dev);
>> +	struct stmmac_priv *priv = netdev_priv(ndev);
>> +	int ret = 0;
>> +
>> +	priv->plat->phy_interface = interface;
>> +
>> +	if (priv->plat->config_serdes)
>> +		ret = priv->plat->config_serdes(ndev, priv->plat->bsp_priv);
>> +
>> +	return ret;
>> +}
>> +#endif
>> +
>>   static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
>>   	.mac_select_pcs = stmmac_mac_select_pcs,
>>   	.mac_config = stmmac_mac_config,
>>   	.mac_link_down = stmmac_mac_link_down,
>>   	.mac_link_up = stmmac_mac_link_up,
>> +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
>> +	.mac_prepare = stmmac_mac_prepare,
>> +#endif
> 
> Please no for the platform-specific ifdef's in the generic code.
> STMMAC driver is already overwhelmed with clumsy solutions. Let's not
> add another one.
> 
> -Serge(y)
> 
>>   };
>>   
>>   /**
>> diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
>> index c0079a7574ae..aa7d4d96391c 100644
>> --- a/include/linux/stmmac.h
>> +++ b/include/linux/stmmac.h
>> @@ -275,6 +275,7 @@ struct plat_stmmacenet_data {
>>   	int (*serdes_powerup)(struct net_device *ndev, void *priv);
>>   	void (*serdes_powerdown)(struct net_device *ndev, void *priv);
>>   	void (*speed_mode_2500)(struct net_device *ndev, void *priv);
>> +	int (*config_serdes)(struct net_device *ndev, void *priv);
>>   	void (*ptp_clk_freq_config)(struct stmmac_priv *priv);
>>   	int (*init)(struct platform_device *pdev, void *priv);
>>   	void (*exit)(struct platform_device *pdev, void *priv);
>> -- 
>> 2.25.1
>>
>>
Hi Russell and Serge,

Thank you for pointing that out.
The ifdef was removed and the config serdes will be implemented in 
mac_link_up in the new patch series.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v3 4/5] net: stmmac: enable Intel mGbE 1G/2.5G auto-negotiation support

[Andrew Lunn]

On Mon, Jan 29, 2024 at 09:19:58PM +0800, Choong Yong Liang wrote:
> > >   static const struct phylink_mac_ops stmmac_phylink_mac_ops = {
> > >   	.mac_select_pcs = stmmac_mac_select_pcs,
> > >   	.mac_config = stmmac_mac_config,
> > >   	.mac_link_down = stmmac_mac_link_down,
> > >   	.mac_link_up = stmmac_mac_link_up,
> > > +#if IS_ENABLED(CONFIG_INTEL_PMC_IPC)
> > > +	.mac_prepare = stmmac_mac_prepare,
> > > +#endif
> > 
> > Please no for the platform-specific ifdef's in the generic code.
> > STMMAC driver is already overwhelmed with clumsy solutions. Let's not
> > add another one.
> > 
> > -Serge(y)
> > 
> > >   };
> > >   /**
> > > diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
> > > index c0079a7574ae..aa7d4d96391c 100644
> > > --- a/include/linux/stmmac.h
> > > +++ b/include/linux/stmmac.h
> > > @@ -275,6 +275,7 @@ struct plat_stmmacenet_data {
> > >   	int (*serdes_powerup)(struct net_device *ndev, void *priv);
> > >   	void (*serdes_powerdown)(struct net_device *ndev, void *priv);
> > >   	void (*speed_mode_2500)(struct net_device *ndev, void *priv);
> > > +	int (*config_serdes)(struct net_device *ndev, void *priv);
> > >   	void (*ptp_clk_freq_config)(struct stmmac_priv *priv);
> > >   	int (*init)(struct platform_device *pdev, void *priv);
> > >   	void (*exit)(struct platform_device *pdev, void *priv);
> > > -- 
> > > 2.25.1
> > > 
> > > 
> Hi Russell and Serge,
> 
> Thank you for pointing that out.
> The ifdef was removed and the config serdes will be implemented in
> mac_link_up in the new patch series.

Hi Choong

Please trim the text when replying. It can be hard to find actually
replies when having to do lots and lots of page downs. Just give the
context needed to understand your reply.

	Andrew

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v3 4/5] net: stmmac: enable Intel mGbE 1G/2.5G auto-negotiation support

[Choong Yong Liang]

On 29/1/2024 9:41 pm, Andrew Lunn wrote:
> Hi Choong
> 
> Please trim the text when replying. It can be hard to find actually
> replies when having to do lots and lots of page downs. Just give the
> context needed to understand your reply.
> 
> 	Andrew
Hi Andrew,

Thank you for the feedback.
I will trim the message next time.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH net-next v3 5/5] stmmac: intel: Add 1G/2.5G auto-negotiation support for ADL-N

[Choong Yong Liang]

Add modphy register lane to have 1G/2.5G auto-negotiation support for
ADL-N.

Signed-off-by: Michael Sit Wei Hong <michael.wei.hong.sit@intel.com>
Signed-off-by: Choong Yong Liang <yong.liang.choong@linux.intel.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-intel.c | 49 ++++++++++++++++++-
 .../net/ethernet/stmicro/stmmac/dwmac-intel.h |  2 +
 2 files changed, 50 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
index a211f42914a2..bece46faa710 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.c
@@ -961,6 +961,53 @@ 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_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;
+	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 = {
+	.setup = adln_sgmii_phy0_data,
+};
+
 static const struct stmmac_pci_func_data galileo_stmmac_func_data[] = {
 	{
 		.func = 6,
@@ -1343,7 +1390,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) },
 	{}
 };
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h b/drivers/net/ethernet/stmicro/stmmac/dwmac-intel.h
index 093eed977ab0..2c6b50958988 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_IPC */
 
-- 
2.25.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v3 0/5] TSN auto negotiation between 1G and 2.5G

[Andrew Lunn]

> 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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v3 0/5] TSN auto negotiation between 1G and 2.5G

[Russell King (Oracle)]

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.

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH net-next v3 0/5] TSN auto negotiation between 1G and 2.5G

[Choong Yong Liang]

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.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel