From: sashiko-bot@kernel.org
To: "Ryan Chen" <ryan_chen@aspeedtech.com>
Cc: devicetree@vger.kernel.org, robh@kernel.org, conor+dt@kernel.org
Subject: Re: [PATCH v4 6/9] EDAC/aspeed: Replace regmap with direct register access
Date: Wed, 30 Sep 2026 05:25:38 +0000 [thread overview]
Message-ID: <20260930052539.6FFE01F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260930-edac-v4-6-c2e526f3ed79@aspeedtech.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] edac: aspeed_probe() mixes goto-based error handling and scoped_guard
--
commit 5981bc9267f2b868500abbb2d7007597303a275a
Author: Ryan Chen <ryan_chen@aspeedtech.com>
EDAC/aspeed: Replace regmap with direct register access
This commit drops the regmap abstraction in favor of direct MMIO access
using readl()/writel() protected by a raw spinlock. It also adds compiler
annotations and uses cleanup helpers like scoped_guard() to manage the
lock.
> diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c
> index 352910e1defc5..26d2c456cc0d6 100644
> --- a/drivers/edac/aspeed_edac.c
> +++ b/drivers/edac/aspeed_edac.c
[ ... ]
> @@ -274,13 +246,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;
[Severity: Low]
Does this mix goto-based error handling and scoped cleanup helpers in the
same function?
The subsystem cleanup guidelines state that the expectation is to never mix
goto and cleanup helpers in the same function. The guidance is to convert
all resources that need a goto cleanup to scope-based cleanup, or convert
none of them.
Looking at aspeed_probe(), it still uses goto labels for error handling
further down:
rc = init_csrows(mci);
if (rc) {
dev_err(&pdev->dev, "failed to init csrows\n");
goto probe_exit02;
}
Should the goto labels in aspeed_probe() also be converted to use cleanup
helpers to avoid mixing paradigms?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260930-edac-v4-0-c2e526f3ed79@aspeedtech.com?part=6
next prev parent reply other threads:[~2026-09-30 5:25 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-30 5:14 [PATCH v4 0/9] Add Aspeed AST2700 SDRAM EDAC support Ryan Chen
2026-09-30 5:14 ` [PATCH v4 1/9] EDAC/aspeed: Set the DIMM grain Ryan Chen
2026-09-30 5:15 ` [PATCH v4 2/9] EDAC/aspeed: Free the interrupt before the mem_ctl_info on remove Ryan Chen
2026-10-04 21:21 ` Borislav Petkov
2026-10-05 3:00 ` Ryan Chen
2026-09-30 5:15 ` [PATCH v4 3/9] EDAC/aspeed: Clean up whitespace and include ordering Ryan Chen
2026-09-30 5:15 ` [PATCH v4 4/9] EDAC/aspeed: Free the mem_ctl_info unconditionally on remove Ryan Chen
2026-09-30 5:15 ` [PATCH v4 5/9] dt-bindings: edac: aspeed: Add AST2700 SDRAM EDAC Ryan Chen
2026-09-30 5:15 ` [PATCH v4 6/9] EDAC/aspeed: Replace regmap with direct register access Ryan Chen
2026-09-30 5:25 ` sashiko-bot [this message]
2026-09-30 5:15 ` [PATCH v4 7/9] EDAC/aspeed: Abstract SoC differences behind chip data Ryan Chen
2026-09-30 5:15 ` [PATCH v4 8/9] EDAC/aspeed: Add AST2700 support Ryan Chen
2026-09-30 5:15 ` [PATCH v4 9/9] MAINTAINERS: Step down as Aspeed AST2500 EDAC driver maintainer Ryan Chen
2026-09-30 5:20 ` sashiko-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260930052539.6FFE01F000FF@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=conor+dt@kernel.org \
--cc=devicetree@vger.kernel.org \
--cc=robh@kernel.org \
--cc=ryan_chen@aspeedtech.com \
--cc=sashiko-reviews@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox