[PATCH net-next 0/4] Add DWMAC glue driver for Motorcomm YT6801

[PATCH net-next 0/4] Add DWMAC glue driver for Motorcomm YT6801

[Yao Zi]

This series adds glue driver for Motorcomm YT6801 PCIe ethernet
controller, which is considered mostly compatible with DWMAC-4 IP by
inspecting the register layout[1]. It integrates a Motorcomm YT8531S PHY
(confirmed by reading PHY ID) and GMII is used to connect the PHY to
MAC[2].

The initialization logic of the MAC is mostly based on previous upstream
effort for the controller[3] and the Deepin-maintained downstream Linux
driver[4] licensed under GPL-2.0 according to its SPDX headers. However,
this series is a completely re-write of the previous patch series,
utilizing the existing DWMAC4 driver and introducing a glue driver only.

This series only aims to add basic networking functions for the
controller, features like WoL, RSS and LED control are omitted for now.
Testing is done on Loongson 3A5000 machine. Through a local GbE switch,
it reaches 871Mbps (TX)/942Mbps (RX) on average,

$ iperf3 -c 172.16.70.230
Connecting to host 172.16.70.230, port 5201
[  5] local 172.16.70.12 port 48590 connected to 172.16.70.230 port 5201
[ ID] Interval           Transfer     Bitrate         Retr  Cwnd
[  5]   0.00-1.00   sec   113 MBytes   950 Mbits/sec    0    376 KBytes
[  5]   1.00-2.00   sec   113 MBytes   946 Mbits/sec    0    376 KBytes
[  5]   2.00-3.00   sec   112 MBytes   941 Mbits/sec    0    376 KBytes
[  5]   3.00-4.00   sec   112 MBytes   939 Mbits/sec    0    376 KBytes
[  5]   4.00-5.00   sec   112 MBytes   939 Mbits/sec    0    376 KBytes
[  5]   5.00-6.00   sec   113 MBytes   946 Mbits/sec    0    399 KBytes
[  5]   6.00-7.00   sec   112 MBytes   940 Mbits/sec    0    399 KBytes
[  5]   7.00-8.00   sec   112 MBytes   940 Mbits/sec    0    399 KBytes
[  5]   8.00-9.00   sec   112 MBytes   938 Mbits/sec    0    399 KBytes
[  5]   9.00-10.00  sec   112 MBytes   937 Mbits/sec    0    399 KBytes

Connecting to host 172.16.70.12, port 5201
[  5] local 172.16.70.230 port 50466 connected to 172.16.70.12 port 5201
[ ID] Interval           Transfer     Bitrate         Retr  Cwnd
[  5]   0.00-1.00   sec   106 MBytes   884 Mbits/sec    0    486 KBytes
[  5]   1.00-2.00   sec   104 MBytes   870 Mbits/sec    0    486 KBytes
[  5]   2.00-3.00   sec   104 MBytes   868 Mbits/sec    0    486 KBytes
[  5]   3.00-4.00   sec   104 MBytes   869 Mbits/sec    0    486 KBytes
[  5]   4.00-5.00   sec   104 MBytes   873 Mbits/sec    0    486 KBytes
[  5]   5.00-6.00   sec   104 MBytes   871 Mbits/sec    0    486 KBytes
[  5]   6.00-7.00   sec   103 MBytes   867 Mbits/sec    0    512 KBytes
[  5]   7.00-8.00   sec   104 MBytes   872 Mbits/sec    0    512 KBytes
[  5]   8.00-9.00   sec   104 MBytes   873 Mbits/sec    0    512 KBytes
[  5]   9.00-10.00  sec   104 MBytes   874 Mbits/sec    0    512 KBytes

Thanks for your time and review.

[1]: https://lore.kernel.org/all/Z_T6vv013jraCzSD@shell.armlinux.org.uk/
[2]: https://lore.kernel.org/all/a48d76ac-db08-46d5-9528-f046a7b541dc@motor-comm.com/
[3]: https://lore.kernel.org/all/a48d76ac-db08-46d5-9528-f046a7b541dc@motor-comm.com/
[4]: https://github.com/deepin-community/kernel/tree/dc61248a0e21/drivers/net/ethernet/motorcomm/yt6801

Yao Zi (4):
  PCI: Add vendor ID for Motorcomm Electronic Technology
  net: phy: motorcomm: Support YT8531S PHY in YT6801 Ethernet controller
  net: stmmac: Add glue driver for Motorcomm YT6801 ethernet controller
  MAINTAINERS: Assign myself as maintainer of Motorcomm DWMAC glue
    driver

 MAINTAINERS                                   |   6 +
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |   9 +
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../ethernet/stmicro/stmmac/dwmac-motorcomm.c | 388 ++++++++++++++++++
 drivers/net/phy/motorcomm.c                   |   4 +
 include/linux/pci_ids.h                       |   2 +
 6 files changed, 410 insertions(+)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-motorcomm.c

-- 
2.50.1

[PATCH net-next 1/4] PCI: Add vendor ID for Motorcomm Electronic Technology

[Yao Zi]

This company produces Ethernet controllers and PHYs. Add their vendor
ID, 0x1f0a[1], which is recorded by PCI-SIG and has been seen on their
PCI Ethernet cards.

Link: https://pcisig.com/membership/member-companies?combine=1f0a # [1]
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 include/linux/pci_ids.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
index 92ffc4373f6d..0824a1a7663d 100644
--- a/include/linux/pci_ids.h
+++ b/include/linux/pci_ids.h
@@ -2631,6 +2631,8 @@
 
 #define PCI_VENDOR_ID_CXL		0x1e98
 
+#define PCI_VENDOR_ID_MOTORCOMM		0x1f0a
+
 #define PCI_VENDOR_ID_TEHUTI		0x1fc9
 #define PCI_DEVICE_ID_TEHUTI_3009	0x3009
 #define PCI_DEVICE_ID_TEHUTI_3010	0x3010
-- 
2.50.1

Re: [PATCH net-next 1/4] PCI: Add vendor ID for Motorcomm Electronic Technology

[Bjorn Helgaas]

On Tue, Oct 14, 2025 at 04:47:44PM +0000, Yao Zi wrote:
> This company produces Ethernet controllers and PHYs. Add their vendor
> ID, 0x1f0a[1], which is recorded by PCI-SIG and has been seen on their
> PCI Ethernet cards.
> 
> Link: https://pcisig.com/membership/member-companies?combine=1f0a # [1]
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  include/linux/pci_ids.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> index 92ffc4373f6d..0824a1a7663d 100644
> --- a/include/linux/pci_ids.h
> +++ b/include/linux/pci_ids.h
> @@ -2631,6 +2631,8 @@
>  
>  #define PCI_VENDOR_ID_CXL		0x1e98
>  
> +#define PCI_VENDOR_ID_MOTORCOMM		0x1f0a

If/when this is used by several drivers add it here.  Until then just
define PCI_VENDOR_ID_MOTORCOMM in the driver that uses it (see the
note at top of the file).

>  #define PCI_VENDOR_ID_TEHUTI		0x1fc9
>  #define PCI_DEVICE_ID_TEHUTI_3009	0x3009
>  #define PCI_DEVICE_ID_TEHUTI_3010	0x3010
> -- 
> 2.50.1
> 

Re: [PATCH net-next 1/4] PCI: Add vendor ID for Motorcomm Electronic Technology

[Yao Zi]

On Tue, Oct 14, 2025 at 03:43:06PM -0500, Bjorn Helgaas wrote:
> On Tue, Oct 14, 2025 at 04:47:44PM +0000, Yao Zi wrote:
> > This company produces Ethernet controllers and PHYs. Add their vendor
> > ID, 0x1f0a[1], which is recorded by PCI-SIG and has been seen on their
> > PCI Ethernet cards.
> > 
> > Link: https://pcisig.com/membership/member-companies?combine=1f0a # [1]
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> >  include/linux/pci_ids.h | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> > index 92ffc4373f6d..0824a1a7663d 100644
> > --- a/include/linux/pci_ids.h
> > +++ b/include/linux/pci_ids.h
> > @@ -2631,6 +2631,8 @@
> >  
> >  #define PCI_VENDOR_ID_CXL		0x1e98
> >  
> > +#define PCI_VENDOR_ID_MOTORCOMM		0x1f0a
> 
> If/when this is used by several drivers add it here.  Until then just
> define PCI_VENDOR_ID_MOTORCOMM in the driver that uses it (see the
> note at top of the file).

Oops, thanks for the hint. I didn't notice the note before, and will
switch to define the ID in driver.

Best regards,
Yao Zi

> >  #define PCI_VENDOR_ID_TEHUTI		0x1fc9
> >  #define PCI_DEVICE_ID_TEHUTI_3009	0x3009
> >  #define PCI_DEVICE_ID_TEHUTI_3010	0x3010
> > -- 
> > 2.50.1
> > 

[PATCH net-next 2/4] net: phy: motorcomm: Support YT8531S PHY in YT6801 Ethernet controller

[Yao Zi]

YT6801's internal PHY is confirmed as a GMII-capable variant of YT8531S
by a previous series[1] and reading PHY ID. Add support for
PHY_INTERFACE_MODE_INTERNAL for YT8531S to allow the Ethernet driver to
reuse the PHY code for its internal PHY.

Link: https://lore.kernel.org/all/a48d76ac-db08-46d5-9528-f046a7b541dc@motor-comm.com/ # [1]
Co-developed-by: Frank Sae <Frank.Sae@motor-comm.com>
Signed-off-by: Frank Sae <Frank.Sae@motor-comm.com>
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 drivers/net/phy/motorcomm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/phy/motorcomm.c b/drivers/net/phy/motorcomm.c
index a3593e663059..e478e4f51755 100644
--- a/drivers/net/phy/motorcomm.c
+++ b/drivers/net/phy/motorcomm.c
@@ -910,6 +910,10 @@ static int ytphy_rgmii_clk_delay_config(struct phy_device *phydev)
 		val |= FIELD_PREP(YT8521_RC1R_RX_DELAY_MASK, rx_reg) |
 		       FIELD_PREP(YT8521_RC1R_GE_TX_DELAY_MASK, tx_reg);
 		break;
+	case PHY_INTERFACE_MODE_INTERNAL:
+		if (phydev->drv->phy_id != PHY_ID_YT8531S)
+			return -EOPNOTSUPP;
+		break;
 	default: /* do not support other modes */
 		return -EOPNOTSUPP;
 	}
-- 
2.50.1

Re: [PATCH net-next 2/4] net: phy: motorcomm: Support YT8531S PHY in YT6801 Ethernet controller

[Russell King (Oracle)]

On Tue, Oct 14, 2025 at 04:47:45PM +0000, Yao Zi wrote:
> YT6801's internal PHY is confirmed as a GMII-capable variant of YT8531S
> by a previous series[1] and reading PHY ID. Add support for
> PHY_INTERFACE_MODE_INTERNAL for YT8531S to allow the Ethernet driver to
> reuse the PHY code for its internal PHY.

If it's known to be connected via a GMII interface, even if it's on the
SoC, please use PHY_INTERFACE_MODE_GMII in preference to
PHY_INTERFACE_MODE_INTERNAL. PHY_INTERFACE_MODE_INTERNAL is really for
"we don't know what the internal interface is".

Thanks.

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

Re: [PATCH net-next 2/4] net: phy: motorcomm: Support YT8531S PHY in YT6801 Ethernet controller

[Yao Zi]

On Tue, Oct 14, 2025 at 06:08:35PM +0100, Russell King (Oracle) wrote:
> On Tue, Oct 14, 2025 at 04:47:45PM +0000, Yao Zi wrote:
> > YT6801's internal PHY is confirmed as a GMII-capable variant of YT8531S
> > by a previous series[1] and reading PHY ID. Add support for
> > PHY_INTERFACE_MODE_INTERNAL for YT8531S to allow the Ethernet driver to
> > reuse the PHY code for its internal PHY.
> 
> If it's known to be connected via a GMII interface, even if it's on the
> SoC, please use PHY_INTERFACE_MODE_GMII in preference to
> PHY_INTERFACE_MODE_INTERNAL. PHY_INTERFACE_MODE_INTERNAL is really for
> "we don't know what the internal interface is".

I use PHY_INTERFACE_MODE_INTERNAL for the driver based on Andrew's
feedback[1] on the series submitted by Motorcomm people,

> Is it really GMII? If so, add GMII to the yt8531 driver.
>
> Often this is described as PHY_INTERFACE_MODE_INTERNAL, meaning it
> does not matter what is being used between the MAC and the PHY, it is
> internal to the SoC. You might want to add that to the PHY driver.

Does PHY_INTERFACE_MODE_INTERNAL mean "unknown interface" or "interface
that doesn't matter"? I think this could lead to different choices for
the mode in YT6801's case.

Thanks for your answer,

Best regards,
Yao Zi

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

[1]: https://lore.kernel.org/all/fde04f06-df39-41a8-8f74-036e315e9a8b@lunn.ch/

[PATCH net-next 3/4] net: stmmac: Add glue driver for Motorcomm YT6801 ethernet controller

[Yao Zi]

Motorcomm YT6801 is a PCIe ethernet controller based on DWMAC4 IP. It
integrates an GbE phy, supporting WOL, VLAN tagging and various types
of offloading. It ships an on-chip eFuse for storing various vendor
configuration, including MAC address.

This patch adds basic glue code for the controller, allowing it to be
set up and transmit data at a reasonable speed. Features like WOL could
be implemented in the future.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |   9 +
 drivers/net/ethernet/stmicro/stmmac/Makefile  |   1 +
 .../ethernet/stmicro/stmmac/dwmac-motorcomm.c | 388 ++++++++++++++++++
 3 files changed, 398 insertions(+)
 create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-motorcomm.c

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 9507131875b2..1f5222060c44 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -358,6 +358,15 @@ config DWMAC_LOONGSON
 	  This selects the LOONGSON PCI bus support for the stmmac driver,
 	  Support for ethernet controller on Loongson-2K1000 SoC and LS7A1000 bridge.
 
+config DWMAC_MOTORCOMM
+	tristate "Motorcomm PCI DWMAC support"
+	depends on STMMAC_ETH
+	depends on PCI
+	select MOTORCOMM_PHY
+	help
+	  This enables glue driver for Motorcomm DWMAC-based PCI Ethernet
+	  controllers. Currently only YT6801 is supported.
+
 config STMMAC_PCI
 	tristate "STMMAC PCI bus support"
 	depends on STMMAC_ETH && PCI
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index 51e068e26ce4..2e925a7089b4 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -46,4 +46,5 @@ dwmac-altr-socfpga-objs := dwmac-socfpga.o
 obj-$(CONFIG_STMMAC_PCI)	+= stmmac-pci.o
 obj-$(CONFIG_DWMAC_INTEL)	+= dwmac-intel.o
 obj-$(CONFIG_DWMAC_LOONGSON)	+= dwmac-loongson.o
+obj-$(CONFIG_DWMAC_MOTORCOMM)	+= dwmac-motorcomm.o
 stmmac-pci-objs:= stmmac_pci.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-motorcomm.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-motorcomm.c
new file mode 100644
index 000000000000..03a88ee6b014
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-motorcomm.c
@@ -0,0 +1,388 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * DWMAC glue driver for Motorcomm PCI Ethernet controllers
+ *
+ * Copyright (c) 2025 Yao Zi <ziyao@disroot.org>
+ */
+
+#include <linux/bits.h>
+#include <linux/dev_printk.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/module.h>
+#include <linux/pci.h>
+#include <linux/slab.h>
+#include <linux/stmmac.h>
+
+#include "dwmac4.h"
+#include "stmmac.h"
+
+#define DRIVER_NAME "dwmac-motorcomm"
+
+/* Register definition */
+#define EPHY_CTRL				0x1004
+#define  EPHY_RESET				BIT(0)
+#define OOB_WOL_CTRL				0x1010
+#define  OOB_WOL_CTRL_DIS			BIT(0)
+#define MGMT_INT_CTRL0				0x1100
+#define MGMT_INT_CTRL0_MASK			GENMASK(31, 16)
+#define  MGMT_INT_CTRL0_MASK_RXCH		GENMASK(3, 0)
+#define  MGMT_INT_CTRL0_MASK_TXCH		BIT(4)
+#define  MGMT_INT_CTRL0_MASK_MISC		BIT(5)
+#define INT_MODERATION				0x1108
+#define INT_MODERATION_RX			GENMASK(11, 0)
+#define INT_MODERATION_TX			GENMASK(27, 16)
+#define EFUSE_OP_CTRL_0				0x1500
+#define  EFUSE_OP_MODE				GENMASK(1, 0)
+#define   EFUSE_OP_ROW_READ			0x1
+#define  EFUSE_OP_START				BIT(2)
+#define  EFUSE_OP_ADDR				GENMASK(15, 8)
+#define EFUSE_OP_CTRL_1				0x1504
+#define  EFUSE_OP_DONE				BIT(1)
+#define  EFUSE_OP_RD_DATA			GENMASK(31, 24)
+#define SYS_RESET				0x152c
+#define  SYS_RESET_RESET			BIT(31)
+#define GMAC_OFFSET				0x2000
+
+/* Constants */
+#define EFUSE_READ_TIMEOUT_US			20000
+#define EFUSE_PATCH_REGION_OFFSET		18
+#define EFUSE_PATCH_MAX_NUM			39
+#define EFUSE_ADDR_MACA0LR			0x1520
+#define EFUSE_ADDR_MACA0HR			0x1524
+
+struct motorcomm_efuse_patch {
+	__le16 addr;
+	__le32 data;
+} __packed;
+
+struct dwmac_motorcomm_priv {
+	void __iomem *base;
+	struct device *dev;
+};
+
+static int motorcomm_efuse_read_byte(struct dwmac_motorcomm_priv *priv,
+				     u8 offset, u8 *byte)
+{
+	u32 reg;
+	int ret;
+
+	writel(FIELD_PREP(EFUSE_OP_MODE, EFUSE_OP_ROW_READ)	|
+	       FIELD_PREP(EFUSE_OP_ADDR, offset)		|
+	       EFUSE_OP_START, priv->base + EFUSE_OP_CTRL_0);
+
+	ret = readl_poll_timeout(priv->base + EFUSE_OP_CTRL_1,
+				 reg, reg & EFUSE_OP_DONE, 2000,
+				 EFUSE_READ_TIMEOUT_US);
+
+	reg = readl(priv->base + EFUSE_OP_CTRL_1);
+	*byte = FIELD_GET(EFUSE_OP_RD_DATA, reg);
+
+	return ret;
+}
+
+static int motorcomm_efuse_read_patch(struct dwmac_motorcomm_priv *priv,
+				      u8 index,
+				      struct motorcomm_efuse_patch *patch)
+{
+	u8 buf[sizeof(*patch)], offset;
+	int i, ret;
+
+	for (i = 0; i < sizeof(*patch); i++) {
+		offset = EFUSE_PATCH_REGION_OFFSET + sizeof(*patch) * index + i;
+
+		ret = motorcomm_efuse_read_byte(priv, offset, &buf[i]);
+		if (ret)
+			return ret;
+	}
+
+	memcpy(patch, buf, sizeof(*patch));
+
+	return 0;
+}
+
+static int motorcomm_efuse_get_patch_value(struct dwmac_motorcomm_priv *priv,
+					   u16 addr, u32 *value)
+{
+	struct motorcomm_efuse_patch patch;
+	int i, ret;
+
+	for (i = 0; i < EFUSE_PATCH_MAX_NUM; i++) {
+		ret = motorcomm_efuse_read_patch(priv, i, &patch);
+		if (ret)
+			return ret;
+
+		if (patch.addr == 0) {
+			return -ENOENT;
+		} else if (le16_to_cpu(patch.addr) == addr) {
+			*value = le32_to_cpu(patch.data);
+			return 0;
+		}
+	}
+
+	return -ENOENT;
+}
+
+static int motorcomm_efuse_read_mac(struct dwmac_motorcomm_priv *priv, u8 *mac)
+{
+	u32 maca0lr, maca0hr;
+	int ret;
+
+	ret = motorcomm_efuse_get_patch_value(priv, EFUSE_ADDR_MACA0LR,
+					      &maca0lr);
+	if (ret)
+		return dev_err_probe(priv->dev, ret,
+				     "failed to read maca0lr from eFuse\n");
+
+	ret = motorcomm_efuse_get_patch_value(priv, EFUSE_ADDR_MACA0HR,
+					      &maca0hr);
+	if (ret)
+		return dev_err_probe(priv->dev, ret,
+				     "failed to read maca0hr from eFuse\n");
+
+	mac[0] = FIELD_GET(GENMASK(15, 8), maca0hr);
+	mac[1] = FIELD_GET(GENMASK(7, 0), maca0hr);
+	mac[2] = FIELD_GET(GENMASK(31, 24), maca0lr);
+	mac[3] = FIELD_GET(GENMASK(23, 16), maca0lr);
+	mac[4] = FIELD_GET(GENMASK(15, 8), maca0lr);
+	mac[5] = FIELD_GET(GENMASK(7, 0), maca0lr);
+
+	return 0;
+}
+
+static void motorcomm_reset_phy(struct dwmac_motorcomm_priv *priv)
+{
+	u32 reg = readl(priv->base + EPHY_CTRL);
+
+	reg &= ~EPHY_RESET;
+	writel(reg, priv->base + EPHY_CTRL);
+
+	reg |= EPHY_RESET;
+	writel(reg, priv->base + EPHY_CTRL);
+}
+
+static void motorcomm_reset(struct dwmac_motorcomm_priv *priv)
+{
+	u32 reg = readl(priv->base + SYS_RESET);
+
+	reg &= ~SYS_RESET_RESET;
+	writel(reg, priv->base + SYS_RESET);
+
+	reg |= SYS_RESET_RESET;
+	writel(reg, priv->base + SYS_RESET);
+
+	motorcomm_reset_phy(priv);
+}
+
+static void motorcomm_init(struct dwmac_motorcomm_priv *priv)
+{
+	writel(0x0, priv->base + MGMT_INT_CTRL0);
+
+	writel(FIELD_PREP(INT_MODERATION_RX, 200) |
+	       FIELD_PREP(INT_MODERATION_TX, 200),
+	       priv->base + INT_MODERATION);
+
+	/*
+	 * OOB WOL must be disabled during normal operation, or DMA interrupts
+	 * cannot be delivered to the host.
+	 */
+	writel(OOB_WOL_CTRL_DIS, priv->base + OOB_WOL_CTRL);
+}
+
+static int motorcomm_suspend(struct device *dev, void *bsp_priv)
+{
+	struct pci_dev *pdev = to_pci_dev(dev);
+	int ret;
+
+	ret = pci_save_state(pdev);
+	if (ret)
+		return ret;
+
+	pci_set_power_state(pdev, PCI_D3hot);
+
+	return 0;
+}
+
+static int motorcomm_resume(struct device *dev, void *bsp_priv)
+{
+	struct dwmac_motorcomm_priv *priv = bsp_priv;
+	struct pci_dev *pdev = to_pci_dev(dev);
+	int ret;
+
+	pci_restore_state(pdev);
+	pci_set_power_state(pdev, PCI_D0);
+
+	ret = pcim_enable_device(pdev);
+	if (ret)
+		return ret;
+
+	pci_set_master(pdev);
+
+	motorcomm_reset_phy(priv);
+	motorcomm_init(priv);
+
+	return 0;
+}
+
+static struct plat_stmmacenet_data *
+motorcomm_default_plat_data(struct pci_dev *pdev)
+{
+	struct plat_stmmacenet_data *plat;
+	struct device *dev = &pdev->dev;
+
+	plat = devm_kzalloc(dev, sizeof(*plat), GFP_KERNEL);
+	if (!plat)
+		return NULL;
+
+	plat->mdio_bus_data = devm_kzalloc(dev, sizeof(*plat->mdio_bus_data),
+					   GFP_KERNEL);
+	if (!plat->mdio_bus_data)
+		return NULL;
+
+	plat->dma_cfg = devm_kzalloc(dev, sizeof(*plat->dma_cfg), GFP_KERNEL);
+	if (!plat->dma_cfg)
+		return NULL;
+
+	plat->axi = devm_kzalloc(dev, sizeof(*plat->axi), GFP_KERNEL);
+	if (!plat->axi)
+		return NULL;
+
+	plat->dma_cfg->pbl		= DEFAULT_DMA_PBL;
+	plat->dma_cfg->pblx8		= true;
+	plat->dma_cfg->txpbl		= 32;
+	plat->dma_cfg->rxpbl		= 32;
+	plat->dma_cfg->eame		= true;
+	plat->dma_cfg->mixed_burst	= true;
+
+	plat->axi->axi_wr_osr_lmt	= 1;
+	plat->axi->axi_rd_osr_lmt	= 1;
+	plat->axi->axi_mb		= true;
+	plat->axi->axi_blen[0]		= 4;
+	plat->axi->axi_blen[1]		= 8;
+	plat->axi->axi_blen[2]		= 16;
+	plat->axi->axi_blen[3]		= 32;
+
+	plat->bus_id		= pci_dev_id(pdev);
+	plat->phy_addr		= -1;
+	plat->phy_interface	= PHY_INTERFACE_MODE_INTERNAL;
+	plat->clk_csr		= STMMAC_CSR_20_35M;
+	plat->tx_coe		= 1;
+	plat->rx_coe		= 1;
+	plat->maxmtu		= JUMBO_LEN;
+	plat->rx_queues_to_use	= 1;
+	plat->tx_queues_to_use	= 1;
+	plat->clk_ref_rate	= 125000000;
+	plat->has_gmac4		= 1;
+	plat->suspend		= motorcomm_suspend;
+	plat->resume		= motorcomm_resume;
+	plat->flags		= STMMAC_FLAG_TSO_EN;
+
+	return plat;
+}
+
+static int motorcomm_setup_irq(struct pci_dev *pdev,
+			       struct stmmac_resources *res,
+			       struct plat_stmmacenet_data *plat)
+{
+	int ret;
+
+	ret = pci_alloc_irq_vectors(pdev, 6, 6, PCI_IRQ_MSIX);
+	if (ret > 0) {
+		res->rx_irq[0]	= pci_irq_vector(pdev, 0);
+		res->tx_irq[0]	= pci_irq_vector(pdev, 4);
+		res->irq	= pci_irq_vector(pdev, 5);
+
+		plat->flags |= STMMAC_FLAG_MULTI_MSI_EN;
+
+		return 0;
+	}
+
+	dev_info(&pdev->dev, "failed to allocate MSI-X vector: %d\n", ret);
+	dev_info(&pdev->dev, "try MSI instead\n");
+
+	ret = pci_alloc_irq_vectors(pdev, 1, 1, PCI_IRQ_MSI);
+	if (ret < 0)
+		return dev_err_probe(&pdev->dev, ret,
+				     "failed to allocate MSI\n");
+
+	res->irq = pci_irq_vector(pdev, 0);
+
+	return 0;
+}
+
+static int motorcomm_probe(struct pci_dev *pdev, const struct pci_device_id *id)
+{
+	struct plat_stmmacenet_data *plat;
+	struct dwmac_motorcomm_priv *priv;
+	struct stmmac_resources res = {};
+	int ret;
+
+	priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return -ENOMEM;
+
+	priv->dev = &pdev->dev;
+
+	plat = motorcomm_default_plat_data(pdev);
+	if (!plat)
+		return -ENOMEM;
+
+	plat->bsp_priv = priv;
+
+	ret = pcim_enable_device(pdev);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "failed to enable device\n");
+
+	priv->base = pcim_iomap_region(pdev, 0, DRIVER_NAME);
+	if (IS_ERR(priv->base))
+		return dev_err_probe(&pdev->dev, PTR_ERR(priv->base),
+				     "failed to map IO region\n");
+
+	pci_set_master(pdev);
+
+	motorcomm_reset(priv);
+
+	ret = motorcomm_efuse_read_mac(priv, res.mac);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret,
+				     "failed to read MAC from eFuse\n");
+
+	ret = motorcomm_setup_irq(pdev, &res, plat);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "failed to setup IRQ\n");
+
+	motorcomm_init(priv);
+
+	res.addr = priv->base + GMAC_OFFSET;
+
+	return stmmac_dvr_probe(&pdev->dev, plat, &res);
+}
+
+static void motorcomm_remove(struct pci_dev *pdev)
+{
+	stmmac_dvr_remove(&pdev->dev);
+	pci_free_irq_vectors(pdev);
+}
+
+static const struct pci_device_id dwmac_motorcomm_pci_id_table[] = {
+	{ PCI_DEVICE(PCI_VENDOR_ID_MOTORCOMM, 0x6801) },
+	{ },
+};
+MODULE_DEVICE_TABLE(pci, dwmac_motorcomm_pci_id_table);
+
+static struct pci_driver dwmac_motorcomm_pci_driver = {
+	.name = DRIVER_NAME,
+	.id_table = dwmac_motorcomm_pci_id_table,
+	.probe = motorcomm_probe,
+	.remove = motorcomm_remove,
+	.driver = {
+		.pm = &stmmac_simple_pm_ops,
+	},
+};
+
+module_pci_driver(dwmac_motorcomm_pci_driver);
+
+MODULE_DESCRIPTION("DWMAC glue driver for Motorcomm PCI Ethernet controllers");
+MODULE_AUTHOR("Yao Zi <ziyao@disroot.org>");
+MODULE_LICENSE("GPL");
-- 
2.50.1

Re: [PATCH net-next 3/4] net: stmmac: Add glue driver for Motorcomm YT6801 ethernet controller

[Andrew Lunn]

> +static int motorcomm_efuse_read_byte(struct dwmac_motorcomm_priv *priv,
> +				     u8 offset, u8 *byte)
> +{
> +	u32 reg;
> +	int ret;
> +
> +	writel(FIELD_PREP(EFUSE_OP_MODE, EFUSE_OP_ROW_READ)	|
> +	       FIELD_PREP(EFUSE_OP_ADDR, offset)		|
> +	       EFUSE_OP_START, priv->base + EFUSE_OP_CTRL_0);
> +
> +	ret = readl_poll_timeout(priv->base + EFUSE_OP_CTRL_1,
> +				 reg, reg & EFUSE_OP_DONE, 2000,
> +				 EFUSE_READ_TIMEOUT_US);
> +
> +	reg = readl(priv->base + EFUSE_OP_CTRL_1);

Do you actually need this read? The documentation says:

 * Returns: 0 on success and -ETIMEDOUT upon a timeout. In either
 * case, the last read value at @addr is stored in @val.

> +	*byte = FIELD_GET(EFUSE_OP_RD_DATA, reg);
> +
> +	return ret;
> +}

> +static void motorcomm_reset_phy(struct dwmac_motorcomm_priv *priv)
> +{
> +	u32 reg = readl(priv->base + EPHY_CTRL);
> +
> +	reg &= ~EPHY_RESET;
> +	writel(reg, priv->base + EPHY_CTRL);
> +
> +	reg |= EPHY_RESET;
> +	writel(reg, priv->base + EPHY_CTRL);
> +}

How does this differ to the PHY doing its own reset via BMCR?

We need to be careful of lifetimes here. It would be better if the PHY
controlled its own reset. We don't want phylib to configure the PHY
and then the MAC driver reset it etc.

> +static int motorcomm_resume(struct device *dev, void *bsp_priv)
> +{
> +	struct dwmac_motorcomm_priv *priv = bsp_priv;
> +	struct pci_dev *pdev = to_pci_dev(dev);
> +	int ret;
> +
> +	pci_restore_state(pdev);
> +	pci_set_power_state(pdev, PCI_D0);
> +
> +	ret = pcim_enable_device(pdev);
> +	if (ret)
> +		return ret;
> +
> +	pci_set_master(pdev);
> +
> +	motorcomm_reset_phy(priv);

Does the PHY support WoL? You probably should not be touching it if it
can wake the system.

> +		return NULL;
> +
> +	plat->mdio_bus_data = devm_kzalloc(dev, sizeof(*plat->mdio_bus_data),
> +					   GFP_KERNEL);
> +	if (!plat->mdio_bus_data)
> +		return NULL;

Is this required? If you look at other glue drivers which allocate
such a structure, they set members in it:

dwmac-intel.c:	plat->mdio_bus_data->needs_reset = true;
dwmac-loongson.c:		plat->mdio_bus_data->needs_reset = true;
dwmac-tegra.c:	plat->mdio_bus_data->needs_reset = true;
stmmac_pci.c:	plat->mdio_bus_data->needs_reset = true;
stmmac_platform.c:		plat->mdio_bus_data->needs_reset = true;

You don't set anything.

	Andrew

Re: [PATCH net-next 3/4] net: stmmac: Add glue driver for Motorcomm YT6801 ethernet controller

[Yao Zi]

On Thu, Oct 16, 2025 at 10:11:02PM +0200, Andrew Lunn wrote:
> > +static int motorcomm_efuse_read_byte(struct dwmac_motorcomm_priv *priv,
> > +				     u8 offset, u8 *byte)
> > +{
> > +	u32 reg;
> > +	int ret;
> > +
> > +	writel(FIELD_PREP(EFUSE_OP_MODE, EFUSE_OP_ROW_READ)	|
> > +	       FIELD_PREP(EFUSE_OP_ADDR, offset)		|
> > +	       EFUSE_OP_START, priv->base + EFUSE_OP_CTRL_0);
> > +
> > +	ret = readl_poll_timeout(priv->base + EFUSE_OP_CTRL_1,
> > +				 reg, reg & EFUSE_OP_DONE, 2000,
> > +				 EFUSE_READ_TIMEOUT_US);
> > +
> > +	reg = readl(priv->base + EFUSE_OP_CTRL_1);
> 
> Do you actually need this read? The documentation says:
> 
>  * Returns: 0 on success and -ETIMEDOUT upon a timeout. In either
>  * case, the last read value at @addr is stored in @val.

Oops, the extra call to readl() is indeed unnecessary. Will remove it in
next version.

> > +	*byte = FIELD_GET(EFUSE_OP_RD_DATA, reg);
> > +
> > +	return ret;
> > +}
> 
> > +static void motorcomm_reset_phy(struct dwmac_motorcomm_priv *priv)
> > +{
> > +	u32 reg = readl(priv->base + EPHY_CTRL);
> > +
> > +	reg &= ~EPHY_RESET;
> > +	writel(reg, priv->base + EPHY_CTRL);
> > +
> > +	reg |= EPHY_RESET;
> > +	writel(reg, priv->base + EPHY_CTRL);
> > +}
> 
> How does this differ to the PHY doing its own reset via BMCR?

It's named as EPHY_RESET according to the vendor driver, but with my
testing, it seems to reset at least the internal MDIO bus as well: with
this reset asserted (which is the default state after power on or
resumption from D3hot), mdiobus_scan() considers each possible MDIO
address corresponds to a PHY, while no one could be connected
successfully.

> We need to be careful of lifetimes here. It would be better if the PHY
> controlled its own reset. We don't want phylib to configure the PHY
> and then the MAC driver reset it etc.

Though it's still unclear the exact effect of the bit on the PHY since
there's no public documentation, it's essential to deassert it in MAC
code before registering and scanning the MDIO bus, or we could even not
probe the PHY correctly.

For the motorcomm_reset_phy() performed in probe function, it happens
before the registration of MDIO bus, and the PHY isn't probed yet, thus
I think it should be okay.

> > +static int motorcomm_resume(struct device *dev, void *bsp_priv)
> > +{
> > +	struct dwmac_motorcomm_priv *priv = bsp_priv;
> > +	struct pci_dev *pdev = to_pci_dev(dev);
> > +	int ret;
> > +
> > +	pci_restore_state(pdev);
> > +	pci_set_power_state(pdev, PCI_D0);
> > +
> > +	ret = pcim_enable_device(pdev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	pci_set_master(pdev);
> > +
> > +	motorcomm_reset_phy(priv);
> 
> Does the PHY support WoL? You probably should not be touching it if it
> can wake the system.

Yes, it supports WoL, though the functionality isn't implemented in this
series.

As I mentioned before, it's necesasry to at least deassert EPHY_RESET
after resuming from D3hot state, or phy_check_link_status() will always
fail with -EBUSY for the PHY and it cannot be correctly resumed.

Do you think it's acceptable to only deassert the EPHY_RESET without
asserting it manually in the resume hook? With this scheme, even though
EPHY_RESET does affect some PHY functionalities, we're not making the
situation worse, since it's already asserted automatically by hardware
after resuming from D3hot.

> > +		return NULL;
> > +
> > +	plat->mdio_bus_data = devm_kzalloc(dev, sizeof(*plat->mdio_bus_data),
> > +					   GFP_KERNEL);
> > +	if (!plat->mdio_bus_data)
> > +		return NULL;
> 
> Is this required? If you look at other glue drivers which allocate
> such a structure, they set members in it:
> 
> dwmac-intel.c:	plat->mdio_bus_data->needs_reset = true;
> dwmac-loongson.c:		plat->mdio_bus_data->needs_reset = true;
> dwmac-tegra.c:	plat->mdio_bus_data->needs_reset = true;
> stmmac_pci.c:	plat->mdio_bus_data->needs_reset = true;
> stmmac_platform.c:		plat->mdio_bus_data->needs_reset = true;
> 
> You don't set anything.

Yes, it's required, since stmmac_mdio.c won't register a MDIO bus if
plat_stmmacenet_data.mdio_bus_data is NULL.

> 
> 	Andrew

Thanks,
Yao Zi

Re: [PATCH net-next 3/4] net: stmmac: Add glue driver for Motorcomm YT6801 ethernet controller

[Andrew Lunn]

> > > +static void motorcomm_reset_phy(struct dwmac_motorcomm_priv *priv)
> > > +{
> > > +	u32 reg = readl(priv->base + EPHY_CTRL);
> > > +
> > > +	reg &= ~EPHY_RESET;
> > > +	writel(reg, priv->base + EPHY_CTRL);
> > > +
> > > +	reg |= EPHY_RESET;
> > > +	writel(reg, priv->base + EPHY_CTRL);
> > > +}
> > 
> > How does this differ to the PHY doing its own reset via BMCR?
> 
> It's named as EPHY_RESET according to the vendor driver, but with my
> testing, it seems to reset at least the internal MDIO bus as well: with
> this reset asserted (which is the default state after power on or
> resumption from D3hot), mdiobus_scan() considers each possible MDIO
> address corresponds to a PHY, while no one could be connected
> successfully.
> 
> > We need to be careful of lifetimes here. It would be better if the PHY
> > controlled its own reset. We don't want phylib to configure the PHY
> > and then the MAC driver reset it etc.
> 
> Though it's still unclear the exact effect of the bit on the PHY since
> there's no public documentation, it's essential to deassert it in MAC
> code before registering and scanning the MDIO bus, or we could even not
> probe the PHY correctly.
> 
> For the motorcomm_reset_phy() performed in probe function, it happens
> before the registration of MDIO bus, and the PHY isn't probed yet, thus
> I think it should be okay.

Since it resets more than the PHY, it probably should have a different
name, and maybe a comment describing what is actually resets.

And maybe rather than asserting and then deasserting reset, maybe just
deassert the reset? That makes it less dangerous in terms of
lifetimes.

> > > +static int motorcomm_resume(struct device *dev, void *bsp_priv)
> > > +{
> > > +	struct dwmac_motorcomm_priv *priv = bsp_priv;
> > > +	struct pci_dev *pdev = to_pci_dev(dev);
> > > +	int ret;
> > > +
> > > +	pci_restore_state(pdev);
> > > +	pci_set_power_state(pdev, PCI_D0);
> > > +
> > > +	ret = pcim_enable_device(pdev);
> > > +	if (ret)
> > > +		return ret;
> > > +
> > > +	pci_set_master(pdev);
> > > +
> > > +	motorcomm_reset_phy(priv);
> > 
> > Does the PHY support WoL? You probably should not be touching it if it
> > can wake the system.
> 
> Yes, it supports WoL, though the functionality isn't implemented in this
> series.
>
> As I mentioned before, it's necesasry to at least deassert EPHY_RESET
> after resuming from D3hot state, or phy_check_link_status() will always
> fail with -EBUSY for the PHY and it cannot be correctly resumed.

When WoL is implemented, what state will the MAC and the PHY be in? Is
it possible to put the MAC into a deeper suspend state than the PHY,
since the MAC is probably not needed? The PHY obviously needs to keep
working, so it cannot be put into a reset state. So resume should not
need to take it out of reset. Also, i _think_ the phylib core will
assume a PHY used for WoL is kept alive and configured, so it will not
reconfigure it on resume.

So it seems like this code will need some changes when WoL is
implemented. So leave it as is for the moment.

> > Is this required? If you look at other glue drivers which allocate
> > such a structure, they set members in it:
> > 
> > dwmac-intel.c:	plat->mdio_bus_data->needs_reset = true;
> > dwmac-loongson.c:		plat->mdio_bus_data->needs_reset = true;
> > dwmac-tegra.c:	plat->mdio_bus_data->needs_reset = true;
> > stmmac_pci.c:	plat->mdio_bus_data->needs_reset = true;
> > stmmac_platform.c:		plat->mdio_bus_data->needs_reset = true;
> > 
> > You don't set anything.
> 
> Yes, it's required, since stmmac_mdio.c won't register a MDIO bus if
> plat_stmmacenet_data.mdio_bus_data is NULL.

Why? Maybe zoom out, look at the big picture for this driver, and
figure out if that is the correct behaviour for stmmac_mdio to
implement. Is it possible to synthesizer this IP without MDIO?

I was also wondering about all the other parameters you set. Why have
i not seen any other glue driver with similar code? What makes this
glue driver different?

	   Andrew

Re: [PATCH net-next 3/4] net: stmmac: Add glue driver for Motorcomm YT6801 ethernet controller

[Russell King (Oracle)]

On Fri, Oct 17, 2025 at 04:56:23PM +0200, Andrew Lunn wrote:
> > Though it's still unclear the exact effect of the bit on the PHY since
> > there's no public documentation, it's essential to deassert it in MAC
> > code before registering and scanning the MDIO bus, or we could even not
> > probe the PHY correctly.
> > 
> > For the motorcomm_reset_phy() performed in probe function, it happens
> > before the registration of MDIO bus, and the PHY isn't probed yet, thus
> > I think it should be okay.
> 
> Since it resets more than the PHY, it probably should have a different
> name, and maybe a comment describing what is actually resets.

I want to back Andrew's comment here up very strongly.

You will not be the only one looking at this code. There are other
people (e.g. me) who are looking at e.g. the core stmmac code, making
changes to it, which impact the platform glue as well.

The platform glue needs to be understandable to those of us who don't
have knowledge of your platform, so that we can make sense of it and
know what it's actually doing, and thus be able to adapt it when we
push out changes to the core that affect platform glue.

Sadly, it seems 99.9% of platform glue is "dump it into the kernel
and run away".

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

Re: [PATCH net-next 3/4] net: stmmac: Add glue driver for Motorcomm YT6801 ethernet controller

[Yao Zi]

On Fri, Oct 17, 2025 at 04:04:02PM +0100, Russell King (Oracle) wrote:
> On Fri, Oct 17, 2025 at 04:56:23PM +0200, Andrew Lunn wrote:
> > > Though it's still unclear the exact effect of the bit on the PHY since
> > > there's no public documentation, it's essential to deassert it in MAC
> > > code before registering and scanning the MDIO bus, or we could even not
> > > probe the PHY correctly.
> > > 
> > > For the motorcomm_reset_phy() performed in probe function, it happens
> > > before the registration of MDIO bus, and the PHY isn't probed yet, thus
> > > I think it should be okay.
> > 
> > Since it resets more than the PHY, it probably should have a different
> > name, and maybe a comment describing what is actually resets.
> 
> I want to back Andrew's comment here up very strongly.
> 
> You will not be the only one looking at this code. There are other
> people (e.g. me) who are looking at e.g. the core stmmac code, making
> changes to it, which impact the platform glue as well.
> 
> The platform glue needs to be understandable to those of us who don't
> have knowledge of your platform, so that we can make sense of it and
> know what it's actually doing, and thus be able to adapt it when we
> push out changes to the core that affect platform glue.

Thanks, that's a really reasonable point to me. At the time of writing
these register offsets, their exact effects are still unclear to me and
I just copied their names from the vendor driver.

Will comment about the effects of EPHY_RESET and give the offset a more
self-descriptive name (maybe MDIO_PHY_RESET) in v2.

> Sadly, it seems 99.9% of platform glue is "dump it into the kernel
> and run away".
> 
> -- 
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

Best regards,
Yao Zi

Re: [PATCH net-next 3/4] net: stmmac: Add glue driver for Motorcomm YT6801 ethernet controller

[Yao Zi]

On Fri, Oct 17, 2025 at 04:56:23PM +0200, Andrew Lunn wrote:
> > > > +static void motorcomm_reset_phy(struct dwmac_motorcomm_priv *priv)
> > > > +{
> > > > +	u32 reg = readl(priv->base + EPHY_CTRL);
> > > > +
> > > > +	reg &= ~EPHY_RESET;
> > > > +	writel(reg, priv->base + EPHY_CTRL);
> > > > +
> > > > +	reg |= EPHY_RESET;
> > > > +	writel(reg, priv->base + EPHY_CTRL);
> > > > +}
> > > 
> > > How does this differ to the PHY doing its own reset via BMCR?
> > 
> > It's named as EPHY_RESET according to the vendor driver, but with my
> > testing, it seems to reset at least the internal MDIO bus as well: with
> > this reset asserted (which is the default state after power on or
> > resumption from D3hot), mdiobus_scan() considers each possible MDIO
> > address corresponds to a PHY, while no one could be connected
> > successfully.
> > 
> > > We need to be careful of lifetimes here. It would be better if the PHY
> > > controlled its own reset. We don't want phylib to configure the PHY
> > > and then the MAC driver reset it etc.
> > 
> > Though it's still unclear the exact effect of the bit on the PHY since
> > there's no public documentation, it's essential to deassert it in MAC
> > code before registering and scanning the MDIO bus, or we could even not
> > probe the PHY correctly.
> > 
> > For the motorcomm_reset_phy() performed in probe function, it happens
> > before the registration of MDIO bus, and the PHY isn't probed yet, thus
> > I think it should be okay.
> 
> Since it resets more than the PHY, it probably should have a different
> name, and maybe a comment describing what is actually resets.

Okay, it's reasonable and I'll do in v2.

> And maybe rather than asserting and then deasserting reset, maybe just
> deassert the reset? That makes it less dangerous in terms of
> lifetimes.

It's okay to only deassert the bit before MDIO bus scan and after
resuming from D3hot, and I'll change to so it in v2.

> > > > +static int motorcomm_resume(struct device *dev, void *bsp_priv)
> > > > +{
> > > > +	struct dwmac_motorcomm_priv *priv = bsp_priv;
> > > > +	struct pci_dev *pdev = to_pci_dev(dev);
> > > > +	int ret;
> > > > +
> > > > +	pci_restore_state(pdev);
> > > > +	pci_set_power_state(pdev, PCI_D0);
> > > > +
> > > > +	ret = pcim_enable_device(pdev);
> > > > +	if (ret)
> > > > +		return ret;
> > > > +
> > > > +	pci_set_master(pdev);
> > > > +
> > > > +	motorcomm_reset_phy(priv);
> > > 
> > > Does the PHY support WoL? You probably should not be touching it if it
> > > can wake the system.
> > 
> > Yes, it supports WoL, though the functionality isn't implemented in this
> > series.
> >
> > As I mentioned before, it's necesasry to at least deassert EPHY_RESET
> > after resuming from D3hot state, or phy_check_link_status() will always
> > fail with -EBUSY for the PHY and it cannot be correctly resumed.
> 
> When WoL is implemented, what state will the MAC and the PHY be in? Is
> it possible to put the MAC into a deeper suspend state than the PHY,
> since the MAC is probably not needed?

Honestly saying, I don't have an accurate answer. Vendor driver's WoL
functionality doesn't work for me, thus I don't have the environment to
test the hardware behavior.

The vendor driver requires both the PHY and MAC to be online for WoL,
and configures the remote wake-up packet filter integrated in MAC. The
standalone YT8531S PHY does have a dedeciated interrupt line and could
be configured to detect magic packets as well, but I'm not sure whether
interrupt of the one integrated in YT6801 is correctly routed.

> The PHY obviously needs to keep
> working, so it cannot be put into a reset state. So resume should not
> need to take it out of reset. Also, i _think_ the phylib core will
> assume a PHY used for WoL is kept alive and configured, so it will not
> reconfigure it on resume.

Agree, and the WoL scheme implemented in vendor driver does require the
PHY to keep working during suspension. A possible explanation for the
automatically-asserted reset is that it's asserted when the controller
is brought out of suspended D3hot state, but not when entering it, so
the PHY just keeps working until the system is resumed. Still I don't
have a working WoL environment to confirm the guess.

> So it seems like this code will need some changes when WoL is
> implemented. So leave it as is for the moment.

Thanks, I could work on WoL support later when I have more time to dig
the controller further.

> > > Is this required? If you look at other glue drivers which allocate
> > > such a structure, they set members in it:
> > > 
> > > dwmac-intel.c:	plat->mdio_bus_data->needs_reset = true;
> > > dwmac-loongson.c:		plat->mdio_bus_data->needs_reset = true;
> > > dwmac-tegra.c:	plat->mdio_bus_data->needs_reset = true;
> > > stmmac_pci.c:	plat->mdio_bus_data->needs_reset = true;
> > > stmmac_platform.c:		plat->mdio_bus_data->needs_reset = true;
> > > 
> > > You don't set anything.
> > 
> > Yes, it's required, since stmmac_mdio.c won't register a MDIO bus if
> > plat_stmmacenet_data.mdio_bus_data is NULL.
> 
> Why? Maybe zoom out, look at the big picture for this driver, and
> figure out if that is the correct behaviour for stmmac_mdio to
> implement. Is it possible to synthesizer this IP without MDIO?

Yes, I think it should be possible, as described in the datasheet of
DesignWare Ethernet Quality-of-Service Controller IP[1],

> Optional MDIO (Clause 22 and Clause 45) master interface for PHY
> device configuration and management

The MDIO interface is described as "optional". I don't have access to
the IP's userbook or code, so couldn't confirm it.

> I was also wondering about all the other parameters you set. Why have
> i not seen any other glue driver with similar code? What makes this
> glue driver different?

Most glue drivers are for SoC-integrated IPs, for which
stmmac_pltfr_probe() helper could be used to retrieve configuration
arguments from devicetree to fill plat_stmmacenet_data. However, YT6801
is a PCIe-based controller, and we couldn't rely on devicetree to carry
these parameters.

You could find similar parameter setup code in stmmac_pltfr_probe(), and
also other glue drivers for PCIe-based controllers, like dwmac-intel.c
(intel_mgbe_common_data) and dwmac-loongson.c (loongson_default_data).

> 	   Andrew

Thanks,
Yao Zi

[1]: https://www.synopsys.com/dw/doc.php/ds/c/dwc_ether_qos.pdf

Re: [PATCH net-next 3/4] net: stmmac: Add glue driver for Motorcomm YT6801 ethernet controller

[Andrew Lunn]

> > I was also wondering about all the other parameters you set. Why have
> > i not seen any other glue driver with similar code? What makes this
> > glue driver different?
> 
> Most glue drivers are for SoC-integrated IPs, for which
> stmmac_pltfr_probe() helper could be used to retrieve configuration
> arguments from devicetree to fill plat_stmmacenet_data. However, YT6801
> is a PCIe-based controller, and we couldn't rely on devicetree to carry
> these parameters.
> 
> You could find similar parameter setup code in stmmac_pltfr_probe(), and
> also other glue drivers for PCIe-based controllers, like dwmac-intel.c
> (intel_mgbe_common_data) and dwmac-loongson.c (loongson_default_data).

Is there anything common with these two drivers? One of the problems
stmmac has had in the past is that glue driver writers just
copy/paste, rather than refactor other glue drivers to share code.  If
there is shared code, maybe move it into stmmac_pci.c as helpers?

	Andrew

Re: [PATCH net-next 3/4] net: stmmac: Add glue driver for Motorcomm YT6801 ethernet controller

[Yao Zi]

On Sat, Oct 18, 2025 at 04:53:04PM +0200, Andrew Lunn wrote:
> > > I was also wondering about all the other parameters you set. Why have
> > > i not seen any other glue driver with similar code? What makes this
> > > glue driver different?
> > 
> > Most glue drivers are for SoC-integrated IPs, for which
> > stmmac_pltfr_probe() helper could be used to retrieve configuration
> > arguments from devicetree to fill plat_stmmacenet_data. However, YT6801
> > is a PCIe-based controller, and we couldn't rely on devicetree to carry
> > these parameters.
> > 
> > You could find similar parameter setup code in stmmac_pltfr_probe(), and
> > also other glue drivers for PCIe-based controllers, like dwmac-intel.c
> > (intel_mgbe_common_data) and dwmac-loongson.c (loongson_default_data).
> 
> Is there anything common with these two drivers? One of the problems
> stmmac has had in the past is that glue driver writers just
> copy/paste, rather than refactor other glue drivers to share code.  If
> there is shared code, maybe move it into stmmac_pci.c as helpers?

I don't think there's code that could be shared. Parameters configured
in plat(.{dma_cfg,axi}) are mostly hardware-details and dependent on
synthesis parameters, making them repeat less across drivers, e.g.
dwmac-loongson.c configures no AXI parameter, while
intel_mgbe_common_data() configures axi_blen as up to 16, but the
motorcomm controller is capable of burst length up to 32.

Another example is the rx/tx queue number (plat.{rx,tx}_queues_to_use),
which even varies among different controllers supported by dwmac-intel.c

Maybe the most common part among these argument setup routines is the
allocation of plat_stmmacenet_data and its members, but I doubt whether
extracting this part out as a routine helps much for maintenance.

But outside of plat_stmmacenet_data setup code, there is some code
duplicated across PCIe controller drivers and could be effectively
re-used. dwmac-intel.c, dwmac-loongson.c and stmmac_pci.c have the
same implementation for platform suspend/resume routines
(plat_stmmacenet_data.{suspend,resume}). I could send a series to
extract this part out, and re-use the common routine in the motorcomm
glue driver as well, though we still need to define a new function to
addtionally deassert EPHY_RESET.

> 	Andrew

Best regards.
Yao Zi

[PATCH net-next 4/4] MAINTAINERS: Assign myself as maintainer of Motorcomm DWMAC glue driver

[Yao Zi]

I volunteer to maintain the DWMAC glue driver for Motorcomm ethernet
controllers.

Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 MAINTAINERS | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 04193ceb9365..6f44a3f57ab5 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -17445,6 +17445,12 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/net/motorcomm,yt8xxx.yaml
 F:	drivers/net/phy/motorcomm.c
 
+MOTORCOMM DWMAC GLUE DRIVER
+M:	Yao Zi <ziyao@disroot.org>
+L:	netdev@vger.kernel.org
+S:	Maintained
+F:	drivers/net/ethernet/stmicro/stmmac/dwmac-motorcomm.c
+
 MOXA SMARTIO/INDUSTIO/INTELLIO SERIAL CARD
 M:	Jiri Slaby <jirislaby@kernel.org>
 S:	Maintained
-- 
2.50.1

Re: [PATCH net-next 0/4] Add DWMAC glue driver for Motorcomm YT6801

[Mingcong Bai]

Hi Yao,

在 2025/10/15 00:47, Yao Zi 写道:
> This series adds glue driver for Motorcomm YT6801 PCIe ethernet
> controller, which is considered mostly compatible with DWMAC-4 IP by
> inspecting the register layout[1]. It integrates a Motorcomm YT8531S PHY
> (confirmed by reading PHY ID) and GMII is used to connect the PHY to
> MAC[2].
> 
> The initialization logic of the MAC is mostly based on previous upstream
> effort for the controller[3] and the Deepin-maintained downstream Linux
> driver[4] licensed under GPL-2.0 according to its SPDX headers. However,
> this series is a completely re-write of the previous patch series,
> utilizing the existing DWMAC4 driver and introducing a glue driver only.
> 
> This series only aims to add basic networking functions for the
> controller, features like WoL, RSS and LED control are omitted for now.
> Testing is done on Loongson 3A5000 machine. Through a local GbE switch,
> it reaches 871Mbps (TX)/942Mbps (RX) on average,

We (along with Xi Ruoyao and Runhua He) tested this patchset against 
v6.18-rc1 on the following platforms featuring YT6801:

- Loongson XA61201 (LoongArch, Loongson 3A6000-HV)
- Loongson AC612A0_V1.1 (LoongArch, Loongson 3C6000/S)
- Loongson XB612B0_V1.1 (LoongArch, Loongson 3B6000)
- MECHREVO WUJIE14-GX4HRXL (x86-64)

With performance within expectation and S3/reboot working. Both ports 
work on those with two interfaces (AC612A0_V1.1 and XB612B0_V1.1). LED 
indicators do not seem to work properly at this moment, but this was 
already described in your original email.

With that:

Tested-by: Mingcong Bai <jeffbai@aosc.io>
Tested-by: Runhua He <hua@aosc.io>
Tested-by: Xi Ruoyao <xry111@xry111.site>

Best Regards,
Mingcong Bai