| Message ID | 0b0d1a02a9661e89ecc047affe08be20478e2c2b.1476825466.git.jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jyri, Thank you for the patch. On Wednesday 19 Oct 2016 00:33:56 Jyri Sarha wrote: > Use unload to handle initialization failures instead of complex goto > label mess. To do this the initialization sequence needed slight > reordering and some unload functions needed to become conditional. While at it, you could replace the deprecated drm_put_dev() calls and use drm_dev_unregister() and drm_dev_unref() directly. Note that the former contains a call to drm_vblank_cleanup(), you can thus remove the direct call to that function. As far as I understand drm_dev_unregister() is safe to call after drm_mode_config_init() only but doesn't require a previous call to even drm_dev_register(). I'm not sure if that's guaranteed though, or if it just works by chance. > Signed-off-by: Jyri Sarha <jsarha@ti.com> > --- > drivers/gpu/drm/tilcdc/tilcdc_drv.c | 91 +++++++++++----------------------- > 1 file changed, 28 insertions(+), 63 deletions(-) > > diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c > b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 2dca822..8820133 100644 > --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c > +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c > @@ -160,8 +160,6 @@ static int modeset_init(struct drm_device *dev) > struct tilcdc_drm_private *priv = dev->dev_private; > struct tilcdc_module *mod; > > - drm_mode_config_init(dev); > - > priv->crtc = tilcdc_crtc_create(dev); > > list_for_each_entry(mod, &module_list, list) { > @@ -200,18 +198,19 @@ static int tilcdc_unload(struct drm_device *dev) > { > struct tilcdc_drm_private *priv = dev->dev_private; > > - tilcdc_remove_external_encoders(dev); > + if (priv->fbdev) > + drm_fbdev_cma_fini(priv->fbdev); > > - drm_fbdev_cma_fini(priv->fbdev); > drm_kms_helper_poll_fini(dev); > drm_mode_config_cleanup(dev); > drm_vblank_cleanup(dev); > - > drm_irq_uninstall(dev); > + tilcdc_remove_external_encoders(dev); > > #ifdef CONFIG_CPU_FREQ > - cpufreq_unregister_notifier(&priv->freq_transition, > - CPUFREQ_TRANSITION_NOTIFIER); > + if (priv->freq_transition.notifier_call) > + cpufreq_unregister_notifier(&priv->freq_transition, > + CPUFREQ_TRANSITION_NOTIFIER); > #endif > > if (priv->clk) > @@ -220,8 +219,10 @@ static int tilcdc_unload(struct drm_device *dev) > if (priv->mmio) > iounmap(priv->mmio); > > - flush_workqueue(priv->wq); > - destroy_workqueue(priv->wq); > + if (priv->wq) { > + flush_workqueue(priv->wq); > + destroy_workqueue(priv->wq); > + } > > dev->dev_private = NULL; > > @@ -252,6 +253,8 @@ static int tilcdc_init(struct drm_driver *ddrv, struct > device *dev) > > ddev->platformdev = pdev; > ddev->dev_private = priv; > + platform_set_drvdata(pdev, ddev); > + drm_mode_config_init(ddev); > > priv->is_componentized = > tilcdc_get_external_components(dev, NULL) > 0; > @@ -259,28 +262,28 @@ static int tilcdc_init(struct drm_driver *ddrv, struct > device *dev) priv->wq = alloc_ordered_workqueue("tilcdc", 0); > if (!priv->wq) { > ret = -ENOMEM; > - goto fail_unset_priv; > + goto init_failed; > } > > res = platform_get_resource(pdev, IORESOURCE_MEM, 0); > if (!res) { > dev_err(dev, "failed to get memory resource\n"); > ret = -EINVAL; > - goto fail_free_wq; > + goto init_failed; > } > > priv->mmio = ioremap_nocache(res->start, resource_size(res)); > if (!priv->mmio) { > dev_err(dev, "failed to ioremap\n"); > ret = -ENOMEM; > - goto fail_free_wq; > + goto init_failed; > } > > priv->clk = clk_get(dev, "fck"); > if (IS_ERR(priv->clk)) { > dev_err(dev, "failed to get functional clock\n"); > ret = -ENODEV; > - goto fail_iounmap; > + goto init_failed; > } > > #ifdef CONFIG_CPU_FREQ > @@ -289,7 +292,8 @@ static int tilcdc_init(struct drm_driver *ddrv, struct > device *dev) CPUFREQ_TRANSITION_NOTIFIER); > if (ret) { > dev_err(dev, "failed to register cpufreq notifier\n"); > - goto fail_put_clk; > + priv->freq_transition.notifier_call = NULL; > + goto init_failed; > } > #endif > > @@ -365,37 +369,35 @@ static int tilcdc_init(struct drm_driver *ddrv, struct > device *dev) ret = modeset_init(ddev); > if (ret < 0) { > dev_err(dev, "failed to initialize mode setting\n"); > - goto fail_cpufreq_unregister; > + goto init_failed; > } > > - platform_set_drvdata(pdev, ddev); > - > if (priv->is_componentized) { > ret = component_bind_all(dev, ddev); > if (ret < 0) > - goto fail_mode_config_cleanup; > + goto init_failed; > > ret = tilcdc_add_external_encoders(ddev); > if (ret < 0) > - goto fail_component_cleanup; > + goto init_failed; > } > > if ((priv->num_encoders == 0) || (priv->num_connectors == 0)) { > dev_err(dev, "no encoders/connectors found\n"); > ret = -ENXIO; > - goto fail_external_cleanup; > + goto init_failed; > } > > ret = drm_vblank_init(ddev, 1); > if (ret < 0) { > dev_err(dev, "failed to initialize vblank\n"); > - goto fail_external_cleanup; > + goto init_failed; > } > > ret = drm_irq_install(ddev, platform_get_irq(pdev, 0)); > if (ret < 0) { > dev_err(dev, "failed to install IRQ handler\n"); > - goto fail_vblank_cleanup; > + goto init_failed; > } > > drm_mode_config_reset(ddev); > @@ -405,56 +407,19 @@ static int tilcdc_init(struct drm_driver *ddrv, struct > device *dev) ddev->mode_config.num_connector); > if (IS_ERR(priv->fbdev)) { > ret = PTR_ERR(priv->fbdev); > - goto fail_irq_uninstall; > + goto init_failed; > } > > drm_kms_helper_poll_init(ddev); > > ret = drm_dev_register(ddev, 0); > if (ret) > - goto fail_platform_init; > + goto init_failed; > > return 0; > > -fail_platform_init: > - drm_kms_helper_poll_fini(ddev); > - drm_fbdev_cma_fini(priv->fbdev); > - > -fail_irq_uninstall: > - drm_irq_uninstall(ddev); > - > -fail_vblank_cleanup: > - drm_vblank_cleanup(ddev); > - > -fail_component_cleanup: > - if (priv->is_componentized) > - component_unbind_all(dev, dev); > - > -fail_mode_config_cleanup: > - drm_mode_config_cleanup(ddev); > - > -fail_external_cleanup: > - tilcdc_remove_external_encoders(ddev); > - > -fail_cpufreq_unregister: > - pm_runtime_disable(dev); > -#ifdef CONFIG_CPU_FREQ > - cpufreq_unregister_notifier(&priv->freq_transition, > - CPUFREQ_TRANSITION_NOTIFIER); > - > -fail_put_clk: > -#endif > - clk_put(priv->clk); > - > -fail_iounmap: > - iounmap(priv->mmio); > - > -fail_free_wq: > - flush_workqueue(priv->wq); > - destroy_workqueue(priv->wq); > - > -fail_unset_priv: > - ddev->dev_private = NULL; > +init_failed: > + tilcdc_unload(ddev); > drm_dev_unref(ddev); > > return ret;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 2dca822..8820133 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -160,8 +160,6 @@ static int modeset_init(struct drm_device *dev) struct tilcdc_drm_private *priv = dev->dev_private; struct tilcdc_module *mod; - drm_mode_config_init(dev); - priv->crtc = tilcdc_crtc_create(dev); list_for_each_entry(mod, &module_list, list) { @@ -200,18 +198,19 @@ static int tilcdc_unload(struct drm_device *dev) { struct tilcdc_drm_private *priv = dev->dev_private; - tilcdc_remove_external_encoders(dev); + if (priv->fbdev) + drm_fbdev_cma_fini(priv->fbdev); - drm_fbdev_cma_fini(priv->fbdev); drm_kms_helper_poll_fini(dev); drm_mode_config_cleanup(dev); drm_vblank_cleanup(dev); - drm_irq_uninstall(dev); + tilcdc_remove_external_encoders(dev); #ifdef CONFIG_CPU_FREQ - cpufreq_unregister_notifier(&priv->freq_transition, - CPUFREQ_TRANSITION_NOTIFIER); + if (priv->freq_transition.notifier_call) + cpufreq_unregister_notifier(&priv->freq_transition, + CPUFREQ_TRANSITION_NOTIFIER); #endif if (priv->clk) @@ -220,8 +219,10 @@ static int tilcdc_unload(struct drm_device *dev) if (priv->mmio) iounmap(priv->mmio); - flush_workqueue(priv->wq); - destroy_workqueue(priv->wq); + if (priv->wq) { + flush_workqueue(priv->wq); + destroy_workqueue(priv->wq); + } dev->dev_private = NULL; @@ -252,6 +253,8 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) ddev->platformdev = pdev; ddev->dev_private = priv; + platform_set_drvdata(pdev, ddev); + drm_mode_config_init(ddev); priv->is_componentized = tilcdc_get_external_components(dev, NULL) > 0; @@ -259,28 +262,28 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) priv->wq = alloc_ordered_workqueue("tilcdc", 0); if (!priv->wq) { ret = -ENOMEM; - goto fail_unset_priv; + goto init_failed; } res = platform_get_resource(pdev, IORESOURCE_MEM, 0); if (!res) { dev_err(dev, "failed to get memory resource\n"); ret = -EINVAL; - goto fail_free_wq; + goto init_failed; } priv->mmio = ioremap_nocache(res->start, resource_size(res)); if (!priv->mmio) { dev_err(dev, "failed to ioremap\n"); ret = -ENOMEM; - goto fail_free_wq; + goto init_failed; } priv->clk = clk_get(dev, "fck"); if (IS_ERR(priv->clk)) { dev_err(dev, "failed to get functional clock\n"); ret = -ENODEV; - goto fail_iounmap; + goto init_failed; } #ifdef CONFIG_CPU_FREQ @@ -289,7 +292,8 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) CPUFREQ_TRANSITION_NOTIFIER); if (ret) { dev_err(dev, "failed to register cpufreq notifier\n"); - goto fail_put_clk; + priv->freq_transition.notifier_call = NULL; + goto init_failed; } #endif @@ -365,37 +369,35 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) ret = modeset_init(ddev); if (ret < 0) { dev_err(dev, "failed to initialize mode setting\n"); - goto fail_cpufreq_unregister; + goto init_failed; } - platform_set_drvdata(pdev, ddev); - if (priv->is_componentized) { ret = component_bind_all(dev, ddev); if (ret < 0) - goto fail_mode_config_cleanup; + goto init_failed; ret = tilcdc_add_external_encoders(ddev); if (ret < 0) - goto fail_component_cleanup; + goto init_failed; } if ((priv->num_encoders == 0) || (priv->num_connectors == 0)) { dev_err(dev, "no encoders/connectors found\n"); ret = -ENXIO; - goto fail_external_cleanup; + goto init_failed; } ret = drm_vblank_init(ddev, 1); if (ret < 0) { dev_err(dev, "failed to initialize vblank\n"); - goto fail_external_cleanup; + goto init_failed; } ret = drm_irq_install(ddev, platform_get_irq(pdev, 0)); if (ret < 0) { dev_err(dev, "failed to install IRQ handler\n"); - goto fail_vblank_cleanup; + goto init_failed; } drm_mode_config_reset(ddev); @@ -405,56 +407,19 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) ddev->mode_config.num_connector); if (IS_ERR(priv->fbdev)) { ret = PTR_ERR(priv->fbdev); - goto fail_irq_uninstall; + goto init_failed; } drm_kms_helper_poll_init(ddev); ret = drm_dev_register(ddev, 0); if (ret) - goto fail_platform_init; + goto init_failed; return 0; -fail_platform_init: - drm_kms_helper_poll_fini(ddev); - drm_fbdev_cma_fini(priv->fbdev); - -fail_irq_uninstall: - drm_irq_uninstall(ddev); - -fail_vblank_cleanup: - drm_vblank_cleanup(ddev); - -fail_component_cleanup: - if (priv->is_componentized) - component_unbind_all(dev, dev); - -fail_mode_config_cleanup: - drm_mode_config_cleanup(ddev); - -fail_external_cleanup: - tilcdc_remove_external_encoders(ddev); - -fail_cpufreq_unregister: - pm_runtime_disable(dev); -#ifdef CONFIG_CPU_FREQ - cpufreq_unregister_notifier(&priv->freq_transition, - CPUFREQ_TRANSITION_NOTIFIER); - -fail_put_clk: -#endif - clk_put(priv->clk); - -fail_iounmap: - iounmap(priv->mmio); - -fail_free_wq: - flush_workqueue(priv->wq); - destroy_workqueue(priv->wq); - -fail_unset_priv: - ddev->dev_private = NULL; +init_failed: + tilcdc_unload(ddev); drm_dev_unref(ddev); return ret;
Use unload to handle initialization failures instead of complex goto label mess. To do this the initialization sequence needed slight reordering and some unload functions needed to become conditional. Signed-off-by: Jyri Sarha <jsarha@ti.com> --- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 91 ++++++++++++------------------------- 1 file changed, 28 insertions(+), 63 deletions(-)