[PATCH] drm/tilcdc: Fix the error path in tilcdc_load()

[m***@public.gmane.org]

From: Ezequiel Garcia <ezequiel-***@public.gmane.org>

The current error path calls tilcdc_unload() in case of an error to release
the resources. However, this is wrong because not all resources have been
allocated by the time an error occurs in tilcdc_load().

To fix it, this commit adds proper labels to bail out at the different
stages in the load function, and release only the resources actually allocated.

Tested-by: Darren Etheridge <detheridge-***@public.gmane.org>
Tested-by: Johannes Pointner <johannes.pointner-5O2GiQo/Ci2aMPzRcYMCawC/***@public.gmane.org>
Signed-off-by: Ezequiel Garcia <ezequiel-***@public.gmane.org>
Signed-off-by: Dave Airlie <airlied-H+wXaHxf7aLQT0dZR+***@public.gmane.org>
Patch-mainline: v3.18-rc1
References: bko#86071
Git-commit: b478e336b3e75505707a11e78ef8b964ef0a03af
Signed-off-by: Matwey V. Kornilov <matwey.kornilov-***@public.gmane.org>

---
Hi,

This is to fix kernel OOPS on BeagleBone Black introduced by 3a49012224ca9016658a831a327ff6a7fe5bb4f9 (and its copies in stable trees).
See https://bugzilla.kernel.org/show_bug.cgi?id=86071 for reference.
Kernal 3.16.3+ and 3.17+ are affected. This commit is for master, stable and openSUSE-13.2 branches.

drivers/gpu/drm/tilcdc/tilcdc_drv.c | 60 ++++++++++++++++++++++++++++++-------
1 file changed, 50 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index aea4b766..79a34cb 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -84,6 +84,7 @@ static int modeset_init(struct drm_device *dev)
if ((priv->num_encoders == 0) || (priv->num_connectors == 0)) {
/* oh nos! */
dev_err(dev->dev, "no encoders/connectors found\n");
+ drm_mode_config_cleanup(dev);
return -ENXIO;
}

@@ -172,33 +173,37 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
dev->dev_private = priv;

priv->wq = alloc_ordered_workqueue("tilcdc", 0);
+ if (!priv->wq) {
+ ret = -ENOMEM;
+ goto fail_free_priv;
+ }

res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
dev_err(dev->dev, "failed to get memory resource\n");
ret = -EINVAL;
- goto fail;
+ goto fail_free_wq;
}

priv->mmio = ioremap_nocache(res->start, resource_size(res));
if (!priv->mmio) {
dev_err(dev->dev, "failed to ioremap\n");
ret = -ENOMEM;
- goto fail;
+ goto fail_free_wq;
}

priv->clk = clk_get(dev->dev, "fck");
if (IS_ERR(priv->clk)) {
dev_err(dev->dev, "failed to get functional clock\n");
ret = -ENODEV;
- goto fail;
+ goto fail_iounmap;
}

priv->disp_clk = clk_get(dev->dev, "dpll_disp_ck");
if (IS_ERR(priv->clk)) {
dev_err(dev->dev, "failed to get display clock\n");
ret = -ENODEV;
- goto fail;
+ goto fail_put_clk;
}

#ifdef CONFIG_CPU_FREQ
@@ -208,7 +213,7 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
CPUFREQ_TRANSITION_NOTIFIER);
if (ret) {
dev_err(dev->dev, "failed to register cpufreq notifier\n");
- goto fail;
+ goto fail_put_disp_clk;
}
#endif

@@ -253,13 +258,13 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
ret = modeset_init(dev);
if (ret < 0) {
dev_err(dev->dev, "failed to initialize mode setting\n");
- goto fail;
+ goto fail_cpufreq_unregister;
}

ret = drm_vblank_init(dev, 1);
if (ret < 0) {
dev_err(dev->dev, "failed to initialize vblank\n");
- goto fail;
+ goto fail_mode_config_cleanup;
}

pm_runtime_get_sync(dev->dev);
@@ -267,7 +272,7 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
pm_runtime_put_sync(dev->dev);
if (ret < 0) {
dev_err(dev->dev, "failed to install IRQ handler\n");
- goto fail;
+ goto fail_vblank_cleanup;
}

platform_set_drvdata(pdev, dev);
@@ -283,13 +288,48 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
priv->fbdev = drm_fbdev_cma_init(dev, bpp,
dev->mode_config.num_crtc,
dev->mode_config.num_connector);
+ if (IS_ERR(priv->fbdev)) {
+ ret = PTR_ERR(priv->fbdev);
+ goto fail_irq_uninstall;
+ }

drm_kms_helper_poll_init(dev);

return 0;

-fail:
- tilcdc_unload(dev);
+fail_irq_uninstall:
+ pm_runtime_get_sync(dev->dev);
+ drm_irq_uninstall(dev);
+ pm_runtime_put_sync(dev->dev);
+
+fail_vblank_cleanup:
+ drm_vblank_cleanup(dev);
+
+fail_mode_config_cleanup:
+ drm_mode_config_cleanup(dev);
+
+fail_cpufreq_unregister:
+ pm_runtime_disable(dev->dev);
+#ifdef CONFIG_CPU_FREQ
+ cpufreq_unregister_notifier(&priv->freq_transition,
+ CPUFREQ_TRANSITION_NOTIFIER);
+fail_put_disp_clk:
+ clk_put(priv->disp_clk);
+#endif
+
+fail_put_clk:
+ clk_put(priv->clk);
+
+fail_iounmap:
+ iounmap(priv->mmio);
+
+fail_free_wq:
+ flush_workqueue(priv->wq);
+ destroy_workqueue(priv->wq);
+
+fail_free_priv:
+ dev->dev_private = NULL;
+ kfree(priv);
return ret;
}

[Michal Hocko]

On Wed 15-10-14 19:06:11, matwey.kornilov-***@public.gmane.org wrote:
[...]

Post by m***@public.gmane.org
Hi,
This is to fix kernel OOPS on BeagleBone Black introduced by 3a49012224ca9016658a831a327ff6a7fe5bb4f9 (and its copies in stable trees).
See https://bugzilla.kernel.org/show_bug.cgi?id=86071 for reference.
Kernal 3.16.3+ and 3.17+ are affected. This commit is for master, stable and openSUSE-13.2 branches.

The patch is not marked for stable so I think it would make more sense
to send it to stable mailing list tagged for the appropriate stable
branches.

Thanks!

Post by m***@public.gmane.org
drivers/gpu/drm/tilcdc/tilcdc_drv.c | 60 ++++++++++++++++++++++++++++++-------
1 file changed, 50 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index aea4b766..79a34cb 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -84,6 +84,7 @@ static int modeset_init(struct drm_device *dev)
if ((priv->num_encoders == 0) || (priv->num_connectors == 0)) {
/* oh nos! */
dev_err(dev->dev, "no encoders/connectors found\n");
+ drm_mode_config_cleanup(dev);
return -ENXIO;
}
@@ -172,33 +173,37 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
dev->dev_private = priv;
priv->wq = alloc_ordered_workqueue("tilcdc", 0);
+ if (!priv->wq) {
+ ret = -ENOMEM;
+ goto fail_free_priv;
+ }
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
dev_err(dev->dev, "failed to get memory resource\n");
ret = -EINVAL;
- goto fail;
+ goto fail_free_wq;
}
priv->mmio = ioremap_nocache(res->start, resource_size(res));
if (!priv->mmio) {
dev_err(dev->dev, "failed to ioremap\n");
ret = -ENOMEM;
- goto fail;
+ goto fail_free_wq;
}
priv->clk = clk_get(dev->dev, "fck");
if (IS_ERR(priv->clk)) {
dev_err(dev->dev, "failed to get functional clock\n");
ret = -ENODEV;
- goto fail;
+ goto fail_iounmap;
}
priv->disp_clk = clk_get(dev->dev, "dpll_disp_ck");
if (IS_ERR(priv->clk)) {
dev_err(dev->dev, "failed to get display clock\n");
ret = -ENODEV;
- goto fail;
+ goto fail_put_clk;
}
#ifdef CONFIG_CPU_FREQ
@@ -208,7 +213,7 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
CPUFREQ_TRANSITION_NOTIFIER);
if (ret) {
dev_err(dev->dev, "failed to register cpufreq notifier\n");
- goto fail;
+ goto fail_put_disp_clk;
}
#endif
@@ -253,13 +258,13 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
ret = modeset_init(dev);
if (ret < 0) {
dev_err(dev->dev, "failed to initialize mode setting\n");
- goto fail;
+ goto fail_cpufreq_unregister;
}
ret = drm_vblank_init(dev, 1);
if (ret < 0) {
dev_err(dev->dev, "failed to initialize vblank\n");
- goto fail;
+ goto fail_mode_config_cleanup;
}
pm_runtime_get_sync(dev->dev);
@@ -267,7 +272,7 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
pm_runtime_put_sync(dev->dev);
if (ret < 0) {
dev_err(dev->dev, "failed to install IRQ handler\n");
- goto fail;
+ goto fail_vblank_cleanup;
}
platform_set_drvdata(pdev, dev);
@@ -283,13 +288,48 @@ static int tilcdc_load(struct drm_device *dev, unsigned long flags)
priv->fbdev = drm_fbdev_cma_init(dev, bpp,
dev->mode_config.num_crtc,
dev->mode_config.num_connector);
+ if (IS_ERR(priv->fbdev)) {
+ ret = PTR_ERR(priv->fbdev);
+ goto fail_irq_uninstall;
+ }
drm_kms_helper_poll_init(dev);
return 0;
- tilcdc_unload(dev);
+ pm_runtime_get_sync(dev->dev);
+ drm_irq_uninstall(dev);
+ pm_runtime_put_sync(dev->dev);
+
+ drm_vblank_cleanup(dev);
+
+ drm_mode_config_cleanup(dev);
+
+ pm_runtime_disable(dev->dev);
+#ifdef CONFIG_CPU_FREQ
+ cpufreq_unregister_notifier(&priv->freq_transition,
+ CPUFREQ_TRANSITION_NOTIFIER);
+ clk_put(priv->disp_clk);
+#endif
+
+ clk_put(priv->clk);
+
+ iounmap(priv->mmio);
+
+ flush_workqueue(priv->wq);
+ destroy_workqueue(priv->wq);
+
+ dev->dev_private = NULL;
+ kfree(priv);
return ret;
}
--
2.1.1
--

[Matwey V. Kornilov]

Post by Michal Hocko
[...]

Post by m***@public.gmane.org
Hi,
This is to fix kernel OOPS on BeagleBone Black introduced by 3a49012224ca9016658a831a327ff6a7fe5bb4f9 (and its copies in stable trees).
See https://bugzilla.kernel.org/show_bug.cgi?id=86071 for reference.
Kernal 3.16.3+ and 3.17+ are affected. This commit is for master, stable and openSUSE-13.2 branches.

The patch is not marked for stable so I think it would make more sense
to send it to stable mailing list tagged for the appropriate stable
branches.

I sent to stable yesterday, but still no reaction.

[Takashi Iwai]

At Thu, 16 Oct 2014 22:33:58 +0400,

Post by Matwey V. Kornilov

Post by Michal Hocko
[...]

Post by m***@public.gmane.org
Hi,
This is to fix kernel OOPS on BeagleBone Black introduced by 3a49012224ca9016658a831a327ff6a7fe5bb4f9 (and its copies in stable trees).
See https://bugzilla.kernel.org/show_bug.cgi?id=86071 for reference.
Kernal 3.16.3+ and 3.17+ are affected. This commit is for master, stable and openSUSE-13.2 branches.

The patch is not marked for stable so I think it would make more sense
to send it to stable mailing list tagged for the appropriate stable
branches.

I sent to stable yesterday, but still no reaction.

Many people are in conferences at Dusseldorf in this week, so
reactions may be slower than usual.


Takashi

[Michal Hocko]

Post by Matwey V. Kornilov

Post by Michal Hocko
[...]

Post by m***@public.gmane.org
Hi,
This is to fix kernel OOPS on BeagleBone Black introduced by 3a49012224ca9016658a831a327ff6a7fe5bb4f9 (and its copies in stable trees).
See https://bugzilla.kernel.org/show_bug.cgi?id=86071 for reference.
Kernal 3.16.3+ and 3.17+ are affected. This commit is for master, stable and openSUSE-13.2 branches.

The patch is not marked for stable so I think it would make more sense
to send it to stable mailing list tagged for the appropriate stable
branches.

I sent to stable yesterday, but still no reaction.

Thanks! Give it some more time. If the issue is particularly annoying
for you then the patch can surely be accepted to openSUSE branch of your
interest and you can use KOTD.

[Matwey V. Kornilov]

Post by Michal Hocko

Post by Matwey V. Kornilov

Post by Michal Hocko
[...]

Post by m***@public.gmane.org
Hi,
This is to fix kernel OOPS on BeagleBone Black introduced by 3a49012224ca9016658a831a327ff6a7fe5bb4f9 (and its copies in stable trees).
See https://bugzilla.kernel.org/show_bug.cgi?id=86071 for reference.
Kernal 3.16.3+ and 3.17+ are affected. This commit is for master, stable and openSUSE-13.2 branches.

The patch is not marked for stable so I think it would make more sense
to send it to stable mailing list tagged for the appropriate stable
branches.

I sent to stable yesterday, but still no reaction.

Thanks! Give it some more time. If the issue is particularly annoying
for you then the patch can surely be accepted to openSUSE branch of your
interest and you can use KOTD.

Oh, I was not grumbling, just informed.
Sure, upstream is the more correct way. I only see the single one
problem with possible upstream delay. I would like to see the patch in
13.2 (not in maintenance update).
For 13.1, I was not able to put kernel maintenance update into
13.1:Ports, not sure how 13.2:Ports will be updated though.

[Matwey V. Kornilov]

Post by Matwey V. Kornilov
Oh, I was not grumbling, just informed.
Sure, upstream is the more correct way. I only see the single one
problem with possible upstream delay. I would like to see the patch in
13.2 (not in maintenance update).
For 13.1, I was not able to put kernel maintenance update into
13.1:Ports, not sure how 13.2:Ports will be updated though.

If JeOS in 13.2:Ports is being built against 13.2 (not updates), then
you won't be able to update kernel for beaglebone from Updates,
because the system image won't boot at all.