On Fri, 4 Mar 2022 at 01:54, Stephen Boyd swboyd@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.