Re: [PATCH RFC/RFT 2/2] net: stmmac: Add support for Allwinner A733 GMAC210

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing AHB reset deassertion before MMIO access causes bus hang.
- [High] Missing suspend/resume callback permanently breaks network after sleep.
--

--- Patch [2]: [PATCH RFC/RFT 2/2] net: stmmac: Add support for Allwinner A733 GMAC210 ---
commit 00717a07b743c8a2cc8bc07f479795f3772f17e1
Author: Jerome Brunet <jbrunet@baylibre.com>

net: stmmac: Add support for Allwinner A733 GMAC210

This commit adds support for the Allwinner A733 GMAC210 controller. It
handles a dedicated MMIO region for glue configuration, specific TX clock
delay fields, and per-DMA-channel interrupts.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun55i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun55i.c
> index 862df173d963f..3d434a52fb02d 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun55i.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun55i.c

[ ... ]

> +static struct regmap *sun60i_gmac210_get_regmap(struct platform_device *pdev,
> +						struct plat_stmmacenet_data *plat)
> +{

[ ... ]

> +	if (!plat->stmmac_rst || !plat->stmmac_ahb_rst) {
> +		dev_err(dev, "missing required reset controls\n");
> +		return ERR_PTR(-EINVAL);
> +	}
> +
> +	/*
> +	 * The configuration registers are inside the controller
> +	 * reset domain, so the reset must happen before any write to them
> +	 * and should not be done again by stmmac or the configuration will
> +	 * be lost.
> +	 */
> +	ret = reset_control_assert(plat->stmmac_rst);
> +	if (!ret)
> +		ret = reset_control_deassert(plat->stmmac_rst);

[Severity: High]
Will this cause a bus hang when writing to the MMIO registers?

The code checks that plat->stmmac_ahb_rst is present earlier in this
function, but only toggles the main mac reset here. 

When sun55i_gmac200_setup() later calls regmap_write(), accessing an IP
block over the AHB bus while its AHB interface is still held in reset
typically causes a synchronous external abort or bus hang.

> +
> +	if (ret) {
> +		dev_err_probe(dev, ret, "device reset failed\n");

[ ... ]

>  static int sun55i_gmac200_setup(struct platform_device *pdev,
>  				struct plat_stmmacenet_data *plat,
>  				const struct sun55i_gmac_data *data)
>  {

[ ... ]

> -	ret = regmap_write(regmap, SYSCON_REG, reg);
> +	ret = regmap_write(regmap, data->offset, reg);
>  	if (ret < 0)
>  		return dev_err_probe(dev, ret, "Failed to write to syscon\n");
>  
> +	plat->flags |= data->flags;
> +	plat->host_dma_width = 32;
> +
>  	return 0;
>  }

[Severity: High]
Are these custom hardware settings lost after a system suspend?

The driver configures critical hardware properties like TX/RX delays and
PHY interface modes in the MAC's internal glue configuration registers
here, but does not appear to assign a plat->resume callback to the
plat_stmmacenet_data structure.

If the MAC loses power during suspend, won't these registers revert to
their default states on resume, leaving the network broken because the
generic stmmac_resume() has no way to invoke this glue logic?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260731-allwinner-a733-gmac-support-v1-0-d0a8732bfa82@baylibre.com?part=2