linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] dt-bindings: phy: mediatek,xfi-tphy: add new bindings
@ 2024-02-01 21:52 Daniel Golle
  2024-02-01 21:53 ` [PATCH 2/2] phy: add driver for MediaTek XFI T-PHY Daniel Golle
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Daniel Golle @ 2024-02-01 21:52 UTC (permalink / raw)
  To: Bc-bocun Chen, Steven Liu, John Crispin, Chunfeng Yun,
	Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Daniel Golle, Qingfang Deng,
	SkyLake Huang, Matthias Brugger, AngeloGioacchino Del Regno,
	Philipp Zabel, linux-arm-kernel, linux-mediatek, linux-phy,
	devicetree, linux-kernel, netdev

Add bindings for the MediaTek XFI T-PHY Ethernet SerDes PHY found in the
MediaTek MT7988 SoC which can operate at various interfaces modes:

via USXGMII PCS:
 * USXGMII
 * 10GBase-R
 * 5GBase-R

via LynxI SGMII PCS:
 * 2500Base-X
 * 1000Base-X
 * Cisco SGMII (MAC side)

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 .../bindings/phy/mediatek,xfi-tphy.yaml       | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml

diff --git a/Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
new file mode 100644
index 0000000000000..e897118dcf7e6
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/phy/mediatek,xfi-tphy.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: MediaTek XFI T-PHY
+
+maintainers:
+  - Daniel Golle <daniel@makrotopia.org>
+
+description:
+  The MediaTek XFI SerDes T-PHY provides the physical SerDes lanes
+  used by the (10G/5G) USXGMII PCS and (1G/2.5G) LynxI PCS found in
+  MediaTek's 10G-capabale SoCs.
+
+properties:
+  $nodename:
+    pattern: "^phy@[0-9a-f]+$"
+
+  compatible:
+    const: mediatek,mt7988-xfi-tphy
+
+  reg:
+    maxItems: 1
+
+  clocks:
+    items:
+      - description: XFI PHY clock
+      - description: XFI register clock
+
+  clock-names:
+    items:
+      - const: xfipll
+      - const: topxtal
+
+  resets:
+    items:
+      - description: PEXTP reset
+
+  mediatek,usxgmii-performance-errata:
+    $ref: /schemas/types.yaml#/definitions/flag
+    description:
+      One instance of the T-PHY on MT7988 suffers from a performance
+      problem in 10GBase-R mode which needs a work-around in the driver.
+      The work-around is enabled using this flag.
+
+  "#phy-cells":
+    const: 0
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - resets
+  - "#phy-cells"
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mediatek,mt7988-clk.h>
+    soc {
+      #address-cells = <2>;
+      #size-cells = <2>;
+
+      phy@11f20000 {
+        compatible = "mediatek,mt7988-xfi-tphy";
+        reg = <0 0x11f20000 0 0x10000>;
+        clocks = <&xfi_pll CLK_XFIPLL_PLL_EN>,
+                 <&topckgen CLK_TOP_XFI_PHY_0_XTAL_SEL>;
+        clock-names = "xfipll", "topxtal";
+        resets = <&watchdog 14>;
+        mediatek,usxgmii-performance-errata;
+        #phy-cells = <0>;
+      };
+    };
+
+...
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/2] phy: add driver for MediaTek XFI T-PHY
  2024-02-01 21:52 [PATCH 1/2] dt-bindings: phy: mediatek,xfi-tphy: add new bindings Daniel Golle
@ 2024-02-01 21:53 ` Daniel Golle
  2024-02-02 13:21   ` AngeloGioacchino Del Regno
  2024-02-04  6:29   ` Chunfeng Yun (云春峰)
  2024-02-02  8:21 ` [PATCH 1/2] dt-bindings: phy: mediatek,xfi-tphy: add new bindings Krzysztof Kozlowski
  2024-02-02 21:24 ` Rob Herring
  2 siblings, 2 replies; 12+ messages in thread
From: Daniel Golle @ 2024-02-01 21:53 UTC (permalink / raw)
  To: Bc-bocun Chen, Chunfeng Yun, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Daniel Golle,
	Qingfang Deng, SkyLake Huang, Matthias Brugger,
	AngeloGioacchino Del Regno, Philipp Zabel, linux-arm-kernel,
	linux-mediatek, linux-phy, devicetree, linux-kernel, netdev

Add driver for MediaTek's XFI T-PHY, 10 Gigabit/s Ethernet SerDes PHY
which can be found in the MT7988 SoC.

The PHY can operates only in PHY_MODE_ETHERNET, the submode is one of
PHY_INTERFACE_MODE_* corresponding to the supported modes:

 * USXGMII                 \
 * 10GBase-R                }- USXGMII PCS - XGDM  \
 * 5GBase-R                /                        \
                                                     }- Ethernet MAC
 * 2500Base-X              \                        /
 * 1000Base-X               }- LynxI PCS - GDM     /
 * Cisco SGMII (MAC side)  /

In order to work-around a performance issue present on the first of
two XFI T-PHYs present in MT7988, special tuning is applied which can be
selected by adding the 'mediatek,usxgmii-performance-errata' property to
the device tree node.

There is no documentation for most registers used for the
analog/tuning part, however, most of the registers have been partially
reverse-engineered from MediaTek's SDK implementation (an opaque
sequence of 32-bit register writes) and descriptions for all relevant
digital registers and bits such as resets and muxes have been supplied
by MediaTek.

Signed-off-by: Daniel Golle <daniel@makrotopia.org>
---
 MAINTAINERS                             |   1 +
 drivers/phy/mediatek/Kconfig            |  12 +
 drivers/phy/mediatek/Makefile           |   1 +
 drivers/phy/mediatek/phy-mtk-xfi-tphy.c | 392 ++++++++++++++++++++++++
 4 files changed, 406 insertions(+)
 create mode 100644 drivers/phy/mediatek/phy-mtk-xfi-tphy.c

diff --git a/MAINTAINERS b/MAINTAINERS
index 52769631bdb1a..52e4192470bd9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13715,6 +13715,7 @@ L:	netdev@vger.kernel.org
 S:	Maintained
 F:	drivers/net/phy/mediatek-ge-soc.c
 F:	drivers/net/phy/mediatek-ge.c
+F:	drivers/phy/mediatek/phy-mtk-xfi-tphy.c
 
 MEDIATEK I2C CONTROLLER DRIVER
 M:	Qii Wang <qii.wang@mediatek.com>
diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
index 3125ecb5d119f..5161d130c7f8b 100644
--- a/drivers/phy/mediatek/Kconfig
+++ b/drivers/phy/mediatek/Kconfig
@@ -13,6 +13,18 @@ config PHY_MTK_PCIE
 	  callback for PCIe GEN3 port, it supports software efuse
 	  initialization.
 
+config PHY_MTK_XFI_TPHY
+	tristate "MediaTek XFI T-PHY Driver"
+	depends on ARCH_MEDIATEK || COMPILE_TEST
+	depends on OF && OF_ADDRESS
+	depends on HAS_IOMEM
+	select GENERIC_PHY
+	help
+	  Say 'Y' here to add support for MediaTek XFI T-PHY driver.
+	  The driver provides access to the Ethernet SerDes T-PHY supporting
+	  1GE and 2.5GE modes via the LynxI PCS, and 5GE and 10GE modes
+	  via the USXGMII PCS found in MediaTek SoCs with 10G Ethernet.
+
 config PHY_MTK_TPHY
 	tristate "MediaTek T-PHY Driver"
 	depends on ARCH_MEDIATEK || COMPILE_TEST
diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
index c9a50395533eb..fa7217178e7f4 100644
--- a/drivers/phy/mediatek/Makefile
+++ b/drivers/phy/mediatek/Makefile
@@ -8,6 +8,7 @@ obj-$(CONFIG_PHY_MTK_PCIE)		+= phy-mtk-pcie.o
 obj-$(CONFIG_PHY_MTK_TPHY)		+= phy-mtk-tphy.o
 obj-$(CONFIG_PHY_MTK_UFS)		+= phy-mtk-ufs.o
 obj-$(CONFIG_PHY_MTK_XSPHY)		+= phy-mtk-xsphy.o
+obj-$(CONFIG_PHY_MTK_XFI_TPHY)		+= phy-mtk-xfi-tphy.o
 
 phy-mtk-hdmi-drv-y			:= phy-mtk-hdmi.o
 phy-mtk-hdmi-drv-y			+= phy-mtk-hdmi-mt2701.o
diff --git a/drivers/phy/mediatek/phy-mtk-xfi-tphy.c b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
new file mode 100644
index 0000000000000..d50e6320860e5
--- /dev/null
+++ b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
@@ -0,0 +1,392 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/* MediaTek 10GE SerDes PHY driver
+ *
+ * Copyright (c) 2024 Daniel Golle <daniel@makrotopia.org>
+ *                    Bc-bocun Chen <bc-bocun.chen@mediatek.com>
+ * based on mtk_usxgmii.c found in MediaTek's SDK released under GPL-2.0
+ * Copyright (c) 2022 MediaTek Inc.
+ * Author: Henry Yen <henry.yen@mediatek.com>
+ */
+
+#include <linux/module.h>
+#include <linux/device.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/io.h>
+#include <linux/clk.h>
+#include <linux/reset.h>
+#include <linux/phy.h>
+#include <linux/phy/phy.h>
+
+#define MTK_XFI_TPHY_NUM_CLOCKS		2
+
+#define REG_DIG_GLB_70			0x0070
+#define  XTP_PCS_RX_EQ_IN_PROGRESS(x)	FIELD_PREP(GENMASK(25, 24), (x))
+#define  XTP_PCS_MODE_MASK		GENMASK(17, 16)
+#define  XTP_PCS_MODE(x)		FIELD_PREP(GENMASK(17, 16), (x))
+#define  XTP_PCS_RST_B			BIT(15)
+#define  XTP_FRC_PCS_RST_B		BIT(14)
+#define  XTP_PCS_PWD_SYNC_MASK		GENMASK(13, 12)
+#define  XTP_PCS_PWD_SYNC(x)		FIELD_PREP(XTP_PCS_PWD_SYNC_MASK, (x))
+#define  XTP_PCS_PWD_ASYNC_MASK		GENMASK(11, 10)
+#define  XTP_PCS_PWD_ASYNC(x)		FIELD_PREP(XTP_PCS_PWD_ASYNC_MASK, (x))
+#define  XTP_FRC_PCS_PWD_ASYNC		BIT(8)
+#define  XTP_PCS_UPDT			BIT(4)
+#define  XTP_PCS_IN_FR_RG		BIT(0)
+
+#define REG_DIG_GLB_F4			0x00f4
+#define  XFI_DPHY_PCS_SEL		BIT(0)
+#define   XFI_DPHY_PCS_SEL_SGMII	FIELD_PREP(XFI_DPHY_PCS_SEL, 1)
+#define   XFI_DPHY_PCS_SEL_USXGMII	FIELD_PREP(XFI_DPHY_PCS_SEL, 0)
+#define  XFI_DPHY_AD_SGDT_FRC_EN	BIT(5)
+
+#define REG_DIG_LN_TRX_40		0x3040
+#define  XTP_LN_FRC_TX_DATA_EN		BIT(29)
+#define  XTP_LN_TX_DATA_EN		BIT(28)
+
+#define REG_DIG_LN_TRX_B0		0x30b0
+#define  XTP_LN_FRC_TX_MACCK_EN		BIT(5)
+#define  XTP_LN_TX_MACCK_EN		BIT(4)
+
+#define REG_ANA_GLB_D0			0x90d0
+#define  XTP_GLB_USXGMII_SEL_MASK	GENMASK(3, 1)
+#define  XTP_GLB_USXGMII_SEL(x)		FIELD_PREP(GENMASK(3, 1), (x))
+#define  XTP_GLB_USXGMII_EN		BIT(0)
+
+struct mtk_xfi_tphy {
+	void __iomem		*base;
+	struct device		*dev;
+	struct reset_control	*reset;
+	struct clk_bulk_data	clocks[MTK_XFI_TPHY_NUM_CLOCKS];
+	bool			da_war;
+};
+
+static void mtk_xfi_tphy_write(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
+			       u32 value)
+{
+	iowrite32(value, xfi_tphy->base + reg);
+}
+
+static void mtk_xfi_tphy_rmw(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
+			     u32 clr, u32 set)
+{
+	u32 val;
+
+	val = ioread32(xfi_tphy->base + reg);
+	val &= ~clr;
+	val |= set;
+	iowrite32(val, xfi_tphy->base + reg);
+}
+
+static void mtk_xfi_tphy_set(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
+			     u32 set)
+{
+	mtk_xfi_tphy_rmw(xfi_tphy, reg, 0, set);
+}
+
+static void mtk_xfi_tphy_clear(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
+			       u32 clr)
+{
+	mtk_xfi_tphy_rmw(xfi_tphy, reg, clr, 0);
+}
+
+static void mtk_xfi_tphy_setup(struct mtk_xfi_tphy *xfi_tphy,
+			       phy_interface_t interface)
+{
+	bool is_2p5g = (interface == PHY_INTERFACE_MODE_2500BASEX);
+	bool is_1g = (interface == PHY_INTERFACE_MODE_1000BASEX ||
+		      interface == PHY_INTERFACE_MODE_SGMII);
+	bool is_10g = (interface == PHY_INTERFACE_MODE_10GBASER ||
+		       interface == PHY_INTERFACE_MODE_USXGMII);
+	bool is_5g = (interface == PHY_INTERFACE_MODE_5GBASER);
+	bool is_xgmii = (is_10g || is_5g);
+
+	dev_dbg(xfi_tphy->dev, "setting up for mode %s\n", phy_modes(interface));
+
+	/* Setup PLL setting */
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x9024, 0x100000, is_10g ? 0x0 : 0x100000);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x2020, 0x202000, is_5g ? 0x202000 : 0x0);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x2030, 0x500, is_1g ? 0x0 : 0x500);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x2034, 0xa00, is_1g ? 0x0 : 0xa00);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x2040, 0x340000, is_1g ? 0x200000 :
+							     0x140000);
+
+	/* Setup RXFE BW setting */
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x50f0, 0xc10, is_1g ? 0x410 :
+							  is_5g ? 0x800 : 0x400);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x50e0, 0x4000, is_5g ? 0x0 : 0x4000);
+
+	/* Setup RX CDR setting */
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x506c, 0x30000, is_5g ? 0x0 : 0x30000);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x5070, 0x670000, is_5g ? 0x620000 : 0x50000);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x5074, 0x180000, is_5g ? 0x180000 : 0x0);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x5078, 0xf000400, is_5g ? 0x8000000 :
+							      0x7000400);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x507c, 0x5000500, is_5g ? 0x4000400 :
+							      0x1000100);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x5080, 0x1410, is_1g ? 0x400 :
+							   is_5g ? 0x1010 : 0x0);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x5084, 0x30300, is_1g ? 0x30300 :
+							    is_5g ? 0x30100 :
+								    0x100);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x5088, 0x60200, is_1g ? 0x20200 :
+							 is_5g ? 0x40000 :
+								 0x20000);
+
+	/* Setting RXFE adaptation range setting */
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x50e4, 0xc0000, is_5g ? 0x0 : 0xc0000);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x50e8, 0x40000, is_5g ? 0x0 : 0x40000);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x50ec, 0xa00, is_1g ? 0x200 : 0x800);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x50a8, 0xee0000, is_5g ? 0x800000 :
+							     0x6e0000);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x6004, 0x190000, is_5g ? 0x0 : 0x190000);
+	if (is_10g)
+		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x01423342);
+	else if (is_5g)
+		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x00a132a1);
+	else if (is_2p5g)
+		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x009c329c);
+	else
+		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x00fa32fa);
+
+	/* Force SGDT_OUT off and select PCS */
+	mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_F4,
+			 XFI_DPHY_AD_SGDT_FRC_EN | XFI_DPHY_PCS_SEL,
+			 XFI_DPHY_AD_SGDT_FRC_EN |
+			 (is_xgmii ? XFI_DPHY_PCS_SEL_USXGMII :
+				     XFI_DPHY_PCS_SEL_SGMII));
+
+
+	/* Force GLB_CKDET_OUT */
+	mtk_xfi_tphy_set(xfi_tphy, 0x0030, 0xc00);
+
+	/* Force AEQ on */
+	mtk_xfi_tphy_write(xfi_tphy, REG_DIG_GLB_70,
+			   XTP_PCS_RX_EQ_IN_PROGRESS(2) |
+			   XTP_PCS_PWD_SYNC(2) |
+			   XTP_PCS_PWD_ASYNC(2));
+
+	usleep_range(1, 5);
+
+	/* Setup TX DA default value */
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x30b0, 0x30, 0x20);
+	mtk_xfi_tphy_write(xfi_tphy, 0x3028, 0x00008a01);
+	mtk_xfi_tphy_write(xfi_tphy, 0x302c, 0x0000a884);
+	mtk_xfi_tphy_write(xfi_tphy, 0x3024, 0x00083002);
+
+	/* Setup RG default value */
+	if (is_xgmii) {
+		mtk_xfi_tphy_write(xfi_tphy, 0x3010, 0x00022220);
+		mtk_xfi_tphy_write(xfi_tphy, 0x5064, 0x0f020a01);
+		mtk_xfi_tphy_write(xfi_tphy, 0x50b4, 0x06100600);
+		if (interface == PHY_INTERFACE_MODE_USXGMII)
+			mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x40704000);
+		else
+			mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x47684100);
+	} else {
+		mtk_xfi_tphy_write(xfi_tphy, 0x3010, 0x00011110);
+		mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x40704000);
+	}
+
+	if (is_1g)
+		mtk_xfi_tphy_write(xfi_tphy, 0x3064, 0x0000c000);
+
+	/* Setup RX EQ initial value */
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x3050, 0xa8000000,
+			 (interface != PHY_INTERFACE_MODE_10GBASER) ?
+			  0xa8000000 : 0x0);
+	mtk_xfi_tphy_rmw(xfi_tphy, 0x3054, 0xaa,
+			 (interface != PHY_INTERFACE_MODE_10GBASER) ?
+			  0xaa : 0x0);
+
+	if (is_xgmii)
+		mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x00000f00);
+	else if (is_2p5g)
+		mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x22000f00);
+	else
+		mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x20200f00);
+
+	if (interface == PHY_INTERFACE_MODE_10GBASER && xfi_tphy->da_war)
+		mtk_xfi_tphy_rmw(xfi_tphy, 0xa008, 0x10000, 0x10000);
+
+	mtk_xfi_tphy_rmw(xfi_tphy, 0xa060, 0x50000, is_xgmii ? 0x40000 :
+							       0x50000);
+
+	/* Setup PHYA speed */
+	mtk_xfi_tphy_rmw(xfi_tphy, REG_ANA_GLB_D0,
+			 XTP_GLB_USXGMII_SEL_MASK | XTP_GLB_USXGMII_EN,
+			 is_10g ?  XTP_GLB_USXGMII_SEL(0) :
+			 is_5g ?   XTP_GLB_USXGMII_SEL(1) :
+			 is_2p5g ? XTP_GLB_USXGMII_SEL(2) :
+				   XTP_GLB_USXGMII_SEL(3));
+	mtk_xfi_tphy_set(xfi_tphy, REG_ANA_GLB_D0, XTP_GLB_USXGMII_EN);
+
+	/* Release reset */
+	mtk_xfi_tphy_set(xfi_tphy, REG_DIG_GLB_70,
+			 XTP_PCS_RST_B | XTP_FRC_PCS_RST_B);
+	usleep_range(150, 500);
+
+	/* Switch to P0 */
+	mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
+			 XTP_PCS_PWD_SYNC_MASK |
+			 XTP_PCS_PWD_ASYNC_MASK,
+			 XTP_FRC_PCS_PWD_ASYNC |
+			 XTP_PCS_UPDT | XTP_PCS_IN_FR_RG);
+	usleep_range(1, 5);
+
+	mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_70, XTP_PCS_UPDT);
+	usleep_range(15, 50);
+
+	if (is_xgmii) {
+		/* Switch to Gen3 */
+		mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
+				 XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
+				 XTP_PCS_MODE(2) | XTP_PCS_UPDT);
+	} else {
+		/* Switch to Gen2 */
+		mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
+				 XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
+				 XTP_PCS_MODE(1) | XTP_PCS_UPDT);
+	}
+	usleep_range(1, 5);
+
+	mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_70, XTP_PCS_UPDT);
+
+	usleep_range(100, 500);
+
+	/* Enable MAC CK */
+	mtk_xfi_tphy_set(xfi_tphy, REG_DIG_LN_TRX_B0, XTP_LN_TX_MACCK_EN);
+	mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_F4, XFI_DPHY_AD_SGDT_FRC_EN);
+
+	/* Enable TX data */
+	mtk_xfi_tphy_set(xfi_tphy, REG_DIG_LN_TRX_40,
+			 XTP_LN_FRC_TX_DATA_EN | XTP_LN_TX_DATA_EN);
+	usleep_range(400, 1000);
+}
+
+static int mtk_xfi_tphy_set_mode(struct phy *phy, enum phy_mode mode, int
+				 submode)
+{
+	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
+
+	if (mode != PHY_MODE_ETHERNET)
+		return -EINVAL;
+
+	switch (submode) {
+	case PHY_INTERFACE_MODE_1000BASEX:
+	case PHY_INTERFACE_MODE_2500BASEX:
+	case PHY_INTERFACE_MODE_SGMII:
+	case PHY_INTERFACE_MODE_5GBASER:
+	case PHY_INTERFACE_MODE_10GBASER:
+	case PHY_INTERFACE_MODE_USXGMII:
+		mtk_xfi_tphy_setup(xfi_tphy, submode);
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static int mtk_xfi_tphy_reset(struct phy *phy)
+{
+	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
+
+	reset_control_assert(xfi_tphy->reset);
+	usleep_range(100, 500);
+	reset_control_deassert(xfi_tphy->reset);
+	usleep_range(1, 10);
+
+	return 0;
+}
+
+static int mtk_xfi_tphy_power_on(struct phy *phy)
+{
+	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
+
+	return clk_bulk_prepare_enable(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
+}
+
+static int mtk_xfi_tphy_power_off(struct phy *phy)
+{
+	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
+
+	clk_bulk_disable_unprepare(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
+
+	return 0;
+}
+
+static const struct phy_ops mtk_xfi_tphy_ops = {
+	.power_on	= mtk_xfi_tphy_power_on,
+	.power_off	= mtk_xfi_tphy_power_off,
+	.set_mode	= mtk_xfi_tphy_set_mode,
+	.reset		= mtk_xfi_tphy_reset,
+	.owner		= THIS_MODULE,
+};
+
+static int mtk_xfi_tphy_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct phy_provider *phy_provider;
+	struct mtk_xfi_tphy *xfi_tphy;
+	struct phy *phy;
+
+	if (!np)
+		return -ENODEV;
+
+	xfi_tphy = devm_kzalloc(&pdev->dev, sizeof(*xfi_tphy), GFP_KERNEL);
+	if (!xfi_tphy)
+		return -ENOMEM;
+
+	xfi_tphy->base = devm_of_iomap(&pdev->dev, np, 0, NULL);
+	if (!xfi_tphy->base)
+		return -EIO;
+
+	xfi_tphy->dev = &pdev->dev;
+
+	xfi_tphy->clocks[0].id = "topxtal";
+	xfi_tphy->clocks[0].clk = devm_clk_get(&pdev->dev, xfi_tphy->clocks[0].id);
+	if (IS_ERR(xfi_tphy->clocks[0].clk))
+		return PTR_ERR(xfi_tphy->clocks[0].clk);
+
+	xfi_tphy->clocks[1].id = "xfipll";
+	xfi_tphy->clocks[1].clk = devm_clk_get(&pdev->dev, xfi_tphy->clocks[1].id);
+	if (IS_ERR(xfi_tphy->clocks[1].clk))
+		return PTR_ERR(xfi_tphy->clocks[1].clk);
+
+	xfi_tphy->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+	if (IS_ERR(xfi_tphy->reset))
+		return PTR_ERR(xfi_tphy->reset);
+
+	xfi_tphy->da_war = of_property_read_bool(np,
+						 "mediatek,usxgmii-performance-errata");
+
+	phy = devm_phy_create(&pdev->dev, NULL, &mtk_xfi_tphy_ops);
+	if (IS_ERR(phy))
+		return PTR_ERR(phy);
+
+	phy_set_drvdata(phy, xfi_tphy);
+
+	phy_provider = devm_of_phy_provider_register(&pdev->dev,
+						     of_phy_simple_xlate);
+
+	return PTR_ERR_OR_ZERO(phy_provider);
+}
+
+static const struct of_device_id mtk_xfi_tphy_match[] = {
+	{ .compatible = "mediatek,mt7988-xfi-tphy", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, mtk_xfi_tphy_match);
+
+static struct platform_driver mtk_xfi_tphy_driver = {
+	.probe = mtk_xfi_tphy_probe,
+	.driver = {
+		.name = "mtk-xfi-tphy",
+		.of_match_table = mtk_xfi_tphy_match,
+	},
+};
+module_platform_driver(mtk_xfi_tphy_driver);
+
+MODULE_DESCRIPTION("MediaTek XFI T-PHY driver");
+MODULE_AUTHOR("Daniel Golle <daniel@makrotopia.org>");
+MODULE_AUTHOR("Bc-bocun Chen <bc-bocun.chen@mediatek.com>");
+MODULE_LICENSE("GPL");
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: phy: mediatek,xfi-tphy: add new bindings
  2024-02-01 21:52 [PATCH 1/2] dt-bindings: phy: mediatek,xfi-tphy: add new bindings Daniel Golle
  2024-02-01 21:53 ` [PATCH 2/2] phy: add driver for MediaTek XFI T-PHY Daniel Golle
@ 2024-02-02  8:21 ` Krzysztof Kozlowski
  2024-02-04  6:17   ` Chunfeng Yun (云春峰)
  2024-02-02 21:24 ` Rob Herring
  2 siblings, 1 reply; 12+ messages in thread
From: Krzysztof Kozlowski @ 2024-02-02  8:21 UTC (permalink / raw)
  To: Daniel Golle, Bc-bocun Chen, Steven Liu, John Crispin,
	Chunfeng Yun, Vinod Koul, Kishon Vijay Abraham I, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Qingfang Deng, SkyLake Huang,
	Matthias Brugger, AngeloGioacchino Del Regno, Philipp Zabel,
	linux-arm-kernel, linux-mediatek, linux-phy, devicetree,
	linux-kernel, netdev

On 01/02/2024 22:52, Daniel Golle wrote:
> Add bindings for the MediaTek XFI T-PHY Ethernet SerDes PHY found in the
> MediaTek MT7988 SoC which can operate at various interfaces modes:
> 
> via USXGMII PCS:
>  * USXGMII
>  * 10GBase-R
>  * 5GBase-R
> 
> via LynxI SGMII PCS:
>  * 2500Base-X
>  * 1000Base-X
>  * Cisco SGMII (MAC side)
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  .../bindings/phy/mediatek,xfi-tphy.yaml       | 80 +++++++++++++++++++
>  1 file changed, 80 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
> new file mode 100644
> index 0000000000000..e897118dcf7e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/mediatek,xfi-tphy.yaml#

Please use compatible as filename. Your binding says only one is
possible (const, not enum), so there is no reasoning for different filename.

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek XFI T-PHY
> +
> +maintainers:
> +  - Daniel Golle <daniel@makrotopia.org>
> +
> +description:
> +  The MediaTek XFI SerDes T-PHY provides the physical SerDes lanes
> +  used by the (10G/5G) USXGMII PCS and (1G/2.5G) LynxI PCS found in
> +  MediaTek's 10G-capabale SoCs.
> +
> +properties:
> +  $nodename:
> +    pattern: "^phy@[0-9a-f]+$"

No need for nodename in individual bindings file.

> +
> +  compatible:
> +    const: mediatek,mt7988-xfi-tphy
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: XFI PHY clock
> +      - description: XFI register clock
> +
> +  clock-names:
> +    items:
> +      - const: xfipll
> +      - const: topxtal
> +
> +  resets:
> +    items:
> +      - description: PEXTP reset
> +
> +  mediatek,usxgmii-performance-errata:
> +    $ref: /schemas/types.yaml#/definitions/flag
> +    description:
> +      One instance of the T-PHY on MT7988 suffers from a performance
> +      problem in 10GBase-R mode which needs a work-around in the driver.

Can you explain what is this issue and errata about (except performance)?

> +      The work-around is enabled using this flag.
> +
> +  "#phy-cells":
> +    const: 0
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - resets
> +  - "#phy-cells"
> +
> +additionalProperties: false


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] phy: add driver for MediaTek XFI T-PHY
  2024-02-01 21:53 ` [PATCH 2/2] phy: add driver for MediaTek XFI T-PHY Daniel Golle
@ 2024-02-02 13:21   ` AngeloGioacchino Del Regno
  2024-02-02 16:33     ` Daniel Golle
  2024-02-04  6:29   ` Chunfeng Yun (云春峰)
  1 sibling, 1 reply; 12+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-02-02 13:21 UTC (permalink / raw)
  To: Daniel Golle, Bc-bocun Chen, Chunfeng Yun, Vinod Koul,
	Kishon Vijay Abraham I, Rob Herring, Krzysztof Kozlowski,
	Conor Dooley, Qingfang Deng, SkyLake Huang, Matthias Brugger,
	Philipp Zabel, linux-arm-kernel, linux-mediatek, linux-phy,
	devicetree, linux-kernel, netdev

Il 01/02/24 22:53, Daniel Golle ha scritto:
> Add driver for MediaTek's XFI T-PHY, 10 Gigabit/s Ethernet SerDes PHY
> which can be found in the MT7988 SoC.
> 
> The PHY can operates only in PHY_MODE_ETHERNET, the submode is one of
> PHY_INTERFACE_MODE_* corresponding to the supported modes:
> 
>   * USXGMII                 \
>   * 10GBase-R                }- USXGMII PCS - XGDM  \
>   * 5GBase-R                /                        \
>                                                       }- Ethernet MAC
>   * 2500Base-X              \                        /
>   * 1000Base-X               }- LynxI PCS - GDM     /
>   * Cisco SGMII (MAC side)  /
> 
> In order to work-around a performance issue present on the first of
> two XFI T-PHYs present in MT7988, special tuning is applied which can be
> selected by adding the 'mediatek,usxgmii-performance-errata' property to
> the device tree node.
> 
> There is no documentation for most registers used for the
> analog/tuning part, however, most of the registers have been partially
> reverse-engineered from MediaTek's SDK implementation (an opaque
> sequence of 32-bit register writes) and descriptions for all relevant
> digital registers and bits such as resets and muxes have been supplied
> by MediaTek.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>   MAINTAINERS                             |   1 +
>   drivers/phy/mediatek/Kconfig            |  12 +
>   drivers/phy/mediatek/Makefile           |   1 +
>   drivers/phy/mediatek/phy-mtk-xfi-tphy.c | 392 ++++++++++++++++++++++++
>   4 files changed, 406 insertions(+)
>   create mode 100644 drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 52769631bdb1a..52e4192470bd9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13715,6 +13715,7 @@ L:	netdev@vger.kernel.org
>   S:	Maintained
>   F:	drivers/net/phy/mediatek-ge-soc.c
>   F:	drivers/net/phy/mediatek-ge.c
> +F:	drivers/phy/mediatek/phy-mtk-xfi-tphy.c
>   
>   MEDIATEK I2C CONTROLLER DRIVER
>   M:	Qii Wang <qii.wang@mediatek.com>
> diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
> index 3125ecb5d119f..5161d130c7f8b 100644
> --- a/drivers/phy/mediatek/Kconfig
> +++ b/drivers/phy/mediatek/Kconfig
> @@ -13,6 +13,18 @@ config PHY_MTK_PCIE
>   	  callback for PCIe GEN3 port, it supports software efuse
>   	  initialization.
>   
> +config PHY_MTK_XFI_TPHY
> +	tristate "MediaTek XFI T-PHY Driver"
> +	depends on ARCH_MEDIATEK || COMPILE_TEST
> +	depends on OF && OF_ADDRESS
> +	depends on HAS_IOMEM
> +	select GENERIC_PHY
> +	help
> +	  Say 'Y' here to add support for MediaTek XFI T-PHY driver.
> +	  The driver provides access to the Ethernet SerDes T-PHY supporting
> +	  1GE and 2.5GE modes via the LynxI PCS, and 5GE and 10GE modes
> +	  via the USXGMII PCS found in MediaTek SoCs with 10G Ethernet.
> +
>   config PHY_MTK_TPHY
>   	tristate "MediaTek T-PHY Driver"
>   	depends on ARCH_MEDIATEK || COMPILE_TEST
> diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
> index c9a50395533eb..fa7217178e7f4 100644
> --- a/drivers/phy/mediatek/Makefile
> +++ b/drivers/phy/mediatek/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_PHY_MTK_PCIE)		+= phy-mtk-pcie.o
>   obj-$(CONFIG_PHY_MTK_TPHY)		+= phy-mtk-tphy.o
>   obj-$(CONFIG_PHY_MTK_UFS)		+= phy-mtk-ufs.o
>   obj-$(CONFIG_PHY_MTK_XSPHY)		+= phy-mtk-xsphy.o
> +obj-$(CONFIG_PHY_MTK_XFI_TPHY)		+= phy-mtk-xfi-tphy.o
>   
>   phy-mtk-hdmi-drv-y			:= phy-mtk-hdmi.o
>   phy-mtk-hdmi-drv-y			+= phy-mtk-hdmi-mt2701.o
> diff --git a/drivers/phy/mediatek/phy-mtk-xfi-tphy.c b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> new file mode 100644
> index 0000000000000..d50e6320860e5
> --- /dev/null
> +++ b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> @@ -0,0 +1,392 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* MediaTek 10GE SerDes PHY driver

MediaTek 10GE SerDes XFI T-PHY driver ?

> + *
> + * Copyright (c) 2024 Daniel Golle <daniel@makrotopia.org>
> + *                    Bc-bocun Chen <bc-bocun.chen@mediatek.com>
> + * based on mtk_usxgmii.c found in MediaTek's SDK released under GPL-2.0
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Henry Yen <henry.yen@mediatek.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/io.h>
> +#include <linux/clk.h>
> +#include <linux/reset.h>
> +#include <linux/phy.h>
> +#include <linux/phy/phy.h>
> +
> +#define MTK_XFI_TPHY_NUM_CLOCKS		2
> +
> +#define REG_DIG_GLB_70			0x0070
> +#define  XTP_PCS_RX_EQ_IN_PROGRESS(x)	FIELD_PREP(GENMASK(25, 24), (x))
> +#define  XTP_PCS_MODE_MASK		GENMASK(17, 16)
> +#define  XTP_PCS_MODE(x)		FIELD_PREP(GENMASK(17, 16), (x))
> +#define  XTP_PCS_RST_B			BIT(15)
> +#define  XTP_FRC_PCS_RST_B		BIT(14)
> +#define  XTP_PCS_PWD_SYNC_MASK		GENMASK(13, 12)
> +#define  XTP_PCS_PWD_SYNC(x)		FIELD_PREP(XTP_PCS_PWD_SYNC_MASK, (x))
> +#define  XTP_PCS_PWD_ASYNC_MASK		GENMASK(11, 10)
> +#define  XTP_PCS_PWD_ASYNC(x)		FIELD_PREP(XTP_PCS_PWD_ASYNC_MASK, (x))
> +#define  XTP_FRC_PCS_PWD_ASYNC		BIT(8)
> +#define  XTP_PCS_UPDT			BIT(4)
> +#define  XTP_PCS_IN_FR_RG		BIT(0)
> +
> +#define REG_DIG_GLB_F4			0x00f4
> +#define  XFI_DPHY_PCS_SEL		BIT(0)
> +#define   XFI_DPHY_PCS_SEL_SGMII	FIELD_PREP(XFI_DPHY_PCS_SEL, 1)
> +#define   XFI_DPHY_PCS_SEL_USXGMII	FIELD_PREP(XFI_DPHY_PCS_SEL, 0)
> +#define  XFI_DPHY_AD_SGDT_FRC_EN	BIT(5)
> +
> +#define REG_DIG_LN_TRX_40		0x3040
> +#define  XTP_LN_FRC_TX_DATA_EN		BIT(29)
> +#define  XTP_LN_TX_DATA_EN		BIT(28)
> +
> +#define REG_DIG_LN_TRX_B0		0x30b0
> +#define  XTP_LN_FRC_TX_MACCK_EN		BIT(5)
> +#define  XTP_LN_TX_MACCK_EN		BIT(4)
> +
> +#define REG_ANA_GLB_D0			0x90d0
> +#define  XTP_GLB_USXGMII_SEL_MASK	GENMASK(3, 1)
> +#define  XTP_GLB_USXGMII_SEL(x)		FIELD_PREP(GENMASK(3, 1), (x))
> +#define  XTP_GLB_USXGMII_EN		BIT(0)
> +
> +struct mtk_xfi_tphy {
> +	void __iomem		*base;
> +	struct device		*dev;
> +	struct reset_control	*reset;
> +	struct clk_bulk_data	clocks[MTK_XFI_TPHY_NUM_CLOCKS];
> +	bool			da_war;
> +};
> +
> +static void mtk_xfi_tphy_write(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
> +			       u32 value)
> +{
> +	iowrite32(value, xfi_tphy->base + reg);
> +}
> +
> +static void mtk_xfi_tphy_rmw(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
> +			     u32 clr, u32 set)
> +{
> +	u32 val;
> +
> +	val = ioread32(xfi_tphy->base + reg);
> +	val &= ~clr;
> +	val |= set;
> +	iowrite32(val, xfi_tphy->base + reg);
> +}
> +
> +static void mtk_xfi_tphy_set(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
> +			     u32 set)
> +{
> +	mtk_xfi_tphy_rmw(xfi_tphy, reg, 0, set);
> +}
> +
> +static void mtk_xfi_tphy_clear(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
> +			       u32 clr)
> +{
> +	mtk_xfi_tphy_rmw(xfi_tphy, reg, clr, 0);
> +}
> +
> +static void mtk_xfi_tphy_setup(struct mtk_xfi_tphy *xfi_tphy,
> +			       phy_interface_t interface)
> +{
> +	bool is_2p5g = (interface == PHY_INTERFACE_MODE_2500BASEX);
> +	bool is_1g = (interface == PHY_INTERFACE_MODE_1000BASEX ||
> +		      interface == PHY_INTERFACE_MODE_SGMII);
> +	bool is_10g = (interface == PHY_INTERFACE_MODE_10GBASER ||
> +		       interface == PHY_INTERFACE_MODE_USXGMII);
> +	bool is_5g = (interface == PHY_INTERFACE_MODE_5GBASER);
> +	bool is_xgmii = (is_10g || is_5g);

is_usxgmii, I'd say.

> +
> +	dev_dbg(xfi_tphy->dev, "setting up for mode %s\n", phy_modes(interface));
> +
> +	/* Setup PLL setting */
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x9024, 0x100000, is_10g ? 0x0 : 0x100000);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x2020, 0x202000, is_5g ? 0x202000 : 0x0);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x2030, 0x500, is_1g ? 0x0 : 0x500);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x2034, 0xa00, is_1g ? 0x0 : 0xa00);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x2040, 0x340000, is_1g ? 0x200000 :
> +							     0x140000);
> +
> +	/* Setup RXFE BW setting */
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50f0, 0xc10, is_1g ? 0x410 :
> +							  is_5g ? 0x800 : 0x400);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50e0, 0x4000, is_5g ? 0x0 : 0x4000);
> +
> +	/* Setup RX CDR setting */
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x506c, 0x30000, is_5g ? 0x0 : 0x30000);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5070, 0x670000, is_5g ? 0x620000 : 0x50000);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5074, 0x180000, is_5g ? 0x180000 : 0x0);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5078, 0xf000400, is_5g ? 0x8000000 :
> +							      0x7000400);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x507c, 0x5000500, is_5g ? 0x4000400 :
> +							      0x1000100);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5080, 0x1410, is_1g ? 0x400 :
> +							   is_5g ? 0x1010 : 0x0);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5084, 0x30300, is_1g ? 0x30300 :
> +							    is_5g ? 0x30100 :
> +								    0x100);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5088, 0x60200, is_1g ? 0x20200 :
> +							 is_5g ? 0x40000 :
> +								 0x20000);
> +
> +	/* Setting RXFE adaptation range setting */
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50e4, 0xc0000, is_5g ? 0x0 : 0xc0000);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50e8, 0x40000, is_5g ? 0x0 : 0x40000);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50ec, 0xa00, is_1g ? 0x200 : 0x800);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50a8, 0xee0000, is_5g ? 0x800000 :
> +							     0x6e0000);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x6004, 0x190000, is_5g ? 0x0 : 0x190000);
> +	if (is_10g)
> +		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x01423342);
> +	else if (is_5g)
> +		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x00a132a1);
> +	else if (is_2p5g)
> +		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x009c329c);
> +	else
> +		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x00fa32fa);
> +
> +	/* Force SGDT_OUT off and select PCS */
> +	mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_F4,
> +			 XFI_DPHY_AD_SGDT_FRC_EN | XFI_DPHY_PCS_SEL,
> +			 XFI_DPHY_AD_SGDT_FRC_EN |
> +			 (is_xgmii ? XFI_DPHY_PCS_SEL_USXGMII :
> +				     XFI_DPHY_PCS_SEL_SGMII));
> +
> +
> +	/* Force GLB_CKDET_OUT */
> +	mtk_xfi_tphy_set(xfi_tphy, 0x0030, 0xc00);
> +
> +	/* Force AEQ on */
> +	mtk_xfi_tphy_write(xfi_tphy, REG_DIG_GLB_70,
> +			   XTP_PCS_RX_EQ_IN_PROGRESS(2) |
> +			   XTP_PCS_PWD_SYNC(2) |
> +			   XTP_PCS_PWD_ASYNC(2));
> +
> +	usleep_range(1, 5);
> +
> +	/* Setup TX DA default value */
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x30b0, 0x30, 0x20);
> +	mtk_xfi_tphy_write(xfi_tphy, 0x3028, 0x00008a01);
> +	mtk_xfi_tphy_write(xfi_tphy, 0x302c, 0x0000a884);
> +	mtk_xfi_tphy_write(xfi_tphy, 0x3024, 0x00083002);
> +
> +	/* Setup RG default value */
> +	if (is_xgmii) {
> +		mtk_xfi_tphy_write(xfi_tphy, 0x3010, 0x00022220);
> +		mtk_xfi_tphy_write(xfi_tphy, 0x5064, 0x0f020a01);
> +		mtk_xfi_tphy_write(xfi_tphy, 0x50b4, 0x06100600);
> +		if (interface == PHY_INTERFACE_MODE_USXGMII)
> +			mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x40704000);
> +		else
> +			mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x47684100);
> +	} else {
> +		mtk_xfi_tphy_write(xfi_tphy, 0x3010, 0x00011110);
> +		mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x40704000);
> +	}
> +
> +	if (is_1g)
> +		mtk_xfi_tphy_write(xfi_tphy, 0x3064, 0x0000c000);
> +
> +	/* Setup RX EQ initial value */
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x3050, 0xa8000000,
> +			 (interface != PHY_INTERFACE_MODE_10GBASER) ?
> +			  0xa8000000 : 0x0);
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x3054, 0xaa,
> +			 (interface != PHY_INTERFACE_MODE_10GBASER) ?
> +			  0xaa : 0x0);
> +
> +	if (is_xgmii)
> +		mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x00000f00);
> +	else if (is_2p5g)
> +		mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x22000f00);
> +	else
> +		mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x20200f00);
> +
> +	if (interface == PHY_INTERFACE_MODE_10GBASER && xfi_tphy->da_war)
> +		mtk_xfi_tphy_rmw(xfi_tphy, 0xa008, 0x10000, 0x10000);
> +
> +	mtk_xfi_tphy_rmw(xfi_tphy, 0xa060, 0x50000, is_xgmii ? 0x40000 :
> +							       0x50000);
> +
> +	/* Setup PHYA speed */
> +	mtk_xfi_tphy_rmw(xfi_tphy, REG_ANA_GLB_D0,
> +			 XTP_GLB_USXGMII_SEL_MASK | XTP_GLB_USXGMII_EN,
> +			 is_10g ?  XTP_GLB_USXGMII_SEL(0) :
> +			 is_5g ?   XTP_GLB_USXGMII_SEL(1) :
> +			 is_2p5g ? XTP_GLB_USXGMII_SEL(2) :
> +				   XTP_GLB_USXGMII_SEL(3));
> +	mtk_xfi_tphy_set(xfi_tphy, REG_ANA_GLB_D0, XTP_GLB_USXGMII_EN);
> +
> +	/* Release reset */
> +	mtk_xfi_tphy_set(xfi_tphy, REG_DIG_GLB_70,
> +			 XTP_PCS_RST_B | XTP_FRC_PCS_RST_B);
> +	usleep_range(150, 500);
> +
> +	/* Switch to P0 */
> +	mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
> +			 XTP_PCS_PWD_SYNC_MASK |
> +			 XTP_PCS_PWD_ASYNC_MASK,
> +			 XTP_FRC_PCS_PWD_ASYNC |
> +			 XTP_PCS_UPDT | XTP_PCS_IN_FR_RG);
> +	usleep_range(1, 5);
> +
> +	mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_70, XTP_PCS_UPDT);
> +	usleep_range(15, 50);
> +
> +	if (is_xgmii) {
> +		/* Switch to Gen3 */
> +		mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
> +				 XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
> +				 XTP_PCS_MODE(2) | XTP_PCS_UPDT);
> +	} else {
> +		/* Switch to Gen2 */
> +		mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
> +				 XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
> +				 XTP_PCS_MODE(1) | XTP_PCS_UPDT);
> +	}
> +	usleep_range(1, 5);
> +
> +	mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_70, XTP_PCS_UPDT);
> +
> +	usleep_range(100, 500);
> +
> +	/* Enable MAC CK */
> +	mtk_xfi_tphy_set(xfi_tphy, REG_DIG_LN_TRX_B0, XTP_LN_TX_MACCK_EN);
> +	mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_F4, XFI_DPHY_AD_SGDT_FRC_EN);
> +
> +	/* Enable TX data */
> +	mtk_xfi_tphy_set(xfi_tphy, REG_DIG_LN_TRX_40,
> +			 XTP_LN_FRC_TX_DATA_EN | XTP_LN_TX_DATA_EN);
> +	usleep_range(400, 1000);
> +}
> +
> +static int mtk_xfi_tphy_set_mode(struct phy *phy, enum phy_mode mode, int
> +				 submode)
> +{
> +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> +
> +	if (mode != PHY_MODE_ETHERNET)
> +		return -EINVAL;
> +
> +	switch (submode) {
> +	case PHY_INTERFACE_MODE_1000BASEX:

fallthrough;

> +	case PHY_INTERFACE_MODE_2500BASEX:

fallthrough;

> +	case PHY_INTERFACE_MODE_SGMII:

... etc :-)

> +	case PHY_INTERFACE_MODE_5GBASER:
> +	case PHY_INTERFACE_MODE_10GBASER:
> +	case PHY_INTERFACE_MODE_USXGMII:

Does this PHY support PHY_INTERFACE_MODE_XGMII ?

> +		mtk_xfi_tphy_setup(xfi_tphy, submode);
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static int mtk_xfi_tphy_reset(struct phy *phy)
> +{
> +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> +
> +	reset_control_assert(xfi_tphy->reset);
> +	usleep_range(100, 500);
> +	reset_control_deassert(xfi_tphy->reset);
> +	usleep_range(1, 10);
> +
> +	return 0;
> +}
> +
> +static int mtk_xfi_tphy_power_on(struct phy *phy)
> +{
> +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> +
> +	return clk_bulk_prepare_enable(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
> +}
> +
> +static int mtk_xfi_tphy_power_off(struct phy *phy)
> +{
> +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> +
> +	clk_bulk_disable_unprepare(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
> +
> +	return 0;
> +}
> +
> +static const struct phy_ops mtk_xfi_tphy_ops = {
> +	.power_on	= mtk_xfi_tphy_power_on,
> +	.power_off	= mtk_xfi_tphy_power_off,
> +	.set_mode	= mtk_xfi_tphy_set_mode,
> +	.reset		= mtk_xfi_tphy_reset,
> +	.owner		= THIS_MODULE,
> +};
> +
> +static int mtk_xfi_tphy_probe(struct platform_device *pdev)
> +{
> +	struct device_node *np = pdev->dev.of_node;
> +	struct phy_provider *phy_provider;
> +	struct mtk_xfi_tphy *xfi_tphy;
> +	struct phy *phy;
> +
> +	if (!np)
> +		return -ENODEV;
> +
> +	xfi_tphy = devm_kzalloc(&pdev->dev, sizeof(*xfi_tphy), GFP_KERNEL);
> +	if (!xfi_tphy)
> +		return -ENOMEM;
> +
> +	xfi_tphy->base = devm_of_iomap(&pdev->dev, np, 0, NULL);

Why devm_of_iomap() and not devm_platform_ioremap_resource()?

> +	if (!xfi_tphy->base)
> +		return -EIO;
> +
> +	xfi_tphy->dev = &pdev->dev;
> +
> +	xfi_tphy->clocks[0].id = "topxtal";

xfi_tphy->clocks[0].id = "topxtal";
xfi_tphy->clocks[1].id = "xfipll";

ret = devm_clk_bulk_get(&pdev->dev, MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
if (ret)
	return ret;

...it's that simple :-P

> +	xfi_tphy->clocks[0].clk = devm_clk_get(&pdev->dev, xfi_tphy->clocks[0].id);
> +	if (IS_ERR(xfi_tphy->clocks[0].clk))
> +		return PTR_ERR(xfi_tphy->clocks[0].clk);
> +
> +	xfi_tphy->clocks[1].id = "xfipll";
> +	xfi_tphy->clocks[1].clk = devm_clk_get(&pdev->dev, xfi_tphy->clocks[1].id);
> +	if (IS_ERR(xfi_tphy->clocks[1].clk))
> +		return PTR_ERR(xfi_tphy->clocks[1].clk);
> +
> +	xfi_tphy->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> +	if (IS_ERR(xfi_tphy->reset))
> +		return PTR_ERR(xfi_tphy->reset);
> +
> +	xfi_tphy->da_war = of_property_read_bool(np,
> +						 "mediatek,usxgmii-performance-errata");
> +

One line please

> +	phy = devm_phy_create(&pdev->dev, NULL, &mtk_xfi_tphy_ops);
> +	if (IS_ERR(phy))
> +		return PTR_ERR(phy);
> +
> +	phy_set_drvdata(phy, xfi_tphy);
> +
> +	phy_provider = devm_of_phy_provider_register(&pdev->dev,
> +						     of_phy_simple_xlate);

ditto

> +
> +	return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct of_device_id mtk_xfi_tphy_match[] = {
> +	{ .compatible = "mediatek,mt7988-xfi-tphy", },
> +	{ }

	{ .compatible = "mediatek,mt7988-xfi-tphy" },
	{ /* sentinel */ }

> +};
> +MODULE_DEVICE_TABLE(of, mtk_xfi_tphy_match);
> +
> +static struct platform_driver mtk_xfi_tphy_driver = {
> +	.probe = mtk_xfi_tphy_probe,
> +	.driver = {
> +		.name = "mtk-xfi-tphy",
> +		.of_match_table = mtk_xfi_tphy_match,
> +	},
> +};
> +module_platform_driver(mtk_xfi_tphy_driver);
> +
> +MODULE_DESCRIPTION("MediaTek XFI T-PHY driver");

MODULE_DESCRIPTION("MediaTek 10GE SerDes XFI T-PHY driver");

Cheers,
Angelo

> +MODULE_AUTHOR("Daniel Golle <daniel@makrotopia.org>");
> +MODULE_AUTHOR("Bc-bocun Chen <bc-bocun.chen@mediatek.com>");
> +MODULE_LICENSE("GPL");


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] phy: add driver for MediaTek XFI T-PHY
  2024-02-02 13:21   ` AngeloGioacchino Del Regno
@ 2024-02-02 16:33     ` Daniel Golle
  2024-02-05 15:26       ` AngeloGioacchino Del Regno
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Golle @ 2024-02-02 16:33 UTC (permalink / raw)
  To: AngeloGioacchino Del Regno
  Cc: Bc-bocun Chen, Chunfeng Yun, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Qingfang Deng,
	SkyLake Huang, Matthias Brugger, Philipp Zabel, linux-arm-kernel,
	linux-mediatek, linux-phy, devicetree, linux-kernel, netdev

Hi Angelo,

thank you for promptly reviewing my submission and providing valuable
feedback!

On Fri, Feb 02, 2024 at 02:21:48PM +0100, AngeloGioacchino Del Regno wrote:
> Il 01/02/24 22:53, Daniel Golle ha scritto:
> > Add driver for MediaTek's XFI T-PHY, 10 Gigabit/s Ethernet SerDes PHY
> > which can be found in the MT7988 SoC.
> > 
> > The PHY can operates only in PHY_MODE_ETHERNET, the submode is one of
> > PHY_INTERFACE_MODE_* corresponding to the supported modes:
> > 
> >   * USXGMII                 \
> >   * 10GBase-R                }- USXGMII PCS - XGDM  \
> >   * 5GBase-R                /                        \
> >                                                       }- Ethernet MAC
> >   * 2500Base-X              \                        /
> >   * 1000Base-X               }- LynxI PCS - GDM     /
> >   * Cisco SGMII (MAC side)  /
> > 
> > In order to work-around a performance issue present on the first of
> > two XFI T-PHYs present in MT7988, special tuning is applied which can be
> > selected by adding the 'mediatek,usxgmii-performance-errata' property to
> > the device tree node.
> > 
> > There is no documentation for most registers used for the
> > analog/tuning part, however, most of the registers have been partially
> > reverse-engineered from MediaTek's SDK implementation (an opaque
> > sequence of 32-bit register writes) and descriptions for all relevant
> > digital registers and bits such as resets and muxes have been supplied
> > by MediaTek.
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >   MAINTAINERS                             |   1 +
> >   drivers/phy/mediatek/Kconfig            |  12 +
> >   drivers/phy/mediatek/Makefile           |   1 +
> >   drivers/phy/mediatek/phy-mtk-xfi-tphy.c | 392 ++++++++++++++++++++++++
> >   4 files changed, 406 insertions(+)
> >   create mode 100644 drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> > 
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 52769631bdb1a..52e4192470bd9 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -13715,6 +13715,7 @@ L:	netdev@vger.kernel.org
> >   S:	Maintained
> >   F:	drivers/net/phy/mediatek-ge-soc.c
> >   F:	drivers/net/phy/mediatek-ge.c
> > +F:	drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> >   MEDIATEK I2C CONTROLLER DRIVER
> >   M:	Qii Wang <qii.wang@mediatek.com>
> > diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
> > index 3125ecb5d119f..5161d130c7f8b 100644
> > --- a/drivers/phy/mediatek/Kconfig
> > +++ b/drivers/phy/mediatek/Kconfig
> > @@ -13,6 +13,18 @@ config PHY_MTK_PCIE
> >   	  callback for PCIe GEN3 port, it supports software efuse
> >   	  initialization.
> > +config PHY_MTK_XFI_TPHY
> > +	tristate "MediaTek XFI T-PHY Driver"
> > +	depends on ARCH_MEDIATEK || COMPILE_TEST
> > +	depends on OF && OF_ADDRESS
> > +	depends on HAS_IOMEM
> > +	select GENERIC_PHY
> > +	help
> > +	  Say 'Y' here to add support for MediaTek XFI T-PHY driver.
> > +	  The driver provides access to the Ethernet SerDes T-PHY supporting
> > +	  1GE and 2.5GE modes via the LynxI PCS, and 5GE and 10GE modes
> > +	  via the USXGMII PCS found in MediaTek SoCs with 10G Ethernet.
> > +
> >   config PHY_MTK_TPHY
> >   	tristate "MediaTek T-PHY Driver"
> >   	depends on ARCH_MEDIATEK || COMPILE_TEST
> > diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
> > index c9a50395533eb..fa7217178e7f4 100644
> > --- a/drivers/phy/mediatek/Makefile
> > +++ b/drivers/phy/mediatek/Makefile
> > @@ -8,6 +8,7 @@ obj-$(CONFIG_PHY_MTK_PCIE)		+= phy-mtk-pcie.o
> >   obj-$(CONFIG_PHY_MTK_TPHY)		+= phy-mtk-tphy.o
> >   obj-$(CONFIG_PHY_MTK_UFS)		+= phy-mtk-ufs.o
> >   obj-$(CONFIG_PHY_MTK_XSPHY)		+= phy-mtk-xsphy.o
> > +obj-$(CONFIG_PHY_MTK_XFI_TPHY)		+= phy-mtk-xfi-tphy.o
> >   phy-mtk-hdmi-drv-y			:= phy-mtk-hdmi.o
> >   phy-mtk-hdmi-drv-y			+= phy-mtk-hdmi-mt2701.o
> > diff --git a/drivers/phy/mediatek/phy-mtk-xfi-tphy.c b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> > new file mode 100644
> > index 0000000000000..d50e6320860e5
> > --- /dev/null
> > +++ b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> > @@ -0,0 +1,392 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/* MediaTek 10GE SerDes PHY driver
> 
> MediaTek 10GE SerDes XFI T-PHY driver ?

Ack. Will unify the name accross all files.

> 
> > + *
> > + * Copyright (c) 2024 Daniel Golle <daniel@makrotopia.org>
> > + *                    Bc-bocun Chen <bc-bocun.chen@mediatek.com>
> > + * based on mtk_usxgmii.c found in MediaTek's SDK released under GPL-2.0
> > + * Copyright (c) 2022 MediaTek Inc.
> > + * Author: Henry Yen <henry.yen@mediatek.com>
> > + */
> > +
> > +#include <linux/module.h>
> > +#include <linux/device.h>
> > +#include <linux/platform_device.h>
> > +#include <linux/of.h>
> > +#include <linux/io.h>
> > +#include <linux/clk.h>
> > +#include <linux/reset.h>
> > +#include <linux/phy.h>
> > +#include <linux/phy/phy.h>
> > +
> > +#define MTK_XFI_TPHY_NUM_CLOCKS		2
> > +
> > +#define REG_DIG_GLB_70			0x0070
> > +#define  XTP_PCS_RX_EQ_IN_PROGRESS(x)	FIELD_PREP(GENMASK(25, 24), (x))
> > +#define  XTP_PCS_MODE_MASK		GENMASK(17, 16)
> > +#define  XTP_PCS_MODE(x)		FIELD_PREP(GENMASK(17, 16), (x))
> > +#define  XTP_PCS_RST_B			BIT(15)
> > +#define  XTP_FRC_PCS_RST_B		BIT(14)
> > +#define  XTP_PCS_PWD_SYNC_MASK		GENMASK(13, 12)
> > +#define  XTP_PCS_PWD_SYNC(x)		FIELD_PREP(XTP_PCS_PWD_SYNC_MASK, (x))
> > +#define  XTP_PCS_PWD_ASYNC_MASK		GENMASK(11, 10)
> > +#define  XTP_PCS_PWD_ASYNC(x)		FIELD_PREP(XTP_PCS_PWD_ASYNC_MASK, (x))
> > +#define  XTP_FRC_PCS_PWD_ASYNC		BIT(8)
> > +#define  XTP_PCS_UPDT			BIT(4)
> > +#define  XTP_PCS_IN_FR_RG		BIT(0)
> > +
> > +#define REG_DIG_GLB_F4			0x00f4
> > +#define  XFI_DPHY_PCS_SEL		BIT(0)
> > +#define   XFI_DPHY_PCS_SEL_SGMII	FIELD_PREP(XFI_DPHY_PCS_SEL, 1)
> > +#define   XFI_DPHY_PCS_SEL_USXGMII	FIELD_PREP(XFI_DPHY_PCS_SEL, 0)
> > +#define  XFI_DPHY_AD_SGDT_FRC_EN	BIT(5)
> > +
> > +#define REG_DIG_LN_TRX_40		0x3040
> > +#define  XTP_LN_FRC_TX_DATA_EN		BIT(29)
> > +#define  XTP_LN_TX_DATA_EN		BIT(28)
> > +
> > +#define REG_DIG_LN_TRX_B0		0x30b0
> > +#define  XTP_LN_FRC_TX_MACCK_EN		BIT(5)
> > +#define  XTP_LN_TX_MACCK_EN		BIT(4)
> > +
> > +#define REG_ANA_GLB_D0			0x90d0
> > +#define  XTP_GLB_USXGMII_SEL_MASK	GENMASK(3, 1)
> > +#define  XTP_GLB_USXGMII_SEL(x)		FIELD_PREP(GENMASK(3, 1), (x))
> > +#define  XTP_GLB_USXGMII_EN		BIT(0)
> > +
> > +struct mtk_xfi_tphy {
> > +	void __iomem		*base;
> > +	struct device		*dev;
> > +	struct reset_control	*reset;
> > +	struct clk_bulk_data	clocks[MTK_XFI_TPHY_NUM_CLOCKS];
> > +	bool			da_war;
> > +};
> > +
> > +static void mtk_xfi_tphy_write(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
> > +			       u32 value)
> > +{
> > +	iowrite32(value, xfi_tphy->base + reg);
> > +}
> > +
> > +static void mtk_xfi_tphy_rmw(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
> > +			     u32 clr, u32 set)
> > +{
> > +	u32 val;
> > +
> > +	val = ioread32(xfi_tphy->base + reg);
> > +	val &= ~clr;
> > +	val |= set;
> > +	iowrite32(val, xfi_tphy->base + reg);
> > +}
> > +
> > +static void mtk_xfi_tphy_set(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
> > +			     u32 set)
> > +{
> > +	mtk_xfi_tphy_rmw(xfi_tphy, reg, 0, set);
> > +}
> > +
> > +static void mtk_xfi_tphy_clear(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
> > +			       u32 clr)
> > +{
> > +	mtk_xfi_tphy_rmw(xfi_tphy, reg, clr, 0);
> > +}
> > +
> > +static void mtk_xfi_tphy_setup(struct mtk_xfi_tphy *xfi_tphy,
> > +			       phy_interface_t interface)
> > +{
> > +	bool is_2p5g = (interface == PHY_INTERFACE_MODE_2500BASEX);
> > +	bool is_1g = (interface == PHY_INTERFACE_MODE_1000BASEX ||
> > +		      interface == PHY_INTERFACE_MODE_SGMII);
> > +	bool is_10g = (interface == PHY_INTERFACE_MODE_10GBASER ||
> > +		       interface == PHY_INTERFACE_MODE_USXGMII);
> > +	bool is_5g = (interface == PHY_INTERFACE_MODE_5GBASER);
> > +	bool is_xgmii = (is_10g || is_5g);
> 
> is_usxgmii, I'd say.

This boolean is mostly used to destinguish which path to use at the
T-junction of the PHY, ie. up to 2500Base-X goes to LynxI PCS while
5GBase-R, 10GBase-R and USXGMII go to USXGMII PCS (which handles all
three modes and not just USXGMII).

Calling the boolean 'is_usxgmii' is a bit confusing as it not only
means (interface == interface == PHY_INTERFACE_MODE_USXGMII) but
rather also covers (interface == PHY_INTERFACE_MODE_10GBASER) as well
as (interface == PHY_INTERFACE_MODE_5GBASER) which are not USXGMII.

Maybe we invert it and call it 'use_lynxi_pcs' instead?

> 
> > +
> > +	dev_dbg(xfi_tphy->dev, "setting up for mode %s\n", phy_modes(interface));
> > +
> > +	/* Setup PLL setting */
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x9024, 0x100000, is_10g ? 0x0 : 0x100000);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x2020, 0x202000, is_5g ? 0x202000 : 0x0);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x2030, 0x500, is_1g ? 0x0 : 0x500);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x2034, 0xa00, is_1g ? 0x0 : 0xa00);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x2040, 0x340000, is_1g ? 0x200000 :
> > +							     0x140000);
> > +
> > +	/* Setup RXFE BW setting */
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50f0, 0xc10, is_1g ? 0x410 :
> > +							  is_5g ? 0x800 : 0x400);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50e0, 0x4000, is_5g ? 0x0 : 0x4000);
> > +
> > +	/* Setup RX CDR setting */
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x506c, 0x30000, is_5g ? 0x0 : 0x30000);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5070, 0x670000, is_5g ? 0x620000 : 0x50000);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5074, 0x180000, is_5g ? 0x180000 : 0x0);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5078, 0xf000400, is_5g ? 0x8000000 :
> > +							      0x7000400);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x507c, 0x5000500, is_5g ? 0x4000400 :
> > +							      0x1000100);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5080, 0x1410, is_1g ? 0x400 :
> > +							   is_5g ? 0x1010 : 0x0);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5084, 0x30300, is_1g ? 0x30300 :
> > +							    is_5g ? 0x30100 :
> > +								    0x100);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5088, 0x60200, is_1g ? 0x20200 :
> > +							 is_5g ? 0x40000 :
> > +								 0x20000);
> > +
> > +	/* Setting RXFE adaptation range setting */
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50e4, 0xc0000, is_5g ? 0x0 : 0xc0000);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50e8, 0x40000, is_5g ? 0x0 : 0x40000);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50ec, 0xa00, is_1g ? 0x200 : 0x800);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50a8, 0xee0000, is_5g ? 0x800000 :
> > +							     0x6e0000);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x6004, 0x190000, is_5g ? 0x0 : 0x190000);
> > +	if (is_10g)
> > +		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x01423342);
> > +	else if (is_5g)
> > +		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x00a132a1);
> > +	else if (is_2p5g)
> > +		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x009c329c);
> > +	else
> > +		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x00fa32fa);
> > +
> > +	/* Force SGDT_OUT off and select PCS */
> > +	mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_F4,
> > +			 XFI_DPHY_AD_SGDT_FRC_EN | XFI_DPHY_PCS_SEL,
> > +			 XFI_DPHY_AD_SGDT_FRC_EN |
> > +			 (is_xgmii ? XFI_DPHY_PCS_SEL_USXGMII :
> > +				     XFI_DPHY_PCS_SEL_SGMII));
> > +
> > +
> > +	/* Force GLB_CKDET_OUT */
> > +	mtk_xfi_tphy_set(xfi_tphy, 0x0030, 0xc00);
> > +
> > +	/* Force AEQ on */
> > +	mtk_xfi_tphy_write(xfi_tphy, REG_DIG_GLB_70,
> > +			   XTP_PCS_RX_EQ_IN_PROGRESS(2) |
> > +			   XTP_PCS_PWD_SYNC(2) |
> > +			   XTP_PCS_PWD_ASYNC(2));
> > +
> > +	usleep_range(1, 5);
> > +
> > +	/* Setup TX DA default value */
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x30b0, 0x30, 0x20);
> > +	mtk_xfi_tphy_write(xfi_tphy, 0x3028, 0x00008a01);
> > +	mtk_xfi_tphy_write(xfi_tphy, 0x302c, 0x0000a884);
> > +	mtk_xfi_tphy_write(xfi_tphy, 0x3024, 0x00083002);
> > +
> > +	/* Setup RG default value */
> > +	if (is_xgmii) {
> > +		mtk_xfi_tphy_write(xfi_tphy, 0x3010, 0x00022220);
> > +		mtk_xfi_tphy_write(xfi_tphy, 0x5064, 0x0f020a01);
> > +		mtk_xfi_tphy_write(xfi_tphy, 0x50b4, 0x06100600);
> > +		if (interface == PHY_INTERFACE_MODE_USXGMII)
> > +			mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x40704000);
> > +		else
> > +			mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x47684100);
> > +	} else {
> > +		mtk_xfi_tphy_write(xfi_tphy, 0x3010, 0x00011110);
> > +		mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x40704000);
> > +	}
> > +
> > +	if (is_1g)
> > +		mtk_xfi_tphy_write(xfi_tphy, 0x3064, 0x0000c000);
> > +
> > +	/* Setup RX EQ initial value */
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x3050, 0xa8000000,
> > +			 (interface != PHY_INTERFACE_MODE_10GBASER) ?
> > +			  0xa8000000 : 0x0);
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0x3054, 0xaa,
> > +			 (interface != PHY_INTERFACE_MODE_10GBASER) ?
> > +			  0xaa : 0x0);
> > +
> > +	if (is_xgmii)
> > +		mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x00000f00);
> > +	else if (is_2p5g)
> > +		mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x22000f00);
> > +	else
> > +		mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x20200f00);
> > +
> > +	if (interface == PHY_INTERFACE_MODE_10GBASER && xfi_tphy->da_war)
> > +		mtk_xfi_tphy_rmw(xfi_tphy, 0xa008, 0x10000, 0x10000);
> > +
> > +	mtk_xfi_tphy_rmw(xfi_tphy, 0xa060, 0x50000, is_xgmii ? 0x40000 :
> > +							       0x50000);
> > +
> > +	/* Setup PHYA speed */
> > +	mtk_xfi_tphy_rmw(xfi_tphy, REG_ANA_GLB_D0,
> > +			 XTP_GLB_USXGMII_SEL_MASK | XTP_GLB_USXGMII_EN,
> > +			 is_10g ?  XTP_GLB_USXGMII_SEL(0) :
> > +			 is_5g ?   XTP_GLB_USXGMII_SEL(1) :
> > +			 is_2p5g ? XTP_GLB_USXGMII_SEL(2) :
> > +				   XTP_GLB_USXGMII_SEL(3));
> > +	mtk_xfi_tphy_set(xfi_tphy, REG_ANA_GLB_D0, XTP_GLB_USXGMII_EN);
> > +
> > +	/* Release reset */
> > +	mtk_xfi_tphy_set(xfi_tphy, REG_DIG_GLB_70,
> > +			 XTP_PCS_RST_B | XTP_FRC_PCS_RST_B);
> > +	usleep_range(150, 500);
> > +
> > +	/* Switch to P0 */
> > +	mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
> > +			 XTP_PCS_PWD_SYNC_MASK |
> > +			 XTP_PCS_PWD_ASYNC_MASK,
> > +			 XTP_FRC_PCS_PWD_ASYNC |
> > +			 XTP_PCS_UPDT | XTP_PCS_IN_FR_RG);
> > +	usleep_range(1, 5);
> > +
> > +	mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_70, XTP_PCS_UPDT);
> > +	usleep_range(15, 50);
> > +
> > +	if (is_xgmii) {
> > +		/* Switch to Gen3 */
> > +		mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
> > +				 XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
> > +				 XTP_PCS_MODE(2) | XTP_PCS_UPDT);
> > +	} else {
> > +		/* Switch to Gen2 */
> > +		mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
> > +				 XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
> > +				 XTP_PCS_MODE(1) | XTP_PCS_UPDT);
> > +	}
> > +	usleep_range(1, 5);
> > +
> > +	mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_70, XTP_PCS_UPDT);
> > +
> > +	usleep_range(100, 500);
> > +
> > +	/* Enable MAC CK */
> > +	mtk_xfi_tphy_set(xfi_tphy, REG_DIG_LN_TRX_B0, XTP_LN_TX_MACCK_EN);
> > +	mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_F4, XFI_DPHY_AD_SGDT_FRC_EN);
> > +
> > +	/* Enable TX data */
> > +	mtk_xfi_tphy_set(xfi_tphy, REG_DIG_LN_TRX_40,
> > +			 XTP_LN_FRC_TX_DATA_EN | XTP_LN_TX_DATA_EN);
> > +	usleep_range(400, 1000);
> > +}
> > +
> > +static int mtk_xfi_tphy_set_mode(struct phy *phy, enum phy_mode mode, int
> > +				 submode)
> > +{
> > +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> > +
> > +	if (mode != PHY_MODE_ETHERNET)
> > +		return -EINVAL;
> > +
> > +	switch (submode) {
> > +	case PHY_INTERFACE_MODE_1000BASEX:
> 
> fallthrough;

I don't think so. No other kernel drivers are doing that and also there
is no compiler warning what-so-ever when matching multiple cases in that
way.

> 
> > +	case PHY_INTERFACE_MODE_2500BASEX:
> 
> fallthrough;
> 
> > +	case PHY_INTERFACE_MODE_SGMII:
> 
> ... etc :-)
> 
> > +	case PHY_INTERFACE_MODE_5GBASER:
> > +	case PHY_INTERFACE_MODE_10GBASER:
> > +	case PHY_INTERFACE_MODE_USXGMII:
> 
> Does this PHY support PHY_INTERFACE_MODE_XGMII ?
> 
> > +		mtk_xfi_tphy_setup(xfi_tphy, submode);
> > +		return 0;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +}
> > +
> > +static int mtk_xfi_tphy_reset(struct phy *phy)
> > +{
> > +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> > +
> > +	reset_control_assert(xfi_tphy->reset);
> > +	usleep_range(100, 500);
> > +	reset_control_deassert(xfi_tphy->reset);
> > +	usleep_range(1, 10);
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtk_xfi_tphy_power_on(struct phy *phy)
> > +{
> > +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> > +
> > +	return clk_bulk_prepare_enable(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
> > +}
> > +
> > +static int mtk_xfi_tphy_power_off(struct phy *phy)
> > +{
> > +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> > +
> > +	clk_bulk_disable_unprepare(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct phy_ops mtk_xfi_tphy_ops = {
> > +	.power_on	= mtk_xfi_tphy_power_on,
> > +	.power_off	= mtk_xfi_tphy_power_off,
> > +	.set_mode	= mtk_xfi_tphy_set_mode,
> > +	.reset		= mtk_xfi_tphy_reset,
> > +	.owner		= THIS_MODULE,
> > +};
> > +
> > +static int mtk_xfi_tphy_probe(struct platform_device *pdev)
> > +{
> > +	struct device_node *np = pdev->dev.of_node;
> > +	struct phy_provider *phy_provider;
> > +	struct mtk_xfi_tphy *xfi_tphy;
> > +	struct phy *phy;
> > +
> > +	if (!np)
> > +		return -ENODEV;
> > +
> > +	xfi_tphy = devm_kzalloc(&pdev->dev, sizeof(*xfi_tphy), GFP_KERNEL);
> > +	if (!xfi_tphy)
> > +		return -ENOMEM;
> > +
> > +	xfi_tphy->base = devm_of_iomap(&pdev->dev, np, 0, NULL);
> 
> Why devm_of_iomap() and not devm_platform_ioremap_resource()?

True, I will make use of devm_platform_ioremap_resource().

> 
> > +	if (!xfi_tphy->base)
> > +		return -EIO;
> > +
> > +	xfi_tphy->dev = &pdev->dev;
> > +
> > +	xfi_tphy->clocks[0].id = "topxtal";
> 
> xfi_tphy->clocks[0].id = "topxtal";
> xfi_tphy->clocks[1].id = "xfipll";
> 
> ret = devm_clk_bulk_get(&pdev->dev, MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
> if (ret)
> 	return ret;
> 
> ...it's that simple :-P

Ok, thanks :)

> 
> > +	xfi_tphy->clocks[0].clk = devm_clk_get(&pdev->dev, xfi_tphy->clocks[0].id);
> > +	if (IS_ERR(xfi_tphy->clocks[0].clk))
> > +		return PTR_ERR(xfi_tphy->clocks[0].clk);
> > +
> > +	xfi_tphy->clocks[1].id = "xfipll";
> > +	xfi_tphy->clocks[1].clk = devm_clk_get(&pdev->dev, xfi_tphy->clocks[1].id);
> > +	if (IS_ERR(xfi_tphy->clocks[1].clk))
> > +		return PTR_ERR(xfi_tphy->clocks[1].clk);
> > +
> > +	xfi_tphy->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
> > +	if (IS_ERR(xfi_tphy->reset))
> > +		return PTR_ERR(xfi_tphy->reset);
> > +
> > +	xfi_tphy->da_war = of_property_read_bool(np,
> > +						 "mediatek,usxgmii-performance-errata");
> > +
> 
> One line please
> 
> > +	phy = devm_phy_create(&pdev->dev, NULL, &mtk_xfi_tphy_ops);
> > +	if (IS_ERR(phy))
> > +		return PTR_ERR(phy);
> > +
> > +	phy_set_drvdata(phy, xfi_tphy);
> > +
> > +	phy_provider = devm_of_phy_provider_register(&pdev->dev,
> > +						     of_phy_simple_xlate);
> 
> ditto
> 
> > +
> > +	return PTR_ERR_OR_ZERO(phy_provider);
> > +}
> > +
> > +static const struct of_device_id mtk_xfi_tphy_match[] = {
> > +	{ .compatible = "mediatek,mt7988-xfi-tphy", },
> > +	{ }
> 
> 	{ .compatible = "mediatek,mt7988-xfi-tphy" },
> 	{ /* sentinel */ }

Ack.

> 
> > +};
> > +MODULE_DEVICE_TABLE(of, mtk_xfi_tphy_match);
> > +
> > +static struct platform_driver mtk_xfi_tphy_driver = {
> > +	.probe = mtk_xfi_tphy_probe,
> > +	.driver = {
> > +		.name = "mtk-xfi-tphy",
> > +		.of_match_table = mtk_xfi_tphy_match,
> > +	},
> > +};
> > +module_platform_driver(mtk_xfi_tphy_driver);
> > +
> > +MODULE_DESCRIPTION("MediaTek XFI T-PHY driver");
> 
> MODULE_DESCRIPTION("MediaTek 10GE SerDes XFI T-PHY driver");

Ack, will unify the name everywhere.

> 
> Cheers,
> Angelo
> 
> > +MODULE_AUTHOR("Daniel Golle <daniel@makrotopia.org>");
> > +MODULE_AUTHOR("Bc-bocun Chen <bc-bocun.chen@mediatek.com>");
> > +MODULE_LICENSE("GPL");
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: phy: mediatek,xfi-tphy: add new bindings
  2024-02-01 21:52 [PATCH 1/2] dt-bindings: phy: mediatek,xfi-tphy: add new bindings Daniel Golle
  2024-02-01 21:53 ` [PATCH 2/2] phy: add driver for MediaTek XFI T-PHY Daniel Golle
  2024-02-02  8:21 ` [PATCH 1/2] dt-bindings: phy: mediatek,xfi-tphy: add new bindings Krzysztof Kozlowski
@ 2024-02-02 21:24 ` Rob Herring
  2024-02-02 21:36   ` Daniel Golle
  2 siblings, 1 reply; 12+ messages in thread
From: Rob Herring @ 2024-02-02 21:24 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Bc-bocun Chen, Steven Liu, John Crispin, Chunfeng Yun,
	Vinod Koul, Kishon Vijay Abraham I, Krzysztof Kozlowski,
	Conor Dooley, Qingfang Deng, SkyLake Huang, Matthias Brugger,
	AngeloGioacchino Del Regno, Philipp Zabel, linux-arm-kernel,
	linux-mediatek, linux-phy, devicetree, linux-kernel, netdev

On Thu, Feb 01, 2024 at 09:52:20PM +0000, Daniel Golle wrote:
> Add bindings for the MediaTek XFI T-PHY Ethernet SerDes PHY found in the
> MediaTek MT7988 SoC which can operate at various interfaces modes:

This is v4 unless I'm confused[1]. Where's the revision history?

Rob

[1] https://lore.kernel.org/all/b875f693f6d4367a610a12ef324584f3bf3a1c1c.1702352117.git.daniel@makrotopia.org/

> 
> via USXGMII PCS:
>  * USXGMII
>  * 10GBase-R
>  * 5GBase-R
> 
> via LynxI SGMII PCS:
>  * 2500Base-X
>  * 1000Base-X
>  * Cisco SGMII (MAC side)
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  .../bindings/phy/mediatek,xfi-tphy.yaml       | 80 +++++++++++++++++++
>  1 file changed, 80 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
> 
> diff --git a/Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
> new file mode 100644
> index 0000000000000..e897118dcf7e6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/phy/mediatek,xfi-tphy.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: MediaTek XFI T-PHY
> +
> +maintainers:
> +  - Daniel Golle <daniel@makrotopia.org>
> +
> +description:
> +  The MediaTek XFI SerDes T-PHY provides the physical SerDes lanes
> +  used by the (10G/5G) USXGMII PCS and (1G/2.5G) LynxI PCS found in
> +  MediaTek's 10G-capabale SoCs.
> +
> +properties:
> +  $nodename:
> +    pattern: "^phy@[0-9a-f]+$"
> +
> +  compatible:
> +    const: mediatek,mt7988-xfi-tphy
> +
> +  reg:
> +    maxItems: 1
> +
> +  clocks:
> +    items:
> +      - description: XFI PHY clock
> +      - description: XFI register clock
> +
> +  clock-names:
> +    items:
> +      - const: xfipll
> +      - const: topxtal
> +
> +  resets:
> +    items:
> +      - description: PEXTP reset

What is PEXTP?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: phy: mediatek,xfi-tphy: add new bindings
  2024-02-02 21:24 ` Rob Herring
@ 2024-02-02 21:36   ` Daniel Golle
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Golle @ 2024-02-02 21:36 UTC (permalink / raw)
  To: Rob Herring
  Cc: Bc-bocun Chen, Steven Liu, John Crispin, Chunfeng Yun,
	Vinod Koul, Kishon Vijay Abraham I, Krzysztof Kozlowski,
	Conor Dooley, Qingfang Deng, SkyLake Huang, Matthias Brugger,
	AngeloGioacchino Del Regno, Philipp Zabel, linux-arm-kernel,
	linux-mediatek, linux-phy, devicetree, linux-kernel, netdev

On Fri, Feb 02, 2024 at 03:24:20PM -0600, Rob Herring wrote:
> On Thu, Feb 01, 2024 at 09:52:20PM +0000, Daniel Golle wrote:
> > Add bindings for the MediaTek XFI T-PHY Ethernet SerDes PHY found in the
> > MediaTek MT7988 SoC which can operate at various interfaces modes:
> 
> This is v4 unless I'm confused[1]. Where's the revision history?

It's a new series only covering the XFI T-PHY driver.
However, I should have written a cover letter referencing the previous
series to netdev. Sorry for that and thank you for pointing it out.

> 
> Rob
> 
> [1] https://lore.kernel.org/all/b875f693f6d4367a610a12ef324584f3bf3a1c1c.1702352117.git.daniel@makrotopia.org/
> 
> > 
> > via USXGMII PCS:
> >  * USXGMII
> >  * 10GBase-R
> >  * 5GBase-R
> > 
> > via LynxI SGMII PCS:
> >  * 2500Base-X
> >  * 1000Base-X
> >  * Cisco SGMII (MAC side)
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >  .../bindings/phy/mediatek,xfi-tphy.yaml       | 80 +++++++++++++++++++
> >  1 file changed, 80 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
> > new file mode 100644
> > index 0000000000000..e897118dcf7e6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
> > @@ -0,0 +1,80 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/mediatek,xfi-tphy.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek XFI T-PHY
> > +
> > +maintainers:
> > +  - Daniel Golle <daniel@makrotopia.org>
> > +
> > +description:
> > +  The MediaTek XFI SerDes T-PHY provides the physical SerDes lanes
> > +  used by the (10G/5G) USXGMII PCS and (1G/2.5G) LynxI PCS found in
> > +  MediaTek's 10G-capabale SoCs.
> > +
> > +properties:
> > +  $nodename:
> > +    pattern: "^phy@[0-9a-f]+$"
> > +
> > +  compatible:
> > +    const: mediatek,mt7988-xfi-tphy
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: XFI PHY clock
> > +      - description: XFI register clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: xfipll
> > +      - const: topxtal
> > +
> > +  resets:
> > +    items:
> > +      - description: PEXTP reset
> 
> What is PEXTP?
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: phy: mediatek,xfi-tphy: add new bindings
  2024-02-02  8:21 ` [PATCH 1/2] dt-bindings: phy: mediatek,xfi-tphy: add new bindings Krzysztof Kozlowski
@ 2024-02-04  6:17   ` Chunfeng Yun (云春峰)
  2024-02-05  8:08     ` Krzysztof Kozlowski
  0 siblings, 1 reply; 12+ messages in thread
From: Chunfeng Yun (云春峰) @ 2024-02-04  6:17 UTC (permalink / raw)
  To: netdev, linux-mediatek, linux-kernel,
	SkyLake Huang (黃啟澤),
	kishon, devicetree, john, krzysztof.kozlowski, p.zabel, conor+dt,
	robh, Bc-bocun Chen (陳柏村),
	linux-arm-kernel, Steven Liu (劉人豪),
	vkoul, matthias.bgg, krzysztof.kozlowski+dt, daniel, linux-phy,
	dqfext, angelogioacchino.delregno

On Fri, 2024-02-02 at 09:21 +0100, Krzysztof Kozlowski wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On 01/02/2024 22:52, Daniel Golle wrote:
> > Add bindings for the MediaTek XFI T-PHY Ethernet SerDes PHY found
> in the
> > MediaTek MT7988 SoC which can operate at various interfaces modes:
> > 
> > via USXGMII PCS:
> >  * USXGMII
> >  * 10GBase-R
> >  * 5GBase-R
> > 
> > via LynxI SGMII PCS:
> >  * 2500Base-X
> >  * 1000Base-X
> >  * Cisco SGMII (MAC side)
> > 
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > ---
> >  .../bindings/phy/mediatek,xfi-tphy.yaml       | 80
> +++++++++++++++++++
> >  1 file changed, 80 insertions(+)
> >  create mode 100644
> Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/phy/mediatek,xfi-
> tphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,xfi-
> tphy.yaml
> > new file mode 100644
> > index 0000000000000..e897118dcf7e6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
> > @@ -0,0 +1,80 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/phy/mediatek,xfi-tphy.yaml#
> 
> Please use compatible as filename. Your binding says only one is
> possible (const, not enum), so there is no reasoning for different
> filename.
> 
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: MediaTek XFI T-PHY
> > +
> > +maintainers:
> > +  - Daniel Golle <daniel@makrotopia.org>
> > +
> > +description:
> > +  The MediaTek XFI SerDes T-PHY provides the physical SerDes lanes
> > +  used by the (10G/5G) USXGMII PCS and (1G/2.5G) LynxI PCS found
> in
> > +  MediaTek's 10G-capabale SoCs.
> > +
> > +properties:
> > +  $nodename:
> > +    pattern: "^phy@[0-9a-f]+$"
> 
> No need for nodename in individual bindings file.
> 
> > +
> > +  compatible:
> > +    const: mediatek,mt7988-xfi-tphy
Add a generic compatible "mediatek,xfi-tphy"?

Other socs also use this phy but not upstream.

> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  clocks:
> > +    items:
> > +      - description: XFI PHY clock
> > +      - description: XFI register clock
> > +
> > +  clock-names:
> > +    items:
> > +      - const: xfipll
> > +      - const: topxtal
> > +
> > +  resets:
> > +    items:
> > +      - description: PEXTP reset
> > +
> > +  mediatek,usxgmii-performance-errata:
> > +    $ref: /schemas/types.yaml#/definitions/flag
> > +    description:
> > +      One instance of the T-PHY on MT7988 suffers from a
> performance
> > +      problem in 10GBase-R mode which needs a work-around in the
> driver.
> 
> Can you explain what is this issue and errata about (except
> performance)?
> 
> > +      The work-around is enabled using this flag.
> > +
> > +  "#phy-cells":
> > +    const: 0
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - clocks
> > +  - clock-names
> > +  - resets
> > +  - "#phy-cells"
> > +
> > +additionalProperties: false
> 
> 
> Best regards,
> Krzysztof
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] phy: add driver for MediaTek XFI T-PHY
  2024-02-01 21:53 ` [PATCH 2/2] phy: add driver for MediaTek XFI T-PHY Daniel Golle
  2024-02-02 13:21   ` AngeloGioacchino Del Regno
@ 2024-02-04  6:29   ` Chunfeng Yun (云春峰)
  2024-02-05 15:29     ` AngeloGioacchino Del Regno
  1 sibling, 1 reply; 12+ messages in thread
From: Chunfeng Yun (云春峰) @ 2024-02-04  6:29 UTC (permalink / raw)
  To: netdev, linux-mediatek, linux-kernel,
	SkyLake Huang (黃啟澤),
	kishon, devicetree, p.zabel, conor+dt, robh,
	Bc-bocun Chen (陳柏村),
	linux-arm-kernel, krzysztof.kozlowski+dt, vkoul, matthias.bgg,
	dqfext, daniel, linux-phy, angelogioacchino.delregno

On Thu, 2024-02-01 at 21:53 +0000, Daniel Golle wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  Add driver for MediaTek's XFI T-PHY, 10 Gigabit/s Ethernet SerDes
> PHY
> which can be found in the MT7988 SoC.
> 
> The PHY can operates only in PHY_MODE_ETHERNET, the submode is one of
> PHY_INTERFACE_MODE_* corresponding to the supported modes:
> 
>  * USXGMII                 \
>  * 10GBase-R                }- USXGMII PCS - XGDM  \
>  * 5GBase-R                /                        \
>                                                      }- Ethernet MAC
>  * 2500Base-X              \                        /
>  * 1000Base-X               }- LynxI PCS - GDM     /
>  * Cisco SGMII (MAC side)  /
> 
> In order to work-around a performance issue present on the first of
> two XFI T-PHYs present in MT7988, special tuning is applied which can
> be
> selected by adding the 'mediatek,usxgmii-performance-errata' property
> to
> the device tree node.
> 
> There is no documentation for most registers used for the
> analog/tuning part, however, most of the registers have been
> partially
> reverse-engineered from MediaTek's SDK implementation (an opaque
> sequence of 32-bit register writes) and descriptions for all relevant
> digital registers and bits such as resets and muxes have been
> supplied
> by MediaTek.
> 
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> ---
>  MAINTAINERS                             |   1 +
>  drivers/phy/mediatek/Kconfig            |  12 +
>  drivers/phy/mediatek/Makefile           |   1 +
>  drivers/phy/mediatek/phy-mtk-xfi-tphy.c | 392
> ++++++++++++++++++++++++
>  4 files changed, 406 insertions(+)
>  create mode 100644 drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 52769631bdb1a..52e4192470bd9 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -13715,6 +13715,7 @@ L:netdev@vger.kernel.org
>  S:Maintained
>  F:drivers/net/phy/mediatek-ge-soc.c
>  F:drivers/net/phy/mediatek-ge.c
> +F:drivers/phy/mediatek/phy-mtk-xfi-tphy.c
>  
>  MEDIATEK I2C CONTROLLER DRIVER
>  M:Qii Wang <qii.wang@mediatek.com>
> diff --git a/drivers/phy/mediatek/Kconfig
> b/drivers/phy/mediatek/Kconfig
> index 3125ecb5d119f..5161d130c7f8b 100644
> --- a/drivers/phy/mediatek/Kconfig
> +++ b/drivers/phy/mediatek/Kconfig
> @@ -13,6 +13,18 @@ config PHY_MTK_PCIE
>    callback for PCIe GEN3 port, it supports software efuse
>    initialization.
>  
> +config PHY_MTK_XFI_TPHY
> +tristate "MediaTek XFI T-PHY Driver"
> +depends on ARCH_MEDIATEK || COMPILE_TEST
> +depends on OF && OF_ADDRESS
> +depends on HAS_IOMEM
> +select GENERIC_PHY
> +help
> +  Say 'Y' here to add support for MediaTek XFI T-PHY driver.
> +  The driver provides access to the Ethernet SerDes T-PHY supporting
> +  1GE and 2.5GE modes via the LynxI PCS, and 5GE and 10GE modes
> +  via the USXGMII PCS found in MediaTek SoCs with 10G Ethernet.
> +
>  config PHY_MTK_TPHY
>  tristate "MediaTek T-PHY Driver"
>  depends on ARCH_MEDIATEK || COMPILE_TEST
> diff --git a/drivers/phy/mediatek/Makefile
> b/drivers/phy/mediatek/Makefile
> index c9a50395533eb..fa7217178e7f4 100644
> --- a/drivers/phy/mediatek/Makefile
> +++ b/drivers/phy/mediatek/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_PHY_MTK_PCIE)+= phy-mtk-pcie.o
>  obj-$(CONFIG_PHY_MTK_TPHY)+= phy-mtk-tphy.o
>  obj-$(CONFIG_PHY_MTK_UFS)+= phy-mtk-ufs.o
>  obj-$(CONFIG_PHY_MTK_XSPHY)+= phy-mtk-xsphy.o
> +obj-$(CONFIG_PHY_MTK_XFI_TPHY)+= phy-mtk-xfi-tphy.o
>  
>  phy-mtk-hdmi-drv-y:= phy-mtk-hdmi.o
>  phy-mtk-hdmi-drv-y+= phy-mtk-hdmi-mt2701.o
> diff --git a/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> new file mode 100644
> index 0000000000000..d50e6320860e5
> --- /dev/null
> +++ b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
> @@ -0,0 +1,392 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/* MediaTek 10GE SerDes PHY driver
> + *
> + * Copyright (c) 2024 Daniel Golle <daniel@makrotopia.org>
> + *                    Bc-bocun Chen <bc-bocun.chen@mediatek.com>
> + * based on mtk_usxgmii.c found in MediaTek's SDK released under
> GPL-2.0
> + * Copyright (c) 2022 MediaTek Inc.
> + * Author: Henry Yen <henry.yen@mediatek.com>
> + */
> +
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/platform_device.h>
> +#include <linux/of.h>
> +#include <linux/io.h>
> +#include <linux/clk.h>
> +#include <linux/reset.h>
> +#include <linux/phy.h>
> +#include <linux/phy/phy.h>
> +
> +#define MTK_XFI_TPHY_NUM_CLOCKS2
> +
> +#define REG_DIG_GLB_700x0070
> +#define  XTP_PCS_RX_EQ_IN_PROGRESS(x)FIELD_PREP(GENMASK(25, 24),
> (x))
> +#define  XTP_PCS_MODE_MASKGENMASK(17, 16)
> +#define  XTP_PCS_MODE(x)FIELD_PREP(GENMASK(17, 16), (x))
> +#define  XTP_PCS_RST_BBIT(15)
> +#define  XTP_FRC_PCS_RST_BBIT(14)
> +#define  XTP_PCS_PWD_SYNC_MASKGENMASK(13, 12)
> +#define  XTP_PCS_PWD_SYNC(x)FIELD_PREP(XTP_PCS_PWD_SYNC_MASK, (x))
> +#define  XTP_PCS_PWD_ASYNC_MASKGENMASK(11, 10)
> +#define  XTP_PCS_PWD_ASYNC(x)FIELD_PREP(XTP_PCS_PWD_ASYNC_MASK, (x))
> +#define  XTP_FRC_PCS_PWD_ASYNCBIT(8)
> +#define  XTP_PCS_UPDTBIT(4)
> +#define  XTP_PCS_IN_FR_RGBIT(0)
> +
> +#define REG_DIG_GLB_F40x00f4
> +#define  XFI_DPHY_PCS_SELBIT(0)
> +#define   XFI_DPHY_PCS_SEL_SGMIIFIELD_PREP(XFI_DPHY_PCS_SEL, 1)
> +#define   XFI_DPHY_PCS_SEL_USXGMIIFIELD_PREP(XFI_DPHY_PCS_SEL, 0)
> +#define  XFI_DPHY_AD_SGDT_FRC_ENBIT(5)
> +
> +#define REG_DIG_LN_TRX_400x3040
> +#define  XTP_LN_FRC_TX_DATA_ENBIT(29)
> +#define  XTP_LN_TX_DATA_ENBIT(28)
> +
> +#define REG_DIG_LN_TRX_B00x30b0
> +#define  XTP_LN_FRC_TX_MACCK_ENBIT(5)
> +#define  XTP_LN_TX_MACCK_ENBIT(4)
> +
> +#define REG_ANA_GLB_D00x90d0
> +#define  XTP_GLB_USXGMII_SEL_MASKGENMASK(3, 1)
> +#define  XTP_GLB_USXGMII_SEL(x)FIELD_PREP(GENMASK(3, 1), (x))
> +#define  XTP_GLB_USXGMII_ENBIT(0)
> +
> +struct mtk_xfi_tphy {
> +void __iomem*base;
> +struct device*dev;
> +struct reset_control*reset;
> +struct clk_bulk_dataclocks[MTK_XFI_TPHY_NUM_CLOCKS];
> +boolda_war;
> +};
> +
> +static void mtk_xfi_tphy_write(struct mtk_xfi_tphy *xfi_tphy, u16
> reg,
> +       u32 value)
> +{
> +iowrite32(value, xfi_tphy->base + reg);
> +}
> +
> +static void mtk_xfi_tphy_rmw(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
> +     u32 clr, u32 set)
> +{
> +u32 val;
> +
> +val = ioread32(xfi_tphy->base + reg);
> +val &= ~clr;
> +val |= set;
> +iowrite32(val, xfi_tphy->base + reg);
> +}
> +
> +static void mtk_xfi_tphy_set(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
> +     u32 set)
> +{
> +mtk_xfi_tphy_rmw(xfi_tphy, reg, 0, set);
> +}
> +
> +static void mtk_xfi_tphy_clear(struct mtk_xfi_tphy *xfi_tphy, u16
> reg,
> +       u32 clr)
> +{
> +mtk_xfi_tphy_rmw(xfi_tphy, reg, clr, 0);
> +}

Helpers defined in phy-mtk-io.h can be used instead?

> +
> +static void mtk_xfi_tphy_setup(struct mtk_xfi_tphy *xfi_tphy,
> +       phy_interface_t interface)
> +{
> +bool is_2p5g = (interface == PHY_INTERFACE_MODE_2500BASEX);
> +bool is_1g = (interface == PHY_INTERFACE_MODE_1000BASEX ||
> +      interface == PHY_INTERFACE_MODE_SGMII);
> +bool is_10g = (interface == PHY_INTERFACE_MODE_10GBASER ||
> +       interface == PHY_INTERFACE_MODE_USXGMII);
> +bool is_5g = (interface == PHY_INTERFACE_MODE_5GBASER);
> +bool is_xgmii = (is_10g || is_5g);
> +
> +dev_dbg(xfi_tphy->dev, "setting up for mode %s\n",
> phy_modes(interface));
> +
> +/* Setup PLL setting */
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x9024, 0x100000, is_10g ? 0x0 :
> 0x100000);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x2020, 0x202000, is_5g ? 0x202000 :
> 0x0);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x2030, 0x500, is_1g ? 0x0 : 0x500);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x2034, 0xa00, is_1g ? 0x0 : 0xa00);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x2040, 0x340000, is_1g ? 0x200000 :
> +     0x140000);
> +
> +/* Setup RXFE BW setting */
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x50f0, 0xc10, is_1g ? 0x410 :
> +  is_5g ? 0x800 : 0x400);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x50e0, 0x4000, is_5g ? 0x0 : 0x4000);
> +
> +/* Setup RX CDR setting */
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x506c, 0x30000, is_5g ? 0x0 : 0x30000);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x5070, 0x670000, is_5g ? 0x620000 :
> 0x50000);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x5074, 0x180000, is_5g ? 0x180000 :
> 0x0);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x5078, 0xf000400, is_5g ? 0x8000000 :
> +      0x7000400);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x507c, 0x5000500, is_5g ? 0x4000400 :
> +      0x1000100);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x5080, 0x1410, is_1g ? 0x400 :
> +   is_5g ? 0x1010 : 0x0);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x5084, 0x30300, is_1g ? 0x30300 :
> +    is_5g ? 0x30100 :
> +    0x100);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x5088, 0x60200, is_1g ? 0x20200 :
> + is_5g ? 0x40000 :
> + 0x20000);
> +
> +/* Setting RXFE adaptation range setting */
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x50e4, 0xc0000, is_5g ? 0x0 : 0xc0000);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x50e8, 0x40000, is_5g ? 0x0 : 0x40000);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x50ec, 0xa00, is_1g ? 0x200 : 0x800);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x50a8, 0xee0000, is_5g ? 0x800000 :
> +     0x6e0000);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x6004, 0x190000, is_5g ? 0x0 :
> 0x190000);
> +if (is_10g)
> +mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x01423342);
> +else if (is_5g)
> +mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x00a132a1);
> +else if (is_2p5g)
> +mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x009c329c);
> +else
> +mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x00fa32fa);
Still have many magic number.

> +
> +/* Force SGDT_OUT off and select PCS */
> +mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_F4,
> + XFI_DPHY_AD_SGDT_FRC_EN | XFI_DPHY_PCS_SEL,
> + XFI_DPHY_AD_SGDT_FRC_EN |
> + (is_xgmii ? XFI_DPHY_PCS_SEL_USXGMII :
> +     XFI_DPHY_PCS_SEL_SGMII));
> +
> +
> +/* Force GLB_CKDET_OUT */
> +mtk_xfi_tphy_set(xfi_tphy, 0x0030, 0xc00);
> +
> +/* Force AEQ on */
> +mtk_xfi_tphy_write(xfi_tphy, REG_DIG_GLB_70,
> +   XTP_PCS_RX_EQ_IN_PROGRESS(2) |
> +   XTP_PCS_PWD_SYNC(2) |
> +   XTP_PCS_PWD_ASYNC(2));
> +
> +usleep_range(1, 5);
> +
> +/* Setup TX DA default value */
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x30b0, 0x30, 0x20);
> +mtk_xfi_tphy_write(xfi_tphy, 0x3028, 0x00008a01);
> +mtk_xfi_tphy_write(xfi_tphy, 0x302c, 0x0000a884);
> +mtk_xfi_tphy_write(xfi_tphy, 0x3024, 0x00083002);
> +
> +/* Setup RG default value */
> +if (is_xgmii) {
> +mtk_xfi_tphy_write(xfi_tphy, 0x3010, 0x00022220);
> +mtk_xfi_tphy_write(xfi_tphy, 0x5064, 0x0f020a01);
> +mtk_xfi_tphy_write(xfi_tphy, 0x50b4, 0x06100600);
> +if (interface == PHY_INTERFACE_MODE_USXGMII)
> +mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x40704000);
> +else
> +mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x47684100);
> +} else {
> +mtk_xfi_tphy_write(xfi_tphy, 0x3010, 0x00011110);
> +mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x40704000);
> +}
> +
> +if (is_1g)
> +mtk_xfi_tphy_write(xfi_tphy, 0x3064, 0x0000c000);
> +
> +/* Setup RX EQ initial value */
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x3050, 0xa8000000,
> + (interface != PHY_INTERFACE_MODE_10GBASER) ?
> +  0xa8000000 : 0x0);
> +mtk_xfi_tphy_rmw(xfi_tphy, 0x3054, 0xaa,
> + (interface != PHY_INTERFACE_MODE_10GBASER) ?
> +  0xaa : 0x0);
> +
> +if (is_xgmii)
> +mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x00000f00);
> +else if (is_2p5g)
> +mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x22000f00);
> +else
> +mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x20200f00);
> +
> +if (interface == PHY_INTERFACE_MODE_10GBASER && xfi_tphy->da_war)
> +mtk_xfi_tphy_rmw(xfi_tphy, 0xa008, 0x10000, 0x10000);
> +
> +mtk_xfi_tphy_rmw(xfi_tphy, 0xa060, 0x50000, is_xgmii ? 0x40000 :
> +       0x50000);
> +
> +/* Setup PHYA speed */
> +mtk_xfi_tphy_rmw(xfi_tphy, REG_ANA_GLB_D0,
> + XTP_GLB_USXGMII_SEL_MASK | XTP_GLB_USXGMII_EN,
> + is_10g ?  XTP_GLB_USXGMII_SEL(0) :
> + is_5g ?   XTP_GLB_USXGMII_SEL(1) :
> + is_2p5g ? XTP_GLB_USXGMII_SEL(2) :
> +   XTP_GLB_USXGMII_SEL(3));
> +mtk_xfi_tphy_set(xfi_tphy, REG_ANA_GLB_D0, XTP_GLB_USXGMII_EN);
> +
> +/* Release reset */
> +mtk_xfi_tphy_set(xfi_tphy, REG_DIG_GLB_70,
> + XTP_PCS_RST_B | XTP_FRC_PCS_RST_B);
> +usleep_range(150, 500);
> +
> +/* Switch to P0 */
> +mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
> + XTP_PCS_PWD_SYNC_MASK |
> + XTP_PCS_PWD_ASYNC_MASK,
> + XTP_FRC_PCS_PWD_ASYNC |
> + XTP_PCS_UPDT | XTP_PCS_IN_FR_RG);
> +usleep_range(1, 5);
> +
> +mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_70, XTP_PCS_UPDT);
> +usleep_range(15, 50);
> +
> +if (is_xgmii) {
> +/* Switch to Gen3 */
> +mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
> + XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
> + XTP_PCS_MODE(2) | XTP_PCS_UPDT);
> +} else {
> +/* Switch to Gen2 */
> +mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
> + XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
> + XTP_PCS_MODE(1) | XTP_PCS_UPDT);
> +}
> +usleep_range(1, 5);
> +
> +mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_70, XTP_PCS_UPDT);
> +
> +usleep_range(100, 500);
> +
> +/* Enable MAC CK */
> +mtk_xfi_tphy_set(xfi_tphy, REG_DIG_LN_TRX_B0, XTP_LN_TX_MACCK_EN);
> +mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_F4,
> XFI_DPHY_AD_SGDT_FRC_EN);
> +
> +/* Enable TX data */
> +mtk_xfi_tphy_set(xfi_tphy, REG_DIG_LN_TRX_40,
> + XTP_LN_FRC_TX_DATA_EN | XTP_LN_TX_DATA_EN);
> +usleep_range(400, 1000);
> +}
> +
> +static int mtk_xfi_tphy_set_mode(struct phy *phy, enum phy_mode
> mode, int
> + submode)
> +{
> +struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> +
> +if (mode != PHY_MODE_ETHERNET)
> +return -EINVAL;
> +
> +switch (submode) {
> +case PHY_INTERFACE_MODE_1000BASEX:
> +case PHY_INTERFACE_MODE_2500BASEX:
> +case PHY_INTERFACE_MODE_SGMII:
> +case PHY_INTERFACE_MODE_5GBASER:
> +case PHY_INTERFACE_MODE_10GBASER:
> +case PHY_INTERFACE_MODE_USXGMII:
> +mtk_xfi_tphy_setup(xfi_tphy, submode);
> +return 0;
> +default:
> +return -EINVAL;
> +}
> +}
> +
> +static int mtk_xfi_tphy_reset(struct phy *phy)
> +{
> +struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> +
> +reset_control_assert(xfi_tphy->reset);
> +usleep_range(100, 500);
> +reset_control_deassert(xfi_tphy->reset);
> +usleep_range(1, 10);
> +
> +return 0;
> +}
> +
> +static int mtk_xfi_tphy_power_on(struct phy *phy)
> +{
> +struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> +
> +return clk_bulk_prepare_enable(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy-
> >clocks);
> +}
> +
> +static int mtk_xfi_tphy_power_off(struct phy *phy)
> +{
> +struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
> +
> +clk_bulk_disable_unprepare(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy-
> >clocks);
> +
> +return 0;
> +}
> +
> +static const struct phy_ops mtk_xfi_tphy_ops = {
> +.power_on= mtk_xfi_tphy_power_on,
> +.power_off= mtk_xfi_tphy_power_off,
> +.set_mode= mtk_xfi_tphy_set_mode,
> +.reset= mtk_xfi_tphy_reset,
> +.owner= THIS_MODULE,
> +};
> +
> +static int mtk_xfi_tphy_probe(struct platform_device *pdev)
> +{
> +struct device_node *np = pdev->dev.of_node;
> +struct phy_provider *phy_provider;
> +struct mtk_xfi_tphy *xfi_tphy;
> +struct phy *phy;
> +
> +if (!np)
> +return -ENODEV;
> +
> +xfi_tphy = devm_kzalloc(&pdev->dev, sizeof(*xfi_tphy), GFP_KERNEL);
> +if (!xfi_tphy)
> +return -ENOMEM;
> +
> +xfi_tphy->base = devm_of_iomap(&pdev->dev, np, 0, NULL);
> +if (!xfi_tphy->base)
> +return -EIO;
> +
> +xfi_tphy->dev = &pdev->dev;
> +
> +xfi_tphy->clocks[0].id = "topxtal";
> +xfi_tphy->clocks[0].clk = devm_clk_get(&pdev->dev, xfi_tphy-
> >clocks[0].id);
> +if (IS_ERR(xfi_tphy->clocks[0].clk))
> +return PTR_ERR(xfi_tphy->clocks[0].clk);
> +
> +xfi_tphy->clocks[1].id = "xfipll";
> +xfi_tphy->clocks[1].clk = devm_clk_get(&pdev->dev, xfi_tphy-
> >clocks[1].id);
> +if (IS_ERR(xfi_tphy->clocks[1].clk))
> +return PTR_ERR(xfi_tphy->clocks[1].clk);
> +
> +xfi_tphy->reset = devm_reset_control_get_exclusive(&pdev->dev,
> NULL);
> +if (IS_ERR(xfi_tphy->reset))
> +return PTR_ERR(xfi_tphy->reset);
> +
> +xfi_tphy->da_war = of_property_read_bool(np,
> + "mediatek,usxgmii-performance-errata");
> +
> +phy = devm_phy_create(&pdev->dev, NULL, &mtk_xfi_tphy_ops);
> +if (IS_ERR(phy))
> +return PTR_ERR(phy);
> +
> +phy_set_drvdata(phy, xfi_tphy);
> +
> +phy_provider = devm_of_phy_provider_register(&pdev->dev,
> +     of_phy_simple_xlate);
> +
> +return PTR_ERR_OR_ZERO(phy_provider);
> +}
> +
> +static const struct of_device_id mtk_xfi_tphy_match[] = {
> +{ .compatible = "mediatek,mt7988-xfi-tphy", },
> +{ }
> +};
> +MODULE_DEVICE_TABLE(of, mtk_xfi_tphy_match);
> +
> +static struct platform_driver mtk_xfi_tphy_driver = {
> +.probe = mtk_xfi_tphy_probe,
> +.driver = {
> +.name = "mtk-xfi-tphy",
> +.of_match_table = mtk_xfi_tphy_match,
> +},
> +};
> +module_platform_driver(mtk_xfi_tphy_driver);
> +
> +MODULE_DESCRIPTION("MediaTek XFI T-PHY driver");
> +MODULE_AUTHOR("Daniel Golle <daniel@makrotopia.org>");
> +MODULE_AUTHOR("Bc-bocun Chen <bc-bocun.chen@mediatek.com>");
> +MODULE_LICENSE("GPL");
> -- 
> 2.43.0

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/2] dt-bindings: phy: mediatek,xfi-tphy: add new bindings
  2024-02-04  6:17   ` Chunfeng Yun (云春峰)
@ 2024-02-05  8:08     ` Krzysztof Kozlowski
  0 siblings, 0 replies; 12+ messages in thread
From: Krzysztof Kozlowski @ 2024-02-05  8:08 UTC (permalink / raw)
  To: Chunfeng Yun (云春峰),
	netdev, linux-mediatek, linux-kernel,
	SkyLake Huang (黃啟澤),
	kishon, devicetree, john, p.zabel, conor+dt, robh,
	Bc-bocun Chen (陳柏村),
	linux-arm-kernel, Steven Liu (劉人豪),
	vkoul, matthias.bgg, krzysztof.kozlowski+dt, daniel, linux-phy,
	dqfext, angelogioacchino.delregno

On 04/02/2024 07:17, Chunfeng Yun (云春峰) wrote:
> On Fri, 2024-02-02 at 09:21 +0100, Krzysztof Kozlowski wrote:
>>  	 
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>  On 01/02/2024 22:52, Daniel Golle wrote:
>>> Add bindings for the MediaTek XFI T-PHY Ethernet SerDes PHY found
>> in the
>>> MediaTek MT7988 SoC which can operate at various interfaces modes:
>>>
>>> via USXGMII PCS:
>>>  * USXGMII
>>>  * 10GBase-R
>>>  * 5GBase-R
>>>
>>> via LynxI SGMII PCS:
>>>  * 2500Base-X
>>>  * 1000Base-X
>>>  * Cisco SGMII (MAC side)
>>>
>>> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
>>> ---
>>>  .../bindings/phy/mediatek,xfi-tphy.yaml       | 80
>> +++++++++++++++++++
>>>  1 file changed, 80 insertions(+)
>>>  create mode 100644
>> Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/phy/mediatek,xfi-
>> tphy.yaml b/Documentation/devicetree/bindings/phy/mediatek,xfi-
>> tphy.yaml
>>> new file mode 100644
>>> index 0000000000000..e897118dcf7e6
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/phy/mediatek,xfi-tphy.yaml
>>> @@ -0,0 +1,80 @@
>>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/phy/mediatek,xfi-tphy.yaml#
>>
>> Please use compatible as filename. Your binding says only one is
>> possible (const, not enum), so there is no reasoning for different
>> filename.
>>
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: MediaTek XFI T-PHY
>>> +
>>> +maintainers:
>>> +  - Daniel Golle <daniel@makrotopia.org>
>>> +
>>> +description:
>>> +  The MediaTek XFI SerDes T-PHY provides the physical SerDes lanes
>>> +  used by the (10G/5G) USXGMII PCS and (1G/2.5G) LynxI PCS found
>> in
>>> +  MediaTek's 10G-capabale SoCs.
>>> +
>>> +properties:
>>> +  $nodename:
>>> +    pattern: "^phy@[0-9a-f]+$"
>>
>> No need for nodename in individual bindings file.
>>
>>> +
>>> +  compatible:
>>> +    const: mediatek,mt7988-xfi-tphy
> Add a generic compatible "mediatek,xfi-tphy"?
> 
> Other socs also use this phy but not upstream.

Are they here? No... They will use this one as fallback. Stop insisting
on some generic fallbacks just because you do not like using other SoCs
as fallbacks.

You ignored other comments, so I understand you agree with them 100%.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] phy: add driver for MediaTek XFI T-PHY
  2024-02-02 16:33     ` Daniel Golle
@ 2024-02-05 15:26       ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 12+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-02-05 15:26 UTC (permalink / raw)
  To: Daniel Golle
  Cc: Bc-bocun Chen, Chunfeng Yun, Vinod Koul, Kishon Vijay Abraham I,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, Qingfang Deng,
	SkyLake Huang, Matthias Brugger, Philipp Zabel, linux-arm-kernel,
	linux-mediatek, linux-phy, devicetree, linux-kernel, netdev

Il 02/02/24 17:33, Daniel Golle ha scritto:
> Hi Angelo,
> 
> thank you for promptly reviewing my submission and providing valuable
> feedback!
> 
> On Fri, Feb 02, 2024 at 02:21:48PM +0100, AngeloGioacchino Del Regno wrote:
>> Il 01/02/24 22:53, Daniel Golle ha scritto:
>>> Add driver for MediaTek's XFI T-PHY, 10 Gigabit/s Ethernet SerDes PHY
>>> which can be found in the MT7988 SoC.
>>>
>>> The PHY can operates only in PHY_MODE_ETHERNET, the submode is one of
>>> PHY_INTERFACE_MODE_* corresponding to the supported modes:
>>>
>>>    * USXGMII                 \
>>>    * 10GBase-R                }- USXGMII PCS - XGDM  \
>>>    * 5GBase-R                /                        \
>>>                                                        }- Ethernet MAC
>>>    * 2500Base-X              \                        /
>>>    * 1000Base-X               }- LynxI PCS - GDM     /
>>>    * Cisco SGMII (MAC side)  /
>>>
>>> In order to work-around a performance issue present on the first of
>>> two XFI T-PHYs present in MT7988, special tuning is applied which can be
>>> selected by adding the 'mediatek,usxgmii-performance-errata' property to
>>> the device tree node.
>>>
>>> There is no documentation for most registers used for the
>>> analog/tuning part, however, most of the registers have been partially
>>> reverse-engineered from MediaTek's SDK implementation (an opaque
>>> sequence of 32-bit register writes) and descriptions for all relevant
>>> digital registers and bits such as resets and muxes have been supplied
>>> by MediaTek.
>>>
>>> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
>>> ---
>>>    MAINTAINERS                             |   1 +
>>>    drivers/phy/mediatek/Kconfig            |  12 +
>>>    drivers/phy/mediatek/Makefile           |   1 +
>>>    drivers/phy/mediatek/phy-mtk-xfi-tphy.c | 392 ++++++++++++++++++++++++
>>>    4 files changed, 406 insertions(+)
>>>    create mode 100644 drivers/phy/mediatek/phy-mtk-xfi-tphy.c
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index 52769631bdb1a..52e4192470bd9 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -13715,6 +13715,7 @@ L:	netdev@vger.kernel.org
>>>    S:	Maintained
>>>    F:	drivers/net/phy/mediatek-ge-soc.c
>>>    F:	drivers/net/phy/mediatek-ge.c
>>> +F:	drivers/phy/mediatek/phy-mtk-xfi-tphy.c
>>>    MEDIATEK I2C CONTROLLER DRIVER
>>>    M:	Qii Wang <qii.wang@mediatek.com>
>>> diff --git a/drivers/phy/mediatek/Kconfig b/drivers/phy/mediatek/Kconfig
>>> index 3125ecb5d119f..5161d130c7f8b 100644
>>> --- a/drivers/phy/mediatek/Kconfig
>>> +++ b/drivers/phy/mediatek/Kconfig
>>> @@ -13,6 +13,18 @@ config PHY_MTK_PCIE
>>>    	  callback for PCIe GEN3 port, it supports software efuse
>>>    	  initialization.
>>> +config PHY_MTK_XFI_TPHY
>>> +	tristate "MediaTek XFI T-PHY Driver"
>>> +	depends on ARCH_MEDIATEK || COMPILE_TEST
>>> +	depends on OF && OF_ADDRESS
>>> +	depends on HAS_IOMEM
>>> +	select GENERIC_PHY
>>> +	help
>>> +	  Say 'Y' here to add support for MediaTek XFI T-PHY driver.
>>> +	  The driver provides access to the Ethernet SerDes T-PHY supporting
>>> +	  1GE and 2.5GE modes via the LynxI PCS, and 5GE and 10GE modes
>>> +	  via the USXGMII PCS found in MediaTek SoCs with 10G Ethernet.
>>> +
>>>    config PHY_MTK_TPHY
>>>    	tristate "MediaTek T-PHY Driver"
>>>    	depends on ARCH_MEDIATEK || COMPILE_TEST
>>> diff --git a/drivers/phy/mediatek/Makefile b/drivers/phy/mediatek/Makefile
>>> index c9a50395533eb..fa7217178e7f4 100644
>>> --- a/drivers/phy/mediatek/Makefile
>>> +++ b/drivers/phy/mediatek/Makefile
>>> @@ -8,6 +8,7 @@ obj-$(CONFIG_PHY_MTK_PCIE)		+= phy-mtk-pcie.o
>>>    obj-$(CONFIG_PHY_MTK_TPHY)		+= phy-mtk-tphy.o
>>>    obj-$(CONFIG_PHY_MTK_UFS)		+= phy-mtk-ufs.o
>>>    obj-$(CONFIG_PHY_MTK_XSPHY)		+= phy-mtk-xsphy.o
>>> +obj-$(CONFIG_PHY_MTK_XFI_TPHY)		+= phy-mtk-xfi-tphy.o
>>>    phy-mtk-hdmi-drv-y			:= phy-mtk-hdmi.o
>>>    phy-mtk-hdmi-drv-y			+= phy-mtk-hdmi-mt2701.o
>>> diff --git a/drivers/phy/mediatek/phy-mtk-xfi-tphy.c b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
>>> new file mode 100644
>>> index 0000000000000..d50e6320860e5
>>> --- /dev/null
>>> +++ b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
>>> @@ -0,0 +1,392 @@
>>> +// SPDX-License-Identifier: GPL-2.0-or-later
>>> +/* MediaTek 10GE SerDes PHY driver
>>
>> MediaTek 10GE SerDes XFI T-PHY driver ?
> 
> Ack. Will unify the name accross all files.
> 
>>
>>> + *
>>> + * Copyright (c) 2024 Daniel Golle <daniel@makrotopia.org>
>>> + *                    Bc-bocun Chen <bc-bocun.chen@mediatek.com>
>>> + * based on mtk_usxgmii.c found in MediaTek's SDK released under GPL-2.0
>>> + * Copyright (c) 2022 MediaTek Inc.
>>> + * Author: Henry Yen <henry.yen@mediatek.com>
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/device.h>
>>> +#include <linux/platform_device.h>
>>> +#include <linux/of.h>
>>> +#include <linux/io.h>
>>> +#include <linux/clk.h>
>>> +#include <linux/reset.h>
>>> +#include <linux/phy.h>
>>> +#include <linux/phy/phy.h>
>>> +
>>> +#define MTK_XFI_TPHY_NUM_CLOCKS		2
>>> +
>>> +#define REG_DIG_GLB_70			0x0070
>>> +#define  XTP_PCS_RX_EQ_IN_PROGRESS(x)	FIELD_PREP(GENMASK(25, 24), (x))
>>> +#define  XTP_PCS_MODE_MASK		GENMASK(17, 16)
>>> +#define  XTP_PCS_MODE(x)		FIELD_PREP(GENMASK(17, 16), (x))
>>> +#define  XTP_PCS_RST_B			BIT(15)
>>> +#define  XTP_FRC_PCS_RST_B		BIT(14)
>>> +#define  XTP_PCS_PWD_SYNC_MASK		GENMASK(13, 12)
>>> +#define  XTP_PCS_PWD_SYNC(x)		FIELD_PREP(XTP_PCS_PWD_SYNC_MASK, (x))
>>> +#define  XTP_PCS_PWD_ASYNC_MASK		GENMASK(11, 10)
>>> +#define  XTP_PCS_PWD_ASYNC(x)		FIELD_PREP(XTP_PCS_PWD_ASYNC_MASK, (x))
>>> +#define  XTP_FRC_PCS_PWD_ASYNC		BIT(8)
>>> +#define  XTP_PCS_UPDT			BIT(4)
>>> +#define  XTP_PCS_IN_FR_RG		BIT(0)
>>> +
>>> +#define REG_DIG_GLB_F4			0x00f4
>>> +#define  XFI_DPHY_PCS_SEL		BIT(0)
>>> +#define   XFI_DPHY_PCS_SEL_SGMII	FIELD_PREP(XFI_DPHY_PCS_SEL, 1)
>>> +#define   XFI_DPHY_PCS_SEL_USXGMII	FIELD_PREP(XFI_DPHY_PCS_SEL, 0)
>>> +#define  XFI_DPHY_AD_SGDT_FRC_EN	BIT(5)
>>> +
>>> +#define REG_DIG_LN_TRX_40		0x3040
>>> +#define  XTP_LN_FRC_TX_DATA_EN		BIT(29)
>>> +#define  XTP_LN_TX_DATA_EN		BIT(28)
>>> +
>>> +#define REG_DIG_LN_TRX_B0		0x30b0
>>> +#define  XTP_LN_FRC_TX_MACCK_EN		BIT(5)
>>> +#define  XTP_LN_TX_MACCK_EN		BIT(4)
>>> +
>>> +#define REG_ANA_GLB_D0			0x90d0
>>> +#define  XTP_GLB_USXGMII_SEL_MASK	GENMASK(3, 1)
>>> +#define  XTP_GLB_USXGMII_SEL(x)		FIELD_PREP(GENMASK(3, 1), (x))
>>> +#define  XTP_GLB_USXGMII_EN		BIT(0)
>>> +
>>> +struct mtk_xfi_tphy {
>>> +	void __iomem		*base;
>>> +	struct device		*dev;
>>> +	struct reset_control	*reset;
>>> +	struct clk_bulk_data	clocks[MTK_XFI_TPHY_NUM_CLOCKS];
>>> +	bool			da_war;
>>> +};
>>> +
>>> +static void mtk_xfi_tphy_write(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
>>> +			       u32 value)
>>> +{
>>> +	iowrite32(value, xfi_tphy->base + reg);
>>> +}
>>> +
>>> +static void mtk_xfi_tphy_rmw(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
>>> +			     u32 clr, u32 set)
>>> +{
>>> +	u32 val;
>>> +
>>> +	val = ioread32(xfi_tphy->base + reg);
>>> +	val &= ~clr;
>>> +	val |= set;
>>> +	iowrite32(val, xfi_tphy->base + reg);
>>> +}
>>> +
>>> +static void mtk_xfi_tphy_set(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
>>> +			     u32 set)
>>> +{
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, reg, 0, set);
>>> +}
>>> +
>>> +static void mtk_xfi_tphy_clear(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
>>> +			       u32 clr)
>>> +{
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, reg, clr, 0);
>>> +}
>>> +
>>> +static void mtk_xfi_tphy_setup(struct mtk_xfi_tphy *xfi_tphy,
>>> +			       phy_interface_t interface)
>>> +{
>>> +	bool is_2p5g = (interface == PHY_INTERFACE_MODE_2500BASEX);
>>> +	bool is_1g = (interface == PHY_INTERFACE_MODE_1000BASEX ||
>>> +		      interface == PHY_INTERFACE_MODE_SGMII);
>>> +	bool is_10g = (interface == PHY_INTERFACE_MODE_10GBASER ||
>>> +		       interface == PHY_INTERFACE_MODE_USXGMII);
>>> +	bool is_5g = (interface == PHY_INTERFACE_MODE_5GBASER);
>>> +	bool is_xgmii = (is_10g || is_5g);
>>
>> is_usxgmii, I'd say.
> 
> This boolean is mostly used to destinguish which path to use at the
> T-junction of the PHY, ie. up to 2500Base-X goes to LynxI PCS while
> 5GBase-R, 10GBase-R and USXGMII go to USXGMII PCS (which handles all
> three modes and not just USXGMII).
> 
> Calling the boolean 'is_usxgmii' is a bit confusing as it not only
> means (interface == interface == PHY_INTERFACE_MODE_USXGMII) but
> rather also covers (interface == PHY_INTERFACE_MODE_10GBASER) as well
> as (interface == PHY_INTERFACE_MODE_5GBASER) which are not USXGMII.
> 
> Maybe we invert it and call it 'use_lynxi_pcs' instead?
> 

Since you were able to provide such a good explanation of that, I wonder why
you didn't add that as a comment in the code?

That way, there will be no confusion for sure, even if you keep "is_xgmii" :-)

>>
>>> +
>>> +	dev_dbg(xfi_tphy->dev, "setting up for mode %s\n", phy_modes(interface));
>>> +
>>> +	/* Setup PLL setting */
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x9024, 0x100000, is_10g ? 0x0 : 0x100000);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x2020, 0x202000, is_5g ? 0x202000 : 0x0);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x2030, 0x500, is_1g ? 0x0 : 0x500);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x2034, 0xa00, is_1g ? 0x0 : 0xa00);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x2040, 0x340000, is_1g ? 0x200000 :
>>> +							     0x140000);
>>> +
>>> +	/* Setup RXFE BW setting */
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50f0, 0xc10, is_1g ? 0x410 :
>>> +							  is_5g ? 0x800 : 0x400);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50e0, 0x4000, is_5g ? 0x0 : 0x4000);
>>> +
>>> +	/* Setup RX CDR setting */
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x506c, 0x30000, is_5g ? 0x0 : 0x30000);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5070, 0x670000, is_5g ? 0x620000 : 0x50000);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5074, 0x180000, is_5g ? 0x180000 : 0x0);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5078, 0xf000400, is_5g ? 0x8000000 :
>>> +							      0x7000400);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x507c, 0x5000500, is_5g ? 0x4000400 :
>>> +							      0x1000100);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5080, 0x1410, is_1g ? 0x400 :
>>> +							   is_5g ? 0x1010 : 0x0);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5084, 0x30300, is_1g ? 0x30300 :
>>> +							    is_5g ? 0x30100 :
>>> +								    0x100);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x5088, 0x60200, is_1g ? 0x20200 :
>>> +							 is_5g ? 0x40000 :
>>> +								 0x20000);
>>> +
>>> +	/* Setting RXFE adaptation range setting */
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50e4, 0xc0000, is_5g ? 0x0 : 0xc0000);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50e8, 0x40000, is_5g ? 0x0 : 0x40000);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50ec, 0xa00, is_1g ? 0x200 : 0x800);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x50a8, 0xee0000, is_5g ? 0x800000 :
>>> +							     0x6e0000);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x6004, 0x190000, is_5g ? 0x0 : 0x190000);
>>> +	if (is_10g)
>>> +		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x01423342);
>>> +	else if (is_5g)
>>> +		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x00a132a1);
>>> +	else if (is_2p5g)
>>> +		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x009c329c);
>>> +	else
>>> +		mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x00fa32fa);
>>> +
>>> +	/* Force SGDT_OUT off and select PCS */
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_F4,
>>> +			 XFI_DPHY_AD_SGDT_FRC_EN | XFI_DPHY_PCS_SEL,
>>> +			 XFI_DPHY_AD_SGDT_FRC_EN |
>>> +			 (is_xgmii ? XFI_DPHY_PCS_SEL_USXGMII :
>>> +				     XFI_DPHY_PCS_SEL_SGMII));
>>> +
>>> +
>>> +	/* Force GLB_CKDET_OUT */
>>> +	mtk_xfi_tphy_set(xfi_tphy, 0x0030, 0xc00);
>>> +
>>> +	/* Force AEQ on */
>>> +	mtk_xfi_tphy_write(xfi_tphy, REG_DIG_GLB_70,
>>> +			   XTP_PCS_RX_EQ_IN_PROGRESS(2) |
>>> +			   XTP_PCS_PWD_SYNC(2) |
>>> +			   XTP_PCS_PWD_ASYNC(2));
>>> +
>>> +	usleep_range(1, 5);
>>> +
>>> +	/* Setup TX DA default value */
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x30b0, 0x30, 0x20);
>>> +	mtk_xfi_tphy_write(xfi_tphy, 0x3028, 0x00008a01);
>>> +	mtk_xfi_tphy_write(xfi_tphy, 0x302c, 0x0000a884);
>>> +	mtk_xfi_tphy_write(xfi_tphy, 0x3024, 0x00083002);
>>> +
>>> +	/* Setup RG default value */
>>> +	if (is_xgmii) {
>>> +		mtk_xfi_tphy_write(xfi_tphy, 0x3010, 0x00022220);
>>> +		mtk_xfi_tphy_write(xfi_tphy, 0x5064, 0x0f020a01);
>>> +		mtk_xfi_tphy_write(xfi_tphy, 0x50b4, 0x06100600);
>>> +		if (interface == PHY_INTERFACE_MODE_USXGMII)
>>> +			mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x40704000);
>>> +		else
>>> +			mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x47684100);
>>> +	} else {
>>> +		mtk_xfi_tphy_write(xfi_tphy, 0x3010, 0x00011110);
>>> +		mtk_xfi_tphy_write(xfi_tphy, 0x3048, 0x40704000);
>>> +	}
>>> +
>>> +	if (is_1g)
>>> +		mtk_xfi_tphy_write(xfi_tphy, 0x3064, 0x0000c000);
>>> +
>>> +	/* Setup RX EQ initial value */
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x3050, 0xa8000000,
>>> +			 (interface != PHY_INTERFACE_MODE_10GBASER) ?
>>> +			  0xa8000000 : 0x0);
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0x3054, 0xaa,
>>> +			 (interface != PHY_INTERFACE_MODE_10GBASER) ?
>>> +			  0xaa : 0x0);
>>> +
>>> +	if (is_xgmii)
>>> +		mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x00000f00);
>>> +	else if (is_2p5g)
>>> +		mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x22000f00);
>>> +	else
>>> +		mtk_xfi_tphy_write(xfi_tphy, 0x306c, 0x20200f00);
>>> +
>>> +	if (interface == PHY_INTERFACE_MODE_10GBASER && xfi_tphy->da_war)
>>> +		mtk_xfi_tphy_rmw(xfi_tphy, 0xa008, 0x10000, 0x10000);
>>> +
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, 0xa060, 0x50000, is_xgmii ? 0x40000 :
>>> +							       0x50000);
>>> +
>>> +	/* Setup PHYA speed */
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, REG_ANA_GLB_D0,
>>> +			 XTP_GLB_USXGMII_SEL_MASK | XTP_GLB_USXGMII_EN,
>>> +			 is_10g ?  XTP_GLB_USXGMII_SEL(0) :
>>> +			 is_5g ?   XTP_GLB_USXGMII_SEL(1) :
>>> +			 is_2p5g ? XTP_GLB_USXGMII_SEL(2) :
>>> +				   XTP_GLB_USXGMII_SEL(3));
>>> +	mtk_xfi_tphy_set(xfi_tphy, REG_ANA_GLB_D0, XTP_GLB_USXGMII_EN);
>>> +
>>> +	/* Release reset */
>>> +	mtk_xfi_tphy_set(xfi_tphy, REG_DIG_GLB_70,
>>> +			 XTP_PCS_RST_B | XTP_FRC_PCS_RST_B);
>>> +	usleep_range(150, 500);
>>> +
>>> +	/* Switch to P0 */
>>> +	mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
>>> +			 XTP_PCS_PWD_SYNC_MASK |
>>> +			 XTP_PCS_PWD_ASYNC_MASK,
>>> +			 XTP_FRC_PCS_PWD_ASYNC |
>>> +			 XTP_PCS_UPDT | XTP_PCS_IN_FR_RG);
>>> +	usleep_range(1, 5);
>>> +
>>> +	mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_70, XTP_PCS_UPDT);
>>> +	usleep_range(15, 50);
>>> +
>>> +	if (is_xgmii) {
>>> +		/* Switch to Gen3 */
>>> +		mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
>>> +				 XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
>>> +				 XTP_PCS_MODE(2) | XTP_PCS_UPDT);
>>> +	} else {
>>> +		/* Switch to Gen2 */
>>> +		mtk_xfi_tphy_rmw(xfi_tphy, REG_DIG_GLB_70,
>>> +				 XTP_PCS_MODE_MASK | XTP_PCS_UPDT,
>>> +				 XTP_PCS_MODE(1) | XTP_PCS_UPDT);
>>> +	}
>>> +	usleep_range(1, 5);
>>> +
>>> +	mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_70, XTP_PCS_UPDT);
>>> +
>>> +	usleep_range(100, 500);
>>> +
>>> +	/* Enable MAC CK */
>>> +	mtk_xfi_tphy_set(xfi_tphy, REG_DIG_LN_TRX_B0, XTP_LN_TX_MACCK_EN);
>>> +	mtk_xfi_tphy_clear(xfi_tphy, REG_DIG_GLB_F4, XFI_DPHY_AD_SGDT_FRC_EN);
>>> +
>>> +	/* Enable TX data */
>>> +	mtk_xfi_tphy_set(xfi_tphy, REG_DIG_LN_TRX_40,
>>> +			 XTP_LN_FRC_TX_DATA_EN | XTP_LN_TX_DATA_EN);
>>> +	usleep_range(400, 1000);
>>> +}
>>> +
>>> +static int mtk_xfi_tphy_set_mode(struct phy *phy, enum phy_mode mode, int
>>> +				 submode)
>>> +{
>>> +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
>>> +
>>> +	if (mode != PHY_MODE_ETHERNET)
>>> +		return -EINVAL;
>>> +
>>> +	switch (submode) {
>>> +	case PHY_INTERFACE_MODE_1000BASEX:
>>
>> fallthrough;
> 
> I don't think so. No other kernel drivers are doing that and also there
> is no compiler warning what-so-ever when matching multiple cases in that
> way.
> 

Right. Just verified. Keep it like that then.

>>
>>> +	case PHY_INTERFACE_MODE_2500BASEX:
>>
>> fallthrough;
>>
>>> +	case PHY_INTERFACE_MODE_SGMII:
>>
>> ... etc :-)
>>
>>> +	case PHY_INTERFACE_MODE_5GBASER:
>>> +	case PHY_INTERFACE_MODE_10GBASER:
>>> +	case PHY_INTERFACE_MODE_USXGMII:
>>
>> Does this PHY support PHY_INTERFACE_MODE_XGMII ?
>>
>>> +		mtk_xfi_tphy_setup(xfi_tphy, submode);
>>> +		return 0;
>>> +	default:
>>> +		return -EINVAL;
>>> +	}
>>> +}
>>> +
>>> +static int mtk_xfi_tphy_reset(struct phy *phy)
>>> +{
>>> +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
>>> +
>>> +	reset_control_assert(xfi_tphy->reset);
>>> +	usleep_range(100, 500);
>>> +	reset_control_deassert(xfi_tphy->reset);
>>> +	usleep_range(1, 10);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int mtk_xfi_tphy_power_on(struct phy *phy)
>>> +{
>>> +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
>>> +
>>> +	return clk_bulk_prepare_enable(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
>>> +}
>>> +
>>> +static int mtk_xfi_tphy_power_off(struct phy *phy)
>>> +{
>>> +	struct mtk_xfi_tphy *xfi_tphy = phy_get_drvdata(phy);
>>> +
>>> +	clk_bulk_disable_unprepare(MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct phy_ops mtk_xfi_tphy_ops = {
>>> +	.power_on	= mtk_xfi_tphy_power_on,
>>> +	.power_off	= mtk_xfi_tphy_power_off,
>>> +	.set_mode	= mtk_xfi_tphy_set_mode,
>>> +	.reset		= mtk_xfi_tphy_reset,
>>> +	.owner		= THIS_MODULE,
>>> +};
>>> +
>>> +static int mtk_xfi_tphy_probe(struct platform_device *pdev)
>>> +{
>>> +	struct device_node *np = pdev->dev.of_node;
>>> +	struct phy_provider *phy_provider;
>>> +	struct mtk_xfi_tphy *xfi_tphy;
>>> +	struct phy *phy;
>>> +
>>> +	if (!np)
>>> +		return -ENODEV;
>>> +
>>> +	xfi_tphy = devm_kzalloc(&pdev->dev, sizeof(*xfi_tphy), GFP_KERNEL);
>>> +	if (!xfi_tphy)
>>> +		return -ENOMEM;
>>> +
>>> +	xfi_tphy->base = devm_of_iomap(&pdev->dev, np, 0, NULL);
>>
>> Why devm_of_iomap() and not devm_platform_ioremap_resource()?
> 
> True, I will make use of devm_platform_ioremap_resource().
> 
>>
>>> +	if (!xfi_tphy->base)
>>> +		return -EIO;
>>> +
>>> +	xfi_tphy->dev = &pdev->dev;
>>> +
>>> +	xfi_tphy->clocks[0].id = "topxtal";
>>
>> xfi_tphy->clocks[0].id = "topxtal";
>> xfi_tphy->clocks[1].id = "xfipll";
>>
>> ret = devm_clk_bulk_get(&pdev->dev, MTK_XFI_TPHY_NUM_CLOCKS, xfi_tphy->clocks);
>> if (ret)
>> 	return ret;
>>
>> ...it's that simple :-P
> 
> Ok, thanks :)
> 
>>
>>> +	xfi_tphy->clocks[0].clk = devm_clk_get(&pdev->dev, xfi_tphy->clocks[0].id);
>>> +	if (IS_ERR(xfi_tphy->clocks[0].clk))
>>> +		return PTR_ERR(xfi_tphy->clocks[0].clk);
>>> +
>>> +	xfi_tphy->clocks[1].id = "xfipll";
>>> +	xfi_tphy->clocks[1].clk = devm_clk_get(&pdev->dev, xfi_tphy->clocks[1].id);
>>> +	if (IS_ERR(xfi_tphy->clocks[1].clk))
>>> +		return PTR_ERR(xfi_tphy->clocks[1].clk);
>>> +
>>> +	xfi_tphy->reset = devm_reset_control_get_exclusive(&pdev->dev, NULL);
>>> +	if (IS_ERR(xfi_tphy->reset))
>>> +		return PTR_ERR(xfi_tphy->reset);
>>> +
>>> +	xfi_tphy->da_war = of_property_read_bool(np,
>>> +						 "mediatek,usxgmii-performance-errata");
>>> +
>>
>> One line please
>>
>>> +	phy = devm_phy_create(&pdev->dev, NULL, &mtk_xfi_tphy_ops);
>>> +	if (IS_ERR(phy))
>>> +		return PTR_ERR(phy);
>>> +
>>> +	phy_set_drvdata(phy, xfi_tphy);
>>> +
>>> +	phy_provider = devm_of_phy_provider_register(&pdev->dev,
>>> +						     of_phy_simple_xlate);
>>
>> ditto
>>
>>> +
>>> +	return PTR_ERR_OR_ZERO(phy_provider);
>>> +}
>>> +
>>> +static const struct of_device_id mtk_xfi_tphy_match[] = {
>>> +	{ .compatible = "mediatek,mt7988-xfi-tphy", },
>>> +	{ }
>>
>> 	{ .compatible = "mediatek,mt7988-xfi-tphy" },
>> 	{ /* sentinel */ }
> 
> Ack.
> 
>>
>>> +};
>>> +MODULE_DEVICE_TABLE(of, mtk_xfi_tphy_match);
>>> +
>>> +static struct platform_driver mtk_xfi_tphy_driver = {
>>> +	.probe = mtk_xfi_tphy_probe,
>>> +	.driver = {
>>> +		.name = "mtk-xfi-tphy",
>>> +		.of_match_table = mtk_xfi_tphy_match,
>>> +	},
>>> +};
>>> +module_platform_driver(mtk_xfi_tphy_driver);
>>> +
>>> +MODULE_DESCRIPTION("MediaTek XFI T-PHY driver");
>>
>> MODULE_DESCRIPTION("MediaTek 10GE SerDes XFI T-PHY driver");
> 
> Ack, will unify the name everywhere.
> 
>>
>> Cheers,
>> Angelo
>>
>>> +MODULE_AUTHOR("Daniel Golle <daniel@makrotopia.org>");
>>> +MODULE_AUTHOR("Bc-bocun Chen <bc-bocun.chen@mediatek.com>");
>>> +MODULE_LICENSE("GPL");
>>
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/2] phy: add driver for MediaTek XFI T-PHY
  2024-02-04  6:29   ` Chunfeng Yun (云春峰)
@ 2024-02-05 15:29     ` AngeloGioacchino Del Regno
  0 siblings, 0 replies; 12+ messages in thread
From: AngeloGioacchino Del Regno @ 2024-02-05 15:29 UTC (permalink / raw)
  To: Chunfeng Yun (云春峰),
	netdev, linux-mediatek, linux-kernel,
	SkyLake Huang (黃啟澤),
	kishon, devicetree, p.zabel, conor+dt, robh,
	Bc-bocun Chen (陳柏村),
	linux-arm-kernel, krzysztof.kozlowski+dt, vkoul, matthias.bgg,
	dqfext, daniel, linux-phy

Il 04/02/24 07:29, Chunfeng Yun (云春峰) ha scritto:
> On Thu, 2024-02-01 at 21:53 +0000, Daniel Golle wrote:
>>   	
>> External email : Please do not click links or open attachments until
>> you have verified the sender or the content.
>>   Add driver for MediaTek's XFI T-PHY, 10 Gigabit/s Ethernet SerDes
>> PHY
>> which can be found in the MT7988 SoC.
>>
>> The PHY can operates only in PHY_MODE_ETHERNET, the submode is one of
>> PHY_INTERFACE_MODE_* corresponding to the supported modes:
>>
>>   * USXGMII                 \
>>   * 10GBase-R                }- USXGMII PCS - XGDM  \
>>   * 5GBase-R                /                        \
>>                                                       }- Ethernet MAC
>>   * 2500Base-X              \                        /
>>   * 1000Base-X               }- LynxI PCS - GDM     /
>>   * Cisco SGMII (MAC side)  /
>>
>> In order to work-around a performance issue present on the first of
>> two XFI T-PHYs present in MT7988, special tuning is applied which can
>> be
>> selected by adding the 'mediatek,usxgmii-performance-errata' property
>> to
>> the device tree node.
>>
>> There is no documentation for most registers used for the
>> analog/tuning part, however, most of the registers have been
>> partially
>> reverse-engineered from MediaTek's SDK implementation (an opaque
>> sequence of 32-bit register writes) and descriptions for all relevant
>> digital registers and bits such as resets and muxes have been
>> supplied
>> by MediaTek.
>>
>> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
>> ---
>>   MAINTAINERS                             |   1 +
>>   drivers/phy/mediatek/Kconfig            |  12 +
>>   drivers/phy/mediatek/Makefile           |   1 +
>>   drivers/phy/mediatek/phy-mtk-xfi-tphy.c | 392
>> ++++++++++++++++++++++++
>>   4 files changed, 406 insertions(+)
>>   create mode 100644 drivers/phy/mediatek/phy-mtk-xfi-tphy.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index 52769631bdb1a..52e4192470bd9 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -13715,6 +13715,7 @@ L:netdev@vger.kernel.org
>>   S:Maintained
>>   F:drivers/net/phy/mediatek-ge-soc.c
>>   F:drivers/net/phy/mediatek-ge.c
>> +F:drivers/phy/mediatek/phy-mtk-xfi-tphy.c
>>   
>>   MEDIATEK I2C CONTROLLER DRIVER
>>   M:Qii Wang <qii.wang@mediatek.com>
>> diff --git a/drivers/phy/mediatek/Kconfig
>> b/drivers/phy/mediatek/Kconfig
>> index 3125ecb5d119f..5161d130c7f8b 100644
>> --- a/drivers/phy/mediatek/Kconfig
>> +++ b/drivers/phy/mediatek/Kconfig
>> @@ -13,6 +13,18 @@ config PHY_MTK_PCIE
>>     callback for PCIe GEN3 port, it supports software efuse
>>     initialization.
>>   
>> +config PHY_MTK_XFI_TPHY
>> +tristate "MediaTek XFI T-PHY Driver"
>> +depends on ARCH_MEDIATEK || COMPILE_TEST
>> +depends on OF && OF_ADDRESS
>> +depends on HAS_IOMEM
>> +select GENERIC_PHY
>> +help
>> +  Say 'Y' here to add support for MediaTek XFI T-PHY driver.
>> +  The driver provides access to the Ethernet SerDes T-PHY supporting
>> +  1GE and 2.5GE modes via the LynxI PCS, and 5GE and 10GE modes
>> +  via the USXGMII PCS found in MediaTek SoCs with 10G Ethernet.
>> +
>>   config PHY_MTK_TPHY
>>   tristate "MediaTek T-PHY Driver"
>>   depends on ARCH_MEDIATEK || COMPILE_TEST
>> diff --git a/drivers/phy/mediatek/Makefile
>> b/drivers/phy/mediatek/Makefile
>> index c9a50395533eb..fa7217178e7f4 100644
>> --- a/drivers/phy/mediatek/Makefile
>> +++ b/drivers/phy/mediatek/Makefile
>> @@ -8,6 +8,7 @@ obj-$(CONFIG_PHY_MTK_PCIE)+= phy-mtk-pcie.o
>>   obj-$(CONFIG_PHY_MTK_TPHY)+= phy-mtk-tphy.o
>>   obj-$(CONFIG_PHY_MTK_UFS)+= phy-mtk-ufs.o
>>   obj-$(CONFIG_PHY_MTK_XSPHY)+= phy-mtk-xsphy.o
>> +obj-$(CONFIG_PHY_MTK_XFI_TPHY)+= phy-mtk-xfi-tphy.o
>>   
>>   phy-mtk-hdmi-drv-y:= phy-mtk-hdmi.o
>>   phy-mtk-hdmi-drv-y+= phy-mtk-hdmi-mt2701.o
>> diff --git a/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
>> b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
>> new file mode 100644
>> index 0000000000000..d50e6320860e5
>> --- /dev/null
>> +++ b/drivers/phy/mediatek/phy-mtk-xfi-tphy.c
>> @@ -0,0 +1,392 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/* MediaTek 10GE SerDes PHY driver
>> + *
>> + * Copyright (c) 2024 Daniel Golle <daniel@makrotopia.org>
>> + *                    Bc-bocun Chen <bc-bocun.chen@mediatek.com>
>> + * based on mtk_usxgmii.c found in MediaTek's SDK released under
>> GPL-2.0
>> + * Copyright (c) 2022 MediaTek Inc.
>> + * Author: Henry Yen <henry.yen@mediatek.com>
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/of.h>
>> +#include <linux/io.h>
>> +#include <linux/clk.h>
>> +#include <linux/reset.h>
>> +#include <linux/phy.h>
>> +#include <linux/phy/phy.h>
>> +
>> +#define MTK_XFI_TPHY_NUM_CLOCKS2
>> +
>> +#define REG_DIG_GLB_700x0070
>> +#define  XTP_PCS_RX_EQ_IN_PROGRESS(x)FIELD_PREP(GENMASK(25, 24),
>> (x))
>> +#define  XTP_PCS_MODE_MASKGENMASK(17, 16)
>> +#define  XTP_PCS_MODE(x)FIELD_PREP(GENMASK(17, 16), (x))
>> +#define  XTP_PCS_RST_BBIT(15)
>> +#define  XTP_FRC_PCS_RST_BBIT(14)
>> +#define  XTP_PCS_PWD_SYNC_MASKGENMASK(13, 12)
>> +#define  XTP_PCS_PWD_SYNC(x)FIELD_PREP(XTP_PCS_PWD_SYNC_MASK, (x))
>> +#define  XTP_PCS_PWD_ASYNC_MASKGENMASK(11, 10)
>> +#define  XTP_PCS_PWD_ASYNC(x)FIELD_PREP(XTP_PCS_PWD_ASYNC_MASK, (x))
>> +#define  XTP_FRC_PCS_PWD_ASYNCBIT(8)
>> +#define  XTP_PCS_UPDTBIT(4)
>> +#define  XTP_PCS_IN_FR_RGBIT(0)
>> +
>> +#define REG_DIG_GLB_F40x00f4
>> +#define  XFI_DPHY_PCS_SELBIT(0)
>> +#define   XFI_DPHY_PCS_SEL_SGMIIFIELD_PREP(XFI_DPHY_PCS_SEL, 1)
>> +#define   XFI_DPHY_PCS_SEL_USXGMIIFIELD_PREP(XFI_DPHY_PCS_SEL, 0)
>> +#define  XFI_DPHY_AD_SGDT_FRC_ENBIT(5)
>> +
>> +#define REG_DIG_LN_TRX_400x3040
>> +#define  XTP_LN_FRC_TX_DATA_ENBIT(29)
>> +#define  XTP_LN_TX_DATA_ENBIT(28)
>> +
>> +#define REG_DIG_LN_TRX_B00x30b0
>> +#define  XTP_LN_FRC_TX_MACCK_ENBIT(5)
>> +#define  XTP_LN_TX_MACCK_ENBIT(4)
>> +
>> +#define REG_ANA_GLB_D00x90d0
>> +#define  XTP_GLB_USXGMII_SEL_MASKGENMASK(3, 1)
>> +#define  XTP_GLB_USXGMII_SEL(x)FIELD_PREP(GENMASK(3, 1), (x))
>> +#define  XTP_GLB_USXGMII_ENBIT(0)
>> +
>> +struct mtk_xfi_tphy {
>> +void __iomem*base;
>> +struct device*dev;
>> +struct reset_control*reset;
>> +struct clk_bulk_dataclocks[MTK_XFI_TPHY_NUM_CLOCKS];
>> +boolda_war;
>> +};
>> +
>> +static void mtk_xfi_tphy_write(struct mtk_xfi_tphy *xfi_tphy, u16
>> reg,
>> +       u32 value)
>> +{
>> +iowrite32(value, xfi_tphy->base + reg);
>> +}
>> +
>> +static void mtk_xfi_tphy_rmw(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
>> +     u32 clr, u32 set)
>> +{
>> +u32 val;
>> +
>> +val = ioread32(xfi_tphy->base + reg);
>> +val &= ~clr;
>> +val |= set;
>> +iowrite32(val, xfi_tphy->base + reg);
>> +}
>> +
>> +static void mtk_xfi_tphy_set(struct mtk_xfi_tphy *xfi_tphy, u16 reg,
>> +     u32 set)
>> +{
>> +mtk_xfi_tphy_rmw(xfi_tphy, reg, 0, set);
>> +}
>> +
>> +static void mtk_xfi_tphy_clear(struct mtk_xfi_tphy *xfi_tphy, u16
>> reg,
>> +       u32 clr)
>> +{
>> +mtk_xfi_tphy_rmw(xfi_tphy, reg, clr, 0);
>> +}
> 
> Helpers defined in phy-mtk-io.h can be used instead?
> 
>> +
>> +static void mtk_xfi_tphy_setup(struct mtk_xfi_tphy *xfi_tphy,
>> +       phy_interface_t interface)
>> +{
>> +bool is_2p5g = (interface == PHY_INTERFACE_MODE_2500BASEX);
>> +bool is_1g = (interface == PHY_INTERFACE_MODE_1000BASEX ||
>> +      interface == PHY_INTERFACE_MODE_SGMII);
>> +bool is_10g = (interface == PHY_INTERFACE_MODE_10GBASER ||
>> +       interface == PHY_INTERFACE_MODE_USXGMII);
>> +bool is_5g = (interface == PHY_INTERFACE_MODE_5GBASER);
>> +bool is_xgmii = (is_10g || is_5g);
>> +
>> +dev_dbg(xfi_tphy->dev, "setting up for mode %s\n",
>> phy_modes(interface));
>> +
>> +/* Setup PLL setting */
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x9024, 0x100000, is_10g ? 0x0 :
>> 0x100000);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x2020, 0x202000, is_5g ? 0x202000 :
>> 0x0);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x2030, 0x500, is_1g ? 0x0 : 0x500);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x2034, 0xa00, is_1g ? 0x0 : 0xa00);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x2040, 0x340000, is_1g ? 0x200000 :
>> +     0x140000);
>> +
>> +/* Setup RXFE BW setting */
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x50f0, 0xc10, is_1g ? 0x410 :
>> +  is_5g ? 0x800 : 0x400);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x50e0, 0x4000, is_5g ? 0x0 : 0x4000);
>> +
>> +/* Setup RX CDR setting */
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x506c, 0x30000, is_5g ? 0x0 : 0x30000);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x5070, 0x670000, is_5g ? 0x620000 :
>> 0x50000);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x5074, 0x180000, is_5g ? 0x180000 :
>> 0x0);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x5078, 0xf000400, is_5g ? 0x8000000 :
>> +      0x7000400);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x507c, 0x5000500, is_5g ? 0x4000400 :
>> +      0x1000100);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x5080, 0x1410, is_1g ? 0x400 :
>> +   is_5g ? 0x1010 : 0x0);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x5084, 0x30300, is_1g ? 0x30300 :
>> +    is_5g ? 0x30100 :
>> +    0x100);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x5088, 0x60200, is_1g ? 0x20200 :
>> + is_5g ? 0x40000 :
>> + 0x20000);
>> +
>> +/* Setting RXFE adaptation range setting */
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x50e4, 0xc0000, is_5g ? 0x0 : 0xc0000);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x50e8, 0x40000, is_5g ? 0x0 : 0x40000);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x50ec, 0xa00, is_1g ? 0x200 : 0x800);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x50a8, 0xee0000, is_5g ? 0x800000 :
>> +     0x6e0000);
>> +mtk_xfi_tphy_rmw(xfi_tphy, 0x6004, 0x190000, is_5g ? 0x0 :
>> 0x190000);
>> +if (is_10g)
>> +mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x01423342);
>> +else if (is_5g)
>> +mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x00a132a1);
>> +else if (is_2p5g)
>> +mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x009c329c);
>> +else
>> +mtk_xfi_tphy_write(xfi_tphy, 0x00f8, 0x00fa32fa);
> Still have many magic number.
> 

There's lack of documentation, that's why Daniel has all those magic numbers there.

If you can help unrolling some definitions that'd be really appreciated, as that
would result in a cleaner driver.

Cheers,
Angelo


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-02-05 15:29 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-01 21:52 [PATCH 1/2] dt-bindings: phy: mediatek,xfi-tphy: add new bindings Daniel Golle
2024-02-01 21:53 ` [PATCH 2/2] phy: add driver for MediaTek XFI T-PHY Daniel Golle
2024-02-02 13:21   ` AngeloGioacchino Del Regno
2024-02-02 16:33     ` Daniel Golle
2024-02-05 15:26       ` AngeloGioacchino Del Regno
2024-02-04  6:29   ` Chunfeng Yun (云春峰)
2024-02-05 15:29     ` AngeloGioacchino Del Regno
2024-02-02  8:21 ` [PATCH 1/2] dt-bindings: phy: mediatek,xfi-tphy: add new bindings Krzysztof Kozlowski
2024-02-04  6:17   ` Chunfeng Yun (云春峰)
2024-02-05  8:08     ` Krzysztof Kozlowski
2024-02-02 21:24 ` Rob Herring
2024-02-02 21:36   ` Daniel Golle

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).