| Message ID | 01d645020004fe135bb5374ecb9f5f6ac0884b6b.1469436431.git.baolin.wang@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 25 July 2016 at 10:48, Baolin Wang <baolin.wang@linaro.org> wrote: > When mmc host HW supports busy signalling (using R1B as response), We > shouldn't use 'host->max_busy_timeout' as the limitation when deciding > the max discard sectors that we tell the generic BLOCK layer about. > Instead, we should pick one preferred erase size as the max discard > sectors. > > If the host controller supports busy signalling and the timeout for > the erase operation does not exceed the max_busy_timeout, we should > use R1B response. Or we need to prevent the host from doing hw busy > detection, which is done by converting to a R1 response instead. > > Signed-off-by: Baolin Wang <baolin.wang@linaro.org> Thanks, applied for next! I took the liberty to update the change-log a bit to clarify things! Thanks a lot for working on this long outstanding problem! In the next step I plan to remove the MMC_CAP_ERASE and instead enable it by default, although let's leave that for v4.9. Kind regards Uffe > --- > Changes since v2: > - Remove the 'MMC_CAP_WAIT_WHILE_BUSY' flag checking when deciding > if we can use R1B response. > - Avoid polling CMD13 when using R1B response. > - Use earlier calculated erase timeout as the polling time. > > Changes since v1: > - Remove the 'MMC_CAP_WAIT_WHILE_BUSY' flag checking when deciding > the max discard sectors. > --- > drivers/mmc/core/core.c | 60 +++++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 50 insertions(+), 10 deletions(-) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index 8b4dfd4..b4c08d1a 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -2060,7 +2060,8 @@ static int mmc_do_erase(struct mmc_card *card, unsigned int from, > unsigned int to, unsigned int arg) > { > struct mmc_command cmd = {0}; > - unsigned int qty = 0; > + unsigned int qty = 0, busy_timeout = 0; > + bool use_r1b_resp = false; > unsigned long timeout; > int err; > > @@ -2128,8 +2129,22 @@ static int mmc_do_erase(struct mmc_card *card, unsigned int from, > memset(&cmd, 0, sizeof(struct mmc_command)); > cmd.opcode = MMC_ERASE; > cmd.arg = arg; > - cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; > - cmd.busy_timeout = mmc_erase_timeout(card, arg, qty); > + busy_timeout = mmc_erase_timeout(card, arg, qty); > + /* > + * If the host controller supports busy signalling and the timeout for > + * the erase operation does not exceed the max_busy_timeout, we should > + * use R1B response. Or we need to prevent the host from doing hw busy > + * detection, which is done by converting to a R1 response instead. > + */ > + if (card->host->max_busy_timeout && > + busy_timeout > card->host->max_busy_timeout) { > + cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC; > + } else { > + cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; > + cmd.busy_timeout = busy_timeout; > + use_r1b_resp = true; > + } > + > err = mmc_wait_for_cmd(card->host, &cmd, 0); > if (err) { > pr_err("mmc_erase: erase error %d, status %#x\n", > @@ -2141,7 +2156,14 @@ 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); > + /* > + * In case of when R1B + MMC_CAP_WAIT_WHILE_BUSY is used, the polling > + * shall be avoided. > + */ > + if ((card->host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp) > + goto out; > + > + timeout = jiffies + msecs_to_jiffies(busy_timeout); > do { > memset(&cmd, 0, sizeof(struct mmc_command)); > cmd.opcode = MMC_SEND_STATUS; > @@ -2321,23 +2343,41 @@ static unsigned int mmc_do_calc_max_discard(struct mmc_card *card, > unsigned int arg) > { > struct mmc_host *host = card->host; > - unsigned int max_discard, x, y, qty = 0, max_qty, timeout; > + unsigned int max_discard, x, y, qty = 0, max_qty, min_qty, timeout; > unsigned int last_timeout = 0; > > - if (card->erase_shift) > + if (card->erase_shift) { > max_qty = UINT_MAX >> card->erase_shift; > - else if (mmc_card_sd(card)) > + min_qty = card->pref_erase >> card->erase_shift; > + } else if (mmc_card_sd(card)) { > max_qty = UINT_MAX; > - else > + min_qty = card->pref_erase; > + } else { > max_qty = UINT_MAX / card->erase_size; > + min_qty = card->pref_erase / card->erase_size; > + } > > - /* Find the largest qty with an OK timeout */ > + /* > + * We should not only use 'host->max_busy_timeout' as the limitation > + * when deciding the max discard sectors. We should set a balance value > + * to improve the erase speed, and it can not get too long timeout at > + * the same time. > + * > + * Here we set 'card->pref_erase' as the minimal discard sectors no > + * matter what size of 'host->max_busy_timeout', but if the > + * 'host->max_busy_timeout' is large enough for more discard sectors, > + * then we can continue to increase the max discard sectors until we > + * get a balance value. > + */ > do { > y = 0; > for (x = 1; x && x <= max_qty && max_qty - x >= qty; x <<= 1) { > timeout = mmc_erase_timeout(card, arg, qty + x); > - if (timeout > host->max_busy_timeout) > + > + if (qty + x > min_qty && > + timeout > host->max_busy_timeout) > break; > + > if (timeout < last_timeout) > break; > last_timeout = timeout; > -- > 1.7.9.5 > -- 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 25 July 2016 at 17:26, Ulf Hansson <ulf.hansson@linaro.org> wrote: > On 25 July 2016 at 10:48, Baolin Wang <baolin.wang@linaro.org> wrote: >> When mmc host HW supports busy signalling (using R1B as response), We >> shouldn't use 'host->max_busy_timeout' as the limitation when deciding >> the max discard sectors that we tell the generic BLOCK layer about. >> Instead, we should pick one preferred erase size as the max discard >> sectors. >> >> If the host controller supports busy signalling and the timeout for >> the erase operation does not exceed the max_busy_timeout, we should >> use R1B response. Or we need to prevent the host from doing hw busy >> detection, which is done by converting to a R1 response instead. >> >> Signed-off-by: Baolin Wang <baolin.wang@linaro.org> > > Thanks, applied for next! I took the liberty to update the change-log > a bit to clarify things! > > Thanks a lot for working on this long outstanding problem! > > In the next step I plan to remove the MMC_CAP_ERASE and instead enable > it by default, although let's leave that for v4.9. OK. I will investigate that and send patch to fix that. Thanks for your useful suggetion.:) >> --- >> Changes since v2: >> - Remove the 'MMC_CAP_WAIT_WHILE_BUSY' flag checking when deciding >> if we can use R1B response. >> - Avoid polling CMD13 when using R1B response. >> - Use earlier calculated erase timeout as the polling time. >> >> Changes since v1: >> - Remove the 'MMC_CAP_WAIT_WHILE_BUSY' flag checking when deciding >> the max discard sectors. >> --- >> drivers/mmc/core/core.c | 60 +++++++++++++++++++++++++++++++++++++++-------- >> 1 file changed, 50 insertions(+), 10 deletions(-) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index 8b4dfd4..b4c08d1a 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -2060,7 +2060,8 @@ static int mmc_do_erase(struct mmc_card *card, unsigned int from, >> unsigned int to, unsigned int arg) >> { >> struct mmc_command cmd = {0}; >> - unsigned int qty = 0; >> + unsigned int qty = 0, busy_timeout = 0; >> + bool use_r1b_resp = false; >> unsigned long timeout; >> int err; >> >> @@ -2128,8 +2129,22 @@ static int mmc_do_erase(struct mmc_card *card, unsigned int from, >> memset(&cmd, 0, sizeof(struct mmc_command)); >> cmd.opcode = MMC_ERASE; >> cmd.arg = arg; >> - cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; >> - cmd.busy_timeout = mmc_erase_timeout(card, arg, qty); >> + busy_timeout = mmc_erase_timeout(card, arg, qty); >> + /* >> + * If the host controller supports busy signalling and the timeout for >> + * the erase operation does not exceed the max_busy_timeout, we should >> + * use R1B response. Or we need to prevent the host from doing hw busy >> + * detection, which is done by converting to a R1 response instead. >> + */ >> + if (card->host->max_busy_timeout && >> + busy_timeout > card->host->max_busy_timeout) { >> + cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC; >> + } else { >> + cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; >> + cmd.busy_timeout = busy_timeout; >> + use_r1b_resp = true; >> + } >> + >> err = mmc_wait_for_cmd(card->host, &cmd, 0); >> if (err) { >> pr_err("mmc_erase: erase error %d, status %#x\n", >> @@ -2141,7 +2156,14 @@ 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); >> + /* >> + * In case of when R1B + MMC_CAP_WAIT_WHILE_BUSY is used, the polling >> + * shall be avoided. >> + */ >> + if ((card->host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp) >> + goto out; >> + >> + timeout = jiffies + msecs_to_jiffies(busy_timeout); >> do { >> memset(&cmd, 0, sizeof(struct mmc_command)); >> cmd.opcode = MMC_SEND_STATUS; >> @@ -2321,23 +2343,41 @@ static unsigned int mmc_do_calc_max_discard(struct mmc_card *card, >> unsigned int arg) >> { >> struct mmc_host *host = card->host; >> - unsigned int max_discard, x, y, qty = 0, max_qty, timeout; >> + unsigned int max_discard, x, y, qty = 0, max_qty, min_qty, timeout; >> unsigned int last_timeout = 0; >> >> - if (card->erase_shift) >> + if (card->erase_shift) { >> max_qty = UINT_MAX >> card->erase_shift; >> - else if (mmc_card_sd(card)) >> + min_qty = card->pref_erase >> card->erase_shift; >> + } else if (mmc_card_sd(card)) { >> max_qty = UINT_MAX; >> - else >> + min_qty = card->pref_erase; >> + } else { >> max_qty = UINT_MAX / card->erase_size; >> + min_qty = card->pref_erase / card->erase_size; >> + } >> >> - /* Find the largest qty with an OK timeout */ >> + /* >> + * We should not only use 'host->max_busy_timeout' as the limitation >> + * when deciding the max discard sectors. We should set a balance value >> + * to improve the erase speed, and it can not get too long timeout at >> + * the same time. >> + * >> + * Here we set 'card->pref_erase' as the minimal discard sectors no >> + * matter what size of 'host->max_busy_timeout', but if the >> + * 'host->max_busy_timeout' is large enough for more discard sectors, >> + * then we can continue to increase the max discard sectors until we >> + * get a balance value. >> + */ >> do { >> y = 0; >> for (x = 1; x && x <= max_qty && max_qty - x >= qty; x <<= 1) { >> timeout = mmc_erase_timeout(card, arg, qty + x); >> - if (timeout > host->max_busy_timeout) >> + >> + if (qty + x > min_qty && >> + timeout > host->max_busy_timeout) >> break; >> + >> if (timeout < last_timeout) >> break; >> last_timeout = timeout; >> -- >> 1.7.9.5 >> -- Baolin.wang Best Regards -- 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/core.c b/drivers/mmc/core/core.c index 8b4dfd4..b4c08d1a 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2060,7 +2060,8 @@ static int mmc_do_erase(struct mmc_card *card, unsigned int from, unsigned int to, unsigned int arg) { struct mmc_command cmd = {0}; - unsigned int qty = 0; + unsigned int qty = 0, busy_timeout = 0; + bool use_r1b_resp = false; unsigned long timeout; int err; @@ -2128,8 +2129,22 @@ static int mmc_do_erase(struct mmc_card *card, unsigned int from, memset(&cmd, 0, sizeof(struct mmc_command)); cmd.opcode = MMC_ERASE; cmd.arg = arg; - cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; - cmd.busy_timeout = mmc_erase_timeout(card, arg, qty); + busy_timeout = mmc_erase_timeout(card, arg, qty); + /* + * If the host controller supports busy signalling and the timeout for + * the erase operation does not exceed the max_busy_timeout, we should + * use R1B response. Or we need to prevent the host from doing hw busy + * detection, which is done by converting to a R1 response instead. + */ + if (card->host->max_busy_timeout && + busy_timeout > card->host->max_busy_timeout) { + cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC; + } else { + cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; + cmd.busy_timeout = busy_timeout; + use_r1b_resp = true; + } + err = mmc_wait_for_cmd(card->host, &cmd, 0); if (err) { pr_err("mmc_erase: erase error %d, status %#x\n", @@ -2141,7 +2156,14 @@ 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); + /* + * In case of when R1B + MMC_CAP_WAIT_WHILE_BUSY is used, the polling + * shall be avoided. + */ + if ((card->host->caps & MMC_CAP_WAIT_WHILE_BUSY) && use_r1b_resp) + goto out; + + timeout = jiffies + msecs_to_jiffies(busy_timeout); do { memset(&cmd, 0, sizeof(struct mmc_command)); cmd.opcode = MMC_SEND_STATUS; @@ -2321,23 +2343,41 @@ static unsigned int mmc_do_calc_max_discard(struct mmc_card *card, unsigned int arg) { struct mmc_host *host = card->host; - unsigned int max_discard, x, y, qty = 0, max_qty, timeout; + unsigned int max_discard, x, y, qty = 0, max_qty, min_qty, timeout; unsigned int last_timeout = 0; - if (card->erase_shift) + if (card->erase_shift) { max_qty = UINT_MAX >> card->erase_shift; - else if (mmc_card_sd(card)) + min_qty = card->pref_erase >> card->erase_shift; + } else if (mmc_card_sd(card)) { max_qty = UINT_MAX; - else + min_qty = card->pref_erase; + } else { max_qty = UINT_MAX / card->erase_size; + min_qty = card->pref_erase / card->erase_size; + } - /* Find the largest qty with an OK timeout */ + /* + * We should not only use 'host->max_busy_timeout' as the limitation + * when deciding the max discard sectors. We should set a balance value + * to improve the erase speed, and it can not get too long timeout at + * the same time. + * + * Here we set 'card->pref_erase' as the minimal discard sectors no + * matter what size of 'host->max_busy_timeout', but if the + * 'host->max_busy_timeout' is large enough for more discard sectors, + * then we can continue to increase the max discard sectors until we + * get a balance value. + */ do { y = 0; for (x = 1; x && x <= max_qty && max_qty - x >= qty; x <<= 1) { timeout = mmc_erase_timeout(card, arg, qty + x); - if (timeout > host->max_busy_timeout) + + if (qty + x > min_qty && + timeout > host->max_busy_timeout) break; + if (timeout < last_timeout) break; last_timeout = timeout;
When mmc host HW supports busy signalling (using R1B as response), We shouldn't use 'host->max_busy_timeout' as the limitation when deciding the max discard sectors that we tell the generic BLOCK layer about. Instead, we should pick one preferred erase size as the max discard sectors. If the host controller supports busy signalling and the timeout for the erase operation does not exceed the max_busy_timeout, we should use R1B response. Or we need to prevent the host from doing hw busy detection, which is done by converting to a R1 response instead. Signed-off-by: Baolin Wang <baolin.wang@linaro.org> --- Changes since v2: - Remove the 'MMC_CAP_WAIT_WHILE_BUSY' flag checking when deciding if we can use R1B response. - Avoid polling CMD13 when using R1B response. - Use earlier calculated erase timeout as the polling time. Changes since v1: - Remove the 'MMC_CAP_WAIT_WHILE_BUSY' flag checking when deciding the max discard sectors. --- drivers/mmc/core/core.c | 60 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 10 deletions(-) -- 1.7.9.5 -- 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