* [PATCH] mmc: core: Keep host claimed while invoking mmc_power_off|up()
@ 2015-09-11 12:41 Ulf Hansson
2015-09-11 13:22 ` Kishon Vijay Abraham I
0 siblings, 1 reply; 4+ messages in thread
From: Ulf Hansson @ 2015-09-11 12:41 UTC (permalink / raw)
To: linux-mmc, Ulf Hansson; +Cc: Kishon Vijay Abraham I
As mmc_claim_host() invokes pm_runtime_get_sync() for the mmc host device,
it's important that the host is kept claimed for *all* accesses to it via
the host_ops callbacks.
In some code paths for SDIO, particularly related to the PM support,
mmc_power_off|up() is invoked without keeping the host claimed. Let's fix
these.
Moreover, mmc_start|stop_host() also invokes mmc_power_off|up() without
claiming the host, let's fix these as well.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/core.c | 6 ++++++
drivers/mmc/core/sdio.c | 20 +++++++++++++++-----
2 files changed, 21 insertions(+), 5 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 0520064..ed5f4ad 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2631,10 +2631,14 @@ void mmc_start_host(struct mmc_host *host)
host->f_init = max(freqs[0], host->f_min);
host->rescan_disable = 0;
host->ios.power_mode = MMC_POWER_UNDEFINED;
+
+ mmc_claim_host(host);
if (host->caps2 & MMC_CAP2_NO_PRESCAN_POWERUP)
mmc_power_off(host);
else
mmc_power_up(host, host->ocr_avail);
+ mmc_release_host(host);
+
mmc_gpiod_request_cd_irq(host);
_mmc_detect_change(host, 0, false);
}
@@ -2672,7 +2676,9 @@ void mmc_stop_host(struct mmc_host *host)
BUG_ON(host->card);
+ mmc_claim_host(host);
mmc_power_off(host);
+ mmc_release_host(host);
}
int mmc_power_save_host(struct mmc_host *host)
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index b91abed..95bc101 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -897,11 +897,10 @@ static int mmc_sdio_pre_suspend(struct mmc_host *host)
*/
static int mmc_sdio_suspend(struct mmc_host *host)
{
- if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) {
- mmc_claim_host(host);
+ mmc_claim_host(host);
+
+ if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host))
sdio_disable_wide(host->card);
- mmc_release_host(host);
- }
if (!mmc_card_keep_power(host)) {
mmc_power_off(host);
@@ -910,6 +909,8 @@ static int mmc_sdio_suspend(struct mmc_host *host)
mmc_retune_needed(host);
}
+ mmc_release_host(host);
+
return 0;
}
@@ -1018,15 +1019,24 @@ out:
static int mmc_sdio_runtime_suspend(struct mmc_host *host)
{
/* No references to the card, cut the power to it. */
+ mmc_claim_host(host);
mmc_power_off(host);
+ mmc_release_host(host);
+
return 0;
}
static int mmc_sdio_runtime_resume(struct mmc_host *host)
{
+ int ret;
+
/* Restore power and re-initialize. */
+ mmc_claim_host(host);
mmc_power_up(host, host->card->ocr);
- return mmc_sdio_power_restore(host);
+ ret = mmc_sdio_power_restore(host);
+ mmc_release_host(host);
+
+ return ret;
}
static int mmc_sdio_reset(struct mmc_host *host)
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] mmc: core: Keep host claimed while invoking mmc_power_off|up()
2015-09-11 12:41 [PATCH] mmc: core: Keep host claimed while invoking mmc_power_off|up() Ulf Hansson
@ 2015-09-11 13:22 ` Kishon Vijay Abraham I
2015-09-11 13:59 ` Ulf Hansson
0 siblings, 1 reply; 4+ messages in thread
From: Kishon Vijay Abraham I @ 2015-09-11 13:22 UTC (permalink / raw)
To: Ulf Hansson, linux-mmc
Hi Ulf,
On Friday 11 September 2015 06:11 PM, Ulf Hansson wrote:
> As mmc_claim_host() invokes pm_runtime_get_sync() for the mmc host device,
> it's important that the host is kept claimed for *all* accesses to it via
> the host_ops callbacks.
>
> In some code paths for SDIO, particularly related to the PM support,
> mmc_power_off|up() is invoked without keeping the host claimed. Let's fix
> these.
>
> Moreover, mmc_start|stop_host() also invokes mmc_power_off|up() without
> claiming the host, let's fix these as well.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> drivers/mmc/core/core.c | 6 ++++++
> drivers/mmc/core/sdio.c | 20 +++++++++++++++-----
> 2 files changed, 21 insertions(+), 5 deletions(-)
>
.
.
<snip>
.
.
> int mmc_power_save_host(struct mmc_host *host)
> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
> index b91abed..95bc101 100644
> --- a/drivers/mmc/core/sdio.c
> +++ b/drivers/mmc/core/sdio.c
> @@ -897,11 +897,10 @@ static int mmc_sdio_pre_suspend(struct mmc_host *host)
> */
> static int mmc_sdio_suspend(struct mmc_host *host)
> {
> - if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) {
> - mmc_claim_host(host);
> + mmc_claim_host(host);
> +
> + if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host))
> sdio_disable_wide(host->card);
> - mmc_release_host(host);
> - }
>
> if (!mmc_card_keep_power(host)) {
> mmc_power_off(host);
> @@ -910,6 +909,8 @@ static int mmc_sdio_suspend(struct mmc_host *host)
> mmc_retune_needed(host);
> }
>
> + mmc_release_host(host);
> +
> return 0;
> }
>
> @@ -1018,15 +1019,24 @@ out:
> static int mmc_sdio_runtime_suspend(struct mmc_host *host)
> {
> /* No references to the card, cut the power to it. */
> + mmc_claim_host(host);
> mmc_power_off(host);
> + mmc_release_host(host);
> +
> return 0;
> }
>
> static int mmc_sdio_runtime_resume(struct mmc_host *host)
> {
> + int ret;
> +
> /* Restore power and re-initialize. */
> + mmc_claim_host(host);
> mmc_power_up(host, host->card->ocr);
> - return mmc_sdio_power_restore(host);
> + ret = mmc_sdio_power_restore(host);
IIUC, this will cause mmc_claim_host to be invoked twice since
mmc_sdio_power_restore also invokes mmc_claim_host. I'm not sure if this
will have undesired side effects.
Cheers
Kishon
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] mmc: core: Keep host claimed while invoking mmc_power_off|up()
2015-09-11 13:22 ` Kishon Vijay Abraham I
@ 2015-09-11 13:59 ` Ulf Hansson
2015-09-11 14:07 ` Kishon Vijay Abraham I
0 siblings, 1 reply; 4+ messages in thread
From: Ulf Hansson @ 2015-09-11 13:59 UTC (permalink / raw)
To: Kishon Vijay Abraham I; +Cc: linux-mmc
On 11 September 2015 at 15:22, Kishon Vijay Abraham I <kishon@ti.com> wrote:
> Hi Ulf,
>
> On Friday 11 September 2015 06:11 PM, Ulf Hansson wrote:
>> As mmc_claim_host() invokes pm_runtime_get_sync() for the mmc host device,
>> it's important that the host is kept claimed for *all* accesses to it via
>> the host_ops callbacks.
>>
>> In some code paths for SDIO, particularly related to the PM support,
>> mmc_power_off|up() is invoked without keeping the host claimed. Let's fix
>> these.
>>
>> Moreover, mmc_start|stop_host() also invokes mmc_power_off|up() without
>> claiming the host, let's fix these as well.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>> drivers/mmc/core/core.c | 6 ++++++
>> drivers/mmc/core/sdio.c | 20 +++++++++++++++-----
>> 2 files changed, 21 insertions(+), 5 deletions(-)
>>
> .
> .
> <snip>
> .
> .
>> int mmc_power_save_host(struct mmc_host *host)
>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>> index b91abed..95bc101 100644
>> --- a/drivers/mmc/core/sdio.c
>> +++ b/drivers/mmc/core/sdio.c
>> @@ -897,11 +897,10 @@ static int mmc_sdio_pre_suspend(struct mmc_host *host)
>> */
>> static int mmc_sdio_suspend(struct mmc_host *host)
>> {
>> - if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) {
>> - mmc_claim_host(host);
>> + mmc_claim_host(host);
>> +
>> + if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host))
>> sdio_disable_wide(host->card);
>> - mmc_release_host(host);
>> - }
>>
>> if (!mmc_card_keep_power(host)) {
>> mmc_power_off(host);
>> @@ -910,6 +909,8 @@ static int mmc_sdio_suspend(struct mmc_host *host)
>> mmc_retune_needed(host);
>> }
>>
>> + mmc_release_host(host);
>> +
>> return 0;
>> }
>>
>> @@ -1018,15 +1019,24 @@ out:
>> static int mmc_sdio_runtime_suspend(struct mmc_host *host)
>> {
>> /* No references to the card, cut the power to it. */
>> + mmc_claim_host(host);
>> mmc_power_off(host);
>> + mmc_release_host(host);
>> +
>> return 0;
>> }
>>
>> static int mmc_sdio_runtime_resume(struct mmc_host *host)
>> {
>> + int ret;
>> +
>> /* Restore power and re-initialize. */
>> + mmc_claim_host(host);
>> mmc_power_up(host, host->card->ocr);
>> - return mmc_sdio_power_restore(host);
>> + ret = mmc_sdio_power_restore(host);
>
> IIUC, this will cause mmc_claim_host to be invoked twice since
> mmc_sdio_power_restore also invokes mmc_claim_host. I'm not sure if this
> will have undesired side effects.
That's not a problem, mmc_claim_host() handles that. As long it's the
same running context that tries to claim the host it will allow it.
Kind regards
Uffe
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] mmc: core: Keep host claimed while invoking mmc_power_off|up()
2015-09-11 13:59 ` Ulf Hansson
@ 2015-09-11 14:07 ` Kishon Vijay Abraham I
0 siblings, 0 replies; 4+ messages in thread
From: Kishon Vijay Abraham I @ 2015-09-11 14:07 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc
Hi,
On Friday 11 September 2015 07:29 PM, Ulf Hansson wrote:
> On 11 September 2015 at 15:22, Kishon Vijay Abraham I <kishon@ti.com> wrote:
>> Hi Ulf,
>>
>> On Friday 11 September 2015 06:11 PM, Ulf Hansson wrote:
>>> As mmc_claim_host() invokes pm_runtime_get_sync() for the mmc host device,
>>> it's important that the host is kept claimed for *all* accesses to it via
>>> the host_ops callbacks.
>>>
>>> In some code paths for SDIO, particularly related to the PM support,
>>> mmc_power_off|up() is invoked without keeping the host claimed. Let's fix
>>> these.
>>>
>>> Moreover, mmc_start|stop_host() also invokes mmc_power_off|up() without
>>> claiming the host, let's fix these as well.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>> drivers/mmc/core/core.c | 6 ++++++
>>> drivers/mmc/core/sdio.c | 20 +++++++++++++++-----
>>> 2 files changed, 21 insertions(+), 5 deletions(-)
>>>
>> .
>> .
>> <snip>
>> .
>> .
>>> int mmc_power_save_host(struct mmc_host *host)
>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
>>> index b91abed..95bc101 100644
>>> --- a/drivers/mmc/core/sdio.c
>>> +++ b/drivers/mmc/core/sdio.c
>>> @@ -897,11 +897,10 @@ static int mmc_sdio_pre_suspend(struct mmc_host *host)
>>> */
>>> static int mmc_sdio_suspend(struct mmc_host *host)
>>> {
>>> - if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) {
>>> - mmc_claim_host(host);
>>> + mmc_claim_host(host);
>>> +
>>> + if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host))
>>> sdio_disable_wide(host->card);
>>> - mmc_release_host(host);
>>> - }
>>>
>>> if (!mmc_card_keep_power(host)) {
>>> mmc_power_off(host);
>>> @@ -910,6 +909,8 @@ static int mmc_sdio_suspend(struct mmc_host *host)
>>> mmc_retune_needed(host);
>>> }
>>>
>>> + mmc_release_host(host);
>>> +
>>> return 0;
>>> }
>>>
>>> @@ -1018,15 +1019,24 @@ out:
>>> static int mmc_sdio_runtime_suspend(struct mmc_host *host)
>>> {
>>> /* No references to the card, cut the power to it. */
>>> + mmc_claim_host(host);
>>> mmc_power_off(host);
>>> + mmc_release_host(host);
>>> +
>>> return 0;
>>> }
>>>
>>> static int mmc_sdio_runtime_resume(struct mmc_host *host)
>>> {
>>> + int ret;
>>> +
>>> /* Restore power and re-initialize. */
>>> + mmc_claim_host(host);
>>> mmc_power_up(host, host->card->ocr);
>>> - return mmc_sdio_power_restore(host);
>>> + ret = mmc_sdio_power_restore(host);
>>
>> IIUC, this will cause mmc_claim_host to be invoked twice since
>> mmc_sdio_power_restore also invokes mmc_claim_host. I'm not sure if this
>> will have undesired side effects.
>
> That's not a problem, mmc_claim_host() handles that. As long it's the
> same running context that tries to claim the host it will allow it.
Alright then.
You can add my
Acked-by: Kishon Vijay Abraham I <kishon@ti.com>
Cheers
Kishon
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-09-11 14:07 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-11 12:41 [PATCH] mmc: core: Keep host claimed while invoking mmc_power_off|up() Ulf Hansson
2015-09-11 13:22 ` Kishon Vijay Abraham I
2015-09-11 13:59 ` Ulf Hansson
2015-09-11 14:07 ` Kishon Vijay Abraham I
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).