Re: [PATCH] mmc: core: don't return 1 for max_discard

[Adrian Hunter <adrian.hunter-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

On 19/12/13 11:14, Vladimir Zapolskiy wrote:
> On 12/19/13 10:01, 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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
>>>>
>>>> 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.
> 
> Please correct me, but if Data Timeout Error is disabled, then this is not
> an issue for most of the host controllers.

That is a very good point.  My experience with SDHCI was that masking the
"Data Timeout Error Status Enable" and "Data Timeout Error Signal Enable
" bits did not disable the timeout i.e. the host controller would not
deliver a TC interrupt if the erase exceeded the timeout.

What happens on your board?