On 22/11/13 14:24, Vladimir Zapolskiy wrote:
> On 22.11.2013 12:38, Adrian Hunter wrote:
>> On 21/11/13 17:07, Vladimir Zapolskiy wrote:
>>> JEDEC specification defines quite high erase timeout value for 300ms
>>> multiplied by erase group number, and SD Host Controller specification
>>> data line timeout may be much less, e.g. 2^13 / 52MHz ~ 160us.
>>>
>>> > From block layer and MMC perfromance perspective it is desirable that
>>> millions of erase groups are discarded at once, so there is no much
>>> sense to limit maximum erase timeout by data line timeout, if a
>>> controller handles correctly erase operation without indication of
>>> data line timeout.
>>
>> Would you explain that some more. Do you mean that:
>> a) it does not have a timeout
>
> JEDEC defines a timeout on erase/trim operations, also in
> drivers/mmc/core/core.c
> there is a reasonable enough 10 minutes limitation for discard operations.
>
>> b) it has a timeout which is less than the timeout specified by the
>> standard but the operation nevertheless completes
>
> SDHC data line timeout is enormously less than erase group timeout, and
> trivial testing shows that those two timeouts are independent, probably
> except some particular cases of controllers not known before commits
> 58d1246db3 and e056a1b5b.
>
> According to the currently implemented logic, mmc_do_erase() commonly is
> instructed to discard 1-2 erase groups at maximum, however it tends to be
> capable to successfully discard millions of erase groups at once ignoring
> that SDHC data line timeout limitation.
>
You seem to be trying to say that the SDHCI spec. says that the host
controller does not timeout erase operations or uses a different timeout
than the one programmed in the "Timeout Control Register". Where is that is
the SDHCI spec?
>>>
>>> Potentially the change may break some of the SDHCs on discard of mmc,
>>> and for backward compatibility a new quirk is introduced, which is NOT
>>> set by default.
>>
>> It sounds to me that what you want to do is not standard so the quirk should
>> be the other way around.
>
> Please take a look at commits 58d1246db3 and e056a1b5b, I'd be glad, if you
> could elaborate to which "some host controllers" the quirk in my definition
> applies, I believe all other host controllers present at that time in
> drivers/mmc/host/* are capable to discard without introduced limitation.
"some host controllers" == SDHCI i.e. to all of the ones you are applying
the change.
>
>>>
>>> Signed-off-by: Vladimir Zapolskiy<vladimir_zapolskiy@xxxxxxxxxx>
>>> Reported-by: Ed Sutter<ed.sutter@xxxxxxxxxxxxxxxxxx>
>>> Cc: Chris Ball<cjb@xxxxxxxxxx>
>>> Cc: Adrian Hunter<adrian.hunter@xxxxxxxxx>
>>> ---
>>> drivers/mmc/host/sdhci.c | 5 ++++-
>>> include/linux/mmc/sdhci.h | 1 +
>>> 2 files changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
>>> index bd8a098..b1fdddb 100644
>>> --- a/drivers/mmc/host/sdhci.c
>>> +++ b/drivers/mmc/host/sdhci.c
>>> @@ -2930,7 +2930,10 @@ int sdhci_add_host(struct sdhci_host *host)
>>> if (host->quirks& SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK)
>>> host->timeout_clk = mmc->f_max / 1000;
>>>
>>> - mmc->max_discard_to = (1<< 27) / host->timeout_clk;
>>> + if (host->quirks2& SDHCI_QUIRK2_DATA_TIMEOUT_ON_DISCARD)
>>> + mmc->max_discard_to = (1<< 27) / host->timeout_clk;
>>> + else
>>> + mmc->max_discard_to = 0;
>>>
>>> mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
>>>
>>> diff --git a/include/linux/mmc/sdhci.h b/include/linux/mmc/sdhci.h
>>> index 3e781b8..e7f6bd2 100644
>>> --- a/include/linux/mmc/sdhci.h
>>> +++ b/include/linux/mmc/sdhci.h
>>> @@ -98,6 +98,7 @@ struct sdhci_host {
>>> #define SDHCI_QUIRK2_CARD_ON_NEEDS_BUS_ON (1<<4)
>>> /* Controller has a non-standard host control register */
>>> #define SDHCI_QUIRK2_BROKEN_HOST_CONTROL (1<<5)
>>> +#define SDHCI_QUIRK2_DATA_TIMEOUT_ON_DISCARD (1<<6)
>>>
>>> int irq; /* Device IRQ */
>>> void __iomem *ioaddr; /* Mapped address */
>>>
>>
>
>
--
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
