diff mbox

[2/8] mmc: omap: Fix DMA configuration to not rely on device id

Message ID 1384396537-3486-3-git-send-email-tony@atomide.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tony Lindgren Nov. 14, 2013, 2:35 a.m. UTC
We are wrongly relying on device id for the DMA configuration
which can lead to wrong DMA channel being selected.

Fix the issue by using the standard resources like we should.

Cc: Chris Ball <cjb@laptop.org>
Cc: linux-mmc@vger.kernel.org
Signed-off-by: Tony Lindgren <tony@atomide.com>
---

If this looks OK, I'd like to merge this as a fix via arm-soc tree
along with the other patches in this series as my later patches
depend on patches in this series.

---

 drivers/mmc/host/omap.c | 32 +++++++++++---------------------
 1 file changed, 11 insertions(+), 21 deletions(-)

Comments

Joel Fernandes Nov. 30, 2013, 12:38 a.m. UTC | #1
On 11/13/2013 08:35 PM, Tony Lindgren wrote:
> We are wrongly relying on device id for the DMA configuration
> which can lead to wrong DMA channel being selected.
> 
> Fix the issue by using the standard resources like we should.
> 
> Cc: Chris Ball <cjb@laptop.org>
> Cc: linux-mmc@vger.kernel.org
> Signed-off-by: Tony Lindgren <tony@atomide.com>
> ---
> 
> If this looks OK, I'd like to merge this as a fix via arm-soc tree
> along with the other patches in this series as my later patches
> depend on patches in this series.
> 
> ---
> 
>  drivers/mmc/host/omap.c | 32 +++++++++++---------------------
>  1 file changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
> index b94f38e..ed56868 100644
> --- a/drivers/mmc/host/omap.c
> +++ b/drivers/mmc/host/omap.c
> @@ -90,17 +90,6 @@
>  #define OMAP_MMC_CMDTYPE_AC	2
>  #define OMAP_MMC_CMDTYPE_ADTC	3
>  
> -#define OMAP_DMA_MMC_TX		21
> -#define OMAP_DMA_MMC_RX		22
> -#define OMAP_DMA_MMC2_TX	54
> -#define OMAP_DMA_MMC2_RX	55
> -
> -#define OMAP24XX_DMA_MMC2_TX	47
> -#define OMAP24XX_DMA_MMC2_RX	48
> -#define OMAP24XX_DMA_MMC1_TX	61
> -#define OMAP24XX_DMA_MMC1_RX	62
> -
> -
>  #define DRIVER_NAME "mmci-omap"
>  
>  /* Specifies how often in millisecs to poll for card status changes
> @@ -1408,19 +1397,20 @@ static int mmc_omap_probe(struct platform_device *pdev)
>  	host->dma_tx_burst = -1;
>  	host->dma_rx_burst = -1;
>  
> -	if (mmc_omap2())
> -		sig = host->id == 0 ? OMAP24XX_DMA_MMC1_TX : OMAP24XX_DMA_MMC2_TX;
> -	else
> -		sig = host->id == 0 ? OMAP_DMA_MMC_TX : OMAP_DMA_MMC2_TX;
> -	host->dma_tx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
> +	res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx");
> +	if (res)
> +		sig = res->start;
> +	host->dma_tx = dma_request_slave_channel_compat(mask,
> +				omap_dma_filter_fn, &sig, &pdev->dev, "tx");

Minor comment, since we're moving to DT-only for platforms using this driver
(hope I'm right about that), why not just do:
	dma_request_slave_channel_(&pdev->dev, "tx");

IORESOURCE_DMA is not created by OF layer so I guess no need to call
platform_get_resource either.

thanks,

-Joel
Tony Lindgren Nov. 30, 2013, 5:33 p.m. UTC | #2
* Joel Fernandes <joelf@ti.com> [131129 16:39]:
> On 11/13/2013 08:35 PM, Tony Lindgren wrote:
> > --- a/drivers/mmc/host/omap.c
> > +++ b/drivers/mmc/host/omap.c
> > @@ -90,17 +90,6 @@
> >  #define OMAP_MMC_CMDTYPE_AC	2
> >  #define OMAP_MMC_CMDTYPE_ADTC	3
> >  
> > -#define OMAP_DMA_MMC_TX		21
> > -#define OMAP_DMA_MMC_RX		22
> > -#define OMAP_DMA_MMC2_TX	54
> > -#define OMAP_DMA_MMC2_RX	55
> > -
> > -#define OMAP24XX_DMA_MMC2_TX	47
> > -#define OMAP24XX_DMA_MMC2_RX	48
> > -#define OMAP24XX_DMA_MMC1_TX	61
> > -#define OMAP24XX_DMA_MMC1_RX	62
> > -
> > -
> >  #define DRIVER_NAME "mmci-omap"
> >  
> >  /* Specifies how often in millisecs to poll for card status changes
> > @@ -1408,19 +1397,20 @@ static int mmc_omap_probe(struct platform_device *pdev)
> >  	host->dma_tx_burst = -1;
> >  	host->dma_rx_burst = -1;
> >  
> > -	if (mmc_omap2())
> > -		sig = host->id == 0 ? OMAP24XX_DMA_MMC1_TX : OMAP24XX_DMA_MMC2_TX;
> > -	else
> > -		sig = host->id == 0 ? OMAP_DMA_MMC_TX : OMAP_DMA_MMC2_TX;
> > -	host->dma_tx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
> > +	res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx");
> > +	if (res)
> > +		sig = res->start;
> > +	host->dma_tx = dma_request_slave_channel_compat(mask,
> > +				omap_dma_filter_fn, &sig, &pdev->dev, "tx");
> 
> Minor comment, since we're moving to DT-only for platforms using this driver
> (hope I'm right about that), why not just do:
> 	dma_request_slave_channel_(&pdev->dev, "tx");
> 
> IORESOURCE_DMA is not created by OF layer so I guess no need to call
> platform_get_resource either.

Well this is not the omap_hsmmc.c driver, this is the one for earlier hardware
that's available on omap1 and 2420. All the later ones use the omap_hsmmc.c
driver instead. So this one we cannot make DT only unless we make mach-omap1
DT only, which is doable, but may never happen because of the effort needed.

Most of the effort would be quite trivial, except for the conversion to use
the common clock framework.

Regards,

Tony
diff mbox

Patch

diff --git a/drivers/mmc/host/omap.c b/drivers/mmc/host/omap.c
index b94f38e..ed56868 100644
--- a/drivers/mmc/host/omap.c
+++ b/drivers/mmc/host/omap.c
@@ -90,17 +90,6 @@ 
 #define OMAP_MMC_CMDTYPE_AC	2
 #define OMAP_MMC_CMDTYPE_ADTC	3
 
-#define OMAP_DMA_MMC_TX		21
-#define OMAP_DMA_MMC_RX		22
-#define OMAP_DMA_MMC2_TX	54
-#define OMAP_DMA_MMC2_RX	55
-
-#define OMAP24XX_DMA_MMC2_TX	47
-#define OMAP24XX_DMA_MMC2_RX	48
-#define OMAP24XX_DMA_MMC1_TX	61
-#define OMAP24XX_DMA_MMC1_RX	62
-
-
 #define DRIVER_NAME "mmci-omap"
 
 /* Specifies how often in millisecs to poll for card status changes
@@ -1408,19 +1397,20 @@  static int mmc_omap_probe(struct platform_device *pdev)
 	host->dma_tx_burst = -1;
 	host->dma_rx_burst = -1;
 
-	if (mmc_omap2())
-		sig = host->id == 0 ? OMAP24XX_DMA_MMC1_TX : OMAP24XX_DMA_MMC2_TX;
-	else
-		sig = host->id == 0 ? OMAP_DMA_MMC_TX : OMAP_DMA_MMC2_TX;
-	host->dma_tx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
+	res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "tx");
+	if (res)
+		sig = res->start;
+	host->dma_tx = dma_request_slave_channel_compat(mask,
+				omap_dma_filter_fn, &sig, &pdev->dev, "tx");
 	if (!host->dma_tx)
 		dev_warn(host->dev, "unable to obtain TX DMA engine channel %u\n",
 			sig);
-	if (mmc_omap2())
-		sig = host->id == 0 ? OMAP24XX_DMA_MMC1_RX : OMAP24XX_DMA_MMC2_RX;
-	else
-		sig = host->id == 0 ? OMAP_DMA_MMC_RX : OMAP_DMA_MMC2_RX;
-	host->dma_rx = dma_request_channel(mask, omap_dma_filter_fn, &sig);
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_DMA, "rx");
+	if (res)
+		sig = res->start;
+	host->dma_rx = dma_request_slave_channel_compat(mask,
+				omap_dma_filter_fn, &sig, &pdev->dev, "rx");
 	if (!host->dma_rx)
 		dev_warn(host->dev, "unable to obtain RX DMA engine channel %u\n",
 			sig);