[PATCH] mmc: s3cmci: Use DMA slave map rather than exported DMA filter

[PATCH] mmc: s3cmci: Use DMA slave map rather than exported DMA filter

[Sylwester Nawrocki]

Support for DMA slave map has been added to the s3c24xx-dma
controller in commit 34681d84a0f7cc22ded1413dc79eef8a2f23d9c3
"dmaengine: s3c24xx: Add dma_slave_map for s3c2440 devices"
This patch converts the s3cmci driver to also use it, so we can
eventually get rid of the exported filter function once all
DMA related clients are updated.

Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
---
 drivers/mmc/host/s3cmci.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/mmc/host/s3cmci.c b/drivers/mmc/host/s3cmci.c
index c531dee..c109e57 100644
--- a/drivers/mmc/host/s3cmci.c
+++ b/drivers/mmc/host/s3cmci.c
@@ -28,7 +28,6 @@
 #include <mach/dma.h>
 #include <mach/gpio-samsung.h>
 
-#include <linux/platform_data/dma-s3c24xx.h>
 #include <linux/platform_data/mmc-s3cmci.h>
 
 #include "s3cmci.h"
@@ -1685,13 +1684,7 @@ static int s3cmci_probe(struct platform_device *pdev)
 	/* depending on the dma state, get a dma channel to use. */
 
 	if (s3cmci_host_usedma(host)) {
-		dma_cap_mask_t mask;
-
-		dma_cap_zero(mask);
-		dma_cap_set(DMA_SLAVE, mask);
-
-		host->dma = dma_request_slave_channel_compat(mask,
-			s3c24xx_dma_filter, (void *)DMACH_SDI, &pdev->dev, "rx-tx");
+		host->dma = dma_request_slave_channel(&pdev->dev, "rx-tx");
 		if (!host->dma) {
 			dev_err(&pdev->dev, "cannot get DMA channel.\n");
 			ret = -EBUSY;
-- 
1.9.1

Re: [PATCH] mmc: s3cmci: Use DMA slave map rather than exported DMA filter

[Arnd Bergmann]

On Wednesday, October 26, 2016 11:08:27 AM CEST Sylwester Nawrocki wrote:
> @@ -1685,13 +1684,7 @@ static int s3cmci_probe(struct platform_device *pdev)
>         /* depending on the dma state, get a dma channel to use. */
>  
>         if (s3cmci_host_usedma(host)) {
> -               dma_cap_mask_t mask;
> -
> -               dma_cap_zero(mask);
> -               dma_cap_set(DMA_SLAVE, mask);
> -
> -               host->dma = dma_request_slave_channel_compat(mask,
> -                       s3c24xx_dma_filter, (void *)DMACH_SDI, &pdev->dev, "rx-tx");
> +               host->dma = dma_request_slave_channel(&pdev->dev, "rx-tx");
>                 if (!host->dma) {
>                         dev_err(&pdev->dev, "cannot get DMA channel.\n");
>                         ret = -EBUSY;

Can you convert it to use dma_request_chan()+PTR_ERR_OR_ZERO() instead?

Otherwise looks good, thanks for looking into this!

	Arnd

Re: [PATCH] mmc: s3cmci: Use DMA slave map rather than exported DMA filter

[Sylwester Nawrocki]

On 10/26/2016 11:21 AM, Arnd Bergmann wrote:
> On Wednesday, October 26, 2016 11:08:27 AM CEST Sylwester Nawrocki wrote:
>> > @@ -1685,13 +1684,7 @@ static int s3cmci_probe(struct platform_device *pdev)
>> >         /* depending on the dma state, get a dma channel to use. */
>> >  
>> >         if (s3cmci_host_usedma(host)) {
>> > -               dma_cap_mask_t mask;
>> > -
>> > -               dma_cap_zero(mask);
>> > -               dma_cap_set(DMA_SLAVE, mask);
>> > -
>> > -               host->dma = dma_request_slave_channel_compat(mask,
>> > -                       s3c24xx_dma_filter, (void *)DMACH_SDI, &pdev->dev, "rx-tx");
>> > +               host->dma = dma_request_slave_channel(&pdev->dev, "rx-tx");
>> >                 if (!host->dma) {
>> >                         dev_err(&pdev->dev, "cannot get DMA channel.\n");
>> >                         ret = -EBUSY;
>
> Can you convert it to use dma_request_chan()+PTR_ERR_OR_ZERO() instead?

Thanks for your review. dma_request_chan() seems to be returning 
either valid pointer or ERR_PTR(), so it would be sufficient 
to use just PTR_ERR(host->dma) on error path?

Re: [PATCH] mmc: s3cmci: Use DMA slave map rather than exported DMA filter

[Arnd Bergmann]

On Wednesday, October 26, 2016 2:49:44 PM CEST Sylwester Nawrocki wrote:
> On 10/26/2016 11:21 AM, Arnd Bergmann wrote:
> > On Wednesday, October 26, 2016 11:08:27 AM CEST Sylwester Nawrocki wrote:
> >> > @@ -1685,13 +1684,7 @@ static int s3cmci_probe(struct platform_device *pdev)
> >> >         /* depending on the dma state, get a dma channel to use. */
> >> >  
> >> >         if (s3cmci_host_usedma(host)) {
> >> > -               dma_cap_mask_t mask;
> >> > -
> >> > -               dma_cap_zero(mask);
> >> > -               dma_cap_set(DMA_SLAVE, mask);
> >> > -
> >> > -               host->dma = dma_request_slave_channel_compat(mask,
> >> > -                       s3c24xx_dma_filter, (void *)DMACH_SDI, &pdev->dev, "rx-tx");
> >> > +               host->dma = dma_request_slave_channel(&pdev->dev, "rx-tx");
> >> >                 if (!host->dma) {
> >> >                         dev_err(&pdev->dev, "cannot get DMA channel.\n");
> >> >                         ret = -EBUSY;
> >
> > Can you convert it to use dma_request_chan()+PTR_ERR_OR_ZERO() instead?
> 
> Thanks for your review. dma_request_chan() seems to be returning 
> either valid pointer or ERR_PTR(), so it would be sufficient 
> to use just PTR_ERR(host->dma) on error path?

Correct. You can use either

	if (IS_ERR(host->dma) {
		ret = PTR_ERR(host->dma);
		goto probe_free_gpio_wp;
	}

or

	ret = PTR_ERR_OR_ZERO(host->dma);
	if (ret)
		goto probe_free_gpio_wp;

The two do exactly the same, I just find the second one slightly more
intuitive, and I've seen a couple of spurious warnings caused by
the first (that won't happen here).

	Arnd