On 04/05/15 16:28, Ulf Hansson wrote: > On 20 April 2015 at 14:09, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: >> At the start of each request, re-tune if needed and >> then hold off re-tuning again until the request is done. >> >> Note that though there is one function that starts >> requests (mmc_start_request) there are two that wait for >> the request to be done (mmc_wait_for_req_done and >> mmc_wait_for_data_req_done). Also note that >> mmc_wait_for_data_req_done can return even when the >> request is not done (which allows the block driver >> to prepare a newly arrived request while still >> waiting for the previous request). >> >> This patch ensures re-tuning is held for the duration >> of a request. Subsequent patches will also hold >> re-tuning at other times when it might cause a >> conflict. >> >> Signed-off-by: Adrian Hunter <adrian.hunter@xxxxxxxxx> > > Patch2 is calling mmc_retune_needed() and thus actually triggers a > re-tune to potentially happen. That won't happen because host->retune_period is not set, so mmc_retune_needed() won't be called. > > To avoid bisectability issues about not holding re-tune when needed, I > suggest we move $subject patch to after patch8. > > Kind regards > Uffe > >> --- >> drivers/mmc/core/core.c | 32 +++++++++++++++++++++++++------- >> 1 file changed, 25 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >> index dd43f9b..a9936cb 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -186,12 +186,29 @@ void mmc_request_done(struct mmc_host *host, struct mmc_request *mrq) >> >> EXPORT_SYMBOL(mmc_request_done); >> >> +static void __mmc_start_request(struct mmc_host *host, struct mmc_request *mrq) >> +{ >> + int err; >> + >> + /* Assumes host controller has been runtime resumed by mmc_claim_host */ >> + err = mmc_retune(host); >> + if (err) { >> + mrq->cmd->error = err; >> + mmc_request_done(host, mrq); >> + return; >> + } >> + >> + host->ops->request(host, mrq); >> +} >> + >> static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq) >> { >> #ifdef CONFIG_MMC_DEBUG >> unsigned int i, sz; >> struct scatterlist *sg; >> #endif >> + mmc_retune_hold(host); >> + >> if (mmc_card_removed(host->card)) >> return -ENOMEDIUM; >> >> @@ -252,7 +269,7 @@ static int mmc_start_request(struct mmc_host *host, struct mmc_request *mrq) >> } >> mmc_host_clk_hold(host); >> led_trigger_event(host->led, LED_FULL); >> - host->ops->request(host, mrq); >> + __mmc_start_request(host, mrq); >> >> return 0; >> } >> @@ -422,17 +439,16 @@ static int mmc_wait_for_data_req_done(struct mmc_host *host, >> cmd->opcode, cmd->error); >> cmd->retries--; >> cmd->error = 0; >> - host->ops->request(host, mrq); >> + __mmc_start_request(host, mrq); >> continue; /* wait for done/new event again */ >> } >> } else if (context_info->is_new_req) { >> context_info->is_new_req = false; >> - if (!next_req) { >> - err = MMC_BLK_NEW_REQUEST; >> - break; /* return err */ >> - } >> + if (!next_req) >> + return MMC_BLK_NEW_REQUEST; >> } >> } >> + mmc_retune_release(host); >> return err; >> } >> >> @@ -471,8 +487,10 @@ static void mmc_wait_for_req_done(struct mmc_host *host, >> mmc_hostname(host), cmd->opcode, cmd->error); >> cmd->retries--; >> cmd->error = 0; >> - host->ops->request(host, mrq); >> + __mmc_start_request(host, mrq); >> } >> + >> + mmc_retune_release(host); >> } >> >> /** >> -- >> 1.9.1 >> >> -- >> 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 > > -- 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