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

Chen-Yu Tsai wens at csie.org
Mon Feb 20 06:16:11 UTC 2017


On Mon, Feb 20, 2017 at 6:08 AM, Jaehoon Chung <jh80.chung at samsung.com> wrote:
> 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.

They were probably produced the patches with "git format-patch
--function-context", which includes the whole body of any changed
functions.

I suppose git would ignore all the extra code when applying the patch,
but it probably increases the likelihood of conflicts.

ChenYu

>
> 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