On 23 January 2014 11:23, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote:
> On 22/01/14 17:00, Ulf Hansson wrote:
>> When sending the sleep command for host drivers supporting
>> MMC_CAP_WAIT_WHILE_BUSY, we need to confirm that max_busy_timeout is
>> big enough comparing to the sleep timeout specified from card's
>> EXT_CSD. If this isn't case, we use a R1 response instead of R1B and
>> fallback to use a delay instead.
>>
>> Do note that a max_busy_timeout set to zero by the host, is interpreted
>> as it can cope with whatever timeout the mmc core provides it with.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx>
>> ---
>> drivers/mmc/core/mmc.c | 19 ++++++++++++++++---
>> 1 file changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index 897fdd1..32e1546 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1359,6 +1359,8 @@ static int mmc_sleep(struct mmc_host *host)
>> {
>> struct mmc_command cmd = {0};
>> struct mmc_card *card = host->card;
>> + unsigned int timeout_ms = DIV_ROUND_UP(card->ext_csd.sa_timeout, 10000);
>> + unsigned int max_busy_timeout;
>> int err;
>>
>> if (host->caps2 & MMC_CAP2_NO_SLEEP_CMD)
>> @@ -1372,7 +1374,18 @@ static int mmc_sleep(struct mmc_host *host)
>> cmd.arg = card->rca << 16;
>> cmd.arg |= 1 << 15;
>>
>> - cmd.flags = MMC_RSP_R1B | MMC_CMD_AC;
>> + /* We interpret unspecified timeouts as the host can cope with all. */
>> + max_busy_timeout = host->max_busy_timeout ?
>> + host->max_busy_timeout : timeout_ms;
>> +
>> + if ((host->caps & MMC_CAP_WAIT_WHILE_BUSY) &&
>> + (timeout_ms <= max_busy_timeout)) {
>> + cmd.flags = MMC_RSP_R1B | MMC_CMD_AC;
>> + cmd.busy_timeout = timeout_ms;
>> + } else {
>> + cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
>> + }
>
> I do not see why this is related to MMC_CAP_WAIT_WHILE_BUSY.
> Why not just:
Before I do any update, we need to decide what host->max_busy_timeout
of zero means. Please see the response in the other patch in this
patchset.
I see that my patch for the mmc_switch function, maintain the R1B for
host not supporting MMC_CAP_WAIT_WHILE_BUSY, but this one for sleep
doesn't. :-) We should align the behaviour.
>
> if (host->max_busy_timeout && timeout_ms > host->max_busy_timeout) {
> cmd.flags = MMC_RSP_R1 | MMC_CMD_AC;
> } else {
> cmd.flags = MMC_RSP_R1B | MMC_CMD_AC;
> cmd.busy_timeout = timeout_ms;
> }
So here your suggestion will mean you would like to keep R1B for hosts
not supporting MMC_CAP_WAIT_WHILE_BUSY. This opposite of what you
proposed for the mmc_switch. :-)
I suggest that we only use R1B when the host are able to handle busy
detection in hw. If you think that is bad idea, please let me know.
>
>> +
>> err = mmc_wait_for_cmd(host, &cmd, 0);
>> if (err)
>> return err;
>> @@ -1383,8 +1396,8 @@ static int mmc_sleep(struct mmc_host *host)
>> * SEND_STATUS command to poll the status because that command (and most
>> * others) is invalid while the card sleeps.
>> */
>> - if (!(host->caps & MMC_CAP_WAIT_WHILE_BUSY))
>> - mmc_delay(DIV_ROUND_UP(card->ext_csd.sa_timeout, 10000));
>> + if (!cmd.busy_timeout)
>> + mmc_delay(timeout_ms);
>
> And this becomes:
>
> if (!cmd.busy_timeout || !(host->caps & MMC_CAP_WAIT_WHILE_BUSY))
> mmc_delay(timeout_ms);
>
>>
>> return err;
>> }
>>
>
Kind regards
Uffe
--
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
