Message ID | 20220826093927.851597-4-dmitry.baryshkov@linaro.org |
---|---|
State | Accepted |
Commit | 69a88d8633ecc577cb6915a4050ec1e7a4099814 |
Headers | show |
Series | drm/msm/hdmi: move resource allocation to probe function | expand |
On 8/26/2022 2:39 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 | 303 +++++++++++++++----------------- > 1 file changed, 138 insertions(+), 165 deletions(-) > > diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c > index 4a364d8f4c0b..c298a36f3b42 100644 > --- a/drivers/gpu/drm/msm/hdmi/hdmi.c > +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c > @@ -76,8 +76,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) Between v1 and v2, it just seems like a rebase to me on top of the 6.1 MR. But what about moving msm_hdmi_get_phy() to probe(). I thought you were going to check that as well for v2. A change log would have been nice here. Because as part of the rebase looks like we even migrate to using panel bridge for hdmi driver. Usage of drm_of_find_panel_or_bridge was not present in v1 and wasnt obvious from the commit text either. > @@ -117,142 +115,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); > - > - ret = drm_of_find_panel_or_bridge(pdev->dev.of_node, 1, 0, NULL, &hdmi->next_bridge); > - if (ret && ret != -ENODEV) > - goto fail; > - > - 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); > > @@ -276,13 +142,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, > @@ -332,13 +198,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); > @@ -358,8 +217,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi, > > priv->bridges[priv->num_bridges++] = hdmi->bridge; > > - platform_set_drvdata(pdev, hdmi); > - > return 0; > > fail: > @@ -387,7 +244,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), > }; > @@ -397,7 +254,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), > @@ -512,23 +369,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); > @@ -561,6 +407,133 @@ 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); > + > + ret = drm_of_find_panel_or_bridge(pdev->dev.of_node, 1, 0, NULL, &hdmi->next_bridge); > + if (ret && ret != -ENODEV) > + return ret; > + > + 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); > } >
On 9/7/2022 11:54 AM, Abhinav Kumar wrote: > > > On 8/26/2022 2:39 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 | 303 +++++++++++++++----------------- >> 1 file changed, 138 insertions(+), 165 deletions(-) >> >> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c >> b/drivers/gpu/drm/msm/hdmi/hdmi.c >> index 4a364d8f4c0b..c298a36f3b42 100644 >> --- a/drivers/gpu/drm/msm/hdmi/hdmi.c >> +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c >> @@ -76,8 +76,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) > > Between v1 and v2, it just seems like a rebase to me on top of the 6.1 > MR. But what about moving msm_hdmi_get_phy() to probe(). I thought you > were going to check that as well for v2. Please ignore this part of the comment, I see that moving msm_hdmi_get_phy() to probe() is in the patch 5 of this series. Thanks for making that change. The below comment still holds true. > > A change log would have been nice here. Because as part of the rebase > looks like we even migrate to using panel bridge for hdmi driver. > > Usage of drm_of_find_panel_or_bridge was not present in v1 and wasnt > obvious from the commit text either. > >> @@ -117,142 +115,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); >> - >> - ret = drm_of_find_panel_or_bridge(pdev->dev.of_node, 1, 0, NULL, >> &hdmi->next_bridge); >> - if (ret && ret != -ENODEV) >> - goto fail; >> - >> - 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); >> @@ -276,13 +142,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, >> @@ -332,13 +198,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); >> @@ -358,8 +217,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi, >> priv->bridges[priv->num_bridges++] = hdmi->bridge; >> - platform_set_drvdata(pdev, hdmi); >> - >> return 0; >> fail: >> @@ -387,7 +244,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), >> }; >> @@ -397,7 +254,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), >> @@ -512,23 +369,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); >> @@ -561,6 +407,133 @@ 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); >> + >> + ret = drm_of_find_panel_or_bridge(pdev->dev.of_node, 1, 0, NULL, >> &hdmi->next_bridge); >> + if (ret && ret != -ENODEV) >> + return ret; >> + >> + 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); >> }
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi.c b/drivers/gpu/drm/msm/hdmi/hdmi.c index 4a364d8f4c0b..c298a36f3b42 100644 --- a/drivers/gpu/drm/msm/hdmi/hdmi.c +++ b/drivers/gpu/drm/msm/hdmi/hdmi.c @@ -76,8 +76,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) @@ -117,142 +115,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); - - ret = drm_of_find_panel_or_bridge(pdev->dev.of_node, 1, 0, NULL, &hdmi->next_bridge); - if (ret && ret != -ENODEV) - goto fail; - - 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); @@ -276,13 +142,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, @@ -332,13 +198,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); @@ -358,8 +217,6 @@ int msm_hdmi_modeset_init(struct hdmi *hdmi, priv->bridges[priv->num_bridges++] = hdmi->bridge; - platform_set_drvdata(pdev, hdmi); - return 0; fail: @@ -387,7 +244,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), }; @@ -397,7 +254,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), @@ -512,23 +369,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); @@ -561,6 +407,133 @@ 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); + + ret = drm_of_find_panel_or_bridge(pdev->dev.of_node, 1, 0, NULL, &hdmi->next_bridge); + if (ret && ret != -ENODEV) + return ret; + + 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); }
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 | 303 +++++++++++++++----------------- 1 file changed, 138 insertions(+), 165 deletions(-)