| Message ID | 20241008191834.2190270-1-jonas@kwiboo.se |
|---|---|
| State | Accepted |
| Commit | bbfa4587b5bdc465dc0c9a739591674b7eedb104 |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | [v2] Revert "mmc: dw_mmc: Extract FIFO data transfer into a separate routine" | expand |
On Tue, 8 Oct 2024 at 13:18, Jonas Karlman <jonas@kwiboo.se> wrote: > > The commit 0252924ac6d4 ("mmc: dw_mmc: Extract FIFO data transfer into a > separate routine") unintentionally changed behavior of the FIFO data > transfer routine. > > When data is read and size reaches 0 the original loop would wait on > DWMCI_INTMSK_DTO or timeout. The remaining size to read and buf position > is no longer tracked across dwmci_data_transfer_fifo() calls and because > of this an extra call to fifo() and dwmci_fifo_ready() may now trigger a > FIFO underflow timeout error and slows down FIFO reading. > > Buswidth = 4, clock: 50000000 > Sending CMD16 > Sending CMD17 > dwmci_fifo_ready: FIFO underflow timeout > Sending CMD16 > Sending CMD18 > dwmci_fifo_ready: FIFO underflow timeout > Sending CMD12 > ## Checking hash(es) for config config-1 ... OK > > This reverts commit 0252924ac6d4af69061bb9589d16b30c5bdb7178 to restore > the old working behavior. > > Fixes: 0252924ac6d4 ("mmc: dw_mmc: Extract FIFO data transfer into a separate routine") > Signed-off-by: Jonas Karlman <jonas@kwiboo.se> > --- > Changes in v2: > - Change to revert the offending commit, both size and buf where reset > between fifo() calls, easier to do a full revert. > - R-b tag not collected because of this being a different patch. > > Reading FIT from SD-card take ~20-30 seconds on a Rockchip RK3328 board > without this reverted, and goes back to sub second once reverted. > --- > drivers/mmc/dw_mmc.c | 103 ++++++++++++++++++++++--------------------- > 1 file changed, 52 insertions(+), 51 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org>
Hi Jonas, On 10/8/24 9:18 PM, Jonas Karlman wrote: > The commit 0252924ac6d4 ("mmc: dw_mmc: Extract FIFO data transfer into a > separate routine") unintentionally changed behavior of the FIFO data > transfer routine. > > When data is read and size reaches 0 the original loop would wait on > DWMCI_INTMSK_DTO or timeout. The remaining size to read and buf position > is no longer tracked across dwmci_data_transfer_fifo() calls and because > of this an extra call to fifo() and dwmci_fifo_ready() may now trigger a > FIFO underflow timeout error and slows down FIFO reading. > > Buswidth = 4, clock: 50000000 > Sending CMD16 > Sending CMD17 > dwmci_fifo_ready: FIFO underflow timeout > Sending CMD16 > Sending CMD18 > dwmci_fifo_ready: FIFO underflow timeout > Sending CMD12 > ## Checking hash(es) for config config-1 ... OK > > This reverts commit 0252924ac6d4af69061bb9589d16b30c5bdb7178 to restore > the old working behavior. > > Fixes: 0252924ac6d4 ("mmc: dw_mmc: Extract FIFO data transfer into a separate routine") > Signed-off-by: Jonas Karlman <jonas@kwiboo.se> This is fixing a VERY slow boot time on my RK3588 Tiger, so: Tested-by: Quentin Schulz <quentin.schulz@cherry.de> # RK3588 Tiger Thanks! Quentin
On Tue, 08 Oct 2024 19:18:31 +0000, Jonas Karlman wrote: > The commit 0252924ac6d4 ("mmc: dw_mmc: Extract FIFO data transfer into a > separate routine") unintentionally changed behavior of the FIFO data > transfer routine. > > When data is read and size reaches 0 the original loop would wait on > DWMCI_INTMSK_DTO or timeout. The remaining size to read and buf position > is no longer tracked across dwmci_data_transfer_fifo() calls and because > of this an extra call to fifo() and dwmci_fifo_ready() may now trigger a > FIFO underflow timeout error and slows down FIFO reading. > > [...] Applied to u-boot/master, thanks!
diff --git a/drivers/mmc/dw_mmc.c b/drivers/mmc/dw_mmc.c index 8551eac70185..e1110cace897 100644 --- a/drivers/mmc/dw_mmc.c +++ b/drivers/mmc/dw_mmc.c @@ -214,66 +214,24 @@ static unsigned int dwmci_get_timeout(struct mmc *mmc, const unsigned int size) return timeout; } -static int dwmci_data_transfer_fifo(struct dwmci_host *host, - struct mmc_data *data, u32 mask) +static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data) { - const u32 int_rx = mask & (DWMCI_INTMSK_RXDR | DWMCI_INTMSK_DTO); - const u32 int_tx = mask & DWMCI_INTMSK_TXDR; + struct mmc *mmc = host->mmc; int ret = 0; - u32 len = 0, size, i; - u32 *buf; - - size = (data->blocksize * data->blocks) / 4; - if (!host->fifo_mode || !size) - return 0; + u32 timeout, mask, size, i, len = 0; + u32 *buf = NULL; + ulong start = get_timer(0); + size = data->blocksize * data->blocks; if (data->flags == MMC_DATA_READ) buf = (unsigned int *)data->dest; else buf = (unsigned int *)data->src; - if (data->flags == MMC_DATA_READ && int_rx) { - dwmci_writel(host, DWMCI_RINTSTS, int_rx); - while (size) { - ret = dwmci_fifo_ready(host, DWMCI_FIFO_EMPTY, &len); - if (ret < 0) - break; - - len = (len >> DWMCI_FIFO_SHIFT) & DWMCI_FIFO_MASK; - len = min(size, len); - for (i = 0; i < len; i++) - *buf++ = dwmci_readl(host, DWMCI_DATA); - size = size > len ? (size - len) : 0; - } - } else if (data->flags == MMC_DATA_WRITE && int_tx) { - while (size) { - ret = dwmci_fifo_ready(host, DWMCI_FIFO_FULL, &len); - if (ret < 0) - break; - - len = host->fifo_depth - ((len >> DWMCI_FIFO_SHIFT) & - DWMCI_FIFO_MASK); - len = min(size, len); - for (i = 0; i < len; i++) - dwmci_writel(host, DWMCI_DATA, *buf++); - size = size > len ? (size - len) : 0; - } - dwmci_writel(host, DWMCI_RINTSTS, DWMCI_INTMSK_TXDR); - } - - return ret; -} - -static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data) -{ - struct mmc *mmc = host->mmc; - int ret = 0; - u32 timeout, mask, size; - ulong start = get_timer(0); - - size = data->blocksize * data->blocks; timeout = dwmci_get_timeout(mmc, size); + size /= 4; + for (;;) { mask = dwmci_readl(host, DWMCI_RINTSTS); /* Error during data transfer */ @@ -283,7 +241,50 @@ static int dwmci_data_transfer(struct dwmci_host *host, struct mmc_data *data) break; } - ret = dwmci_data_transfer_fifo(host, data, mask); + if (host->fifo_mode && size) { + len = 0; + if (data->flags == MMC_DATA_READ && + (mask & (DWMCI_INTMSK_RXDR | DWMCI_INTMSK_DTO))) { + dwmci_writel(host, DWMCI_RINTSTS, + mask & (DWMCI_INTMSK_RXDR | + DWMCI_INTMSK_DTO)); + while (size) { + ret = dwmci_fifo_ready(host, + DWMCI_FIFO_EMPTY, + &len); + if (ret < 0) + break; + + len = (len >> DWMCI_FIFO_SHIFT) & + DWMCI_FIFO_MASK; + len = min(size, len); + for (i = 0; i < len; i++) + *buf++ = + dwmci_readl(host, DWMCI_DATA); + size = size > len ? (size - len) : 0; + } + } else if (data->flags == MMC_DATA_WRITE && + (mask & DWMCI_INTMSK_TXDR)) { + while (size) { + ret = dwmci_fifo_ready(host, + DWMCI_FIFO_FULL, + &len); + if (ret < 0) + break; + + len = host->fifo_depth - ((len >> + DWMCI_FIFO_SHIFT) & + DWMCI_FIFO_MASK); + len = min(size, len); + for (i = 0; i < len; i++) + dwmci_writel(host, DWMCI_DATA, + *buf++); + size = size > len ? (size - len) : 0; + } + dwmci_writel(host, DWMCI_RINTSTS, + DWMCI_INTMSK_TXDR); + } + } /* Data arrived correctly */ if (mask & DWMCI_INTMSK_DTO) {
The commit 0252924ac6d4 ("mmc: dw_mmc: Extract FIFO data transfer into a separate routine") unintentionally changed behavior of the FIFO data transfer routine. When data is read and size reaches 0 the original loop would wait on DWMCI_INTMSK_DTO or timeout. The remaining size to read and buf position is no longer tracked across dwmci_data_transfer_fifo() calls and because of this an extra call to fifo() and dwmci_fifo_ready() may now trigger a FIFO underflow timeout error and slows down FIFO reading. Buswidth = 4, clock: 50000000 Sending CMD16 Sending CMD17 dwmci_fifo_ready: FIFO underflow timeout Sending CMD16 Sending CMD18 dwmci_fifo_ready: FIFO underflow timeout Sending CMD12 ## Checking hash(es) for config config-1 ... OK This reverts commit 0252924ac6d4af69061bb9589d16b30c5bdb7178 to restore the old working behavior. Fixes: 0252924ac6d4 ("mmc: dw_mmc: Extract FIFO data transfer into a separate routine") Signed-off-by: Jonas Karlman <jonas@kwiboo.se> --- Changes in v2: - Change to revert the offending commit, both size and buf where reset between fifo() calls, easier to do a full revert. - R-b tag not collected because of this being a different patch. Reading FIT from SD-card take ~20-30 seconds on a Rockchip RK3328 board without this reverted, and goes back to sub second once reverted. --- drivers/mmc/dw_mmc.c | 103 ++++++++++++++++++++++--------------------- 1 file changed, 52 insertions(+), 51 deletions(-)