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 <[email protected]>

Reviewed-by: Liviu Dudau <[email protected]>

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!  /
  ---------------
    ¯\_(ツ)_/¯

Reply via email to