[PATCH v3 2/3] drm/msm/dpu: simplify clocks handling

[Stephen Boyd]

Quoting Dmitry Baryshkov (2022-01-19 14:16:15)
> diff --git a/drivers/gpu/drm/msm/msm_io_utils.c b/drivers/gpu/drm/msm/msm_io_utils.c
> index 7b504617833a..5533c87c7158 100644
> --- a/drivers/gpu/drm/msm/msm_io_utils.c
> +++ b/drivers/gpu/drm/msm/msm_io_utils.c
> @@ -5,6 +5,8 @@
>   * Author: Rob Clark <robdclark at gmail.com>
>   */
>
> +#include <linux/clk/clk-conf.h>
> +
>  #include "msm_drv.h"
>
>  /*
> @@ -47,6 +49,54 @@ struct clk *msm_clk_get(struct platform_device *pdev, const char *name)
>         return clk;
>  }
>
> +int msm_parse_clock(struct platform_device *pdev, struct clk_bulk_data **clocks)
> +{
> +       u32 i, rc = 0;
> +       const char *clock_name;
> +       struct clk_bulk_data *bulk;
> +       int num_clk = 0;

No need to assign and then reassign before testing. Same goes for 'rc'.

> +
> +       if (!pdev)
> +               return -EINVAL;
> +
> +       num_clk = of_property_count_strings(pdev->dev.of_node, "clock-names");
> +       if (num_clk <= 0) {
> +               pr_debug("clocks are not defined\n");
> +               return 0;
> +       }
> +
> +       bulk = devm_kcalloc(&pdev->dev, num_clk, sizeof(struct clk_bulk_data), GFP_KERNEL);
> +       if (!bulk)
> +               return -ENOMEM;
> +
> +       for (i = 0; i < num_clk; i++) {
> +               rc = of_property_read_string_index(pdev->dev.of_node,
> +                                                  "clock-names", i,
> +                                                  &clock_name);
> +               if (rc) {
> +                       DRM_DEV_ERROR(&pdev->dev, "Failed to get clock name for %d\n", i);
> +                       return rc;
> +               }
> +               bulk[i].id = devm_kstrdup(&pdev->dev, clock_name, GFP_KERNEL);
> +       }
> +
> +       rc = devm_clk_bulk_get(&pdev->dev, num_clk, bulk);

Use devm_clk_bulk_get_all()?

> +       if (rc) {
> +               DRM_DEV_ERROR(&pdev->dev, "Failed to get clock refs %d\n", rc);
> +               return rc;
> +       }
> +
> +       rc = of_clk_set_defaults(pdev->dev.of_node, false);

Why is this needed?

> +       if (rc) {
> +               DRM_DEV_ERROR(&pdev->dev, "Failed to set clock defaults %d\n", rc);
> +               return rc;
> +       }
> +
> +       *clocks = bulk;
> +
> +       return num_clk;