[net-next,4/4] net: stmmac: dwmac-sun8i: Add Allwinner H616 EMAC clock index and internal PHY support
Commit Message
Support EMAC1 and the internal Fast Ethernet PHY (AC300) on Allwinner
H616 and H618 SoCs:
- Allow reading a register index from the syscon property to offset the
regmap field for EMAC1's dedicated clock control register.
- Add emac_variant_h616_internal with support for internal PHY, MII,
RMII, and RGMII modes.
- Delay the MAC software reset until sun8i_dwmac_init() when using the
internal PHY, ensuring the PHY has powered on and begun generating
the receive clock before the MAC attempts to reset.
- Skip setting H3_EPHY_SELECT for H616 internal EMAC, which does not
use the H3-style EPHY select bit.
- Handle -EPROBE_DEFER cleanly during clock and reset control acquisition
in get_ephy_nodes().
Assisted-by: LLM
Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
Notes:
Tested on Mellow Fly-C5 (Allwinner H618) under Armbian, verifying stable
EMAC1 probe, deferred soft reset with the internal AC300 EPHY, and 100M full
duplex operation under load.
.../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 65 ++++++++++++++++---
1 file changed, 55 insertions(+), 10 deletions(-)
Comments
Hi,
On 9/16/26 06:41, Alastair D'Silva wrote:
> Support EMAC1 and the internal Fast Ethernet PHY (AC300) on Allwinner
> H616 and H618 SoCs:
> - Allow reading a register index from the syscon property to offset the
> regmap field for EMAC1's dedicated clock control register.
> - Add emac_variant_h616_internal with support for internal PHY, MII,
> RMII, and RGMII modes.
> - Delay the MAC software reset until sun8i_dwmac_init() when using the
> internal PHY, ensuring the PHY has powered on and begun generating
> the receive clock before the MAC attempts to reset.
> - Skip setting H3_EPHY_SELECT for H616 internal EMAC, which does not
> use the H3-style EPHY select bit.
> - Handle -EPROBE_DEFER cleanly during clock and reset control acquisition
> in get_ephy_nodes().
>
> Assisted-by: LLM
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
Even if this is a duplicate some of the work done by James Hilliard, this
raises interesting points.
> Notes:
> Tested on Mellow Fly-C5 (Allwinner H618) under Armbian, verifying stable
> EMAC1 probe, deferred soft reset with the internal AC300 EPHY, and 100M full
> duplex operation under load.
>
> .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 65 ++++++++++++++++---
> 1 file changed, 55 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 48c52eb96233..1728893589a6 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> @@ -130,6 +130,16 @@ static const struct emac_variant emac_variant_a64 = {
> .tx_delay_max = 7,
> };
>
> +static const struct emac_variant emac_variant_h616_internal = {
> + .syscon_field = &sun8i_syscon_reg_field,
> + .soc_has_internal_phy = true,
> + .support_mii = true,
> + .support_rmii = true,
> + .support_rgmii = true,
> + .rx_delay_max = 31,
> + .tx_delay_max = 7,
> +};
James's series [1] says this SoC only supports RMII (yet includes RGMII timing
control), and here for seemingly the same device, there's MII and RGMII
support listed.
[1] : https://lore.kernel.org/r/20260915-submit-h616-emac1-v1-v1-0-195de0bb1f8a@gmail.com
Maybe you can coordinate with one another, can we connect that SoC to a non-internal PHY ?
If so, can we use MII, RMII and RGMII ? or is it just RMII ?
Maxime
On Wed, Sep 16, 2026 at 12:25 AM Maxime Chevallier
<maxime.chevallier@bootlin.com> wrote:
>
> Hi,
>
> On 9/16/26 06:41, Alastair D'Silva wrote:
> > Support EMAC1 and the internal Fast Ethernet PHY (AC300) on Allwinner
> > H616 and H618 SoCs:
> > - Allow reading a register index from the syscon property to offset the
> > regmap field for EMAC1's dedicated clock control register.
> > - Add emac_variant_h616_internal with support for internal PHY, MII,
> > RMII, and RGMII modes.
> > - Delay the MAC software reset until sun8i_dwmac_init() when using the
> > internal PHY, ensuring the PHY has powered on and begun generating
> > the receive clock before the MAC attempts to reset.
> > - Skip setting H3_EPHY_SELECT for H616 internal EMAC, which does not
> > use the H3-style EPHY select bit.
> > - Handle -EPROBE_DEFER cleanly during clock and reset control acquisition
> > in get_ephy_nodes().
> >
> > Assisted-by: LLM
> > Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> > ---
>
> Even if this is a duplicate some of the work done by James Hilliard, this
> raises interesting points.
>
> > Notes:
> > Tested on Mellow Fly-C5 (Allwinner H618) under Armbian, verifying stable
> > EMAC1 probe, deferred soft reset with the internal AC300 EPHY, and 100M full
> > duplex operation under load.
> >
> > .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 65 ++++++++++++++++---
> > 1 file changed, 55 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> > index 48c52eb96233..1728893589a6 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> > @@ -130,6 +130,16 @@ static const struct emac_variant emac_variant_a64 = {
> > .tx_delay_max = 7,
> > };
> >
> > +static const struct emac_variant emac_variant_h616_internal = {
> > + .syscon_field = &sun8i_syscon_reg_field,
> > + .soc_has_internal_phy = true,
> > + .support_mii = true,
> > + .support_rmii = true,
> > + .support_rgmii = true,
> > + .rx_delay_max = 31,
> > + .tx_delay_max = 7,
> > +};
> James's series [1] says this SoC only supports RMII (yet includes RGMII timing
> control), and here for seemingly the same device, there's MII and RGMII
> support listed.
>
> [1] : https://lore.kernel.org/r/20260915-submit-h616-emac1-v1-v1-0-195de0bb1f8a@gmail.com
>
> Maybe you can coordinate with one another, can we connect that SoC to a non-internal PHY ?
The SoC, yes, on emac0 for H616, but not for emac1.
> If so, can we use MII, RMII and RGMII ? or is it just RMII ?
Looks like it's just RMII.
>
> Maxime
On Wed, 2026-09-16 at 08:24 +0200, Maxime Chevallier wrote:
> Hi,
>
> >
<snip>
> > .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 65
> > ++++++++++++++++---
> > 1 file changed, 55 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> > b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> > index 48c52eb96233..1728893589a6 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> > @@ -130,6 +130,16 @@ static const struct emac_variant
> > emac_variant_a64 = {
> > .tx_delay_max = 7,
> > };
> >
> > +static const struct emac_variant emac_variant_h616_internal = {
> > + .syscon_field = &sun8i_syscon_reg_field,
> > + .soc_has_internal_phy = true,
> > + .support_mii = true,
> > + .support_rmii = true,
> > + .support_rgmii = true,
> > + .rx_delay_max = 31,
> > + .tx_delay_max = 7,
> > +};
> James's series [1] says this SoC only supports RMII (yet includes
> RGMII timing
> control), and here for seemingly the same device, there's MII and
> RGMII
> support listed.
>
> [1] : https://lore.kernel.org/r/20260915-submit-h616-emac1-v1-v1-0-
> 195de0bb1f8a@gmail.com
>
> Maybe you can coordinate with one another, can we connect that SoC to
> a non-internal PHY ?
> If so, can we use MII, RMII and RGMII ? or is it just RMII ?
>
> Maxime
I just double checked, MII & RGMII are bogus.
Only EMAC0 can be connected to an external PHY.
Hi,
On 9/16/26 08:24, Maxime Chevallier wrote:
> Hi,
>
> On 9/16/26 06:41, Alastair D'Silva wrote:
>> Support EMAC1 and the internal Fast Ethernet PHY (AC300) on Allwinner
>> H616 and H618 SoCs:
>> - Allow reading a register index from the syscon property to offset the
>> regmap field for EMAC1's dedicated clock control register.
>> - Add emac_variant_h616_internal with support for internal PHY, MII,
>> RMII, and RGMII modes.
>> - Delay the MAC software reset until sun8i_dwmac_init() when using the
>> internal PHY, ensuring the PHY has powered on and begun generating
>> the receive clock before the MAC attempts to reset.
>> - Skip setting H3_EPHY_SELECT for H616 internal EMAC, which does not
>> use the H3-style EPHY select bit.
>> - Handle -EPROBE_DEFER cleanly during clock and reset control acquisition
>> in get_ephy_nodes().
>>
>> Assisted-by: LLM
>> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
>> ---
>
> Even if this is a duplicate some of the work done by James Hilliard, this
> raises interesting points.
I think the point is that this patch is just wrong, in many parts ;-)
I think much of the code stems from the original Allwinner BSP code,
which is just, say, misinformed in many respects. The use of compatible
strings, and shoehorning the PHY clock into the MAC code are just
examples. More below.
>> Notes:
>> Tested on Mellow Fly-C5 (Allwinner H618) under Armbian, verifying stable
>> EMAC1 probe, deferred soft reset with the internal AC300 EPHY, and 100M full
>> duplex operation under load.
>>
>> .../net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 65 ++++++++++++++++---
>> 1 file changed, 55 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> index 48c52eb96233..1728893589a6 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
>> @@ -130,6 +130,16 @@ static const struct emac_variant emac_variant_a64 = {
>> .tx_delay_max = 7,
>> };
>>
>> +static const struct emac_variant emac_variant_h616_internal = {
>> + .syscon_field = &sun8i_syscon_reg_field,
>> + .soc_has_internal_phy = true,
For instance this cannot be true. internal_phy refers to the H3
generation, which works quite differently. We had this issue already
with the H6, and there is a comment there mentioning this.
And this becomes obvious in other parts of the code, where this patch
reverts the has_internal_phy tests, because they don't apply here.
>> + .support_mii = true,
>> + .support_rmii = true,
>> + .support_rgmii = true,
>> + .rx_delay_max = 31,
>> + .tx_delay_max = 7,
>> +};
> James's series [1] says this SoC only supports RMII (yet includes RGMII timing
> control), and here for seemingly the same device, there's MII and RGMII
> support listed.
>
> [1] : https://lore.kernel.org/r/20260915-submit-h616-emac1-v1-v1-0-195de0bb1f8a@gmail.com
>
> Maybe you can coordinate with one another, can we connect that SoC to a non-internal PHY ?
> If so, can we use MII, RMII and RGMII ? or is it just RMII ?
This MAC is *only* connected to the AC200/AC300 PHY, which is a separate
die, but co-packaged into the SoC. So that's set in stone. And while the
MAC IP might support more MII modes (because it's maybe the same IP as
the primary MAC? We don't know), it is irrelevant here, since only RMII
is usable.
Cheers,
Andre
>
> Maxime
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] net: stmmac: MAC soft reset is completely bypassed for internal PHY
- [Medium] net: stmmac: silent acceptance of missing clocks and resets for internal PHY
- [Medium] net: stmmac: internal PHY clock and reset leaked if MAC reset fails
--
commit a3ca47044e22ce48b791f8eb8f51a44d49d7ba43
Author: Alastair D'Silva <alastair@d-silva.org>
net: stmmac: dwmac-sun8i: Add Allwinner H616 EMAC clock index and internal PHY support
This commit adds support for EMAC1 and the internal Fast Ethernet PHY (AC300)
on Allwinner H616 and H618 SoCs. It allows reading a register index from the
syscon property and introduces emac_variant_h616_internal with internal PHY
support.
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> index 48c52eb962335..1728893589a68 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
[ ... ]
> @@ -589,6 +600,10 @@ static int sun8i_dwmac_init(struct device *dev, void *priv)
> ret = sun8i_dwmac_power_internal_phy(netdev_priv(ndev));
> if (ret)
> goto err_disable_regulator;
> +
> + ret = sun8i_dwmac_reset(netdev_priv(ndev));
> + if (ret)
> + goto err_disable_regulator;
[Severity: Medium]
If sun8i_dwmac_reset() returns an error here, the code jumps directly to
err_disable_regulator. Will this skip the call to
sun8i_dwmac_unpower_internal_phy(), leaving the internal PHY powered and its
clock enabled?
[ ... ]
> @@ -784,16 +799,23 @@ static int get_ephy_nodes(struct stmmac_priv *priv)
> /* Seek for internal PHY */
> for_each_child_of_node_scoped(mdio_internal, iphynode) {
> gmac->ephy_clk = of_clk_get(iphynode, 0);
> - if (IS_ERR(gmac->ephy_clk))
> - continue;
> + if (IS_ERR(gmac->ephy_clk)) {
> + ret = PTR_ERR(gmac->ephy_clk);
> + if (ret == -EPROBE_DEFER) {
> + of_node_put(mdio_internal);
> + return ret;
> + }
> + gmac->ephy_clk = NULL;
> + }
[Severity: Medium]
If of_clk_get() returns an error other than -EPROBE_DEFER (such as -EINVAL),
the error handling now sets gmac->ephy_clk to NULL and continues to the reset
control fetch, which eventually returns 0 on success. Does this silently
accept a device tree node missing the required clock instead of skipping it
via continue?
> gmac->rst_ephy = of_reset_control_get_exclusive(iphynode, NULL);
> if (IS_ERR(gmac->rst_ephy)) {
> ret = PTR_ERR(gmac->rst_ephy);
> if (ret == -EPROBE_DEFER) {
> + clk_put(gmac->ephy_clk);
> of_node_put(mdio_internal);
> return ret;
> }
> - continue;
> + gmac->rst_ephy = NULL;
> }
[Severity: Medium]
Similarly, if of_reset_control_get_exclusive() returns a non-EPROBE_DEFER
error, the pointer is set to NULL and the loop returns 0 (success). Will this
cause the driver to silently accept missing resets and later perform
operations with NULL pointers?
[ ... ]
> @@ -892,10 +916,13 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
> } else {
> sun8i_dwmac_unpower_internal_phy(gmac);
> }
> - /* After changing syscon value, the MAC need reset or it will
> + /* After changing syscon value, the MAC needs reset or it will
> * use the last value (and so the last PHY set).
> + * For internal PHY, the MAC reset will timeout because the PHY
> + * is not yet enabled/clocked. Delay the reset to dwmac_init.
> */
> - ret = sun8i_dwmac_reset(priv);
> + if (!gmac->use_internal_phy)
> + ret = sun8i_dwmac_reset(priv);
> }
> return ret;
[Severity: High]
The comment states the MAC reset is delayed to dwmac_init. However, during
driver probe in sun8i_dwmac_probe(), plat_dat->init (which points to
sun8i_dwmac_init) executes before the MDIO mux is registered.
During that initial execution of sun8i_dwmac_init(), gmac->use_internal_phy
is false (zero-initialized), so the reset block is skipped. Later, this
MDIO switch function sets gmac->use_internal_phy = true but explicitly skips
the MAC reset here.
Because both execution paths skip the reset, does this mean the MAC soft
reset is completely bypassed during initialization when using the internal
PHY?
@@ -130,6 +130,16 @@ static const struct emac_variant emac_variant_a64 = {
.tx_delay_max = 7,
};
+static const struct emac_variant emac_variant_h616_internal = {
+ .syscon_field = &sun8i_syscon_reg_field,
+ .soc_has_internal_phy = true,
+ .support_mii = true,
+ .support_rmii = true,
+ .support_rgmii = true,
+ .rx_delay_max = 31,
+ .tx_delay_max = 7,
+};
+
static const struct emac_variant emac_variant_h6 = {
.syscon_field = &sun8i_syscon_reg_field,
/* The "Internal PHY" of H6 is not on the die. It's on the
@@ -570,6 +580,7 @@ static const struct stmmac_dma_ops sun8i_dwmac_dma_ops = {
};
static int sun8i_dwmac_power_internal_phy(struct stmmac_priv *priv);
+static int sun8i_dwmac_reset(struct stmmac_priv *priv);
static int sun8i_dwmac_init(struct device *dev, void *priv)
{
@@ -589,6 +600,10 @@ static int sun8i_dwmac_init(struct device *dev, void *priv)
ret = sun8i_dwmac_power_internal_phy(netdev_priv(ndev));
if (ret)
goto err_disable_regulator;
+
+ ret = sun8i_dwmac_reset(netdev_priv(ndev));
+ if (ret)
+ goto err_disable_regulator;
}
return 0;
@@ -784,16 +799,23 @@ static int get_ephy_nodes(struct stmmac_priv *priv)
/* Seek for internal PHY */
for_each_child_of_node_scoped(mdio_internal, iphynode) {
gmac->ephy_clk = of_clk_get(iphynode, 0);
- if (IS_ERR(gmac->ephy_clk))
- continue;
+ if (IS_ERR(gmac->ephy_clk)) {
+ ret = PTR_ERR(gmac->ephy_clk);
+ if (ret == -EPROBE_DEFER) {
+ of_node_put(mdio_internal);
+ return ret;
+ }
+ gmac->ephy_clk = NULL;
+ }
gmac->rst_ephy = of_reset_control_get_exclusive(iphynode, NULL);
if (IS_ERR(gmac->rst_ephy)) {
ret = PTR_ERR(gmac->rst_ephy);
if (ret == -EPROBE_DEFER) {
+ clk_put(gmac->ephy_clk);
of_node_put(mdio_internal);
return ret;
}
- continue;
+ gmac->rst_ephy = NULL;
}
dev_info(priv->device, "Found internal PHY node\n");
of_node_put(mdio_internal);
@@ -871,7 +893,9 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
switch (desired_child) {
case DWMAC_SUN8I_MDIO_MUX_INTERNAL_ID:
dev_info(priv->device, "Switch mux to internal PHY");
- val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SELECT;
+ val = (reg & ~H3_EPHY_MUX_MASK);
+ if (gmac->variant != &emac_variant_h616_internal)
+ val |= H3_EPHY_SELECT;
gmac->use_internal_phy = true;
break;
case DWMAC_SUN8I_MDIO_MUX_EXTERNAL_ID:
@@ -892,10 +916,13 @@ static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
} else {
sun8i_dwmac_unpower_internal_phy(gmac);
}
- /* After changing syscon value, the MAC need reset or it will
+ /* After changing syscon value, the MAC needs reset or it will
* use the last value (and so the last PHY set).
+ * For internal PHY, the MAC reset will timeout because the PHY
+ * is not yet enabled/clocked. Delay the reset to dwmac_init.
*/
- ret = sun8i_dwmac_reset(priv);
+ if (!gmac->use_internal_phy)
+ ret = sun8i_dwmac_reset(priv);
}
return ret;
}
@@ -1000,9 +1027,13 @@ static int sun8i_dwmac_set_syscon(struct device *dev,
static void sun8i_dwmac_unset_syscon(struct sunxi_priv_data *gmac)
{
- if (gmac->variant->soc_has_internal_phy)
- regmap_field_write(gmac->regmap_field,
- (H3_EPHY_SHUTDOWN | H3_EPHY_SELECT));
+ if (gmac->variant->soc_has_internal_phy) {
+ u32 val = H3_EPHY_SHUTDOWN;
+
+ if (gmac->variant != &emac_variant_h616_internal)
+ val |= H3_EPHY_SELECT;
+ regmap_field_write(gmac->regmap_field, val);
+ }
}
static void sun8i_dwmac_exit(struct device *dev, void *priv)
@@ -1111,6 +1142,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
struct stmmac_priv *priv;
struct net_device *ndev;
struct regmap *regmap;
+ u32 syscon_idx = 0;
int ret;
ret = stmmac_get_platform_resources(pdev, &stmmac_res);
@@ -1163,8 +1195,15 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
return ret;
}
+ ret = of_property_read_u32_index(pdev->dev.of_node, "syscon", 1,
+ &syscon_idx);
gmac->regmap_field = devm_regmap_field_alloc(dev, regmap,
- *gmac->variant->syscon_field);
+ (const struct reg_field) {
+ .reg = gmac->variant->syscon_field->reg +
+ syscon_idx * sizeof(u32),
+ .lsb = gmac->variant->syscon_field->lsb,
+ .msb = gmac->variant->syscon_field->msb,
+ });
if (IS_ERR(gmac->regmap_field)) {
ret = PTR_ERR(gmac->regmap_field);
dev_err(dev, "Unable to map syscon register: %d\n", ret);
@@ -1278,6 +1317,12 @@ 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-emac",
+ .data = &emac_variant_h6 },
+ { .compatible = "allwinner,sun50i-h616-emac1",
+ .data = &emac_variant_h6 },
+ { .compatible = "allwinner,sun50i-h616-internal-emac",
+ .data = &emac_variant_h616_internal },
{ }
};
MODULE_DEVICE_TABLE(of, sun8i_dwmac_match);