| Message ID | 20190323175527.5314-1-marex@denx.de |
|---|---|
| State | Deferred |
| Delegated to: | Marek Vasut |
| Headers | show |
| Series | [U-Boot] mmc: dw_mmc: Calculate timeout from transfer length | expand |
Hi Marek, On Sat, 23 Mar 2019 at 11:55, Marek Vasut <marex@denx.de> wrote: > > The current 4-minute data transfer timeout is misleading and broken. > Instead of such a long wait, calculate the timeout duration based on > the length of the data transfer. The current formula is the transfer > length in bits, divided by a multiplication of bus frequency in Hz, > bus width, DDR mode and converted the mSec. The value is bounded from > the bottom to 1000 mSec. > > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: Jaehoon Chung <jh80.chung@samsung.com> > Cc: Simon Glass <sjg@chromium.org> > --- > drivers/mmc/dw_mmc.c | 15 ++++++++++++--- > 1 file changed, 12 insertions(+), 3 deletions(-) > > diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c > index 93a836eac3..8278f1dacb 100644 > --- a/drivers/mmc/dw_mmc.c > +++ b/drivers/mmc/dw_mmc.c > @@ -116,20 +116,29 @@ static int dwmci_fifo_ready(struct dwmci_host *host, u32 bit, u32 *len) > > static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data) > { > + struct mmc *mmc = host->mmc; > int ret = 0; > - u32 timeout = 240000; > - u32 mask, size, i, len = 0; > + u32 timeout, mask, size, i, len = 0; > u32 *buf = NULL; > ulong start = get_timer(0); > u32 fifo_depth = (((host->fifoth_val & RX_WMARK_MASK) >> > RX_WMARK_SHIFT) + 1) * 2; > > - size = data->blocksize * data->blocks / 4; > + size = data->blocksize * data->blocks; > if (data->flags == MMC_DATA_READ) > buf = (unsigned int *)data->dest; > else > buf = (unsigned int *)data->src; > > + timeout = size * 8 * 1000; /* counting in bits and msec */ > + timeout *= 2; /* wait twice as long */ > + timeout /= mmc->clock; > + timeout /= mmc->bus_width; > + timeout /= mmc->ddr_mode ? 2 : 1; > + timeout = (timeout < 1000) ? 1000 : timeout; Can you put all this in a function (like calc_timeout_ms(size, clock, bus_width, ddr_mode)) that returns the correct timeout given the parameters? That would provide a modicum of documentation. > + > + size /= 4; > + > for (;;) { > mask = dwmci_readl(host, DWMCI_RINTSTS); > /* Error during data transfer. */ > -- > 2.20.1 > Regards, Simon
On 3/30/19 10:18 PM, Simon Glass wrote: > Hi Marek, > > On Sat, 23 Mar 2019 at 11:55, Marek Vasut <marex@denx.de> wrote: >> >> The current 4-minute data transfer timeout is misleading and broken. >> Instead of such a long wait, calculate the timeout duration based on >> the length of the data transfer. The current formula is the transfer >> length in bits, divided by a multiplication of bus frequency in Hz, >> bus width, DDR mode and converted the mSec. The value is bounded from >> the bottom to 1000 mSec. >> >> Signed-off-by: Marek Vasut <marex@denx.de> >> Cc: Jaehoon Chung <jh80.chung@samsung.com> >> Cc: Simon Glass <sjg@chromium.org> >> --- >> drivers/mmc/dw_mmc.c | 15 ++++++++++++--- >> 1 file changed, 12 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c >> index 93a836eac3..8278f1dacb 100644 >> --- a/drivers/mmc/dw_mmc.c >> +++ b/drivers/mmc/dw_mmc.c >> @@ -116,20 +116,29 @@ static int dwmci_fifo_ready(struct dwmci_host *host, u32 bit, u32 *len) >> >> static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data) >> { >> + struct mmc *mmc = host->mmc; >> int ret = 0; >> - u32 timeout = 240000; >> - u32 mask, size, i, len = 0; >> + u32 timeout, mask, size, i, len = 0; >> u32 *buf = NULL; >> ulong start = get_timer(0); >> u32 fifo_depth = (((host->fifoth_val & RX_WMARK_MASK) >> >> RX_WMARK_SHIFT) + 1) * 2; >> >> - size = data->blocksize * data->blocks / 4; >> + size = data->blocksize * data->blocks; >> if (data->flags == MMC_DATA_READ) >> buf = (unsigned int *)data->dest; >> else >> buf = (unsigned int *)data->src; >> >> + timeout = size * 8 * 1000; /* counting in bits and msec */ >> + timeout *= 2; /* wait twice as long */ >> + timeout /= mmc->clock; >> + timeout /= mmc->bus_width; >> + timeout /= mmc->ddr_mode ? 2 : 1; >> + timeout = (timeout < 1000) ? 1000 : timeout; > > Can you put all this in a function (like calc_timeout_ms(size, clock, > bus_width, ddr_mode)) that returns the correct timeout given the > parameters? That would provide a modicum of documentation. Sure
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 93a836eac3..8278f1dacb 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -116,20 +116,29 @@ static int dwmci_fifo_ready(struct dwmci_host *host, u32 bit, u32 *len) static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data) { + struct mmc *mmc = host->mmc; int ret = 0; - u32 timeout = 240000; - u32 mask, size, i, len = 0; + u32 timeout, mask, size, i, len = 0; u32 *buf = NULL; ulong start = get_timer(0); u32 fifo_depth = (((host->fifoth_val & RX_WMARK_MASK) >> RX_WMARK_SHIFT) + 1) * 2; - size = data->blocksize * data->blocks / 4; + size = data->blocksize * data->blocks; if (data->flags == MMC_DATA_READ) buf = (unsigned int *)data->dest; else buf = (unsigned int *)data->src; + timeout = size * 8 * 1000; /* counting in bits and msec */ + timeout *= 2; /* wait twice as long */ + timeout /= mmc->clock; + timeout /= mmc->bus_width; + timeout /= mmc->ddr_mode ? 2 : 1; + timeout = (timeout < 1000) ? 1000 : timeout; + + size /= 4; + for (;;) { mask = dwmci_readl(host, DWMCI_RINTSTS); /* Error during data transfer. */
The current 4-minute data transfer timeout is misleading and broken. Instead of such a long wait, calculate the timeout duration based on the length of the data transfer. The current formula is the transfer length in bits, divided by a multiplication of bus frequency in Hz, bus width, DDR mode and converted the mSec. The value is bounded from the bottom to 1000 mSec. Signed-off-by: Marek Vasut <marex@denx.de> Cc: Jaehoon Chung <jh80.chung@samsung.com> Cc: Simon Glass <sjg@chromium.org> --- drivers/mmc/dw_mmc.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)