Re: [PATCH 3/3] drm/msm/hdmi: move resource allocation to probe function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 24/08/2022 02:58, Abhinav Kumar wrote:


On 6/16/2022 12:59 AM, Dmitry Baryshkov wrote:
Rather than having all resource allocation happen in the _bind function
(resulting in possible EPROBE_DEFER returns and component bind/unbind
cycles) allocate and check all resources in _probe function. While we
are at it, use platform_get_irq() to get the IRQ rather than going
through the irq_of_parse_and_map().

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
---
  drivers/gpu/drm/msm/hdmi/hdmi.c | 295 +++++++++++++++-----------------
  1 file changed, 134 insertions(+), 161 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 8dfe5690366b..429abd622c81 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -75,8 +75,6 @@ static void msm_hdmi_destroy(struct hdmi *hdmi)
      if (hdmi->i2c)
          msm_hdmi_i2c_destroy(hdmi->i2c);
-
-    platform_set_drvdata(hdmi->pdev, NULL);
Do we still not need to do this in .remove?
  }
  static int msm_hdmi_get_phy(struct hdmi *hdmi)
@@ -116,138 +114,10 @@ static int msm_hdmi_get_phy(struct hdmi *hdmi)
   * we are to EPROBE_DEFER we want to do it here, rather than later
   * at modeset_init() time
   */
-static struct hdmi *msm_hdmi_init(struct platform_device *pdev)
+static int msm_hdmi_init(struct hdmi *hdmi)
  {
-    struct hdmi_platform_config *config = pdev->dev.platform_data;
-    struct hdmi *hdmi = NULL;
-    struct resource *res;
-    int i, ret;
-
-    hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
-    if (!hdmi) {
-        ret = -ENOMEM;
-        goto fail;
-    }
-
-    hdmi->pdev = pdev;
-    hdmi->config = config;
-    spin_lock_init(&hdmi->reg_lock);
-
-    hdmi->mmio = msm_ioremap(pdev, "core_physical");
-    if (IS_ERR(hdmi->mmio)) {
-        ret = PTR_ERR(hdmi->mmio);
-        goto fail;
-    }
-
-    /* HDCP needs physical address of hdmi register */
-    res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-        "core_physical");
-    if (!res) {
-        ret = -EINVAL;
-        goto fail;
-    }
-    hdmi->mmio_phy_addr = res->start;
-
-    hdmi->qfprom_mmio = msm_ioremap(pdev, "qfprom_physical");
-    if (IS_ERR(hdmi->qfprom_mmio)) {
-        DRM_DEV_INFO(&pdev->dev, "can't find qfprom resource\n");
-        hdmi->qfprom_mmio = NULL;
-    }
-
-    hdmi->hpd_regs = devm_kcalloc(&pdev->dev,
-                      config->hpd_reg_cnt,
-                      sizeof(hdmi->hpd_regs[0]),
-                      GFP_KERNEL);
-    if (!hdmi->hpd_regs) {
-        ret = -ENOMEM;
-        goto fail;
-    }
-    for (i = 0; i < config->hpd_reg_cnt; i++)
-        hdmi->hpd_regs[i].supply = config->hpd_reg_names[i];
-
-    ret = devm_regulator_bulk_get(&pdev->dev, config->hpd_reg_cnt, hdmi->hpd_regs);
-    if (ret) {
-        DRM_DEV_ERROR(&pdev->dev, "failed to get hpd regulator: %d\n", ret);
-        goto fail;
-    }
-
-    hdmi->pwr_regs = devm_kcalloc(&pdev->dev,
-                      config->pwr_reg_cnt,
-                      sizeof(hdmi->pwr_regs[0]),
-                      GFP_KERNEL);
-    if (!hdmi->pwr_regs) {
-        ret = -ENOMEM;
-        goto fail;
-    }
-
-    for (i = 0; i < config->pwr_reg_cnt; i++)
-        hdmi->pwr_regs[i].supply = config->pwr_reg_names[i];
-
-    ret = devm_regulator_bulk_get(&pdev->dev, config->pwr_reg_cnt, hdmi->pwr_regs);
-    if (ret) {
-        DRM_DEV_ERROR(&pdev->dev, "failed to get pwr regulator: %d\n", ret);
-        goto fail;
-    }
-
-    hdmi->hpd_clks = devm_kcalloc(&pdev->dev,
-                      config->hpd_clk_cnt,
-                      sizeof(hdmi->hpd_clks[0]),
-                      GFP_KERNEL);
-    if (!hdmi->hpd_clks) {
-        ret = -ENOMEM;
-        goto fail;
-    }
-    for (i = 0; i < config->hpd_clk_cnt; i++) {
-        struct clk *clk;
-
-        clk = msm_clk_get(pdev, config->hpd_clk_names[i]);
-        if (IS_ERR(clk)) {
-            ret = PTR_ERR(clk);
-            DRM_DEV_ERROR(&pdev->dev, "failed to get hpd clk: %s (%d)\n",
-                    config->hpd_clk_names[i], ret);
-            goto fail;
-        }
-
-        hdmi->hpd_clks[i] = clk;
-    }
-
-    hdmi->pwr_clks = devm_kcalloc(&pdev->dev,
-                      config->pwr_clk_cnt,
-                      sizeof(hdmi->pwr_clks[0]),
-                      GFP_KERNEL);
-    if (!hdmi->pwr_clks) {
-        ret = -ENOMEM;
-        goto fail;
-    }
-    for (i = 0; i < config->pwr_clk_cnt; i++) {
-        struct clk *clk;
-
-        clk = msm_clk_get(pdev, config->pwr_clk_names[i]);
-        if (IS_ERR(clk)) {
-            ret = PTR_ERR(clk);
-            DRM_DEV_ERROR(&pdev->dev, "failed to get pwr clk: %s (%d)\n",
-                    config->pwr_clk_names[i], ret);
-            goto fail;
-        }
-
-        hdmi->pwr_clks[i] = clk;
-    }
-
-    hdmi->hpd_gpiod = devm_gpiod_get_optional(&pdev->dev, "hpd", GPIOD_IN);
-    /* This will catch e.g. -EPROBE_DEFER */
-    if (IS_ERR(hdmi->hpd_gpiod)) {
-        ret = PTR_ERR(hdmi->hpd_gpiod);
-        DRM_DEV_ERROR(&pdev->dev, "failed to get hpd gpio: (%d)\n", ret);
-        goto fail;
-    }
-
-    if (!hdmi->hpd_gpiod)
-        DBG("failed to get HPD gpio");
-
-    if (hdmi->hpd_gpiod)
-        gpiod_set_consumer_name(hdmi->hpd_gpiod, "HDMI_HPD");
-
-    devm_pm_runtime_enable(&pdev->dev);
+    struct platform_device *pdev = hdmi->pdev;
+    int ret;

What about the rest of the msm_hdmi_init() function?

msm_hdmi_i2c_init, msm_hdmi_get_phy and msm_hdmi_hdcp_init have been left behind. Any reason for that?

msm_hdmi_i2c_init() allocates new adapter, so it should be part of bind().

msm_hdmi_hdcp_init() just allocates a chunk of memory (other actions are infallible). Also I did not want to move a piece of code that I can not really test.

As for the msm_hdmi_get_phy(), I don't remember why I didn't move it. But you are right, it makes sense to move it. I'll check it for v2.



      hdmi->workq = alloc_ordered_workqueue("msm_hdmi", 0);

--
With best wishes
Dmitry




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux