Re: [PATCH -next v2 3/3] drm/rockchip: Simplified with dev_err_probe()

Heiko Stuebner
2024-09-19T15:25:39Z

Am Dienstag, 27. August 2024, 05:03:57 CEST schrieb Jinjie Ruan:
> Use dev_err_probe to replace deprecated() DRM_DEV_ERROR(), which
> can simplfy code.
>
> Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com>
> ---
> drivers/gpu/drm/rockchip/rockchip_lvds.c | 30 +++++++++---------------
> 1 file changed, 11 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> index 700ac730887d..92679e54c71d 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c

> @@ -467,24 +465,22 @@ static int rk3288_lvds_probe(struct platform_device *pdev,
>
> lvds->pins->p = devm_pinctrl_get(lvds->dev);
> if (IS_ERR(lvds->pins->p)) {
> - DRM_DEV_ERROR(lvds->dev, "no pinctrl handle\n");
> + dev_err(lvds->dev, "no pinctrl handle\n");

you're mixing things here.

The commit message talks about dev_err_probe but you're doing
here and below a DRM_DEV_ERROR to dev_err conversion, which also
partially happens in patch 2 of this series.

Please structure things in a way that the patch content matches
the patch description :-)

Thanks a lot
Heiko

> devm_kfree(lvds->dev, lvds->pins);
> lvds->pins = NULL;
> } else {
> lvds->pins->default_state =
> pinctrl_lookup_state(lvds->pins->p, "lcdc");
> if (IS_ERR(lvds->pins->default_state)) {
> - DRM_DEV_ERROR(lvds->dev, "no default pinctrl state\n");
> + dev_err(lvds->dev, "no default pinctrl state\n");
> devm_kfree(lvds->dev, lvds->pins);
> lvds->pins = NULL;
> }
> }
>
> ret = clk_prepare(lvds->pclk);
> - if (ret < 0) {
> - DRM_DEV_ERROR(lvds->dev, "failed to prepare pclk_lvds\n");
> - return ret;
> - }
> + if (ret < 0)
> + return dev_err_probe(lvds->dev, ret, "failed to prepare pclk_lvds\n");
>
> return 0;
> }
> @@ -700,22 +696,18 @@ static int rockchip_lvds_probe(struct platform_device *pdev)
>
> lvds->grf = syscon_regmap_lookup_by_phandle(dev->of_node,
> "rockchip,grf");
> - if (IS_ERR(lvds->grf)) {
> - DRM_DEV_ERROR(dev, "missing rockchip,grf property\n");
> - return PTR_ERR(lvds->grf);
> - }
> + if (IS_ERR(lvds->grf))
> + return dev_err_probe(dev, PTR_ERR(lvds->grf), "missing rockchip,grf property\n");
>
> ret = lvds->soc_data->probe(pdev, lvds);
> - if (ret) {
> - DRM_DEV_ERROR(dev, "Platform initialization failed\n");
> - return ret;
> - }
> + if (ret)
> + return dev_err_probe(dev, ret, "Platform initialization failed\n");
>
> dev_set_drvdata(dev, lvds);
>
> ret = component_add(&pdev->dev, &rockchip_lvds_component_ops);
> if (ret < 0) {
> - DRM_DEV_ERROR(dev, "failed to add component\n");
> + dev_err(dev, "failed to add component\n");
> clk_unprepare(lvds->pclk);
> }
>
>