public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] i.MX28: Enable SD DMA only if DCache enabled
@ 2012-04-04 21:51 Marek Vasut
  2012-04-04 23:12 ` Fabio Estevam
  2012-04-05 10:24 ` [U-Boot] [PATCH] i.MX28: Allow coexistence of PIO and DMA mode for SD/MMC Marek Vasut
  0 siblings, 2 replies; 13+ messages in thread
From: Marek Vasut @ 2012-04-04 21:51 UTC (permalink / raw)
  To: u-boot

This SD DMA function of i.MX28 is still apparently too experimental to be
enabled by default in 2012.04 release. Enable this feature only if the user
plans to tinker with DCache or explicitly enables it.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/mmc/mxsmmc.c |   46 +++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 45 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/mxsmmc.c b/drivers/mmc/mxsmmc.c
index e8bad9d..10b9d34 100644
--- a/drivers/mmc/mxsmmc.c
+++ b/drivers/mmc/mxsmmc.c
@@ -55,6 +55,11 @@ struct mxsmmc_priv {
 
 #define	MXSMMC_MAX_TIMEOUT	10000
 
+/* Enable DMA transfers only if DCache is on. */
+#ifndef CONFIG_SYS_DCACHE_OFF
+#define CONFIG_MXS_MMC_DMA
+#endif
+
 /*
  * Sends a command out on the bus.  Takes the mmc pointer,
  * a command pointer, and an optional data pointer.
@@ -66,8 +71,13 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 	struct mx28_ssp_regs *ssp_regs = priv->regs;
 	uint32_t reg;
 	int timeout;
-	uint32_t data_count, cache_data_count;
+	uint32_t data_count;
 	uint32_t ctrl0;
+#ifndef CONFIG_MXS_MMC_DMA
+	uint32_t *data_ptr;
+#else
+	uint32_t cache_data_count;
+#endif
 
 	debug("MMC%d: CMD%d\n", mmc->block_dev.dev, cmd->cmdidx);
 
@@ -185,7 +195,9 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 		return 0;
 
 	data_count = data->blocksize * data->blocks;
+	timeout = MXSMMC_MAX_TIMEOUT;
 
+#ifdef CONFIG_MXS_MMC_DMA
 	if (data_count % ARCH_DMA_MINALIGN)
 		cache_data_count = roundup(data_count, ARCH_DMA_MINALIGN);
 	else
@@ -218,6 +230,38 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 		invalidate_dcache_range((uint32_t)priv->desc->cmd.address,
 			(uint32_t)(priv->desc->cmd.address + cache_data_count));
 	}
+#else
+	if (data->flags & MMC_DATA_READ) {
+		data_ptr = (uint32_t *)data->dest;
+		while (data_count && --timeout) {
+			reg = readl(&ssp_regs->hw_ssp_status);
+			if (!(reg & SSP_STATUS_FIFO_EMPTY)) {
+				*data_ptr++ = readl(&ssp_regs->hw_ssp_data);
+				data_count -= 4;
+				timeout = MXSMMC_MAX_TIMEOUT;
+			} else
+				udelay(1000);
+		}
+	} else {
+		data_ptr = (uint32_t *)data->src;
+		timeout *= 100;
+		while (data_count && --timeout) {
+			reg = readl(&ssp_regs->hw_ssp_status);
+			if (!(reg & SSP_STATUS_FIFO_FULL)) {
+				writel(*data_ptr++, &ssp_regs->hw_ssp_data);
+				data_count -= 4;
+				timeout = MXSMMC_MAX_TIMEOUT;
+			} else
+				udelay(1000);
+		}
+	}
+
+	if (!timeout) {
+		printf("MMC%d: Data timeout with command %d (status 0x%08x)!\n",
+			mmc->block_dev.dev, cmd->cmdidx, reg);
+		return COMM_ERR;
+	}
+#endif
 
 	/* Check data errors */
 	reg = readl(&ssp_regs->hw_ssp_status);
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH] i.MX28: Enable SD DMA only if DCache enabled
  2012-04-04 21:51 [U-Boot] [PATCH] i.MX28: Enable SD DMA only if DCache enabled Marek Vasut
@ 2012-04-04 23:12 ` Fabio Estevam
  2012-04-05  0:48   ` Marek Vasut
  2012-04-05 10:24 ` [U-Boot] [PATCH] i.MX28: Allow coexistence of PIO and DMA mode for SD/MMC Marek Vasut
  1 sibling, 1 reply; 13+ messages in thread
From: Fabio Estevam @ 2012-04-04 23:12 UTC (permalink / raw)
  To: u-boot

On Wed, Apr 4, 2012 at 6:51 PM, Marek Vasut <marex@denx.de> wrote:

> +/* Enable DMA transfers only if DCache is on. */
> +#ifndef CONFIG_SYS_DCACHE_OFF
> +#define CONFIG_MXS_MMC_DMA
> +#endif

Your patch makes sense, but I would prefer not to couple
CONFIG_MXS_MMC_DMA with DCACHE support.

Someone may want to enable cache, but still uses MMC driver in PIO mode.

IMHO CONFIG_MXS_MMC_DMA could be defined in configs/board.h when needed.

Yesterday I was running MMC DMA without DCACHE support and it did work
(after the mxs_dma_init changes).

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH] i.MX28: Enable SD DMA only if DCache enabled
  2012-04-04 23:12 ` Fabio Estevam
@ 2012-04-05  0:48   ` Marek Vasut
  2012-04-05  3:36     ` Fabio Estevam
  2012-04-06  0:34     ` Fabio Estevam
  0 siblings, 2 replies; 13+ messages in thread
From: Marek Vasut @ 2012-04-05  0:48 UTC (permalink / raw)
  To: u-boot

Dear Fabio Estevam,

> On Wed, Apr 4, 2012 at 6:51 PM, Marek Vasut <marex@denx.de> wrote:
> > +/* Enable DMA transfers only if DCache is on. */
> > +#ifndef CONFIG_SYS_DCACHE_OFF
> > +#define CONFIG_MXS_MMC_DMA
> > +#endif
> 
> Your patch makes sense, but I would prefer not to couple
> CONFIG_MXS_MMC_DMA with DCACHE support.
> 
> Someone may want to enable cache, but still uses MMC driver in PIO mode.

You don't, it's slow as crap then.

> IMHO CONFIG_MXS_MMC_DMA could be defined in configs/board.h when needed.
> 
> Yesterday I was running MMC DMA without DCACHE support and it did work
> (after the mxs_dma_init changes).

I have some obscure card now and I'm seeing breakage (DMA transfer timeout).

btw. Fabio, can you ping me on jabber please?

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH] i.MX28: Enable SD DMA only if DCache enabled
  2012-04-05  0:48   ` Marek Vasut
@ 2012-04-05  3:36     ` Fabio Estevam
  2012-04-05 10:08       ` Marek Vasut
  2012-04-06  0:34     ` Fabio Estevam
  1 sibling, 1 reply; 13+ messages in thread
From: Fabio Estevam @ 2012-04-05  3:36 UTC (permalink / raw)
  To: u-boot

On Wed, Apr 4, 2012 at 9:48 PM, Marek Vasut <marex@denx.de> wrote:

>> Someone may want to enable cache, but still uses MMC driver in PIO mode.
>
> You don't, it's slow as crap then.

Well, this is how we have been using the MMC driver for the last 4 months.

I like the fact that your patch let PIO and DMA mode co-exist in the
driver and that they are selectable by the CONFIG_MXS_MMC_DMA option.

What I really don?t like is that by enabling CONFIG_SYS_DCACHE_OFF you
force CONFIG_MXS_MMC_DMA.

These are separate things and one config option should not force the other.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH] i.MX28: Enable SD DMA only if DCache enabled
  2012-04-05  3:36     ` Fabio Estevam
@ 2012-04-05 10:08       ` Marek Vasut
  0 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2012-04-05 10:08 UTC (permalink / raw)
  To: u-boot

Dear Fabio Estevam,

> On Wed, Apr 4, 2012 at 9:48 PM, Marek Vasut <marex@denx.de> wrote:
> >> Someone may want to enable cache, but still uses MMC driver in PIO mode.
> > 
> > You don't, it's slow as crap then.
> 
> Well, this is how we have been using the MMC driver for the last 4 months.

With DCache disabled though ;-)

> I like the fact that your patch let PIO and DMA mode co-exist in the
> driver and that they are selectable by the CONFIG_MXS_MMC_DMA option.
> 
> What I really don?t like is that by enabling CONFIG_SYS_DCACHE_OFF you
> force CONFIG_MXS_MMC_DMA.

All right, agreed.

> These are separate things and one config option should not force the other.

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH] i.MX28: Allow coexistence of PIO and DMA mode for SD/MMC
  2012-04-04 21:51 [U-Boot] [PATCH] i.MX28: Enable SD DMA only if DCache enabled Marek Vasut
  2012-04-04 23:12 ` Fabio Estevam
@ 2012-04-05 10:24 ` Marek Vasut
  2012-04-05 12:12   ` Stefano Babic
  2012-04-05 13:30   ` [U-Boot] [PATCH V3] " Marek Vasut
  1 sibling, 2 replies; 13+ messages in thread
From: Marek Vasut @ 2012-04-05 10:24 UTC (permalink / raw)
  To: u-boot

This SD DMA function of i.MX28 is still apparently too experimental to be
enabled by default in 2012.04 release. Enable this feature only if the user
plans to tinker with DCache or explicitly enables it.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/mmc/mxsmmc.c |   41 ++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 40 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/mxsmmc.c b/drivers/mmc/mxsmmc.c
index e8bad9d..7187796 100644
--- a/drivers/mmc/mxsmmc.c
+++ b/drivers/mmc/mxsmmc.c
@@ -66,8 +66,13 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 	struct mx28_ssp_regs *ssp_regs = priv->regs;
 	uint32_t reg;
 	int timeout;
-	uint32_t data_count, cache_data_count;
+	uint32_t data_count;
 	uint32_t ctrl0;
+#ifndef CONFIG_MXS_MMC_DMA
+	uint32_t *data_ptr;
+#else
+	uint32_t cache_data_count;
+#endif
 
 	debug("MMC%d: CMD%d\n", mmc->block_dev.dev, cmd->cmdidx);
 
@@ -185,7 +190,9 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 		return 0;
 
 	data_count = data->blocksize * data->blocks;
+	timeout = MXSMMC_MAX_TIMEOUT;
 
+#ifdef CONFIG_MXS_MMC_DMA
 	if (data_count % ARCH_DMA_MINALIGN)
 		cache_data_count = roundup(data_count, ARCH_DMA_MINALIGN);
 	else
@@ -218,6 +225,38 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 		invalidate_dcache_range((uint32_t)priv->desc->cmd.address,
 			(uint32_t)(priv->desc->cmd.address + cache_data_count));
 	}
+#else
+	if (data->flags & MMC_DATA_READ) {
+		data_ptr = (uint32_t *)data->dest;
+		while (data_count && --timeout) {
+			reg = readl(&ssp_regs->hw_ssp_status);
+			if (!(reg & SSP_STATUS_FIFO_EMPTY)) {
+				*data_ptr++ = readl(&ssp_regs->hw_ssp_data);
+				data_count -= 4;
+				timeout = MXSMMC_MAX_TIMEOUT;
+			} else
+				udelay(1000);
+		}
+	} else {
+		data_ptr = (uint32_t *)data->src;
+		timeout *= 100;
+		while (data_count && --timeout) {
+			reg = readl(&ssp_regs->hw_ssp_status);
+			if (!(reg & SSP_STATUS_FIFO_FULL)) {
+				writel(*data_ptr++, &ssp_regs->hw_ssp_data);
+				data_count -= 4;
+				timeout = MXSMMC_MAX_TIMEOUT;
+			} else
+				udelay(1000);
+		}
+	}
+
+	if (!timeout) {
+		printf("MMC%d: Data timeout with command %d (status 0x%08x)!\n",
+			mmc->block_dev.dev, cmd->cmdidx, reg);
+		return COMM_ERR;
+	}
+#endif
 
 	/* Check data errors */
 	reg = readl(&ssp_regs->hw_ssp_status);
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH] i.MX28: Allow coexistence of PIO and DMA mode for SD/MMC
  2012-04-05 10:24 ` [U-Boot] [PATCH] i.MX28: Allow coexistence of PIO and DMA mode for SD/MMC Marek Vasut
@ 2012-04-05 12:12   ` Stefano Babic
  2012-04-05 12:17     ` Marek Vasut
  2012-04-05 13:30   ` [U-Boot] [PATCH V3] " Marek Vasut
  1 sibling, 1 reply; 13+ messages in thread
From: Stefano Babic @ 2012-04-05 12:12 UTC (permalink / raw)
  To: u-boot

On 05/04/2012 12:24, Marek Vasut wrote:
> This SD DMA function of i.MX28 is still apparently too experimental to be
> enabled by default in 2012.04 release. Enable this feature only if the user
> plans to tinker with DCache or explicitly enables it.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Detlev Zundel <dzu@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> ---

Hi Marek,

>  drivers/mmc/mxsmmc.c |   41 ++++++++++++++++++++++++++++++++++++++++-
>  1 files changed, 40 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/mxsmmc.c b/drivers/mmc/mxsmmc.c
> index e8bad9d..7187796 100644
> --- a/drivers/mmc/mxsmmc.c
> +++ b/drivers/mmc/mxsmmc.c
> @@ -66,8 +66,13 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>  	struct mx28_ssp_regs *ssp_regs = priv->regs;
>  	uint32_t reg;
>  	int timeout;
> -	uint32_t data_count, cache_data_count;
> +	uint32_t data_count;
>  	uint32_t ctrl0;
> +#ifndef CONFIG_MXS_MMC_DMA
> +	uint32_t *data_ptr;
> +#else
> +	uint32_t cache_data_count;
> +#endif
>  
>  	debug("MMC%d: CMD%d\n", mmc->block_dev.dev, cmd->cmdidx);
>  
> @@ -185,7 +190,9 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>  		return 0;
>  
>  	data_count = data->blocksize * data->blocks;
> +	timeout = MXSMMC_MAX_TIMEOUT;
>  
> +#ifdef CONFIG_MXS_MMC_DMA
>  	if (data_count % ARCH_DMA_MINALIGN)
>  		cache_data_count = roundup(data_count, ARCH_DMA_MINALIGN);
>  	else
> @@ -218,6 +225,38 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
>  		invalidate_dcache_range((uint32_t)priv->desc->cmd.address,
>  			(uint32_t)(priv->desc->cmd.address + cache_data_count));
>  	}
> +#else
> +	if (data->flags & MMC_DATA_READ) {
> +		data_ptr = (uint32_t *)data->dest;
> +		while (data_count && --timeout) {
> +			reg = readl(&ssp_regs->hw_ssp_status);
> +			if (!(reg & SSP_STATUS_FIFO_EMPTY)) {
> +				*data_ptr++ = readl(&ssp_regs->hw_ssp_data);
> +				data_count -= 4;
> +				timeout = MXSMMC_MAX_TIMEOUT;
> +			} else
> +				udelay(1000);
> +		}
> +	} else {
> +		data_ptr = (uint32_t *)data->src;
> +		timeout *= 100;
> +		while (data_count && --timeout) {
> +			reg = readl(&ssp_regs->hw_ssp_status);
> +			if (!(reg & SSP_STATUS_FIFO_FULL)) {
> +				writel(*data_ptr++, &ssp_regs->hw_ssp_data);
> +				data_count -= 4;
> +				timeout = MXSMMC_MAX_TIMEOUT;
> +			} else
> +				udelay(1000);
> +		}
> +	}

Ok, I see. This patch reverts to the status before applying the DMA's patch.
I would only ask if it makes sense to leave DMA support on MMC when we
know that is buggy. Is it DMA support working for NAND ? If yes, it is
not possible to enable DMA for NAND and disable it for MMC, as both
driver use the same CONFIG_MXS_MMC_DMA.

Anyway, I am not saying we should introduce a new switch, but maybe we
should drop DMA support in msx_mmc for this release avoiding confusion.
The current status with DMA support can be re-enabled on a -next branch.

Any thoughts ?

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH] i.MX28: Allow coexistence of PIO and DMA mode for SD/MMC
  2012-04-05 12:12   ` Stefano Babic
@ 2012-04-05 12:17     ` Marek Vasut
  2012-04-05 12:28       ` Stefano Babic
  0 siblings, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2012-04-05 12:17 UTC (permalink / raw)
  To: u-boot

Dear Stefano Babic,

> On 05/04/2012 12:24, Marek Vasut wrote:
> > This SD DMA function of i.MX28 is still apparently too experimental to be
> > enabled by default in 2012.04 release. Enable this feature only if the
> > user plans to tinker with DCache or explicitly enables it.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Stefano Babic <sbabic@denx.de>
> > Cc: Wolfgang Denk <wd@denx.de>
> > Cc: Detlev Zundel <dzu@denx.de>
> > Cc: Fabio Estevam <fabio.estevam@freescale.com>
> > ---
> 
> Hi Marek,
> 
> >  drivers/mmc/mxsmmc.c |   41 ++++++++++++++++++++++++++++++++++++++++-
> >  1 files changed, 40 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/mmc/mxsmmc.c b/drivers/mmc/mxsmmc.c
> > index e8bad9d..7187796 100644
> > --- a/drivers/mmc/mxsmmc.c
> > +++ b/drivers/mmc/mxsmmc.c
> > @@ -66,8 +66,13 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> > struct mmc_data *data)
> > 
> >  	struct mx28_ssp_regs *ssp_regs = priv->regs;
> >  	uint32_t reg;
> >  	int timeout;
> > 
> > -	uint32_t data_count, cache_data_count;
> > +	uint32_t data_count;
> > 
> >  	uint32_t ctrl0;
> > 
> > +#ifndef CONFIG_MXS_MMC_DMA
> > +	uint32_t *data_ptr;
> > +#else
> > +	uint32_t cache_data_count;
> > +#endif
> > 
> >  	debug("MMC%d: CMD%d\n", mmc->block_dev.dev, cmd->cmdidx);
> > 
> > @@ -185,7 +190,9 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> > struct mmc_data *data)
> > 
> >  		return 0;
> >  	
> >  	data_count = data->blocksize * data->blocks;
> > 
> > +	timeout = MXSMMC_MAX_TIMEOUT;
> > 
> > +#ifdef CONFIG_MXS_MMC_DMA
> > 
> >  	if (data_count % ARCH_DMA_MINALIGN)
> >  	
> >  		cache_data_count = roundup(data_count, ARCH_DMA_MINALIGN);
> >  	
> >  	else
> > 
> > @@ -218,6 +225,38 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd
> > *cmd, struct mmc_data *data)
> > 
> >  		invalidate_dcache_range((uint32_t)priv->desc->cmd.address,
> >  		
> >  			(uint32_t)(priv->desc->cmd.address + cache_data_count));
> >  	
> >  	}
> > 
> > +#else
> > +	if (data->flags & MMC_DATA_READ) {
> > +		data_ptr = (uint32_t *)data->dest;
> > +		while (data_count && --timeout) {
> > +			reg = readl(&ssp_regs->hw_ssp_status);
> > +			if (!(reg & SSP_STATUS_FIFO_EMPTY)) {
> > +				*data_ptr++ = readl(&ssp_regs->hw_ssp_data);
> > +				data_count -= 4;
> > +				timeout = MXSMMC_MAX_TIMEOUT;
> > +			} else
> > +				udelay(1000);
> > +		}
> > +	} else {
> > +		data_ptr = (uint32_t *)data->src;
> > +		timeout *= 100;
> > +		while (data_count && --timeout) {
> > +			reg = readl(&ssp_regs->hw_ssp_status);
> > +			if (!(reg & SSP_STATUS_FIFO_FULL)) {
> > +				writel(*data_ptr++, &ssp_regs->hw_ssp_data);
> > +				data_count -= 4;
> > +				timeout = MXSMMC_MAX_TIMEOUT;
> > +			} else
> > +				udelay(1000);
> > +		}
> > +	}
> 
> Ok, I see. This patch reverts to the status before applying the DMA's
> patch. I would only ask if it makes sense to leave DMA support on MMC when
> we know that is buggy.

It's buggy on some cards, I suspect some timing issue.

> Is it DMA support working for NAND ? If yes, it is
> not possible to enable DMA for NAND and disable it for MMC, as both driver
> use the same CONFIG_MXS_MMC_DMA.

What do you mean? DMA is used for NAND ever since, I just recently tried 
enabling it also for MMC.

> Anyway, I am not saying we should introduce a new switch, but maybe we
> should drop DMA support in msx_mmc for this release avoiding confusion.
> The current status with DMA support can be re-enabled on a -next branch.

Maybe adding comment to that switch that it's experimental and that it should be 
off to avoid problems would be enough?

> Any thoughts ?
> 
> Best regards,
> Stefano Babic

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH] i.MX28: Allow coexistence of PIO and DMA mode for SD/MMC
  2012-04-05 12:17     ` Marek Vasut
@ 2012-04-05 12:28       ` Stefano Babic
  0 siblings, 0 replies; 13+ messages in thread
From: Stefano Babic @ 2012-04-05 12:28 UTC (permalink / raw)
  To: u-boot

On 05/04/2012 14:17, Marek Vasut wrote:

> 
> What do you mean? DMA is used for NAND ever since, I just recently tried 
> enabling it also for MMC.

Sorry, I have not read accurately - we have two different CONFIG_

>> Anyway, I am not saying we should introduce a new switch, but maybe we
>> should drop DMA support in msx_mmc for this release avoiding confusion.
>> The current status with DMA support can be re-enabled on a -next branch.
> 
> Maybe adding comment to that switch that it's experimental and that it should be 
> off to avoid problems would be enough?

Yes. We need to make clear that DMA on MMC is in a working status.

Stefano

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH V3] i.MX28: Allow coexistence of PIO and DMA mode for SD/MMC
  2012-04-05 10:24 ` [U-Boot] [PATCH] i.MX28: Allow coexistence of PIO and DMA mode for SD/MMC Marek Vasut
  2012-04-05 12:12   ` Stefano Babic
@ 2012-04-05 13:30   ` Marek Vasut
  2012-04-09 10:57     ` Stefano Babic
  1 sibling, 1 reply; 13+ messages in thread
From: Marek Vasut @ 2012-04-05 13:30 UTC (permalink / raw)
  To: u-boot

This SD DMA function of i.MX28 is still apparently too experimental to be
enabled by default in 2012.04 release. Enable this feature only if the user
plans to tinker with DCache or explicitly enables it.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Stefano Babic <sbabic@denx.de>
Cc: Wolfgang Denk <wd@denx.de>
Cc: Detlev Zundel <dzu@denx.de>
Cc: Fabio Estevam <fabio.estevam@freescale.com>
---
 drivers/mmc/mxsmmc.c |   48 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 files changed, 47 insertions(+), 1 deletions(-)

V2: Drop dependency on enabled data cache
V3: Add comment that this feature is experimental

diff --git a/drivers/mmc/mxsmmc.c b/drivers/mmc/mxsmmc.c
index e8bad9d..dfceaef 100644
--- a/drivers/mmc/mxsmmc.c
+++ b/drivers/mmc/mxsmmc.c
@@ -43,6 +43,13 @@
 #include <asm/arch/sys_proto.h>
 #include <asm/arch/dma.h>
 
+/*
+ * CONFIG_MXS_MMC_DMA: This feature is highly experimental and has no
+ *                     performance benefit unless you operate the platform with
+ *                     data cache enabled. This is disabled by default, enable
+ *                     only if you know what you're doing.
+ */
+
 struct mxsmmc_priv {
 	int			id;
 	struct mx28_ssp_regs	*regs;
@@ -66,8 +73,13 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 	struct mx28_ssp_regs *ssp_regs = priv->regs;
 	uint32_t reg;
 	int timeout;
-	uint32_t data_count, cache_data_count;
+	uint32_t data_count;
 	uint32_t ctrl0;
+#ifndef CONFIG_MXS_MMC_DMA
+	uint32_t *data_ptr;
+#else
+	uint32_t cache_data_count;
+#endif
 
 	debug("MMC%d: CMD%d\n", mmc->block_dev.dev, cmd->cmdidx);
 
@@ -185,7 +197,9 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 		return 0;
 
 	data_count = data->blocksize * data->blocks;
+	timeout = MXSMMC_MAX_TIMEOUT;
 
+#ifdef CONFIG_MXS_MMC_DMA
 	if (data_count % ARCH_DMA_MINALIGN)
 		cache_data_count = roundup(data_count, ARCH_DMA_MINALIGN);
 	else
@@ -218,6 +232,38 @@ mxsmmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd, struct mmc_data *data)
 		invalidate_dcache_range((uint32_t)priv->desc->cmd.address,
 			(uint32_t)(priv->desc->cmd.address + cache_data_count));
 	}
+#else
+	if (data->flags & MMC_DATA_READ) {
+		data_ptr = (uint32_t *)data->dest;
+		while (data_count && --timeout) {
+			reg = readl(&ssp_regs->hw_ssp_status);
+			if (!(reg & SSP_STATUS_FIFO_EMPTY)) {
+				*data_ptr++ = readl(&ssp_regs->hw_ssp_data);
+				data_count -= 4;
+				timeout = MXSMMC_MAX_TIMEOUT;
+			} else
+				udelay(1000);
+		}
+	} else {
+		data_ptr = (uint32_t *)data->src;
+		timeout *= 100;
+		while (data_count && --timeout) {
+			reg = readl(&ssp_regs->hw_ssp_status);
+			if (!(reg & SSP_STATUS_FIFO_FULL)) {
+				writel(*data_ptr++, &ssp_regs->hw_ssp_data);
+				data_count -= 4;
+				timeout = MXSMMC_MAX_TIMEOUT;
+			} else
+				udelay(1000);
+		}
+	}
+
+	if (!timeout) {
+		printf("MMC%d: Data timeout with command %d (status 0x%08x)!\n",
+			mmc->block_dev.dev, cmd->cmdidx, reg);
+		return COMM_ERR;
+	}
+#endif
 
 	/* Check data errors */
 	reg = readl(&ssp_regs->hw_ssp_status);
-- 
1.7.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH] i.MX28: Enable SD DMA only if DCache enabled
  2012-04-05  0:48   ` Marek Vasut
  2012-04-05  3:36     ` Fabio Estevam
@ 2012-04-06  0:34     ` Fabio Estevam
  2012-04-06  1:03       ` Marek Vasut
  1 sibling, 1 reply; 13+ messages in thread
From: Fabio Estevam @ 2012-04-06  0:34 UTC (permalink / raw)
  To: u-boot

On Wed, Apr 4, 2012 at 9:48 PM, Marek Vasut <marex@denx.de> wrote:

>
> I have some obscure card now and I'm seeing breakage (DMA transfer timeout).

Can you post more details, please? How do you reproduce the timeouts
and where exactly do they occur?

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH] i.MX28: Enable SD DMA only if DCache enabled
  2012-04-06  0:34     ` Fabio Estevam
@ 2012-04-06  1:03       ` Marek Vasut
  0 siblings, 0 replies; 13+ messages in thread
From: Marek Vasut @ 2012-04-06  1:03 UTC (permalink / raw)
  To: u-boot

Dear Fabio Estevam,

> On Wed, Apr 4, 2012 at 9:48 PM, Marek Vasut <marex@denx.de> wrote:
> > I have some obscure card now and I'm seeing breakage (DMA transfer
> > timeout).
> 
> Can you post more details, please? How do you reproduce the timeouts
> and where exactly do they occur?

Yes, can you remind me on april 9th? I won't have access to that until then ;-)

btw have a nice easter :)

Best regards,
Marek Vasut

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [U-Boot] [PATCH V3] i.MX28: Allow coexistence of PIO and DMA mode for SD/MMC
  2012-04-05 13:30   ` [U-Boot] [PATCH V3] " Marek Vasut
@ 2012-04-09 10:57     ` Stefano Babic
  0 siblings, 0 replies; 13+ messages in thread
From: Stefano Babic @ 2012-04-09 10:57 UTC (permalink / raw)
  To: u-boot

On 05/04/2012 15:30, Marek Vasut wrote:
> This SD DMA function of i.MX28 is still apparently too experimental to be
> enabled by default in 2012.04 release. Enable this feature only if the user
> plans to tinker with DCache or explicitly enables it.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Stefano Babic <sbabic@denx.de>
> Cc: Wolfgang Denk <wd@denx.de>
> Cc: Detlev Zundel <dzu@denx.de>
> Cc: Fabio Estevam <fabio.estevam@freescale.com>
> ---

Applied to u-boot-imx (fix), thanks.

Best regards,
Stefano Babic

-- 
=====================================================================
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic at denx.de
=====================================================================

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2012-04-09 10:57 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-04 21:51 [U-Boot] [PATCH] i.MX28: Enable SD DMA only if DCache enabled Marek Vasut
2012-04-04 23:12 ` Fabio Estevam
2012-04-05  0:48   ` Marek Vasut
2012-04-05  3:36     ` Fabio Estevam
2012-04-05 10:08       ` Marek Vasut
2012-04-06  0:34     ` Fabio Estevam
2012-04-06  1:03       ` Marek Vasut
2012-04-05 10:24 ` [U-Boot] [PATCH] i.MX28: Allow coexistence of PIO and DMA mode for SD/MMC Marek Vasut
2012-04-05 12:12   ` Stefano Babic
2012-04-05 12:17     ` Marek Vasut
2012-04-05 12:28       ` Stefano Babic
2012-04-05 13:30   ` [U-Boot] [PATCH V3] " Marek Vasut
2012-04-09 10:57     ` Stefano Babic

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox