* [PATCH 1/4] mmc: core: Remove unnecessary check for the remove callback
2013-05-30 12:53 [PATCH 0/4] mmc: core: Let the mmc_bus handle suspend|resume sequence Ulf Hansson
@ 2013-05-30 12:53 ` Ulf Hansson
2013-05-31 7:43 ` Jaehoon Chung
2013-05-30 12:53 ` [PATCH 2/4] mmc: core: Validate suspend prerequisites for SDIO at SUSPEND_PREPARE Ulf Hansson
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2013-05-30 12:53 UTC (permalink / raw)
To: linux-mmc, Chris Ball; +Cc: Ulf Hansson
From: Ulf Hansson <ulf.hansson@linaro.org>
For every bus_ops type the .remove callback always exist, thus there
are no need to check the existence of it, before we decide to call it.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/core.c | 11 +++--------
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index e9a104b..d2ee282 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2483,9 +2483,7 @@ void mmc_stop_host(struct mmc_host *host)
mmc_bus_get(host);
if (host->bus_ops && !host->bus_dead) {
/* Calling bus_ops->remove() with a claimed host can deadlock */
- if (host->bus_ops->remove)
- host->bus_ops->remove(host);
-
+ host->bus_ops->remove(host);
mmc_claim_host(host);
mmc_detach_bus(host);
mmc_power_off(host);
@@ -2638,8 +2636,7 @@ int mmc_suspend_host(struct mmc_host *host)
* bus_ops->remove() with a claimed host can
* deadlock.)
*/
- if (host->bus_ops->remove)
- host->bus_ops->remove(host);
+ host->bus_ops->remove(host);
mmc_claim_host(host);
mmc_detach_bus(host);
mmc_power_off(host);
@@ -2722,9 +2719,7 @@ int mmc_pm_notify(struct notifier_block *notify_block,
break;
/* Calling bus_ops->remove() with a claimed host can deadlock */
- if (host->bus_ops->remove)
- host->bus_ops->remove(host);
-
+ host->bus_ops->remove(host);
mmc_claim_host(host);
mmc_detach_bus(host);
mmc_power_off(host);
--
1.7.10
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 1/4] mmc: core: Remove unnecessary check for the remove callback
2013-05-30 12:53 ` [PATCH 1/4] mmc: core: Remove unnecessary check for the remove callback Ulf Hansson
@ 2013-05-31 7:43 ` Jaehoon Chung
2013-05-31 8:11 ` Ulf Hansson
0 siblings, 1 reply; 16+ messages in thread
From: Jaehoon Chung @ 2013-05-31 7:43 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Chris Ball, Ulf Hansson
Hi Ulf,
According to your commit message, host->bus_ops is also always existed, isn't?
Best Regards,
Jaehoon Chung
On 05/30/2013 09:53 PM, Ulf Hansson wrote:
> From: Ulf Hansson <ulf.hansson@linaro.org>
>
> For every bus_ops type the .remove callback always exist, thus there
> are no need to check the existence of it, before we decide to call it.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> drivers/mmc/core/core.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index e9a104b..d2ee282 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2483,9 +2483,7 @@ void mmc_stop_host(struct mmc_host *host)
> mmc_bus_get(host);
> if (host->bus_ops && !host->bus_dead) {
> /* Calling bus_ops->remove() with a claimed host can deadlock */
> - if (host->bus_ops->remove)
> - host->bus_ops->remove(host);
> -
> + host->bus_ops->remove(host);
> mmc_claim_host(host);
> mmc_detach_bus(host);
> mmc_power_off(host);
> @@ -2638,8 +2636,7 @@ int mmc_suspend_host(struct mmc_host *host)
> * bus_ops->remove() with a claimed host can
> * deadlock.)
> */
> - if (host->bus_ops->remove)
> - host->bus_ops->remove(host);
> + host->bus_ops->remove(host);
> mmc_claim_host(host);
> mmc_detach_bus(host);
> mmc_power_off(host);
> @@ -2722,9 +2719,7 @@ int mmc_pm_notify(struct notifier_block *notify_block,
> break;
>
> /* Calling bus_ops->remove() with a claimed host can deadlock */
> - if (host->bus_ops->remove)
> - host->bus_ops->remove(host);
> -
> + host->bus_ops->remove(host);
> mmc_claim_host(host);
> mmc_detach_bus(host);
> mmc_power_off(host);
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/4] mmc: core: Remove unnecessary check for the remove callback
2013-05-31 7:43 ` Jaehoon Chung
@ 2013-05-31 8:11 ` Ulf Hansson
2013-05-31 8:22 ` Jaehoon Chung
0 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2013-05-31 8:11 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: Ulf Hansson, linux-mmc, Chris Ball
On 31 May 2013 09:43, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi Ulf,
>
> According to your commit message, host->bus_ops is also always existed, isn't?
I am saying that the .remove callback always exist. Maybe I should
clarify it further somehow?
Kind regards
Ulf Hansson
>
> Best Regards,
> Jaehoon Chung
>
> On 05/30/2013 09:53 PM, Ulf Hansson wrote:
>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>
>> For every bus_ops type the .remove callback always exist, thus there
>> are no need to check the existence of it, before we decide to call it.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>> drivers/mmc/core/core.c | 11 +++--------
>> 1 file changed, 3 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index e9a104b..d2ee282 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -2483,9 +2483,7 @@ void mmc_stop_host(struct mmc_host *host)
>> mmc_bus_get(host);
>> if (host->bus_ops && !host->bus_dead) {
>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>> - if (host->bus_ops->remove)
>> - host->bus_ops->remove(host);
>> -
>> + host->bus_ops->remove(host);
>> mmc_claim_host(host);
>> mmc_detach_bus(host);
>> mmc_power_off(host);
>> @@ -2638,8 +2636,7 @@ int mmc_suspend_host(struct mmc_host *host)
>> * bus_ops->remove() with a claimed host can
>> * deadlock.)
>> */
>> - if (host->bus_ops->remove)
>> - host->bus_ops->remove(host);
>> + host->bus_ops->remove(host);
>> mmc_claim_host(host);
>> mmc_detach_bus(host);
>> mmc_power_off(host);
>> @@ -2722,9 +2719,7 @@ int mmc_pm_notify(struct notifier_block *notify_block,
>> break;
>>
>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>> - if (host->bus_ops->remove)
>> - host->bus_ops->remove(host);
>> -
>> + host->bus_ops->remove(host);
>> mmc_claim_host(host);
>> mmc_detach_bus(host);
>> mmc_power_off(host);
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 1/4] mmc: core: Remove unnecessary check for the remove callback
2013-05-31 8:11 ` Ulf Hansson
@ 2013-05-31 8:22 ` Jaehoon Chung
2013-05-31 9:33 ` Ulf Hansson
0 siblings, 1 reply; 16+ messages in thread
From: Jaehoon Chung @ 2013-05-31 8:22 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Jaehoon Chung, Ulf Hansson, linux-mmc, Chris Ball
On 05/31/2013 05:11 PM, Ulf Hansson wrote:
> On 31 May 2013 09:43, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi Ulf,
>>
>> According to your commit message, host->bus_ops is also always existed, isn't?
>
> I am saying that the .remove callback always exist. Maybe I should
> clarify it further somehow?
Sorry if you are confused. My meaning is that according to your commit message,
it didn't need also to check whether host->bus_ops is existed or not.
For example,
if (host->bus_ops && !host->bus_dead) {
Best Regards,
Jaehoon Chung
>
> Kind regards
> Ulf Hansson
>
>>
>> Best Regards,
>> Jaehoon Chung
>>
>> On 05/30/2013 09:53 PM, Ulf Hansson wrote:
>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>>
>>> For every bus_ops type the .remove callback always exist, thus there
>>> are no need to check the existence of it, before we decide to call it.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>> drivers/mmc/core/core.c | 11 +++--------
>>> 1 file changed, 3 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>> index e9a104b..d2ee282 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -2483,9 +2483,7 @@ void mmc_stop_host(struct mmc_host *host)
>>> mmc_bus_get(host);
>>> if (host->bus_ops && !host->bus_dead) {
>>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>>> - if (host->bus_ops->remove)
>>> - host->bus_ops->remove(host);
>>> -
>>> + host->bus_ops->remove(host);
>>> mmc_claim_host(host);
>>> mmc_detach_bus(host);
>>> mmc_power_off(host);
>>> @@ -2638,8 +2636,7 @@ int mmc_suspend_host(struct mmc_host *host)
>>> * bus_ops->remove() with a claimed host can
>>> * deadlock.)
>>> */
>>> - if (host->bus_ops->remove)
>>> - host->bus_ops->remove(host);
>>> + host->bus_ops->remove(host);
>>> mmc_claim_host(host);
>>> mmc_detach_bus(host);
>>> mmc_power_off(host);
>>> @@ -2722,9 +2719,7 @@ int mmc_pm_notify(struct notifier_block *notify_block,
>>> break;
>>>
>>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>>> - if (host->bus_ops->remove)
>>> - host->bus_ops->remove(host);
>>> -
>>> + host->bus_ops->remove(host);
>>> mmc_claim_host(host);
>>> mmc_detach_bus(host);
>>> mmc_power_off(host);
>>>
>>
> --
> 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] 16+ messages in thread* Re: [PATCH 1/4] mmc: core: Remove unnecessary check for the remove callback
2013-05-31 8:22 ` Jaehoon Chung
@ 2013-05-31 9:33 ` Ulf Hansson
2013-05-31 10:50 ` Jaehoon Chung
0 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2013-05-31 9:33 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: Ulf Hansson, linux-mmc, Chris Ball
On 31 May 2013 10:22, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> On 05/31/2013 05:11 PM, Ulf Hansson wrote:
>> On 31 May 2013 09:43, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> Hi Ulf,
>>>
>>> According to your commit message, host->bus_ops is also always existed, isn't?
>>
>> I am saying that the .remove callback always exist. Maybe I should
>> clarify it further somehow?
> Sorry if you are confused. My meaning is that according to your commit message,
> it didn't need also to check whether host->bus_ops is existed or not.
> For example,
> if (host->bus_ops && !host->bus_dead) {
I really can't see this information in the commit msg, I guess we are
reading it differently :-)
Anyway, I will rephrase it so it gets clear.
>
> Best Regards,
> Jaehoon Chung
>>
>> Kind regards
>> Ulf Hansson
>>
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>
>>> On 05/30/2013 09:53 PM, Ulf Hansson wrote:
>>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>>>
>>>> For every bus_ops type the .remove callback always exist, thus there
>>>> are no need to check the existence of it, before we decide to call it.
>>>>
>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>> ---
>>>> drivers/mmc/core/core.c | 11 +++--------
>>>> 1 file changed, 3 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>> index e9a104b..d2ee282 100644
>>>> --- a/drivers/mmc/core/core.c
>>>> +++ b/drivers/mmc/core/core.c
>>>> @@ -2483,9 +2483,7 @@ void mmc_stop_host(struct mmc_host *host)
>>>> mmc_bus_get(host);
>>>> if (host->bus_ops && !host->bus_dead) {
>>>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>>>> - if (host->bus_ops->remove)
>>>> - host->bus_ops->remove(host);
>>>> -
>>>> + host->bus_ops->remove(host);
>>>> mmc_claim_host(host);
>>>> mmc_detach_bus(host);
>>>> mmc_power_off(host);
>>>> @@ -2638,8 +2636,7 @@ int mmc_suspend_host(struct mmc_host *host)
>>>> * bus_ops->remove() with a claimed host can
>>>> * deadlock.)
>>>> */
>>>> - if (host->bus_ops->remove)
>>>> - host->bus_ops->remove(host);
>>>> + host->bus_ops->remove(host);
>>>> mmc_claim_host(host);
>>>> mmc_detach_bus(host);
>>>> mmc_power_off(host);
>>>> @@ -2722,9 +2719,7 @@ int mmc_pm_notify(struct notifier_block *notify_block,
>>>> break;
>>>>
>>>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>>>> - if (host->bus_ops->remove)
>>>> - host->bus_ops->remove(host);
>>>> -
>>>> + host->bus_ops->remove(host);
>>>> mmc_claim_host(host);
>>>> mmc_detach_bus(host);
>>>> mmc_power_off(host);
>>>>
>>>
>> --
>> 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] 16+ messages in thread* Re: [PATCH 1/4] mmc: core: Remove unnecessary check for the remove callback
2013-05-31 9:33 ` Ulf Hansson
@ 2013-05-31 10:50 ` Jaehoon Chung
2013-05-31 15:05 ` Ulf Hansson
0 siblings, 1 reply; 16+ messages in thread
From: Jaehoon Chung @ 2013-05-31 10:50 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Jaehoon Chung, Ulf Hansson, linux-mmc, Chris Ball
Hi Ulf,
Sorry..:-) I understood clear. it's my mis-understanding.
Best Regards,
Jaehoon Chung
On 05/31/2013 06:33 PM, Ulf Hansson wrote:
> On 31 May 2013 10:22, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> On 05/31/2013 05:11 PM, Ulf Hansson wrote:
>>> On 31 May 2013 09:43, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>> Hi Ulf,
>>>>
>>>> According to your commit message, host->bus_ops is also always existed, isn't?
>>>
>>> I am saying that the .remove callback always exist. Maybe I should
>>> clarify it further somehow?
>> Sorry if you are confused. My meaning is that according to your commit message,
>> it didn't need also to check whether host->bus_ops is existed or not.
>> For example,
>> if (host->bus_ops && !host->bus_dead) {
>
> I really can't see this information in the commit msg, I guess we are
> reading it differently :-)
>
> Anyway, I will rephrase it so it gets clear.
>
>>
>> Best Regards,
>> Jaehoon Chung
>>>
>>> Kind regards
>>> Ulf Hansson
>>>
>>>>
>>>> Best Regards,
>>>> Jaehoon Chung
>>>>
>>>> On 05/30/2013 09:53 PM, Ulf Hansson wrote:
>>>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>>>>
>>>>> For every bus_ops type the .remove callback always exist, thus there
>>>>> are no need to check the existence of it, before we decide to call it.
>>>>>
>>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>>> ---
>>>>> drivers/mmc/core/core.c | 11 +++--------
>>>>> 1 file changed, 3 insertions(+), 8 deletions(-)
>>>>>
>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>> index e9a104b..d2ee282 100644
>>>>> --- a/drivers/mmc/core/core.c
>>>>> +++ b/drivers/mmc/core/core.c
>>>>> @@ -2483,9 +2483,7 @@ void mmc_stop_host(struct mmc_host *host)
>>>>> mmc_bus_get(host);
>>>>> if (host->bus_ops && !host->bus_dead) {
>>>>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>>>>> - if (host->bus_ops->remove)
>>>>> - host->bus_ops->remove(host);
>>>>> -
>>>>> + host->bus_ops->remove(host);
>>>>> mmc_claim_host(host);
>>>>> mmc_detach_bus(host);
>>>>> mmc_power_off(host);
>>>>> @@ -2638,8 +2636,7 @@ int mmc_suspend_host(struct mmc_host *host)
>>>>> * bus_ops->remove() with a claimed host can
>>>>> * deadlock.)
>>>>> */
>>>>> - if (host->bus_ops->remove)
>>>>> - host->bus_ops->remove(host);
>>>>> + host->bus_ops->remove(host);
>>>>> mmc_claim_host(host);
>>>>> mmc_detach_bus(host);
>>>>> mmc_power_off(host);
>>>>> @@ -2722,9 +2719,7 @@ int mmc_pm_notify(struct notifier_block *notify_block,
>>>>> break;
>>>>>
>>>>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>>>>> - if (host->bus_ops->remove)
>>>>> - host->bus_ops->remove(host);
>>>>> -
>>>>> + host->bus_ops->remove(host);
>>>>> mmc_claim_host(host);
>>>>> mmc_detach_bus(host);
>>>>> mmc_power_off(host);
>>>>>
>>>>
>>> --
>>> 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] 16+ messages in thread* Re: [PATCH 1/4] mmc: core: Remove unnecessary check for the remove callback
2013-05-31 10:50 ` Jaehoon Chung
@ 2013-05-31 15:05 ` Ulf Hansson
0 siblings, 0 replies; 16+ messages in thread
From: Ulf Hansson @ 2013-05-31 15:05 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: Ulf Hansson, linux-mmc, Chris Ball
On 31 May 2013 12:50, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi Ulf,
>
> Sorry..:-) I understood clear. it's my mis-understanding.
Hi Jaehoon,
OK, cool! :-)
Would be very nice if you had some time to check the other patches in
this series as well.
Thanks!
Ulf Hansson
>
> Best Regards,
> Jaehoon Chung
>
> On 05/31/2013 06:33 PM, Ulf Hansson wrote:
>> On 31 May 2013 10:22, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> On 05/31/2013 05:11 PM, Ulf Hansson wrote:
>>>> On 31 May 2013 09:43, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>>> Hi Ulf,
>>>>>
>>>>> According to your commit message, host->bus_ops is also always existed, isn't?
>>>>
>>>> I am saying that the .remove callback always exist. Maybe I should
>>>> clarify it further somehow?
>>> Sorry if you are confused. My meaning is that according to your commit message,
>>> it didn't need also to check whether host->bus_ops is existed or not.
>>> For example,
>>> if (host->bus_ops && !host->bus_dead) {
>>
>> I really can't see this information in the commit msg, I guess we are
>> reading it differently :-)
>>
>> Anyway, I will rephrase it so it gets clear.
>>
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>>
>>>> Kind regards
>>>> Ulf Hansson
>>>>
>>>>>
>>>>> Best Regards,
>>>>> Jaehoon Chung
>>>>>
>>>>> On 05/30/2013 09:53 PM, Ulf Hansson wrote:
>>>>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>>>>>
>>>>>> For every bus_ops type the .remove callback always exist, thus there
>>>>>> are no need to check the existence of it, before we decide to call it.
>>>>>>
>>>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>>>> ---
>>>>>> drivers/mmc/core/core.c | 11 +++--------
>>>>>> 1 file changed, 3 insertions(+), 8 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>>>> index e9a104b..d2ee282 100644
>>>>>> --- a/drivers/mmc/core/core.c
>>>>>> +++ b/drivers/mmc/core/core.c
>>>>>> @@ -2483,9 +2483,7 @@ void mmc_stop_host(struct mmc_host *host)
>>>>>> mmc_bus_get(host);
>>>>>> if (host->bus_ops && !host->bus_dead) {
>>>>>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>>>>>> - if (host->bus_ops->remove)
>>>>>> - host->bus_ops->remove(host);
>>>>>> -
>>>>>> + host->bus_ops->remove(host);
>>>>>> mmc_claim_host(host);
>>>>>> mmc_detach_bus(host);
>>>>>> mmc_power_off(host);
>>>>>> @@ -2638,8 +2636,7 @@ int mmc_suspend_host(struct mmc_host *host)
>>>>>> * bus_ops->remove() with a claimed host can
>>>>>> * deadlock.)
>>>>>> */
>>>>>> - if (host->bus_ops->remove)
>>>>>> - host->bus_ops->remove(host);
>>>>>> + host->bus_ops->remove(host);
>>>>>> mmc_claim_host(host);
>>>>>> mmc_detach_bus(host);
>>>>>> mmc_power_off(host);
>>>>>> @@ -2722,9 +2719,7 @@ int mmc_pm_notify(struct notifier_block *notify_block,
>>>>>> break;
>>>>>>
>>>>>> /* Calling bus_ops->remove() with a claimed host can deadlock */
>>>>>> - if (host->bus_ops->remove)
>>>>>> - host->bus_ops->remove(host);
>>>>>> -
>>>>>> + host->bus_ops->remove(host);
>>>>>> mmc_claim_host(host);
>>>>>> mmc_detach_bus(host);
>>>>>> mmc_power_off(host);
>>>>>>
>>>>>
>>>> --
>>>> 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] 16+ messages in thread
* [PATCH 2/4] mmc: core: Validate suspend prerequisites for SDIO at SUSPEND_PREPARE
2013-05-30 12:53 [PATCH 0/4] mmc: core: Let the mmc_bus handle suspend|resume sequence Ulf Hansson
2013-05-30 12:53 ` [PATCH 1/4] mmc: core: Remove unnecessary check for the remove callback Ulf Hansson
@ 2013-05-30 12:53 ` Ulf Hansson
2013-05-30 12:53 ` [PATCH 3/4] mmc: core: Push common suspend|resume code into each bus_ops Ulf Hansson
` (2 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: Ulf Hansson @ 2013-05-30 12:53 UTC (permalink / raw)
To: linux-mmc, Chris Ball; +Cc: Ulf Hansson
From: Ulf Hansson <ulf.hansson@linaro.org>
This patch moves the validation for all the suspend prerequisites to be
done at SUSPEND_PREPARE notification. Previously in the SDIO case parts
of the validation was done from mmc_suspend_host.
This patch invents a new pre_suspend bus_ops callback and implements it
for SDIO. Returning an error code from it, will mean at SUSPEND_PREPARE
notification, the card will be removed before proceeding with the
suspend sequence.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/core.c | 25 ++++++++-----------------
drivers/mmc/core/core.h | 1 +
drivers/mmc/core/sdio.c | 27 +++++++++++++++++++++++----
3 files changed, 32 insertions(+), 21 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index d2ee282..7a8a42d 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2628,22 +2628,6 @@ int mmc_suspend_host(struct mmc_host *host)
if (host->bus_ops && !host->bus_dead) {
if (host->bus_ops->suspend)
err = host->bus_ops->suspend(host);
-
- if (err == -ENOSYS || !host->bus_ops->resume) {
- /*
- * We simply "remove" the card in this case.
- * It will be redetected on resume. (Calling
- * bus_ops->remove() with a claimed host can
- * deadlock.)
- */
- host->bus_ops->remove(host);
- mmc_claim_host(host);
- mmc_detach_bus(host);
- mmc_power_off(host);
- mmc_release_host(host);
- host->pm_flags = 0;
- err = 0;
- }
}
mmc_bus_put(host);
@@ -2706,6 +2690,7 @@ int mmc_pm_notify(struct notifier_block *notify_block,
struct mmc_host *host = container_of(
notify_block, struct mmc_host, pm_notify);
unsigned long flags;
+ int err = 0;
switch (mode) {
case PM_HIBERNATION_PREPARE:
@@ -2715,7 +2700,13 @@ int mmc_pm_notify(struct notifier_block *notify_block,
spin_unlock_irqrestore(&host->lock, flags);
cancel_delayed_work_sync(&host->detect);
- if (!host->bus_ops || host->bus_ops->suspend)
+ if (!host->bus_ops)
+ break;
+
+ /* Validate prerequisites for suspend */
+ if (host->bus_ops->pre_suspend)
+ err = host->bus_ops->pre_suspend(host);
+ if (!err && host->bus_ops->suspend)
break;
/* Calling bus_ops->remove() with a claimed host can deadlock */
diff --git a/drivers/mmc/core/core.h b/drivers/mmc/core/core.h
index 52a3650..79f37cf 100644
--- a/drivers/mmc/core/core.h
+++ b/drivers/mmc/core/core.h
@@ -18,6 +18,7 @@
struct mmc_bus_ops {
void (*remove)(struct mmc_host *);
void (*detect)(struct mmc_host *);
+ int (*pre_suspend)(struct mmc_host *);
int (*suspend)(struct mmc_host *);
int (*resume)(struct mmc_host *);
int (*runtime_suspend)(struct mmc_host *);
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index 1fbbd1b..be8cca8 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -910,11 +910,11 @@ out:
}
/*
- * SDIO suspend. We need to suspend all functions separately.
+ * SDIO pre_suspend. We need to suspend all functions separately.
* Therefore all registered functions must have drivers with suspend
* and resume methods. Failing that we simply remove the whole card.
*/
-static int mmc_sdio_suspend(struct mmc_host *host)
+static int mmc_sdio_pre_suspend(struct mmc_host *host)
{
int i, err = 0;
@@ -925,8 +925,26 @@ static int mmc_sdio_suspend(struct mmc_host *host)
if (!pmops || !pmops->suspend || !pmops->resume) {
/* force removal of entire card in that case */
err = -ENOSYS;
- } else
- err = pmops->suspend(&func->dev);
+ break;
+ }
+ }
+ }
+
+ return err;
+}
+
+/*
+ * SDIO suspend. Suspend all functions separately.
+ */
+static int mmc_sdio_suspend(struct mmc_host *host)
+{
+ int i, err = 0;
+
+ for (i = 0; i < host->card->sdio_funcs; i++) {
+ struct sdio_func *func = host->card->sdio_func[i];
+ if (func && sdio_func_present(func) && func->dev.driver) {
+ const struct dev_pm_ops *pmops = func->dev.driver->pm;
+ err = pmops->suspend(&func->dev);
if (err)
break;
}
@@ -1076,6 +1094,7 @@ static int mmc_sdio_runtime_resume(struct mmc_host *host)
static const struct mmc_bus_ops mmc_sdio_ops = {
.remove = mmc_sdio_remove,
.detect = mmc_sdio_detect,
+ .pre_suspend = mmc_sdio_pre_suspend,
.suspend = mmc_sdio_suspend,
.resume = mmc_sdio_resume,
.runtime_suspend = mmc_sdio_runtime_suspend,
--
1.7.10
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 3/4] mmc: core: Push common suspend|resume code into each bus_ops
2013-05-30 12:53 [PATCH 0/4] mmc: core: Let the mmc_bus handle suspend|resume sequence Ulf Hansson
2013-05-30 12:53 ` [PATCH 1/4] mmc: core: Remove unnecessary check for the remove callback Ulf Hansson
2013-05-30 12:53 ` [PATCH 2/4] mmc: core: Validate suspend prerequisites for SDIO at SUSPEND_PREPARE Ulf Hansson
@ 2013-05-30 12:53 ` Ulf Hansson
2013-05-30 12:53 ` [PATCH 4/4] mmc: core: Initiate suspend|resume from mmc bus instead of mmc host Ulf Hansson
2013-06-04 9:53 ` [PATCH 0/4] mmc: core: Let the mmc_bus handle suspend|resume sequence Jaehoon Chung
4 siblings, 0 replies; 16+ messages in thread
From: Ulf Hansson @ 2013-05-30 12:53 UTC (permalink / raw)
To: linux-mmc, Chris Ball; +Cc: Ulf Hansson
From: Ulf Hansson <ulf.hansson@linaro.org>
By moving code from the mmc_suspend|resume_host down into each
.suspend|resume bus_ops callback, we get a more flexible solution.
Some nice side effects are that we get a better understanding of each
bus_ops suspend|resume sequence and the common code don't have to take
care of specific corner cases, especially for the SDIO case.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/core.c | 31 +++----------------------------
drivers/mmc/core/mmc.c | 4 ++++
drivers/mmc/core/sd.c | 4 ++++
drivers/mmc/core/sdio.c | 21 +++++++++++++++++++++
4 files changed, 32 insertions(+), 28 deletions(-)
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 7a8a42d..da3b907 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2621,9 +2621,6 @@ int mmc_suspend_host(struct mmc_host *host)
{
int err = 0;
- cancel_delayed_work(&host->detect);
- mmc_flush_scheduled_work();
-
mmc_bus_get(host);
if (host->bus_ops && !host->bus_dead) {
if (host->bus_ops->suspend)
@@ -2631,9 +2628,6 @@ int mmc_suspend_host(struct mmc_host *host)
}
mmc_bus_put(host);
- if (!err && !mmc_card_keep_power(host))
- mmc_power_off(host);
-
return err;
}
EXPORT_SYMBOL(mmc_suspend_host);
@@ -2644,39 +2638,20 @@ EXPORT_SYMBOL(mmc_suspend_host);
*/
int mmc_resume_host(struct mmc_host *host)
{
- int err = 0;
+ int err;
mmc_bus_get(host);
if (host->bus_ops && !host->bus_dead) {
- if (!mmc_card_keep_power(host)) {
- mmc_power_up(host);
- mmc_select_voltage(host, host->ocr);
- /*
- * Tell runtime PM core we just powered up the card,
- * since it still believes the card is powered off.
- * Note that currently runtime PM is only enabled
- * for SDIO cards that are MMC_CAP_POWER_OFF_CARD
- */
- if (mmc_card_sdio(host->card) &&
- (host->caps & MMC_CAP_POWER_OFF_CARD)) {
- pm_runtime_disable(&host->card->dev);
- pm_runtime_set_active(&host->card->dev);
- pm_runtime_enable(&host->card->dev);
- }
- }
BUG_ON(!host->bus_ops->resume);
err = host->bus_ops->resume(host);
- if (err) {
+ if (err)
pr_warning("%s: error %d during resume "
"(card was removed?)\n",
mmc_hostname(host), err);
- err = 0;
- }
}
- host->pm_flags &= ~MMC_PM_KEEP_POWER;
mmc_bus_put(host);
- return err;
+ return 0;
}
EXPORT_SYMBOL(mmc_resume_host);
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 3a69b94..86ca103 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1494,6 +1494,8 @@ static int mmc_suspend(struct mmc_host *host)
err = mmc_deselect_cards(host);
host->card->state &= ~(MMC_STATE_HIGHSPEED | MMC_STATE_HIGHSPEED_200);
+ if (!err)
+ mmc_power_off(host);
out:
mmc_release_host(host);
return err;
@@ -1513,6 +1515,8 @@ static int mmc_resume(struct mmc_host *host)
BUG_ON(!host->card);
mmc_claim_host(host);
+ mmc_power_up(host);
+ mmc_select_voltage(host, host->ocr);
err = mmc_init_card(host, host->ocr, host->card);
mmc_release_host(host);
diff --git a/drivers/mmc/core/sd.c b/drivers/mmc/core/sd.c
index aeaae7c..cacef27 100644
--- a/drivers/mmc/core/sd.c
+++ b/drivers/mmc/core/sd.c
@@ -1075,6 +1075,8 @@ static int mmc_sd_suspend(struct mmc_host *host)
if (!mmc_host_is_spi(host))
err = mmc_deselect_cards(host);
host->card->state &= ~MMC_STATE_HIGHSPEED;
+ if (!err)
+ mmc_power_off(host);
mmc_release_host(host);
return err;
@@ -1094,6 +1096,8 @@ static int mmc_sd_resume(struct mmc_host *host)
BUG_ON(!host->card);
mmc_claim_host(host);
+ mmc_power_up(host);
+ mmc_select_voltage(host, host->ocr);
err = mmc_sd_init_card(host, host->ocr, host->card);
mmc_release_host(host);
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c
index be8cca8..80d89cff 100644
--- a/drivers/mmc/core/sdio.c
+++ b/drivers/mmc/core/sdio.c
@@ -963,6 +963,9 @@ static int mmc_sdio_suspend(struct mmc_host *host)
mmc_release_host(host);
}
+ if (!err && !mmc_card_keep_power(host))
+ mmc_power_off(host);
+
return err;
}
@@ -976,6 +979,23 @@ static int mmc_sdio_resume(struct mmc_host *host)
/* Basic card reinitialization. */
mmc_claim_host(host);
+ /* Restore power if needed */
+ if (!mmc_card_keep_power(host)) {
+ mmc_power_up(host);
+ mmc_select_voltage(host, host->ocr);
+ /*
+ * Tell runtime PM core we just powered up the card,
+ * since it still believes the card is powered off.
+ * Note that currently runtime PM is only enabled
+ * for SDIO cards that are MMC_CAP_POWER_OFF_CARD
+ */
+ if (host->caps & MMC_CAP_POWER_OFF_CARD) {
+ pm_runtime_disable(&host->card->dev);
+ pm_runtime_set_active(&host->card->dev);
+ pm_runtime_enable(&host->card->dev);
+ }
+ }
+
/* No need to reinitialize powered-resumed nonremovable cards */
if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) {
sdio_reset(host);
@@ -1013,6 +1033,7 @@ static int mmc_sdio_resume(struct mmc_host *host)
}
}
+ host->pm_flags &= ~MMC_PM_KEEP_POWER;
return err;
}
--
1.7.10
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH 4/4] mmc: core: Initiate suspend|resume from mmc bus instead of mmc host
2013-05-30 12:53 [PATCH 0/4] mmc: core: Let the mmc_bus handle suspend|resume sequence Ulf Hansson
` (2 preceding siblings ...)
2013-05-30 12:53 ` [PATCH 3/4] mmc: core: Push common suspend|resume code into each bus_ops Ulf Hansson
@ 2013-05-30 12:53 ` Ulf Hansson
2013-06-04 5:28 ` Jaehoon Chung
2013-06-04 9:53 ` [PATCH 0/4] mmc: core: Let the mmc_bus handle suspend|resume sequence Jaehoon Chung
4 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2013-05-30 12:53 UTC (permalink / raw)
To: linux-mmc, Chris Ball; +Cc: Ulf Hansson
From: Ulf Hansson <ulf.hansson@linaro.org>
The host should be responsible to suspend|resume the host and not the
card. This patch changes this behaviour, by moving the responsiblity
to the mmc bus instead which already holds the card device.
The exported functions mmc_suspend|resume_host are now to be considered
as depcrecated. Once all host drivers moves away from using them, we
can remove them. As of now, a successful error code is always returned.
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
drivers/mmc/core/bus.c | 15 ++++++++++++++-
drivers/mmc/core/core.c | 26 +++-----------------------
2 files changed, 17 insertions(+), 24 deletions(-)
diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
index d9e8c2b..2842684 100644
--- a/drivers/mmc/core/bus.c
+++ b/drivers/mmc/core/bus.c
@@ -127,10 +127,16 @@ static int mmc_bus_suspend(struct device *dev)
{
struct mmc_driver *drv = to_mmc_driver(dev->driver);
struct mmc_card *card = mmc_dev_to_card(dev);
+ struct mmc_host *host = card->host;
int ret = 0;
- if (dev->driver && drv->suspend)
+ if (dev->driver && drv->suspend) {
ret = drv->suspend(card);
+ if (ret)
+ return ret;
+ }
+
+ ret = host->bus_ops->suspend(host);
return ret;
}
@@ -138,10 +144,17 @@ static int mmc_bus_resume(struct device *dev)
{
struct mmc_driver *drv = to_mmc_driver(dev->driver);
struct mmc_card *card = mmc_dev_to_card(dev);
+ struct mmc_host *host = card->host;
int ret = 0;
+ ret = host->bus_ops->resume(host);
+ if (ret)
+ pr_warn("%s: error %d during resume (card was removed?)\n",
+ mmc_hostname(host), ret);
+
if (dev->driver && drv->resume)
ret = drv->resume(card);
+
return ret;
}
#endif
diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index da3b907..49a5bca 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2619,16 +2619,8 @@ EXPORT_SYMBOL(mmc_cache_ctrl);
*/
int mmc_suspend_host(struct mmc_host *host)
{
- int err = 0;
-
- mmc_bus_get(host);
- if (host->bus_ops && !host->bus_dead) {
- if (host->bus_ops->suspend)
- err = host->bus_ops->suspend(host);
- }
- mmc_bus_put(host);
-
- return err;
+ /* This function is deprecated */
+ return 0;
}
EXPORT_SYMBOL(mmc_suspend_host);
@@ -2638,19 +2630,7 @@ EXPORT_SYMBOL(mmc_suspend_host);
*/
int mmc_resume_host(struct mmc_host *host)
{
- int err;
-
- mmc_bus_get(host);
- if (host->bus_ops && !host->bus_dead) {
- BUG_ON(!host->bus_ops->resume);
- err = host->bus_ops->resume(host);
- if (err)
- pr_warning("%s: error %d during resume "
- "(card was removed?)\n",
- mmc_hostname(host), err);
- }
- mmc_bus_put(host);
-
+ /* This function is deprecated */
return 0;
}
EXPORT_SYMBOL(mmc_resume_host);
--
1.7.10
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [PATCH 4/4] mmc: core: Initiate suspend|resume from mmc bus instead of mmc host
2013-05-30 12:53 ` [PATCH 4/4] mmc: core: Initiate suspend|resume from mmc bus instead of mmc host Ulf Hansson
@ 2013-06-04 5:28 ` Jaehoon Chung
2013-06-04 8:34 ` Ulf Hansson
0 siblings, 1 reply; 16+ messages in thread
From: Jaehoon Chung @ 2013-06-04 5:28 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Chris Ball, Ulf Hansson
Hi Ulf,
On 05/30/2013 09:53 PM, Ulf Hansson wrote:
> From: Ulf Hansson <ulf.hansson@linaro.org>
>
> The host should be responsible to suspend|resume the host and not the
> card. This patch changes this behaviour, by moving the responsiblity
> to the mmc bus instead which already holds the card device.
>
> The exported functions mmc_suspend|resume_host are now to be considered
> as depcrecated. Once all host drivers moves away from using them, we
> can remove them. As of now, a successful error code is always returned.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> drivers/mmc/core/bus.c | 15 ++++++++++++++-
> drivers/mmc/core/core.c | 26 +++-----------------------
> 2 files changed, 17 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
> index d9e8c2b..2842684 100644
> --- a/drivers/mmc/core/bus.c
> +++ b/drivers/mmc/core/bus.c
> @@ -127,10 +127,16 @@ static int mmc_bus_suspend(struct device *dev)
> {
> struct mmc_driver *drv = to_mmc_driver(dev->driver);
> struct mmc_card *card = mmc_dev_to_card(dev);
> + struct mmc_host *host = card->host;
> int ret = 0;
>
> - if (dev->driver && drv->suspend)
> + if (dev->driver && drv->suspend) {
> ret = drv->suspend(card);
> + if (ret)
> + return ret;
> + }
> +
> + ret = host->bus_ops->suspend(host);
Need not to check whether host->bus_ops->suspend is existed or not?
> return ret;
> }
>
> @@ -138,10 +144,17 @@ static int mmc_bus_resume(struct device *dev)
> {
> struct mmc_driver *drv = to_mmc_driver(dev->driver);
> struct mmc_card *card = mmc_dev_to_card(dev);
> + struct mmc_host *host = card->host;
> int ret = 0;
>
> + ret = host->bus_ops->resume(host);
Ditto
Best Regards,
Jaehoon Chung
> + if (ret)
> + pr_warn("%s: error %d during resume (card was removed?)\n",
> + mmc_hostname(host), ret);
> +
> if (dev->driver && drv->resume)
> ret = drv->resume(card);
> +
> return ret;
> }
> #endif
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index da3b907..49a5bca 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2619,16 +2619,8 @@ EXPORT_SYMBOL(mmc_cache_ctrl);
> */
> int mmc_suspend_host(struct mmc_host *host)
> {
> - int err = 0;
> -
> - mmc_bus_get(host);
> - if (host->bus_ops && !host->bus_dead) {
> - if (host->bus_ops->suspend)
> - err = host->bus_ops->suspend(host);
> - }
> - mmc_bus_put(host);
> -
> - return err;
> + /* This function is deprecated */
> + return 0;
> }
> EXPORT_SYMBOL(mmc_suspend_host);
>
> @@ -2638,19 +2630,7 @@ EXPORT_SYMBOL(mmc_suspend_host);
> */
> int mmc_resume_host(struct mmc_host *host)
> {
> - int err;
> -
> - mmc_bus_get(host);
> - if (host->bus_ops && !host->bus_dead) {
> - BUG_ON(!host->bus_ops->resume);
> - err = host->bus_ops->resume(host);
> - if (err)
> - pr_warning("%s: error %d during resume "
> - "(card was removed?)\n",
> - mmc_hostname(host), err);
> - }
> - mmc_bus_put(host);
> -
> + /* This function is deprecated */
> return 0;
> }
> EXPORT_SYMBOL(mmc_resume_host);
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 4/4] mmc: core: Initiate suspend|resume from mmc bus instead of mmc host
2013-06-04 5:28 ` Jaehoon Chung
@ 2013-06-04 8:34 ` Ulf Hansson
2013-06-04 8:47 ` Jaehoon Chung
0 siblings, 1 reply; 16+ messages in thread
From: Ulf Hansson @ 2013-06-04 8:34 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: Ulf Hansson, linux-mmc, Chris Ball
On 4 June 2013 07:28, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Hi Ulf,
>
> On 05/30/2013 09:53 PM, Ulf Hansson wrote:
>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>
>> The host should be responsible to suspend|resume the host and not the
>> card. This patch changes this behaviour, by moving the responsiblity
>> to the mmc bus instead which already holds the card device.
>>
>> The exported functions mmc_suspend|resume_host are now to be considered
>> as depcrecated. Once all host drivers moves away from using them, we
>> can remove them. As of now, a successful error code is always returned.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> ---
>> drivers/mmc/core/bus.c | 15 ++++++++++++++-
>> drivers/mmc/core/core.c | 26 +++-----------------------
>> 2 files changed, 17 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
>> index d9e8c2b..2842684 100644
>> --- a/drivers/mmc/core/bus.c
>> +++ b/drivers/mmc/core/bus.c
>> @@ -127,10 +127,16 @@ static int mmc_bus_suspend(struct device *dev)
>> {
>> struct mmc_driver *drv = to_mmc_driver(dev->driver);
>> struct mmc_card *card = mmc_dev_to_card(dev);
>> + struct mmc_host *host = card->host;
>> int ret = 0;
>>
>> - if (dev->driver && drv->suspend)
>> + if (dev->driver && drv->suspend) {
>> ret = drv->suspend(card);
>> + if (ret)
>> + return ret;
>> + }
>> +
>> + ret = host->bus_ops->suspend(host);
> Need not to check whether host->bus_ops->suspend is existed or not?
We don't need to check it here.
It is only for those cards that were not removed from the
mmc_pm_notify function (PM_SUSPEND_PREPARE) that get suspended here.
And the validation of the bus_ops has then already been done.
>> return ret;
>> }
>>
>> @@ -138,10 +144,17 @@ static int mmc_bus_resume(struct device *dev)
>> {
>> struct mmc_driver *drv = to_mmc_driver(dev->driver);
>> struct mmc_card *card = mmc_dev_to_card(dev);
>> + struct mmc_host *host = card->host;
>> int ret = 0;
>>
>> + ret = host->bus_ops->resume(host);
> Ditto
See comment above. Moreover, in the case were a bus_ops>suspend
function exist, there also exist an bus_ops->resume function.
Kind regards
Ulf Hansson
>
> Best Regards,
> Jaehoon Chung
>> + if (ret)
>> + pr_warn("%s: error %d during resume (card was removed?)\n",
>> + mmc_hostname(host), ret);
>> +
>> if (dev->driver && drv->resume)
>> ret = drv->resume(card);
>> +
>> return ret;
>> }
>> #endif
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index da3b907..49a5bca 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -2619,16 +2619,8 @@ EXPORT_SYMBOL(mmc_cache_ctrl);
>> */
>> int mmc_suspend_host(struct mmc_host *host)
>> {
>> - int err = 0;
>> -
>> - mmc_bus_get(host);
>> - if (host->bus_ops && !host->bus_dead) {
>> - if (host->bus_ops->suspend)
>> - err = host->bus_ops->suspend(host);
>> - }
>> - mmc_bus_put(host);
>> -
>> - return err;
>> + /* This function is deprecated */
>> + return 0;
>> }
>> EXPORT_SYMBOL(mmc_suspend_host);
>>
>> @@ -2638,19 +2630,7 @@ EXPORT_SYMBOL(mmc_suspend_host);
>> */
>> int mmc_resume_host(struct mmc_host *host)
>> {
>> - int err;
>> -
>> - mmc_bus_get(host);
>> - if (host->bus_ops && !host->bus_dead) {
>> - BUG_ON(!host->bus_ops->resume);
>> - err = host->bus_ops->resume(host);
>> - if (err)
>> - pr_warning("%s: error %d during resume "
>> - "(card was removed?)\n",
>> - mmc_hostname(host), err);
>> - }
>> - mmc_bus_put(host);
>> -
>> + /* This function is deprecated */
>> return 0;
>> }
>> EXPORT_SYMBOL(mmc_resume_host);
>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH 4/4] mmc: core: Initiate suspend|resume from mmc bus instead of mmc host
2013-06-04 8:34 ` Ulf Hansson
@ 2013-06-04 8:47 ` Jaehoon Chung
2013-06-04 8:57 ` Ulf Hansson
0 siblings, 1 reply; 16+ messages in thread
From: Jaehoon Chung @ 2013-06-04 8:47 UTC (permalink / raw)
To: Ulf Hansson; +Cc: Jaehoon Chung, Ulf Hansson, linux-mmc, Chris Ball
Thanks for your explanation.
I have tested your patch with sdhci and dw-mmc controller.
(It's working fine <eMMC/SD-card/SDIO> with exynos)
Best Regards,
Jaehoon Chung
On 06/04/2013 05:34 PM, Ulf Hansson wrote:
> On 4 June 2013 07:28, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>> Hi Ulf,
>>
>> On 05/30/2013 09:53 PM, Ulf Hansson wrote:
>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>>
>>> The host should be responsible to suspend|resume the host and not the
>>> card. This patch changes this behaviour, by moving the responsiblity
>>> to the mmc bus instead which already holds the card device.
>>>
>>> The exported functions mmc_suspend|resume_host are now to be considered
>>> as depcrecated. Once all host drivers moves away from using them, we
>>> can remove them. As of now, a successful error code is always returned.
>>>
>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> ---
>>> drivers/mmc/core/bus.c | 15 ++++++++++++++-
>>> drivers/mmc/core/core.c | 26 +++-----------------------
>>> 2 files changed, 17 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
>>> index d9e8c2b..2842684 100644
>>> --- a/drivers/mmc/core/bus.c
>>> +++ b/drivers/mmc/core/bus.c
>>> @@ -127,10 +127,16 @@ static int mmc_bus_suspend(struct device *dev)
>>> {
>>> struct mmc_driver *drv = to_mmc_driver(dev->driver);
>>> struct mmc_card *card = mmc_dev_to_card(dev);
>>> + struct mmc_host *host = card->host;
>>> int ret = 0;
>>>
>>> - if (dev->driver && drv->suspend)
>>> + if (dev->driver && drv->suspend) {
>>> ret = drv->suspend(card);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> +
>>> + ret = host->bus_ops->suspend(host);
>> Need not to check whether host->bus_ops->suspend is existed or not?
>
> We don't need to check it here.
>
> It is only for those cards that were not removed from the
> mmc_pm_notify function (PM_SUSPEND_PREPARE) that get suspended here.
> And the validation of the bus_ops has then already been done.
>
>>> return ret;
>>> }
>>>
>>> @@ -138,10 +144,17 @@ static int mmc_bus_resume(struct device *dev)
>>> {
>>> struct mmc_driver *drv = to_mmc_driver(dev->driver);
>>> struct mmc_card *card = mmc_dev_to_card(dev);
>>> + struct mmc_host *host = card->host;
>>> int ret = 0;
>>>
>>> + ret = host->bus_ops->resume(host);
>> Ditto
>
> See comment above. Moreover, in the case were a bus_ops>suspend
> function exist, there also exist an bus_ops->resume function.
>
>
> Kind regards
> Ulf Hansson
>
>>
>> Best Regards,
>> Jaehoon Chung
>>> + if (ret)
>>> + pr_warn("%s: error %d during resume (card was removed?)\n",
>>> + mmc_hostname(host), ret);
>>> +
>>> if (dev->driver && drv->resume)
>>> ret = drv->resume(card);
>>> +
>>> return ret;
>>> }
>>> #endif
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>> index da3b907..49a5bca 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -2619,16 +2619,8 @@ EXPORT_SYMBOL(mmc_cache_ctrl);
>>> */
>>> int mmc_suspend_host(struct mmc_host *host)
>>> {
>>> - int err = 0;
>>> -
>>> - mmc_bus_get(host);
>>> - if (host->bus_ops && !host->bus_dead) {
>>> - if (host->bus_ops->suspend)
>>> - err = host->bus_ops->suspend(host);
>>> - }
>>> - mmc_bus_put(host);
>>> -
>>> - return err;
>>> + /* This function is deprecated */
>>> + return 0;
>>> }
>>> EXPORT_SYMBOL(mmc_suspend_host);
>>>
>>> @@ -2638,19 +2630,7 @@ EXPORT_SYMBOL(mmc_suspend_host);
>>> */
>>> int mmc_resume_host(struct mmc_host *host)
>>> {
>>> - int err;
>>> -
>>> - mmc_bus_get(host);
>>> - if (host->bus_ops && !host->bus_dead) {
>>> - BUG_ON(!host->bus_ops->resume);
>>> - err = host->bus_ops->resume(host);
>>> - if (err)
>>> - pr_warning("%s: error %d during resume "
>>> - "(card was removed?)\n",
>>> - mmc_hostname(host), err);
>>> - }
>>> - mmc_bus_put(host);
>>> -
>>> + /* This function is deprecated */
>>> return 0;
>>> }
>>> EXPORT_SYMBOL(mmc_resume_host);
>>>
>>
> --
> 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] 16+ messages in thread* Re: [PATCH 4/4] mmc: core: Initiate suspend|resume from mmc bus instead of mmc host
2013-06-04 8:47 ` Jaehoon Chung
@ 2013-06-04 8:57 ` Ulf Hansson
0 siblings, 0 replies; 16+ messages in thread
From: Ulf Hansson @ 2013-06-04 8:57 UTC (permalink / raw)
To: Jaehoon Chung; +Cc: Ulf Hansson, linux-mmc, Chris Ball
On 4 June 2013 10:47, Jaehoon Chung <jh80.chung@samsung.com> wrote:
> Thanks for your explanation.
>
> I have tested your patch with sdhci and dw-mmc controller.
> (It's working fine <eMMC/SD-card/SDIO> with exynos)
Thanks a lot for your help Jaehoon. So we could add your "Tested-by"
on the hole patch set then?
Kind regards
Ulf Hansson
>
> Best Regards,
> Jaehoon Chung
>
> On 06/04/2013 05:34 PM, Ulf Hansson wrote:
>> On 4 June 2013 07:28, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>> Hi Ulf,
>>>
>>> On 05/30/2013 09:53 PM, Ulf Hansson wrote:
>>>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>>>
>>>> The host should be responsible to suspend|resume the host and not the
>>>> card. This patch changes this behaviour, by moving the responsiblity
>>>> to the mmc bus instead which already holds the card device.
>>>>
>>>> The exported functions mmc_suspend|resume_host are now to be considered
>>>> as depcrecated. Once all host drivers moves away from using them, we
>>>> can remove them. As of now, a successful error code is always returned.
>>>>
>>>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>>> ---
>>>> drivers/mmc/core/bus.c | 15 ++++++++++++++-
>>>> drivers/mmc/core/core.c | 26 +++-----------------------
>>>> 2 files changed, 17 insertions(+), 24 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/core/bus.c b/drivers/mmc/core/bus.c
>>>> index d9e8c2b..2842684 100644
>>>> --- a/drivers/mmc/core/bus.c
>>>> +++ b/drivers/mmc/core/bus.c
>>>> @@ -127,10 +127,16 @@ static int mmc_bus_suspend(struct device *dev)
>>>> {
>>>> struct mmc_driver *drv = to_mmc_driver(dev->driver);
>>>> struct mmc_card *card = mmc_dev_to_card(dev);
>>>> + struct mmc_host *host = card->host;
>>>> int ret = 0;
>>>>
>>>> - if (dev->driver && drv->suspend)
>>>> + if (dev->driver && drv->suspend) {
>>>> ret = drv->suspend(card);
>>>> + if (ret)
>>>> + return ret;
>>>> + }
>>>> +
>>>> + ret = host->bus_ops->suspend(host);
>>> Need not to check whether host->bus_ops->suspend is existed or not?
>>
>> We don't need to check it here.
>>
>> It is only for those cards that were not removed from the
>> mmc_pm_notify function (PM_SUSPEND_PREPARE) that get suspended here.
>> And the validation of the bus_ops has then already been done.
>>
>>>> return ret;
>>>> }
>>>>
>>>> @@ -138,10 +144,17 @@ static int mmc_bus_resume(struct device *dev)
>>>> {
>>>> struct mmc_driver *drv = to_mmc_driver(dev->driver);
>>>> struct mmc_card *card = mmc_dev_to_card(dev);
>>>> + struct mmc_host *host = card->host;
>>>> int ret = 0;
>>>>
>>>> + ret = host->bus_ops->resume(host);
>>> Ditto
>>
>> See comment above. Moreover, in the case were a bus_ops>suspend
>> function exist, there also exist an bus_ops->resume function.
>>
>>
>> Kind regards
>> Ulf Hansson
>>
>>>
>>> Best Regards,
>>> Jaehoon Chung
>>>> + if (ret)
>>>> + pr_warn("%s: error %d during resume (card was removed?)\n",
>>>> + mmc_hostname(host), ret);
>>>> +
>>>> if (dev->driver && drv->resume)
>>>> ret = drv->resume(card);
>>>> +
>>>> return ret;
>>>> }
>>>> #endif
>>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>>> index da3b907..49a5bca 100644
>>>> --- a/drivers/mmc/core/core.c
>>>> +++ b/drivers/mmc/core/core.c
>>>> @@ -2619,16 +2619,8 @@ EXPORT_SYMBOL(mmc_cache_ctrl);
>>>> */
>>>> int mmc_suspend_host(struct mmc_host *host)
>>>> {
>>>> - int err = 0;
>>>> -
>>>> - mmc_bus_get(host);
>>>> - if (host->bus_ops && !host->bus_dead) {
>>>> - if (host->bus_ops->suspend)
>>>> - err = host->bus_ops->suspend(host);
>>>> - }
>>>> - mmc_bus_put(host);
>>>> -
>>>> - return err;
>>>> + /* This function is deprecated */
>>>> + return 0;
>>>> }
>>>> EXPORT_SYMBOL(mmc_suspend_host);
>>>>
>>>> @@ -2638,19 +2630,7 @@ EXPORT_SYMBOL(mmc_suspend_host);
>>>> */
>>>> int mmc_resume_host(struct mmc_host *host)
>>>> {
>>>> - int err;
>>>> -
>>>> - mmc_bus_get(host);
>>>> - if (host->bus_ops && !host->bus_dead) {
>>>> - BUG_ON(!host->bus_ops->resume);
>>>> - err = host->bus_ops->resume(host);
>>>> - if (err)
>>>> - pr_warning("%s: error %d during resume "
>>>> - "(card was removed?)\n",
>>>> - mmc_hostname(host), err);
>>>> - }
>>>> - mmc_bus_put(host);
>>>> -
>>>> + /* This function is deprecated */
>>>> return 0;
>>>> }
>>>> EXPORT_SYMBOL(mmc_resume_host);
>>>>
>>>
>> --
>> 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] 16+ messages in thread
* Re: [PATCH 0/4] mmc: core: Let the mmc_bus handle suspend|resume sequence
2013-05-30 12:53 [PATCH 0/4] mmc: core: Let the mmc_bus handle suspend|resume sequence Ulf Hansson
` (3 preceding siblings ...)
2013-05-30 12:53 ` [PATCH 4/4] mmc: core: Initiate suspend|resume from mmc bus instead of mmc host Ulf Hansson
@ 2013-06-04 9:53 ` Jaehoon Chung
4 siblings, 0 replies; 16+ messages in thread
From: Jaehoon Chung @ 2013-06-04 9:53 UTC (permalink / raw)
To: Ulf Hansson; +Cc: linux-mmc, Chris Ball, Ulf Hansson
Tested-by: Jaehoon Chung <jh80.chung@samsung.com>
Best Regards,
Jaehoon Chung
On 05/30/2013 09:53 PM, Ulf Hansson wrote:
> From: Ulf Hansson <ulf.hansson@linaro.org>
>
> The MMC/SD/SDIO cards are registered on the mmc_bus and should from a power
> management perspective be controlled from there. As of today each and every
> host driver needs to issue mmc_suspend|resume_host from their respective
> .suspend|resume methods, which seems like an unnecessary requirement to
> put on them.
>
> This patch set moves the responsiblity to suspend the cards into the mmc_bus.
> In this patch set, the mmc_suspend|resume_host functions are not removed, but
> will instead always return successful. As a separate patch build on top of
> this patch set, we shall remove the API:s together with updating each and
> every host driver.
>
> Ulf Hansson (4):
> mmc: core: Remove unnecessary check for the remove callback
> mmc: core: Validate suspend prerequisites for SDIO at SUSPEND_PREPARE
> mmc: core: Push common suspend|resume code into each bus_ops
> mmc: core: Initiate suspend|resume from mmc bus instead of mmc host
>
> drivers/mmc/core/bus.c | 28 ++++++++++++++-
> drivers/mmc/core/core.c | 87 ++++++++---------------------------------------
> drivers/mmc/core/core.h | 1 +
> drivers/mmc/core/mmc.c | 4 +++
> drivers/mmc/core/sd.c | 4 +++
> drivers/mmc/core/sdio.c | 48 +++++++++++++++++++++++---
> 6 files changed, 94 insertions(+), 78 deletions(-)
>
^ permalink raw reply [flat|nested] 16+ messages in thread