[PATCH net-next v3 0/3] net: stmmac: add Allwinner H616 EMAC1 support

[PATCH net-next v3 0/3] net: stmmac: add Allwinner H616 EMAC1 support

[James Hilliard]

The H616 secondary EMAC supports RMII at 10/100 Mbps and uses a separate
system-control clock register at offset 0x34. Add its binding and a
sun8i stmmac variant using that register. A distinct compatible without
an older fallback prevents the driver from using EMAC0's clock register.

EMAC1 connects internally to the co-packaged AC200 or AC300 EPHY and has
no external PHY pins. Leave PHY initialization to the PHY driver instead
of using the H3 internal-PHY controls. The RMII-only variant does not
expose the RGMII clock-delay properties.

First move the MAC software reset from probe to the DMA reset callback,
after PHY initialization. This lets the MAC and its MDIO bus remain
registered when the PHY driver or one of its suppliers is not ready yet.
Keep the separate H3 MDIO-mux reset sequence unchanged.

The AC200/AC300 EPHY driver and package bindings are already in
net-next. This series separates the H616 EMAC1 MAC driver and binding
support from the earlier combined series. PWM, MFD and device-tree
enablement are being handled separately.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
Changes in v3:
- Add a prerequisite fix moving the MAC software reset to the DMA reset
  callback, after PHY initialization, so delayed module loading and
  deferred PHY probes do not tear down the MAC and its MDIO bus.
- Preserve the H3 MDIO-mux reset and propagate hardware-reset failures
  through the normal stmmac hardware-setup error path.
- Add Alastair D'Silva to Cc and rebase onto current net-next.
- Link to v2: https://patch.msgid.link/20260915-submit-h616-emac1-v1-v2-0-322b32e40eb9@gmail.com

Changes in v2:
- Drop EMAC1 TX/RX clock-delay property support and keep the existing
  RGMII-only delay descriptions unchanged, as requested by Maxime Ripard.
- Clarify that EMAC1 connects internally to a co-packaged PHY, not an
  external PHY or the H3-style internal-PHY controls.
- Rebase onto current net-next.
- Link to v1: https://patch.msgid.link/20260915-submit-h616-emac1-v1-v1-0-195de0bb1f8a@gmail.com

---
James Hilliard (3):
      net: stmmac: sun8i: reset the MAC after PHY initialization
      dt-bindings: net: allwinner: add H616 EMAC1
      net: stmmac: sun8i: add support for Allwinner H616 EMAC1

 .../bindings/net/allwinner,sun8i-a83t-emac.yaml    | 13 +++++
 .../devicetree/bindings/net/snps,dwmac.yaml        |  2 +
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c  | 66 +++++++++++++---------
 3 files changed, 55 insertions(+), 26 deletions(-)
---
base-commit: 26ee8cd69d46a14b37ba5e512084fe80d730127a
change-id: 20260914-submit-h616-emac1-v1-143703842abb

Best regards,
--  
James Hilliard <james.hilliard1@gmail.com>

[PATCH net-next v3 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization

[James Hilliard]

The MAC software reset needs a running receive clock from the PHY.
Resetting the MAC at the end of probe therefore fails when the PHY driver
has not been loaded or its probe has deferred on a missing supplier. The
failure removes the MAC and its MDIO bus, so loading the missing driver
later cannot recover the interface without reprobing the MAC.

Perform the software reset in the DMA reset callback instead. The stmmac
core calls it during hardware setup after attaching and initializing the
PHY, and resumes a suspended PHY before reopening or resuming the MAC.
Keep the existing register clearing after a successful reset and return
reset errors through the normal hardware-setup error path.

Remove the unconditional reset from probe. Keep the separate H3 MDIO-mux
reset after switching the mux and powering the selected PHY, since it is
needed to latch the selected interface before MDIO accesses.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 50 +++++++++++------------
 1 file changed, 24 insertions(+), 26 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 48c52eb96233..4523a14f5e0c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -269,11 +269,32 @@ static const struct emac_variant emac_variant_h6 = {
 #define SYSCON_ETCS_EXT_GMII	0x1
 #define SYSCON_ETCS_INT_GMII	0x2
 
+static int sun8i_dwmac_reset(void __iomem *ioaddr)
+{
+	u32 v;
+
+	v = readl(ioaddr + EMAC_BASIC_CTL1);
+	writel(v | 0x01, ioaddr + EMAC_BASIC_CTL1);
+
+	/* The timeout was previously set to 10ms, but some board (OrangePI0)
+	 * need more if no cable plugged. 100ms seems OK
+	 */
+	return readl_poll_timeout(ioaddr + EMAC_BASIC_CTL1, v,
+				  !(v & 0x01), 100, 100000);
+}
+
 /* sun8i_dwmac_dma_reset() - reset the EMAC
  * Called from stmmac via stmmac_dma_ops->reset
  */
 static int sun8i_dwmac_dma_reset(void __iomem *ioaddr)
 {
+	int ret;
+
+	/* The PHY receive clock must be running for the reset to complete. */
+	ret = sun8i_dwmac_reset(ioaddr);
+	if (ret)
+		return ret;
+
 	writel(0, ioaddr + EMAC_RX_CTL1);
 	writel(0, ioaddr + EMAC_TX_CTL1);
 	writel(0, ioaddr + EMAC_RX_FRM_FLT);
@@ -738,27 +759,6 @@ static void sun8i_dwmac_flow_ctrl(struct mac_device_info *hw,
 	writel(v, ioaddr + EMAC_TX_FLOW_CTL);
 }
 
-static int sun8i_dwmac_reset(struct stmmac_priv *priv)
-{
-	u32 v;
-	int err;
-
-	v = readl(priv->ioaddr + EMAC_BASIC_CTL1);
-	writel(v | 0x01, priv->ioaddr + EMAC_BASIC_CTL1);
-
-	/* The timeout was previously set to 10ms, but some board (OrangePI0)
-	 * need more if no cable plugged. 100ms seems OK
-	 */
-	err = readl_poll_timeout(priv->ioaddr + EMAC_BASIC_CTL1, v,
-				 !(v & 0x01), 100, 100000);
-
-	if (err) {
-		dev_err(priv->device, "EMAC reset timeout\n");
-		return err;
-	}
-	return 0;
-}
-
 /* Search in mdio-mux node for internal PHY node and get its clk/reset */
 static int get_ephy_nodes(struct stmmac_priv *priv)
 {
@@ -895,7 +895,9 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
 		/* After changing syscon value, the MAC need reset or it will
 		 * use the last value (and so the last PHY set).
 		 */
-		ret = sun8i_dwmac_reset(priv);
+		ret = sun8i_dwmac_reset(priv->ioaddr);
+		if (ret)
+			dev_err(priv->device, "EMAC reset timeout\n");
 	}
 	return ret;
 }
@@ -1217,10 +1219,6 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 			dev_err(&pdev->dev, "Failed to register mux\n");
 			goto dwmac_mux;
 		}
-	} else {
-		ret = sun8i_dwmac_reset(priv);
-		if (ret)
-			goto dwmac_remove;
 	}
 
 	pm_runtime_put(&pdev->dev);

-- 
2.53.0

[PATCH net-next v3 2/3] dt-bindings: net: allwinner: add H616 EMAC1

[James Hilliard]

The H616 secondary EMAC uses a separate system-control clock register
and supports only RMII at 10/100 Mbps. Add its distinct compatible
without an older fallback, since using the primary EMAC clock register
would not work.

Constrain the PHY interface mode to RMII, matching the hardware.

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 .../devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml  | 13 +++++++++++++
 Documentation/devicetree/bindings/net/snps,dwmac.yaml       |  2 ++
 2 files changed, 15 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml b/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml
index 323a669fa982..2cae0133b3cd 100644
--- a/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml
+++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml
@@ -21,6 +21,7 @@ select:
           - allwinner,sun8i-r40-gmac
           - allwinner,sun8i-v3s-emac
           - allwinner,sun50i-a64-emac
+          - allwinner,sun50i-h616-emac1
           - allwinner,sun55i-a523-gmac200
   required:
     - compatible
@@ -33,6 +34,7 @@ properties:
       - const: allwinner,sun8i-r40-gmac
       - const: allwinner,sun8i-v3s-emac
       - const: allwinner,sun50i-a64-emac
+      - const: allwinner,sun50i-h616-emac1
       - items:
           - enum:
               - allwinner,sun20i-d1-emac
@@ -91,6 +93,17 @@ required:
 
 allOf:
   - $ref: snps,dwmac.yaml#
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: allwinner,sun50i-h616-emac1
+
+    then:
+      properties:
+        phy-mode:
+          const: rmii
+
   - if:
       properties:
         compatible:
diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
index 2ed4564ae99d..b20149c3e850 100644
--- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
+++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
@@ -57,6 +57,7 @@ properties:
         - allwinner,sun8i-r40-gmac
         - allwinner,sun8i-v3s-emac
         - allwinner,sun50i-a64-emac
+        - allwinner,sun50i-h616-emac1
         - amlogic,meson6-dwmac
         - amlogic,meson8b-dwmac
         - amlogic,meson8m2-dwmac
@@ -623,6 +624,7 @@ allOf:
                 - allwinner,sun8i-r40-gmac
                 - allwinner,sun8i-v3s-emac
                 - allwinner,sun50i-a64-emac
+                - allwinner,sun50i-h616-emac1
                 - loongson,ls2k-dwmac
                 - loongson,ls7a-dwmac
                 - ingenic,jz4775-mac

-- 
2.53.0

[PATCH net-next v3 3/3] net: stmmac: sun8i: add support for Allwinner H616 EMAC1

[James Hilliard]

The H616 secondary EMAC uses a separate system-control clock register
and supports only RMII at 10/100 Mbps. It connects internally to the
co-packaged AC200 or AC300 EPHY and has no external PHY pins.

Add an EMAC1 variant using the dedicated register and enable only RMII.
Leave PHY initialization to the PHY driver instead of using the H3
internal-PHY controls. No RX or TX clock delays are configured for this
RMII-only variant.

Co-developed-by: Richard Genoud <richard.genoud@bootlin.com>
Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 4523a14f5e0c..1020168f1c3a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -81,6 +81,13 @@ static const struct reg_field sun8i_syscon_reg_field = {
 	.msb = 31,
 };
 
+/* EMAC1 clock register @ 0x34 in the "system control" address range */
+static const struct reg_field sun8i_syscon_reg_field_emac1 = {
+	.reg = 0x34,
+	.lsb = 0,
+	.msb = 31,
+};
+
 /* EMAC clock register @ 0x164 in the CCU address range */
 static const struct reg_field sun8i_ccu_reg_field = {
 	.reg = 0x164,
@@ -143,6 +150,13 @@ static const struct emac_variant emac_variant_h6 = {
 	.tx_delay_max = 7,
 };
 
+static const struct emac_variant emac_variant_h616_emac1 = {
+	.syscon_field = &sun8i_syscon_reg_field_emac1,
+	/* The co-packaged AC200/AC300 PHY does not use the H3 PHY controls. */
+	.soc_has_internal_phy = false,
+	.support_rmii = true,
+};
+
 #define EMAC_BASIC_CTL0 0x00
 #define EMAC_BASIC_CTL1 0x04
 #define EMAC_INT_STA    0x08
@@ -1276,6 +1290,8 @@ static const struct of_device_id sun8i_dwmac_match[] = {
 		.data = &emac_variant_a64 },
 	{ .compatible = "allwinner,sun50i-h6-emac",
 		.data = &emac_variant_h6 },
+	{ .compatible = "allwinner,sun50i-h616-emac1",
+		.data = &emac_variant_h616_emac1 },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, sun8i_dwmac_match);

-- 
2.53.0

Re: [PATCH net-next v3 0/3] net: stmmac: add Allwinner H616 EMAC1 support

[Alastair D'Silva]

On Thu, 2026-09-17 at 11:55 -0600, James Hilliard wrote:
> The H616 secondary EMAC supports RMII at 10/100 Mbps and uses a
> separate
> system-control clock register at offset 0x34. Add its binding and a
> sun8i stmmac variant using that register. A distinct compatible
> without
> an older fallback prevents the driver from using EMAC0's clock
> register.
> 
> EMAC1 connects internally to the co-packaged AC200 or AC300 EPHY and
> has
> no external PHY pins. Leave PHY initialization to the PHY driver
> instead
> of using the H3 internal-PHY controls. The RMII-only variant does not
> expose the RGMII clock-delay properties.
> 
> First move the MAC software reset from probe to the DMA reset
> callback,
> after PHY initialization. This lets the MAC and its MDIO bus remain
> registered when the PHY driver or one of its suppliers is not ready
> yet.
> Keep the separate H3 MDIO-mux reset sequence unchanged.
> 
> The AC200/AC300 EPHY driver and package bindings are already in
> net-next. This series separates the H616 EMAC1 MAC driver and binding
> support from the earlier combined series. PWM, MFD and device-tree
> enablement are being handled separately.
> 
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
> Changes in v3:
> - Add a prerequisite fix moving the MAC software reset to the DMA
> reset
>   callback, after PHY initialization, so delayed module loading and
>   deferred PHY probes do not tear down the MAC and its MDIO bus.
> - Preserve the H3 MDIO-mux reset and propagate hardware-reset
> failures
>   through the normal stmmac hardware-setup error path.
> - Add Alastair D'Silva to Cc and rebase onto current net-next.
> - Link to v2: https://patch.msgid.link/20260915-submit-h616-emac1-v1-
> v2-0-322b32e40eb9@gmail.com
> 
> Changes in v2:
> - Drop EMAC1 TX/RX clock-delay property support and keep the existing
>   RGMII-only delay descriptions unchanged, as requested by Maxime
> Ripard.
> - Clarify that EMAC1 connects internally to a co-packaged PHY, not an
>   external PHY or the H3-style internal-PHY controls.
> - Rebase onto current net-next.
> - Link to v1: https://patch.msgid.link/20260915-submit-h616-emac1-v1-
> v1-0-195de0bb1f8a@gmail.com
> 
> ---
> James Hilliard (3):
>       net: stmmac: sun8i: reset the MAC after PHY initialization
>       dt-bindings: net: allwinner: add H616 EMAC1
>       net: stmmac: sun8i: add support for Allwinner H616 EMAC1
> 
>  .../bindings/net/allwinner,sun8i-a83t-emac.yaml    | 13 +++++
>  .../devicetree/bindings/net/snps,dwmac.yaml        |  2 +
>  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c  | 66
> +++++++++++++---------
>  3 files changed, 55 insertions(+), 26 deletions(-)
> ---
> base-commit: 26ee8cd69d46a14b37ba5e512084fe80d730127a
> change-id: 20260914-submit-h616-emac1-v1-143703842abb
> 
> Best regards,
> --  
> James Hilliard <james.hilliard1@gmail.com>
> 

Confirmed working on the Mellow Fly C5 when brought in as a module and
backported to 6.18, tested in the Armbian environment, along with the
recommended PWM patch:
https://lore.kernel.org/all/20260804-h616-pwm-v8-v8-0-db37ab8624ae@gmail.com/T/


root@mellowflyc5:~# lsmod
Module                  Size  Used by
rtw88_8821cs           12288  0
rtw88_8821c            86016  1 rtw88_8821cs
rtw88_sdio             20480  1 rtw88_8821cs
rtw88_core            180224  2 rtw88_8821c,rtw88_sdio
snd_soc_hdmi_codec     16384  0
mac80211              929792  2 rtw88_sdio,rtw88_core
zram                   36864  2
842_decompress         12288  1 zram
842_compress           16384  1 zram
gs_usb                 20480  0
can_dev                36864  1 gs_usb
dw_hdmi_i2s_audio      12288  0
dw_hdmi_cec            12288  0
cdc_acm                32768  0
sun50i_h6_prcm_ppu     12288  0
panfrost               73728  0
governor_simpleondemand    12288  0
gpu_sched              45056  1 panfrost
sun8i_ce               36864  0
drm_shmem_helper       24576  1 panfrost
crypto_engine          12288  1 sun8i_ce
cfg80211              831488  2 rtw88_core,mac80211
binfmt_misc            16384  1
rfkill                 24576  2 cfg80211
sch_fq_codel           16384  2
fuse                  163840  1
configfs               40960  1
nfnetlink              16384  2
ip_tables              24576  0
x_tables               28672  1 ip_tables
btrfs                1441792  0
blake2b_generic        16384  0
xor                    12288  1 btrfs
raid6_pq               94208  1 btrfs
ac300_phy              12288  1
ac200_phy              12288  0
dwmac_sun8i            20480  0

root@mellowflyc5:~# uname -a
Linux mellowflyc5 6.18.52-current-sunxi64 #27 SMP PREEMPT Mon Sep 14
21:36:19 AEST 2026 aarch64 GNU/Linux

root@mellowflyc5:~# ifconfig end0
end0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST>  mtu 1500
        inet 10.0.1.136  netmask 255.255.255.0  broadcast 10.0.1.255
        inet6 fe80::9aff:fea2:59e8  prefixlen 64  scopeid 0x20<link>
        ether 02:00:9a:a2:59:e8  txqueuelen 1000  (Ethernet)
        RX packets 5202  bytes 941999 (919.9 KiB)
        RX errors 0  dropped 0  overruns 0  frame 0
        TX packets 4059  bytes 431730 (421.6 KiB)
        TX errors 0  dropped 5 overruns 0  carrier 0  collisions 0
        device interrupt 50  

root@mellowflyc5:~# iperf3 -c 10.0.1.1
Connecting to host 10.0.1.1, port 5201
[  5] local 10.0.1.136 port 53578 connected to 10.0.1.1 port 5201
[ ID] Interval           Transfer     Bitrate         Retr  Cwnd
[  5]   0.00-1.00   sec  12.0 MBytes   101 Mbits/sec    0    191 KBytes
[  5]   1.00-2.00   sec  11.5 MBytes  96.5 Mbits/sec    0    191 KBytes
[  5]   2.00-3.00   sec  11.1 MBytes  93.3 Mbits/sec    0    191 KBytes
[  5]   3.00-4.00   sec  11.2 MBytes  94.4 Mbits/sec    0    191 KBytes
[  5]   4.00-5.00   sec  11.2 MBytes  94.4 Mbits/sec    0    191 KBytes
[  5]   5.00-6.00   sec  11.1 MBytes  93.3 Mbits/sec    0    191 KBytes
[  5]   6.00-7.00   sec  11.4 MBytes  95.4 Mbits/sec    0    191 KBytes
[  5]   7.00-8.00   sec  11.2 MBytes  94.3 Mbits/sec    0    191 KBytes
[  5]   8.00-9.00   sec  11.2 MBytes  94.4 Mbits/sec    0    191 KBytes
[  5]   9.00-10.00  sec  11.1 MBytes  93.2 Mbits/sec    0    191 KBytes
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.00  sec   113 MBytes  95.0 Mbits/sec    0           
sender
[  5]   0.00-10.01  sec   112 MBytes  94.1 Mbits/sec                 
receiver

iperf Done.



I did notice that the speed and activity LEDs on the magjack remained
dark.

LED Output Pad Enables (Register 0x05 - SYS_IO)
-----------------------------------------------

According to the AC300 datasheet (Section 4.2.5), bits [3:1] default to
0 (disabled):
   - Bit 1: E_LNK_LED_IO_EN
   - Bit 2: E_SPD_LED_IO_EN
   - Bit 3: E_DPX_LED_IO_EN

In drivers/net/phy/xpowers/ac300.c, AC300_SYS_IO_VALUE does not set   
any of these bits. Consequently, the LED outputs remain disabled/tri-
stated, and neither the link nor speed LEDs illuminate on the board.

LED Polarity (Register 0x06 - EPHY_CONFIG)
------------------------------------------

Once the I/O pads are enabled, Register 0x06 bit 1 (LED_POL) controls
the drive logic:
   - Bit 1 = 0: Active-High (Default)
   - Bit 1 = 1: Active-Low

Because common RJ45 magjacks (such as the HY911105AE on Fly-C5, Orange
Pi Zero 2W/3, etc.) have LED anodes connected to 3.3V, the PHY must
sink current (Active-Low) to drive them. Without setting LED_POL = 1,
the LED logic is inverted.

Could we update AC300_SYS_IO_VALUE to enable the LED IO pads, and
configure LED_POL for active-low operation (or wire it up to the phylib
LED framework)?


Suggested patch for drivers/net/phy/xpowers/ac300.c:

--- a/drivers/net/phy/xpowers/ac300.c
+++ b/drivers/net/phy/xpowers/ac300.c
@@ -43,10 +43,14 @@
 #define AC300_IO_DRV_LEVEL_2			2
 #define AC300_CLKIN_PAD_ENABLE			BIT(4)
+#define AC300_EPHY_DPX_LED_IO_ENABLE		BIT(3)
+#define AC300_EPHY_SPD_LED_IO_ENABLE		BIT(2)
+#define AC300_EPHY_LNK_LED_IO_ENABLE		BIT(1)
 #define AC300_EPHY_MII_IO_ENABLE		BIT(0)
 
 #define AC300_EPHY_CONFIG_REG			0x06
 #define AC300_EPHY_BGS_EFFUSE_MASK		GENMASK(15, 12)
 #define AC300_EPHY_RMII_SEL			BIT(11)
+#define AC300_EPHY_LED_POL_ACTIVE_LOW		BIT(1)
 #define AC300_EPHY_SHUTDOWN			BIT(0)
 
@@ -58,7 +62,10 @@
 #define AC300_SYS_IO_VALUE \
 	(FIELD_PREP(AC300_MDIO_DRV_MASK, AC300_IO_DRV_LEVEL_2) | \
 	 FIELD_PREP(AC300_MII_DRV_MASK, AC300_IO_DRV_LEVEL_2) | \
-	 AC300_CLKIN_PAD_ENABLE | AC300_EPHY_MII_IO_ENABLE)
+	 AC300_CLKIN_PAD_ENABLE | AC300_EPHY_MII_IO_ENABLE | \
+	 AC300_EPHY_LNK_LED_IO_ENABLE | \
+	 AC300_EPHY_SPD_LED_IO_ENABLE | \
+	 AC300_EPHY_DPX_LED_IO_ENABLE)
 
 static u16 ac300_ephy_ctl_config(const struct ac300_ephy_ctl *priv)
 {
@@ -131,7 +138,8 @@ static u16 ac300_ephy_ctl_config(const struct
ac300_ephy_ctl *priv)
 	return priv->ephy_config |
+		AC300_EPHY_LED_POL_ACTIVE_LOW |
 		(priv->interface == PHY_INTERFACE_MODE_RMII ?
 		 AC300_EPHY_RMII_SEL : 0);
 }


-- 
Alastair D'Silva

Re: [PATCH net-next v3 3/3] net: stmmac: sun8i: add support for Allwinner H616 EMAC1

[Maxime Chevallier]

Hi,

On 9/17/26 19:55, James Hilliard wrote:
> The H616 secondary EMAC uses a separate system-control clock register
> and supports only RMII at 10/100 Mbps. It connects internally to the
> co-packaged AC200 or AC300 EPHY and has no external PHY pins.
> 
> Add an EMAC1 variant using the dedicated register and enable only RMII.
> Leave PHY initialization to the PHY driver instead of using the H3
> internal-PHY controls. No RX or TX clock delays are configured for this
> RMII-only variant.
> 
> Co-developed-by: Richard Genoud <richard.genoud@bootlin.com>
> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Thanks :)

Maxime

> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 4523a14f5e0c..1020168f1c3a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -81,6 +81,13 @@ static const struct reg_field sun8i_syscon_reg_field = {
>  	.msb = 31,
>  };
>  
> +/* EMAC1 clock register @ 0x34 in the "system control" address range */
> +static const struct reg_field sun8i_syscon_reg_field_emac1 = {
> +	.reg = 0x34,
> +	.lsb = 0,
> +	.msb = 31,
> +};
> +
>  /* EMAC clock register @ 0x164 in the CCU address range */
>  static const struct reg_field sun8i_ccu_reg_field = {
>  	.reg = 0x164,
> @@ -143,6 +150,13 @@ static const struct emac_variant emac_variant_h6 = {
>  	.tx_delay_max = 7,
>  };
>  
> +static const struct emac_variant emac_variant_h616_emac1 = {
> +	.syscon_field = &sun8i_syscon_reg_field_emac1,
> +	/* The co-packaged AC200/AC300 PHY does not use the H3 PHY controls. */
> +	.soc_has_internal_phy = false,
> +	.support_rmii = true,
> +};
> +
>  #define EMAC_BASIC_CTL0 0x00
>  #define EMAC_BASIC_CTL1 0x04
>  #define EMAC_INT_STA    0x08
> @@ -1276,6 +1290,8 @@ static const struct of_device_id sun8i_dwmac_match[] = {
>  		.data = &emac_variant_a64 },
>  	{ .compatible = "allwinner,sun50i-h6-emac",
>  		.data = &emac_variant_h6 },
> +	{ .compatible = "allwinner,sun50i-h616-emac1",
> +		.data = &emac_variant_h616_emac1 },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, sun8i_dwmac_match);
> 

Re: [PATCH net-next v3 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization

[Maxime Chevallier]

Hi,

On 9/17/26 19:55, James Hilliard wrote:
> The MAC software reset needs a running receive clock from the PHY.
> Resetting the MAC at the end of probe therefore fails when the PHY driver
> has not been loaded or its probe has deferred on a missing supplier. The
> failure removes the MAC and its MDIO bus, so loading the missing driver
> later cannot recover the interface without reprobing the MAC.
> 
> Perform the software reset in the DMA reset callback instead. The stmmac
> core calls it during hardware setup after attaching and initializing the
> PHY, and resumes a suspended PHY before reopening or resuming the MAC.
> Keep the existing register clearing after a successful reset and return
> reset errors through the normal hardware-setup error path.
> 
> Remove the unconditional reset from probe. Keep the separate H3 MDIO-mux
> reset after switching the mux and powering the selected PHY, since it is
> needed to latch the selected interface before MDIO accesses.
> 
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Maxime

> ---
>  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 50 +++++++++++------------
>  1 file changed, 24 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 48c52eb96233..4523a14f5e0c 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -269,11 +269,32 @@ static const struct emac_variant emac_variant_h6 = {
>  #define SYSCON_ETCS_EXT_GMII	0x1
>  #define SYSCON_ETCS_INT_GMII	0x2
>  
> +static int sun8i_dwmac_reset(void __iomem *ioaddr)
> +{
> +	u32 v;
> +
> +	v = readl(ioaddr + EMAC_BASIC_CTL1);
> +	writel(v | 0x01, ioaddr + EMAC_BASIC_CTL1);
> +
> +	/* The timeout was previously set to 10ms, but some board (OrangePI0)
> +	 * need more if no cable plugged. 100ms seems OK
> +	 */
> +	return readl_poll_timeout(ioaddr + EMAC_BASIC_CTL1, v,
> +				  !(v & 0x01), 100, 100000);
> +}
> +
>  /* sun8i_dwmac_dma_reset() - reset the EMAC
>   * Called from stmmac via stmmac_dma_ops->reset
>   */
>  static int sun8i_dwmac_dma_reset(void __iomem *ioaddr)
>  {
> +	int ret;
> +
> +	/* The PHY receive clock must be running for the reset to complete. */
> +	ret = sun8i_dwmac_reset(ioaddr);
> +	if (ret)
> +		return ret;
> +
>  	writel(0, ioaddr + EMAC_RX_CTL1);
>  	writel(0, ioaddr + EMAC_TX_CTL1);
>  	writel(0, ioaddr + EMAC_RX_FRM_FLT);
> @@ -738,27 +759,6 @@ static void sun8i_dwmac_flow_ctrl(struct mac_device_info *hw,
>  	writel(v, ioaddr + EMAC_TX_FLOW_CTL);
>  }
>  
> -static int sun8i_dwmac_reset(struct stmmac_priv *priv)
> -{
> -	u32 v;
> -	int err;
> -
> -	v = readl(priv->ioaddr + EMAC_BASIC_CTL1);
> -	writel(v | 0x01, priv->ioaddr + EMAC_BASIC_CTL1);
> -
> -	/* The timeout was previously set to 10ms, but some board (OrangePI0)
> -	 * need more if no cable plugged. 100ms seems OK
> -	 */
> -	err = readl_poll_timeout(priv->ioaddr + EMAC_BASIC_CTL1, v,
> -				 !(v & 0x01), 100, 100000);
> -
> -	if (err) {
> -		dev_err(priv->device, "EMAC reset timeout\n");
> -		return err;
> -	}
> -	return 0;
> -}
> -
>  /* Search in mdio-mux node for internal PHY node and get its clk/reset */
>  static int get_ephy_nodes(struct stmmac_priv *priv)
>  {
> @@ -895,7 +895,9 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
>  		/* After changing syscon value, the MAC need reset or it will
>  		 * use the last value (and so the last PHY set).
>  		 */
> -		ret = sun8i_dwmac_reset(priv);
> +		ret = sun8i_dwmac_reset(priv->ioaddr);
> +		if (ret)
> +			dev_err(priv->device, "EMAC reset timeout\n");
>  	}
>  	return ret;
>  }
> @@ -1217,10 +1219,6 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
>  			dev_err(&pdev->dev, "Failed to register mux\n");
>  			goto dwmac_mux;
>  		}
> -	} else {
> -		ret = sun8i_dwmac_reset(priv);
> -		if (ret)
> -			goto dwmac_remove;
>  	}
>  
>  	pm_runtime_put(&pdev->dev);
> 

Re: [PATCH net-next v3 0/3] net: stmmac: add Allwinner H616 EMAC1 support

[Maxime Chevallier]

Hi Alastair,


> Confirmed working on the Mellow Fly C5 when brought in as a module and
> backported to 6.18, tested in the Armbian environment, along with the
> recommended PWM patch:
> https://lore.kernel.org/all/20260804-h616-pwm-v8-v8-0-db37ab8624ae@gmail.com/T/
> 
Thanks a lot for testing, this is great :)

Can you add you Tested-by tag ?

Thanks,

Maxime

Re: [PATCH net-next v3 3/3] net: stmmac: sun8i: add support for Allwinner H616 EMAC1

[Andre Przywara]

Hi,

On 9/17/26 19:55, James Hilliard wrote:
> The H616 secondary EMAC uses a separate system-control clock register
> and supports only RMII at 10/100 Mbps. It connects internally to the
> co-packaged AC200 or AC300 EPHY and has no external PHY pins.
> 
> Add an EMAC1 variant using the dedicated register and enable only RMII.
> Leave PHY initialization to the PHY driver instead of using the H3
> internal-PHY controls. No RX or TX clock delays are configured for this
> RMII-only variant.
> 
> Co-developed-by: Richard Genoud <richard.genoud@bootlin.com>
> Signed-off-by: Richard Genoud <richard.genoud@bootlin.com>
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>

Looks good to me, nice and minimal:

Reviewed-by: Andre Przywara <andre.przywara@arm.com>

Cheers,
Andre


> ---
>   drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
> 
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 4523a14f5e0c..1020168f1c3a 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -81,6 +81,13 @@ static const struct reg_field sun8i_syscon_reg_field = {
>   	.msb = 31,
>   };
>   
> +/* EMAC1 clock register @ 0x34 in the "system control" address range */
> +static const struct reg_field sun8i_syscon_reg_field_emac1 = {
> +	.reg = 0x34,
> +	.lsb = 0,
> +	.msb = 31,
> +};
> +
>   /* EMAC clock register @ 0x164 in the CCU address range */
>   static const struct reg_field sun8i_ccu_reg_field = {
>   	.reg = 0x164,
> @@ -143,6 +150,13 @@ static const struct emac_variant emac_variant_h6 = {
>   	.tx_delay_max = 7,
>   };
>   
> +static const struct emac_variant emac_variant_h616_emac1 = {
> +	.syscon_field = &sun8i_syscon_reg_field_emac1,
> +	/* The co-packaged AC200/AC300 PHY does not use the H3 PHY controls. */
> +	.soc_has_internal_phy = false,
> +	.support_rmii = true,
> +};
> +
>   #define EMAC_BASIC_CTL0 0x00
>   #define EMAC_BASIC_CTL1 0x04
>   #define EMAC_INT_STA    0x08
> @@ -1276,6 +1290,8 @@ static const struct of_device_id sun8i_dwmac_match[] = {
>   		.data = &emac_variant_a64 },
>   	{ .compatible = "allwinner,sun50i-h6-emac",
>   		.data = &emac_variant_h6 },
> +	{ .compatible = "allwinner,sun50i-h616-emac1",
> +		.data = &emac_variant_h616_emac1 },
>   	{ }
>   };
>   MODULE_DEVICE_TABLE(of, sun8i_dwmac_match);
> 

Re: [PATCH net-next v3 0/3] net: stmmac: add Allwinner H616 EMAC1 support

[Alastair D'Silva]

On Thu, 2026-09-17 at 11:55 -0600, James Hilliard wrote:
> The H616 secondary EMAC supports RMII at 10/100 Mbps and uses a
> separate
> system-control clock register at offset 0x34. Add its binding and a
> sun8i stmmac variant using that register. A distinct compatible
> without
> an older fallback prevents the driver from using EMAC0's clock
> register.
> 
> EMAC1 connects internally to the co-packaged AC200 or AC300 EPHY and
> has
> no external PHY pins. Leave PHY initialization to the PHY driver
> instead
> of using the H3 internal-PHY controls. The RMII-only variant does not
> expose the RGMII clock-delay properties.
> 
> First move the MAC software reset from probe to the DMA reset
> callback,
> after PHY initialization. This lets the MAC and its MDIO bus remain
> registered when the PHY driver or one of its suppliers is not ready
> yet.
> Keep the separate H3 MDIO-mux reset sequence unchanged.
> 
> The AC200/AC300 EPHY driver and package bindings are already in
> net-next. This series separates the H616 EMAC1 MAC driver and binding
> support from the earlier combined series. PWM, MFD and device-tree
> enablement are being handled separately.
> 
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
> Changes in v3:
> - Add a prerequisite fix moving the MAC software reset to the DMA
> reset
>   callback, after PHY initialization, so delayed module loading and
>   deferred PHY probes do not tear down the MAC and its MDIO bus.
> - Preserve the H3 MDIO-mux reset and propagate hardware-reset
> failures
>   through the normal stmmac hardware-setup error path.
> - Add Alastair D'Silva to Cc and rebase onto current net-next.
> - Link to v2: https://patch.msgid.link/20260915-submit-h616-emac1-v1-
> v2-0-322b32e40eb9@gmail.com
> 
> Changes in v2:
> - Drop EMAC1 TX/RX clock-delay property support and keep the existing
>   RGMII-only delay descriptions unchanged, as requested by Maxime
> Ripard.
> - Clarify that EMAC1 connects internally to a co-packaged PHY, not an
>   external PHY or the H3-style internal-PHY controls.
> - Rebase onto current net-next.
> - Link to v1: https://patch.msgid.link/20260915-submit-h616-emac1-v1-
> v1-0-195de0bb1f8a@gmail.com
> 
> ---
> James Hilliard (3):
>       net: stmmac: sun8i: reset the MAC after PHY initialization
>       dt-bindings: net: allwinner: add H616 EMAC1
>       net: stmmac: sun8i: add support for Allwinner H616 EMAC1
> 
>  .../bindings/net/allwinner,sun8i-a83t-emac.yaml    | 13 +++++
>  .../devicetree/bindings/net/snps,dwmac.yaml        |  2 +
>  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c  | 66
> +++++++++++++---------
>  3 files changed, 55 insertions(+), 26 deletions(-)
> ---
> base-commit: 26ee8cd69d46a14b37ba5e512084fe80d730127a
> change-id: 20260914-submit-h616-emac1-v1-143703842abb
> 
> Best regards,
> --  
> James Hilliard <james.hilliard1@gmail.com>
> 

Tested-by: Alastair D'Silva <alastair@d-silva.org>
Reviewed-by: Alastair D'Silva <alastair@d-silva.org>

As a follow-up (non blocking), I would add the LED control registers I
mentioned at the bottom of my test results. 

-- 
Alastair D'Silva

Re: [PATCH net-next v3 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 6 potential
issue(s) to consider.

Critical: 0 · High: 1 (1) · Medium: 3 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] A reset timeout during an MTU-change reopen leaves the still-
  running netdev owning freed DMA resources, causing a later double free.
- [Medium] Incomplete fix: the retained probe-time MAC soft reset in
  mdio_mux_syscon_switch_fn()…
- [Medium] Reset-timeout failure path leaves the EMAC un-quiesced.
- [Medium] The MAC reset timeout is shorter than the PHY receive-clock
  startup interval that IEEE 802.3 permits, so the newly relocated reset…
- [Low] The patch fixes a user-visible failure (MAC probe fails and the
  netdev plus its MDIO bus are torn down when the PHY driver has not…
- [Low] Stale comment and vacuous runtime-PM window in
  sun8i_dwmac_probe().

Pre-existing issues:
- [High] Use-after-free on the sun8i_dwmac_probe() `dwmac_mux:` error
  path.

--

net: stmmac: sun8i: reset the MAC after PHY initialization

This moves the sun8i EMAC software reset out of probe and into the stmmac
->reset (DMA reset) callback, so it runs during hardware setup once the PHY
has been attached and resumed. The H3 MDIO-mux reset after switching the
syscon mux is kept, and sun8i_dwmac_reset() now takes an ioaddr instead of
a stmmac_priv.

A few questions below.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 48c52eb962335..4523a14f5e0c3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -269,11 +269,32 @@ static const struct emac_variant emac_variant_h6 = {
>  #define SYSCON_ETCS_EXT_GMII	0x1
>  #define SYSCON_ETCS_INT_GMII	0x2
>  
> +static int sun8i_dwmac_reset(void __iomem *ioaddr)
> +{
> +	u32 v;
> +
> +	v = readl(ioaddr + EMAC_BASIC_CTL1);
> +	writel(v | 0x01, ioaddr + EMAC_BASIC_CTL1);
> +
> +	/* The timeout was previously set to 10ms, but some board (OrangePI0)
> +	 * need more if no cable plugged. 100ms seems OK
> +	 */
> +	return readl_poll_timeout(ioaddr + EMAC_BASIC_CTL1, v,
> +				  !(v & 0x01), 100, 100000);
> +}

[Severity: Medium]

Is 100 ms enough for this poll now that the reset runs right after the PHY
is brought back out of power-down?

sun8i_dwmac_dma_reset() is reached from stmmac_hw_setup(), which is called
after phylink_prepare_resume() on the reopen and resume paths.
phylink_prepare_resume() says it does not wait for the receive clock:

drivers/net/phy/phylink.c:phylink_prepare_resume() {
	/* IEEE 802.3 22.2.4.1.5 allows PHYs to stop their receive clock
	 * when PDOWN is set. ... Note that 802.3 allows PHYs 500ms before
	 * the clock meets requirements. We do not implement this delay.
	 */
	if (pl->config->mac_requires_rxc && phydev && phydev->suspended)
		phy_resume(phydev);
}

stmmac sets mac_requires_rxc, so for a PHY that takes longer than 100 ms
after leaving PDOWN to produce a valid RXC, would the reset time out and
make stmmac_hw_setup() fail on MTU change and on system resume?

>  /* sun8i_dwmac_dma_reset() - reset the EMAC
>   * Called from stmmac via stmmac_dma_ops->reset
>   */
>  static int sun8i_dwmac_dma_reset(void __iomem *ioaddr)
>  {
> +	int ret;
> +
> +	/* The PHY receive clock must be running for the reset to complete. */
> +	ret = sun8i_dwmac_reset(ioaddr);
> +	if (ret)
> +		return ret;
> +
>  	writel(0, ioaddr + EMAC_RX_CTL1);
>  	writel(0, ioaddr + EMAC_TX_CTL1);
>  	writel(0, ioaddr + EMAC_RX_FRM_FLT);

[Severity: Medium]

Does this early return leave the EMAC un-quiesced?

Two things change on the timeout path.  First, sun8i_dwmac_reset() has
already set the self-clearing SOFT_RST request (bit 0 of EMAC_BASIC_CTL1)
and nothing withdraws it.  Second, the register clearing that the ->reset
callback previously did unconditionally is now skipped:

	writel(0, ioaddr + EMAC_RX_CTL1);
	writel(0, ioaddr + EMAC_TX_CTL1);
	writel(0, ioaddr + EMAC_RX_FRM_FLT);
	writel(0, ioaddr + EMAC_RX_DESC_LIST);
	writel(0, ioaddr + EMAC_TX_DESC_LIST);
	writel(0, ioaddr + EMAC_INT_EN);
	writel(0x1FFFFFF, ioaddr + EMAC_INT_STA);

The callers do not compensate.  In stmmac_main.c, __stmmac_open() reaches
init_error: which only returns, unlike irq_error: which calls
stmmac_stop_all_dma().  stmmac_resume() likewise just powers down the
legacy serdes and returns.

On resume the host IRQ is still registered while EMAC_INT_EN keeps the
enables from before suspend (stmmac_suspend only calls
stmmac_stop_all_dma()) and EMAC_INT_STA is never acknowledged.  Should the
descriptor lists and interrupt registers still be cleared before returning
the error?

[Severity: High]

Can a reset timeout here turn an MTU change into a double free of the DMA
resources?

stmmac_change_mtu() releases the interface and reopens it with the new
configuration:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_change_mtu() {
		__stmmac_release(dev);

		ret = __stmmac_open(dev, dma_conf);
		if (ret) {
			free_dma_desc_resources(priv, dma_conf);
			kfree(dma_conf);
			netdev_err(priv->dev, "failed reopening the interface after MTU change\n");
			return ret;
		}
}

__stmmac_open() publishes the new pointers before the fallible setup:

	memcpy(&priv->dma_conf, dma_conf, sizeof(*dma_conf));
	...
	phylink_prepare_resume(priv->phylink);
	...
	ret = stmmac_hw_setup(dev);
	if (ret < 0)
		goto init_error;

struct stmmac_dma_conf embeds rx_queue[] and tx_queue[] by value, so
priv->dma_conf ends up with its own copies of the descriptor, buffer and
page pool pointers that stmmac_change_mtu() then frees.  The interface
stays administratively up, and the next close does:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:__stmmac_release() {
	/* Release and free the Rx/Tx resources */
	free_dma_desc_resources(priv, &priv->dma_conf);
}

The MTU reopen is also the case where phylink_stop() has just powered the
PHY down, so the new -ETIMEDOUT from sun8i_dwmac_dma_reset() is reachable
there.  The core recovery gap in stmmac_change_mtu() and __stmmac_open()
predates this patch, but the commit message says it wants to "return reset
errors through the normal hardware-setup error path" - is that path safe to
use on sun8i today?

[ ... ]

> @@ -895,7 +895,9 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
>  		/* After changing syscon value, the MAC need reset or it will
>  		 * use the last value (and so the last PHY set).
>  		 */
> -		ret = sun8i_dwmac_reset(priv);
> +		ret = sun8i_dwmac_reset(priv->ioaddr);
> +		if (ret)
> +			dev_err(priv->device, "EMAC reset timeout\n");
>  	}
>  	return ret;
>  }

[Severity: Medium]

Does this call site keep the receive-clock dependency the commit message
says it removes?

The new comment in sun8i_dwmac_dma_reset() states "The PHY receive clock
must be running for the reset to complete", and the core honours that:
stmmac_hw_setup() wraps stmmac_init_dma_engine() in
phylink_rx_clk_stop_block(), and __stmmac_open()/stmmac_resume() call
phylink_prepare_resume() first.

mdio_mux_syscon_switch_fn() has neither guarantee.  It runs during MAC
probe:

sun8i_dwmac_register_mdio_mux()
  mdio_mux_init()
    of_mdiobus_register(child)
      mdio_mux_read()
        mdio_mux_syscon_switch_fn()
          sun8i_dwmac_reset(priv->ioaddr)

so no PHY driver can have been bound yet, and for
DWMAC_SUN8I_MDIO_MUX_EXTERNAL_ID the internal EPHY has just been unpowered
by sun8i_dwmac_unpower_internal_phy().

On timeout the error propagates out of mdio_mux_read(), the child bus PHY
scan fails, and mdio-mux.c drops that child without failing mdio_mux_init()
as a whole:

drivers/net/mdio/mdio-mux.c:mdio_mux_init() {
		r = of_mdiobus_register(cb->mii_bus, child_bus_node);
		if (r) {
			mdiobus_free(cb->mii_bus);
			if (r == -EPROBE_DEFER) { ... }
			devm_kfree(dev, cb);
			dev_err(dev, "Error: Failed to register MDIO bus for child %pOF: %pe\n", ...);
}

Would probe then succeed with the external PHY permanently absent, which is
the same reprobe-to-recover state the commit describes fixing?

Separately, the timeout now only logs.  The syscon mux field has already
been rewritten and the EPHY power state already changed while the MAC has
not latched the new selection - should either be restored before returning?

[Severity: High]

This isn't a bug introduced by this patch, but the reset kept here makes an
existing unbalanced probe teardown easier to reach.  When
mdio_mux_syscon_switch_fn() has already taken the internal branch,
sun8i_dwmac_power_internal_phy() has set gmac->internal_phy_powered, and
sun8i_dwmac_register_mdio_mux() then fails (this reset timing out, or
of_mdiobus_register() returning -EPROBE_DEFER, or mdio_mux_init()
returning -ENODEV for "No acceptable child buses found"), probe takes
goto dwmac_mux:

drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c:sun8i_dwmac_probe() {
dwmac_mux:
	reset_control_put(gmac->rst_ephy);
	clk_put(gmac->ephy_clk);
dwmac_remove:
	pm_runtime_put_noidle(&pdev->dev);
	stmmac_pltfr_remove(pdev);
}

stmmac_pltfr_remove() -> stmmac_dvr_remove() ends with
priv->plat->exit(dev, priv->plat->bsp_priv), which is sun8i_dwmac_exit():

	if (gmac->variant->soc_has_internal_phy)
		sun8i_dwmac_unpower_internal_phy(gmac);

and because internal_phy_powered is still true:

drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c:sun8i_dwmac_unpower_internal_phy() {
	if (!gmac->internal_phy_powered)
		return;

	clk_disable_unprepare(gmac->ephy_clk);
	reset_control_assert(gmac->rst_ephy);
}

Does this use the clk and reset_control after they were put?
sun8i_dwmac_remove() uses the other order, unpowering before putting:

		mdio_mux_uninit(gmac->mux_handle);
		sun8i_dwmac_unpower_internal_phy(gmac);
		reset_control_put(gmac->rst_ephy);
		clk_put(gmac->ephy_clk);

> @@ -1217,10 +1219,6 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
>  			dev_err(&pdev->dev, "Failed to register mux\n");
>  			goto dwmac_mux;
>  		}
> -	} else {
> -		ret = sun8i_dwmac_reset(priv);
> -		if (ret)
> -			goto dwmac_remove;
>  	}
>  
>  	pm_runtime_put(&pdev->dev);

[Severity: Low]

With this else arm gone, the comment above pm_runtime_get_sync() in
sun8i_dwmac_probe() no longer matches the code:

	/* the MAC is runtime suspended after stmmac_dvr_probe(), so we
	 * need to ensure the MAC resume back before other operations such
	 * as reset.
	 */
	pm_runtime_get_sync(&pdev->dev);

There is no reset left in probe.  The only remaining work between the
get_sync and pm_runtime_put() is the if (gmac->variant->soc_has_internal_phy)
block, so for v3s, a83t, r40, a64 and h6 the pair now brackets nothing.
Should the comment be updated, and should the get/put be moved inside the
soc_has_internal_phy branch?

[Severity: Low]

Should this carry a Fixes: tag and a target tree?  The commit fixes a
user-visible failure where probe tears down the netdev and its MDIO bus.
git blame on the removed if/else and probe-time reset points at
634db83b8265 ("net: stmmac: dwmac-sun8i: add support for mdio-mux"), on top
of the original driver 9f93ac8d4085 ("net-next: stmmac: Add dwmac-sun8i").

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260917-submit-h616-emac1-v1-v3-0-62cb8316e19b%40gmail.com

Re: [PATCH net-next v3 3/3] net: stmmac: sun8i: add support for Allwinner H616 EMAC1

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new `emac_variant_h616_emac1` in
  drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c (lines 153-158)…

--

net: stmmac: sun8i: add support for Allwinner H616 EMAC1

Adds a new emac_variant for the H616 secondary EMAC, pointing at a
dedicated syscon clock register at offset 0x34, and registers the
allwinner,sun50i-h616-emac1 compatible.  The variant is described as
RMII-only with the PHY handled by the co-packaged AC200/AC300 driver.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 4523a14f5e0c3..1020168f1c3a0 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -81,6 +81,13 @@ static const struct reg_field sun8i_syscon_reg_field = {
>  	.msb = 31,
>  };
>  
> +/* EMAC1 clock register @ 0x34 in the "system control" address range */
> +static const struct reg_field sun8i_syscon_reg_field_emac1 = {
> +	.reg = 0x34,
> +	.lsb = 0,
> +	.msb = 31,
> +};
> +
>  /* EMAC clock register @ 0x164 in the CCU address range */
>  static const struct reg_field sun8i_ccu_reg_field = {
>  	.reg = 0x164,

[ ... ]

> @@ -143,6 +150,13 @@ static const struct emac_variant emac_variant_h6 = {
>  	.tx_delay_max = 7,
>  };
>  
> +static const struct emac_variant emac_variant_h616_emac1 = {
> +	.syscon_field = &sun8i_syscon_reg_field_emac1,
> +	/* The co-packaged AC200/AC300 PHY does not use the H3 PHY controls. */
> +	.soc_has_internal_phy = false,
> +	.support_rmii = true,
> +};

[Severity: Medium]
The commit message says "Add an EMAC1 variant using the dedicated register
and enable only RMII".  Where is support_rmii actually consumed?

Grepping drivers/net/ethernet/stmicro/stmmac/ at this commit, every
occurrence of support_mii/support_rmii/support_rgmii is either the
kernel-doc line for struct emac_variant, the bool member declaration, or a
per-variant initializer.  There is no variant->support_* read anywhere in
the tree, so setting only .support_rmii here has no runtime effect.

The one interface-mode gate left in the driver is variant agnostic, in
sun8i_dwmac_set_syscon():

	switch (plat->phy_interface) {
	case PHY_INTERFACE_MODE_MII:
		/* default */
		break;
	case PHY_INTERFACE_MODE_RGMII:
	case PHY_INTERFACE_MODE_RGMII_ID:
	case PHY_INTERFACE_MODE_RGMII_RXID:
	case PHY_INTERFACE_MODE_RGMII_TXID:
		reg |= SYSCON_EPIT | SYSCON_ETCS_INT_GMII;
		break;
	case PHY_INTERFACE_MODE_RMII:
		reg |= SYSCON_RMII_EN | SYSCON_ETCS_EXT_GMII;
		break;
	default:
		dev_err(dev, "Unsupported interface mode: %s",
			phy_modes(plat->phy_interface));
		return -EINVAL;
	}

	regmap_field_write(gmac->regmap_field, reg);

With compatible = "allwinner,sun50i-h616-emac1" and phy-mode = "rgmii",
does this probe successfully and program SYSCON_EPIT |
SYSCON_ETCS_INT_GMII into the new EMAC1 clock register at syscon 0x34?
And with phy-mode = "mii", is SYSCON_RMII_EN simply left clear?  Both
select a clock/pin routing this instance does not have, with no -EINVAL and
no message logged.

Nothing downstream re-adds the restriction either: dwmac-sun8i provides no
get_interfaces callback, so stmmac_phylink_setup() just accepts whatever
firmware picked:

	if (phy_interface_empty(config->supported_interfaces))
		__set_bit(priv->plat->phy_interface,
			  config->supported_interfaces);

That leaves the RMII-only constraint expressed only in the binding added by
the preceding patch in the series, "dt-bindings: net: allwinner: add H616
EMAC1" (phy-mode: const: rmii for this compatible), which is checked by
dtbs_check at build time rather than at probe time.

Would it make sense to reject non-RMII modes for this variant in
sun8i_dwmac_set_syscon(), or to start reading variant->support_* there?

On a related note, the commit message says the block "supports only RMII at
10/100 Mbps", but sun8i_dwmac_setup() still advertises gigabit for every
variant, including this one:

	mac->link.caps = MAC_ASYM_PAUSE | MAC_SYM_PAUSE |
			 MAC_10 | MAC_100 | MAC_1000;

Should the H616 EMAC1 variant also limit link.caps to MAC_10 | MAC_100?

> +
>  #define EMAC_BASIC_CTL0 0x00
>  #define EMAC_BASIC_CTL1 0x04
>  #define EMAC_INT_STA    0x08
> @@ -1276,6 +1290,8 @@ static const struct of_device_id sun8i_dwmac_match[] = {
>  		.data = &emac_variant_a64 },
>  	{ .compatible = "allwinner,sun50i-h6-emac",
>  		.data = &emac_variant_h6 },
> +	{ .compatible = "allwinner,sun50i-h616-emac1",
> +		.data = &emac_variant_h616_emac1 },
>  	{ }
>  };
>  MODULE_DEVICE_TABLE(of, sun8i_dwmac_match);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260917-submit-h616-emac1-v1-v3-0-62cb8316e19b%40gmail.com