All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Ming Qian" <ming.qian@oss.nxp.com>
Cc: Frank.Li@kernel.org, imx@lists.linux.dev
Subject: Re: [PATCH v3] pmdomain: imx8m-blk-ctrl: Serialize power on/off across sibling domains
Date: Mon, 21 Sep 2026 10:11:06 +0000	[thread overview]
Message-ID: <20260921101106.D8E811F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260921-imx8mp-blk-ctrl-v3-1-c86b0a9d4781@oss.nxp.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] cleanup: mixing goto-based cleanup with guard() in imx8m_blk_ctrl_power_on
--

commit 3b7233b47967e5b29603c44328524d995a2c4def
Author: Ming Qian <ming.qian@oss.nxp.com>

pmdomain: imx8m-blk-ctrl: Serialize power on/off across sibling domains

On i.MX8MP the VPU blk-ctrl exposes G1, G2 and VC8000E as three separate
genpds, each serialized only by its own genpd lock, so their power_on and
power_off callbacks can run concurrently on SMP. Serialize the blk-ctrl
reset sequence with a per-blk-ctrl mutex to prevent sibling transitions
from disturbing shared clock and handshake timing.

> diff --git a/drivers/pmdomain/imx/imx8m-blk-ctrl.c b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> index 479789009c7f4..f8105e87ea3c0 100644
> --- a/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> +++ b/drivers/pmdomain/imx/imx8m-blk-ctrl.c
> @@ -98,6 +105,8 @@ static int imx8m_blk_ctrl_power_on(struct generic_pm_domain *genpd)
>  	struct imx8m_blk_ctrl *bc = domain->bc;
>  	int ret;
>  
> +	guard(mutex)(&bc->power_lock);
> +
>  	/* make sure bus domain is awake */
>  	ret = pm_runtime_get_sync(bc->bus_power_dev);
>  	if (ret < 0) {

[Severity: Low]
This isn't a bug, but does this mix goto-based cleanup with guard() in the
same function?

The subsystem guidelines in include/linux/cleanup.h state that goto and
cleanup helpers should never be mixed in the same function.

imx8m_blk_ctrl_power_on() still has goto labels for manual unwinding:

	/* power up upstream GPC domain */
	ret = pm_runtime_get_sync(domain->power_dev);
	if (ret < 0) {
		dev_err(bc->dev, "failed to power up peripheral domain\n");
		goto clk_disable;
	}

	...

clk_disable:
	clk_bulk_disable_unprepare(data->num_clks, domain->clks);
bus_put:
	pm_runtime_put(bc->bus_power_dev);

	return ret;

Should the existing resources be converted to scope-based cleanup, or should
this lock be managed manually to avoid mixing the two patterns?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-imx8mp-blk-ctrl-v3-1-c86b0a9d4781@oss.nxp.com?part=1

  reply	other threads:[~2026-09-21 10:11 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-21 10:03 [PATCH v3] pmdomain: imx8m-blk-ctrl: Serialize power on/off across sibling domains Ming Qian
2026-09-21 10:11 ` sashiko-bot [this message]
2026-09-21 16:10 ` Frank Li
2026-09-23 12:15 ` Ulf Hansson

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=20260921101106.D8E811F000FF@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=Frank.Li@kernel.org \
    --cc=imx@lists.linux.dev \
    --cc=ming.qian@oss.nxp.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.