| Message ID | 1333576270-8477-1-git-send-email-marex@denx.de |
|---|---|
| State | Changes Requested |
| Delegated to: | Stefano Babic |
| Headers | show |
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).
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
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.
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
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?
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
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);
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(-)