of_fsl_dma_probe:
- kfree(NULL) doesn't hurt but dereferencing the pointer in iounmap does
- also, the irq can be freed
of_fsl_dma_chan_probe:
- iounmap(NULL) resolved in vunmap() what which in turn is able to handle NULL
pointer but dereferencing still doesn't work
- don't clean up not yet allocated ressources, like list_del before list_add
fsl_dma_self_test:
- call fsl_dma_free_chan_resources() if the first dma trans didn't complete
Signed-off-by: Sebastian Siewior <[email protected]>
---
drivers/dma/fsldma.c | 38 ++++++++++++++++++++++++--------------
1 files changed, 24 insertions(+), 14 deletions(-)
diff --git a/drivers/dma/fsldma.c b/drivers/dma/fsldma.c
index 054eabf..d36098c 100644
--- a/drivers/dma/fsldma.c
+++ b/drivers/dma/fsldma.c
@@ -842,7 +842,7 @@ static int fsl_dma_self_test(struct fsl_dma_chan *fsl_chan)
if (fsl_dma_is_complete(chan, cookie, NULL, NULL) != DMA_SUCCESS) {
dev_err(fsl_chan->dev, "selftest: Time out!\n");
err = -ENODEV;
- goto out;
+ goto free_resources;
}
/* Test free and re-alloc channel resources */
@@ -927,8 +927,7 @@ static int __devinit of_fsl_dma_chan_probe(struct of_device *dev,
if (!new_fsl_chan) {
dev_err(&dev->dev, "No free memory for allocating "
"dma channels!\n");
- err = -ENOMEM;
- goto err;
+ return -ENOMEM;
}
/* get dma channel register base */
@@ -936,7 +935,7 @@ static int __devinit of_fsl_dma_chan_probe(struct of_device *dev,
if (err) {
dev_err(&dev->dev, "Can't get %s property 'reg'\n",
dev->node->full_name);
- goto err;
+ goto err_free_mem;
}
new_fsl_chan->feature = *(u32 *)match->data;
@@ -953,12 +952,17 @@ static int __devinit of_fsl_dma_chan_probe(struct of_device *dev,
new_fsl_chan->reg_base = ioremap(new_fsl_chan->reg.start,
new_fsl_chan->reg.end - new_fsl_chan->reg.start + 1);
+ if (!new_fsl_chan->reg_base) {
+ err = -ENOMEM;
+ goto err_free_mem;
+ }
+
new_fsl_chan->id = ((new_fsl_chan->reg.start - 0x100) & 0xfff) >> 7;
if (new_fsl_chan->id > FSL_DMA_MAX_CHANS_PER_DEVICE) {
dev_err(&dev->dev, "There is no %d channel!\n",
new_fsl_chan->id);
err = -EINVAL;
- goto err;
+ goto err_unnmap;
}
fdev->chan[new_fsl_chan->id] = new_fsl_chan;
tasklet_init(&new_fsl_chan->tasklet, dma_do_tasklet,
@@ -997,23 +1001,28 @@ static int __devinit of_fsl_dma_chan_probe(struct of_device *dev,
if (err) {
dev_err(&dev->dev, "DMA channel %s request_irq error "
"with return %d\n", dev->node->full_name, err);
- goto err;
+ goto err_remove_list;
}
}
err = fsl_dma_self_test(new_fsl_chan);
if (err)
- goto err;
+ goto err_test;
dev_info(&dev->dev, "#%d (%s), irq %d\n", new_fsl_chan->id,
match->compatible, new_fsl_chan->irq);
-
return 0;
-err:
+
+err_test:
dma_halt(new_fsl_chan);
- iounmap(new_fsl_chan->reg_base);
- free_irq(new_fsl_chan->irq, new_fsl_chan);
+ if (new_fsl_chan->irq != NO_IRQ)
+ free_irq(new_fsl_chan->irq, new_fsl_chan);
+err_remove_list:
list_del(&new_fsl_chan->common.device_node);
+ fdev->common.chancnt--;
+err_unnmap:
+ iounmap(new_fsl_chan->reg_base);
+err_free_mem:
kfree(new_fsl_chan);
return err;
}
@@ -1054,8 +1063,7 @@ static int __devinit of_fsl_dma_probe(struct of_device *dev,
fdev = kzalloc(sizeof(struct fsl_dma_device), GFP_KERNEL);
if (!fdev) {
dev_err(&dev->dev, "No enough memory for 'priv'\n");
- err = -ENOMEM;
- goto err;
+ return -ENOMEM;
}
fdev->dev = &dev->dev;
INIT_LIST_HEAD(&fdev->common.channels);
@@ -1091,7 +1099,7 @@ static int __devinit of_fsl_dma_probe(struct of_device *dev,
if (err) {
dev_err(&dev->dev, "DMA device request_irq error "
"with return %d\n", err);
- goto err;
+ goto err_remove_irq;
}
}
@@ -1101,6 +1109,8 @@ static int __devinit of_fsl_dma_probe(struct of_device *dev,
dma_async_device_register(&fdev->common);
return 0;
+err_remove_irq:
+ free_irq(irq, &fsl_dma_do_interrupt);
err:
iounmap(fdev->reg_base);
kfree(fdev);
--
1.5.5.2
> -----Original Message-----
> From: Sebastian Siewior [mailto:[email protected]]
> Sent: Tuesday, July 01, 2008 2:19 AM
> To: Li Yang
> Cc: Zhang Wei; [email protected];
> [email protected]
> Subject: [PATCH] DMA Engine: fix error path(s) in fsl-dma driver
>
> of_fsl_dma_probe:
> - kfree(NULL) doesn't hurt but dereferencing the pointer in
> iounmap does
> - also, the irq can be freed
>
> of_fsl_dma_chan_probe:
> - iounmap(NULL) resolved in vunmap() what which in turn is
> able to handle NULL
> pointer but dereferencing still doesn't work
> - don't clean up not yet allocated ressources, like list_del
> before list_add
>
> fsl_dma_self_test:
> - call fsl_dma_free_chan_resources() if the first dma trans
> didn't complete
Thanks Sebastian,
But similar patch has already been waiting in sub-system maintainer's tree which can be found at
http://git.kernel.org/?p=linux/kernel/git/djbw/async_tx.git;a=commit;h=29ec9bdef73d68134e7070ee91ccda0718d46150
- Leo