| Submitter | Marek Vasut |
|---|---|
| Date | April 5, 2012, 10:24 a.m. |
| Message ID | <1333621458-6846-1-git-send-email-marex@denx.de> |
| Download | mbox | patch |
| Permalink | /patch/150904/ |
| State | Superseded |
| Headers | show |
Comments
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
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
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
Patch
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);
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(-)