diff mbox series

[RFC,v4,net-next,1/4] net: phy: add MediaTek PHY driver

Message ID 20210412034237.2473017-2-dqfext@gmail.com (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series MT7530 interrupt support | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 1 maintainers not CCed: linux-arm-kernel@lists.infradead.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit fail Errors and warnings before: 1 this patch: 3
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? WARNING: please write a paragraph that describes the config symbol fully
netdev/build_allmodconfig_warn fail Errors and warnings before: 1 this patch: 3
netdev/header_inline success Link

Commit Message

Qingfang Deng April 12, 2021, 3:42 a.m. UTC
Add support for MediaTek PHYs found in MT7530 and MT7531 switches.
The initialization procedure is from the vendor driver, but due to lack
of documentation, the function of some register values remains unknown.

Signed-off-by: DENG Qingfang <dqfext@gmail.com>
---
RFC v3 -> RFC v4:
- Remove unused include.

 drivers/net/phy/Kconfig    |   5 ++
 drivers/net/phy/Makefile   |   1 +
 drivers/net/phy/mediatek.c | 111 +++++++++++++++++++++++++++++++++++++
 3 files changed, 117 insertions(+)
 create mode 100644 drivers/net/phy/mediatek.c

Comments

René van Dorst April 12, 2021, 7:04 a.m. UTC | #1
Hi Qingfang,

Quoting DENG Qingfang <dqfext@gmail.com>:

> Add support for MediaTek PHYs found in MT7530 and MT7531 switches.
> The initialization procedure is from the vendor driver, but due to lack
> of documentation, the function of some register values remains unknown.
>
> Signed-off-by: DENG Qingfang <dqfext@gmail.com>
> ---
> RFC v3 -> RFC v4:
> - Remove unused include.
>
>  drivers/net/phy/Kconfig    |   5 ++
>  drivers/net/phy/Makefile   |   1 +
>  drivers/net/phy/mediatek.c | 111 +++++++++++++++++++++++++++++++++++++
>  3 files changed, 117 insertions(+)
>  create mode 100644 drivers/net/phy/mediatek.c
>
> diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
> index a615b3660b05..edd858cec9ec 100644
> --- a/drivers/net/phy/Kconfig
> +++ b/drivers/net/phy/Kconfig
> @@ -207,6 +207,11 @@ config MARVELL_88X2222_PHY
>  	  Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet
>  	  Transceiver.
>
> +config MEDIATEK_PHY
> +	tristate "MediaTek PHYs"
> +	help
> +	  Supports the MediaTek switch integrated PHYs.
> +
>  config MICREL_PHY
>  	tristate "Micrel PHYs"
>  	help
> diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
> index de683e3abe63..9ed7dbab7770 100644
> --- a/drivers/net/phy/Makefile
> +++ b/drivers/net/phy/Makefile
> @@ -64,6 +64,7 @@ obj-$(CONFIG_LXT_PHY)		+= lxt.o
>  obj-$(CONFIG_MARVELL_10G_PHY)	+= marvell10g.o
>  obj-$(CONFIG_MARVELL_PHY)	+= marvell.o
>  obj-$(CONFIG_MARVELL_88X2222_PHY)	+= marvell-88x2222.o
> +obj-$(CONFIG_MEDIATEK_PHY)	+= mediatek.o
>  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.c b/drivers/net/phy/mediatek.c
> new file mode 100644
> index 000000000000..1627b7c04345
> --- /dev/null
> +++ b/drivers/net/phy/mediatek.c
> @@ -0,0 +1,111 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +#include <linux/module.h>
> +#include <linux/phy.h>
> +
> +#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
> +
> +static int mtk_phy_read_page(struct phy_device *phydev)
> +{
> +	return __phy_read(phydev, MTK_EXT_PAGE_ACCESS);
> +}
> +
> +static int mtk_phy_write_page(struct phy_device *phydev, int page)
> +{
> +	return __phy_write(phydev, MTK_EXT_PAGE_ACCESS, page);
> +}
> +
> +static void mtk_phy_config_init(struct phy_device *phydev)
> +{
> +	/* Disable EEE */
> +	phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);

For my EEE patch I changed this line to:

genphy_config_eee_advert(phydev);

So PHY EEE part is setup properly at boot, instead enable it manual  
via ethtool.
This function also takes the DTS parameters "eee-broken-xxxx" in to  
account while
setting-up the PHY.

> +
> +	/* Enable HW auto downshift */
> +	phy_modify_paged(phydev, MTK_PHY_PAGE_EXTENDED, 0x14, 0, BIT(4));
> +
> +	/* 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);
> +
> +	/* Adjust 100_mse_threshold */
> +	phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x123, 0xffff);
> +
> +	/* Disable mcc */
> +	phy_write_mmd(phydev, MDIO_MMD_VEND1, 0xa6, 0x300);
> +}
> +

Greats,

René
Qingfang Deng April 12, 2021, 3:08 p.m. UTC | #2
On Mon, Apr 12, 2021 at 07:04:49AM +0000, René van Dorst wrote:
> Hi Qingfang,
> > +static void mtk_phy_config_init(struct phy_device *phydev)
> > +{
> > +	/* Disable EEE */
> > +	phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
> 
> For my EEE patch I changed this line to:
> 
> genphy_config_eee_advert(phydev);
> 
> So PHY EEE part is setup properly at boot, instead enable it manual via
> ethtool.
> This function also takes the DTS parameters "eee-broken-xxxx" in to account
> while
i> setting-up the PHY.

Thanks, I'm now testing with it.

> 
> > +
> > +	/* Enable HW auto downshift */
> > +	phy_modify_paged(phydev, MTK_PHY_PAGE_EXTENDED, 0x14, 0, BIT(4));
Qingfang Deng April 13, 2021, 3:59 a.m. UTC | #3
On Mon, Apr 12, 2021 at 11:08:36PM +0800, DENG Qingfang wrote:
> On Mon, Apr 12, 2021 at 07:04:49AM +0000, René van Dorst wrote:
> > Hi Qingfang,
> > > +static void mtk_phy_config_init(struct phy_device *phydev)
> > > +{
> > > +	/* Disable EEE */
> > > +	phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
> > 
> > For my EEE patch I changed this line to:
> > 
> > genphy_config_eee_advert(phydev);
> > 
> > So PHY EEE part is setup properly at boot, instead enable it manual via
> > ethtool.
> > This function also takes the DTS parameters "eee-broken-xxxx" in to account
> > while
> > setting-up the PHY.
> 
> Thanks, I'm now testing with it.

Hi Rene,

Within 12 hours, I got some spontaneous link down/ups when EEE is enabled:

[16334.236233] mt7530 mdio-bus:1f wan: Link is Down
[16334.241340] br-lan: port 3(wan) entered disabled state
[16337.355988] mt7530 mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx
[16337.363468] br-lan: port 3(wan) entered blocking state
[16337.368638] br-lan: port 3(wan) entered forwarding state

The cable is a 30m Cat.6 and never has such issue when EEE is disabled.
Perhaps WAKEUP_TIME_1000/100 or some PHY registers need to be fine-tuned,
but for now I think it should be disabled by default.

> 
> > 
> > > +
> > > +	/* Enable HW auto downshift */
> > > +	phy_modify_paged(phydev, MTK_PHY_PAGE_EXTENDED, 0x14, 0, BIT(4));
René van Dorst April 13, 2021, 9:55 a.m. UTC | #4
Quoting DENG Qingfang <dqfext@gmail.com>:

> On Mon, Apr 12, 2021 at 11:08:36PM +0800, DENG Qingfang wrote:
>> On Mon, Apr 12, 2021 at 07:04:49AM +0000, René van Dorst wrote:
>> > Hi Qingfang,
>> > > +static void mtk_phy_config_init(struct phy_device *phydev)
>> > > +{
>> > > +	/* Disable EEE */
>> > > +	phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
>> >
>> > For my EEE patch I changed this line to:
>> >
>> > genphy_config_eee_advert(phydev);
>> >
>> > So PHY EEE part is setup properly at boot, instead enable it manual via
>> > ethtool.
>> > This function also takes the DTS parameters "eee-broken-xxxx" in  
>> to account
>> > while
>> > setting-up the PHY.
>>
>> Thanks, I'm now testing with it.
>
> Hi Rene,
>
> Within 12 hours, I got some spontaneous link down/ups when EEE is enabled:
>
> [16334.236233] mt7530 mdio-bus:1f wan: Link is Down
> [16334.241340] br-lan: port 3(wan) entered disabled state
> [16337.355988] mt7530 mdio-bus:1f wan: Link is Up - 1Gbps/Full -  
> flow control rx/tx
> [16337.363468] br-lan: port 3(wan) entered blocking state
> [16337.368638] br-lan: port 3(wan) entered forwarding state
>
> The cable is a 30m Cat.6 and never has such issue when EEE is disabled.
> Perhaps WAKEUP_TIME_1000/100 or some PHY registers need to be fine-tuned,
> but for now I think it should be disabled by default.

Hi Qingfang,

Problem is that, may be the other device on the other side, may is issue.
So it is hard to tell which device is the issue.


I have a low traffic access point with 1meter cable running the  
openwrt 5.10 kernel with the openwrt EEE patch.
EEE is active and uptime with 16days without an issue.
This was with the old patch which clears the WAKEUP_TIME_1000/100 values.


I changed my ethernet setup so that access point switch has more  
traffic and longer cable ~18meters.
I also upgraded the kernel to 5.10.28 and added the EEE v2 patch.
Let's see what happens for a longer time and with more real world traffic.

Greats,

René
>
>>
>> >
>> > > +
>> > > +	/* Enable HW auto downshift */
>> > > +	phy_modify_paged(phydev, MTK_PHY_PAGE_EXTENDED, 0x14, 0, BIT(4));
Russell King (Oracle) April 13, 2021, 1:12 p.m. UTC | #5
On Tue, Apr 13, 2021 at 11:59:20AM +0800, DENG Qingfang wrote:
> Within 12 hours, I got some spontaneous link down/ups when EEE is enabled:
> 
> [16334.236233] mt7530 mdio-bus:1f wan: Link is Down
> [16334.241340] br-lan: port 3(wan) entered disabled state
> [16337.355988] mt7530 mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx
> [16337.363468] br-lan: port 3(wan) entered blocking state
> [16337.368638] br-lan: port 3(wan) entered forwarding state
> 
> The cable is a 30m Cat.6 and never has such issue when EEE is disabled.
> Perhaps WAKEUP_TIME_1000/100 or some PHY registers need to be fine-tuned,
> but for now I think it should be disabled by default.

Experience with Atheros AR8035 which has a very similar issue would
suggest that before resorting to the blunt hammer of disabling
SmartEEE, one should definitely experiment with the 1G Tw settings.

Using 24us for 1G speeds on AR8035 helps a great deal, whereas the PHY
defaults to 17us for 1G and 23us for 100M.
Qingfang Deng April 15, 2021, 9:49 a.m. UTC | #6
On Tue, Apr 13, 2021 at 02:12:59PM +0100, Russell King - ARM Linux admin wrote:
> On Tue, Apr 13, 2021 at 11:59:20AM +0800, DENG Qingfang wrote:
> > Within 12 hours, I got some spontaneous link down/ups when EEE is enabled:
> > 
> > [16334.236233] mt7530 mdio-bus:1f wan: Link is Down
> > [16334.241340] br-lan: port 3(wan) entered disabled state
> > [16337.355988] mt7530 mdio-bus:1f wan: Link is Up - 1Gbps/Full - flow control rx/tx
> > [16337.363468] br-lan: port 3(wan) entered blocking state
> > [16337.368638] br-lan: port 3(wan) entered forwarding state
> > 
> > The cable is a 30m Cat.6 and never has such issue when EEE is disabled.
> > Perhaps WAKEUP_TIME_1000/100 or some PHY registers need to be fine-tuned,
> > but for now I think it should be disabled by default.
> 
> Experience with Atheros AR8035 which has a very similar issue would
> suggest that before resorting to the blunt hammer of disabling
> SmartEEE, one should definitely experiment with the 1G Tw settings.
> 
> Using 24us for 1G speeds on AR8035 helps a great deal, whereas the PHY
> defaults to 17us for 1G and 23us for 100M.

I set the 1G Tw to maximum 255us and still got the link issue..
diff mbox series

Patch

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index a615b3660b05..edd858cec9ec 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -207,6 +207,11 @@  config MARVELL_88X2222_PHY
 	  Support for the Marvell 88X2222 Dual-port Multi-speed Ethernet
 	  Transceiver.
 
+config MEDIATEK_PHY
+	tristate "MediaTek PHYs"
+	help
+	  Supports the MediaTek switch integrated PHYs.
+
 config MICREL_PHY
 	tristate "Micrel PHYs"
 	help
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index de683e3abe63..9ed7dbab7770 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -64,6 +64,7 @@  obj-$(CONFIG_LXT_PHY)		+= lxt.o
 obj-$(CONFIG_MARVELL_10G_PHY)	+= marvell10g.o
 obj-$(CONFIG_MARVELL_PHY)	+= marvell.o
 obj-$(CONFIG_MARVELL_88X2222_PHY)	+= marvell-88x2222.o
+obj-$(CONFIG_MEDIATEK_PHY)	+= mediatek.o
 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.c b/drivers/net/phy/mediatek.c
new file mode 100644
index 000000000000..1627b7c04345
--- /dev/null
+++ b/drivers/net/phy/mediatek.c
@@ -0,0 +1,111 @@ 
+// SPDX-License-Identifier: GPL-2.0+
+#include <linux/module.h>
+#include <linux/phy.h>
+
+#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
+
+static int mtk_phy_read_page(struct phy_device *phydev)
+{
+	return __phy_read(phydev, MTK_EXT_PAGE_ACCESS);
+}
+
+static int mtk_phy_write_page(struct phy_device *phydev, int page)
+{
+	return __phy_write(phydev, MTK_EXT_PAGE_ACCESS, page);
+}
+
+static void mtk_phy_config_init(struct phy_device *phydev)
+{
+	/* Disable EEE */
+	phy_write_mmd(phydev, MDIO_MMD_AN, MDIO_AN_EEE_ADV, 0);
+
+	/* Enable HW auto downshift */
+	phy_modify_paged(phydev, MTK_PHY_PAGE_EXTENDED, 0x14, 0, BIT(4));
+
+	/* 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);
+
+	/* Adjust 100_mse_threshold */
+	phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x123, 0xffff);
+
+	/* Disable mcc */
+	phy_write_mmd(phydev, MDIO_MMD_VEND1, 0xa6, 0x300);
+}
+
+static int mt7530_phy_config_init(struct phy_device *phydev)
+{
+	mtk_phy_config_init(phydev);
+
+	/* Increase post_update_timer */
+	phy_write_paged(phydev, MTK_PHY_PAGE_EXTENDED_3, 0x11, 0x4b);
+
+	return 0;
+}
+
+static int mt7531_phy_config_init(struct phy_device *phydev)
+{
+	if (phydev->interface != PHY_INTERFACE_MODE_INTERNAL)
+		return -EINVAL;
+
+	mtk_phy_config_init(phydev);
+
+	/* PHY link down power saving enable */
+	phy_set_bits(phydev, 0x17, BIT(4));
+	phy_clear_bits_mmd(phydev, MDIO_MMD_VEND1, 0xc6, 0x300);
+
+	/* Set TX Pair delay selection */
+	phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x13, 0x404);
+	phy_write_mmd(phydev, MDIO_MMD_VEND1, 0x14, 0x404);
+
+	return 0;
+}
+
+static struct phy_driver mtk_phy_driver[] = {
+	{
+		PHY_ID_MATCH_EXACT(0x03a29412),
+		.name		= "MediaTek MT7530 PHY",
+		.config_init	= mt7530_phy_config_init,
+		/* Interrupts are handled by the switch, not the PHY
+		 * itself.
+		 */
+		.config_intr	= genphy_no_config_intr,
+		.handle_interrupt = genphy_handle_interrupt_no_ack,
+		.read_page	= mtk_phy_read_page,
+		.write_page	= mtk_phy_write_page,
+	},
+	{
+		PHY_ID_MATCH_EXACT(0x03a29441),
+		.name		= "MediaTek MT7531 PHY",
+		.config_init	= mt7531_phy_config_init,
+		/* Interrupts are handled by the switch, not the PHY
+		 * itself.
+		 */
+		.config_intr	= genphy_no_config_intr,
+		.handle_interrupt = genphy_handle_interrupt_no_ack,
+		.read_page	= mtk_phy_read_page,
+		.write_page	= mtk_phy_write_page,
+	},
+};
+
+module_phy_driver(mtk_phy_driver);
+
+static struct mdio_device_id __maybe_unused mtk_phy_tbl[] = {
+	{ PHY_ID_MATCH_VENDOR(0x03a29400) },
+	{ }
+};
+
+MODULE_DESCRIPTION("MediaTek switch integrated PHY driver");
+MODULE_AUTHOR("DENG, Qingfang <dqfext@gmail.com>");
+MODULE_LICENSE("GPL");
+
+MODULE_DEVICE_TABLE(mdio, mtk_phy_tbl);