[PATCH 0/2] dma: imx-sdma: Modify SDMA's remove operation

[PATCH 0/2] dma: imx-sdma: Modify SDMA's remove operation

[Vignesh Raman]

This is a patch set which contains two patches which modify's sdma_remove 
operation and add tasklet_kill in sdma_remove function.

Vignesh Raman (2):
  dma: imx-sdma: use module_platform_driver for SDMA driver
  dma: imx-sdma: Adding tasklet_kill() in sdma_remove function.

 drivers/dma/imx-sdma.c |   23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

-- 
1.7.9.5

[PATCH 1/2] dma: imx-sdma: use module_platform_driver for SDMA driver

[Vignesh Raman]

Currently there is no module_exit declared in SDMA driver, so that once
sdma module is inserted, it's shown with permanent attribute by lsmod,
and it can't be removed.
Use module_platform_driver to register/unregister SDMA driver and modify
SDMA's remove operation, to make SDMA driver possible to be removed.

Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
---
 drivers/dma/imx-sdma.c |   16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 14867e3..1360b9d 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1633,7 +1633,21 @@ err_irq:
 
 static int sdma_remove(struct platform_device *pdev)
 {
-	return -EBUSY;
+	struct sdma_engine *sdma = platform_get_drvdata(pdev);
+	struct resource *iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+	int irq = platform_get_irq(pdev, 0);
+
+	dma_async_device_unregister(&sdma->dma_device);
+	kfree(sdma->script_addrs);
+	free_irq(irq, sdma);
+	iounmap(sdma->regs);
+	release_mem_region(iores->start, resource_size(iores));
+	kfree(sdma);
+
+	platform_set_drvdata(pdev, NULL);
+
+	dev_info(&pdev->dev, "Removed...\n");
+	return 0;
 }
 
 static struct platform_driver sdma_driver = {
-- 
1.7.9.5

[PATCH 2/2] dma: imx-sdma: Adding tasklet_kill() in sdma_remove function.

[Vignesh Raman]

Several dma drivers calls tasklet_kill() in remove function. This is missing in
imx driver, so adding tasklet_kill() in sdma_remove function.

Signed-off-by: Vignesh Raman <Vignesh_Raman@mentor.com>
---
 drivers/dma/imx-sdma.c |    7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 1360b9d..974337f 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -1636,12 +1636,19 @@ static int sdma_remove(struct platform_device *pdev)
 	struct sdma_engine *sdma = platform_get_drvdata(pdev);
 	struct resource *iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	int irq = platform_get_irq(pdev, 0);
+	int i;
 
 	dma_async_device_unregister(&sdma->dma_device);
 	kfree(sdma->script_addrs);
 	free_irq(irq, sdma);
 	iounmap(sdma->regs);
 	release_mem_region(iores->start, resource_size(iores));
+	/* Kill the tasklet */
+	for (i = 0; i < MAX_DMA_CHANNELS; i++) {
+		struct sdma_channel *sdmac = &sdma->channel[i];
+
+		tasklet_kill(&sdmac->tasklet);
+	}
 	kfree(sdma);
 
 	platform_set_drvdata(pdev, NULL);
-- 
1.7.9.5

Re: [PATCH 1/2] dma: imx-sdma: use module_platform_driver for SDMA driver

[Vinod Koul]

On Wed, Jul 30, 2014 at 06:41:14PM +0530, Vignesh Raman wrote:
> Currently there is no module_exit declared in SDMA driver, so that once
> sdma module is inserted, it's shown with permanent attribute by lsmod,
> and it can't be removed.
> Use module_platform_driver to register/unregister SDMA driver and modify
> SDMA's remove operation, to make SDMA driver possible to be removed.
where is this bit below?

-- 
~Vinod
> 
> Signed-off-by: Jiada Wang <jiada_wang@mentor.com>
> ---
>  drivers/dma/imx-sdma.c |   16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 14867e3..1360b9d 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -1633,7 +1633,21 @@ err_irq:
>  
>  static int sdma_remove(struct platform_device *pdev)
>  {
> -	return -EBUSY;
> +	struct sdma_engine *sdma = platform_get_drvdata(pdev);
> +	struct resource *iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +	int irq = platform_get_irq(pdev, 0);
> +
> +	dma_async_device_unregister(&sdma->dma_device);
> +	kfree(sdma->script_addrs);
> +	free_irq(irq, sdma);
> +	iounmap(sdma->regs);
> +	release_mem_region(iores->start, resource_size(iores));
> +	kfree(sdma);
> +
> +	platform_set_drvdata(pdev, NULL);
> +
> +	dev_info(&pdev->dev, "Removed...\n");
> +	return 0;
>  }
>  
>  static struct platform_driver sdma_driver = {
> -- 
> 1.7.9.5
> 

-- 

Re: [PATCH 2/2] dma: imx-sdma: Adding tasklet_kill() in sdma_remove function.

[Vinod Koul]

On Wed, Jul 30, 2014 at 06:41:15PM +0530, Vignesh Raman wrote:
> Several dma drivers calls tasklet_kill() in remove function. This is missing in
> imx driver, so adding tasklet_kill() in sdma_remove function.
And why should we do that ?

-- 
~Vinod

> 
> Signed-off-by: Vignesh Raman <Vignesh_Raman@mentor.com>
> ---
>  drivers/dma/imx-sdma.c |    7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 1360b9d..974337f 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -1636,12 +1636,19 @@ static int sdma_remove(struct platform_device *pdev)
>  	struct sdma_engine *sdma = platform_get_drvdata(pdev);
>  	struct resource *iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>  	int irq = platform_get_irq(pdev, 0);
> +	int i;
>  
>  	dma_async_device_unregister(&sdma->dma_device);
>  	kfree(sdma->script_addrs);
>  	free_irq(irq, sdma);
>  	iounmap(sdma->regs);
>  	release_mem_region(iores->start, resource_size(iores));
> +	/* Kill the tasklet */
> +	for (i = 0; i < MAX_DMA_CHANNELS; i++) {
> +		struct sdma_channel *sdmac = &sdma->channel[i];
> +
> +		tasklet_kill(&sdmac->tasklet);
> +	}
>  	kfree(sdma);
>  
>  	platform_set_drvdata(pdev, NULL);
> -- 
> 1.7.9.5
> 

-- 

Re: [PATCH 2/2] dma: imx-sdma: Adding tasklet_kill() in sdma_remove function.

[Vignesh Raman]

On Thursday 31 July 2014 05:32 PM, Vinod Koul wrote:
> On Wed, Jul 30, 2014 at 06:41:15PM +0530, Vignesh Raman wrote:
>> Several dma drivers calls tasklet_kill() in remove function. This is missing in
>> imx driver, so adding tasklet_kill() in sdma_remove function.
> And why should we do that ?
> 
This is done because all running tasklets should be killed on remove.

Regards,
Vignesh.

Re: [PATCH 2/2] dma: imx-sdma: Adding tasklet_kill() in sdma_remove function.

[Vinod Koul]

On Fri, Aug 01, 2014 at 07:36:53PM +0530, Vignesh Raman wrote:
> On Thursday 31 July 2014 05:32 PM, Vinod Koul wrote:
> > On Wed, Jul 30, 2014 at 06:41:15PM +0530, Vignesh Raman wrote:
> >> Several dma drivers calls tasklet_kill() in remove function. This is missing in
> >> imx driver, so adding tasklet_kill() in sdma_remove function.
> > And why should we do that ?
> > 
> This is done because all running tasklets should be killed on remove.

And what prevents is from mentioning this in changelog!

-- 
~Vinod

Re: [PATCH 2/2] dma: imx-sdma: Adding tasklet_kill() in sdma_remove function.

[Vignesh Raman]

On Friday 01 August 2014 10:15 PM, Vinod Koul wrote:
> On Fri, Aug 01, 2014 at 07:36:53PM +0530, Vignesh Raman wrote:
>> On Thursday 31 July 2014 05:32 PM, Vinod Koul wrote:
>>> On Wed, Jul 30, 2014 at 06:41:15PM +0530, Vignesh Raman wrote:
>>>> Several dma drivers calls tasklet_kill() in remove function. This is missing in
>>>> imx driver, so adding tasklet_kill() in sdma_remove function.
>>> And why should we do that ?
>>>
>> This is done because all running tasklets should be killed on remove.
> 
> And what prevents is from mentioning this in changelog!
> 

I should have mentioned in the Changelog but missed it, sorry about
that. I will add it and send updated version.

Regards,
Vignesh.

Re: [PATCH 1/2] dma: imx-sdma: use module_platform_driver for SDMA driver

[Vinod Koul]

On Mon, Aug 04, 2014 at 02:11:01PM +0530, Vignesh Raman wrote:
> On Thursday 31 July 2014 05:30 PM, Vinod Koul wrote:
> > On Wed, Jul 30, 2014 at 06:41:14PM +0530, Vignesh Raman wrote:
> >> Currently there is no module_exit declared in SDMA driver, so that once
> >> sdma module is inserted, it's shown with permanent attribute by lsmod,
> >> and it can't be removed.
> >> Use module_platform_driver to register/unregister SDMA driver and modify
> >> SDMA's remove operation, to make SDMA driver possible to be removed.
> > where is this bit below?
> > 
> I'm not clear with your question. Are you asking about the unregister
> SDMA driver function? It is done by dma_async_device_unregister.
Driver already uses module_platform_driver, so I dont see what information
the last two lines are trying to convey.

Patch needs to talk about what is done in current patch

-- 
~Vinod

Re: [PATCH 1/2] dma: imx-sdma: use module_platform_driver for SDMA driver

[Vignesh Raman]

On Thursday 31 July 2014 05:30 PM, Vinod Koul wrote:
> On Wed, Jul 30, 2014 at 06:41:14PM +0530, Vignesh Raman wrote:
>> Currently there is no module_exit declared in SDMA driver, so that once
>> sdma module is inserted, it's shown with permanent attribute by lsmod,
>> and it can't be removed.
>> Use module_platform_driver to register/unregister SDMA driver and modify
>> SDMA's remove operation, to make SDMA driver possible to be removed.
> where is this bit below?
> 
I'm not clear with your question. Are you asking about the unregister
SDMA driver function? It is done by dma_async_device_unregister.

Regards,
Vignesh.

Re: [PATCH 1/2] dma: imx-sdma: use module_platform_driver for SDMA driver

[Vignesh Raman]

On Monday 04 August 2014 02:08 PM, Vinod Koul wrote:
> On Mon, Aug 04, 2014 at 02:11:01PM +0530, Vignesh Raman wrote:
>> On Thursday 31 July 2014 05:30 PM, Vinod Koul wrote:
>>> On Wed, Jul 30, 2014 at 06:41:14PM +0530, Vignesh Raman wrote:
>>>> Currently there is no module_exit declared in SDMA driver, so that once
>>>> sdma module is inserted, it's shown with permanent attribute by lsmod,
>>>> and it can't be removed.
>>>> Use module_platform_driver to register/unregister SDMA driver and modify
>>>> SDMA's remove operation, to make SDMA driver possible to be removed.
>>> where is this bit below?
>>>
>> I'm not clear with your question. Are you asking about the unregister
>> SDMA driver function? It is done by dma_async_device_unregister.
> Driver already uses module_platform_driver, so I dont see what information
> the last two lines are trying to convey.
> 
> Patch needs to talk about what is done in current patch
> 
I missed some changes in Jiada's patch while merging to mainline kernel.
I will update and send the latest version.

Thanks,
Vignesh.