From: "SkyLake.Huang" <[email protected]>
v2:
- Apply correct PATCH tag.
- Break LED/Token ring/Extend-link-pulse-time features into 3 patches.
- Fix contents according to v1 comments.
v3:
- Rebase code and now this patch series can apply to net-next tree.
[PATCH 4/5]
Refactor mtk_gphy_cl22_read_status() with genphy_read_status().
[PATCH 5/5]
1. Add range check for firmware.
2. Fix c45_ids.mmds_present in probe function.
3. Still use genphy_update_link() in read_status because
genphy_c45_read_link() can't correct detect link on this phy.
v4:
[PATCH 4/5]
1. Change extend_an_new_lp_cnt_limit()'s return type and all return values
2. Refactor comments in extend_an_new_lp_cnt_limit()
[PATCH 5/5]
1. Move firmware loading function to mt798x_2p5ge_phy_load_fw()
2. Add AN disable warning in mt798x_2p5ge_phy_config_aneg()
3. Clarify the HDX comments in mt798x_2p5ge_phy_get_features()
v5:
- Fix syntax errors of comments in drivers/net/phy/mediatek/*
[PATCH 1/5]
- Change MEDIATEK_GE_SOC_PHY from bool back to tristate.
[PATCH 5/5]
1. Move md32_en_cfg_base & pmb_addr to local variables to achieve
symmetric code.
2. Print out firmware date code & version.
3. Don't return error if LED pinctrl switching fails. Also, add
comments to this unusual operations.
4. Return -EOPNOTSUPP for AN off case in config_aneg().
SkyLake.Huang (5):
net: phy: mediatek: Re-organize MediaTek ethernet phy drivers
net: phy: mediatek: Move LED and read/write page helper functions into
mtk phy lib
net: phy: mediatek: Add token ring access helper functions in
mtk-phy-lib
net: phy: mediatek: Extend 1G TX/RX link pulse time
net: phy: add driver for built-in 2.5G ethernet PHY on MT7988
MAINTAINERS | 7 +-
drivers/net/phy/Kconfig | 17 +-
drivers/net/phy/Makefile | 3 +-
drivers/net/phy/mediatek-ge.c | 111 ----
drivers/net/phy/mediatek/Kconfig | 38 ++
drivers/net/phy/mediatek/Makefile | 5 +
drivers/net/phy/mediatek/mtk-2p5ge.c | 423 ++++++++++++++
.../mtk-ge-soc.c} | 528 ++++++------------
drivers/net/phy/mediatek/mtk-ge.c | 244 ++++++++
drivers/net/phy/mediatek/mtk-phy-lib.c | 385 +++++++++++++
drivers/net/phy/mediatek/mtk.h | 108 ++++
11 files changed, 1391 insertions(+), 478 deletions(-)
delete mode 100644 drivers/net/phy/mediatek-ge.c
create mode 100644 drivers/net/phy/mediatek/Kconfig
create mode 100644 drivers/net/phy/mediatek/Makefile
create mode 100644 drivers/net/phy/mediatek/mtk-2p5ge.c
rename drivers/net/phy/{mediatek-ge-soc.c => mediatek/mtk-ge-soc.c} (77%)
create mode 100644 drivers/net/phy/mediatek/mtk-ge.c
create mode 100644 drivers/net/phy/mediatek/mtk-phy-lib.c
create mode 100644 drivers/net/phy/mediatek/mtk.h
--
2.18.0
From: "SkyLake.Huang" <[email protected]>
Re-organize MediaTek ethernet phy driver files and get ready to integrate
some common functions and add new 2.5G phy driver.
mtk-ge.c: MT7530 Gphy on MT7621 & MT7531 Gphy
mtk-ge-soc.c: Built-in Gphy on MT7981 & Built-in switch Gphy on MT7988
mtk-2p5ge.c: Planned for built-in 2.5G phy on MT7988
v5:
Change MEDIATEK_GE_SOC_PHY from bool back to tristate.
Signed-off-by: SkyLake.Huang <[email protected]>
---
MAINTAINERS | 4 ++--
drivers/net/phy/Kconfig | 17 +-------------
drivers/net/phy/Makefile | 3 +--
drivers/net/phy/mediatek/Kconfig | 22 +++++++++++++++++++
drivers/net/phy/mediatek/Makefile | 3 +++
.../mtk-ge-soc.c} | 2 +-
.../phy/{mediatek-ge.c => mediatek/mtk-ge.c} | 0
7 files changed, 30 insertions(+), 21 deletions(-)
create mode 100644 drivers/net/phy/mediatek/Kconfig
create mode 100644 drivers/net/phy/mediatek/Makefile
rename drivers/net/phy/{mediatek-ge-soc.c => mediatek/mtk-ge-soc.c} (99%)
rename drivers/net/phy/{mediatek-ge.c => mediatek/mtk-ge.c} (100%)
diff --git a/MAINTAINERS b/MAINTAINERS
index e291445..6deaf94 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13793,8 +13793,8 @@ M: Qingfang Deng <[email protected]>
M: SkyLake Huang <[email protected]>
L: [email protected]
S: Maintained
-F: drivers/net/phy/mediatek-ge-soc.c
-F: drivers/net/phy/mediatek-ge.c
+F: drivers/net/phy/mediatek/mtk-ge-soc.c
+F: drivers/net/phy/mediatek/mtk-ge.c
F: drivers/phy/mediatek/phy-mtk-xfi-tphy.c
MEDIATEK I2C CONTROLLER DRIVER
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 1df0595..e0e4b5e 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -251,22 +251,7 @@ config MAXLINEAR_GPHY
Support for the Maxlinear GPY115, GPY211, GPY212, GPY215,
GPY241, GPY245 PHYs.
-config MEDIATEK_GE_PHY
- tristate "MediaTek Gigabit Ethernet PHYs"
- help
- Supports the MediaTek Gigabit Ethernet PHYs.
-
-config MEDIATEK_GE_SOC_PHY
- tristate "MediaTek SoC Ethernet PHYs"
- depends on (ARM64 && ARCH_MEDIATEK) || COMPILE_TEST
- depends on NVMEM_MTK_EFUSE
- help
- Supports MediaTek SoC built-in Gigabit Ethernet PHYs.
-
- Include support for built-in Ethernet PHYs which are present in
- 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.
+source "drivers/net/phy/mediatek/Kconfig"
config MICREL_PHY
tristate "Micrel PHYs"
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 197acfa..de38cbf 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -71,8 +71,7 @@ obj-$(CONFIG_MARVELL_PHY) += marvell.o
obj-$(CONFIG_MARVELL_88Q2XXX_PHY) += marvell-88q2xxx.o
obj-$(CONFIG_MARVELL_88X2222_PHY) += marvell-88x2222.o
obj-$(CONFIG_MAXLINEAR_GPHY) += mxl-gpy.o
-obj-$(CONFIG_MEDIATEK_GE_PHY) += mediatek-ge.o
-obj-$(CONFIG_MEDIATEK_GE_SOC_PHY) += mediatek-ge-soc.o
+obj-y += mediatek/
obj-$(CONFIG_MESON_GXL_PHY) += meson-gxl.o
obj-$(CONFIG_MICREL_KS8995MA) += spi_ks8995.o
obj-$(CONFIG_MICREL_PHY) += micrel.o
diff --git a/drivers/net/phy/mediatek/Kconfig b/drivers/net/phy/mediatek/Kconfig
new file mode 100644
index 0000000..6839ea6
--- /dev/null
+++ b/drivers/net/phy/mediatek/Kconfig
@@ -0,0 +1,22 @@
+# SPDX-License-Identifier: GPL-2.0-only
+config MEDIATEK_GE_PHY
+ tristate "MediaTek Gigabit Ethernet PHYs"
+ help
+ Supports the MediaTek non-built-in Gigabit Ethernet PHYs.
+
+ Non-built-in Gigabit Ethernet PHYs include mt7530/mt7531.
+ You may find mt7530 inside mt7621. This driver shares some
+ common operations with MediaTek SoC built-in Gigabit
+ Ethernet PHYs.
+
+config MEDIATEK_GE_SOC_PHY
+ tristate "MediaTek SoC Ethernet PHYs"
+ depends on (ARM64 && ARCH_MEDIATEK) || COMPILE_TEST
+ select NVMEM_MTK_EFUSE
+ help
+ Supports MediaTek SoC built-in Gigabit Ethernet PHYs.
+
+ Include support for built-in Ethernet PHYs which are present in
+ 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.
diff --git a/drivers/net/phy/mediatek/Makefile b/drivers/net/phy/mediatek/Makefile
new file mode 100644
index 0000000..005bde2
--- /dev/null
+++ b/drivers/net/phy/mediatek/Makefile
@@ -0,0 +1,3 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_MEDIATEK_GE_PHY) += mtk-ge.o
+obj-$(CONFIG_MEDIATEK_GE_SOC_PHY) += mtk-ge-soc.o
diff --git a/drivers/net/phy/mediatek-ge-soc.c b/drivers/net/phy/mediatek/mtk-ge-soc.c
similarity index 99%
rename from drivers/net/phy/mediatek-ge-soc.c
rename to drivers/net/phy/mediatek/mtk-ge-soc.c
index f4f9412..47af872 100644
--- a/drivers/net/phy/mediatek-ge-soc.c
+++ b/drivers/net/phy/mediatek/mtk-ge-soc.c
@@ -1415,7 +1415,7 @@ static int mt7988_phy_probe_shared(struct phy_device *phydev)
* LED_C and LED_D respectively. At the same time those pins are used to
* bootstrap configuration of the reference clock source (LED_A),
* DRAM DDRx16b x2/x1 (LED_B) and boot device (LED_C, LED_D).
- * In practise this is done using a LED and a resistor pulling the pin
+ * In practice this is done using a LED and a resistor pulling the pin
* either to GND or to VIO.
* The detected value at boot time is accessible at run-time using the
* TPBANK0 register located in the gpio base of the pinctrl, in order
diff --git a/drivers/net/phy/mediatek-ge.c b/drivers/net/phy/mediatek/mtk-ge.c
similarity index 100%
rename from drivers/net/phy/mediatek-ge.c
rename to drivers/net/phy/mediatek/mtk-ge.c
--
2.18.0
From: "SkyLake.Huang" <[email protected]>
This patch moves mtk-ge-soc.c's LED code into mtk phy lib. We
can use those helper functions in mtk-ge.c as well. That is to
say, we have almost the same HW LED controller design in
mt7530/mt7531/mt7981/mt7988's Giga ethernet phy.
Also integrate read/write pages into one helper function. They
are basically the same.
Signed-off-by: SkyLake.Huang <[email protected]>
---
MAINTAINERS | 2 +
drivers/net/phy/mediatek/Kconfig | 5 +
drivers/net/phy/mediatek/Makefile | 1 +
drivers/net/phy/mediatek/mtk-ge-soc.c | 267 +++----------------------
drivers/net/phy/mediatek/mtk-ge.c | 128 ++++++++++--
drivers/net/phy/mediatek/mtk-phy-lib.c | 231 +++++++++++++++++++++
drivers/net/phy/mediatek/mtk.h | 82 ++++++++
7 files changed, 456 insertions(+), 260 deletions(-)
create mode 100644 drivers/net/phy/mediatek/mtk-phy-lib.c
create mode 100644 drivers/net/phy/mediatek/mtk.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 6deaf94..e58e05c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13794,7 +13794,9 @@ M: SkyLake Huang <[email protected]>
L: [email protected]
S: Maintained
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
+F: drivers/net/phy/mediatek/mtk.h
F: drivers/phy/mediatek/phy-mtk-xfi-tphy.c
MEDIATEK I2C CONTROLLER DRIVER
diff --git a/drivers/net/phy/mediatek/Kconfig b/drivers/net/phy/mediatek/Kconfig
index 6839ea6..448bc20 100644
--- a/drivers/net/phy/mediatek/Kconfig
+++ b/drivers/net/phy/mediatek/Kconfig
@@ -1,6 +1,10 @@
# SPDX-License-Identifier: GPL-2.0-only
+config MTK_NET_PHYLIB
+ tristate
+
config MEDIATEK_GE_PHY
tristate "MediaTek Gigabit Ethernet PHYs"
+ select MTK_NET_PHYLIB
help
Supports the MediaTek non-built-in Gigabit Ethernet PHYs.
@@ -13,6 +17,7 @@ config MEDIATEK_GE_SOC_PHY
tristate "MediaTek SoC Ethernet PHYs"
depends on (ARM64 && ARCH_MEDIATEK) || COMPILE_TEST
select NVMEM_MTK_EFUSE
+ select MTK_NET_PHYLIB
help
Supports MediaTek SoC built-in Gigabit Ethernet PHYs.
diff --git a/drivers/net/phy/mediatek/Makefile b/drivers/net/phy/mediatek/Makefile
index 005bde2..814879d 100644
--- a/drivers/net/phy/mediatek/Makefile
+++ b/drivers/net/phy/mediatek/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0
+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
diff --git a/drivers/net/phy/mediatek/mtk-ge-soc.c b/drivers/net/phy/mediatek/mtk-ge-soc.c
index 47af872..ee83b27 100644
--- a/drivers/net/phy/mediatek/mtk-ge-soc.c
+++ b/drivers/net/phy/mediatek/mtk-ge-soc.c
@@ -8,6 +8,8 @@
#include <linux/phy.h>
#include <linux/regmap.h>
+#include "mtk.h"
+
#define MTK_GPHY_ID_MT7981 0x03a29461
#define MTK_GPHY_ID_MT7988 0x03a29481
@@ -210,41 +212,6 @@
#define MTK_PHY_DA_TX_R50_PAIR_D 0x540
/* Registers on MDIO_MMD_VEND2 */
-#define MTK_PHY_LED0_ON_CTRL 0x24
-#define MTK_PHY_LED1_ON_CTRL 0x26
-#define MTK_PHY_LED_ON_MASK GENMASK(6, 0)
-#define MTK_PHY_LED_ON_LINK1000 BIT(0)
-#define MTK_PHY_LED_ON_LINK100 BIT(1)
-#define MTK_PHY_LED_ON_LINK10 BIT(2)
-#define MTK_PHY_LED_ON_LINK (MTK_PHY_LED_ON_LINK10 |\
- MTK_PHY_LED_ON_LINK100 |\
- MTK_PHY_LED_ON_LINK1000)
-#define MTK_PHY_LED_ON_LINKDOWN BIT(3)
-#define MTK_PHY_LED_ON_FDX BIT(4) /* Full duplex */
-#define MTK_PHY_LED_ON_HDX BIT(5) /* Half duplex */
-#define MTK_PHY_LED_ON_FORCE_ON BIT(6)
-#define MTK_PHY_LED_ON_POLARITY BIT(14)
-#define MTK_PHY_LED_ON_ENABLE BIT(15)
-
-#define MTK_PHY_LED0_BLINK_CTRL 0x25
-#define MTK_PHY_LED1_BLINK_CTRL 0x27
-#define MTK_PHY_LED_BLINK_1000TX BIT(0)
-#define MTK_PHY_LED_BLINK_1000RX BIT(1)
-#define MTK_PHY_LED_BLINK_100TX BIT(2)
-#define MTK_PHY_LED_BLINK_100RX BIT(3)
-#define MTK_PHY_LED_BLINK_10TX BIT(4)
-#define MTK_PHY_LED_BLINK_10RX BIT(5)
-#define MTK_PHY_LED_BLINK_RX (MTK_PHY_LED_BLINK_10RX |\
- MTK_PHY_LED_BLINK_100RX |\
- MTK_PHY_LED_BLINK_1000RX)
-#define MTK_PHY_LED_BLINK_TX (MTK_PHY_LED_BLINK_10TX |\
- MTK_PHY_LED_BLINK_100TX |\
- MTK_PHY_LED_BLINK_1000TX)
-#define MTK_PHY_LED_BLINK_COLLISION BIT(6)
-#define MTK_PHY_LED_BLINK_RX_CRC_ERR BIT(7)
-#define MTK_PHY_LED_BLINK_RX_IDLE_ERR BIT(8)
-#define MTK_PHY_LED_BLINK_FORCE_BLINK BIT(9)
-
#define MTK_PHY_LED1_DEFAULT_POLARITIES BIT(1)
#define MTK_PHY_RG_BG_RASEL 0x115
@@ -299,10 +266,6 @@ enum CAL_MODE {
SW_M
};
-#define MTK_PHY_LED_STATE_FORCE_ON 0
-#define MTK_PHY_LED_STATE_FORCE_BLINK 1
-#define MTK_PHY_LED_STATE_NETDEV 2
-
struct mtk_socphy_priv {
unsigned long led_state;
};
@@ -312,16 +275,6 @@ struct mtk_socphy_shared {
struct mtk_socphy_priv priv[4];
};
-static int mtk_socphy_read_page(struct phy_device *phydev)
-{
- return __phy_read(phydev, MTK_EXT_PAGE_ACCESS);
-}
-
-static int mtk_socphy_write_page(struct phy_device *phydev, int page)
-{
- return __phy_write(phydev, MTK_EXT_PAGE_ACCESS, page);
-}
-
/* One calibration cycle consists of:
* 1.Set DA_CALIN_FLAG high to start calibration. Keep it high
* until AD_CAL_COMP is ready to output calibration result.
@@ -1130,57 +1083,13 @@ static int mt798x_phy_config_init(struct phy_device *phydev)
return mt798x_phy_calibration(phydev);
}
-static int mt798x_phy_hw_led_on_set(struct phy_device *phydev, u8 index,
- bool on)
-{
- unsigned int bit_on = MTK_PHY_LED_STATE_FORCE_ON + (index ? 16 : 0);
- struct mtk_socphy_priv *priv = phydev->priv;
- bool changed;
-
- if (on)
- changed = !test_and_set_bit(bit_on, &priv->led_state);
- else
- changed = !!test_and_clear_bit(bit_on, &priv->led_state);
-
- changed |= !!test_and_clear_bit(MTK_PHY_LED_STATE_NETDEV +
- (index ? 16 : 0), &priv->led_state);
- if (changed)
- return phy_modify_mmd(phydev, MDIO_MMD_VEND2, index ?
- MTK_PHY_LED1_ON_CTRL : MTK_PHY_LED0_ON_CTRL,
- MTK_PHY_LED_ON_MASK,
- on ? MTK_PHY_LED_ON_FORCE_ON : 0);
- else
- return 0;
-}
-
-static int mt798x_phy_hw_led_blink_set(struct phy_device *phydev, u8 index,
- bool blinking)
-{
- unsigned int bit_blink = MTK_PHY_LED_STATE_FORCE_BLINK + (index ? 16 : 0);
- struct mtk_socphy_priv *priv = phydev->priv;
- bool changed;
-
- if (blinking)
- changed = !test_and_set_bit(bit_blink, &priv->led_state);
- else
- changed = !!test_and_clear_bit(bit_blink, &priv->led_state);
-
- changed |= !!test_bit(MTK_PHY_LED_STATE_NETDEV +
- (index ? 16 : 0), &priv->led_state);
- if (changed)
- return phy_write_mmd(phydev, MDIO_MMD_VEND2, index ?
- MTK_PHY_LED1_BLINK_CTRL : MTK_PHY_LED0_BLINK_CTRL,
- blinking ? MTK_PHY_LED_BLINK_FORCE_BLINK : 0);
- else
- return 0;
-}
-
static int mt798x_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_socphy_priv *priv = phydev->priv;
if (index > 1)
return -EINVAL;
@@ -1191,23 +1100,25 @@ static int mt798x_phy_led_blink_set(struct phy_device *phydev, u8 index,
*delay_off = 50;
}
- err = mt798x_phy_hw_led_blink_set(phydev, index, blinking);
+ err = mtk_phy_hw_led_blink_set(phydev, index, &priv->led_state, blinking);
if (err)
return err;
- return mt798x_phy_hw_led_on_set(phydev, index, false);
+ return mtk_phy_hw_led_on_set(phydev, index, &priv->led_state, MTK_GPHY_LED_ON_MASK, false);
}
static int mt798x_phy_led_brightness_set(struct phy_device *phydev,
u8 index, enum led_brightness value)
{
int err;
+ struct mtk_socphy_priv *priv = phydev->priv;
- err = mt798x_phy_hw_led_blink_set(phydev, index, false);
+ err = mtk_phy_hw_led_blink_set(phydev, index, &priv->led_state, false);
if (err)
return err;
- return mt798x_phy_hw_led_on_set(phydev, index, (value != LED_OFF));
+ return mtk_phy_hw_led_on_set(phydev, index, &priv->led_state,
+ MTK_GPHY_LED_ON_MASK, (value != LED_OFF));
}
static const unsigned long supported_triggers = (BIT(TRIGGER_NETDEV_FULL_DUPLEX) |
@@ -1219,151 +1130,29 @@ static const unsigned long supported_triggers = (BIT(TRIGGER_NETDEV_FULL_DUPLEX)
BIT(TRIGGER_NETDEV_RX) |
BIT(TRIGGER_NETDEV_TX));
-static int mt798x_phy_led_hw_is_supported(struct phy_device *phydev, u8 index,
- unsigned long rules)
+static int mt798x_phy_led_hw_is_supported(struct phy_device *phydev, u8 index, unsigned long rules)
{
- if (index > 1)
- return -EINVAL;
-
- /* All combinations of the supported triggers are allowed */
- if (rules & ~supported_triggers)
- return -EOPNOTSUPP;
-
- return 0;
-};
+ return mtk_phy_led_hw_is_supported(phydev, index, rules, supported_triggers);
+}
static int mt798x_phy_led_hw_control_get(struct phy_device *phydev, u8 index,
unsigned long *rules)
{
- unsigned int bit_blink = MTK_PHY_LED_STATE_FORCE_BLINK + (index ? 16 : 0);
- unsigned int bit_netdev = MTK_PHY_LED_STATE_NETDEV + (index ? 16 : 0);
- unsigned int bit_on = MTK_PHY_LED_STATE_FORCE_ON + (index ? 16 : 0);
struct mtk_socphy_priv *priv = phydev->priv;
- int on, blink;
-
- if (index > 1)
- return -EINVAL;
-
- on = phy_read_mmd(phydev, MDIO_MMD_VEND2,
- index ? MTK_PHY_LED1_ON_CTRL : MTK_PHY_LED0_ON_CTRL);
-
- if (on < 0)
- return -EIO;
-
- blink = phy_read_mmd(phydev, MDIO_MMD_VEND2,
- index ? MTK_PHY_LED1_BLINK_CTRL :
- MTK_PHY_LED0_BLINK_CTRL);
- if (blink < 0)
- return -EIO;
- if ((on & (MTK_PHY_LED_ON_LINK | MTK_PHY_LED_ON_FDX | MTK_PHY_LED_ON_HDX |
- MTK_PHY_LED_ON_LINKDOWN)) ||
- (blink & (MTK_PHY_LED_BLINK_RX | MTK_PHY_LED_BLINK_TX)))
- set_bit(bit_netdev, &priv->led_state);
- else
- clear_bit(bit_netdev, &priv->led_state);
-
- if (on & MTK_PHY_LED_ON_FORCE_ON)
- set_bit(bit_on, &priv->led_state);
- else
- clear_bit(bit_on, &priv->led_state);
-
- if (blink & MTK_PHY_LED_BLINK_FORCE_BLINK)
- set_bit(bit_blink, &priv->led_state);
- else
- clear_bit(bit_blink, &priv->led_state);
-
- if (!rules)
- return 0;
-
- if (on & MTK_PHY_LED_ON_LINK)
- *rules |= BIT(TRIGGER_NETDEV_LINK);
-
- if (on & MTK_PHY_LED_ON_LINK10)
- *rules |= BIT(TRIGGER_NETDEV_LINK_10);
-
- if (on & MTK_PHY_LED_ON_LINK100)
- *rules |= BIT(TRIGGER_NETDEV_LINK_100);
-
- if (on & MTK_PHY_LED_ON_LINK1000)
- *rules |= BIT(TRIGGER_NETDEV_LINK_1000);
-
- if (on & MTK_PHY_LED_ON_FDX)
- *rules |= BIT(TRIGGER_NETDEV_FULL_DUPLEX);
-
- if (on & MTK_PHY_LED_ON_HDX)
- *rules |= BIT(TRIGGER_NETDEV_HALF_DUPLEX);
-
- if (blink & MTK_PHY_LED_BLINK_RX)
- *rules |= BIT(TRIGGER_NETDEV_RX);
-
- if (blink & MTK_PHY_LED_BLINK_TX)
- *rules |= BIT(TRIGGER_NETDEV_TX);
-
- return 0;
+ return mtk_phy_led_hw_ctrl_get(phydev, index, rules, &priv->led_state,
+ MTK_GPHY_LED_ON_SET,
+ MTK_GPHY_LED_RX_BLINK_SET, MTK_GPHY_LED_TX_BLINK_SET);
};
static int mt798x_phy_led_hw_control_set(struct phy_device *phydev, u8 index,
unsigned long rules)
{
- unsigned int bit_netdev = MTK_PHY_LED_STATE_NETDEV + (index ? 16 : 0);
struct mtk_socphy_priv *priv = phydev->priv;
- u16 on = 0, blink = 0;
- int ret;
-
- if (index > 1)
- return -EINVAL;
- if (rules & BIT(TRIGGER_NETDEV_FULL_DUPLEX))
- on |= MTK_PHY_LED_ON_FDX;
-
- if (rules & BIT(TRIGGER_NETDEV_HALF_DUPLEX))
- on |= MTK_PHY_LED_ON_HDX;
-
- if (rules & (BIT(TRIGGER_NETDEV_LINK_10) | BIT(TRIGGER_NETDEV_LINK)))
- on |= MTK_PHY_LED_ON_LINK10;
-
- if (rules & (BIT(TRIGGER_NETDEV_LINK_100) | BIT(TRIGGER_NETDEV_LINK)))
- on |= MTK_PHY_LED_ON_LINK100;
-
- if (rules & (BIT(TRIGGER_NETDEV_LINK_1000) | BIT(TRIGGER_NETDEV_LINK)))
- on |= MTK_PHY_LED_ON_LINK1000;
-
- if (rules & BIT(TRIGGER_NETDEV_RX)) {
- blink |= (on & MTK_PHY_LED_ON_LINK) ?
- (((on & MTK_PHY_LED_ON_LINK10) ? MTK_PHY_LED_BLINK_10RX : 0) |
- ((on & MTK_PHY_LED_ON_LINK100) ? MTK_PHY_LED_BLINK_100RX : 0) |
- ((on & MTK_PHY_LED_ON_LINK1000) ? MTK_PHY_LED_BLINK_1000RX : 0)) :
- MTK_PHY_LED_BLINK_RX;
- }
-
- if (rules & BIT(TRIGGER_NETDEV_TX)) {
- blink |= (on & MTK_PHY_LED_ON_LINK) ?
- (((on & MTK_PHY_LED_ON_LINK10) ? MTK_PHY_LED_BLINK_10TX : 0) |
- ((on & MTK_PHY_LED_ON_LINK100) ? MTK_PHY_LED_BLINK_100TX : 0) |
- ((on & MTK_PHY_LED_ON_LINK1000) ? MTK_PHY_LED_BLINK_1000TX : 0)) :
- MTK_PHY_LED_BLINK_TX;
- }
-
- if (blink || on)
- set_bit(bit_netdev, &priv->led_state);
- else
- clear_bit(bit_netdev, &priv->led_state);
-
- ret = phy_modify_mmd(phydev, MDIO_MMD_VEND2, index ?
- MTK_PHY_LED1_ON_CTRL :
- MTK_PHY_LED0_ON_CTRL,
- MTK_PHY_LED_ON_FDX |
- MTK_PHY_LED_ON_HDX |
- MTK_PHY_LED_ON_LINK,
- on);
-
- if (ret)
- return ret;
-
- return phy_write_mmd(phydev, MDIO_MMD_VEND2, index ?
- MTK_PHY_LED1_BLINK_CTRL :
- MTK_PHY_LED0_BLINK_CTRL, blink);
+ return mtk_phy_led_hw_ctrl_set(phydev, index, rules, &priv->led_state,
+ MTK_GPHY_LED_ON_SET,
+ MTK_GPHY_LED_RX_BLINK_SET, MTK_GPHY_LED_TX_BLINK_SET);
};
static bool mt7988_phy_led_get_polarity(struct phy_device *phydev, int led_num)
@@ -1437,14 +1226,6 @@ static int mt7988_phy_probe_shared(struct phy_device *phydev)
return 0;
}
-static void mt798x_phy_leds_state_init(struct phy_device *phydev)
-{
- int i;
-
- for (i = 0; i < 2; ++i)
- mt798x_phy_led_hw_control_get(phydev, i, NULL);
-}
-
static int mt7988_phy_probe(struct phy_device *phydev)
{
struct mtk_socphy_shared *shared;
@@ -1470,7 +1251,7 @@ static int mt7988_phy_probe(struct phy_device *phydev)
phydev->priv = priv;
- mt798x_phy_leds_state_init(phydev);
+ mtk_phy_leds_state_init(phydev);
err = mt7988_phy_fix_leds_polarities(phydev);
if (err)
@@ -1497,7 +1278,7 @@ static int mt7981_phy_probe(struct phy_device *phydev)
phydev->priv = priv;
- mt798x_phy_leds_state_init(phydev);
+ mtk_phy_leds_state_init(phydev);
return mt798x_phy_calibration(phydev);
}
@@ -1512,8 +1293,8 @@ static struct phy_driver mtk_socphy_driver[] = {
.probe = mt7981_phy_probe,
.suspend = genphy_suspend,
.resume = genphy_resume,
- .read_page = mtk_socphy_read_page,
- .write_page = mtk_socphy_write_page,
+ .read_page = mtk_phy_read_page,
+ .write_page = mtk_phy_write_page,
.led_blink_set = mt798x_phy_led_blink_set,
.led_brightness_set = mt798x_phy_led_brightness_set,
.led_hw_is_supported = mt798x_phy_led_hw_is_supported,
@@ -1529,8 +1310,8 @@ static struct phy_driver mtk_socphy_driver[] = {
.probe = mt7988_phy_probe,
.suspend = genphy_suspend,
.resume = genphy_resume,
- .read_page = mtk_socphy_read_page,
- .write_page = mtk_socphy_write_page,
+ .read_page = mtk_phy_read_page,
+ .write_page = mtk_phy_write_page,
.led_blink_set = mt798x_phy_led_blink_set,
.led_brightness_set = mt798x_phy_led_brightness_set,
.led_hw_is_supported = mt798x_phy_led_hw_is_supported,
diff --git a/drivers/net/phy/mediatek/mtk-ge.c b/drivers/net/phy/mediatek/mtk-ge.c
index 54ea64a..80425d6 100644
--- a/drivers/net/phy/mediatek/mtk-ge.c
+++ b/drivers/net/phy/mediatek/mtk-ge.c
@@ -3,6 +3,11 @@
#include <linux/module.h>
#include <linux/phy.h>
+#include "mtk.h"
+
+#define MTK_GPHY_ID_MT7530 0x03a29412
+#define MTK_GPHY_ID_MT7531 0x03a29441
+
#define MTK_EXT_PAGE_ACCESS 0x1f
#define MTK_PHY_PAGE_STANDARD 0x0000
#define MTK_PHY_PAGE_EXTENDED 0x0001
@@ -11,15 +16,9 @@
#define MTK_PHY_PAGE_EXTENDED_2A30 0x2a30
#define MTK_PHY_PAGE_EXTENDED_52B5 0x52b5
-static int mtk_gephy_read_page(struct phy_device *phydev)
-{
- return __phy_read(phydev, MTK_EXT_PAGE_ACCESS);
-}
-
-static int mtk_gephy_write_page(struct phy_device *phydev, int page)
-{
- return __phy_write(phydev, MTK_EXT_PAGE_ACCESS, page);
-}
+struct mtk_gephy_priv {
+ unsigned long led_state;
+};
static void mtk_gephy_config_init(struct phy_device *phydev)
{
@@ -65,9 +64,97 @@ static int mt7531_phy_config_init(struct phy_device *phydev)
return 0;
}
+static int mt7531_phy_probe(struct phy_device *phydev)
+{
+ struct mtk_gephy_priv *priv;
+
+ priv = devm_kzalloc(&phydev->mdio.dev, sizeof(struct mtk_gephy_priv),
+ GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ phydev->priv = priv;
+
+ mtk_phy_leds_state_init(phydev);
+
+ return 0;
+}
+
+static int mt753x_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_gephy_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_GPHY_LED_ON_MASK, false);
+}
+
+static int mt753x_phy_led_brightness_set(struct phy_device *phydev,
+ u8 index, enum led_brightness value)
+{
+ int err;
+ struct mtk_gephy_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_GPHY_LED_ON_MASK, (value != LED_OFF));
+}
+
+static const unsigned long supported_triggers = (BIT(TRIGGER_NETDEV_FULL_DUPLEX) |
+ BIT(TRIGGER_NETDEV_HALF_DUPLEX) |
+ BIT(TRIGGER_NETDEV_LINK) |
+ BIT(TRIGGER_NETDEV_LINK_10) |
+ BIT(TRIGGER_NETDEV_LINK_100) |
+ BIT(TRIGGER_NETDEV_LINK_1000) |
+ BIT(TRIGGER_NETDEV_RX) |
+ BIT(TRIGGER_NETDEV_TX));
+
+static int mt753x_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 mt753x_phy_led_hw_control_get(struct phy_device *phydev, u8 index,
+ unsigned long *rules)
+{
+ struct mtk_gephy_priv *priv = phydev->priv;
+
+ return mtk_phy_led_hw_ctrl_get(phydev, index, rules, &priv->led_state,
+ MTK_GPHY_LED_ON_SET,
+ MTK_GPHY_LED_RX_BLINK_SET, MTK_GPHY_LED_TX_BLINK_SET);
+};
+
+static int mt753x_phy_led_hw_control_set(struct phy_device *phydev, u8 index,
+ unsigned long rules)
+{
+ struct mtk_gephy_priv *priv = phydev->priv;
+
+ return mtk_phy_led_hw_ctrl_set(phydev, index, rules, &priv->led_state,
+ MTK_GPHY_LED_ON_SET,
+ MTK_GPHY_LED_RX_BLINK_SET, MTK_GPHY_LED_TX_BLINK_SET);
+};
+
static struct phy_driver mtk_gephy_driver[] = {
{
- PHY_ID_MATCH_EXACT(0x03a29412),
+ PHY_ID_MATCH_EXACT(MTK_GPHY_ID_MT7530),
.name = "MediaTek MT7530 PHY",
.config_init = mt7530_phy_config_init,
/* Interrupts are handled by the switch, not the PHY
@@ -77,12 +164,14 @@ static struct phy_driver mtk_gephy_driver[] = {
.handle_interrupt = genphy_handle_interrupt_no_ack,
.suspend = genphy_suspend,
.resume = genphy_resume,
- .read_page = mtk_gephy_read_page,
- .write_page = mtk_gephy_write_page,
+ .read_page = mtk_phy_read_page,
+ .write_page = mtk_phy_write_page,
+ .led_hw_is_supported = mt753x_phy_led_hw_is_supported,
},
{
- PHY_ID_MATCH_EXACT(0x03a29441),
+ PHY_ID_MATCH_EXACT(MTK_GPHY_ID_MT7531),
.name = "MediaTek MT7531 PHY",
+ .probe = mt7531_phy_probe,
.config_init = mt7531_phy_config_init,
/* Interrupts are handled by the switch, not the PHY
* itself.
@@ -91,16 +180,21 @@ static struct phy_driver mtk_gephy_driver[] = {
.handle_interrupt = genphy_handle_interrupt_no_ack,
.suspend = genphy_suspend,
.resume = genphy_resume,
- .read_page = mtk_gephy_read_page,
- .write_page = mtk_gephy_write_page,
+ .read_page = mtk_phy_read_page,
+ .write_page = mtk_phy_write_page,
+ .led_blink_set = mt753x_phy_led_blink_set,
+ .led_brightness_set = mt753x_phy_led_brightness_set,
+ .led_hw_is_supported = mt753x_phy_led_hw_is_supported,
+ .led_hw_control_set = mt753x_phy_led_hw_control_set,
+ .led_hw_control_get = mt753x_phy_led_hw_control_get,
},
};
module_phy_driver(mtk_gephy_driver);
static struct mdio_device_id __maybe_unused mtk_gephy_tbl[] = {
- { PHY_ID_MATCH_EXACT(0x03a29441) },
- { PHY_ID_MATCH_EXACT(0x03a29412) },
+ { PHY_ID_MATCH_EXACT(MTK_GPHY_ID_MT7530) },
+ { PHY_ID_MATCH_EXACT(MTK_GPHY_ID_MT7531) },
{ }
};
diff --git a/drivers/net/phy/mediatek/mtk-phy-lib.c b/drivers/net/phy/mediatek/mtk-phy-lib.c
new file mode 100644
index 0000000..4608837
--- /dev/null
+++ b/drivers/net/phy/mediatek/mtk-phy-lib.c
@@ -0,0 +1,231 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/phy.h>
+#include <linux/module.h>
+
+#include <linux/netdevice.h>
+
+#include "mtk.h"
+
+int mtk_phy_read_page(struct phy_device *phydev)
+{
+ return __phy_read(phydev, MTK_EXT_PAGE_ACCESS);
+}
+EXPORT_SYMBOL_GPL(mtk_phy_read_page);
+
+int mtk_phy_write_page(struct phy_device *phydev, int page)
+{
+ return __phy_write(phydev, MTK_EXT_PAGE_ACCESS, page);
+}
+EXPORT_SYMBOL_GPL(mtk_phy_write_page);
+
+int mtk_phy_led_hw_is_supported(struct phy_device *phydev, u8 index, unsigned long rules,
+ unsigned long supported_triggers)
+{
+ if (index > 1)
+ return -EINVAL;
+
+ /* All combinations of the supported triggers are allowed */
+ if (rules & ~supported_triggers)
+ return -EOPNOTSUPP;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mtk_phy_led_hw_is_supported);
+
+int mtk_phy_led_hw_ctrl_get(struct phy_device *phydev, u8 index, unsigned long *rules,
+ unsigned long *led_state, u16 on_set,
+ u16 rx_blink_set, u16 tx_blink_set)
+{
+ unsigned int bit_blink = MTK_PHY_LED_STATE_FORCE_BLINK + (index ? 16 : 0);
+ unsigned int bit_netdev = MTK_PHY_LED_STATE_NETDEV + (index ? 16 : 0);
+ unsigned int bit_on = MTK_PHY_LED_STATE_FORCE_ON + (index ? 16 : 0);
+ int on, blink;
+
+ if (index > 1)
+ return -EINVAL;
+
+ on = phy_read_mmd(phydev, MDIO_MMD_VEND2,
+ index ? MTK_PHY_LED1_ON_CTRL : MTK_PHY_LED0_ON_CTRL);
+
+ if (on < 0)
+ return -EIO;
+
+ blink = phy_read_mmd(phydev, MDIO_MMD_VEND2,
+ index ? MTK_PHY_LED1_BLINK_CTRL :
+ MTK_PHY_LED0_BLINK_CTRL);
+ if (blink < 0)
+ return -EIO;
+
+ if ((on & (on_set | MTK_PHY_LED_ON_FDX | MTK_PHY_LED_ON_HDX | MTK_PHY_LED_ON_LINKDOWN)) ||
+ (blink & (rx_blink_set | tx_blink_set)))
+ set_bit(bit_netdev, led_state);
+ else
+ clear_bit(bit_netdev, led_state);
+
+ if (on & MTK_PHY_LED_ON_FORCE_ON)
+ set_bit(bit_on, led_state);
+ else
+ clear_bit(bit_on, led_state);
+
+ if (blink & MTK_PHY_LED_BLINK_FORCE_BLINK)
+ set_bit(bit_blink, led_state);
+ else
+ clear_bit(bit_blink, led_state);
+
+ if (!rules)
+ return 0;
+
+ if (on & on_set)
+ *rules |= BIT(TRIGGER_NETDEV_LINK);
+
+ if (on & MTK_PHY_LED_ON_LINK10)
+ *rules |= BIT(TRIGGER_NETDEV_LINK_10);
+
+ if (on & MTK_PHY_LED_ON_LINK100)
+ *rules |= BIT(TRIGGER_NETDEV_LINK_100);
+
+ if (on & MTK_PHY_LED_ON_LINK1000)
+ *rules |= BIT(TRIGGER_NETDEV_LINK_1000);
+
+ if (on & MTK_PHY_LED_ON_LINK2500)
+ *rules |= BIT(TRIGGER_NETDEV_LINK_2500);
+
+ if (on & MTK_PHY_LED_ON_FDX)
+ *rules |= BIT(TRIGGER_NETDEV_FULL_DUPLEX);
+
+ if (on & MTK_PHY_LED_ON_HDX)
+ *rules |= BIT(TRIGGER_NETDEV_HALF_DUPLEX);
+
+ if (blink & rx_blink_set)
+ *rules |= BIT(TRIGGER_NETDEV_RX);
+
+ if (blink & tx_blink_set)
+ *rules |= BIT(TRIGGER_NETDEV_TX);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mtk_phy_led_hw_ctrl_get);
+
+int mtk_phy_led_hw_ctrl_set(struct phy_device *phydev, u8 index, unsigned long rules,
+ unsigned long *led_state, u16 on_set,
+ u16 rx_blink_set, u16 tx_blink_set)
+{
+ unsigned int bit_netdev = MTK_PHY_LED_STATE_NETDEV + (index ? 16 : 0);
+ u16 on = 0, blink = 0;
+ int ret;
+
+ if (index > 1)
+ return -EINVAL;
+
+ if (rules & BIT(TRIGGER_NETDEV_FULL_DUPLEX))
+ on |= MTK_PHY_LED_ON_FDX;
+
+ if (rules & BIT(TRIGGER_NETDEV_HALF_DUPLEX))
+ on |= MTK_PHY_LED_ON_HDX;
+
+ if (rules & (BIT(TRIGGER_NETDEV_LINK_10) | BIT(TRIGGER_NETDEV_LINK)))
+ on |= MTK_PHY_LED_ON_LINK10;
+
+ if (rules & (BIT(TRIGGER_NETDEV_LINK_100) | BIT(TRIGGER_NETDEV_LINK)))
+ on |= MTK_PHY_LED_ON_LINK100;
+
+ if (rules & (BIT(TRIGGER_NETDEV_LINK_1000) | BIT(TRIGGER_NETDEV_LINK)))
+ on |= MTK_PHY_LED_ON_LINK1000;
+
+ if (rules & (BIT(TRIGGER_NETDEV_LINK_2500) | BIT(TRIGGER_NETDEV_LINK)))
+ on |= MTK_PHY_LED_ON_LINK2500;
+
+ if (rules & BIT(TRIGGER_NETDEV_RX)) {
+ blink |= (on & on_set) ?
+ (((on & MTK_PHY_LED_ON_LINK10) ? MTK_PHY_LED_BLINK_10RX : 0) |
+ ((on & MTK_PHY_LED_ON_LINK100) ? MTK_PHY_LED_BLINK_100RX : 0) |
+ ((on & MTK_PHY_LED_ON_LINK1000) ? MTK_PHY_LED_BLINK_1000RX : 0) |
+ ((on & MTK_PHY_LED_ON_LINK2500) ? MTK_PHY_LED_BLINK_2500RX : 0)) :
+ rx_blink_set;
+ }
+
+ if (rules & BIT(TRIGGER_NETDEV_TX)) {
+ blink |= (on & on_set) ?
+ (((on & MTK_PHY_LED_ON_LINK10) ? MTK_PHY_LED_BLINK_10TX : 0) |
+ ((on & MTK_PHY_LED_ON_LINK100) ? MTK_PHY_LED_BLINK_100TX : 0) |
+ ((on & MTK_PHY_LED_ON_LINK1000) ? MTK_PHY_LED_BLINK_1000TX : 0) |
+ ((on & MTK_PHY_LED_ON_LINK2500) ? MTK_PHY_LED_BLINK_2500TX : 0)) :
+ tx_blink_set;
+ }
+
+ if (blink || on)
+ set_bit(bit_netdev, led_state);
+ else
+ clear_bit(bit_netdev, led_state);
+
+ ret = phy_modify_mmd(phydev, MDIO_MMD_VEND2, index ?
+ MTK_PHY_LED1_ON_CTRL : MTK_PHY_LED0_ON_CTRL,
+ MTK_PHY_LED_ON_FDX | MTK_PHY_LED_ON_HDX | on_set, on);
+
+ if (ret)
+ return ret;
+
+ return phy_write_mmd(phydev, MDIO_MMD_VEND2, index ?
+ MTK_PHY_LED1_BLINK_CTRL :
+ MTK_PHY_LED0_BLINK_CTRL, blink);
+}
+EXPORT_SYMBOL_GPL(mtk_phy_led_hw_ctrl_set);
+
+int mtk_phy_hw_led_on_set(struct phy_device *phydev, u8 index, unsigned long *led_state,
+ u16 led_on_mask, bool on)
+{
+ unsigned int bit_on = MTK_PHY_LED_STATE_FORCE_ON + (index ? 16 : 0);
+ bool changed;
+
+ if (on)
+ changed = !test_and_set_bit(bit_on, led_state);
+ else
+ changed = !!test_and_clear_bit(bit_on, led_state);
+
+ changed |= !!test_and_clear_bit(MTK_PHY_LED_STATE_NETDEV +
+ (index ? 16 : 0), led_state);
+ if (changed)
+ return phy_modify_mmd(phydev, MDIO_MMD_VEND2, index ?
+ MTK_PHY_LED1_ON_CTRL : MTK_PHY_LED0_ON_CTRL,
+ led_on_mask,
+ on ? MTK_PHY_LED_ON_FORCE_ON : 0);
+ else
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mtk_phy_hw_led_on_set);
+
+int mtk_phy_hw_led_blink_set(struct phy_device *phydev, u8 index, unsigned long *led_state,
+ bool blinking)
+{
+ unsigned int bit_blink = MTK_PHY_LED_STATE_FORCE_BLINK + (index ? 16 : 0);
+ bool changed;
+
+ if (blinking)
+ changed = !test_and_set_bit(bit_blink, led_state);
+ else
+ changed = !!test_and_clear_bit(bit_blink, led_state);
+
+ changed |= !!test_bit(MTK_PHY_LED_STATE_NETDEV +
+ (index ? 16 : 0), led_state);
+ if (changed)
+ return phy_write_mmd(phydev, MDIO_MMD_VEND2, index ?
+ MTK_PHY_LED1_BLINK_CTRL : MTK_PHY_LED0_BLINK_CTRL,
+ blinking ? MTK_PHY_LED_BLINK_FORCE_BLINK : 0);
+ else
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mtk_phy_hw_led_blink_set);
+
+void mtk_phy_leds_state_init(struct phy_device *phydev)
+{
+ int i;
+
+ for (i = 0; i < 2; ++i)
+ phydev->drv->led_hw_control_get(phydev, i, NULL);
+}
+EXPORT_SYMBOL_GPL(mtk_phy_leds_state_init);
+
+MODULE_DESCRIPTION("MediaTek Ethernet PHY driver common");
+MODULE_AUTHOR("Sky Huang <[email protected]>");
+MODULE_AUTHOR("Daniel Golle <[email protected]>");
+MODULE_LICENSE("GPL");
diff --git a/drivers/net/phy/mediatek/mtk.h b/drivers/net/phy/mediatek/mtk.h
new file mode 100644
index 0000000..c392c38
--- /dev/null
+++ b/drivers/net/phy/mediatek/mtk.h
@@ -0,0 +1,82 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Common definition for Mediatek Ethernet PHYs
+ * Author: SkyLake Huang <[email protected]>
+ * Copyright (c) 2024 MediaTek Inc.
+ */
+
+#ifndef _MTK_EPHY_H_
+#define _MTK_EPHY_H_
+
+#define MTK_EXT_PAGE_ACCESS 0x1f
+
+/* Registers on MDIO_MMD_VEND2 */
+#define MTK_PHY_LED0_ON_CTRL 0x24
+#define MTK_PHY_LED1_ON_CTRL 0x26
+#define MTK_GPHY_LED_ON_MASK GENMASK(6, 0)
+#define MTK_2P5GPHY_LED_ON_MASK GENMASK(7, 0)
+#define MTK_PHY_LED_ON_LINK1000 BIT(0)
+#define MTK_PHY_LED_ON_LINK100 BIT(1)
+#define MTK_PHY_LED_ON_LINK10 BIT(2)
+#define MTK_PHY_LED_ON_LINKDOWN BIT(3)
+#define MTK_PHY_LED_ON_FDX BIT(4) /* Full duplex */
+#define MTK_PHY_LED_ON_HDX BIT(5) /* Half duplex */
+#define MTK_PHY_LED_ON_FORCE_ON BIT(6)
+#define MTK_PHY_LED_ON_LINK2500 BIT(7)
+#define MTK_PHY_LED_ON_POLARITY BIT(14)
+#define MTK_PHY_LED_ON_ENABLE BIT(15)
+
+#define MTK_PHY_LED0_BLINK_CTRL 0x25
+#define MTK_PHY_LED1_BLINK_CTRL 0x27
+#define MTK_PHY_LED_BLINK_1000TX BIT(0)
+#define MTK_PHY_LED_BLINK_1000RX BIT(1)
+#define MTK_PHY_LED_BLINK_100TX BIT(2)
+#define MTK_PHY_LED_BLINK_100RX BIT(3)
+#define MTK_PHY_LED_BLINK_10TX BIT(4)
+#define MTK_PHY_LED_BLINK_10RX BIT(5)
+#define MTK_PHY_LED_BLINK_COLLISION BIT(6)
+#define MTK_PHY_LED_BLINK_RX_CRC_ERR BIT(7)
+#define MTK_PHY_LED_BLINK_RX_IDLE_ERR BIT(8)
+#define MTK_PHY_LED_BLINK_FORCE_BLINK BIT(9)
+#define MTK_PHY_LED_BLINK_2500TX BIT(10)
+#define MTK_PHY_LED_BLINK_2500RX BIT(11)
+
+#define MTK_GPHY_LED_ON_SET (MTK_PHY_LED_ON_LINK1000 | \
+ MTK_PHY_LED_ON_LINK100 | \
+ MTK_PHY_LED_ON_LINK10)
+#define MTK_GPHY_LED_RX_BLINK_SET (MTK_PHY_LED_BLINK_1000RX | \
+ MTK_PHY_LED_BLINK_100RX | \
+ MTK_PHY_LED_BLINK_10RX)
+#define MTK_GPHY_LED_TX_BLINK_SET (MTK_PHY_LED_BLINK_1000RX | \
+ MTK_PHY_LED_BLINK_100RX | \
+ MTK_PHY_LED_BLINK_10RX)
+
+#define MTK_2P5GPHY_LED_ON_SET (MTK_PHY_LED_ON_LINK2500 | \
+ MTK_GPHY_LED_ON_SET)
+#define MTK_2P5GPHY_LED_RX_BLINK_SET (MTK_PHY_LED_BLINK_2500RX | \
+ MTK_GPHY_LED_RX_BLINK_SET)
+#define MTK_2P5GPHY_LED_TX_BLINK_SET (MTK_PHY_LED_BLINK_2500RX | \
+ MTK_GPHY_LED_TX_BLINK_SET)
+
+#define MTK_PHY_LED_STATE_FORCE_ON 0
+#define MTK_PHY_LED_STATE_FORCE_BLINK 1
+#define MTK_PHY_LED_STATE_NETDEV 2
+
+int mtk_phy_read_page(struct phy_device *phydev);
+int mtk_phy_write_page(struct phy_device *phydev, int page);
+
+int mtk_phy_led_hw_is_supported(struct phy_device *phydev, u8 index, unsigned long rules,
+ unsigned long supported_triggers);
+int mtk_phy_led_hw_ctrl_set(struct phy_device *phydev, u8 index, unsigned long rules,
+ unsigned long *led_state, u16 on_set,
+ u16 rx_blink_set, u16 tx_blink_set);
+int mtk_phy_led_hw_ctrl_get(struct phy_device *phydev, u8 index, unsigned long *rules,
+ unsigned long *led_state, u16 on_set,
+ u16 rx_blink_set, u16 tx_blink_set);
+int mtk_phy_hw_led_on_set(struct phy_device *phydev, u8 index, unsigned long *led_state,
+ u16 led_on_mask, bool on);
+int mtk_phy_hw_led_blink_set(struct phy_device *phydev, u8 index, unsigned long *led_state,
+ bool blinking);
+void mtk_phy_leds_state_init(struct phy_device *phydev);
+
+#endif /* _MTK_EPHY_H_ */
--
2.18.0
From: "SkyLake.Huang" <[email protected]>
This patch adda TR(token ring) manipulations and add correct
macro name for those magic numbers. TR is a way to access
proprietary register on page 52b5. Use these helper functions
so we can see which fields we're going to modify/set/clear.
This patch doesn't really change registers' settings but just
enhances readability and maintainability.
Signed-off-by: SkyLake.Huang <[email protected]>
---
drivers/net/phy/mediatek/mtk-ge-soc.c | 259 +++++++++++++++----------
drivers/net/phy/mediatek/mtk-ge.c | 80 ++++++--
drivers/net/phy/mediatek/mtk-phy-lib.c | 88 +++++++++
drivers/net/phy/mediatek/mtk.h | 10 +
4 files changed, 312 insertions(+), 125 deletions(-)
diff --git a/drivers/net/phy/mediatek/mtk-ge-soc.c b/drivers/net/phy/mediatek/mtk-ge-soc.c
index ee83b27..c566bf9 100644
--- a/drivers/net/phy/mediatek/mtk-ge-soc.c
+++ b/drivers/net/phy/mediatek/mtk-ge-soc.c
@@ -24,7 +24,78 @@
#define MTK_PHY_SMI_DET_ON_THRESH_MASK GENMASK(13, 8)
#define MTK_PHY_PAGE_EXTENDED_2A30 0x2a30
-#define MTK_PHY_PAGE_EXTENDED_52B5 0x52b5
+
+/* Registers on Token Ring debug nodes */
+/* ch_addr = 0x0, node_addr = 0x7, data_addr = 0x15 */
+#define NORMAL_MSE_LO_THRESH_MASK GENMASK(15, 8) /* NormMseLoThresh */
+
+/* ch_addr = 0x0, node_addr = 0xf, data_addr = 0x3c */
+#define REMOTE_ACK_COUNT_LIMIT_CTRL_MASK GENMASK(2, 1) /* RemAckCntLimitCtrl */
+
+/* ch_addr = 0x1, node_addr = 0xd, data_addr = 0x20 */
+#define VCO_SLICER_THRESH_HIGH_MASK GENMASK(23, 0) /* VcoSlicerThreshBitsHigh */
+
+/* ch_addr = 0x1, node_addr = 0xf, data_addr = 0x0 */
+#define DFE_TAIL_EANBLE_VGA_TRHESH_1000 GENMASK(5, 1) /* DfeTailEnableVgaThresh1000 */
+
+/* ch_addr = 0x1, node_addr = 0xf, data_addr = 0x1 */
+#define MRVL_TR_FIX_100KP_MASK GENMASK(22, 20) /* MrvlTrFix100Kp */
+#define MRVL_TR_FIX_100KF_MASK GENMASK(19, 17) /* MrvlTrFix100Kf */
+#define MRVL_TR_FIX_1000KP_MASK GENMASK(16, 14) /* MrvlTrFix1000Kp */
+#define MRVL_TR_FIX_1000KF_MASK GENMASK(13, 11) /* MrvlTrFix1000Kf */
+
+/* ch_addr = 0x1, node_addr = 0xf, data_addr = 0x12 */
+#define VGA_DECIMATION_RATE_MASK GENMASK(8, 5) /* VgaDecRate */
+
+/* ch_addr = 0x1, node_addr = 0xf, data_addr = 0x17 */
+#define SLAVE_DSP_READY_TIME_MASK GENMASK(22, 15) /* SlvDSPreadyTime */
+#define MASTER_DSP_READY_TIME_MASK GENMASK(14, 7) /* MasDSPreadyTime */
+
+/* ch_addr = 0x1, node_addr = 0xf, data_addr = 0x18 */
+#define ENABLE_RANDOM_UPDOWN_COUNTER_TRIGGER BIT(8) /* EnabRandUpdTrig */
+
+/* ch_addr = 0x1, node_addr = 0xf, data_addr = 0x20 */
+#define RESET_SYNC_OFFSET_MASK GENMASK(11, 8) /* ResetSyncOffset */
+
+/* ch_addr = 0x2, node_addr = 0xd, data_addr = 0x0 */
+#define FFE_UPDATE_GAIN_FORCE_VAL_MASK GENMASK(9, 7) /* FfeUpdGainForceVal */
+#define FFE_UPDATE_GAIN_FORCE BIT(6) /* FfeUpdGainForce */
+
+/* ch_addr = 0x2, node_addr = 0xd, data_addr = 0x3 */
+#define TR_FREEZE_MASK GENMASK(11, 0) /* TrFreeze */
+
+/* ch_addr = 0x2, node_addr = 0xd, data_addr = 0x6 */
+/* SS: Steady-state, KP: Proportional Gain */
+#define SS_TR_KP100_MASK GENMASK(21, 19) /* SSTrKp100 */
+#define SS_TR_KF100_MASK GENMASK(18, 16) /* SSTrKf100 */
+#define SS_TR_KP1000_MASTER_MASK GENMASK(15, 13) /* SSTrKp1000Mas */
+#define SS_TR_KF1000_MASTER_MASK GENMASK(12, 10) /* SSTrKf1000Mas */
+#define SS_TR_KP1000_SLAVE_MASK GENMASK(9, 7) /* SSTrKp1000Slv */
+#define SS_TR_KF1000_SLAVE_MASK GENMASK(6, 4) /* SSTrKf1000Slv */
+
+/* ch_addr = 0x2, node_addr = 0xd, data_addr = 0x8 */
+/* clear this bit if wanna select from AFE */
+#define EEE1000_SELECT_SIGNEL_DETECTION_FROM_DFE BIT(4) /* Regsigdet_sel_1000 */
+
+/* ch_addr = 0x2, node_addr = 0xd, data_addr = 0xd */
+#define EEE1000_STAGE2_TR_KF_MASK GENMASK(13, 11) /* RegEEE_st2TrKf1000 */
+
+/* ch_addr = 0x2, node_addr = 0xd, data_addr = 0xf */
+#define SLAVE_WAKETR_TIMER_MASK GENMASK(20, 11) /* RegEEE_slv_waketr_timer_tar */
+#define SLAVE_REMTX_TIMER_MASK GENMASK(10, 1) /* RegEEE_slv_remtx_timer_tar */
+
+/* ch_addr = 0x2, node_addr = 0xd, data_addr = 0x10 */
+#define SLAVE_WAKEINT_TIMER_MASK GENMASK(10, 1) /* RegEEE_slv_wake_int_timer_tar */
+
+/* ch_addr = 0x2, node_addr = 0xd, data_addr = 0x14 */
+#define TR_FREEZE_TIMER2_MASK GENMASK(9, 0) /* RegEEE_trfreeze_timer2 */
+
+/* ch_addr = 0x2, node_addr = 0xd, data_addr = 0x1c */
+#define EEE100_LPSYNC_STAGE1_UPDATE_TIMER_MASK GENMASK(8, 0) /* RegEEE100Stg1_tar */
+
+/* ch_addr = 0x2, node_addr = 0xd, data_addr = 0x25 */
+#define WAKE_SLAVE_TR_WAIT_DFE_DETECTION_EN BIT(11) /* REGEEE_wake_slv_tr_wait_dfesigdet_en */
+
#define ANALOG_INTERNAL_OPERATION_MAX_US 20
#define TXRESERVE_MIN 0
@@ -667,40 +738,34 @@ static int tx_vcm_cal_sw(struct phy_device *phydev, u8 rg_txreserve_x)
static void mt798x_phy_common_finetune(struct phy_device *phydev)
{
phy_select_page(phydev, MTK_PHY_PAGE_EXTENDED_52B5);
- /* SlvDSPreadyTime = 24, MasDSPreadyTime = 24 */
- __phy_write(phydev, 0x11, 0xc71);
- __phy_write(phydev, 0x12, 0xc);
- __phy_write(phydev, 0x10, 0x8fae);
-
- /* EnabRandUpdTrig = 1 */
- __phy_write(phydev, 0x11, 0x2f00);
- __phy_write(phydev, 0x12, 0xe);
- __phy_write(phydev, 0x10, 0x8fb0);
-
- /* NormMseLoThresh = 85 */
- __phy_write(phydev, 0x11, 0x55a0);
- __phy_write(phydev, 0x12, 0x0);
- __phy_write(phydev, 0x10, 0x83aa);
-
- /* FfeUpdGainForce = 1(Enable), FfeUpdGainForceVal = 4 */
- __phy_write(phydev, 0x11, 0x240);
- __phy_write(phydev, 0x12, 0x0);
- __phy_write(phydev, 0x10, 0x9680);
-
- /* TrFreeze = 0 (mt7988 default) */
- __phy_write(phydev, 0x11, 0x0);
- __phy_write(phydev, 0x12, 0x0);
- __phy_write(phydev, 0x10, 0x9686);
-
- /* SSTrKp100 = 5 */
- /* SSTrKf100 = 6 */
- /* SSTrKp1000Mas = 5 */
- /* SSTrKf1000Mas = 6 */
- /* SSTrKp1000Slv = 5 */
- /* SSTrKf1000Slv = 6 */
- __phy_write(phydev, 0x11, 0xbaef);
- __phy_write(phydev, 0x12, 0x2e);
- __phy_write(phydev, 0x10, 0x968c);
+ __tr_modify(phydev, 0x1, 0xf, 0x17,
+ SLAVE_DSP_READY_TIME_MASK | MASTER_DSP_READY_TIME_MASK,
+ FIELD_PREP(SLAVE_DSP_READY_TIME_MASK, 0x18) |
+ FIELD_PREP(MASTER_DSP_READY_TIME_MASK, 0x18));
+
+ __tr_set_bits(phydev, 0x1, 0xf, 0x18, ENABLE_RANDOM_UPDOWN_COUNTER_TRIGGER);
+
+ __tr_modify(phydev, 0x0, 0x7, 0x15,
+ NORMAL_MSE_LO_THRESH_MASK,
+ FIELD_PREP(NORMAL_MSE_LO_THRESH_MASK, 0x55));
+
+ __tr_modify(phydev, 0x2, 0xd, 0x0,
+ FFE_UPDATE_GAIN_FORCE_VAL_MASK,
+ FIELD_PREP(FFE_UPDATE_GAIN_FORCE_VAL_MASK, 0x4) | FFE_UPDATE_GAIN_FORCE);
+
+ __tr_clr_bits(phydev, 0x2, 0xd, 0x3, TR_FREEZE_MASK);
+
+ __tr_modify(phydev, 0x2, 0xd, 0x6,
+ SS_TR_KP100_MASK | SS_TR_KF100_MASK |
+ SS_TR_KP1000_MASTER_MASK | SS_TR_KF1000_MASTER_MASK |
+ SS_TR_KP1000_SLAVE_MASK | SS_TR_KF1000_SLAVE_MASK,
+ FIELD_PREP(SS_TR_KP100_MASK, 0x5) |
+ FIELD_PREP(SS_TR_KF100_MASK, 0x6) |
+ FIELD_PREP(SS_TR_KP1000_MASTER_MASK, 0x5) |
+ FIELD_PREP(SS_TR_KF1000_MASTER_MASK, 0x6) |
+ FIELD_PREP(SS_TR_KP1000_SLAVE_MASK, 0x5) |
+ FIELD_PREP(SS_TR_KF1000_SLAVE_MASK, 0x6));
+
phy_restore_page(phydev, MTK_PHY_PAGE_STANDARD, 0);
}
@@ -723,27 +788,27 @@ static void mt7981_phy_finetune(struct phy_device *phydev)
}
phy_select_page(phydev, MTK_PHY_PAGE_EXTENDED_52B5);
- /* ResetSyncOffset = 6 */
- __phy_write(phydev, 0x11, 0x600);
- __phy_write(phydev, 0x12, 0x0);
- __phy_write(phydev, 0x10, 0x8fc0);
+ __tr_modify(phydev, 0x1, 0xf, 0x20,
+ RESET_SYNC_OFFSET_MASK, FIELD_PREP(RESET_SYNC_OFFSET_MASK, 0x6));
- /* VgaDecRate = 1 */
- __phy_write(phydev, 0x11, 0x4c2a);
- __phy_write(phydev, 0x12, 0x3e);
- __phy_write(phydev, 0x10, 0x8fa4);
+ __tr_modify(phydev, 0x1, 0xf, 0x12,
+ VGA_DECIMATION_RATE_MASK, FIELD_PREP(VGA_DECIMATION_RATE_MASK, 0x1));
/* MrvlTrFix100Kp = 3, MrvlTrFix100Kf = 2,
* MrvlTrFix1000Kp = 3, MrvlTrFix1000Kf = 2
*/
- __phy_write(phydev, 0x11, 0xd10a);
- __phy_write(phydev, 0x12, 0x34);
- __phy_write(phydev, 0x10, 0x8f82);
+ __tr_modify(phydev, 0x1, 0xf, 0x1,
+ MRVL_TR_FIX_100KP_MASK | MRVL_TR_FIX_100KF_MASK |
+ MRVL_TR_FIX_1000KP_MASK | MRVL_TR_FIX_1000KF_MASK,
+ FIELD_PREP(MRVL_TR_FIX_100KP_MASK, 0x3) |
+ FIELD_PREP(MRVL_TR_FIX_100KF_MASK, 0x2) |
+ FIELD_PREP(MRVL_TR_FIX_1000KP_MASK, 0x3) |
+ FIELD_PREP(MRVL_TR_FIX_1000KF_MASK, 0x2));
/* VcoSlicerThreshBitsHigh */
- __phy_write(phydev, 0x11, 0x5555);
- __phy_write(phydev, 0x12, 0x55);
- __phy_write(phydev, 0x10, 0x8ec0);
+ __tr_modify(phydev, 0x1, 0xd, 0x20,
+ VCO_SLICER_THRESH_HIGH_MASK,
+ FIELD_PREP(VCO_SLICER_THRESH_HIGH_MASK, 0x555555));
phy_restore_page(phydev, MTK_PHY_PAGE_STANDARD, 0);
/* TR_OPEN_LOOP_EN = 1, lpf_x_average = 9 */
@@ -794,25 +859,22 @@ static void mt7988_phy_finetune(struct phy_device *phydev)
phy_write_mmd(phydev, MDIO_MMD_VEND1, MTK_PHY_RG_TX_FILTER, 0x5);
phy_select_page(phydev, MTK_PHY_PAGE_EXTENDED_52B5);
- /* ResetSyncOffset = 5 */
- __phy_write(phydev, 0x11, 0x500);
- __phy_write(phydev, 0x12, 0x0);
- __phy_write(phydev, 0x10, 0x8fc0);
+ __tr_modify(phydev, 0x1, 0xf, 0x20,
+ RESET_SYNC_OFFSET_MASK, FIELD_PREP(RESET_SYNC_OFFSET_MASK, 0x5));
/* VgaDecRate is 1 at default on mt7988 */
- /* MrvlTrFix100Kp = 6, MrvlTrFix100Kf = 7,
- * MrvlTrFix1000Kp = 6, MrvlTrFix1000Kf = 7
- */
- __phy_write(phydev, 0x11, 0xb90a);
- __phy_write(phydev, 0x12, 0x6f);
- __phy_write(phydev, 0x10, 0x8f82);
-
- /* RemAckCntLimitCtrl = 1 */
- __phy_write(phydev, 0x11, 0xfbba);
- __phy_write(phydev, 0x12, 0xc3);
- __phy_write(phydev, 0x10, 0x87f8);
-
+ __tr_modify(phydev, 0x1, 0xf, 0x1,
+ MRVL_TR_FIX_100KP_MASK | MRVL_TR_FIX_100KF_MASK |
+ MRVL_TR_FIX_1000KP_MASK | MRVL_TR_FIX_1000KF_MASK,
+ FIELD_PREP(MRVL_TR_FIX_100KP_MASK, 0x6) |
+ FIELD_PREP(MRVL_TR_FIX_100KF_MASK, 0x7) |
+ FIELD_PREP(MRVL_TR_FIX_1000KP_MASK, 0x6) |
+ FIELD_PREP(MRVL_TR_FIX_1000KF_MASK, 0x7));
+
+ __tr_modify(phydev, 0x0, 0xf, 0x3c,
+ REMOTE_ACK_COUNT_LIMIT_CTRL_MASK,
+ FIELD_PREP(REMOTE_ACK_COUNT_LIMIT_CTRL_MASK, 0x1));
phy_restore_page(phydev, MTK_PHY_PAGE_STANDARD, 0);
/* TR_OPEN_LOOP_EN = 1, lpf_x_average = 10 */
@@ -887,45 +949,34 @@ static void mt798x_phy_eee(struct phy_device *phydev)
MTK_PHY_TR_READY_SKIP_AFE_WAKEUP);
phy_select_page(phydev, MTK_PHY_PAGE_EXTENDED_52B5);
- /* Regsigdet_sel_1000 = 0 */
- __phy_write(phydev, 0x11, 0xb);
- __phy_write(phydev, 0x12, 0x0);
- __phy_write(phydev, 0x10, 0x9690);
-
- /* REG_EEE_st2TrKf1000 = 2 */
- __phy_write(phydev, 0x11, 0x114f);
- __phy_write(phydev, 0x12, 0x2);
- __phy_write(phydev, 0x10, 0x969a);
-
- /* RegEEE_slv_wake_tr_timer_tar = 6, RegEEE_slv_remtx_timer_tar = 20 */
- __phy_write(phydev, 0x11, 0x3028);
- __phy_write(phydev, 0x12, 0x0);
- __phy_write(phydev, 0x10, 0x969e);
-
- /* RegEEE_slv_wake_int_timer_tar = 8 */
- __phy_write(phydev, 0x11, 0x5010);
- __phy_write(phydev, 0x12, 0x0);
- __phy_write(phydev, 0x10, 0x96a0);
-
- /* RegEEE_trfreeze_timer2 = 586 */
- __phy_write(phydev, 0x11, 0x24a);
- __phy_write(phydev, 0x12, 0x0);
- __phy_write(phydev, 0x10, 0x96a8);
-
- /* RegEEE100Stg1_tar = 16 */
- __phy_write(phydev, 0x11, 0x3210);
- __phy_write(phydev, 0x12, 0x0);
- __phy_write(phydev, 0x10, 0x96b8);
-
- /* REGEEE_wake_slv_tr_wait_dfesigdet_en = 0 */
- __phy_write(phydev, 0x11, 0x1463);
- __phy_write(phydev, 0x12, 0x0);
- __phy_write(phydev, 0x10, 0x96ca);
-
- /* DfeTailEnableVgaThresh1000 = 27 */
- __phy_write(phydev, 0x11, 0x36);
- __phy_write(phydev, 0x12, 0x0);
- __phy_write(phydev, 0x10, 0x8f80);
+ __tr_clr_bits(phydev, 0x2, 0xd, 0x8, EEE1000_SELECT_SIGNEL_DETECTION_FROM_DFE);
+
+ __tr_modify(phydev, 0x2, 0xd, 0xd,
+ EEE1000_STAGE2_TR_KF_MASK,
+ FIELD_PREP(EEE1000_STAGE2_TR_KF_MASK, 0x2));
+
+ __tr_modify(phydev, 0x2, 0xd, 0xf,
+ SLAVE_WAKETR_TIMER_MASK | SLAVE_REMTX_TIMER_MASK,
+ FIELD_PREP(SLAVE_WAKETR_TIMER_MASK, 0x6) |
+ FIELD_PREP(SLAVE_REMTX_TIMER_MASK, 0x14));
+
+ __tr_modify(phydev, 0x2, 0xd, 0x10,
+ SLAVE_WAKEINT_TIMER_MASK,
+ FIELD_PREP(SLAVE_WAKEINT_TIMER_MASK, 0x8));
+
+ __tr_modify(phydev, 0x2, 0xd, 0x14,
+ TR_FREEZE_TIMER2_MASK,
+ FIELD_PREP(TR_FREEZE_TIMER2_MASK, 0x24a));
+
+ __tr_modify(phydev, 0x2, 0xd, 0x1c,
+ EEE100_LPSYNC_STAGE1_UPDATE_TIMER_MASK,
+ FIELD_PREP(EEE100_LPSYNC_STAGE1_UPDATE_TIMER_MASK, 0x10));
+
+ __tr_clr_bits(phydev, 0x2, 0xd, 0x25, WAKE_SLAVE_TR_WAIT_DFE_DETECTION_EN);
+
+ __tr_modify(phydev, 0x1, 0xf, 0x0,
+ DFE_TAIL_EANBLE_VGA_TRHESH_1000,
+ FIELD_PREP(DFE_TAIL_EANBLE_VGA_TRHESH_1000, 0x1b));
phy_restore_page(phydev, MTK_PHY_PAGE_STANDARD, 0);
phy_select_page(phydev, MTK_PHY_PAGE_EXTENDED_3);
diff --git a/drivers/net/phy/mediatek/mtk-ge.c b/drivers/net/phy/mediatek/mtk-ge.c
index 80425d6..5c0226d 100644
--- a/drivers/net/phy/mediatek/mtk-ge.c
+++ b/drivers/net/phy/mediatek/mtk-ge.c
@@ -8,13 +8,35 @@
#define MTK_GPHY_ID_MT7530 0x03a29412
#define MTK_GPHY_ID_MT7531 0x03a29441
-#define MTK_EXT_PAGE_ACCESS 0x1f
-#define MTK_PHY_PAGE_STANDARD 0x0000
-#define MTK_PHY_PAGE_EXTENDED 0x0001
-#define MTK_PHY_PAGE_EXTENDED_2 0x0002
-#define MTK_PHY_PAGE_EXTENDED_3 0x0003
-#define MTK_PHY_PAGE_EXTENDED_2A30 0x2a30
-#define MTK_PHY_PAGE_EXTENDED_52B5 0x52b5
+#define MTK_PHY_PAGE_EXTENDED_1 0x0001
+#define MTK_PHY_AUX_CTRL_AND_STATUS 0x14
+#define MTK_PHY_ENABLE_DOWNSHIFT BIT(4)
+
+#define MTK_PHY_PAGE_EXTENDED_2 0x0002
+#define MTK_PHY_PAGE_EXTENDED_3 0x0003
+#define MTK_PHY_RG_LPI_PCS_DSP_CTRL_REG11 0x11
+
+#define MTK_PHY_PAGE_EXTENDED_2A30 0x2a30
+
+/* Registers on Token Ring debug nodes */
+/* ch_addr = 0x1, node_addr = 0xf, data_addr = 0x17 */
+#define SLAVE_DSP_READY_TIME_MASK GENMASK(22, 15)
+
+/* Registers on MDIO_MMD_VEND1 */
+#define MTK_PHY_GBE_MODE_TX_DELAY_SEL 0x13
+#define MTK_PHY_TEST_MODE_TX_DELAY_SEL 0x14
+#define MTK_TX_DELAY_PAIR_B_MASK GENMASK(10, 8)
+#define MTK_TX_DELAY_PAIR_D_MASK GENMASK(2, 0)
+
+#define MTK_PHY_MCC_CTRL_AND_TX_POWER_CTRL 0xa6
+#define MTK_MCC_NEARECHO_OFFSET_MASK GENMASK(15, 8)
+
+#define MTK_PHY_RXADC_CTRL_RG7 0xc6
+#define MTK_PHY_DA_AD_BUF_BIAS_LP_MASK GENMASK(9, 8)
+
+#define MTK_PHY_RG_LPI_PCS_DSP_CTRL_REG123 0x123
+#define MTK_PHY_LPI_NORM_MSE_LO_THRESH100_MASK GENMASK(15, 8)
+#define MTK_PHY_LPI_NORM_MSE_HI_THRESH100_MASK GENMASK(7, 0)
struct mtk_gephy_priv {
unsigned long led_state;
@@ -23,20 +45,27 @@ struct mtk_gephy_priv {
static void mtk_gephy_config_init(struct phy_device *phydev)
{
/* Enable HW auto downshift */
- phy_modify_paged(phydev, MTK_PHY_PAGE_EXTENDED, 0x14, 0, BIT(4));
+ phy_modify_paged(phydev, MTK_PHY_PAGE_EXTENDED_1, MTK_PHY_AUX_CTRL_AND_STATUS,
+ 0, MTK_PHY_ENABLE_DOWNSHIFT);
/* Increase SlvDPSready time */
- phy_select_page(phydev, MTK_PHY_PAGE_EXTENDED_52B5);
- __phy_write(phydev, 0x10, 0xafae);
- __phy_write(phydev, 0x12, 0x2f);
- __phy_write(phydev, 0x10, 0x8fae);
- phy_restore_page(phydev, MTK_PHY_PAGE_STANDARD, 0);
+ tr_modify(phydev, 0x1, 0xf, 0x17, SLAVE_DSP_READY_TIME_MASK,
+ FIELD_PREP(SLAVE_DSP_READY_TIME_MASK, 0x5e));
/* Adjust 100_mse_threshold */
- phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x123, 0xffff);
-
- /* Disable mcc */
- phy_write_mmd(phydev, MDIO_MMD_VEND1, 0xa6, 0x300);
+ phy_modify_mmd(phydev, MDIO_MMD_VEND1,
+ MTK_PHY_RG_LPI_PCS_DSP_CTRL_REG123,
+ MTK_PHY_LPI_NORM_MSE_LO_THRESH100_MASK |
+ MTK_PHY_LPI_NORM_MSE_HI_THRESH100_MASK,
+ FIELD_PREP(MTK_PHY_LPI_NORM_MSE_LO_THRESH100_MASK,
+ 0xff) |
+ FIELD_PREP(MTK_PHY_LPI_NORM_MSE_HI_THRESH100_MASK,
+ 0xff));
+
+ /* If echo time is narrower than 0x3, it will be regarded as noise */
+ phy_modify_mmd(phydev, MDIO_MMD_VEND1, MTK_PHY_MCC_CTRL_AND_TX_POWER_CTRL,
+ MTK_MCC_NEARECHO_OFFSET_MASK,
+ FIELD_PREP(MTK_MCC_NEARECHO_OFFSET_MASK, 0x3));
}
static int mt7530_phy_config_init(struct phy_device *phydev)
@@ -44,7 +73,8 @@ static int mt7530_phy_config_init(struct phy_device *phydev)
mtk_gephy_config_init(phydev);
/* Increase post_update_timer */
- phy_write_paged(phydev, MTK_PHY_PAGE_EXTENDED_3, 0x11, 0x4b);
+ phy_write_paged(phydev, MTK_PHY_PAGE_EXTENDED_3,
+ MTK_PHY_RG_LPI_PCS_DSP_CTRL_REG11, 0x4b);
return 0;
}
@@ -55,11 +85,19 @@ static int mt7531_phy_config_init(struct phy_device *phydev)
/* PHY link down power saving enable */
phy_set_bits(phydev, 0x17, BIT(4));
- phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, 0xc6, 0x300);
+ phy_modify_mmd(phydev, MDIO_MMD_VEND1, MTK_PHY_RXADC_CTRL_RG7,
+ MTK_PHY_DA_AD_BUF_BIAS_LP_MASK,
+ FIELD_PREP(MTK_PHY_DA_AD_BUF_BIAS_LP_MASK, 0x3));
/* Set TX Pair delay selection */
- phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x13, 0x404);
- phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x14, 0x404);
+ phy_modify_mmd(phydev, MDIO_MMD_VEND1, MTK_PHY_GBE_MODE_TX_DELAY_SEL,
+ MTK_TX_DELAY_PAIR_B_MASK | MTK_TX_DELAY_PAIR_D_MASK,
+ FIELD_PREP(MTK_TX_DELAY_PAIR_B_MASK, 0x4) |
+ FIELD_PREP(MTK_TX_DELAY_PAIR_D_MASK, 0x4));
+ phy_modify_mmd(phydev, MDIO_MMD_VEND1, MTK_PHY_TEST_MODE_TX_DELAY_SEL,
+ MTK_TX_DELAY_PAIR_B_MASK | MTK_TX_DELAY_PAIR_D_MASK,
+ FIELD_PREP(MTK_TX_DELAY_PAIR_B_MASK, 0x4) |
+ FIELD_PREP(MTK_TX_DELAY_PAIR_D_MASK, 0x4));
return 0;
}
diff --git a/drivers/net/phy/mediatek/mtk-phy-lib.c b/drivers/net/phy/mediatek/mtk-phy-lib.c
index 4608837..3f1a24c 100644
--- a/drivers/net/phy/mediatek/mtk-phy-lib.c
+++ b/drivers/net/phy/mediatek/mtk-phy-lib.c
@@ -6,6 +6,94 @@
#include "mtk.h"
+/* Difference between functions with tr* and __tr* prefixes is
+ * tr* functions: wrapped by page switching operations
+ * __tr* functions: no page switching operations
+ */
+
+static void __tr_access(struct phy_device *phydev, bool read, u8 ch_addr,
+ u8 node_addr, u8 data_addr)
+{
+ u16 tr_cmd = BIT(15); /* bit 14 & 0 are reserved */
+
+ if (read)
+ tr_cmd |= BIT(13);
+
+ tr_cmd |= (((ch_addr & 0x3) << 11) |
+ ((node_addr & 0xf) << 7) |
+ ((data_addr & 0x3f) << 1));
+ dev_dbg(&phydev->mdio.dev, "tr_cmd: 0x%x\n", tr_cmd);
+ __phy_write(phydev, 0x10, tr_cmd);
+}
+
+static void __tr_read(struct phy_device *phydev, u8 ch_addr, u8 node_addr, u8 data_addr,
+ u16 *tr_high, u16 *tr_low)
+{
+ __tr_access(phydev, true, ch_addr, node_addr, data_addr);
+ *tr_low = __phy_read(phydev, 0x11);
+ *tr_high = __phy_read(phydev, 0x12);
+ dev_dbg(&phydev->mdio.dev, "tr_high read: 0x%x, tr_low read: 0x%x\n",
+ *tr_high, *tr_low);
+}
+
+u32 tr_read(struct phy_device *phydev, u8 ch_addr, u8 node_addr, u8 data_addr)
+{
+ u16 tr_high;
+ u16 tr_low;
+
+ phy_select_page(phydev, MTK_PHY_PAGE_EXTENDED_52B5);
+ __tr_read(phydev, ch_addr, node_addr, data_addr, &tr_high, &tr_low);
+ phy_restore_page(phydev, MTK_PHY_PAGE_STANDARD, 0);
+
+ return (tr_high << 16) | tr_low;
+}
+EXPORT_SYMBOL_GPL(tr_read);
+
+static void __tr_write(struct phy_device *phydev, u8 ch_addr, u8 node_addr, u8 data_addr,
+ u32 tr_data)
+{
+ __phy_write(phydev, 0x11, tr_data & 0xffff);
+ __phy_write(phydev, 0x12, tr_data >> 16);
+ dev_dbg(&phydev->mdio.dev, "tr_high write: 0x%x, tr_low write: 0x%x\n",
+ tr_data >> 16, tr_data & 0xffff);
+ __tr_access(phydev, false, ch_addr, node_addr, data_addr);
+}
+
+void __tr_modify(struct phy_device *phydev, u8 ch_addr, u8 node_addr, u8 data_addr,
+ u32 mask, u32 set)
+{
+ u32 tr_data;
+ u16 tr_high;
+ u16 tr_low;
+
+ __tr_read(phydev, ch_addr, node_addr, data_addr, &tr_high, &tr_low);
+ tr_data = (tr_high << 16) | tr_low;
+ tr_data = (tr_data & ~mask) | set;
+ __tr_write(phydev, ch_addr, node_addr, data_addr, tr_data);
+}
+EXPORT_SYMBOL_GPL(__tr_modify);
+
+void tr_modify(struct phy_device *phydev, u8 ch_addr, u8 node_addr, u8 data_addr,
+ u32 mask, u32 set)
+{
+ phy_select_page(phydev, MTK_PHY_PAGE_EXTENDED_52B5);
+ __tr_modify(phydev, ch_addr, node_addr, data_addr, mask, set);
+ phy_restore_page(phydev, MTK_PHY_PAGE_STANDARD, 0);
+}
+EXPORT_SYMBOL_GPL(tr_modify);
+
+void __tr_set_bits(struct phy_device *phydev, u8 ch_addr, u8 node_addr, u8 data_addr, u32 set)
+{
+ __tr_modify(phydev, ch_addr, node_addr, data_addr, 0, set);
+}
+EXPORT_SYMBOL_GPL(__tr_set_bits);
+
+void __tr_clr_bits(struct phy_device *phydev, u8 ch_addr, u8 node_addr, u8 data_addr, u32 clr)
+{
+ __tr_modify(phydev, ch_addr, node_addr, data_addr, clr, 0);
+}
+EXPORT_SYMBOL_GPL(__tr_clr_bits);
+
int mtk_phy_read_page(struct phy_device *phydev)
{
return __phy_read(phydev, MTK_EXT_PAGE_ACCESS);
diff --git a/drivers/net/phy/mediatek/mtk.h b/drivers/net/phy/mediatek/mtk.h
index c392c38..10ee9be 100644
--- a/drivers/net/phy/mediatek/mtk.h
+++ b/drivers/net/phy/mediatek/mtk.h
@@ -9,6 +9,8 @@
#define _MTK_EPHY_H_
#define MTK_EXT_PAGE_ACCESS 0x1f
+#define MTK_PHY_PAGE_STANDARD 0x0000
+#define MTK_PHY_PAGE_EXTENDED_52B5 0x52b5
/* Registers on MDIO_MMD_VEND2 */
#define MTK_PHY_LED0_ON_CTRL 0x24
@@ -62,6 +64,14 @@
#define MTK_PHY_LED_STATE_FORCE_BLINK 1
#define MTK_PHY_LED_STATE_NETDEV 2
+u32 tr_read(struct phy_device *phydev, u8 ch_addr, u8 node_addr, u8 data_addr);
+void __tr_modify(struct phy_device *phydev, u8 ch_addr, u8 node_addr, u8 data_addr,
+ u32 mask, u32 set);
+void tr_modify(struct phy_device *phydev, u8 ch_addr, u8 node_addr, u8 data_addr,
+ u32 mask, u32 set);
+void __tr_set_bits(struct phy_device *phydev, u8 ch_addr, u8 node_addr, u8 data_addr, u32 set);
+void __tr_clr_bits(struct phy_device *phydev, u8 ch_addr, u8 node_addr, u8 data_addr, u32 clr);
+
int mtk_phy_read_page(struct phy_device *phydev);
int mtk_phy_write_page(struct phy_device *phydev, int page);
--
2.18.0
From: "SkyLake.Huang" <[email protected]>
We observe that some 10G devices' (mostly Marvell's chips inside) 1G
training time violates specification, which may last 2230ms and affect
later TX/RX link pulse time. This will invalidate MediaTek series
gigabit Ethernet PHYs' hardware auto downshift mechanism.
Without this patch, if someone is trying to use "4-wire" cable to
connect above devices, MediaTek' gigabit Ethernet PHYs may fail
to downshift to 100Mbps. (If partner 10G devices' downshift mechanism
stops at 1G)
This patch extends our 1G TX/RX link pulse time so that we can still
link up with those 10G devices.
Tested device:
- Netgear GS110EMX's 10G port (Marvell 88X3340P)
- QNAP QSW-M408-4C
v3:
Refactor mtk_gphy_cl22_read_status() with genphy_read_status().
v4:
1. Change extend_an_new_lp_cnt_limit()'s return type and all return values
2. Refactor comments in extend_an_new_lp_cnt_limit()
Signed-off-by: SkyLake.Huang <[email protected]>
---
drivers/net/phy/mediatek/mtk-ge-soc.c | 2 +
drivers/net/phy/mediatek/mtk-ge.c | 1 +
drivers/net/phy/mediatek/mtk-phy-lib.c | 66 ++++++++++++++++++++++++++
drivers/net/phy/mediatek/mtk.h | 16 +++++++
4 files changed, 85 insertions(+)
diff --git a/drivers/net/phy/mediatek/mtk-ge-soc.c b/drivers/net/phy/mediatek/mtk-ge-soc.c
index c566bf9..d7b7fb2 100644
--- a/drivers/net/phy/mediatek/mtk-ge-soc.c
+++ b/drivers/net/phy/mediatek/mtk-ge-soc.c
@@ -1339,6 +1339,7 @@ static struct phy_driver mtk_socphy_driver[] = {
PHY_ID_MATCH_EXACT(MTK_GPHY_ID_MT7981),
.name = "MediaTek MT7981 PHY",
.config_init = mt798x_phy_config_init,
+ .read_status = mtk_gphy_cl22_read_status,
.config_intr = genphy_no_config_intr,
.handle_interrupt = genphy_handle_interrupt_no_ack,
.probe = mt7981_phy_probe,
@@ -1356,6 +1357,7 @@ static struct phy_driver mtk_socphy_driver[] = {
PHY_ID_MATCH_EXACT(MTK_GPHY_ID_MT7988),
.name = "MediaTek MT7988 PHY",
.config_init = mt798x_phy_config_init,
+ .read_status = mtk_gphy_cl22_read_status,
.config_intr = genphy_no_config_intr,
.handle_interrupt = genphy_handle_interrupt_no_ack,
.probe = mt7988_phy_probe,
diff --git a/drivers/net/phy/mediatek/mtk-ge.c b/drivers/net/phy/mediatek/mtk-ge.c
index 5c0226d..c832c90 100644
--- a/drivers/net/phy/mediatek/mtk-ge.c
+++ b/drivers/net/phy/mediatek/mtk-ge.c
@@ -211,6 +211,7 @@ static struct phy_driver mtk_gephy_driver[] = {
.name = "MediaTek MT7531 PHY",
.probe = mt7531_phy_probe,
.config_init = mt7531_phy_config_init,
+ .read_status = mtk_gphy_cl22_read_status,
/* Interrupts are handled by the switch, not the PHY
* itself.
*/
diff --git a/drivers/net/phy/mediatek/mtk-phy-lib.c b/drivers/net/phy/mediatek/mtk-phy-lib.c
index 3f1a24c..41bd1b0 100644
--- a/drivers/net/phy/mediatek/mtk-phy-lib.c
+++ b/drivers/net/phy/mediatek/mtk-phy-lib.c
@@ -106,6 +106,72 @@ int mtk_phy_write_page(struct phy_device *phydev, int page)
}
EXPORT_SYMBOL_GPL(mtk_phy_write_page);
+static int extend_an_new_lp_cnt_limit(struct phy_device *phydev)
+{
+ int mmd_read_ret;
+ u32 reg_val;
+ int timeout;
+
+ timeout = read_poll_timeout(mmd_read_ret = phy_read_mmd, reg_val,
+ (mmd_read_ret < 0) || reg_val & MTK_PHY_FINAL_SPEED_1000,
+ 10000, 1000000, false, phydev,
+ MDIO_MMD_VEND1, MTK_PHY_LINK_STATUS_MISC);
+ if (mmd_read_ret < 0)
+ return mmd_read_ret;
+
+ /* [When cable is plugged in]
+ * We expect final_speed_1000 will be set, which means this phy starts 1G link-up
+ * training. In this case, try to extend timeout period of auto downshift.
+ * [When cable is unplugged]
+ * We expect that reading MTK_PHY_FINAL_SPEED_1000 will time out. Do nothing but
+ * return.
+ */
+ if (!timeout) {
+ tr_modify(phydev, 0x0, 0xf, 0x3c, AN_NEW_LP_CNT_LIMIT_MASK,
+ FIELD_PREP(AN_NEW_LP_CNT_LIMIT_MASK, 0xf));
+ mdelay(1500);
+
+ timeout = read_poll_timeout(tr_read, reg_val,
+ (reg_val & AN_STATE_MASK) !=
+ (AN_STATE_TX_DISABLE << AN_STATE_SHIFT),
+ 10000, 1000000, false, phydev,
+ 0x0, 0xf, 0x2);
+ if (!timeout) {
+ mdelay(625);
+ tr_modify(phydev, 0x0, 0xf, 0x3c, AN_NEW_LP_CNT_LIMIT_MASK,
+ FIELD_PREP(AN_NEW_LP_CNT_LIMIT_MASK, 0x8));
+ mdelay(500);
+ tr_modify(phydev, 0x0, 0xf, 0x3c, AN_NEW_LP_CNT_LIMIT_MASK,
+ FIELD_PREP(AN_NEW_LP_CNT_LIMIT_MASK, 0xf));
+ } else {
+ return -ETIMEDOUT;
+ }
+ }
+
+ return 0;
+}
+
+int mtk_gphy_cl22_read_status(struct phy_device *phydev)
+{
+ int ret;
+
+ ret = genphy_read_status(phydev);
+ if (ret)
+ return ret;
+
+ if (phydev->autoneg == AUTONEG_ENABLE && !phydev->autoneg_complete) {
+ ret = phy_read(phydev, MII_CTRL1000);
+ if ((ret & ADVERTISE_1000FULL) || (ret & ADVERTISE_1000HALF)) {
+ ret = extend_an_new_lp_cnt_limit(phydev);
+ if (ret < 0)
+ return ret;
+ }
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(mtk_gphy_cl22_read_status);
+
int mtk_phy_led_hw_is_supported(struct phy_device *phydev, u8 index, unsigned long rules,
unsigned long supported_triggers)
{
diff --git a/drivers/net/phy/mediatek/mtk.h b/drivers/net/phy/mediatek/mtk.h
index 10ee9be..32fa3f1 100644
--- a/drivers/net/phy/mediatek/mtk.h
+++ b/drivers/net/phy/mediatek/mtk.h
@@ -12,6 +12,20 @@
#define MTK_PHY_PAGE_STANDARD 0x0000
#define MTK_PHY_PAGE_EXTENDED_52B5 0x52b5
+/* Registers on Token Ring debug nodes */
+/* ch_addr = 0x0, node_addr = 0xf, data_addr = 0x2 */
+#define AN_STATE_MASK GENMASK(22, 19)
+#define AN_STATE_SHIFT 19
+#define AN_STATE_TX_DISABLE 1
+
+/* ch_addr = 0x0, node_addr = 0xf, data_addr = 0x3c */
+#define AN_NEW_LP_CNT_LIMIT_MASK GENMASK(23, 20)
+#define AUTO_NP_10XEN BIT(6)
+
+/* Registers on MDIO_MMD_VEND1 */
+#define MTK_PHY_LINK_STATUS_MISC (0xa2)
+#define MTK_PHY_FINAL_SPEED_1000 BIT(3)
+
/* Registers on MDIO_MMD_VEND2 */
#define MTK_PHY_LED0_ON_CTRL 0x24
#define MTK_PHY_LED1_ON_CTRL 0x26
@@ -75,6 +89,8 @@ void __tr_clr_bits(struct phy_device *phydev, u8 ch_addr, u8 node_addr, u8 data_
int mtk_phy_read_page(struct phy_device *phydev);
int mtk_phy_write_page(struct phy_device *phydev, int page);
+int mtk_gphy_cl22_read_status(struct phy_device *phydev);
+
int mtk_phy_led_hw_is_supported(struct phy_device *phydev, u8 index, unsigned long rules,
unsigned long supported_triggers);
int mtk_phy_led_hw_ctrl_set(struct phy_device *phydev, u8 index, unsigned long rules,
--
2.18.0
From: "SkyLake.Huang" <[email protected]>
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.
v3:
1. Add range check for firmware.
2. Fix c45_ids.mmds_present in probe function.
3. Still use genphy_update_link() in read_status because
genphy_c45_read_link() can't correct detect link on this phy.
v4:
1. Move firmware loading function to mt798x_2p5ge_phy_load_fw()
2. Add AN disable warning in mt798x_2p5ge_phy_config_aneg()
3. Clarify the HDX comments in mt798x_2p5ge_phy_get_features()
v5:
1. Move md32_en_cfg_base & pmb_addr to local variables to achieve
symmetric code.
2. Print out firmware date code & version.
3. Don't return error if LED pinctrl switching fails. Also, add
comments to this unusual operations.
4. Return -EOPNOTSUPP for AN off case in config_aneg().
Signed-off-by: SkyLake.Huang <[email protected]>
---
MAINTAINERS | 1 +
drivers/net/phy/mediatek/Kconfig | 11 +
drivers/net/phy/mediatek/Makefile | 1 +
drivers/net/phy/mediatek/mtk-2p5ge.c | 422 +++++++++++++++++++++++++++
4 files changed, 435 insertions(+)
create mode 100644 drivers/net/phy/mediatek/mtk-2p5ge.c
diff --git a/MAINTAINERS b/MAINTAINERS
index e58e05c..fe380f2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13793,6 +13793,7 @@ M: Qingfang Deng <[email protected]>
M: SkyLake Huang <[email protected]>
L: [email protected]
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 448bc20..1490352 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..33d1c8d
--- /dev/null
+++ b/drivers/net/phy/mediatek/mtk-2p5ge.c
@@ -0,0 +1,422 @@
+// 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_SIZE (0x20000)
+#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;
+ 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_load_fw(struct phy_device *phydev)
+{
+ struct mtk_i2p5ge_phy_priv *priv = phydev->priv;
+ void __iomem *md32_en_cfg_base, *pmb_addr;
+ struct device *dev = &phydev->mdio.dev;
+ const struct firmware *fw;
+ int ret, i;
+ u16 reg;
+
+ if (priv->fw_loaded)
+ return 0;
+
+ pmb_addr = ioremap(MT7988_2P5GE_PMB_BASE, MT7988_2P5GE_PMB_LEN);
+ if (!pmb_addr)
+ return -ENOMEM;
+ md32_en_cfg_base = ioremap(MT7988_2P5GE_MD32_EN_CFG_BASE, MT7988_2P5GE_MD32_EN_CFG_LEN);
+ if (!md32_en_cfg_base) {
+ ret = -ENOMEM;
+ goto free_pmb;
+ }
+
+ 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);
+ goto free;
+ }
+
+ if (fw->size != MT7988_2P5GE_PMB_SIZE) {
+ dev_err(dev, "Firmware size 0x%zx != 0x%x\n",
+ fw->size, MT7988_2P5GE_PMB_SIZE);
+ ret = -EINVAL;
+ goto free;
+ }
+
+ reg = readw(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 < MT7988_2P5GE_PMB_SIZE - 1; i += 4)
+ writel(*((uint32_t *)(fw->data + i)), pmb_addr + i);
+ release_firmware(fw);
+ dev_info(dev, "Firmware date code: %x/%x/%x, version: %x.%x\n",
+ be16_to_cpu(*((uint16_t *)(fw->data + MT7988_2P5GE_PMB_SIZE - 8))),
+ *(fw->data + MT7988_2P5GE_PMB_SIZE - 6),
+ *(fw->data + MT7988_2P5GE_PMB_SIZE - 5),
+ *(fw->data + MT7988_2P5GE_PMB_SIZE - 2),
+ *(fw->data + MT7988_2P5GE_PMB_SIZE - 1));
+
+ writew(reg & ~MD32_EN, md32_en_cfg_base);
+ writew(reg | MD32_EN, 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;
+
+free:
+ iounmap(md32_en_cfg_base);
+free_pmb:
+ iounmap(pmb_addr);
+
+ return ret ? ret : 0;
+}
+
+static int mt798x_2p5ge_phy_config_init(struct phy_device *phydev)
+{
+ struct pinctrl *pinctrl;
+ int ret;
+
+ ret = mt798x_2p5ge_phy_load_fw(phydev);
+ if (ret < 0)
+ return ret;
+
+ /* 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);
+
+ /* Switch pinctrl after setting polarity to avoid bogus blinking */
+ 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");
+
+ 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 advertising */
+ 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;
+
+ /* In fact, if we disable autoneg, we can't link up correctly:
+ * 2.5G/1G: Need AN to exchange master/slave information.
+ * 100M: Without AN, link starts at half duplex(According to IEEE 802.3-2018),
+ * which this phy doesn't support.
+ * 10M: Deprecated in this ethernet phy.
+ */
+ if (phydev->autoneg == AUTONEG_DISABLE)
+ return -EOPNOTSUPP;
+
+ 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;
+
+ /* 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);
+
+ return 0;
+}
+
+static int mt798x_2p5ge_phy_read_status(struct phy_device *phydev)
+{
+ int ret;
+
+ /* When MDIO_STAT1_LSTATUS is raised genphy_c45_read_link(), this phy actually
+ * hasn't finished AN. So use CL22's link update function instead.
+ */
+ 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) {
+ linkmode_zero(phydev->lp_advertising);
+ }
+
+ if (phydev->link) {
+ 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:
+ /* The original hardware only sets MDIO_DEVS_PMAPMD */
+ phydev->c45_ids.mmds_present |= (MDIO_DEVS_PCS | MDIO_DEVS_AN |
+ MDIO_DEVS_VEND1 | MDIO_DEVS_VEND2);
+ 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 MT7988 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 <[email protected]>");
+MODULE_LICENSE("GPL");
+
+MODULE_DEVICE_TABLE(mdio, mtk_2p5ge_phy_tbl);
--
2.18.0
Hi,
A few suggestions:
On Thu, May 30, 2024 at 11:48:43AM +0800, Sky Huang wrote:
> +static int extend_an_new_lp_cnt_limit(struct phy_device *phydev)
> +{
> + int mmd_read_ret;
> + u32 reg_val;
> + int timeout;
> +
> + timeout = read_poll_timeout(mmd_read_ret = phy_read_mmd, reg_val,
> + (mmd_read_ret < 0) || reg_val & MTK_PHY_FINAL_SPEED_1000,
> + 10000, 1000000, false, phydev,
> + MDIO_MMD_VEND1, MTK_PHY_LINK_STATUS_MISC);
timeout = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND1,
MTK_PHY_LINK_STATUS_MISC,
reg_val,
reg_val & MTK_PHY_FINAL_SPEED_1000,
10000, 1000000, false);
> + if (mmd_read_ret < 0)
> + return mmd_read_ret;
So, what if the poll times out (timeout == -ETIMEDOUT) ? If you want to
ignore that, then:
if (timeout < 0 && timeout != -ETIMEDOUT)
return timeout;
> +int mtk_gphy_cl22_read_status(struct phy_device *phydev)
> +{
> + int ret;
> +
> + ret = genphy_read_status(phydev);
> + if (ret)
> + return ret;
> +
> + if (phydev->autoneg == AUTONEG_ENABLE && !phydev->autoneg_complete) {
> + ret = phy_read(phydev, MII_CTRL1000);
> + if ((ret & ADVERTISE_1000FULL) || (ret & ADVERTISE_1000HALF)) {
This is equivalent to:
if (ret & (ADVERTISE_1000FULL | ADVERTISE_1000HALF)) {
which is easier to read.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Thu, May 30, 2024 at 11:48:44AM +0800, Sky Huang wrote:
> +static int mt798x_2p5ge_phy_config_aneg(struct phy_device *phydev)
> +{
> + bool changed = false;
> + u32 adv;
> + int ret;
> +
> + /* In fact, if we disable autoneg, we can't link up correctly:
> + * 2.5G/1G: Need AN to exchange master/slave information.
> + * 100M: Without AN, link starts at half duplex(According to IEEE 802.3-2018),
> + * which this phy doesn't support.
> + * 10M: Deprecated in this ethernet phy.
> + */
> + if (phydev->autoneg == AUTONEG_DISABLE)
> + return -EOPNOTSUPP;
We have another driver (stmmac) where a platform driver is wanting to
put a hack in the ksettings_set() ethtool path to error out on
disabling AN for 1G speeds. This sounds like something that is
applicable to more than one hardware (and I've been wondering whether
it is universally true that 1G copper links and faster all require
AN to function.)
Thus, I'm wondering whether this is something that the core code should
be doing.
> + /* 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.
> + */
What the MAC can and can't do really has little bearing on what link
modes the PHY driver should be providing. It is the responsibility of
the MAC driver to appropriately change what is supported when attaching
to the PHY. If using phylink, this is done by phylink via the MAC driver
telling phylink what it is capable of via mac_capabilities.
> +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;
You mention above XFI...
XFI is 10GBASE-R protocol to XFP module electrical standards.
SFI is 10GBASE-R protocol to SFP+ module electrical standards.
phy_interface_t is interested in the protocol. So, given that you
mention XFI, why doesn't this test for PHY_INTERFACE_MODE_10GBASER?
> +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:
> + /* The original hardware only sets MDIO_DEVS_PMAPMD */
> + phydev->c45_ids.mmds_present |= (MDIO_DEVS_PCS | MDIO_DEVS_AN |
> + MDIO_DEVS_VEND1 | MDIO_DEVS_VEND2);
No need for parens on the RHS. The RHS is an expression in its own
right, and there's no point in putting parens around the expression
to turn it into another expression!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Thu, 2024-05-30 at 11:23 +0100, Russell King (Oracle) wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Hi,
>
> A few suggestions:
>
> On Thu, May 30, 2024 at 11:48:43AM +0800, Sky Huang wrote:
> > +static int extend_an_new_lp_cnt_limit(struct phy_device *phydev)
> > +{
> > +int mmd_read_ret;
> > +u32 reg_val;
> > +int timeout;
> > +
> > +timeout = read_poll_timeout(mmd_read_ret = phy_read_mmd, reg_val,
> > + (mmd_read_ret < 0) || reg_val & MTK_PHY_FINAL_SPEED_1000,
> > + 10000, 1000000, false, phydev,
> > + MDIO_MMD_VEND1, MTK_PHY_LINK_STATUS_MISC);
>
> timeout = phy_read_mmd_poll_timeout(phydev, MDIO_MMD_VEND1,
> MTK_PHY_LINK_STATUS_MISC,
> reg_val,
> reg_val & MTK_PHY_FINAL_SPEED_1000,
> 10000, 1000000, false);
>
> > +if (mmd_read_ret < 0)
> > +return mmd_read_ret;
>
> So, what if the poll times out (timeout == -ETIMEDOUT) ? If you want
> to
> ignore that, then:
>
> if (timeout < 0 && timeout != -ETIMEDOUT)
> return timeout;
>
I'm not going to handle timeout case here. If we can't detect
MTK_PHY_FINAL_SPEED_1000 in 1 second, let it go and we'll detect it
next round.
> > +int mtk_gphy_cl22_read_status(struct phy_device *phydev)
> > +{
> > +int ret;
> > +
> > +ret = genphy_read_status(phydev);
> > +if (ret)
> > +return ret;
> > +
> > +if (phydev->autoneg == AUTONEG_ENABLE && !phydev-
> >autoneg_complete) {
> > +ret = phy_read(phydev, MII_CTRL1000);
> > +if ((ret & ADVERTISE_1000FULL) || (ret & ADVERTISE_1000HALF)) {
>
> This is equivalent to:
>
> if (ret & (ADVERTISE_1000FULL | ADVERTISE_1000HALF)) {
>
> which is easier to read.
>
> Thanks.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Agree. I'll modify this in next version.
Sky
On Thu, May 30, 2024 at 04:01:08PM +0000, SkyLake Huang (黃啟澤) wrote:
> I'm not going to handle timeout case here. If we can't detect
> MTK_PHY_FINAL_SPEED_1000 in 1 second, let it go and we'll detect it
> next round.
With this waiting up to one second for MTK_PHY_FINAL_SPEED_1000 to be
set...
> > > +int mtk_gphy_cl22_read_status(struct phy_device *phydev)
> > > +{
> > > +int ret;
> > > +
> > > +ret = genphy_read_status(phydev);
> > > +if (ret)
> > > +return ret;
> > > +
> > > +if (phydev->autoneg == AUTONEG_ENABLE && !phydev-
> > >autoneg_complete) {
Are you sure you want this condition like this? When the link is down,
and 1G speeds are being advertised, it means that you'll call
extend_an_new_lp_cnt_limit(). If MTK_PHY_FINAL_SPEED_1000 doesn't get
set, that'll take one second each and every time we poll the PHY for
its status - which will be done while holding phydev->lock.
This doesn't sound very good.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Thu, 2024-05-30 at 11:35 +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 Thu, May 30, 2024 at 11:48:44AM +0800, Sky Huang wrote:
> > +static int mt798x_2p5ge_phy_config_aneg(struct phy_device *phydev)
> > +{
> > +bool changed = false;
> > +u32 adv;
> > +int ret;
> > +
> > +/* In fact, if we disable autoneg, we can't link up correctly:
> > + * 2.5G/1G: Need AN to exchange master/slave information.
> > + * 100M: Without AN, link starts at half duplex(According to IEEE
> 802.3-2018),
> > + * which this phy doesn't support.
> > + * 10M: Deprecated in this ethernet phy.
> > + */
> > +if (phydev->autoneg == AUTONEG_DISABLE)
> > +return -EOPNOTSUPP;
>
> We have another driver (stmmac) where a platform driver is wanting to
> put a hack in the ksettings_set() ethtool path to error out on
> disabling AN for 1G speeds. This sounds like something that is
> applicable to more than one hardware (and I've been wondering whether
> it is universally true that 1G copper links and faster all require
> AN to function.)
>
> Thus, I'm wondering whether this is something that the core code
> should
> be doing.
>
Yeah..As far as I know, 1G/2.5G/5G/10G speed require AN to decide
master/slave role. Actually I can use force mode by calling
genphy_c45_pma_set_forced, which will set correspoding C45 registers.
However, after that, this 2.5G PHY can't still link up with partners.
I'll leave EOPNOTSUPP here temporarily. Hope phylib can be patched
someday.
> > +/* 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.
> > + */
>
> What the MAC can and can't do really has little bearing on what link
> modes the PHY driver should be providing. It is the responsibility of
> the MAC driver to appropriately change what is supported when
> attaching
> to the PHY. If using phylink, this is done by phylink via the MAC
> driver
> telling phylink what it is capable of via mac_capabilities.
>
> > +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;
>
> You mention above XFI...
>
> XFI is 10GBASE-R protocol to XFP module electrical standards.
> SFI is 10GBASE-R protocol to SFP+ module electrical standards.
>
> phy_interface_t is interested in the protocol. So, given that you
> mention XFI, why doesn't this test for PHY_INTERFACE_MODE_10GBASER?
>
We have 2 XFI-MAC on mt7988 platform. One is connected to internal
2.5Gphy(SoC built-in), as we discussed here (We don't test this phy for
10G speed.) Another one is connected to external 10G phy.
> > +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:
> > +/* The original hardware only sets MDIO_DEVS_PMAPMD */
> > +phydev->c45_ids.mmds_present |= (MDIO_DEVS_PCS | MDIO_DEVS_AN |
> > + MDIO_DEVS_VEND1 | MDIO_DEVS_VEND2);
>
> No need for parens on the RHS. The RHS is an expression in its own
> right, and there's no point in putting parens around the expression
> to turn it into another expression!
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Do you mean these two line?
+phydev->c45_ids.mmds_present |= (MDIO_DEVS_PCS | MDIO_DEVS_AN |
+ MDIO_DEVS_VEND1 | MDIO_DEVS_VEND2);
What do you mean by "RHS is an expression in its own right"?
I put parens here to enhance readability so we don't need check
operator precedence again.
Sky
On Thu, May 30, 2024 at 04:25:56PM +0000, SkyLake Huang (黃啟澤) wrote:
> On Thu, 2024-05-30 at 11:35 +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 Thu, May 30, 2024 at 11:48:44AM +0800, Sky Huang wrote:
> > > +static int mt798x_2p5ge_phy_config_aneg(struct phy_device *phydev)
> > > +{
> > > +bool changed = false;
> > > +u32 adv;
> > > +int ret;
> > > +
> > > +/* In fact, if we disable autoneg, we can't link up correctly:
> > > + * 2.5G/1G: Need AN to exchange master/slave information.
> > > + * 100M: Without AN, link starts at half duplex(According to IEEE
> > 802.3-2018),
> > > + * which this phy doesn't support.
> > > + * 10M: Deprecated in this ethernet phy.
> > > + */
> > > +if (phydev->autoneg == AUTONEG_DISABLE)
> > > +return -EOPNOTSUPP;
> >
> > We have another driver (stmmac) where a platform driver is wanting to
> > put a hack in the ksettings_set() ethtool path to error out on
> > disabling AN for 1G speeds. This sounds like something that is
> > applicable to more than one hardware (and I've been wondering whether
> > it is universally true that 1G copper links and faster all require
> > AN to function.)
> >
> > Thus, I'm wondering whether this is something that the core code
> > should
> > be doing.
> >
> Yeah..As far as I know, 1G/2.5G/5G/10G speed require AN to decide
> master/slave role. Actually I can use force mode by calling
> genphy_c45_pma_set_forced, which will set correspoding C45 registers.
> However, after that, this 2.5G PHY can't still link up with partners.
>
> I'll leave EOPNOTSUPP here temporarily. Hope phylib can be patched
> someday.
Please no. "someday" tends to never happen, and you're basically
throwing the problem over the wall to other people to solve who
then have to spot your hack and eventually remove it.
We need this solved properly, not by people hacking drivers. This
is open source, you can propose a patch to phylib to fix this for
everyone.
> > > +/* 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.
> > > + */
> >
> > What the MAC can and can't do really has little bearing on what link
> > modes the PHY driver should be providing. It is the responsibility of
> > the MAC driver to appropriately change what is supported when
> > attaching
> > to the PHY. If using phylink, this is done by phylink via the MAC
> > driver
> > telling phylink what it is capable of via mac_capabilities.
> >
> > > +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;
> >
> > You mention above XFI...
> >
> > XFI is 10GBASE-R protocol to XFP module electrical standards.
> > SFI is 10GBASE-R protocol to SFP+ module electrical standards.
> >
> > phy_interface_t is interested in the protocol. So, given that you
> > mention XFI, why doesn't this test for PHY_INTERFACE_MODE_10GBASER?
> >
> We have 2 XFI-MAC on mt7988 platform. One is connected to internal
> 2.5Gphy(SoC built-in), as we discussed here (We don't test this phy for
> 10G speed.) Another one is connected to external 10G phy.
I can't parse your response in a meaningful way, to me it doesn't
address my point.
>
> > > +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:
> > > +/* The original hardware only sets MDIO_DEVS_PMAPMD */
> > > +phydev->c45_ids.mmds_present |= (MDIO_DEVS_PCS | MDIO_DEVS_AN |
> > > + MDIO_DEVS_VEND1 | MDIO_DEVS_VEND2);
> >
> > No need for parens on the RHS. The RHS is an expression in its own
> > right, and there's no point in putting parens around the expression
> > to turn it into another expression!
> >
> > --
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
>
> Do you mean these two line?
> +phydev->c45_ids.mmds_present |= (MDIO_DEVS_PCS | MDIO_DEVS_AN |
> + MDIO_DEVS_VEND1 | MDIO_DEVS_VEND2);
>
> What do you mean by "RHS is an expression in its own right"?
> I put parens here to enhance readability so we don't need check
> operator precedence again.
|= one of the assignment operators, all of which have one of the
lowest precedence. Only the , operator has a lower precedence.
Therefore, everything except , has higher precedence. Therefore,
the parens on the right hand side of |= make no difference.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi Sky,
kernel test robot noticed the following build warnings:
[auto build test WARNING on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Sky-Huang/net-phy-mediatek-Re-organize-MediaTek-ethernet-phy-drivers/20240530-115522
base: net-next/main
patch link: https://lore.kernel.org/r/20240530034844.11176-6-SkyLake.Huang%40mediatek.com
patch subject: [PATCH net-next v5 5/5] net: phy: add driver for built-in 2.5G ethernet PHY on MT7988
config: openrisc-randconfig-r121-20240531 (https://download.01.org/0day-ci/archive/20240531/[email protected]/config)
compiler: or1k-linux-gcc (GCC) 13.2.0
reproduce: (https://download.01.org/0day-ci/archive/20240531/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
sparse warnings: (new ones prefixed by >>)
>> drivers/net/phy/mediatek/mtk-2p5ge.c:106:9: sparse: sparse: cast to restricted __be16
vim +106 drivers/net/phy/mediatek/mtk-2p5ge.c
56
57 static int mt798x_2p5ge_phy_load_fw(struct phy_device *phydev)
58 {
59 struct mtk_i2p5ge_phy_priv *priv = phydev->priv;
60 void __iomem *md32_en_cfg_base, *pmb_addr;
61 struct device *dev = &phydev->mdio.dev;
62 const struct firmware *fw;
63 int ret, i;
64 u16 reg;
65
66 if (priv->fw_loaded)
67 return 0;
68
69 pmb_addr = ioremap(MT7988_2P5GE_PMB_BASE, MT7988_2P5GE_PMB_LEN);
70 if (!pmb_addr)
71 return -ENOMEM;
72 md32_en_cfg_base = ioremap(MT7988_2P5GE_MD32_EN_CFG_BASE, MT7988_2P5GE_MD32_EN_CFG_LEN);
73 if (!md32_en_cfg_base) {
74 ret = -ENOMEM;
75 goto free_pmb;
76 }
77
78 ret = request_firmware(&fw, MT7988_2P5GE_PMB, dev);
79 if (ret) {
80 dev_err(dev, "failed to load firmware: %s, ret: %d\n",
81 MT7988_2P5GE_PMB, ret);
82 goto free;
83 }
84
85 if (fw->size != MT7988_2P5GE_PMB_SIZE) {
86 dev_err(dev, "Firmware size 0x%zx != 0x%x\n",
87 fw->size, MT7988_2P5GE_PMB_SIZE);
88 ret = -EINVAL;
89 goto free;
90 }
91
92 reg = readw(md32_en_cfg_base);
93 if (reg & MD32_EN) {
94 phy_set_bits(phydev, MII_BMCR, BMCR_RESET);
95 usleep_range(10000, 11000);
96 }
97 phy_set_bits(phydev, MII_BMCR, BMCR_PDOWN);
98
99 /* Write magic number to safely stall MCU */
100 phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x800e, 0x1100);
101 phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x800f, 0x00df);
102
103 for (i = 0; i < MT7988_2P5GE_PMB_SIZE - 1; i += 4)
104 writel(*((uint32_t *)(fw->data + i)), pmb_addr + i);
105 release_firmware(fw);
> 106 dev_info(dev, "Firmware date code: %x/%x/%x, version: %x.%x\n",
107 be16_to_cpu(*((uint16_t *)(fw->data + MT7988_2P5GE_PMB_SIZE - 8))),
108 *(fw->data + MT7988_2P5GE_PMB_SIZE - 6),
109 *(fw->data + MT7988_2P5GE_PMB_SIZE - 5),
110 *(fw->data + MT7988_2P5GE_PMB_SIZE - 2),
111 *(fw->data + MT7988_2P5GE_PMB_SIZE - 1));
112
113 writew(reg & ~MD32_EN, md32_en_cfg_base);
114 writew(reg | MD32_EN, md32_en_cfg_base);
115 phy_set_bits(phydev, MII_BMCR, BMCR_RESET);
116 /* We need a delay here to stabilize initialization of MCU */
117 usleep_range(7000, 8000);
118 dev_info(dev, "Firmware loading/trigger ok.\n");
119
120 priv->fw_loaded = true;
121
122 free:
123 iounmap(md32_en_cfg_base);
124 free_pmb:
125 iounmap(pmb_addr);
126
127 return ret ? ret : 0;
128 }
129
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Thu, May 30, 2024 at 11:48:44AM +0800, Sky Huang wrote:
> From: "SkyLake.Huang" <[email protected]>
>
> 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.
>
> v3:
> 1. Add range check for firmware.
> 2. Fix c45_ids.mmds_present in probe function.
> 3. Still use genphy_update_link() in read_status because
> genphy_c45_read_link() can't correct detect link on this phy.
>
> v4:
> 1. Move firmware loading function to mt798x_2p5ge_phy_load_fw()
> 2. Add AN disable warning in mt798x_2p5ge_phy_config_aneg()
> 3. Clarify the HDX comments in mt798x_2p5ge_phy_get_features()
>
> v5:
> 1. Move md32_en_cfg_base & pmb_addr to local variables to achieve
> symmetric code.
> 2. Print out firmware date code & version.
> 3. Don't return error if LED pinctrl switching fails. Also, add
> comments to this unusual operations.
> 4. Return -EOPNOTSUPP for AN off case in config_aneg().
>
Hi Sky,
This is a somewhat unusual way to arrange a patch description.
Usually the description describes the change, particularly why
the change is being made.
While the per-version changes are listed below the scissors ("---").
> Signed-off-by: SkyLake.Huang <[email protected]>
...
> diff --git a/drivers/net/phy/mediatek/mtk-2p5ge.c b/drivers/net/phy/mediatek/mtk-2p5ge.c
...
> +static int mt798x_2p5ge_phy_load_fw(struct phy_device *phydev)
> +{
> + struct mtk_i2p5ge_phy_priv *priv = phydev->priv;
> + void __iomem *md32_en_cfg_base, *pmb_addr;
> + struct device *dev = &phydev->mdio.dev;
> + const struct firmware *fw;
> + int ret, i;
> + u16 reg;
> +
> + if (priv->fw_loaded)
> + return 0;
> +
> + pmb_addr = ioremap(MT7988_2P5GE_PMB_BASE, MT7988_2P5GE_PMB_LEN);
> + if (!pmb_addr)
> + return -ENOMEM;
> + md32_en_cfg_base = ioremap(MT7988_2P5GE_MD32_EN_CFG_BASE, MT7988_2P5GE_MD32_EN_CFG_LEN);
nit: Networking still prefers code to be 80 columns wide or less.
It looks like that can be trivially achieved here and
several other places in this patch.
OTOH, I don't think there is no need to break lines to meet this
requirement where it is particularly awkward to do so.
Flagged by checkpatch.pl --max-line-length=80
> + if (!md32_en_cfg_base) {
> + ret = -ENOMEM;
> + goto free_pmb;
> + }
> +
> + 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);
> + goto free;
> + }
> +
> + if (fw->size != MT7988_2P5GE_PMB_SIZE) {
> + dev_err(dev, "Firmware size 0x%zx != 0x%x\n",
> + fw->size, MT7988_2P5GE_PMB_SIZE);
> + ret = -EINVAL;
> + goto free;
> + }
> +
> + reg = readw(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 < MT7988_2P5GE_PMB_SIZE - 1; i += 4)
> + writel(*((uint32_t *)(fw->data + i)), pmb_addr + i);
> + release_firmware(fw);
> + dev_info(dev, "Firmware date code: %x/%x/%x, version: %x.%x\n",
> + be16_to_cpu(*((uint16_t *)(fw->data + MT7988_2P5GE_PMB_SIZE - 8))),
If the data at fw->data + MT7988_2P5GE_PMB_SIZE - 8 is a 16-bit
Big Endian value, then I think the cast should be to __be16 rather
than uint16_t (and in any case u16 would be preferred to uint16_t
as this is Kernel code).
Flagged by Sparse.
> + *(fw->data + MT7988_2P5GE_PMB_SIZE - 6),
> + *(fw->data + MT7988_2P5GE_PMB_SIZE - 5),
> + *(fw->data + MT7988_2P5GE_PMB_SIZE - 2),
> + *(fw->data + MT7988_2P5GE_PMB_SIZE - 1));
> +
> + writew(reg & ~MD32_EN, md32_en_cfg_base);
> + writew(reg | MD32_EN, 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;
> +
> +free:
> + iounmap(md32_en_cfg_base);
> +free_pmb:
> + iounmap(pmb_addr);
> +
> + return ret ? ret : 0;
> +}
...
> +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;
nit: Please consider arranging local variables in reverse xmas tree order -
longest line to shortest.
Edward Cree's tool can be helpful:
https://github.com/ecree-solarflare/xmastree
> +
> + 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);
> +}
...
On Thu, 2024-05-30 at 17:10 +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 Thu, May 30, 2024 at 04:01:08PM +0000, SkyLake Huang (黃啟澤) wrote:
> > I'm not going to handle timeout case here. If we can't detect
> > MTK_PHY_FINAL_SPEED_1000 in 1 second, let it go and we'll detect it
> > next round.
>
> With this waiting up to one second for MTK_PHY_FINAL_SPEED_1000 to be
> set...
>
> > > > +int mtk_gphy_cl22_read_status(struct phy_device *phydev)
> > > > +{
> > > > +int ret;
> > > > +
> > > > +ret = genphy_read_status(phydev);
> > > > +if (ret)
> > > > +return ret;
> > > > +
> > > > +if (phydev->autoneg == AUTONEG_ENABLE && !phydev-
> > > >autoneg_complete) {
>
> Are you sure you want this condition like this? When the link is
> down,
> and 1G speeds are being advertised, it means that you'll call
> extend_an_new_lp_cnt_limit(). If MTK_PHY_FINAL_SPEED_1000 doesn't get
> set, that'll take one second each and every time we poll the PHY for
> its status - which will be done while holding phydev->lock.
>
> This doesn't sound very good.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
I add another condition to make sure we enter
extend_an_new_lp_cnt_limit() only in first few seconds when we plug in
cable.
It will look like this:
===============================================================
#define MTK_PHY_AUX_CTRL_AND_STATUS 0x14
#define MTK_PHY_LP_DETECTED_MASK GENMASK(7, 6)
if (phydev->autoneg == AUTONEG_ENABLE && !phydev->autoneg_complete) {
phy_select_page(phydev, MTK_PHY_PAGE_EXTENDED_1);
ret = __phy_read(phydev, MTK_PHY_AUX_CTRL_AND_STATUS);
phy_restore_page(phydev, MTK_PHY_PAGE_STANDARD, 0);
/* Once LP_DETECTED is set, it means that"ability_match" in IEEE 802.3
* Figure 28-18 is set. This happens after we plug in cable. Also,
LP_DETECTED
* will be cleared after AN complete.
*/
if (!FIELD_GET(MTK_PHY_LP_DETECTED_MASK, ret))
return 0;
ret = phy_read(phydev, MII_CTRL1000);
if (ret & (ADVERTISE_1000FULL | ADVERTISE_1000HALF)) {
ret = extend_an_new_lp_cnt_limit(phydev);
if (ret < 0)
return ret;
}
}
return 0;
===============================================================
This is tested OK on my mt7988 platform.
Sky
On Thu, 2024-05-30 at 17:55 +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 Thu, May 30, 2024 at 04:25:56PM +0000, SkyLake Huang (黃啟澤) wrote:
> > On Thu, 2024-05-30 at 11:35 +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 Thu, May 30, 2024 at 11:48:44AM +0800, Sky Huang wrote:
> > > > +static int mt798x_2p5ge_phy_config_aneg(struct phy_device
> *phydev)
> > > > +{
> > > > +bool changed = false;
> > > > +u32 adv;
> > > > +int ret;
> > > > +
> > > > +/* In fact, if we disable autoneg, we can't link up correctly:
> > > > + * 2.5G/1G: Need AN to exchange master/slave information.
> > > > + * 100M: Without AN, link starts at half duplex(According to
> IEEE
> > > 802.3-2018),
> > > > + * which this phy doesn't support.
> > > > + * 10M: Deprecated in this ethernet phy.
> > > > + */
> > > > +if (phydev->autoneg == AUTONEG_DISABLE)
> > > > +return -EOPNOTSUPP;
> > >
> > > We have another driver (stmmac) where a platform driver is
> wanting to
> > > put a hack in the ksettings_set() ethtool path to error out on
> > > disabling AN for 1G speeds. This sounds like something that is
> > > applicable to more than one hardware (and I've been wondering
> whether
> > > it is universally true that 1G copper links and faster all
> require
> > > AN to function.)
> > >
> > > Thus, I'm wondering whether this is something that the core code
> > > should
> > > be doing.
> > >
> > Yeah..As far as I know, 1G/2.5G/5G/10G speed require AN to decide
> > master/slave role. Actually I can use force mode by calling
> > genphy_c45_pma_set_forced, which will set correspoding C45
> registers.
> > However, after that, this 2.5G PHY can't still link up with
> partners.
> >
> > I'll leave EOPNOTSUPP here temporarily. Hope phylib can be patched
> > someday.
>
> Please no. "someday" tends to never happen, and you're basically
> throwing the problem over the wall to other people to solve who
> then have to spot your hack and eventually remove it.
>
> We need this solved properly, not by people hacking drivers. This
> is open source, you can propose a patch to phylib to fix this for
> everyone.
>
I don't intend to throw problems to other people. And actually this
isn't a "problem" currently(at least in this driver). IMHO, disabling
AN isn't a normal operation for current ethernet environment. However,
now, ethtool supports this kind of configuration. Maybe we should
prohibit "AN disable" config for certain speed? Or maybe for all
speed? This will take lots of time for discussion. No matter what,
these discussions have little relevance to this driver.
For your reference, there's the same design in en8811h_config_aneg() of
drivers/net/phy/air_en8811h.c.
> > > > +/* 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.
> > > > + */
> > >
> > > What the MAC can and can't do really has little bearing on what
> link
> > > modes the PHY driver should be providing. It is the
> responsibility of
> > > the MAC driver to appropriately change what is supported when
> > > attaching
> > > to the PHY. If using phylink, this is done by phylink via the MAC
> > > driver
> > > telling phylink what it is capable of via mac_capabilities.
> > >
> > > > +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;
> > >
> > > You mention above XFI...
> > >
> > > XFI is 10GBASE-R protocol to XFP module electrical standards.
> > > SFI is 10GBASE-R protocol to SFP+ module electrical standards.
> > >
> > > phy_interface_t is interested in the protocol. So, given that you
> > > mention XFI, why doesn't this test for
> PHY_INTERFACE_MODE_10GBASER?
> > >
> > We have 2 XFI-MAC on mt7988 platform. One is connected to internal
> > 2.5Gphy(SoC built-in), as we discussed here (We don't test this phy
> for
> > 10G speed.) Another one is connected to external 10G phy.
>
> I can't parse your response in a meaningful way, to me it doesn't
> address my point.
>
I guess I got your point.
On mt7988
1st XFI-MAC (XGMAC2): For built-in 2.5Gphy or external 10Gphy
2nd XFI-MAC (XGMAC3): Only for external 10Gphy
Basically, if we use this driver for built-in 2.5Gphy. We'll only pass
phy-mode = "xgmii" or phy-mode = "internal" in dts, i.e,
PHY_INTERFACE_MODE_XGMII or PHY_INTERFACE_MODE_INTERNAL.
Also, we pass phy-mode = "usxgmii" (PHY_INTERFACE_MODE_10GBASER) in dts
once we use external 10Gphy with XGMAC2.
So we don't test PHY_INTERFACE_MODE_10GBASER or
PHY_INTERFACE_MODE_10GKR in driver's rate_matching().
But I think I should change it this way:
if (iface == PHY_INTERFACE_MODE_XGMII ||
iface == PHY_INTERFACE_MODE_INTERNAL)
return RATE_MATCH_PAUSE;
return RATE_MATCH_NONE;
> >
> > > > +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:
> > > > +/* The original hardware only sets MDIO_DEVS_PMAPMD */
> > > > +phydev->c45_ids.mmds_present |= (MDIO_DEVS_PCS | MDIO_DEVS_AN
> |
> > > > + MDIO_DEVS_VEND1 | MDIO_DEVS_VEND2);
> > >
> > > No need for parens on the RHS. The RHS is an expression in its
> own
> > > right, and there's no point in putting parens around the
> expression
> > > to turn it into another expression!
> > >
> > > --
> > > RMK's Patch system:
> https://www.armlinux.org.uk/developer/patches/
> > > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> >
> > Do you mean these two line?
> > +phydev->c45_ids.mmds_present |= (MDIO_DEVS_PCS | MDIO_DEVS_AN |
> > + MDIO_DEVS_VEND1 | MDIO_DEVS_VEND2);
> >
> > What do you mean by "RHS is an expression in its own right"?
> > I put parens here to enhance readability so we don't need check
> > operator precedence again.
>
> |= one of the assignment operators, all of which have one of the
> lowest precedence. Only the , operator has a lower precedence.
> Therefore, everything except , has higher precedence. Therefore,
> the parens on the right hand side of |= make no difference.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
I'll fix this in v6.
Sky
On Sat, 2024-06-01 at 13:51 +0100, Simon Horman wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> On Thu, May 30, 2024 at 11:48:44AM +0800, Sky Huang wrote:
> > From: "SkyLake.Huang" <[email protected]>
> >
> > 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.
> >
> > v3:
> > 1. Add range check for firmware.
> > 2. Fix c45_ids.mmds_present in probe function.
> > 3. Still use genphy_update_link() in read_status because
> > genphy_c45_read_link() can't correct detect link on this phy.
> >
> > v4:
> > 1. Move firmware loading function to mt798x_2p5ge_phy_load_fw()
> > 2. Add AN disable warning in mt798x_2p5ge_phy_config_aneg()
> > 3. Clarify the HDX comments in mt798x_2p5ge_phy_get_features()
> >
> > v5:
> > 1. Move md32_en_cfg_base & pmb_addr to local variables to achieve
> > symmetric code.
> > 2. Print out firmware date code & version.
> > 3. Don't return error if LED pinctrl switching fails. Also, add
> > comments to this unusual operations.
> > 4. Return -EOPNOTSUPP for AN off case in config_aneg().
> >
>
> Hi Sky,
>
> This is a somewhat unusual way to arrange a patch description.
>
> Usually the description describes the change, particularly why
> the change is being made.
>
> While the per-version changes are listed below the scissors ("---").
>
> > Signed-off-by: SkyLake.Huang <[email protected]>
>
> ...
>
> > diff --git a/drivers/net/phy/mediatek/mtk-2p5ge.c
> b/drivers/net/phy/mediatek/mtk-2p5ge.c
>
> ...
>
> > +static int mt798x_2p5ge_phy_load_fw(struct phy_device *phydev)
> > +{
> > +struct mtk_i2p5ge_phy_priv *priv = phydev->priv;
> > +void __iomem *md32_en_cfg_base, *pmb_addr;
> > +struct device *dev = &phydev->mdio.dev;
> > +const struct firmware *fw;
> > +int ret, i;
> > +u16 reg;
> > +
> > +if (priv->fw_loaded)
> > +return 0;
> > +
> > +pmb_addr = ioremap(MT7988_2P5GE_PMB_BASE, MT7988_2P5GE_PMB_LEN);
> > +if (!pmb_addr)
> > +return -ENOMEM;
> > +md32_en_cfg_base = ioremap(MT7988_2P5GE_MD32_EN_CFG_BASE,
> MT7988_2P5GE_MD32_EN_CFG_LEN);
>
> nit: Networking still prefers code to be 80 columns wide or less.
> It looks like that can be trivially achieved here and
> several other places in this patch.
>
> OTOH, I don't think there is no need to break lines to meet this
> requirement where it is particularly awkward to do so.
>
> Flagged by checkpatch.pl --max-line-length=80
>
> > +if (!md32_en_cfg_base) {
> > +ret = -ENOMEM;
> > +goto free_pmb;
> > +}
> > +
> > +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);
> > +goto free;
> > +}
> > +
> > +if (fw->size != MT7988_2P5GE_PMB_SIZE) {
> > +dev_err(dev, "Firmware size 0x%zx != 0x%x\n",
> > +fw->size, MT7988_2P5GE_PMB_SIZE);
> > +ret = -EINVAL;
> > +goto free;
> > +}
> > +
> > +reg = readw(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 < MT7988_2P5GE_PMB_SIZE - 1; i += 4)
> > +writel(*((uint32_t *)(fw->data + i)), pmb_addr + i);
> > +release_firmware(fw);
> > +dev_info(dev, "Firmware date code: %x/%x/%x, version: %x.%x\n",
> > + be16_to_cpu(*((uint16_t *)(fw->data + MT7988_2P5GE_PMB_SIZE -
> 8))),
>
> If the data at fw->data + MT7988_2P5GE_PMB_SIZE - 8 is a 16-bit
> Big Endian value, then I think the cast should be to __be16 rather
> than uint16_t (and in any case u16 would be preferred to uint16_t
> as this is Kernel code).
>
> Flagged by Sparse.
>
> > + *(fw->data + MT7988_2P5GE_PMB_SIZE - 6),
> > + *(fw->data + MT7988_2P5GE_PMB_SIZE - 5),
> > + *(fw->data + MT7988_2P5GE_PMB_SIZE - 2),
> > + *(fw->data + MT7988_2P5GE_PMB_SIZE - 1));
> > +
> > +writew(reg & ~MD32_EN, md32_en_cfg_base);
> > +writew(reg | MD32_EN, 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;
> > +
> > +free:
> > +iounmap(md32_en_cfg_base);
> > +free_pmb:
> > +iounmap(pmb_addr);
> > +
> > +return ret ? ret : 0;
> > +}
>
> ...
>
> > +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;
>
> nit: Please consider arranging local variables in reverse xmas tree
> order -
> longest line to shortest.
>
> Edward Cree's tool can be helpful:
> https://github.com/ecree-solarflare/xmastree
>
> > +
> > +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);
> > +}
>
> ...
Thanks. I'll fix all of the above in v6.
Sky
On Mon, Jun 03, 2024 at 03:15:36AM +0000, SkyLake Huang (黃啟澤) wrote:
> On Thu, 2024-05-30 at 17:10 +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 Thu, May 30, 2024 at 04:01:08PM +0000, SkyLake Huang (黃啟澤) wrote:
> > > I'm not going to handle timeout case here. If we can't detect
> > > MTK_PHY_FINAL_SPEED_1000 in 1 second, let it go and we'll detect it
> > > next round.
> >
> > With this waiting up to one second for MTK_PHY_FINAL_SPEED_1000 to be
> > set...
> >
> > > > > +int mtk_gphy_cl22_read_status(struct phy_device *phydev)
> > > > > +{
> > > > > +int ret;
> > > > > +
> > > > > +ret = genphy_read_status(phydev);
> > > > > +if (ret)
> > > > > +return ret;
> > > > > +
> > > > > +if (phydev->autoneg == AUTONEG_ENABLE && !phydev-
> > > > >autoneg_complete) {
> >
> > Are you sure you want this condition like this? When the link is
> > down,
> > and 1G speeds are being advertised, it means that you'll call
> > extend_an_new_lp_cnt_limit(). If MTK_PHY_FINAL_SPEED_1000 doesn't get
> > set, that'll take one second each and every time we poll the PHY for
> > its status - which will be done while holding phydev->lock.
> >
> > This doesn't sound very good.
> >
> > --
> > RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
>
> I add another condition to make sure we enter
> extend_an_new_lp_cnt_limit() only in first few seconds when we plug in
> cable.
>
> It will look like this:
> ===============================================================
> #define MTK_PHY_AUX_CTRL_AND_STATUS 0x14
> #define MTK_PHY_LP_DETECTED_MASK GENMASK(7, 6)
>
> if (phydev->autoneg == AUTONEG_ENABLE && !phydev->autoneg_complete) {
> phy_select_page(phydev, MTK_PHY_PAGE_EXTENDED_1);
> ret = __phy_read(phydev, MTK_PHY_AUX_CTRL_AND_STATUS);
> phy_restore_page(phydev, MTK_PHY_PAGE_STANDARD, 0);
We provide a helper for this:
ret = phy_read_paged(phydev, MTK_PHY_PAGE_EXTENDED_1,
MTK_PHY_AUX_CTRL_AND_STATUS);
but please check "ret" for errors.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Mon, 2024-06-03 at 09:06 +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, Jun 03, 2024 at 03:15:36AM +0000, SkyLake Huang (黃啟澤) wrote:
> > On Thu, 2024-05-30 at 17:10 +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 Thu, May 30, 2024 at 04:01:08PM +0000, SkyLake Huang (黃啟澤)
> wrote:
> > > > I'm not going to handle timeout case here. If we can't detect
> > > > MTK_PHY_FINAL_SPEED_1000 in 1 second, let it go and we'll
> detect it
> > > > next round.
> > >
> > > With this waiting up to one second for MTK_PHY_FINAL_SPEED_1000
> to be
> > > set...
> > >
> > > > > > +int mtk_gphy_cl22_read_status(struct phy_device *phydev)
> > > > > > +{
> > > > > > +int ret;
> > > > > > +
> > > > > > +ret = genphy_read_status(phydev);
> > > > > > +if (ret)
> > > > > > +return ret;
> > > > > > +
> > > > > > +if (phydev->autoneg == AUTONEG_ENABLE && !phydev-
> > > > > >autoneg_complete) {
> > >
> > > Are you sure you want this condition like this? When the link is
> > > down,
> > > and 1G speeds are being advertised, it means that you'll call
> > > extend_an_new_lp_cnt_limit(). If MTK_PHY_FINAL_SPEED_1000 doesn't
> get
> > > set, that'll take one second each and every time we poll the PHY
> for
> > > its status - which will be done while holding phydev->lock.
> > >
> > > This doesn't sound very good.
> > >
> > > --
> > > RMK's Patch system:
> https://www.armlinux.org.uk/developer/patches/
> > > FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> >
> > I add another condition to make sure we enter
> > extend_an_new_lp_cnt_limit() only in first few seconds when we plug
> in
> > cable.
> >
> > It will look like this:
> > ===============================================================
> > #define MTK_PHY_AUX_CTRL_AND_STATUS0x14
> > #define MTK_PHY_LP_DETECTED_MASKGENMASK(7, 6)
> >
> > if (phydev->autoneg == AUTONEG_ENABLE && !phydev->autoneg_complete)
> {
> > phy_select_page(phydev, MTK_PHY_PAGE_EXTENDED_1);
> > ret = __phy_read(phydev, MTK_PHY_AUX_CTRL_AND_STATUS);
> > phy_restore_page(phydev, MTK_PHY_PAGE_STANDARD, 0);
>
> We provide a helper for this:
>
> ret = phy_read_paged(phydev, MTK_PHY_PAGE_EXTENDED_1,
> MTK_PHY_AUX_CTRL_AND_STATUS);
>
> but please check "ret" for errors.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
OK, I'll fix this in v6.
Sky