Re: [PATCH v2] drm/arm/komeda: fix error handling for clk_prepare_enable() and callers

[Liviu Dudau]

On Tue, Jun 09, 2026 at 01:08:33PM +0000, Gustavo Kenji Mendonça Kaneko wrote:
> komeda_dev_resume() calls clk_prepare_enable() without checking the
> return value. If the clock fails to enable, the function returns 0
> (success) while IRQs are enabled and IOMMU is connected on potentially
> unclocked hardware, causing undefined behavior on resume.
> 
> Propagate the error from clk_prepare_enable() and fix all call sites
> in komeda_drv.c that previously ignored the return value of
> komeda_dev_resume():
> 
> - komeda_platform_probe(): if resume fails, jump to err_destroy_mdev
>   (skipping the suspend call, since the clock was never enabled)
> - komeda_pm_resume(): propagate the error and skip
>   drm_mode_config_helper_resume() on failure
> 
> This issue was found by code review without access to Komeda hardware.
> 
> Signed-off-by: Gustavo Kenji Mendonça Kaneko <kaneko.dev@xxxxx>

Reviewed-by: Liviu Dudau <liviu.dudau@xxxxxxx>

Best regards,
Liviu

> ---
> Changes in v2:
>   - Fix all call sites in komeda_drv.c that ignored the return value of
>     komeda_dev_resume() (Liviu Dudau)
>   - In komeda_platform_probe(), use a new err_destroy_mdev label to skip
>     the komeda_dev_suspend() call when resume itself failed
>   - In komeda_pm_resume(), propagate the error from komeda_dev_resume()
>     and skip drm_mode_config_helper_resume() on failure
> 
>  drivers/gpu/drm/arm/display/komeda/komeda_dev.c |  6 +++++-
>  drivers/gpu/drm/arm/display/komeda/komeda_drv.c | 14 +++++++++-----
>  2 files changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_dev.c b/drivers/gpu/drm/arm/display/komeda/komeda_dev.c
> index 5ba62e637a61..9aad1d1d28ec 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);
>  
> diff --git a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
> index 4bb5f250e95e..67fffab018ae 100644
> --- a/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
> +++ b/drivers/gpu/drm/arm/display/komeda/komeda_drv.c
> @@ -74,8 +74,11 @@ static int komeda_platform_probe(struct platform_device *pdev)
>  	}
>  
>  	pm_runtime_enable(dev);
> -	if (!pm_runtime_enabled(dev))
> -		komeda_dev_resume(mdrv->mdev);
> +	if (!pm_runtime_enabled(dev)) {
> +		err = komeda_dev_resume(mdrv->mdev);
> +		if (err)
> +			goto err_destroy_mdev;
> +	}
>  
>  	mdrv->kms = komeda_kms_attach(mdrv->mdev);
>  	if (IS_ERR(mdrv->kms)) {
> @@ -93,7 +96,7 @@ static int komeda_platform_probe(struct platform_device *pdev)
>  		pm_runtime_disable(dev);
>  	else
>  		komeda_dev_suspend(mdrv->mdev);
> -
> +err_destroy_mdev:
>  	komeda_dev_destroy(mdrv->mdev);
>  
>  free_mdrv:
> @@ -140,11 +143,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);
>  }
>  
>  static const struct dev_pm_ops komeda_pm_ops = {
> -- 
> 2.54.0
> 
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯