Re: [PATCH v2 2/4] drm/msm: remove extra indirection for msm_mdss

[Stephen Boyd <swboyd@chromium.org>]

Quoting Dmitry Baryshkov (2022-01-19 14:40:03)
> diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
> index be06a62d7ccb..f18dfbb614f0 100644
> --- a/drivers/gpu/drm/msm/msm_drv.c
> +++ b/drivers/gpu/drm/msm/msm_drv.c
> @@ -1211,19 +1212,32 @@ static int msm_pdev_probe(struct platform_device *pdev)
>
>         switch (get_mdp_ver(pdev)) {
>         case KMS_MDP5:
> -               ret = msm_mdss_init(pdev, true);
> +               mdss = msm_mdss_init(pdev, true);
> +               if (IS_ERR(mdss)) {
> +                       ret = PTR_ERR(mdss);
> +                       platform_set_drvdata(pdev, NULL);
> +
> +                       return ret;
> +               } else {

Drop else

> +                       priv->mdss = mdss;
> +                       pm_runtime_enable(&pdev->dev);
> +               }
>                 break;
>         case KMS_DPU:
> -               ret = msm_mdss_init(pdev, false);
> +               mdss = msm_mdss_init(pdev, false);
> +               if (IS_ERR(mdss)) {
> +                       ret = PTR_ERR(mdss);
> +                       platform_set_drvdata(pdev, NULL);
> +
> +                       return ret;
> +               } else {
> +                       priv->mdss = mdss;
> +                       pm_runtime_enable(&pdev->dev);
> +               }

This is the same so why can't it be done below in the deleted if (ret)?

>                 break;
>         default:
> -               ret = 0;
>                 break;
>         }
> -       if (ret) {
> -               platform_set_drvdata(pdev, NULL);
> -               return ret;
> -       }
>
>         if (get_mdp_ver(pdev)) {
>                 ret = add_display_components(pdev, &match);
> diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
> index 2459ba479caf..0c341660941a 100644
> --- a/drivers/gpu/drm/msm/msm_kms.h
> +++ b/drivers/gpu/drm/msm/msm_kms.h
> @@ -239,50 +228,44 @@ int mdp5_mdss_parse_clock(struct platform_device *pdev, struct clk_bulk_data **c
>         return num_clocks;
>  }
>
> -int msm_mdss_init(struct platform_device *pdev, bool mdp5)
> +struct msm_mdss *msm_mdss_init(struct platform_device *pdev, bool mdp5)

Ah I see it will quickly become not static. Still should have static
first and remove it here.