coda: Fix error paths
PATCH v3 1/3 coda: Fix error paths
From: Fabio Estevam <[email protected]>
Some resources were not being released in the error path and some were released in the incorrect order.
---
Changes since v2:
- Newly introduced in this series
drivers/media/platform/coda.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-)
drivers/media/platform/coda.c +20 -12
@@ -1514,15 +1514,17 @@ static int coda_open(struct file *file)int ret =0 ;int idx; - idx =coda_next_free_instance (dev); -if (idx >=CODA_MAX_INSTANCES ) -return -EBUSY ; -set_bit (idx, &dev->instance_mask); - ctx =kzalloc (sizeof *ctx,GFP_KERNEL );if (!ctx)return -ENOMEM ; + idx =coda_next_free_instance (dev); +if (idx >=CODA_MAX_INSTANCES ) { + ret = -EBUSY ; + goto err_coda_max; + } +set_bit (idx, &dev->instance_mask); +v4l2_fh_init (&ctx->fh,video_devdata (file)); file->private_data = &ctx->fh;v4l2_fh_add (&ctx->fh);
@@ -1537,12 +1539,12 @@ static int coda_open(struct file *file)v4l2_err (&dev->v4l2_dev,"%s return error (%d)\n" , __func__, ret); - goto err; + goto err_ctx_init; } ret =coda_ctrls_setup (ctx);if (ret) {v4l2_err (&dev->v4l2_dev,"failed to setup coda controls\n" ); - goto err; + goto err_ctrls_setup; } ctx->fh.ctrl_handler = &ctx->ctrls;
@@ -1552,7 +1554,7 @@ static int coda_open(struct file *file)if (!ctx->parabuf.vaddr) {v4l2_err (&dev->v4l2_dev,"failed to allocate parabuf" ); ret = -ENOMEM ; - goto err; + goto err_dma_alloc; }coda_lock (ctx);
@@ -1567,9 +1569,15 @@ static int coda_open(struct file *file)return 0 ; -err: +err_dma_alloc: +v4l2_ctrl_handler_free (&ctx->ctrls); +err_ctrls_setup: +v4l2_m2m_ctx_release (ctx->m2m_ctx); +err_ctx_init:v4l2_fh_del (&ctx->fh);v4l2_fh_exit (&ctx->fh); +clear_bit (ctx->idx, &dev->instance_mask); +err_coda_max:kfree (ctx);return ret; }
@@ -1582,16 +1590,16 @@ static int coda_release(struct file *file)v4l2_dbg (1 , coda_debug, &dev->v4l2_dev,"Releasing instance %p\n" , ctx); +clk_disable_unprepare (dev->clk_ahb); +clk_disable_unprepare (dev->clk_per);coda_lock (ctx);list_del (&ctx->list);coda_unlock (ctx);dma_free_coherent (&dev->plat_dev->dev,CODA_PARA_BUF_SIZE , ctx->parabuf.vaddr, ctx->parabuf.paddr); -v4l2_m2m_ctx_release (ctx->m2m_ctx);v4l2_ctrl_handler_free (&ctx->ctrls); -clk_disable_unprepare (dev->clk_per); -clk_disable_unprepare (dev->clk_ahb); +v4l2_m2m_ctx_release (ctx->m2m_ctx);v4l2_fh_del (&ctx->fh);v4l2_fh_exit (&ctx->fh);clear_bit (ctx->idx, &dev->instance_mask);
Signature · 1 line
1.8.1.2
PATCH v3 2/3 coda: Check the return value from clk_prepare_enable()
From: Fabio Estevam <[email protected]>
clk_prepare_enable() may fail, so let's check its return value and propagate it in the case of error.
---
- Changes since v2:
- Release the previously acquired resources
Changes since v1:
- Add missing 'if'
drivers/media/platform/coda.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
drivers/media/platform/coda.c +15 -2
@@ -1561,14 +1561,27 @@ static int coda_open(struct file *file)list_add (&ctx->list, &dev->instances);coda_unlock (ctx); -clk_prepare_enable (dev->clk_per); -clk_prepare_enable (dev->clk_ahb); + ret =clk_prepare_enable (dev->clk_per); +if (ret) + goto err_clk_per; + + ret =clk_prepare_enable (dev->clk_ahb); +if (ret) + goto err_clk_ahb;v4l2_dbg (1 , coda_debug, &dev->v4l2_dev,"Created instance %d (%p)\n" , ctx->idx, ctx);return 0 ; +err_clk_ahb: +clk_disable_unprepare (dev->clk_per); +err_clk_per: +coda_lock (ctx); +list_del (&ctx->list); +coda_unlock (ctx); +dma_free_coherent (&dev->plat_dev->dev,CODA_PARA_BUF_SIZE , + ctx->parabuf.vaddr, ctx->parabuf.paddr); err_dma_alloc:v4l2_ctrl_handler_free (&ctx->ctrls); err_ctrls_setup:
Signature · 1 line
1.8.1.2
Re: [PATCH v3 2/3] [media] coda: Check the return value from clk_prepare_enable()
Hi Fabio,
Am Montag, den 22.07.2013, 22:38 -0300 schrieb Fabio Estevam:
Quoted 39 lines
From: Fabio Estevam <[email protected]>
clk_prepare_enable()may fail, so let's check its return value and propagate it in the case of error.Signed-off-by Fabio Estevam <[email protected]>---
- Changes since v2:
- Release the previously acquired resources
Changes since v1:
- Add missing 'if'drivers/media/platform/coda.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)drivers/media/platform/coda.c +8
-2@@ -1561,14 +1561,27 @@ static int coda_open(struct file *file)list_add (&ctx->list, &dev->instances);coda_unlock (ctx); -clk_prepare_enable (dev->clk_per); -clk_prepare_enable (dev->clk_ahb); + ret =clk_prepare_enable (dev->clk_per); +if (ret) + goto err_clk_per; + + ret =clk_prepare_enable (dev->clk_ahb); +if (ret) + goto err_clk_ahb;v4l2_dbg (1 , coda_debug, &dev->v4l2_dev,"Created instance %d (%p)\n" , ctx->idx, ctx);return 0 ; +err_clk_ahb:
drivers/media/platform/coda.c static int coda_open(struct file *file) · line 1587
Quoted 10 lines
drivers/media/platform/coda.c +7
-0+clk_disable_unprepare (dev->clk_per); +err_clk_per: +coda_lock (ctx); +list_del (&ctx->list); +coda_unlock (ctx); +dma_free_coherent (&dev->plat_dev->dev,CODA_PARA_BUF_SIZE , + ctx->parabuf.vaddr, ctx->parabuf.paddr); err_dma_alloc:v4l2_ctrl_handler_free (&ctx->ctrls); err_ctrls_setup:
I still think the list_add() should be moved after the last possible error case and the lock/list_del/unlock should be removed from the error path.
regards
Philipp
PATCH v3 3/3 coda: No need to check the return value of platform_get_resource()
In reply to Fabio EstevamFrom: Fabio Estevam <[email protected]>
When using devm_ioremap_resource(), we do not need to check the return value of platform_get_resource(), so just remove it.
---
Changes since v2:
- None
Changes since v1:
- None
drivers/media/platform/coda.c | 5 ----- 1 file changed, 5 deletions(-)
drivers/media/platform/coda.c +0 -5
@@ -2053,11 +2053,6 @@ static int coda_probe(struct platform_device *pdev)/* Get memory for physical registers */ res =platform_get_resource (pdev,IORESOURCE_MEM ,0 ); -if (res ==NULL ) { -dev_err (&pdev->dev,"failed to get memory region resource\n" ); -return -ENOENT ; - } - dev->regs_base =devm_ioremap_resource (&pdev->dev, res);if (IS_ERR (dev->regs_base))return PTR_ERR (dev->regs_base);
Signature · 1 line
1.8.1.2
Hi Fabio,
Am Montag, den 22.07.2013, 22:38 -0300 schrieb Fabio Estevam:
Quoted 88 lines
From: Fabio Estevam <[email protected]>
Some resources were not being released in the error path and some were released in the incorrect order.
Signed-off-by Fabio Estevam <[email protected]>---
Changes since v2:
- Newly introduced in this seriesdrivers/media/platform/coda.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-)drivers/media/platform/coda.c +19
-9@@ -1514,15 +1514,17 @@ static int coda_open(struct file *file)int ret =0 ;int idx; - idx =coda_next_free_instance (dev); -if (idx >=CODA_MAX_INSTANCES ) -return -EBUSY ; -set_bit (idx, &dev->instance_mask); - ctx =kzalloc (sizeof *ctx,GFP_KERNEL );if (!ctx)return -ENOMEM ; + idx =coda_next_free_instance (dev); +if (idx >=CODA_MAX_INSTANCES ) { + ret = -EBUSY ; + goto err_coda_max; + } +set_bit (idx, &dev->instance_mask); +v4l2_fh_init (&ctx->fh,video_devdata (file)); file->private_data = &ctx->fh;v4l2_fh_add (&ctx->fh);@@ -1537,12 +1539,12 @@ static int coda_open(struct file *file)v4l2_err (&dev->v4l2_dev,"%s return error (%d)\n" , __func__, ret); - goto err; + goto err_ctx_init; } ret =coda_ctrls_setup (ctx);if (ret) {v4l2_err (&dev->v4l2_dev,"failed to setup coda controls\n" ); - goto err; + goto err_ctrls_setup; } ctx->fh.ctrl_handler = &ctx->ctrls;@@ -1552,7 +1554,7 @@ static int coda_open(struct file *file)if (!ctx->parabuf.vaddr) {v4l2_err (&dev->v4l2_dev,"failed to allocate parabuf" ); ret = -ENOMEM ; - goto err; + goto err_dma_alloc; }coda_lock (ctx);@@ -1567,9 +1569,15 @@ static int coda_open(struct file *file)return 0 ; -err: +err_dma_alloc: +v4l2_ctrl_handler_free (&ctx->ctrls); +err_ctrls_setup: +v4l2_m2m_ctx_release (ctx->m2m_ctx); +err_ctx_init:v4l2_fh_del (&ctx->fh);v4l2_fh_exit (&ctx->fh); +clear_bit (ctx->idx, &dev->instance_mask); +err_coda_max:kfree (ctx);return ret; }@@ -1582,16 +1590,16 @@ static int coda_release(struct file *file)v4l2_dbg (1 , coda_debug, &dev->v4l2_dev,"Releasing instance %p\n" , ctx); +clk_disable_unprepare (dev->clk_ahb); +clk_disable_unprepare (dev->clk_per);coda_lock (ctx);
drivers/media/platform/coda.c static int coda_release(struct file *file) · line 1602
Quoted 10 lines
drivers/media/platform/coda.c +1
-3list_del (&ctx->list);coda_unlock (ctx);dma_free_coherent (&dev->plat_dev->dev,CODA_PARA_BUF_SIZE , ctx->parabuf.vaddr, ctx->parabuf.paddr); -v4l2_m2m_ctx_release (ctx->m2m_ctx);v4l2_ctrl_handler_free (&ctx->ctrls); -clk_disable_unprepare (dev->clk_per); -clk_disable_unprepare (dev->clk_ahb); +v4l2_m2m_ctx_release (ctx->m2m_ctx);
the clocks must not be disabled until after v4l2_m2m_ctx_release
returns: this function might wait for the currently running job to finish.
drivers/media/platform/coda.c static int coda_release(struct file *file) · line 1605
Quoted 3 lines
drivers/media/platform/coda.c +0
-0v4l2_fh_del (&ctx->fh);v4l2_fh_exit (&ctx->fh);clear_bit (ctx->idx, &dev->instance_mask);
regards
Philipp