Re: [PATCH] pmdomain: imx8m-blk-ctrl: Serialize power on/off across sibling domains

[sashiko-bot@kernel.org]

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

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

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

This commit adds a per-blk-ctrl mutex to serialize the blk-ctrl register
and reset sequence across sibling domains in the i.MX8MP VPU. This fixes
a defect where concurrent power transitions disturb shared clock timings,
leaving a VPU stuck in reset.

> 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);
> +

[Severity: Low]
Does this mix the guard macro with legacy goto-based error handling in
imx8m_blk_ctrl_power_on()?

Later in this function, there are goto statements for error handling:

    ret = clk_bulk_prepare_enable(data->num_clks, domain->clks);
    if (ret) {
        ...
        goto bus_put;
    }

According to the cleanup subsystem guidelines (include/linux/cleanup.h),
using goto and scope-based cleanup helpers shouldn't be mixed in the same
function. 

While this specific instance doesn't introduce a functional runtime bug right
now (since there are no other scope-based variables causing LIFO ordering
conflicts), it violates the subsystem policy and might lead to confusing
ownership semantics or regressions during future refactoring.

Could this function be fully converted to scope-based cleanup, or should it
stick entirely to standard lock/unlock calls with goto-based cleanup?

>  	/* make sure bus domain is awake */
>  	ret = pm_runtime_get_sync(bc->bus_power_dev);
>  	if (ret < 0) {

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