Re: [PATCH v3 13/18] drm/mcde: Use power domain for display power

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] pm: incorrect success check for pm_runtime_put_sync_suspend()
--

commit 1421b098d2f7f0ffecc255355537663b64ad9955
Author: Linus Walleij <linusw@kernel.org>

    drm/mcde: Use power domain for display power

This commit replaces explicit EPOD regulator handling with runtime PM.
It uses the MCDE power domain and drops the regulator dependency.

> diff --git a/drivers/gpu/drm/mcde/mcde_display.c b/drivers/gpu/drm/mcde/mcde_display.c
> index 257a6e84dd583..52f071bb347c4 100644
> --- a/drivers/gpu/drm/mcde/mcde_display.c
> +++ b/drivers/gpu/drm/mcde/mcde_display.c

[ ... ]

> @@ -1327,9 +1326,9 @@ static void mcde_display_disable(struct drm_simple_display_pipe *pipe)
>  		spin_unlock_irq(&crtc->dev->event_lock);
>  	}
>  
> -	ret = regulator_disable(mcde->epod);
> +	ret = pm_runtime_put_sync_suspend(mcde->dev);
>  	if (ret)
> -		dev_err(drm->dev, "can't disable EPOD regulator\n");
> +		dev_err(drm->dev, "can't disable MCDE power domain\n");

[Severity: Low]
Does this incorrectly treat a successful return value as an error?

When mcde_display_disable() calls pm_runtime_put_sync_suspend(), the PM core
returns 1 if the device is already suspended. Because this code checks
if (ret) rather than if (ret < 0), the success value of 1 will trigger the
error path.

This will cause a spurious "can't disable MCDE power domain" message to be
printed to the kernel log even when the operation succeeds.

>  	/* Make sure we are powered down (before we may power up again) */
>  	usleep_range(50000, 70000);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260921-ux500-power-domains-v7-1-v3-0-9352be6b86b1@kernel.org?part=13