* [PATCH net-next v6 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization
2026-09-27 18:52 [PATCH net-next v6 0/3] net: stmmac: add Allwinner H616 EMAC1 support James Hilliard
@ 2026-09-27 18:52 ` James Hilliard
2026-09-29 3:49 ` netdev-bot+sashiko
2026-09-27 18:52 ` [PATCH net-next v6 2/3] dt-bindings: net: allwinner: add H616 EMAC1 James Hilliard
2026-09-27 18:52 ` [PATCH net-next v6 3/3] net: stmmac: sun8i: add support for Allwinner " James Hilliard
2 siblings, 1 reply; 6+ messages in thread
From: James Hilliard @ 2026-09-27 18:52 UTC (permalink / raw)
To: Richard Genoud, Maxime Chevallier, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Maxime Coquelin,
Alexandre Torgue, LABBE Corentin, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
Jose Abreu
Cc: Alastair D'Silva, Maxime Ripard, James Hilliard, netdev,
linux-arm-kernel, linux-sunxi, linux-stm32, linux-kernel,
devicetree
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.
Mask interrupts before requesting the reset and retain the existing
DMA and interrupt-register clearing even if the reset times out. 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.
Fixes: 9f93ac8d4085 ("net-next: stmmac: Add dwmac-sun8i")
Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 53 +++++++++++------------
1 file changed, 26 insertions(+), 27 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 38d7e71de925..5691da796454 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -269,11 +269,33 @@ 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;
+
+ writel(0, ioaddr + EMAC_INT_EN);
+
+ /* The PHY receive clock must be running for the reset to complete. */
+ ret = sun8i_dwmac_reset(ioaddr);
+
+ /* Leave DMA and interrupts disabled even if the reset timed out. */
writel(0, ioaddr + EMAC_RX_CTL1);
writel(0, ioaddr + EMAC_TX_CTL1);
writel(0, ioaddr + EMAC_RX_FRM_FLT);
@@ -281,7 +303,7 @@ static int sun8i_dwmac_dma_reset(void __iomem *ioaddr)
writel(0, ioaddr + EMAC_TX_DESC_LIST);
writel(0, ioaddr + EMAC_INT_EN);
writel(0x1FFFFFF, ioaddr + EMAC_INT_STA);
- return 0;
+ return ret;
}
/* sun8i_dwmac_dma_init() - initialize the EMAC
@@ -738,27 +760,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)
{
@@ -899,7 +900,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;
}
@@ -1221,10 +1224,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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next v6 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization
2026-09-27 18:52 ` [PATCH net-next v6 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization James Hilliard
@ 2026-09-29 3:49 ` netdev-bot+sashiko
0 siblings, 0 replies; 6+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 3:49 UTC (permalink / raw)
To: james.hilliard1
Cc: richard.genoud, maxime.chevallier, andrew+netdev, davem,
edumazet, kuba, pabeni, wens, jernej.skrabec, samuel,
mcoquelin.stm32, alexandre.torgue, clabbe.montjoie, robh,
krzk+dt, conor+dt, peppe.cavallaro, joabreu, alastair, mripard,
netdev, linux-arm-kernel, linux-sunxi, linux-stm32, linux-kernel,
devicetree, andre.przywara
Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.
Critical: 0 · High: 1 · Medium: 2 · 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).
- [High] Before this patch, sun8i_dwmac_dma_reset() always returned 0.
- [Medium] The failure mode the patch sets out to fix can still happen on
H3/H5 boards with an external PHY.
- [Medium] The sun8i MAC soft reset now runs on the MTU-change, ethtool
reopen, ndo_open and resume paths.
--
net: stmmac: sun8i: reset the MAC after PHY initialization
This patch moves the sun8i EMAC software reset out of the end of probe and
into the stmmac dma_ops->reset callback. The reset then runs in
stmmac_hw_setup(), after the PHY has been attached and initialized. Reset
timeouts now go through the hardware-setup error path, and the H3
MDIO-mux reset is kept.
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 38d7e71de925c..5691da796454d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -269,11 +269,33 @@ 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;
> +
> + writel(0, ioaddr + EMAC_INT_EN);
> +
> + /* The PHY receive clock must be running for the reset to complete. */
> + ret = sun8i_dwmac_reset(ioaddr);
[Severity: Medium]
Is the 100 ms poll in sun8i_dwmac_reset() long enough now that the reset
runs right after the PHY is resumed?
On the reopen and resume paths, __stmmac_open() and stmmac_resume() in
stmmac_main.c call phylink_prepare_resume() and then go straight into
stmmac_hw_setup():
__stmmac_open()
phylink_prepare_resume(priv->phylink);
stmmac_reset_queues_param(priv);
ret = stmmac_hw_setup(dev);
stmmac_init_dma_engine()
stmmac_reset()
sun8i_dwmac_dma_reset()
sun8i_dwmac_reset()
phylink_prepare_resume() in drivers/net/phy/phylink.c says it does not
wait for the PHY clock:
* then resume the PHY. Note that 802.3 allows PHYs 500ms before
* the clock meets requirements. We do not implement this delay.
*/
The comment in sun8i_dwmac_reset() already says 10 ms was too short on
OrangePi0 with no cable plugged in. Other dwmac reset implementations
allow more time: dwmac1000 200 ms, dwmac4/imx 1 s, loongson 2 s.
Before this patch, sun8i_dwmac_dma_reset() did not need RXC, so these
paths could not fail here. Could a PHY that is slow but still within the
standard now make the reset time out, so that ndo_open, resume or an MTU
change fails? Could that also leave EMAC_BASIC_CTL1 with a soft reset
still pending?
> +
> + /* Leave DMA and interrupts disabled even if the reset timed out. */
> writel(0, ioaddr + EMAC_RX_CTL1);
> writel(0, ioaddr + EMAC_TX_CTL1);
> writel(0, ioaddr + EMAC_RX_FRM_FLT);
> @@ -281,7 +303,7 @@ static int sun8i_dwmac_dma_reset(void __iomem *ioaddr)
> writel(0, ioaddr + EMAC_TX_DESC_LIST);
> writel(0, ioaddr + EMAC_INT_EN);
> writel(0x1FFFFFF, ioaddr + EMAC_INT_STA);
> - return 0;
> + return ret;
[Severity: High]
sun8i_dwmac_dma_reset() can now return -ETIMEDOUT. Can a failed MTU
change then free the DMA rings twice?
On a running interface, stmmac_change_mtu() in stmmac_main.c calls
__stmmac_release() and then __stmmac_open() with a newly allocated
dma_conf. __stmmac_open() copies that dma_conf into priv->dma_conf before
the reset runs:
__stmmac_open()
memcpy(&priv->dma_conf, dma_conf, sizeof(*dma_conf));
...
ret = stmmac_hw_setup(dev);
if (ret < 0) {
netdev_err(priv->dev, "%s: Hw setup failed\n", __func__);
return ret;
}
priv->dma_conf is not restored when this fails. stmmac_change_mtu() then
frees the resources that priv->dma_conf still points to:
stmmac_change_mtu()
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;
}
The interface is left IFF_UP. On a later ndo_stop, __stmmac_release()
calls free_dma_desc_resources(priv, &priv->dma_conf) again. That touches
descriptor rings, the buf_pool, tx_skbuff arrays and page_pools that
have already been freed.
The same ndo_stop also frees dev->irq a second time, calls
phylink_stop() again, and calls stmmac_release_ptp() again, so the PTP
ref clock is disabled twice.
The broken unwind is in the stmmac core. However, before this patch the
sun8i dma reset always returned 0, so this path could not be reached on
sun8i. The commit message now relies on "the normal hardware-setup error
path".
> }
>
[ ... ]
> @@ -899,7 +900,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]
Can the failure described in the commit message still happen on H3/H5
boards with an external PHY?
The reset in mdio_mux_syscon_switch_fn() still runs during probe, before
any PHY driver has bound. It runs when the external child bus is
registered and its PHY ID is read:
sun8i_dwmac_probe()
sun8i_dwmac_register_mdio_mux()
mdio_mux_init()
of_mdiobus_register(external child)
get_phy_device()
mdio_mux_read()
mdio_mux_syscon_switch_fn(1 -> 2)
sun8i_dwmac_reset()
The external PHY's RX clock may not be running yet, for example because
the PHY or one of its suppliers is not ready. The reset then returns
-ETIMEDOUT, which get_phy_c22_id() in drivers/net/phy/phy_device.c turns
into -EIO:
return (phy_reg == -EIO || phy_reg == -ENODEV) ? -ENODEV : -EIO;
__of_mdiobus_parse_phys() treats -EIO as fatal, so of_mdiobus_register()
fails. mdio_mux_init() in drivers/net/mdio/mdio-mux.c then frees any
child bus that failed with an error other than -EPROBE_DEFER, and never
retries it:
r = of_mdiobus_register(cb->mii_bus, child_bus_node);
if (r) {
mdiobus_free(cb->mii_bus);
if (r == -EPROBE_DEFER) {
ret_val = r;
goto err_loop;
}
devm_kfree(dev, cb);
If the internal child registered, probe succeeds without the external
PHY and ndo_open cannot connect to it. If no child registered,
sun8i_dwmac_probe() fails and the MAC is removed.
Either way, loading the missing driver or supplier later cannot bring
the interface back without reprobing the MAC. The commit message
explains why this reset is kept. Should it also say that H3/H5
external-PHY configurations are still not covered by this fix?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927-submit-h616-emac1-v1-v6-0-e64971f4e414%40gmail.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net-next v6 2/3] dt-bindings: net: allwinner: add H616 EMAC1
2026-09-27 18:52 [PATCH net-next v6 0/3] net: stmmac: add Allwinner H616 EMAC1 support James Hilliard
2026-09-27 18:52 ` [PATCH net-next v6 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization James Hilliard
@ 2026-09-27 18:52 ` James Hilliard
2026-09-27 18:52 ` [PATCH net-next v6 3/3] net: stmmac: sun8i: add support for Allwinner " James Hilliard
2 siblings, 0 replies; 6+ messages in thread
From: James Hilliard @ 2026-09-27 18:52 UTC (permalink / raw)
To: Richard Genoud, Maxime Chevallier, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Maxime Coquelin,
Alexandre Torgue, LABBE Corentin, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
Jose Abreu
Cc: Alastair D'Silva, Maxime Ripard, James Hilliard, netdev,
linux-arm-kernel, linux-sunxi, linux-stm32, linux-kernel,
devicetree, Andre Przywara, Krzysztof Kozlowski
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.
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
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 d3e591157faf..2218a492817f 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
@@ -573,6 +574,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
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net-next v6 3/3] net: stmmac: sun8i: add support for Allwinner H616 EMAC1
2026-09-27 18:52 [PATCH net-next v6 0/3] net: stmmac: add Allwinner H616 EMAC1 support James Hilliard
2026-09-27 18:52 ` [PATCH net-next v6 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization James Hilliard
2026-09-27 18:52 ` [PATCH net-next v6 2/3] dt-bindings: net: allwinner: add H616 EMAC1 James Hilliard
@ 2026-09-27 18:52 ` James Hilliard
2026-09-29 3:49 ` netdev-bot+sashiko
2 siblings, 1 reply; 6+ messages in thread
From: James Hilliard @ 2026-09-27 18:52 UTC (permalink / raw)
To: Richard Genoud, Maxime Chevallier, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Chen-Yu Tsai,
Jernej Skrabec, Samuel Holland, Maxime Coquelin,
Alexandre Torgue, LABBE Corentin, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Giuseppe Cavallaro,
Jose Abreu
Cc: Alastair D'Silva, Maxime Ripard, James Hilliard, netdev,
linux-arm-kernel, linux-sunxi, linux-stm32, linux-kernel,
devicetree, Andre Przywara
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>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Andre Przywara <andre.przywara@arm.com>
Reviewed-by: Alastair D'Silva <alastair@d-silva.org>
Tested-by: Alastair D'Silva <alastair@d-silva.org>
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 5691da796454..47954537b29e 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
@@ -1281,6 +1295,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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next v6 3/3] net: stmmac: sun8i: add support for Allwinner H616 EMAC1
2026-09-27 18:52 ` [PATCH net-next v6 3/3] net: stmmac: sun8i: add support for Allwinner " James Hilliard
@ 2026-09-29 3:49 ` netdev-bot+sashiko
0 siblings, 0 replies; 6+ messages in thread
From: netdev-bot+sashiko @ 2026-09-29 3:49 UTC (permalink / raw)
To: james.hilliard1
Cc: richard.genoud, maxime.chevallier, andrew+netdev, davem,
edumazet, kuba, pabeni, wens, jernej.skrabec, samuel,
mcoquelin.stm32, alexandre.torgue, clabbe.montjoie, robh,
krzk+dt, conor+dt, peppe.cavallaro, joabreu, alastair, mripard,
netdev, linux-arm-kernel, linux-sunxi, linux-stm32, linux-kernel,
devicetree, andre.przywara
Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.
Low priority / nit picks for your consideration:
- [Low] The new H616 EMAC1 variant (`emac_variant_h616_emac1` in
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c) is meant to be…
--
net: stmmac: sun8i: add support for Allwinner H616 EMAC1
This adds an emac_variant for the H616 secondary EMAC. The variant uses a
dedicated syscon register at 0x34 and is described as RMII-only at 10/100
Mbps. It is wired internally to the co-packaged AC200/AC300 EPHY.
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 5691da796454d..47954537b29e4 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
[ ... ]
> @@ -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: Low]
Does anything actually restrict this variant to RMII?
The only place support_mii, support_rmii and support_rgmii appear is in
dwmac-sun8i.c: the kernel-doc, the struct emac_variant definition and the
variant initializers. Nothing reads them.
sun8i_dwmac_set_syscon() picks the register value from plat->phy_interface
and does it the same way for every variant:
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c: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;
...
regmap_field_write(gmac->regmap_field, reg);
...
}
Suppose a DT node with "allwinner,sun50i-h616-emac1" sets phy-mode to "mii"
or "rgmii*". Probe would still succeed, and a value without SYSCON_RMII_EN
would be written to the new register at 0x34. This MAC has no external pins.
dwmac-sun8i has no get_interfaces callback, so stmmac_phylink_setup() adds
the DT-supplied mode to supported_interfaces as is:
if (phy_interface_empty(config->supported_interfaces))
__set_bit(priv->plat->phy_interface,
config->supported_interfaces);
sun8i_dwmac_setup() also always advertises MAC_10 | MAC_100 | MAC_1000 in
mac->link.caps. With a non-RMII phy-mode, phylink could then allow gigabit
on a 10/100-only MAC.
The commit message says:
Add an EMAC1 variant using the dedicated register and enable only RMII.
There is also no in-tree binding for "allwinner,sun50i-h616-emac1" yet. As
far as I can tell, only a correct DT enforces the RMII-only restriction.
Older variants such as emac_variant_v3s have the same unchecked flags, but
this new variant's RMII-only behaviour depends on them. Would it make sense
to reject unsupported phy_interface values in sun8i_dwmac_set_syscon() or at
probe? Another option is a get_interfaces callback built from the support_*
flags. If neither fits, the commit message could say the restriction comes
only from the DT.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260927-submit-h616-emac1-v1-v6-0-e64971f4e414%40gmail.com
^ permalink raw reply [flat|nested] 6+ messages in thread