| Message ID | 1390402824-9850-7-git-send-email-ulf.hansson@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 23 January 2014 11:23, Adrian Hunter <adrian.hunter@intel.com> 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@linaro.org> >> --- >> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 27 January 2014 11:46, Adrian Hunter <adrian.hunter@intel.com> wrote: > On 23/01/14 16:26, Ulf Hansson wrote: >> On 23 January 2014 11:23, Adrian Hunter <adrian.hunter@intel.com> 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@linaro.org> >>>> --- >>>> 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. > > Unless you want to change all the host controller drivers, zero means > don't know. > Agree! >> >> 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 suggested: > > if (timeout_ms && host->max_busy_timeout && timeout_ms > host->max_busy_timeout) > use_r1b_resp = false; > > (without modifying timeout_ms) which wasn't related to MMC_CAP_WAIT_WHILE_BUSY > i.e. keeps R1B for hosts not supporting MMC_CAP_WAIT_WHILE_BUSY Will fix in v2, thanks! > >> >> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; + } + 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); return err; }
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@linaro.org> --- drivers/mmc/core/mmc.c | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-)