* [PATCH net-next v5 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization
2026-09-22 0:45 [PATCH net-next v5 0/3] net: stmmac: add Allwinner H616 EMAC1 support James Hilliard
@ 2026-09-22 0:45 ` James Hilliard
2026-09-22 7:02 ` Maxime Chevallier
2026-09-23 0:47 ` netdev-bot+sashiko
2026-09-22 0:45 ` [PATCH net-next v5 2/3] dt-bindings: net: allwinner: add H616 EMAC1 James Hilliard
2026-09-22 0:45 ` [PATCH net-next v5 3/3] net: stmmac: sun8i: add support for Allwinner " James Hilliard
2 siblings, 2 replies; 8+ messages in thread
From: James Hilliard @ 2026-09-22 0:45 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")
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 48c52eb96233..9598c386bb21 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)
{
@@ -895,7 +896,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 +1220,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] 8+ messages in thread* Re: [PATCH net-next v5 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization
2026-09-22 0:45 ` [PATCH net-next v5 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization James Hilliard
@ 2026-09-22 7:02 ` Maxime Chevallier
2026-09-23 0:47 ` netdev-bot+sashiko
1 sibling, 0 replies; 8+ messages in thread
From: Maxime Chevallier @ 2026-09-22 7:02 UTC (permalink / raw)
To: James Hilliard, Richard Genoud, 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, netdev, linux-arm-kernel,
linux-sunxi, linux-stm32, linux-kernel, devicetree
Hi,
On 9/22/26 02:45, 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.
> 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")
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
Tested on H2S that it doesn't break the platform,
Tested-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Maxime
> ---
> 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 48c52eb96233..9598c386bb21 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)
> {
> @@ -895,7 +896,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 +1220,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);
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH net-next v5 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization
2026-09-22 0:45 ` [PATCH net-next v5 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization James Hilliard
2026-09-22 7:02 ` Maxime Chevallier
@ 2026-09-23 0:47 ` netdev-bot+sashiko
1 sibling, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-09-23 0:47 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 1 potential
issue(s) to consider.
Critical: 0 · High: 1 (1) · Medium: 0 · 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] Incomplete fix: on the soc_has_internal_phy variants (H3, V3s)
the clock-dependent MAC software reset is still executed at probe time…
Pre-existing issues:
- [High] Use-after-free in the sun8i_dwmac_probe() 'dwmac_mux:' error
path: it calls reset_control_put(gmac->rst_ephy) and…
--
net: stmmac: sun8i: reset the MAC after PHY initialization
The MAC software reset is moved out of probe and into the
stmmac_dma_ops->reset callback, so it runs during hardware setup after the
PHY has been attached and initialized. Interrupts are masked before the
reset is requested, the DMA and interrupt registers are still cleared on
timeout, and the reset error is returned through the normal hardware-setup
error path. The separate 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 48c52eb962335..9598c386bb216 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);
> +
[ ... ]
> @@ -895,7 +896,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: High]
On the soc_has_internal_phy variants (H3, V3s), does this retained reset
still hit the exact condition the changelog says is being removed? The
switch callback runs at probe time from PHY-ID discovery, before any
external PHY driver is bound:
sun8i_dwmac_probe()
sun8i_dwmac_register_mdio_mux()
mdio_mux_init() /* pb->current_child = -1 */
of_mdiobus_register() /* external child bus */
get_phy_device()
mdio_mux_read()
mdio_mux_syscon_switch_fn()
sun8i_dwmac_unpower_internal_phy() /* clock gated,
reset asserted */
sun8i_dwmac_reset(priv->ioaddr)
On the DWMAC_SUN8I_MDIO_MUX_EXTERNAL_ID branch the internal EPHY has just
been unpowered and the external PHY has no driver-managed clock yet, which
is the same "no running receive clock" case that sun8i_dwmac_dma_reset()
now documents with:
/* The PHY receive clock must be running for the reset to complete. */
What happens when this times out during discovery? The -ETIMEDOUT is
returned from switch_fn instead of the register value:
drivers/net/mdio/mdio-mux.c:mdio_mux_read() {
r = pb->switch_fn(pb->current_child, cb->bus_number, pb->switch_data);
if (r)
goto out;
pb->current_child = cb->bus_number;
...
}
get_phy_c22_id() only maps -EIO/-ENODEV to -ENODEV, so -ETIMEDOUT reaches:
drivers/net/mdio/of_mdio.c:__of_mdiobus_parse_phys() {
if (rc == -ENODEV)
dev_err(...);
else if (rc)
goto exit;
...
}
which aborts registration of the whole external child bus, and mdio_mux_init()
then drops that child with "Error: Failed to register MDIO bus for child".
Probe still returns 0 because the internal child registered, but the external
PHY device is never created, so phylink_of_phy_connect() at open fails and
loading the PHY driver later cannot recover without reprobing the MAC. Isn't
that the same unrecoverable failure the commit message describes?
Also, since mdio_mux_read() leaves pb->current_child unchanged on error,
does every subsequent access to that child repeat the switch and spend
another 100 ms in the timeout?
> @@ -1217,10 +1220,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: High]
This isn't a bug introduced by this patch, but the "goto dwmac_mux" error
path above looks like it uses the EPHY clock and reset handles after they
have been put, and the retained mux reset makes that path easier to reach.
sun8i_dwmac_probe() error path:
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:
if (priv->plat->exit)
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 that still calls clk_disable_unprepare(gmac->ephy_clk) and
reset_control_assert(gmac->rst_ephy) on the handles freed just above.
Does this touch freed memory whenever mdio_mux_syscon_switch_fn() already
powered the internal PHY (gmac->internal_phy_powered == true) and
mdio_mux_init() then fails, for example a second child bus returning
-EPROBE_DEFER, "No acceptable child buses found", or the retained MAC reset
returning -ETIMEDOUT?
Would the ordering used by sun8i_dwmac_remove() work here too, i.e.
mdio_mux_uninit(), then sun8i_dwmac_unpower_internal_phy(), then
reset_control_put() and clk_put()?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921-submit-h616-emac1-v1-v5-0-ada661ed9e9b%40gmail.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next v5 2/3] dt-bindings: net: allwinner: add H616 EMAC1
2026-09-22 0:45 [PATCH net-next v5 0/3] net: stmmac: add Allwinner H616 EMAC1 support James Hilliard
2026-09-22 0:45 ` [PATCH net-next v5 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization James Hilliard
@ 2026-09-22 0:45 ` James Hilliard
2026-09-23 0:47 ` netdev-bot+sashiko
2026-09-22 0:45 ` [PATCH net-next v5 3/3] net: stmmac: sun8i: add support for Allwinner " James Hilliard
2 siblings, 1 reply; 8+ messages in thread
From: James Hilliard @ 2026-09-22 0:45 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 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: 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>
---
.../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 0ff9e3284a06..3fa6e17c3d12 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
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH net-next v5 2/3] dt-bindings: net: allwinner: add H616 EMAC1
2026-09-22 0:45 ` [PATCH net-next v5 2/3] dt-bindings: net: allwinner: add H616 EMAC1 James Hilliard
@ 2026-09-23 0:47 ` netdev-bot+sashiko
0 siblings, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-09-23 0:47 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] Concern (validated as a FALSE POSITIVE in the series context):
the patch documents `allwinner,sun50i-h616-emac1` in…
--
dt-bindings: net: allwinner: add H616 EMAC1
Adds the allwinner,sun50i-h616-emac1 compatible to the sun8i-a83t-emac
binding and to snps,dwmac.yaml, with no older fallback compatible, and
constrains phy-mode to rmii for that compatible. It is also added to the
snps,dwmac.yaml list that excludes snps,tso.
> diff --git a/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml b/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml
> index 323a669fa9823..2cae0133b3cd4 100644
> --- a/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml
> +++ b/Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml
[ ... ]
> @@ -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
[Severity: Low]
This isn't a bug, just a note for the record after checking it.
At this commit allwinner,sun50i-h616-emac1 has no match in the tree:
git grep h616-emac1 drivers/net/ethernet/stmicro/stmmac/
(no matches)
so a device tree node using only this compatible, with no older
fallback, validates but does not probe here.
The next and final patch in the series, "net: stmmac: sun8i: add support
for Allwinner H616 EMAC1", adds it to sun8i_dwmac_match[]:
{ .compatible = "allwinner,sun50i-h616-emac1",
.data = &emac_variant_h616_emac1 },
and emac_variant_h616_emac1 uses sun8i_syscon_reg_field_emac1 with only
.support_rmii = true, which matches both the "separate system-control
clock register" statement in the changelog and the phy-mode: const: rmii
constraint added here.
Since bindings are expected to land before the driver, and the
intermediate state only leaves the compatible unmatched, there is
nothing to change.
[ ... ]
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921-submit-h616-emac1-v1-v5-0-ada661ed9e9b%40gmail.com
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH net-next v5 3/3] net: stmmac: sun8i: add support for Allwinner H616 EMAC1
2026-09-22 0:45 [PATCH net-next v5 0/3] net: stmmac: add Allwinner H616 EMAC1 support James Hilliard
2026-09-22 0:45 ` [PATCH net-next v5 1/3] net: stmmac: sun8i: reset the MAC after PHY initialization James Hilliard
2026-09-22 0:45 ` [PATCH net-next v5 2/3] dt-bindings: net: allwinner: add H616 EMAC1 James Hilliard
@ 2026-09-22 0:45 ` James Hilliard
2026-09-23 0:47 ` netdev-bot+sashiko
2 siblings, 1 reply; 8+ messages in thread
From: James Hilliard @ 2026-09-22 0:45 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 9598c386bb21..0ea1860cf203 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
@@ -1277,6 +1291,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] 8+ messages in thread* Re: [PATCH net-next v5 3/3] net: stmmac: sun8i: add support for Allwinner H616 EMAC1
2026-09-22 0:45 ` [PATCH net-next v5 3/3] net: stmmac: sun8i: add support for Allwinner " James Hilliard
@ 2026-09-23 0:47 ` netdev-bot+sashiko
0 siblings, 0 replies; 8+ messages in thread
From: netdev-bot+sashiko @ 2026-09-23 0:47 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 `emac_variant_h616_emac1`
(drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c:153-158) expresses…
--
net: stmmac: sun8i: add support for Allwinner H616 EMAC1
Adds an emac_variant for the H616 secondary EMAC, using a dedicated
system-control clock register at offset 0x34 and declaring RMII-only
support, plus the matching allwinner,sun50i-h616-emac1 compatible entry.
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 9598c386bb216..0ea1860cf2031 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,
> +};
> +
[ ... ]
> @@ -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]
The commit message says the variant "supports only RMII at 10/100 Mbps"
and that the change will "enable only RMII", but does .support_rmii
actually restrict anything at runtime?
Grepping drivers/net/ethernet/stmicro/stmmac/ finds 21 references to
support_mii/support_rmii/support_rgmii, and all of them are the kernel-doc
block, the struct emac_variant member declarations and the per-variant
initializers. Nothing in the driver ever reads these fields.
The only place the interface mode is checked is the variant-independent
switch 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);
So with compatible = "allwinner,sun50i-h616-emac1" and phy-mode = "mii" or
"rgmii" in the device tree, does probe succeed while the EMAC1 clock
register at syscon offset 0x34 gets programmed for a pad mode and TX clock
source this instance does not implement, leaving a silently dead interface
rather than an error?
The driver also installs no phylink get_interfaces/supported_interfaces
callback, so stmmac takes the firmware-selected mode as supported.
In practice the restriction is only enforced by the binding, via the
allOf if compatible contains allwinner,sun50i-h616-emac1 then phy-mode
const: rmii clause in
Documentation/devicetree/bindings/net/allwinner,sun8i-a83t-emac.yaml, so a
device tree that passes dt_binding_check cannot reach the MII/RGMII cases.
The unread flags are an existing convention in this file (emac_variant_v3s
sets only .support_mii yet RMII and RGMII are accepted for it too).
Would it be worth either making sun8i_dwmac_set_syscon() consult
gmac->variant->support_* before writing the register, or rewording the
commit message so it does not claim the driver enables only RMII?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260921-submit-h616-emac1-v1-v5-0-ada661ed9e9b%40gmail.com
^ permalink raw reply [flat|nested] 8+ messages in thread