[U-Boot] [PATCH] i.MX28: Allow coexistence of PIO and DMA mode for SD/MMC

[Marek Vasut]

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 at denx.de>
> > Cc: Stefano Babic <sbabic at denx.de>
> > Cc: Wolfgang Denk <wd at denx.de>
> > Cc: Detlev Zundel <dzu at denx.de>
> > Cc: Fabio Estevam <fabio.estevam at 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