Re: [PATCH v5 6/9] EDAC/aspeed: Replace regmap with direct register access

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] cleanup: mixing goto-based error handling with cleanup helpers
--

commit deee611aba8be1433a92290de22d7d73ee5ad14d
Author: Ryan Chen <ryan_chen@aspeedtech.com>

EDAC/aspeed: Replace regmap with direct register access

This commit replaces the driver's regmap implementation with direct readl()
and writel() accesses protected by an explicit raw spinlock. It also adds
context analysis annotations to ensure hardware register accesses are 
properly locked.

> diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
> index 610da8aa2a228..124e8a39eaaba 100644
> --- a/drivers/edac/aspeed_edac.c
> +++ b/drivers/edac/aspeed_edac.c

[ ... ]

> @@ -276,13 +248,11 @@ static int aspeed_probe(struct platform_device *pdev)
>  	if (IS_ERR(regs))
>  		return PTR_ERR(regs);
>  
> -	aspeed_regmap = devm_regmap_init(dev, NULL, (__force void *)regs,
> -					 &aspeed_regmap_config);
> -	if (IS_ERR(aspeed_regmap))
> -		return PTR_ERR(aspeed_regmap);
> +	scoped_guard(raw_spinlock_irqsave, &aspeed_lock)
> +		aspeed_regs = regs;
>  
>  	/* bail out if ECC mode is not configured */
> -	regmap_read(aspeed_regmap, ASPEED_MCR_CONF, &reg04);
> +	reg04 = readl(regs + ASPEED_MCR_CONF);

[ ... ]

>  	rc = init_csrows(mci);
>  	if (rc) {
>  		dev_err(&pdev->dev, "failed to init csrows\n");
>  		goto probe_exit02;

[Severity: Medium]
The aspeed_probe() function introduces a scope-based cleanup helper earlier
in the function with scoped_guard(), but continues to use legacy goto-based
error handling here and in other error paths.

The kernel cleanup subsystem guidelines specify that usage of goto and
cleanup helpers should not be mixed in the same function to prevent confusing
ownership semantics and LIFO ordering bugs. 

Would it be possible to convert all resources in this routine to scope-based
cleanups (for example, using __free() for the EDAC MC data structure), or
alternatively, rely entirely on the existing goto-based error paths and skip
using scoped_guard() in this specific function?

>  	}
>  
>  	platform_set_drvdata(pdev, mci);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261007-edac-v5-0-5c31fc5df4cf@aspeedtech.com?part=6