[U-Boot] [PATCH v1] sunxi: improve throughput in the sunxi_mmc driver

Jaehoon Chung jh80.chung at samsung.com
Sun Feb 19 22:08:05 UTC 2017


Hi,

On 02/18/2017 02:22 AM, Philipp Tomsich wrote:
> Throughput tests have shown the sunxi_mmc driver to take over 10s to
> read 10MB from a fast eMMC device due to excessive delays in polling
> loops.
> 
> This commit restructures the main polling loops to use get_timer(...)
> to determine whether a (millisecond) timeout has expired.  We choose
> not to use the wait_bit function, as we don't need interruptability
> with ctrl-c and have at least one case where two bits (one for an
> error condition and another one for completion) need to be read and
> using wait_bit would have not added to the clarity.
> 
> The observed speedup in testing on a A31 is greater than 10x (e.g. a
> 10MB write decreases from 9.302s to 0.884s).

Your patch's format looks strange.
Some unchanging codes are included in patches. 
Except them, Looks good to me. 
If you will resend the patch, you can resend with my acked-by tag.

Best Regards,
Jaehoon Chung

> 
> X-Affected-platforms: A31-uQ7, A80-Q7, A64-uQ7
> Signed-off-by: Philipp Tomsich <philipp.tomsich at theobroma-systems.com>
> ---
>  drivers/mmc/sunxi_mmc.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mmc/sunxi_mmc.c b/drivers/mmc/sunxi_mmc.c
> index d3106db..46abe4a 100644
> --- a/drivers/mmc/sunxi_mmc.c
> +++ b/drivers/mmc/sunxi_mmc.c
> @@ -180,22 +180,22 @@ static int mmc_clk_io_on(int sdc_no)
>  static int mmc_update_clk(struct mmc *mmc)
>  {
>  	struct sunxi_mmc_host *mmchost = mmc->priv;
>  	unsigned int cmd;
>  	unsigned timeout_msecs = 2000;
> +	unsigned long start = get_timer(0);
>  
>  	cmd = SUNXI_MMC_CMD_START |
>  	      SUNXI_MMC_CMD_UPCLK_ONLY |
>  	      SUNXI_MMC_CMD_WAIT_PRE_OVER;
>  	writel(cmd, &mmchost->reg->cmd);
>  	while (readl(&mmchost->reg->cmd) & SUNXI_MMC_CMD_START) {
> -		if (!timeout_msecs--)
> +		if (get_timer(start) > timeout_msecs)
>  			return -1;
> -		udelay(1000);
>  	}
>  
>  	/* clock update sets various irq status bits, clear these */
>  	writel(readl(&mmchost->reg->rint), &mmchost->reg->rint);
>  
>  	return 0;
>  }
>  
> @@ -265,161 +265,165 @@ static int sunxi_mmc_core_init(struct mmc *mmc)
>  static int mmc_trans_data_by_cpu(struct mmc *mmc, struct mmc_data *data)
>  {
>  	struct sunxi_mmc_host *mmchost = mmc->priv;
>  	const int reading = !!(data->flags & MMC_DATA_READ);
>  	const uint32_t status_bit = reading ? SUNXI_MMC_STATUS_FIFO_EMPTY :
>  					      SUNXI_MMC_STATUS_FIFO_FULL;
>  	unsigned i;
>  	unsigned *buff = (unsigned int *)(reading ? data->dest : data->src);
>  	unsigned byte_cnt = data->blocksize * data->blocks;
> -	unsigned timeout_usecs = (byte_cnt >> 8) * 1000;
> -	if (timeout_usecs < 2000000)
> -		timeout_usecs = 2000000;
> +	unsigned timeout_msecs = byte_cnt >> 8;
> +	unsigned long  start;
> +
> +	if (timeout_msecs < 2000)
> +		timeout_msecs = 2000;
>  
>  	/* Always read / write data through the CPU */
>  	setbits_le32(&mmchost->reg->gctrl, SUNXI_MMC_GCTRL_ACCESS_BY_AHB);
>  
> +	start = get_timer(0);
> +
>  	for (i = 0; i < (byte_cnt >> 2); i++) {
>  		while (readl(&mmchost->reg->status) & status_bit) {
> -			if (!timeout_usecs--)
> +			if (get_timer(start) > timeout_msecs)
>  				return -1;
> -			udelay(1);
>  		}
>  
>  		if (reading)
>  			buff[i] = readl(&mmchost->reg->fifo);
>  		else
>  			writel(buff[i], &mmchost->reg->fifo);
>  	}
>  
>  	return 0;
>  }
>  
>  static int mmc_rint_wait(struct mmc *mmc, unsigned int timeout_msecs,
>  			 unsigned int done_bit, const char *what)
>  {
>  	struct sunxi_mmc_host *mmchost = mmc->priv;
>  	unsigned int status;
> +	unsigned long start = get_timer(0);
>  
>  	do {
>  		status = readl(&mmchost->reg->rint);
> -		if (!timeout_msecs-- ||
> +		if ((get_timer(start) > timeout_msecs) ||
>  		    (status & SUNXI_MMC_RINT_INTERRUPT_ERROR_BIT)) {
>  			debug("%s timeout %x\n", what,
>  			      status & SUNXI_MMC_RINT_INTERRUPT_ERROR_BIT);
>  			return -ETIMEDOUT;
>  		}
> -		udelay(1000);
>  	} while (!(status & done_bit));
>  
>  	return 0;
>  }
>  
>  static int sunxi_mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>  			      struct mmc_data *data)
>  {
>  	struct sunxi_mmc_host *mmchost = mmc->priv;
>  	unsigned int cmdval = SUNXI_MMC_CMD_START;
>  	unsigned int timeout_msecs;
>  	int error = 0;
>  	unsigned int status = 0;
>  	unsigned int bytecnt = 0;
>  
>  	if (mmchost->fatal_err)
>  		return -1;
>  	if (cmd->resp_type & MMC_RSP_BUSY)
>  		debug("mmc cmd %d check rsp busy\n", cmd->cmdidx);
>  	if (cmd->cmdidx == 12)
>  		return 0;
>  
>  	if (!cmd->cmdidx)
>  		cmdval |= SUNXI_MMC_CMD_SEND_INIT_SEQ;
>  	if (cmd->resp_type & MMC_RSP_PRESENT)
>  		cmdval |= SUNXI_MMC_CMD_RESP_EXPIRE;
>  	if (cmd->resp_type & MMC_RSP_136)
>  		cmdval |= SUNXI_MMC_CMD_LONG_RESPONSE;
>  	if (cmd->resp_type & MMC_RSP_CRC)
>  		cmdval |= SUNXI_MMC_CMD_CHK_RESPONSE_CRC;
>  
>  	if (data) {
>  		if ((u32)(long)data->dest & 0x3) {
>  			error = -1;
>  			goto out;
>  		}
>  
>  		cmdval |= SUNXI_MMC_CMD_DATA_EXPIRE|SUNXI_MMC_CMD_WAIT_PRE_OVER;
>  		if (data->flags & MMC_DATA_WRITE)
>  			cmdval |= SUNXI_MMC_CMD_WRITE;
>  		if (data->blocks > 1)
>  			cmdval |= SUNXI_MMC_CMD_AUTO_STOP;
>  		writel(data->blocksize, &mmchost->reg->blksz);
>  		writel(data->blocks * data->blocksize, &mmchost->reg->bytecnt);
>  	}
>  
>  	debug("mmc %d, cmd %d(0x%08x), arg 0x%08x\n", mmchost->mmc_no,
>  	      cmd->cmdidx, cmdval | cmd->cmdidx, cmd->cmdarg);
>  	writel(cmd->cmdarg, &mmchost->reg->arg);
>  
>  	if (!data)
>  		writel(cmdval | cmd->cmdidx, &mmchost->reg->cmd);
>  
>  	/*
>  	 * transfer data and check status
>  	 * STATREG[2] : FIFO empty
>  	 * STATREG[3] : FIFO full
>  	 */
>  	if (data) {
>  		int ret = 0;
>  
>  		bytecnt = data->blocksize * data->blocks;
>  		debug("trans data %d bytes\n", bytecnt);
>  		writel(cmdval | cmd->cmdidx, &mmchost->reg->cmd);
>  		ret = mmc_trans_data_by_cpu(mmc, data);
>  		if (ret) {
>  			error = readl(&mmchost->reg->rint) & \
>  				SUNXI_MMC_RINT_INTERRUPT_ERROR_BIT;
>  			error = -ETIMEDOUT;
>  			goto out;
>  		}
>  	}
>  
>  	error = mmc_rint_wait(mmc, 1000, SUNXI_MMC_RINT_COMMAND_DONE, "cmd");
>  	if (error)
>  		goto out;
>  
>  	if (data) {
>  		timeout_msecs = 120;
>  		debug("cacl timeout %x msec\n", timeout_msecs);
>  		error = mmc_rint_wait(mmc, timeout_msecs,
>  				      data->blocks > 1 ?
>  				      SUNXI_MMC_RINT_AUTO_COMMAND_DONE :
>  				      SUNXI_MMC_RINT_DATA_OVER,
>  				      "data");
>  		if (error)
>  			goto out;
>  	}
>  
>  	if (cmd->resp_type & MMC_RSP_BUSY) {
> +		unsigned long start = get_timer(0);
>  		timeout_msecs = 2000;
> +
>  		do {
>  			status = readl(&mmchost->reg->status);
> -			if (!timeout_msecs--) {
> +			if (get_timer(start) > timeout_msecs) {
>  				debug("busy timeout\n");
>  				error = -ETIMEDOUT;
>  				goto out;
>  			}
> -			udelay(1000);
>  		} while (status & SUNXI_MMC_STATUS_CARD_DATA_BUSY);
>  	}
>  
>  	if (cmd->resp_type & MMC_RSP_136) {
>  		cmd->response[0] = readl(&mmchost->reg->resp3);
>  		cmd->response[1] = readl(&mmchost->reg->resp2);
>  		cmd->response[2] = readl(&mmchost->reg->resp1);
>  		cmd->response[3] = readl(&mmchost->reg->resp0);
>  		debug("mmc resp 0x%08x 0x%08x 0x%08x 0x%08x\n",
>  		      cmd->response[3], cmd->response[2],
>  		      cmd->response[1], cmd->response[0]);
>  	} else {
>  		cmd->response[0] = readl(&mmchost->reg->resp0);
>  		debug("mmc resp 0x%08x\n", cmd->response[0]);
>  	}
> 



More information about the U-Boot mailing list