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

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

[Dmitry Baryshkov]

As pointed several times in the discussions, start moving resource
allocation from component bind to the probe function. This simplifies
boot process, as the component will not be registered until all
resources (clocks, regulators, IRQ, etc.) are not registered.

Dmitry Baryshkov (3):
  drm/msm/hdmi: use devres helper for runtime PM management
  drm/msm/hdmi: drop constant resource names from platform config
  drm/msm/hdmi: move resource allocation to probe function

 drivers/gpu/drm/msm/hdmi/hdmi.c | 298 ++++++++++++++------------------
 drivers/gpu/drm/msm/hdmi/hdmi.h |   3 -
 2 files changed, 134 insertions(+), 167 deletions(-)

-- 
2.35.1

[PATCH 1/3] drm/msm/hdmi: use devres helper for runtime PM management

[Dmitry Baryshkov]

Use devm_pm_runtime_enable() to enable runtime PM. This way its effect
will be reverted on device unbind/destruction.

Fixes: 6ed9ed484d04 ("drm/msm/hdmi: Set up runtime PM for HDMI")
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/hdmi/hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 6acc17e0efc1..9ff9a68b201b 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -247,7 +247,7 @@ static struct hdmi *msm_hdmi_init(struct platform_device *pdev)
 	if (hdmi->hpd_gpiod)
 		gpiod_set_consumer_name(hdmi->hpd_gpiod, "HDMI_HPD");
 
-	pm_runtime_enable(&pdev->dev);
+	devm_pm_runtime_enable(&pdev->dev);
 
 	hdmi->workq = alloc_ordered_workqueue("msm_hdmi", 0);
 
-- 
2.35.1

Re: [PATCH 1/3] drm/msm/hdmi: use devres helper for runtime PM management

[Abhinav Kumar]

On 6/16/2022 12:59 AM, Dmitry Baryshkov wrote:
> Use devm_pm_runtime_enable() to enable runtime PM. This way its effect
> will be reverted on device unbind/destruction.
> 
> Fixes: 6ed9ed484d04 ("drm/msm/hdmi: Set up runtime PM for HDMI")
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---
>   drivers/gpu/drm/msm/hdmi/hdmi.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index 6acc17e0efc1..9ff9a68b201b 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -247,7 +247,7 @@ static struct hdmi *msm_hdmi_init(struct platform_device *pdev)
>   	if (hdmi->hpd_gpiod)
>   		gpiod_set_consumer_name(hdmi->hpd_gpiod, "HDMI_HPD");
>   
> -	pm_runtime_enable(&pdev->dev);
> +	devm_pm_runtime_enable(&pdev->dev);
>   
>   	hdmi->workq = alloc_ordered_workqueue("msm_hdmi", 0);
>   

[PATCH 2/3] drm/msm/hdmi: drop constant resource names from platform config

[Dmitry Baryshkov]

All MSM HDMI devices use "core_physical" and "qfprom_physical" names for
register areas. Drop them from the platform config.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/hdmi/hdmi.c | 9 +++------
 drivers/gpu/drm/msm/hdmi/hdmi.h | 3 ---
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
index 9ff9a68b201b..8dfe5690366b 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
@@ -133,7 +133,7 @@ static struct hdmi *msm_hdmi_init(struct platform_device *pdev)
 	hdmi->config = config;
 	spin_lock_init(&hdmi->reg_lock);
 
-	hdmi->mmio = msm_ioremap(pdev, config->mmio_name);
+	hdmi->mmio = msm_ioremap(pdev, "core_physical");
 	if (IS_ERR(hdmi->mmio)) {
 		ret = PTR_ERR(hdmi->mmio);
 		goto fail;
@@ -141,14 +141,14 @@ static struct hdmi *msm_hdmi_init(struct platform_device *pdev)
 
 	/* HDCP needs physical address of hdmi register */
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
-		config->mmio_name);
+		"core_physical");
 	if (!res) {
 		ret = -EINVAL;
 		goto fail;
 	}
 	hdmi->mmio_phy_addr = res->start;
 
-	hdmi->qfprom_mmio = msm_ioremap(pdev, config->qfprom_mmio_name);
+	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;
@@ -510,9 +510,6 @@ static int msm_hdmi_bind(struct device *dev, struct device *master, void *data)
 		return -ENXIO;
 	}
 
-	hdmi_cfg->mmio_name     = "core_physical";
-	hdmi_cfg->qfprom_mmio_name = "qfprom_physical";
-
 	dev->platform_data = hdmi_cfg;
 
 	hdmi = msm_hdmi_init(to_platform_device(dev));
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
index a6c88d157bc3..7263bcbf4d06 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi.h
+++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
@@ -84,9 +84,6 @@ struct hdmi {
 
 /* platform config data (ie. from DT, or pdata) */
 struct hdmi_platform_config {
-	const char *mmio_name;
-	const char *qfprom_mmio_name;
-
 	/* regulators that need to be on for hpd: */
 	const char **hpd_reg_names;
 	int hpd_reg_cnt;
-- 
2.35.1

Re: [PATCH 2/3] drm/msm/hdmi: drop constant resource names from platform config

[Abhinav Kumar]

On 6/16/2022 12:59 AM, Dmitry Baryshkov wrote:
> All MSM HDMI devices use "core_physical" and "qfprom_physical" names for
> register areas. Drop them from the platform config.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> ---
>   drivers/gpu/drm/msm/hdmi/hdmi.c | 9 +++------
>   drivers/gpu/drm/msm/hdmi/hdmi.h | 3 ---
>   2 files changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c
> index 9ff9a68b201b..8dfe5690366b 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c
> @@ -133,7 +133,7 @@ static struct hdmi *msm_hdmi_init(struct platform_device *pdev)
>   	hdmi->config = config;
>   	spin_lock_init(&hdmi->reg_lock);
>   
> -	hdmi->mmio = msm_ioremap(pdev, config->mmio_name);
> +	hdmi->mmio = msm_ioremap(pdev, "core_physical");
>   	if (IS_ERR(hdmi->mmio)) {
>   		ret = PTR_ERR(hdmi->mmio);
>   		goto fail;
> @@ -141,14 +141,14 @@ static struct hdmi *msm_hdmi_init(struct platform_device *pdev)
>   
>   	/* HDCP needs physical address of hdmi register */
>   	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> -		config->mmio_name);
> +		"core_physical");
>   	if (!res) {
>   		ret = -EINVAL;
>   		goto fail;
>   	}
>   	hdmi->mmio_phy_addr = res->start;
>   
> -	hdmi->qfprom_mmio = msm_ioremap(pdev, config->qfprom_mmio_name);
> +	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;
> @@ -510,9 +510,6 @@ static int msm_hdmi_bind(struct device *dev, struct device *master, void *data)
>   		return -ENXIO;
>   	}
>   
> -	hdmi_cfg->mmio_name     = "core_physical";
> -	hdmi_cfg->qfprom_mmio_name = "qfprom_physical";
> -
>   	dev->platform_data = hdmi_cfg;
>   
>   	hdmi = msm_hdmi_init(to_platform_device(dev));
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.h b/drivers/gpu/drm/msm/hdmi/hdmi.h
> index a6c88d157bc3..7263bcbf4d06 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi.h
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.h
> @@ -84,9 +84,6 @@ struct hdmi {
>   
>   /* platform config data (ie. from DT, or pdata) */
>   struct hdmi_platform_config {
> -	const char *mmio_name;
> -	const char *qfprom_mmio_name;
> -
>   	/* regulators that need to be on for hpd: */
>   	const char **hpd_reg_names;
>   	int hpd_reg_cnt;

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

[Dmitry Baryshkov]

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@linaro.org>
---
 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);
 }
 
 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;
 
 	hdmi->workq = alloc_ordered_workqueue("msm_hdmi", 0);
 
@@ -271,13 +141,13 @@ static struct hdmi *msm_hdmi_init(struct platform_device *pdev)
 		hdmi->hdcp_ctrl = NULL;
 	}
 
-	return hdmi;
+	return 0;
 
 fail:
 	if (hdmi)
 		msm_hdmi_destroy(hdmi);
 
-	return ERR_PTR(ret);
+	return ret;
 }
 
 /* Second part of initialization, the drm/kms level modeset_init,
@@ -318,13 +188,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
 
 	drm_connector_attach_encoder(hdmi->connector, hdmi->encoder);
 
-	hdmi->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
-	if (!hdmi->irq) {
-		ret = -EINVAL;
-		DRM_DEV_ERROR(dev->dev, "failed to get irq\n");
-		goto fail;
-	}
-
 	ret = devm_request_irq(&pdev->dev, hdmi->irq,
 			msm_hdmi_irq, IRQF_TRIGGER_HIGH,
 			"hdmi_isr", hdmi);
@@ -344,8 +207,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
 
 	priv->bridges[priv->num_bridges++]       = hdmi->bridge;
 
-	platform_set_drvdata(pdev, hdmi);
-
 	return 0;
 
 fail:
@@ -373,7 +234,7 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
 static const char *hpd_reg_names_8960[] = {"core-vdda"};
 static const char *hpd_clk_names_8960[] = {"core", "master_iface", "slave_iface"};
 
-static struct hdmi_platform_config hdmi_tx_8960_config = {
+const static struct hdmi_platform_config hdmi_tx_8960_config = {
 		HDMI_CFG(hpd_reg, 8960),
 		HDMI_CFG(hpd_clk, 8960),
 };
@@ -383,7 +244,7 @@ static const char *pwr_clk_names_8x74[] = {"extp", "alt_iface"};
 static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core"};
 static unsigned long hpd_clk_freq_8x74[] = {0, 19200000, 0};
 
-static struct hdmi_platform_config hdmi_tx_8974_config = {
+const static struct hdmi_platform_config hdmi_tx_8974_config = {
 		HDMI_CFG(pwr_reg, 8x74),
 		HDMI_CFG(pwr_clk, 8x74),
 		HDMI_CFG(hpd_clk, 8x74),
@@ -498,23 +359,12 @@ static int msm_hdmi_register_audio_driver(struct hdmi *hdmi, struct device *dev)
 static int msm_hdmi_bind(struct device *dev, struct device *master, void *data)
 {
 	struct msm_drm_private *priv = dev_get_drvdata(master);
-	struct hdmi_platform_config *hdmi_cfg;
-	struct hdmi *hdmi;
-	struct device_node *of_node = dev->of_node;
+	struct hdmi *hdmi = dev_get_drvdata(dev);
 	int err;
 
-	hdmi_cfg = (struct hdmi_platform_config *)
-			of_device_get_match_data(dev);
-	if (!hdmi_cfg) {
-		DRM_DEV_ERROR(dev, "unknown hdmi_cfg: %pOFn\n", of_node);
-		return -ENXIO;
-	}
-
-	dev->platform_data = hdmi_cfg;
-
-	hdmi = msm_hdmi_init(to_platform_device(dev));
-	if (IS_ERR(hdmi))
-		return PTR_ERR(hdmi);
+	err = msm_hdmi_init(hdmi);
+	if (err)
+		return err;
 	priv->hdmi = hdmi;
 
 	err = msm_hdmi_register_audio_driver(hdmi, dev);
@@ -547,6 +397,129 @@ static const struct component_ops msm_hdmi_ops = {
 
 static int msm_hdmi_dev_probe(struct platform_device *pdev)
 {
+	const struct hdmi_platform_config *config;
+	struct device *dev = &pdev->dev;
+	struct hdmi *hdmi;
+	struct resource *res;
+	int i, ret;
+
+	config = of_device_get_match_data(dev);
+	if (!config)
+		return -EINVAL;
+
+	hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
+	if (!hdmi)
+		return -ENOMEM;
+
+	hdmi->pdev = pdev;
+	hdmi->config = config;
+	spin_lock_init(&hdmi->reg_lock);
+
+	hdmi->mmio = msm_ioremap(pdev, "core_physical");
+	if (IS_ERR(hdmi->mmio))
+		return PTR_ERR(hdmi->mmio);
+
+	/* HDCP needs physical address of hdmi register */
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
+		"core_physical");
+	if (!res)
+		return -EINVAL;
+	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->irq = platform_get_irq(pdev, 0);
+	if (hdmi->irq < 0)
+		return hdmi->irq;
+
+	hdmi->hpd_regs = devm_kcalloc(&pdev->dev,
+				      config->hpd_reg_cnt,
+				      sizeof(hdmi->hpd_regs[0]),
+				      GFP_KERNEL);
+	if (!hdmi->hpd_regs)
+		return -ENOMEM;
+
+	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)
+		return dev_err_probe(dev, ret, "failed to get hpd regulators\n");
+
+	hdmi->pwr_regs = devm_kcalloc(&pdev->dev,
+				      config->pwr_reg_cnt,
+				      sizeof(hdmi->pwr_regs[0]),
+				      GFP_KERNEL);
+	if (!hdmi->pwr_regs)
+		return -ENOMEM;
+
+	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)
+		return dev_err_probe(dev, ret, "failed to get pwr regulators\n");
+
+	hdmi->hpd_clks = devm_kcalloc(&pdev->dev,
+				      config->hpd_clk_cnt,
+				      sizeof(hdmi->hpd_clks[0]),
+				      GFP_KERNEL);
+	if (!hdmi->hpd_clks)
+		return -ENOMEM;
+
+	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))
+			return dev_err_probe(dev, PTR_ERR(clk),
+					     "failed to get hpd clk: %s\n",
+					     config->hpd_clk_names[i]);
+
+		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)
+		return -ENOMEM;
+
+	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))
+			return dev_err_probe(dev, PTR_ERR(clk),
+					     "failed to get pwr clk: %s\n",
+					     config->pwr_clk_names[i]);
+
+		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))
+		return dev_err_probe(dev, PTR_ERR(hdmi->hpd_gpiod),
+				     "failed to get hpd gpio\n");
+
+	if (!hdmi->hpd_gpiod)
+		DBG("failed to get HPD gpio");
+
+	if (hdmi->hpd_gpiod)
+		gpiod_set_consumer_name(hdmi->hpd_gpiod, "HDMI_HPD");
+
+	ret = devm_pm_runtime_enable(&pdev->dev);
+	if (ret)
+		return ret;
+
+	platform_set_drvdata(pdev, hdmi);
+
 	return component_add(&pdev->dev, &msm_hdmi_ops);
 }
 
-- 
2.35.1

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

[Abhinav Kumar]

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@linaro.org>
> ---
>   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?


>   
>   	hdmi->workq = alloc_ordered_workqueue("msm_hdmi", 0);
>   
> @@ -271,13 +141,13 @@ static struct hdmi *msm_hdmi_init(struct platform_device *pdev)
>   		hdmi->hdcp_ctrl = NULL;
>   	}
>   
> -	return hdmi;
> +	return 0;
>   
>   fail:
>   	if (hdmi)
>   		msm_hdmi_destroy(hdmi);
>   
> -	return ERR_PTR(ret);
> +	return ret;
>   }
>   
>   /* Second part of initialization, the drm/kms level modeset_init,
> @@ -318,13 +188,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
>   
>   	drm_connector_attach_encoder(hdmi->connector, hdmi->encoder);
>   
> -	hdmi->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
> -	if (!hdmi->irq) {
> -		ret = -EINVAL;
> -		DRM_DEV_ERROR(dev->dev, "failed to get irq\n");
> -		goto fail;
> -	}
> -
>   	ret = devm_request_irq(&pdev->dev, hdmi->irq,
>   			msm_hdmi_irq, IRQF_TRIGGER_HIGH,
>   			"hdmi_isr", hdmi);
> @@ -344,8 +207,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
>   
>   	priv->bridges[priv->num_bridges++]       = hdmi->bridge;
>   
> -	platform_set_drvdata(pdev, hdmi);
> -
>   	return 0;
>   
>   fail:
> @@ -373,7 +234,7 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi,
>   static const char *hpd_reg_names_8960[] = {"core-vdda"};
>   static const char *hpd_clk_names_8960[] = {"core", "master_iface", "slave_iface"};
>   
> -static struct hdmi_platform_config hdmi_tx_8960_config = {
> +const static struct hdmi_platform_config hdmi_tx_8960_config = {
>   		HDMI_CFG(hpd_reg, 8960),
>   		HDMI_CFG(hpd_clk, 8960),
>   };
> @@ -383,7 +244,7 @@ static const char *pwr_clk_names_8x74[] = {"extp", "alt_iface"};
>   static const char *hpd_clk_names_8x74[] = {"iface", "core", "mdp_core"};
>   static unsigned long hpd_clk_freq_8x74[] = {0, 19200000, 0};
>   
> -static struct hdmi_platform_config hdmi_tx_8974_config = {
> +const static struct hdmi_platform_config hdmi_tx_8974_config = {
>   		HDMI_CFG(pwr_reg, 8x74),
>   		HDMI_CFG(pwr_clk, 8x74),
>   		HDMI_CFG(hpd_clk, 8x74),
> @@ -498,23 +359,12 @@ static int msm_hdmi_register_audio_driver(struct hdmi *hdmi, struct device *dev)
>   static int msm_hdmi_bind(struct device *dev, struct device *master, void *data)
>   {
>   	struct msm_drm_private *priv = dev_get_drvdata(master);
> -	struct hdmi_platform_config *hdmi_cfg;
> -	struct hdmi *hdmi;
> -	struct device_node *of_node = dev->of_node;
> +	struct hdmi *hdmi = dev_get_drvdata(dev);
>   	int err;
>   
> -	hdmi_cfg = (struct hdmi_platform_config *)
> -			of_device_get_match_data(dev);
> -	if (!hdmi_cfg) {
> -		DRM_DEV_ERROR(dev, "unknown hdmi_cfg: %pOFn\n", of_node);
> -		return -ENXIO;
> -	}
> -
> -	dev->platform_data = hdmi_cfg;
> -
> -	hdmi = msm_hdmi_init(to_platform_device(dev));
> -	if (IS_ERR(hdmi))
> -		return PTR_ERR(hdmi);
> +	err = msm_hdmi_init(hdmi);
> +	if (err)
> +		return err;
>   	priv->hdmi = hdmi;
>   
>   	err = msm_hdmi_register_audio_driver(hdmi, dev);
> @@ -547,6 +397,129 @@ static const struct component_ops msm_hdmi_ops = {
>   
>   static int msm_hdmi_dev_probe(struct platform_device *pdev)
>   {
> +	const struct hdmi_platform_config *config;
> +	struct device *dev = &pdev->dev;
> +	struct hdmi *hdmi;
> +	struct resource *res;
> +	int i, ret;
> +
> +	config = of_device_get_match_data(dev);
> +	if (!config)
> +		return -EINVAL;
> +
> +	hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL);
> +	if (!hdmi)
> +		return -ENOMEM;
> +
> +	hdmi->pdev = pdev;
> +	hdmi->config = config;
> +	spin_lock_init(&hdmi->reg_lock);
> +
> +	hdmi->mmio = msm_ioremap(pdev, "core_physical");
> +	if (IS_ERR(hdmi->mmio))
> +		return PTR_ERR(hdmi->mmio);
> +
> +	/* HDCP needs physical address of hdmi register */
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM,
> +		"core_physical");
> +	if (!res)
> +		return -EINVAL;
> +	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->irq = platform_get_irq(pdev, 0);
> +	if (hdmi->irq < 0)
> +		return hdmi->irq;
> +
> +	hdmi->hpd_regs = devm_kcalloc(&pdev->dev,
> +				      config->hpd_reg_cnt,
> +				      sizeof(hdmi->hpd_regs[0]),
> +				      GFP_KERNEL);
> +	if (!hdmi->hpd_regs)
> +		return -ENOMEM;
> +
> +	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)
> +		return dev_err_probe(dev, ret, "failed to get hpd regulators\n");
> +
> +	hdmi->pwr_regs = devm_kcalloc(&pdev->dev,
> +				      config->pwr_reg_cnt,
> +				      sizeof(hdmi->pwr_regs[0]),
> +				      GFP_KERNEL);
> +	if (!hdmi->pwr_regs)
> +		return -ENOMEM;
> +
> +	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)
> +		return dev_err_probe(dev, ret, "failed to get pwr regulators\n");
> +
> +	hdmi->hpd_clks = devm_kcalloc(&pdev->dev,
> +				      config->hpd_clk_cnt,
> +				      sizeof(hdmi->hpd_clks[0]),
> +				      GFP_KERNEL);
> +	if (!hdmi->hpd_clks)
> +		return -ENOMEM;
> +
> +	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))
> +			return dev_err_probe(dev, PTR_ERR(clk),
> +					     "failed to get hpd clk: %s\n",
> +					     config->hpd_clk_names[i]);
> +
> +		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)
> +		return -ENOMEM;
> +
> +	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))
> +			return dev_err_probe(dev, PTR_ERR(clk),
> +					     "failed to get pwr clk: %s\n",
> +					     config->pwr_clk_names[i]);
> +
> +		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))
> +		return dev_err_probe(dev, PTR_ERR(hdmi->hpd_gpiod),
> +				     "failed to get hpd gpio\n");
> +
> +	if (!hdmi->hpd_gpiod)
> +		DBG("failed to get HPD gpio");
> +
> +	if (hdmi->hpd_gpiod)
> +		gpiod_set_consumer_name(hdmi->hpd_gpiod, "HDMI_HPD");
> +
> +	ret = devm_pm_runtime_enable(&pdev->dev);
> +	if (ret)
> +		return ret;
> +
> +	platform_set_drvdata(pdev, hdmi);
> +
>   	return component_add(&pdev->dev, &msm_hdmi_ops);
>   }
>   

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

[Dmitry Baryshkov]

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@linaro.org>
>> ---
>>   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