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

[Dmitry Baryshkov]

On Fri, 4 Mar 2022 at 01:54, Stephen Boyd <swboyd at chromium.org> wrote:
>
> 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)?

I didn't like the idea of checking the if (IS_ERR(mdss)) outside of
the case blocks, but now I can move it back.

>
> >                 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.



-- 
With best wishes
Dmitry