On 12/19/2013 02:01 AM, Adrian Hunter wrote: > On 19/12/13 01:00, Stephen Warren wrote: >> On 12/18/2013 03:27 PM, Stephen Warren wrote: >>> From: Stephen Warren <swarren@xxxxxxxxxx> >>> >>> In mmc_do_calc_max_discard(), if only a single erase block can be >>> discarded within the host controller's timeout, don't allow discard >>> operations at all. >>> >>> Previously, the code allowed sector-at-a-time discard (rather than >>> erase-block-at-a-time), which was chronically slow. >>> >>> Without this patch, on the NVIDIA Tegra Cardhu board, the loops result >>> in qty == 1, which is immediately returned. This causes discard to >>> operate a single sector at a time, which is chronically slow. With this >>> patch in place, discard operates a single erase block at a time, which >>> is reasonably fast. >> >> Alternatively, is the real fix a revert of e056a1b5b67b "mmc: queue: let >> host controllers specify maximum discard timeout", followed by: >> >>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >>> index 050eb262485c..35c5b5d86c99 100644 >>> --- a/drivers/mmc/core/core.c >>> +++ b/drivers/mmc/core/core.c >>> @@ -1950,7 +1950,6 @@ static int mmc_do_erase(struct mmc_card *card, unsigned int from, >>> cmd.opcode = MMC_ERASE; >>> cmd.arg = arg; >>> cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; >>> - cmd.cmd_timeout_ms = mmc_erase_timeout(card, arg, qty); >>> err = mmc_wait_for_cmd(card->host, &cmd, 0); >>> if (err) { >>> pr_err("mmc_erase: erase error %d, status %#x\n", >>> @@ -1962,7 +1961,7 @@ static int mmc_do_erase(struct mmc_card *card, unsigned int from, >>> if (mmc_host_is_spi(card->host)) >>> goto out; >>> >>> - timeout = jiffies + msecs_to_jiffies(MMC_CORE_TIMEOUT_MS); >>> + timeout = jiffies + msecs_to_jiffies(mmc_erase_timeout(card, arg, qty)); >>> do { >>> memset(&cmd, 0, sizeof(struct mmc_command)); >>> cmd.opcode = MMC_SEND_STATUS; >> >> That certainly also seems to solve the problem on my board... > > But large erases will timeout when they should have been split into smaller > chunks. > > A generic solution needs to be able to explain what happens when the host > controller *does* timeout. I thought the whole point of this discussion was that the timeout in the first code segment above only applies to command submission, whereas completion of the erase operation itself was determined by polling using CMD13, which still uses mmc_erase_timeout(). As such, I don't /think/ that "large erases will timeout" here, unless I'm significantly misunderstanding something? -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html