Re: [PATCH net-next v3 2/5] net: stmmac: sun55i: Add per-compatible match data

[Jerome Brunet <jbrunet@baylibre.com>]

On jeu. 24 sept. 2026 at 20:48, sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] net: stmmac: sun55i: sun55i_gmac pointer is lost after probe
> --
>
> commit 96a080b2a3499368db013319e188847cee61183d
> Author: Jerome Brunet <jbrunet@baylibre.com>
>
> net: stmmac: sun55i: Add per-compatible match data
>
> The A733 GMAC210 glue registers are in a dedicated MMIO region of the
> controller rather than in a syscon. This commit adds per-compatible match
> data holding the glue resource init callback, the stmmac flags, and the
> glue register offset.
>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun55i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun55i.c
>> index 862df173d963f..c4df532850074 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun55i.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun55i.c
>
> [ ... ]
>
>> @@ -107,22 +128,34 @@ static int sun55i_gmac200_probe(struct platform_device *pdev)
>>  	struct plat_stmmacenet_data *plat_dat;
>>  	struct stmmac_resources stmmac_res;
>>  	struct device *dev = &pdev->dev;
>> +	struct sun55i_gmac *gmac;
>>  	struct clk *clk;
>>  	int ret;
>>  
>> +	gmac = devm_kzalloc(dev, sizeof(*gmac), GFP_KERNEL);
>> +	if (!gmac)
>> +		return -ENOMEM;
>> +
>
> [ ... ]
>
>> @@ -138,8 +171,15 @@ static int sun55i_gmac200_probe(struct platform_device *pdev)
>>  	if (ret)
>>  		return dev_err_probe(dev, ret, "Failed to get or enable PHY supply\n");
>>  
>>  	return devm_stmmac_pltfr_probe(pdev, plat_dat, &stmmac_res);
>>  }
>
> [Severity: Low]
> Does this code intentionally discard the gmac pointer after probe?
>
> The sun55i_gmac structure is dynamically allocated via devm_kzalloc(), but
> it does not appear to be saved (for example, to plat_dat->bsp_priv) before
> sun55i_gmac200_probe() returns.
>
> If the structure is only intended for use during initialization, could it be
> allocated on the stack to prevent an unnecessary heap allocation that
> persists for the lifetime of the device? Alternatively, if it is meant to
> hold state (like the regmap) for future callbacks, does the pointer need to
> be saved here?

Yeah, At one point I was thinking of setting stmmac bsp_priv to prepare
for a possible suspend/resume support later on. Eventually it will be necessary.

I could do the stack option or keep it that way and set bsp_priv.
I don't mind one way or they other. If someone as a preference, please
let me know.

>
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260923-allwinner-a733-gmac-support-v3-0-15735155a789@baylibre.com?part=2

-- 
Jerome