* [PATCH] mmc: core: disable the cache before suspend only after stopping BKOPS @ 2013-01-11 20:42 Maya Erez 2013-01-12 7:06 ` Subhash Jadavani 0 siblings, 1 reply; 7+ messages in thread From: Maya Erez @ 2013-01-11 20:42 UTC (permalink / raw) To: linux-mmc; +Cc: linux-arm-msm, Maya Erez, open list mmc_cache_ctrl was called in runtime suspend before MMC interrupted BKOPS in case it is still running on the card. This caused the cache disable to timeout. Therefore, mmc_cache_ctrl has to move to mmc_suspend where we are sure that the card can go into suspend and there is no pending activity. Signed-off-by: Maya Erez <merez@codeaurora.org> --- drivers/mmc/core/core.c | 7 +------ drivers/mmc/core/mmc.c | 8 +++++++- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index aaed768..b438bb2 100644 --- a/drivers/mmc/core/core.c +++ b/drivers/mmc/core/core.c @@ -2388,6 +2388,7 @@ EXPORT_SYMBOL(mmc_flush_cache); * Turn the cache ON/OFF. * Turning the cache OFF shall trigger flushing of the data * to the non-volatile storage. + * This function should be called with host claimed */ int mmc_cache_ctrl(struct mmc_host *host, u8 enable) { @@ -2399,7 +2400,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable) mmc_card_is_removable(host)) return err; - mmc_claim_host(host); if (card && mmc_card_mmc(card) && (card->ext_csd.cache_size > 0)) { enable = !!enable; @@ -2417,7 +2417,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable) card->ext_csd.cache_ctrl = enable; } } - mmc_release_host(host); return err; } @@ -2436,10 +2435,6 @@ int mmc_suspend_host(struct mmc_host *host) cancel_delayed_work(&host->detect); mmc_flush_scheduled_work(); - err = mmc_cache_ctrl(host, 0); - if (err) - goto out; - mmc_bus_get(host); if (host->bus_ops && !host->bus_dead) { if (host->bus_ops->suspend) { diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index e6e3911..dc17d40 100644 --- a/drivers/mmc/core/mmc.c +++ b/drivers/mmc/core/mmc.c @@ -1379,6 +1379,11 @@ static int mmc_suspend(struct mmc_host *host) BUG_ON(!host->card); mmc_claim_host(host); + + err = mmc_cache_ctrl(host, 0); + if (err) + goto out; + if (mmc_can_poweroff_notify(host->card)) err = mmc_poweroff_notify(host->card, EXT_CSD_POWER_OFF_SHORT); else if (mmc_card_can_sleep(host)) @@ -1386,8 +1391,9 @@ static int mmc_suspend(struct mmc_host *host) else if (!mmc_host_is_spi(host)) err = mmc_deselect_cards(host); host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); - mmc_release_host(host); +out: + mmc_release_host(host); return err; } -- 1.7.3.3 -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: core: disable the cache before suspend only after stopping BKOPS 2013-01-11 20:42 [PATCH] mmc: core: disable the cache before suspend only after stopping BKOPS Maya Erez @ 2013-01-12 7:06 ` Subhash Jadavani 2013-01-13 21:47 ` Maya Erez 0 siblings, 1 reply; 7+ messages in thread From: Subhash Jadavani @ 2013-01-12 7:06 UTC (permalink / raw) To: Maya Erez; +Cc: linux-mmc, linux-arm-msm, open list On 1/12/2013 2:12 AM, Maya Erez wrote: > mmc_cache_ctrl was called in runtime suspend before MMC interrupted > BKOPS in case it is still running on the card. This caused the cache > disable to timeout. I guess even if the idle time bkops polling is not implemented, this patch is good to have. cache control is the eMMC feature and in that sense, it should have been part of MMC's bus resume (mmc_resume) rather than generic mmc_suspend_host(). Patch as such is fine and if you agree, you may want to remove the mentioning of bkops as part of commit text and may just want to mention above reason as the main motivation for this patch. > Therefore, mmc_cache_ctrl has to move to mmc_suspend where we are sure > that the card can go into suspend and there is no pending activity. > > Signed-off-by: Maya Erez <merez@codeaurora.org> > --- > drivers/mmc/core/core.c | 7 +------ > drivers/mmc/core/mmc.c | 8 +++++++- > 2 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c > index aaed768..b438bb2 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -2388,6 +2388,7 @@ EXPORT_SYMBOL(mmc_flush_cache); > * Turn the cache ON/OFF. > * Turning the cache OFF shall trigger flushing of the data > * to the non-volatile storage. > + * This function should be called with host claimed > */ > int mmc_cache_ctrl(struct mmc_host *host, u8 enable) > { > @@ -2399,7 +2400,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable) > mmc_card_is_removable(host)) > return err; > > - mmc_claim_host(host); > if (card && mmc_card_mmc(card) && > (card->ext_csd.cache_size > 0)) { > enable = !!enable; > @@ -2417,7 +2417,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable) > card->ext_csd.cache_ctrl = enable; > } > } > - mmc_release_host(host); > > return err; > } > @@ -2436,10 +2435,6 @@ int mmc_suspend_host(struct mmc_host *host) > cancel_delayed_work(&host->detect); > mmc_flush_scheduled_work(); > > - err = mmc_cache_ctrl(host, 0); > - if (err) > - goto out; > - > mmc_bus_get(host); > if (host->bus_ops && !host->bus_dead) { > if (host->bus_ops->suspend) { > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c > index e6e3911..dc17d40 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1379,6 +1379,11 @@ static int mmc_suspend(struct mmc_host *host) > BUG_ON(!host->card); > > mmc_claim_host(host); > + > + err = mmc_cache_ctrl(host, 0); > + if (err) > + goto out; > + > if (mmc_can_poweroff_notify(host->card)) > err = mmc_poweroff_notify(host->card, EXT_CSD_POWER_OFF_SHORT); > else if (mmc_card_can_sleep(host)) > @@ -1386,8 +1391,9 @@ static int mmc_suspend(struct mmc_host *host) > else if (!mmc_host_is_spi(host)) > err = mmc_deselect_cards(host); > host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); > - mmc_release_host(host); > > +out: > + mmc_release_host(host); > return err; > } > ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] mmc: core: disable the cache before suspend only after stopping BKOPS 2013-01-12 7:06 ` Subhash Jadavani @ 2013-01-13 21:47 ` Maya Erez 2013-01-16 21:37 ` Ulf Hansson 2013-01-28 21:21 ` Chris Ball 0 siblings, 2 replies; 7+ messages in thread From: Maya Erez @ 2013-01-13 21:47 UTC (permalink / raw) To: 'Subhash Jadavani'; +Cc: linux-mmc, linux-arm-msm, 'open list' -----Original Message----- From: Subhash Jadavani [mailto:subhashj@codeaurora.org] Sent: Saturday, January 12, 2013 9:07 AM To: Maya Erez Cc: linux-mmc@vger.kernel.org; linux-arm-msm@vger.kernel.org; open list Subject: Re: [PATCH] mmc: core: disable the cache before suspend only after stopping BKOPS On 1/12/2013 2:12 AM, Maya Erez wrote: > mmc_cache_ctrl was called in runtime suspend before MMC interrupted > BKOPS in case it is still running on the card. This caused the cache > disable to timeout. I guess even if the idle time bkops polling is not implemented, this patch is good to have. cache control is the eMMC feature and in that sense, it should have been part of MMC's bus resume (mmc_resume) rather than generic mmc_suspend_host(). Patch as such is fine and if you agree, you may want to remove the mentioning of bkops as part of commit text and may just want to mention above reason as the main motivation for this patch. Agreed, I will change the commit text in the next uploaded version. > Therefore, mmc_cache_ctrl has to move to mmc_suspend where we are sure > that the card can go into suspend and there is no pending activity. > > Signed-off-by: Maya Erez <merez@codeaurora.org> > --- > drivers/mmc/core/core.c | 7 +------ > drivers/mmc/core/mmc.c | 8 +++++++- > 2 files changed, 8 insertions(+), 7 deletions(-) > > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index > aaed768..b438bb2 100644 > --- a/drivers/mmc/core/core.c > +++ b/drivers/mmc/core/core.c > @@ -2388,6 +2388,7 @@ EXPORT_SYMBOL(mmc_flush_cache); > * Turn the cache ON/OFF. > * Turning the cache OFF shall trigger flushing of the data > * to the non-volatile storage. > + * This function should be called with host claimed > */ > int mmc_cache_ctrl(struct mmc_host *host, u8 enable) > { > @@ -2399,7 +2400,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable) > mmc_card_is_removable(host)) > return err; > > - mmc_claim_host(host); > if (card && mmc_card_mmc(card) && > (card->ext_csd.cache_size > 0)) { > enable = !!enable; > @@ -2417,7 +2417,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable) > card->ext_csd.cache_ctrl = enable; > } > } > - mmc_release_host(host); > > return err; > } > @@ -2436,10 +2435,6 @@ int mmc_suspend_host(struct mmc_host *host) > cancel_delayed_work(&host->detect); > mmc_flush_scheduled_work(); > > - err = mmc_cache_ctrl(host, 0); > - if (err) > - goto out; > - > mmc_bus_get(host); > if (host->bus_ops && !host->bus_dead) { > if (host->bus_ops->suspend) { > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index > e6e3911..dc17d40 100644 > --- a/drivers/mmc/core/mmc.c > +++ b/drivers/mmc/core/mmc.c > @@ -1379,6 +1379,11 @@ static int mmc_suspend(struct mmc_host *host) > BUG_ON(!host->card); > > mmc_claim_host(host); > + > + err = mmc_cache_ctrl(host, 0); > + if (err) > + goto out; > + > if (mmc_can_poweroff_notify(host->card)) > err = mmc_poweroff_notify(host->card, EXT_CSD_POWER_OFF_SHORT); > else if (mmc_card_can_sleep(host)) > @@ -1386,8 +1391,9 @@ static int mmc_suspend(struct mmc_host *host) > else if (!mmc_host_is_spi(host)) > err = mmc_deselect_cards(host); > host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200); > - mmc_release_host(host); > > +out: > + mmc_release_host(host); > return err; > } > -- QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: core: disable the cache before suspend only after stopping BKOPS 2013-01-13 21:47 ` Maya Erez @ 2013-01-16 21:37 ` Ulf Hansson 2013-01-17 6:48 ` Seungwon Jeon 2013-01-28 21:21 ` Chris Ball 1 sibling, 1 reply; 7+ messages in thread From: Ulf Hansson @ 2013-01-16 21:37 UTC (permalink / raw) To: Maya Erez; +Cc: Subhash Jadavani, linux-mmc, linux-arm-msm, open list On 13 January 2013 22:47, Maya Erez <merez@codeaurora.org> wrote: > -----Original Message----- > From: Subhash Jadavani [mailto:subhashj@codeaurora.org] > Sent: Saturday, January 12, 2013 9:07 AM > To: Maya Erez > Cc: linux-mmc@vger.kernel.org; linux-arm-msm@vger.kernel.org; open list > Subject: Re: [PATCH] mmc: core: disable the cache before suspend only after > stopping BKOPS > > On 1/12/2013 2:12 AM, Maya Erez wrote: >> mmc_cache_ctrl was called in runtime suspend before MMC interrupted >> BKOPS in case it is still running on the card. This caused the cache >> disable to timeout. > I guess even if the idle time bkops polling is not implemented, this patch > is good to have. cache control is the eMMC feature and in that sense, it > should have been part of MMC's bus resume (mmc_resume) rather than generic > mmc_suspend_host(). > > Patch as such is fine and if you agree, you may want to remove the > mentioning of bkops as part of commit text and may just want to mention > above reason as the main motivation for this patch. > > Agreed, I will change the commit text in the next uploaded version. > >> Therefore, mmc_cache_ctrl has to move to mmc_suspend where we are sure >> that the card can go into suspend and there is no pending activity. >> >> Signed-off-by: Maya Erez <merez@codeaurora.org> >> --- >> drivers/mmc/core/core.c | 7 +------ >> drivers/mmc/core/mmc.c | 8 +++++++- >> 2 files changed, 8 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index >> aaed768..b438bb2 100644 >> --- a/drivers/mmc/core/core.c >> +++ b/drivers/mmc/core/core.c >> @@ -2388,6 +2388,7 @@ EXPORT_SYMBOL(mmc_flush_cache); >> * Turn the cache ON/OFF. >> * Turning the cache OFF shall trigger flushing of the data >> * to the non-volatile storage. >> + * This function should be called with host claimed >> */ >> int mmc_cache_ctrl(struct mmc_host *host, u8 enable) >> { >> @@ -2399,7 +2400,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable) >> mmc_card_is_removable(host)) >> return err; >> >> - mmc_claim_host(host); >> if (card && mmc_card_mmc(card) && >> (card->ext_csd.cache_size > 0)) { >> enable = !!enable; >> @@ -2417,7 +2417,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable) >> card->ext_csd.cache_ctrl = enable; >> } >> } >> - mmc_release_host(host); >> >> return err; >> } >> @@ -2436,10 +2435,6 @@ int mmc_suspend_host(struct mmc_host *host) >> cancel_delayed_work(&host->detect); >> mmc_flush_scheduled_work(); >> >> - err = mmc_cache_ctrl(host, 0); >> - if (err) >> - goto out; >> - >> mmc_bus_get(host); >> if (host->bus_ops && !host->bus_dead) { >> if (host->bus_ops->suspend) { >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index >> e6e3911..dc17d40 100644 >> --- a/drivers/mmc/core/mmc.c >> +++ b/drivers/mmc/core/mmc.c >> @@ -1379,6 +1379,11 @@ static int mmc_suspend(struct mmc_host *host) >> BUG_ON(!host->card); >> >> mmc_claim_host(host); >> + >> + err = mmc_cache_ctrl(host, 0); >> + if (err) >> + goto out; >> + >> if (mmc_can_poweroff_notify(host->card)) >> err = mmc_poweroff_notify(host->card, > EXT_CSD_POWER_OFF_SHORT); >> else if (mmc_card_can_sleep(host)) >> @@ -1386,8 +1391,9 @@ static int mmc_suspend(struct mmc_host *host) >> else if (!mmc_host_is_spi(host)) >> err = mmc_deselect_cards(host); >> host->card->state &= ~(MMC_STATE_HIGHSPEED | > MMC_STATE_HIGHSPEED_200); >> - mmc_release_host(host); >> >> +out: >> + mmc_release_host(host); >> return err; >> } >> > -- > QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member > of Code Aurora Forum, hosted by The Linux Foundation > > -- > 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 I agree that some update to the commit msg is wanted; since this fixup is not related to runtime suspend as such. Anyway, you have my ack! Acked-by: Ulf Hansson <ulf.hansson@linaro.org> Kind regards Ulf Hansson ^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [PATCH] mmc: core: disable the cache before suspend only after stopping BKOPS 2013-01-16 21:37 ` Ulf Hansson @ 2013-01-17 6:48 ` Seungwon Jeon 2013-01-17 6:54 ` Subhash Jadavani 0 siblings, 1 reply; 7+ messages in thread From: Seungwon Jeon @ 2013-01-17 6:48 UTC (permalink / raw) To: 'Ulf Hansson', 'Maya Erez' Cc: 'Subhash Jadavani', linux-mmc, linux-arm-msm, 'open list' On Thursday, January 17, 2013 , Ulf Hansson wrote: > On 13 January 2013 22:47, Maya Erez <merez@codeaurora.org> wrote: > > -----Original Message----- > > From: Subhash Jadavani [mailto:subhashj@codeaurora.org] > > Sent: Saturday, January 12, 2013 9:07 AM > > To: Maya Erez > > Cc: linux-mmc@vger.kernel.org; linux-arm-msm@vger.kernel.org; open list > > Subject: Re: [PATCH] mmc: core: disable the cache before suspend only after > > stopping BKOPS > > > > On 1/12/2013 2:12 AM, Maya Erez wrote: > >> mmc_cache_ctrl was called in runtime suspend before MMC interrupted > >> BKOPS in case it is still running on the card. This caused the cache > >> disable to timeout. > > I guess even if the idle time bkops polling is not implemented, this patch > > is good to have. cache control is the eMMC feature and in that sense, it > > should have been part of MMC's bus resume (mmc_resume) rather than generic > > mmc_suspend_host(). > > > > Patch as such is fine and if you agree, you may want to remove the > > mentioning of bkops as part of commit text and may just want to mention > > above reason as the main motivation for this patch. > > > > Agreed, I will change the commit text in the next uploaded version. > > > >> Therefore, mmc_cache_ctrl has to move to mmc_suspend where we are sure > >> that the card can go into suspend and there is no pending activity. > >> > >> Signed-off-by: Maya Erez <merez@codeaurora.org> > >> --- > >> drivers/mmc/core/core.c | 7 +------ > >> drivers/mmc/core/mmc.c | 8 +++++++- > >> 2 files changed, 8 insertions(+), 7 deletions(-) > >> > >> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index > >> aaed768..b438bb2 100644 > >> --- a/drivers/mmc/core/core.c > >> +++ b/drivers/mmc/core/core.c > >> @@ -2388,6 +2388,7 @@ EXPORT_SYMBOL(mmc_flush_cache); > >> * Turn the cache ON/OFF. > >> * Turning the cache OFF shall trigger flushing of the data > >> * to the non-volatile storage. > >> + * This function should be called with host claimed > >> */ > >> int mmc_cache_ctrl(struct mmc_host *host, u8 enable) > >> { > >> @@ -2399,7 +2400,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable) > >> mmc_card_is_removable(host)) > >> return err; > >> > >> - mmc_claim_host(host); > >> if (card && mmc_card_mmc(card) && > >> (card->ext_csd.cache_size > 0)) { > >> enable = !!enable; > >> @@ -2417,7 +2417,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable) > >> card->ext_csd.cache_ctrl = enable; > >> } > >> } > >> - mmc_release_host(host); > >> > >> return err; > >> } > >> @@ -2436,10 +2435,6 @@ int mmc_suspend_host(struct mmc_host *host) > >> cancel_delayed_work(&host->detect); > >> mmc_flush_scheduled_work(); > >> > >> - err = mmc_cache_ctrl(host, 0); > >> - if (err) > >> - goto out; > >> - > >> mmc_bus_get(host); > >> if (host->bus_ops && !host->bus_dead) { > >> if (host->bus_ops->suspend) { > >> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index > >> e6e3911..dc17d40 100644 > >> --- a/drivers/mmc/core/mmc.c > >> +++ b/drivers/mmc/core/mmc.c > >> @@ -1379,6 +1379,11 @@ static int mmc_suspend(struct mmc_host *host) > >> BUG_ON(!host->card); > >> > >> mmc_claim_host(host); > >> + > >> + err = mmc_cache_ctrl(host, 0); > >> + if (err) > >> + goto out; > >> + > >> if (mmc_can_poweroff_notify(host->card)) > >> err = mmc_poweroff_notify(host->card, > > EXT_CSD_POWER_OFF_SHORT); > >> else if (mmc_card_can_sleep(host)) > >> @@ -1386,8 +1391,9 @@ static int mmc_suspend(struct mmc_host *host) > >> else if (!mmc_host_is_spi(host)) > >> err = mmc_deselect_cards(host); > >> host->card->state &= ~(MMC_STATE_HIGHSPEED | > > MMC_STATE_HIGHSPEED_200); > >> - mmc_release_host(host); > >> > >> +out: > >> + mmc_release_host(host); > >> return err; > >> } > >> > > -- > > QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member > > of Code Aurora Forum, hosted by The Linux Foundation > > > > -- > > 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 > > I agree that some update to the commit msg is wanted; since this fixup > is not related to runtime suspend as such. > Anyway, you have my ack! > > Acked-by: Ulf Hansson <ulf.hansson@linaro.org> Yes, it's reasonable. Cache feature is closely connected with eMMC. Acked-by: Seungwon Jeon <tgin.jun@samsung.com> Thanks, Seungwon Jeon > > Kind regards > Ulf Hansson > -- > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: core: disable the cache before suspend only after stopping BKOPS 2013-01-17 6:48 ` Seungwon Jeon @ 2013-01-17 6:54 ` Subhash Jadavani 0 siblings, 0 replies; 7+ messages in thread From: Subhash Jadavani @ 2013-01-17 6:54 UTC (permalink / raw) To: Seungwon Jeon Cc: 'Ulf Hansson', 'Maya Erez', linux-mmc, linux-arm-msm, 'open list' As Maya is going to fix the commit text, Acked-by: Subhash Jadavani <subhashj@codeaurora.org> On 1/17/2013 12:18 PM, Seungwon Jeon wrote: > On Thursday, January 17, 2013 , Ulf Hansson wrote: >> On 13 January 2013 22:47, Maya Erez <merez@codeaurora.org> wrote: >>> -----Original Message----- >>> From: Subhash Jadavani [mailto:subhashj@codeaurora.org] >>> Sent: Saturday, January 12, 2013 9:07 AM >>> To: Maya Erez >>> Cc: linux-mmc@vger.kernel.org; linux-arm-msm@vger.kernel.org; open list >>> Subject: Re: [PATCH] mmc: core: disable the cache before suspend only after >>> stopping BKOPS >>> >>> On 1/12/2013 2:12 AM, Maya Erez wrote: >>>> mmc_cache_ctrl was called in runtime suspend before MMC interrupted >>>> BKOPS in case it is still running on the card. This caused the cache >>>> disable to timeout. >>> I guess even if the idle time bkops polling is not implemented, this patch >>> is good to have. cache control is the eMMC feature and in that sense, it >>> should have been part of MMC's bus resume (mmc_resume) rather than generic >>> mmc_suspend_host(). >>> >>> Patch as such is fine and if you agree, you may want to remove the >>> mentioning of bkops as part of commit text and may just want to mention >>> above reason as the main motivation for this patch. >>> >>> Agreed, I will change the commit text in the next uploaded version. >>> >>>> Therefore, mmc_cache_ctrl has to move to mmc_suspend where we are sure >>>> that the card can go into suspend and there is no pending activity. >>>> >>>> Signed-off-by: Maya Erez <merez@codeaurora.org> >>>> --- >>>> drivers/mmc/core/core.c | 7 +------ >>>> drivers/mmc/core/mmc.c | 8 +++++++- >>>> 2 files changed, 8 insertions(+), 7 deletions(-) >>>> >>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c index >>>> aaed768..b438bb2 100644 >>>> --- a/drivers/mmc/core/core.c >>>> +++ b/drivers/mmc/core/core.c >>>> @@ -2388,6 +2388,7 @@ EXPORT_SYMBOL(mmc_flush_cache); >>>> * Turn the cache ON/OFF. >>>> * Turning the cache OFF shall trigger flushing of the data >>>> * to the non-volatile storage. >>>> + * This function should be called with host claimed >>>> */ >>>> int mmc_cache_ctrl(struct mmc_host *host, u8 enable) >>>> { >>>> @@ -2399,7 +2400,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable) >>>> mmc_card_is_removable(host)) >>>> return err; >>>> >>>> - mmc_claim_host(host); >>>> if (card && mmc_card_mmc(card) && >>>> (card->ext_csd.cache_size > 0)) { >>>> enable = !!enable; >>>> @@ -2417,7 +2417,6 @@ int mmc_cache_ctrl(struct mmc_host *host, u8 enable) >>>> card->ext_csd.cache_ctrl = enable; >>>> } >>>> } >>>> - mmc_release_host(host); >>>> >>>> return err; >>>> } >>>> @@ -2436,10 +2435,6 @@ int mmc_suspend_host(struct mmc_host *host) >>>> cancel_delayed_work(&host->detect); >>>> mmc_flush_scheduled_work(); >>>> >>>> - err = mmc_cache_ctrl(host, 0); >>>> - if (err) >>>> - goto out; >>>> - >>>> mmc_bus_get(host); >>>> if (host->bus_ops && !host->bus_dead) { >>>> if (host->bus_ops->suspend) { >>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index >>>> e6e3911..dc17d40 100644 >>>> --- a/drivers/mmc/core/mmc.c >>>> +++ b/drivers/mmc/core/mmc.c >>>> @@ -1379,6 +1379,11 @@ static int mmc_suspend(struct mmc_host *host) >>>> BUG_ON(!host->card); >>>> >>>> mmc_claim_host(host); >>>> + >>>> + err = mmc_cache_ctrl(host, 0); >>>> + if (err) >>>> + goto out; >>>> + >>>> if (mmc_can_poweroff_notify(host->card)) >>>> err = mmc_poweroff_notify(host->card, >>> EXT_CSD_POWER_OFF_SHORT); >>>> else if (mmc_card_can_sleep(host)) >>>> @@ -1386,8 +1391,9 @@ static int mmc_suspend(struct mmc_host *host) >>>> else if (!mmc_host_is_spi(host)) >>>> err = mmc_deselect_cards(host); >>>> host->card->state &= ~(MMC_STATE_HIGHSPEED | >>> MMC_STATE_HIGHSPEED_200); >>>> - mmc_release_host(host); >>>> >>>> +out: >>>> + mmc_release_host(host); >>>> return err; >>>> } >>>> >>> -- >>> QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, Inc. is a member >>> of Code Aurora Forum, hosted by The Linux Foundation >>> >>> -- >>> 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 >> I agree that some update to the commit msg is wanted; since this fixup >> is not related to runtime suspend as such. >> Anyway, you have my ack! >> >> Acked-by: Ulf Hansson <ulf.hansson@linaro.org> > Yes, it's reasonable. > Cache feature is closely connected with eMMC. > > Acked-by: Seungwon Jeon <tgin.jun@samsung.com> > > Thanks, > Seungwon Jeon >> Kind regards >> Ulf Hansson >> -- >> 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 > -- > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] mmc: core: disable the cache before suspend only after stopping BKOPS 2013-01-13 21:47 ` Maya Erez 2013-01-16 21:37 ` Ulf Hansson @ 2013-01-28 21:21 ` Chris Ball 1 sibling, 0 replies; 7+ messages in thread From: Chris Ball @ 2013-01-28 21:21 UTC (permalink / raw) To: Maya Erez Cc: 'Subhash Jadavani', linux-mmc, linux-arm-msm, 'open list' Hi Maya, On Sun, Jan 13 2013, Maya Erez wrote: > Agreed, I will change the commit text in the next uploaded version. I haven't seen the updated patch go by -- please could you send it when you get a chance? Thanks, - Chris. -- Chris Ball <cjb@laptop.org> <http://printf.net/> One Laptop Per Child ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2013-01-28 21:22 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-01-11 20:42 [PATCH] mmc: core: disable the cache before suspend only after stopping BKOPS Maya Erez 2013-01-12 7:06 ` Subhash Jadavani 2013-01-13 21:47 ` Maya Erez 2013-01-16 21:37 ` Ulf Hansson 2013-01-17 6:48 ` Seungwon Jeon 2013-01-17 6:54 ` Subhash Jadavani 2013-01-28 21:21 ` Chris Ball
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).