diff mbox series

[net-next,v2,5/5] net: phy: add driver for built-in 2.5G ethernet PHY on MT7988

Message ID 20240517102908.12079-6-SkyLake.Huang@mediatek.com (mailing list archive)
State New
Headers show
Series net: phy: mediatek: Introduce mtk-phy-lib and add 2.5Gphy support | expand

Commit Message

SkyLake Huang (黃啟澤) May 17, 2024, 10:29 a.m. UTC
From: "SkyLake.Huang" <skylake.huang@mediatek.com>

v1:
Add support for internal 2.5Gphy on MT7988. This driver will load
necessary firmware, add appropriate time delay and figure out LED.
Also, certain control registers will be set to fix link-up issues.

v2:
1. Move md32_en_cfg_base & pmb_addr detection in probe function.
2. Do not read PMB & MD32_EN_CFG base addresses from dts. We won't
change that from board to board. Leave them in driver code. Also,
release those addresses after firmware is triggered.
3. Remove half duplex code which leads to ambiguity. Those are for
testing & developing previously.
4. Use correct BMCR definitions
5. Correct config_aneg / get_features / read_status functions
6. Change mt7988_2p5ge prefix to mt798x_2p5ge in case that our next
platform uses this 2.5Gphy driver

Signed-off-by: SkyLake.Huang <skylake.huang@mediatek.com>
---
 MAINTAINERS                          |   1 +
 drivers/net/phy/mediatek/Kconfig     |  11 +
 drivers/net/phy/mediatek/Makefile    |   1 +
 drivers/net/phy/mediatek/mtk-2p5ge.c | 400 +++++++++++++++++++++++++++
 4 files changed, 413 insertions(+)
 create mode 100644 drivers/net/phy/mediatek/mtk-2p5ge.c

Comments

Andrew Lunn May 18, 2024, 1:29 a.m. UTC | #1
> +static int mt798x_2p5ge_phy_config_init(struct phy_device *phydev)
> +{
> +	struct mtk_i2p5ge_phy_priv *priv = phydev->priv;
> +	struct device *dev = &phydev->mdio.dev;
> +	const struct firmware *fw;
> +	struct pinctrl *pinctrl;
> +	int ret, i;
> +	u16 reg;
> +
> +	if (!priv->fw_loaded) {
> +		if (!priv->md32_en_cfg_base || !priv->pmb_addr) {
> +			dev_err(dev, "MD32_EN_CFG base & PMB addresses aren't valid\n");
> +			return -EINVAL;
> +		}
> +
> +		ret = request_firmware(&fw, MT7988_2P5GE_PMB, dev);
> +		if (ret) {
> +			dev_err(dev, "failed to load firmware: %s, ret: %d\n",
> +				MT7988_2P5GE_PMB, ret);
> +			return ret;
> +		}
> +
> +		reg = readw(priv->md32_en_cfg_base);
> +		if (reg & MD32_EN) {
> +			phy_set_bits(phydev, MII_BMCR, BMCR_RESET);
> +			usleep_range(10000, 11000);
> +		}
> +		phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN);
> +
> +		/* Write magic number to safely stall MCU */
> +		phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x800e, 0x1100);
> +		phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x800f, 0x00df);
> +
> +		for (i = 0; i < fw->size - 1; i += 4)
> +			writel(*((uint32_t *)(fw->data + i)), priv->pmb_addr + i);

You should not trust the firmware. At least do a range check. How big
is the SRAM the firmware is being written into? If you are given a
firmware which is 1MB in size, what will happen?

> +		release_firmware(fw);
> +
> +		writew(reg & ~MD32_EN, priv->md32_en_cfg_base);
> +		writew(reg | MD32_EN, priv->md32_en_cfg_base);
> +		phy_set_bits(phydev, MII_BMCR, BMCR_RESET);
> +		/* We need a delay here to stabilize initialization of MCU */
> +		usleep_range(7000, 8000);
> +		dev_info(dev, "Firmware loading/trigger ok.\n");

Is there a version available anywhere for the firmware?

> +static int mt798x_2p5ge_phy_get_features(struct phy_device *phydev)
> +{
> +	int ret;
> +
> +	ret = genphy_c45_pma_read_abilities(phydev);
> +	if (ret)
> +		return ret;
> +
> +	/* We don't support HDX at MAC layer on mt7988.

That is a MAC limitation, so it should be the MAC which disables this,
not the Phy.

> +	/* FIXME: AN device (MDIO_DEVS_AN)is indeed in this package. However, MDIO_DEVS_AN seems
> +	 * that it won't be set as we detect phydev->c45_ids.mmds_present. So Autoneg_BIT won't be
> +	 * set in genphy_c45_pma_read_abilities(), either. Workaround here temporarily.
> +	 */
> +	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported);
> +
> +	return 0;
> +}
> +
> +static int mt798x_2p5ge_phy_read_status(struct phy_device *phydev)
> +{
> +	u16 bmsr;
> +	int ret;
> +
> +	/* Use this instead of genphy_c45_read_link() because MDIO_DEVS_AN bit isn't set in
> +	 * phydev->c45_ids.mmds_present.

You have this twice now. Is the hardware broken? If so, maybe change
phydev->c45_ids.mmds_present in the probe function to set the bit?

> +	 */
> +	ret = genphy_update_link(phydev);
> +	if (ret)
> +		return ret;
> +
> +	phydev->speed = SPEED_UNKNOWN;
> +	phydev->duplex = DUPLEX_UNKNOWN;
> +	phydev->pause = 0;
> +	phydev->asym_pause = 0;
> +
> +	/* We'll read link speed through vendor specific registers down below. So remove
> +	 * phy_resolve_aneg_linkmode (AN on) & genphy_c45_read_pma (AN off).
> +	 */
> +	if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) {
> +		ret = genphy_c45_read_lpa(phydev);
> +		if (ret < 0)
> +			return ret;
> +
> +		/* Clause 45 doesn't define 1000BaseT support. Read the link partner's 1G
> +		 * advertisement via Clause 22
> +		 */
> +		ret = phy_read(phydev, MII_STAT1000);
> +		if (ret < 0)
> +			return ret;
> +		mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising, ret);
> +	} else if (phydev->autoneg == AUTONEG_DISABLE) {
> +		/* Mask link partner's all advertising capabilities when AN is off. In fact,
> +		 * if we disable antuneg, we can't link up correctly:
> +		 *   2.5G/1G: Need AN to exchange master/slave information.
> +		 *   100M: Without AN, link starts at half duplex, which this phy doesn't support.
> +		 *   10M: Deprecated in this ethernet phy.
> +		 */

So it sounds like phydev->autoneg == AUTONEG_DISABLE is broken with
this hardware. So just don't allow it, return -EOPNOTSUPP in config_aneg()

     Andrew
SkyLake Huang (黃啟澤) May 20, 2024, 11:49 a.m. UTC | #2
On Sat, 2024-05-18 at 03:29 +0200, Andrew Lunn wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  > +static int mt798x_2p5ge_phy_config_init(struct phy_device
> *phydev)
> > +{
> > +struct mtk_i2p5ge_phy_priv *priv = phydev->priv;
> > +struct device *dev = &phydev->mdio.dev;
> > +const struct firmware *fw;
> > +struct pinctrl *pinctrl;
> > +int ret, i;
> > +u16 reg;
> > +
> > +if (!priv->fw_loaded) {
> > +if (!priv->md32_en_cfg_base || !priv->pmb_addr) {
> > +dev_err(dev, "MD32_EN_CFG base & PMB addresses aren't valid\n");
> > +return -EINVAL;
> > +}
> > +
> > +ret = request_firmware(&fw, MT7988_2P5GE_PMB, dev);
> > +if (ret) {
> > +dev_err(dev, "failed to load firmware: %s, ret: %d\n",
> > +MT7988_2P5GE_PMB, ret);
> > +return ret;
> > +}
> > +
> > +reg = readw(priv->md32_en_cfg_base);
> > +if (reg & MD32_EN) {
> > +phy_set_bits(phydev, MII_BMCR, BMCR_RESET);
> > +usleep_range(10000, 11000);
> > +}
> > +phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN);
> > +
> > +/* Write magic number to safely stall MCU */
> > +phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x800e, 0x1100);
> > +phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x800f, 0x00df);
> > +
> > +for (i = 0; i < fw->size - 1; i += 4)
> > +writel(*((uint32_t *)(fw->data + i)), priv->pmb_addr + i);
> 
> You should not trust the firmware. At least do a range check. How big
> is the SRAM the firmware is being written into? If you are given a
> firmware which is 1MB in size, what will happen?
> 
Fix in v3.

> > +release_firmware(fw);
> > +
> > +writew(reg & ~MD32_EN, priv->md32_en_cfg_base);
> > +writew(reg | MD32_EN, priv->md32_en_cfg_base);
> > +phy_set_bits(phydev, MII_BMCR, BMCR_RESET);
> > +/* We need a delay here to stabilize initialization of MCU */
> > +usleep_range(7000, 8000);
> > +dev_info(dev, "Firmware loading/trigger ok.\n");
> 
> Is there a version available anywhere for the firmware?
> 
Currently, I use "$md5sum /lib/firmware/mediatek/mt7988/i2p5ge-phy-
pmb.bin" command to check version.

> > +static int mt798x_2p5ge_phy_get_features(struct phy_device
> *phydev)
> > +{
> > +int ret;
> > +
> > +ret = genphy_c45_pma_read_abilities(phydev);
> > +if (ret)
> > +return ret;
> > +
> > +/* We don't support HDX at MAC layer on mt7988.
> 
> That is a MAC limitation, so it should be the MAC which disables
> this,
> not the Phy.
> 
Actually this phy is strictly binded to (XFI)MAC on this platform.
So I directly disable HDX feature of PHY.

> > +/* FIXME: AN device (MDIO_DEVS_AN)is indeed in this package.
> However, MDIO_DEVS_AN seems
> > + * that it won't be set as we detect phydev->c45_ids.mmds_present. 
> So Autoneg_BIT won't be
> > + * set in genphy_c45_pma_read_abilities(), either. Workaround here
> temporarily.
> > + */
> > +linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev-
> >supported);
> > +
> > +return 0;
> > +}
> > +
> > +static int mt798x_2p5ge_phy_read_status(struct phy_device *phydev)
> > +{
> > +u16 bmsr;
> > +int ret;
> > +
> > +/* Use this instead of genphy_c45_read_link() because MDIO_DEVS_AN
> bit isn't set in
> > + * phydev->c45_ids.mmds_present.
> 
> You have this twice now. Is the hardware broken? If so, maybe change
> phydev->c45_ids.mmds_present in the probe function to set the bit?
> 
Fix in v3.

> > + */
> > +ret = genphy_update_link(phydev);
> > +if (ret)
> > +return ret;
> > +
> > +phydev->speed = SPEED_UNKNOWN;
> > +phydev->duplex = DUPLEX_UNKNOWN;
> > +phydev->pause = 0;
> > +phydev->asym_pause = 0;
> > +
> > +/* We'll read link speed through vendor specific registers down
> below. So remove
> > + * phy_resolve_aneg_linkmode (AN on) & genphy_c45_read_pma (AN
> off).
> > + */
> > +if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) 
> {
> > +ret = genphy_c45_read_lpa(phydev);
> > +if (ret < 0)
> > +return ret;
> > +
> > +/* Clause 45 doesn't define 1000BaseT support. Read the link
> partner's 1G
> > + * advertisement via Clause 22
> > + */
> > +ret = phy_read(phydev, MII_STAT1000);
> > +if (ret < 0)
> > +return ret;
> > +mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising, ret);
> > +} else if (phydev->autoneg == AUTONEG_DISABLE) {
> > +/* Mask link partner's all advertising capabilities when AN is
> off. In fact,
> > + * if we disable antuneg, we can't link up correctly:
> > + *   2.5G/1G: Need AN to exchange master/slave information.
> > + *   100M: Without AN, link starts at half duplex, which this phy
> doesn't support.
> > + *   10M: Deprecated in this ethernet phy.
> > + */
> 
> So it sounds like phydev->autoneg == AUTONEG_DISABLE is broken with
> this hardware. So just don't allow it, return -EOPNOTSUPP in
> config_aneg()
> 
>      Andrew
Fix in v3.
Andrew Lunn May 20, 2024, 1:02 p.m. UTC | #3
> > Is there a version available anywhere for the firmware?
> > 
> Currently, I use "$md5sum /lib/firmware/mediatek/mt7988/i2p5ge-phy-
> pmb.bin" command to check version.

An md5sum is not really a version. How do you tell if one md5sum is
newer or older than another?

Is there is an MDIO register you can read to get the version? Or a
version in a header in the firmware file?

> > > +static int mt798x_2p5ge_phy_get_features(struct phy_device
> > *phydev)
> > > +{
> > > +int ret;
> > > +
> > > +ret = genphy_c45_pma_read_abilities(phydev);
> > > +if (ret)
> > > +return ret;
> > > +
> > > +/* We don't support HDX at MAC layer on mt7988.
> > 
> > That is a MAC limitation, so it should be the MAC which disables
> > this,
> > not the Phy.
> > 
> Actually this phy is strictly binded to (XFI)MAC on this platform.
> So I directly disable HDX feature of PHY.

Sorry, i don't follow your answer:

Can the PHY do half duplex?
Can the MAC do half duplex?

The part which cannot do half-duplex should be the part which disables
half-duplex.

You should be able to take any vendors MAC and any vendors PHY and put
them together, if they have the same MII interface. Say this PHY does
support half duplex. And some other vendors MAC supports half
duplex. We want it to work.

	Andrew
Russell King (Oracle) May 20, 2024, 5:48 p.m. UTC | #4
On Mon, May 20, 2024 at 03:02:21PM +0200, Andrew Lunn wrote:
> > Actually this phy is strictly binded to (XFI)MAC on this platform.
> > So I directly disable HDX feature of PHY.
> 
> Sorry, i don't follow your answer:
> 
> Can the PHY do half duplex?
> Can the MAC do half duplex?
> 
> The part which cannot do half-duplex should be the part which disables
> half-duplex.

Note that if the PHY is doing rate adaption (preferably in a form that
it can control the MAC transmission rate), then even if the MAC can't
do half-duplex, the PHY can still do half-duplex.
SkyLake Huang (黃啟澤) May 21, 2024, 9:36 a.m. UTC | #5
On Mon, 2024-05-20 at 18:48 +0100, Russell King (Oracle) wrote:
>  	 
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>  On Mon, May 20, 2024 at 03:02:21PM +0200, Andrew Lunn wrote:
> > > Actually this phy is strictly binded to (XFI)MAC on this
> platform.
> > > So I directly disable HDX feature of PHY.
> > 
> > Sorry, i don't follow your answer:
> > 
> > Can the PHY do half duplex?
> > Can the MAC do half duplex?

Sorry about ambiguity.
- Can the PHY do half duplex?
The short answer is no. I clarify the comments like below so everyone
should know this phy's capability

/* This phy can't handle collision, and neither can (XFI)MAC it's
connected to.
 * Although it can do HDX handshake, it doesn't support CSMA/CD that
HDX requires.
 */
linkmode_clear_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, phydev-
>supported);

- Can the MAC do half duplex?
No.
Daniel has explained MT7988 SoC's network interfaces in v1 patch:
https://lore.kernel.org/netdev/Zjumd3bnZDu9LYGH@makrotopia.org/

- GMAC0 is typically connected as conduit to a 4-port MT7530-like DSA
  switch offering 4 1GE user ports. PHY driver mediatek-ge-soc.c takes
  care of those, and yes, they do need some "care"...

- GMAC1 can be used with the internal 2.5GE PHY (ie. with the driver
  discussed here) OR for to connect an external PHY or SFP via
  1000Base-X, 2500Base-X, SGMII, 5GBase-R, 10GBase-R or USXGMII.

- GMAC2 is can only be used with an external PHY or SFP using
  1000Base-X, 2500Base-X, SGMII, 5GBase-R, 10GBa

Actually, internal 2.5GbE PHY is connected to XFIMAC via XMGII.
According to IEEE std 802.3-2018 section 46.1(page.188). XGMII provides
for full duplex operation only.

> > 
> > The part which cannot do half-duplex should be the part which
> disables
> > half-duplex.
> 
> Note that if the PHY is doing rate adaption (preferably in a form
> that
> it can control the MAC transmission rate), then even if the MAC can't
> do half-duplex, the PHY can still do half-duplex.
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Yes, this phy is doing rate adaption. But just like what I said above,
its rate adaption module can't support CSMA/CD.

Sky
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index e58e05c..fe380f2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13793,6 +13793,7 @@  M:	Qingfang Deng <dqfext@gmail.com>
 M:	SkyLake Huang <SkyLake.Huang@mediatek.com>
 L:	netdev@vger.kernel.org
 S:	Maintained
+F:	drivers/net/phy/mediatek/mtk-2p5ge.c
 F:	drivers/net/phy/mediatek/mtk-ge-soc.c
 F:	drivers/net/phy/mediatek/mtk-phy-lib.c
 F:	drivers/net/phy/mediatek/mtk-ge.c
diff --git a/drivers/net/phy/mediatek/Kconfig b/drivers/net/phy/mediatek/Kconfig
index b11b195..88ca881 100644
--- a/drivers/net/phy/mediatek/Kconfig
+++ b/drivers/net/phy/mediatek/Kconfig
@@ -25,3 +25,14 @@  config MEDIATEK_GE_SOC_PHY
 	  the MT7981 and MT7988 SoCs. These PHYs need calibration data
 	  present in the SoCs efuse and will dynamically calibrate VCM
 	  (common-mode voltage) during startup.
+
+config MEDIATEK_2P5GE_PHY
+	tristate "MediaTek 2.5Gb Ethernet PHYs"
+	depends on (ARM64 && ARCH_MEDIATEK) || COMPILE_TEST
+	select MTK_NET_PHYLIB
+	help
+	  Supports MediaTek SoC built-in 2.5Gb Ethernet PHYs.
+
+	  This will load necessary firmware and add appropriate time delay.
+	  Accelerate this procedure through internal pbus instead of MDIO
+	  bus. Certain link-up issues will also be fixed here.
diff --git a/drivers/net/phy/mediatek/Makefile b/drivers/net/phy/mediatek/Makefile
index 814879d..c6db8ab 100644
--- a/drivers/net/phy/mediatek/Makefile
+++ b/drivers/net/phy/mediatek/Makefile
@@ -2,3 +2,4 @@ 
 obj-$(CONFIG_MTK_NET_PHYLIB)		+= mtk-phy-lib.o
 obj-$(CONFIG_MEDIATEK_GE_PHY)		+= mtk-ge.o
 obj-$(CONFIG_MEDIATEK_GE_SOC_PHY)	+= mtk-ge-soc.o
+obj-$(CONFIG_MEDIATEK_2P5GE_PHY)	+= mtk-2p5ge.o
diff --git a/drivers/net/phy/mediatek/mtk-2p5ge.c b/drivers/net/phy/mediatek/mtk-2p5ge.c
new file mode 100644
index 0000000..ac04b6f
--- /dev/null
+++ b/drivers/net/phy/mediatek/mtk-2p5ge.c
@@ -0,0 +1,400 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+#include <linux/bitfield.h>
+#include <linux/firmware.h>
+#include <linux/module.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of_address.h>
+#include <linux/of_platform.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/phy.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_runtime.h>
+
+#include "mtk.h"
+
+#define MTK_2P5GPHY_ID_MT7988	(0x00339c11)
+
+#define MT7988_2P5GE_PMB "mediatek/mt7988/i2p5ge-phy-pmb.bin"
+#define MT7988_2P5GE_PMB_BASE	(0x0f100000)
+#define MT7988_2P5GE_PMB_LEN	(0x20000)
+#define MT7988_2P5GE_MD32_EN_CFG_BASE	(0x0f0f0018)
+#define MT7988_2P5GE_MD32_EN_CFG_LEN	(0x20)
+#define   MD32_EN			BIT(0)
+
+#define BASE100T_STATUS_EXTEND		(0x10)
+#define BASE1000T_STATUS_EXTEND		(0x11)
+#define EXTEND_CTRL_AND_STATUS		(0x16)
+
+#define PHY_AUX_CTRL_STATUS		(0x1d)
+#define   PHY_AUX_DPX_MASK		GENMASK(5, 5)
+#define   PHY_AUX_SPEED_MASK		GENMASK(4, 2)
+
+#define MTK_PHY_LPI_PCS_DSP_CTRL		(0x121)
+#define   MTK_PHY_LPI_SIG_EN_LO_THRESH100_MASK	GENMASK(12, 8)
+
+/* Registers on MTK phy page 1*/
+#define MTK_PHY_PAGE_EXTENDED_1			0x0001
+#define MTK_PHY_AUX_CTRL_AND_STATUS		(0x14)
+#define   MTK_PHY_ENABLE_DOWNSHIFT		BIT(4)
+
+/* Registers on Token Ring debug nodes */
+/* ch_addr = 0x0, node_addr = 0xf, data_addr = 0x3c */
+#define AUTO_NP_10XEN				BIT(6)
+
+struct mtk_i2p5ge_phy_priv {
+	bool fw_loaded;
+	void __iomem *pmb_addr;
+	void __iomem *md32_en_cfg_base;
+	unsigned long led_state;
+};
+
+enum {
+	PHY_AUX_SPD_10 = 0,
+	PHY_AUX_SPD_100,
+	PHY_AUX_SPD_1000,
+	PHY_AUX_SPD_2500,
+};
+
+static int mt798x_2p5ge_phy_config_init(struct phy_device *phydev)
+{
+	struct mtk_i2p5ge_phy_priv *priv = phydev->priv;
+	struct device *dev = &phydev->mdio.dev;
+	const struct firmware *fw;
+	struct pinctrl *pinctrl;
+	int ret, i;
+	u16 reg;
+
+	if (!priv->fw_loaded) {
+		if (!priv->md32_en_cfg_base || !priv->pmb_addr) {
+			dev_err(dev, "MD32_EN_CFG base & PMB addresses aren't valid\n");
+			return -EINVAL;
+		}
+
+		ret = request_firmware(&fw, MT7988_2P5GE_PMB, dev);
+		if (ret) {
+			dev_err(dev, "failed to load firmware: %s, ret: %d\n",
+				MT7988_2P5GE_PMB, ret);
+			return ret;
+		}
+
+		reg = readw(priv->md32_en_cfg_base);
+		if (reg & MD32_EN) {
+			phy_set_bits(phydev, MII_BMCR, BMCR_RESET);
+			usleep_range(10000, 11000);
+		}
+		phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN);
+
+		/* Write magic number to safely stall MCU */
+		phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x800e, 0x1100);
+		phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x800f, 0x00df);
+
+		for (i = 0; i < fw->size - 1; i += 4)
+			writel(*((uint32_t *)(fw->data + i)), priv->pmb_addr + i);
+		release_firmware(fw);
+
+		writew(reg & ~MD32_EN, priv->md32_en_cfg_base);
+		writew(reg | MD32_EN, priv->md32_en_cfg_base);
+		phy_set_bits(phydev, MII_BMCR, BMCR_RESET);
+		/* We need a delay here to stabilize initialization of MCU */
+		usleep_range(7000, 8000);
+		dev_info(dev, "Firmware loading/trigger ok.\n");
+
+		priv->fw_loaded = true;
+		iounmap(priv->md32_en_cfg_base);
+		iounmap(priv->pmb_addr);
+	}
+
+	/* Setup LED */
+	phy_set_bits_mmd(phydev, MDIO_MMD_VEND2, MTK_PHY_LED0_ON_CTRL,
+			 MTK_PHY_LED_ON_POLARITY | MTK_PHY_LED_ON_LINK10 |
+			 MTK_PHY_LED_ON_LINK100 | MTK_PHY_LED_ON_LINK1000 |
+			 MTK_PHY_LED_ON_LINK2500);
+	phy_set_bits_mmd(phydev, MDIO_MMD_VEND2, MTK_PHY_LED1_ON_CTRL,
+			 MTK_PHY_LED_ON_FDX | MTK_PHY_LED_ON_HDX);
+
+	pinctrl = devm_pinctrl_get_select(&phydev->mdio.dev, "i2p5gbe-led");
+	if (IS_ERR(pinctrl)) {
+		dev_err(&phydev->mdio.dev, "Fail to set LED pins!\n");
+		return PTR_ERR(pinctrl);
+	}
+
+	phy_modify_mmd(phydev, MDIO_MMD_VEND1, MTK_PHY_LPI_PCS_DSP_CTRL,
+		       MTK_PHY_LPI_SIG_EN_LO_THRESH100_MASK, 0);
+
+	/* Enable 16-bit next page exchange bit if 1000-BT isn't advertizing */
+	tr_modify(phydev, 0x0, 0xf, 0x3c, AUTO_NP_10XEN,
+		  FIELD_PREP(AUTO_NP_10XEN, 0x1));
+
+	/* Enable HW auto downshift */
+	phy_modify_paged(phydev, MTK_PHY_PAGE_EXTENDED_1, MTK_PHY_AUX_CTRL_AND_STATUS,
+			 0, MTK_PHY_ENABLE_DOWNSHIFT);
+
+	return 0;
+}
+
+static int mt798x_2p5ge_phy_config_aneg(struct phy_device *phydev)
+{
+	bool changed = false;
+	u32 adv;
+	int ret;
+
+	if (phydev->autoneg == AUTONEG_DISABLE)
+		return genphy_c45_pma_setup_forced(phydev);
+
+	ret = genphy_c45_an_config_aneg(phydev);
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		changed = true;
+
+	/* Clause 45 doesn't define 1000BaseT support. Use Clause 22 instead in our design.
+	 */
+	adv = linkmode_adv_to_mii_ctrl1000_t(phydev->advertising);
+	ret = phy_modify_changed(phydev, MII_CTRL1000, ADVERTISE_1000FULL, adv);
+	if (ret < 0)
+		return ret;
+	if (ret > 0)
+		changed = true;
+
+	return genphy_c45_check_and_restart_aneg(phydev, changed);
+}
+
+static int mt798x_2p5ge_phy_get_features(struct phy_device *phydev)
+{
+	int ret;
+
+	ret = genphy_c45_pma_read_abilities(phydev);
+	if (ret)
+		return ret;
+
+	/* We don't support HDX at MAC layer on mt7988. So mask phy's HDX capabilities here. */
+	linkmode_clear_bit(ETHTOOL_LINK_MODE_100baseT_Half_BIT, phydev->supported);
+	/* FIXME: AN device (MDIO_DEVS_AN)is indeed in this package. However, MDIO_DEVS_AN seems
+	 * that it won't be set as we detect phydev->c45_ids.mmds_present. So Autoneg_BIT won't be
+	 * set in genphy_c45_pma_read_abilities(), either. Workaround here temporarily.
+	 */
+	linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT, phydev->supported);
+
+	return 0;
+}
+
+static int mt798x_2p5ge_phy_read_status(struct phy_device *phydev)
+{
+	u16 bmsr;
+	int ret;
+
+	/* Use this instead of genphy_c45_read_link() because MDIO_DEVS_AN bit isn't set in
+	 * phydev->c45_ids.mmds_present.
+	 */
+	ret = genphy_update_link(phydev);
+	if (ret)
+		return ret;
+
+	phydev->speed = SPEED_UNKNOWN;
+	phydev->duplex = DUPLEX_UNKNOWN;
+	phydev->pause = 0;
+	phydev->asym_pause = 0;
+
+	/* We'll read link speed through vendor specific registers down below. So remove
+	 * phy_resolve_aneg_linkmode (AN on) & genphy_c45_read_pma (AN off).
+	 */
+	if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) {
+		ret = genphy_c45_read_lpa(phydev);
+		if (ret < 0)
+			return ret;
+
+		/* Clause 45 doesn't define 1000BaseT support. Read the link partner's 1G
+		 * advertisement via Clause 22
+		 */
+		ret = phy_read(phydev, MII_STAT1000);
+		if (ret < 0)
+			return ret;
+		mii_stat1000_mod_linkmode_lpa_t(phydev->lp_advertising, ret);
+	} else if (phydev->autoneg == AUTONEG_DISABLE) {
+		/* Mask link partner's all advertising capabilities when AN is off. In fact,
+		 * if we disable antuneg, we can't link up correctly:
+		 *   2.5G/1G: Need AN to exchange master/slave information.
+		 *   100M: Without AN, link starts at half duplex, which this phy doesn't support.
+		 *   10M: Deprecated in this ethernet phy.
+		 */
+		linkmode_zero(phydev->lp_advertising);
+	}
+
+	bmsr = phy_read(phydev, MII_BMSR);
+	if (bmsr & BMSR_LSTATUS) {
+		ret = phy_read(phydev, PHY_AUX_CTRL_STATUS);
+		if (ret < 0)
+			return ret;
+
+		switch (FIELD_GET(PHY_AUX_SPEED_MASK, ret)) {
+		case PHY_AUX_SPD_10:
+			phydev->speed = SPEED_10;
+			break;
+		case PHY_AUX_SPD_100:
+			phydev->speed = SPEED_100;
+			break;
+		case PHY_AUX_SPD_1000:
+			phydev->speed = SPEED_1000;
+			break;
+		case PHY_AUX_SPD_2500:
+			phydev->speed = SPEED_2500;
+			break;
+		}
+
+		phydev->duplex = DUPLEX_FULL;
+		/* FIXME: The current firmware always enables rate adaptation mode. */
+		phydev->rate_matching = RATE_MATCH_PAUSE;
+	}
+
+	return 0;
+}
+
+static int mt798x_2p5ge_phy_get_rate_matching(struct phy_device *phydev,
+					      phy_interface_t iface)
+{
+	if (iface == PHY_INTERFACE_MODE_XGMII)
+		return RATE_MATCH_PAUSE;
+	return RATE_MATCH_NONE;
+}
+
+static const unsigned long supported_triggers = (BIT(TRIGGER_NETDEV_FULL_DUPLEX) |
+						 BIT(TRIGGER_NETDEV_LINK)        |
+						 BIT(TRIGGER_NETDEV_LINK_10)     |
+						 BIT(TRIGGER_NETDEV_LINK_100)    |
+						 BIT(TRIGGER_NETDEV_LINK_1000)   |
+						 BIT(TRIGGER_NETDEV_LINK_2500)   |
+						 BIT(TRIGGER_NETDEV_RX)          |
+						 BIT(TRIGGER_NETDEV_TX));
+
+static int mt798x_2p5ge_phy_led_blink_set(struct phy_device *phydev, u8 index,
+					  unsigned long *delay_on,
+					  unsigned long *delay_off)
+{
+	bool blinking = false;
+	int err = 0;
+	struct mtk_i2p5ge_phy_priv *priv = phydev->priv;
+
+	if (index > 1)
+		return -EINVAL;
+
+	if (delay_on && delay_off && (*delay_on > 0) && (*delay_off > 0)) {
+		blinking = true;
+		*delay_on = 50;
+		*delay_off = 50;
+	}
+
+	err = mtk_phy_hw_led_blink_set(phydev, index, &priv->led_state, blinking);
+	if (err)
+		return err;
+
+	return mtk_phy_hw_led_on_set(phydev, index, &priv->led_state,
+				     MTK_2P5GPHY_LED_ON_MASK, false);
+}
+
+static int mt798x_2p5ge_phy_led_brightness_set(struct phy_device *phydev,
+					       u8 index, enum led_brightness value)
+{
+	int err;
+	struct mtk_i2p5ge_phy_priv *priv = phydev->priv;
+
+	err = mtk_phy_hw_led_blink_set(phydev, index, &priv->led_state, false);
+	if (err)
+		return err;
+
+	return mtk_phy_hw_led_on_set(phydev, index, &priv->led_state,
+				     MTK_2P5GPHY_LED_ON_MASK, (value != LED_OFF));
+}
+
+static int mt798x_2p5ge_phy_led_hw_is_supported(struct phy_device *phydev, u8 index,
+						unsigned long rules)
+{
+	return mtk_phy_led_hw_is_supported(phydev, index, rules, supported_triggers);
+}
+
+static int mt798x_2p5ge_phy_led_hw_control_get(struct phy_device *phydev, u8 index,
+					       unsigned long *rules)
+{
+	struct mtk_i2p5ge_phy_priv *priv = phydev->priv;
+
+	return mtk_phy_led_hw_ctrl_get(phydev, index, rules, &priv->led_state,
+				       MTK_2P5GPHY_LED_ON_SET,
+				       MTK_2P5GPHY_LED_RX_BLINK_SET,
+				       MTK_2P5GPHY_LED_TX_BLINK_SET);
+};
+
+static int mt798x_2p5ge_phy_led_hw_control_set(struct phy_device *phydev, u8 index,
+					       unsigned long rules)
+{
+	struct mtk_i2p5ge_phy_priv *priv = phydev->priv;
+
+	return mtk_phy_led_hw_ctrl_set(phydev, index, rules, &priv->led_state,
+				       MTK_2P5GPHY_LED_ON_SET,
+				       MTK_2P5GPHY_LED_RX_BLINK_SET,
+				       MTK_2P5GPHY_LED_TX_BLINK_SET);
+};
+
+static int mt798x_2p5ge_phy_probe(struct phy_device *phydev)
+{
+	struct mtk_i2p5ge_phy_priv *priv;
+
+	priv = devm_kzalloc(&phydev->mdio.dev,
+			    sizeof(struct mtk_i2p5ge_phy_priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	switch (phydev->drv->phy_id) {
+	case MTK_2P5GPHY_ID_MT7988:
+		priv->pmb_addr = ioremap(MT7988_2P5GE_PMB_BASE, MT7988_2P5GE_PMB_LEN);
+		if (!priv->pmb_addr)
+			return -ENOMEM;
+		priv->md32_en_cfg_base = ioremap(MT7988_2P5GE_MD32_EN_CFG_BASE,
+						 MT7988_2P5GE_MD32_EN_CFG_LEN);
+		if (!priv->md32_en_cfg_base)
+			return -ENOMEM;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	priv->fw_loaded = false;
+	phydev->priv = priv;
+
+	mtk_phy_leds_state_init(phydev);
+
+	return 0;
+}
+
+static struct phy_driver mtk_gephy_driver[] = {
+	{
+		PHY_ID_MATCH_MODEL(MTK_2P5GPHY_ID_MT7988),
+		.name		= "MediaTek MT798x 2.5GbE PHY",
+		.probe		= mt798x_2p5ge_phy_probe,
+		.config_init	= mt798x_2p5ge_phy_config_init,
+		.config_aneg    = mt798x_2p5ge_phy_config_aneg,
+		.get_features	= mt798x_2p5ge_phy_get_features,
+		.read_status	= mt798x_2p5ge_phy_read_status,
+		.get_rate_matching	= mt798x_2p5ge_phy_get_rate_matching,
+		.suspend	= genphy_suspend,
+		.resume		= genphy_resume,
+		.read_page	= mtk_phy_read_page,
+		.write_page	= mtk_phy_write_page,
+		.led_blink_set	= mt798x_2p5ge_phy_led_blink_set,
+		.led_brightness_set = mt798x_2p5ge_phy_led_brightness_set,
+		.led_hw_is_supported = mt798x_2p5ge_phy_led_hw_is_supported,
+		.led_hw_control_get = mt798x_2p5ge_phy_led_hw_control_get,
+		.led_hw_control_set = mt798x_2p5ge_phy_led_hw_control_set,
+	},
+};
+
+module_phy_driver(mtk_gephy_driver);
+
+static struct mdio_device_id __maybe_unused mtk_2p5ge_phy_tbl[] = {
+	{ PHY_ID_MATCH_VENDOR(0x00339c00) },
+	{ }
+};
+
+MODULE_DESCRIPTION("MediaTek 2.5Gb Ethernet PHY driver");
+MODULE_AUTHOR("SkyLake Huang <SkyLake.Huang@mediatek.com>");
+MODULE_LICENSE("GPL");
+
+MODULE_DEVICE_TABLE(mdio, mtk_2p5ge_phy_tbl);