Re: [PATCH AUTOSEL 6.18-5.10] drm/arm/komeda: fix error handling for clk_prepare_enable() and callers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Skipping drm_mode_config_helper_resume() on hardware resume failure leaks the DRM atomic suspend state and breaks future suspends.
- [High] Unbalanced hardware state causes a bus hang on the next system suspend if komeda_dev_resume() fails.

Pre-existing issues:
- [High] Ignored failure of connect_iommu() in komeda_dev_resume() leads to false-success and invalid DMA.
--

commit f5ccdd439f7c420a2895a7d27b82a5ce2f112b7d
Author: Gustavo Kenji Mendonça Kaneko <kaneko.dev@pm.me>

drm/arm/komeda: fix error handling for clk_prepare_enable() and callers

This commit adds error checking to clk_prepare_enable() in
komeda_dev_resume() and propagates the error to callers in the probe
and system PM resume paths.

Link: https://patch.msgid.link/20260609130828.1066038-1-kaneko.dev@pm.me

> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> index 5ba62e637a616..9aad1d1d28ec0 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> @@ -313,7 +313,11 @@ void komeda_dev_destroy(struct komeda_dev *mdev)
>  
>  int komeda_dev_resume(struct komeda_dev *mdev)
>  {
> -	clk_prepare_enable(mdev->aclk);
> +	int err;
> +
> +	err = clk_prepare_enable(mdev->aclk);
> +	if (err)
> +		return err;
>  
>  	mdev->funcs->enable_irq(mdev);

[Severity: High]
This is a pre-existing issue, but does ignoring the failure of connect_iommu()
further down in komeda_dev_resume() lead to a false-success?

If mdev->funcs->connect_iommu(mdev) fails, the function logs an error
but still returns 0. The DRM core would then proceed with display
initialization on an unattached IOMMU, which could result in unintended DMA
behavior and IOMMU fault storms.

[ ... ]
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
> index 358c1512b0879..fc1816c634087 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
[ ... ]
> @@ -139,11 +142,12 @@ static int __maybe_unused komeda_pm_suspend(struct device *dev)
>  static int __maybe_unused komeda_pm_resume(struct device *dev)
>  {
>  	struct komeda_drv *mdrv = dev_get_drvdata(dev);
> +	int err = 0;
>  
>  	if (!pm_runtime_status_suspended(dev))
> -		komeda_dev_resume(mdrv->mdev);
> +		err = komeda_dev_resume(mdrv->mdev);
>  
> -	return drm_mode_config_helper_resume(&mdrv->kms->base);
> +	return err ? err : drm_mode_config_helper_resume(&mdrv->kms->base);

[Severity: High]
If komeda_dev_resume() fails, this early return bypasses
drm_mode_config_helper_resume(), which is required to free and clear
dev->mode_config.suspend_state. Could this leak the DRM atomic suspend state
memory and permanently break subsequent suspends with -EBUSY?

[Severity: High]
Additionally, if komeda_pm_resume() returns an error early, the PM core
still marks the device as active.

On the next system suspend, komeda_pm_suspend() would call
komeda_dev_suspend(), which unconditionally writes to MMIO via disable_irq().

Could this unbalanced hardware state cause a bus hang or crash on the
next suspend cycle due to unclocked MMIO access?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831133314.4125787-34-sashal@kernel.org?part=1