| Message ID | 1427489863-9050-4-git-send-email-adrian.hunter@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 27 March 2015 at 21:57, Adrian Hunter <adrian.hunter@intel.com> 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. How about holding of re-tune from __mmc_claim_host() (after pm_runtime_get_sync() ) -> mmc_release_host(). Would that work? If so, that would simplify a lot around this. Kind regards Uffe > > Signed-off-by: Adrian Hunter <adrian.hunter@intel.com> > --- > 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 2679264..bb50d82 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 01/04/15 13:13, Ulf Hansson wrote: > On 27 March 2015 at 21:57, Adrian Hunter <adrian.hunter@intel.com> 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. > > How about holding of re-tune from __mmc_claim_host() (after > pm_runtime_get_sync() ) -> mmc_release_host(). Would that work? If so, > that would simplify a lot around this. I don't think it would work. The host is claimed both when we want to do re-tuning (timer, CRC error, or host requests it) and when we want to prevent it (in between dependent commands like erase, polling when card is busy, SDIO card custom wake-up command). -- 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 2679264..bb50d82 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); } /**
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@intel.com> --- drivers/mmc/core/core.c | 32 +++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-)