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?
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);
}
