| Message ID | 1354620980-23764-2-git-send-email-subhashj@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 4 December 2012 12:36, Subhash Jadavani <subhashj@codeaurora.org> wrote: > If SDIO keep power flag (MMC_PM_KEEP_POWER) is not set, card would > be reinitialized during resume but as we are not resetting > (CMD52 reset) the SDIO card during this reinitialization, card may > fail to respond back to subsequent commands (CMD5 etc...). > > This change resets the card before the reinitialization of card > during resume. > > Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> > --- > drivers/mmc/core/sdio.c | 6 ++++-- > 1 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c > index 2273ce6..34ad4c8 100644 > --- a/drivers/mmc/core/sdio.c > +++ b/drivers/mmc/core/sdio.c > @@ -937,10 +937,12 @@ static int mmc_sdio_resume(struct mmc_host *host) > mmc_claim_host(host); > > /* No need to reinitialize powered-resumed nonremovable cards */ > - if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) > + if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) { > + sdio_reset(host); > + mmc_go_idle(host); If the card has lost power, why is this needed? Could it be that the host is not capable of cutting the power to card? It might be more safe to do this but I would like to understand more, before you get my ack on this patch. > err = mmc_sdio_init_card(host, host->ocr, host->card, > mmc_card_keep_power(host)); > - else if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) { > + } else if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) { > /* We may have switched to 1-bit mode during suspend */ > err = sdio_enable_4bit_bus(host->card); > if (err > 0) { > -- > -- > QUALCOMM INDIA, 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 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
On 12/6/2012 4:03 PM, Ulf Hansson wrote: > On 4 December 2012 12:36, Subhash Jadavani <subhashj@codeaurora.org> wrote: >> If SDIO keep power flag (MMC_PM_KEEP_POWER) is not set, card would >> be reinitialized during resume but as we are not resetting >> (CMD52 reset) the SDIO card during this reinitialization, card may >> fail to respond back to subsequent commands (CMD5 etc...). >> >> This change resets the card before the reinitialization of card >> during resume. >> >> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> >> --- >> drivers/mmc/core/sdio.c | 6 ++++-- >> 1 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >> index 2273ce6..34ad4c8 100644 >> --- a/drivers/mmc/core/sdio.c >> +++ b/drivers/mmc/core/sdio.c >> @@ -937,10 +937,12 @@ static int mmc_sdio_resume(struct mmc_host *host) >> mmc_claim_host(host); >> >> /* No need to reinitialize powered-resumed nonremovable cards */ >> - if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) >> + if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) { >> + sdio_reset(host); >> + mmc_go_idle(host); > If the card has lost power, why is this needed? Could it be that the > host is not capable of cutting the power to card? Yes, the regulator powering the card is always on in this case which means SDIO VDD is not powered off during the suspend. > > It might be more safe to do this but I would like to understand more, > before you get my ack on this patch. > >> err = mmc_sdio_init_card(host, host->ocr, host->card, >> mmc_card_keep_power(host)); >> - else if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) { >> + } else if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) { >> /* We may have switched to 1-bit mode during suspend */ >> err = sdio_enable_4bit_bus(host->card); >> if (err > 0) { >> -- >> -- >> QUALCOMM INDIA, 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 > 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
On 6 December 2012 16:25, Subhash Jadavani <subhashj@codeaurora.org> wrote: > > On 12/6/2012 4:03 PM, Ulf Hansson wrote: >> >> On 4 December 2012 12:36, Subhash Jadavani <subhashj@codeaurora.org> >> wrote: >>> >>> If SDIO keep power flag (MMC_PM_KEEP_POWER) is not set, card would >>> be reinitialized during resume but as we are not resetting >>> (CMD52 reset) the SDIO card during this reinitialization, card may >>> fail to respond back to subsequent commands (CMD5 etc...). >>> >>> This change resets the card before the reinitialization of card >>> during resume. >>> >>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> >>> --- >>> drivers/mmc/core/sdio.c | 6 ++++-- >>> 1 files changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >>> index 2273ce6..34ad4c8 100644 >>> --- a/drivers/mmc/core/sdio.c >>> +++ b/drivers/mmc/core/sdio.c >>> @@ -937,10 +937,12 @@ static int mmc_sdio_resume(struct mmc_host *host) >>> mmc_claim_host(host); >>> >>> /* No need to reinitialize powered-resumed nonremovable cards */ >>> - if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) >>> + if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) { >>> + sdio_reset(host); >>> + mmc_go_idle(host); >> >> If the card has lost power, why is this needed? Could it be that the >> host is not capable of cutting the power to card? > > > Yes, the regulator powering the card is always on in this case which means > SDIO VDD is not powered off during the suspend. > So in principle the host should be able to notify the protocol layer about that mmc_card_keep_power will always be set somehow. That could be a more proper way of solving this maybe? What do you think? > >> >> It might be more safe to do this but I would like to understand more, >> before you get my ack on this patch. >> >>> err = mmc_sdio_init_card(host, host->ocr, host->card, >>> mmc_card_keep_power(host)); >>> - else if (mmc_card_keep_power(host) && >>> mmc_card_wake_sdio_irq(host)) { >>> + } else if (mmc_card_keep_power(host) && >>> mmc_card_wake_sdio_irq(host)) { >>> /* We may have switched to 1-bit mode during suspend */ >>> err = sdio_enable_4bit_bus(host->card); >>> if (err > 0) { >>> -- >>> -- >>> QUALCOMM INDIA, 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 >> >> 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
On 12/7/2012 3:40 AM, Ulf Hansson wrote: > On 6 December 2012 16:25, Subhash Jadavani <subhashj@codeaurora.org> wrote: >> On 12/6/2012 4:03 PM, Ulf Hansson wrote: >>> On 4 December 2012 12:36, Subhash Jadavani <subhashj@codeaurora.org> >>> wrote: >>>> If SDIO keep power flag (MMC_PM_KEEP_POWER) is not set, card would >>>> be reinitialized during resume but as we are not resetting >>>> (CMD52 reset) the SDIO card during this reinitialization, card may >>>> fail to respond back to subsequent commands (CMD5 etc...). >>>> >>>> This change resets the card before the reinitialization of card >>>> during resume. >>>> >>>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> >>>> --- >>>> drivers/mmc/core/sdio.c | 6 ++++-- >>>> 1 files changed, 4 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >>>> index 2273ce6..34ad4c8 100644 >>>> --- a/drivers/mmc/core/sdio.c >>>> +++ b/drivers/mmc/core/sdio.c >>>> @@ -937,10 +937,12 @@ static int mmc_sdio_resume(struct mmc_host *host) >>>> mmc_claim_host(host); >>>> >>>> /* No need to reinitialize powered-resumed nonremovable cards */ >>>> - if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) >>>> + if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) { >>>> + sdio_reset(host); >>>> + mmc_go_idle(host); >>> If the card has lost power, why is this needed? Could it be that the >>> host is not capable of cutting the power to card? >> >> Yes, the regulator powering the card is always on in this case which means >> SDIO VDD is not powered off during the suspend. >> > So in principle the host should be able to notify the protocol layer > about that mmc_card_keep_power will always be set somehow. > That could be a more proper way of solving this maybe? What do you think? Ok. Basically this would be the suspend/resume sequence with Keep Power flag cleared. Suspend: 1. Execute Function driver suspend 2. mmc_power_off() 2.1 As this is regulator powering card SDIO VDD line is always on regulator, it will stay on. Resume: 1. mmc_power_up() 1.1 Regulator was anyway on so nothing change here. 2. mmc_sdio_init_card() - reinitialize the card 2.1 CMD5 gets the commands response timeout which means card doesn't respond back. My understanding is that behaviour seen in 2.1 (resume) is because during suspend, function driver suspend might have done something with the SDIO card (as keep power flag was disabled) and that's the reason during resume, card fails to respond back to CMD5 (unless we send the CMD52 to reset the card). It's an athros SDIO wifi card here. >> So in principle the host should be able to notify the protocol layer about that mmc_card_keep_power will always be set somehow. Given the above issue, i am not sure how is this suggestion going to help in this case? Regards, Subhash > >>> It might be more safe to do this but I would like to understand more, >>> before you get my ack on this patch. >>> >>>> err = mmc_sdio_init_card(host, host->ocr, host->card, >>>> mmc_card_keep_power(host)); >>>> - else if (mmc_card_keep_power(host) && >>>> mmc_card_wake_sdio_irq(host)) { >>>> + } else if (mmc_card_keep_power(host) && >>>> mmc_card_wake_sdio_irq(host)) { >>>> /* We may have switched to 1-bit mode during suspend */ >>>> err = sdio_enable_4bit_bus(host->card); >>>> if (err > 0) { >>>> -- >>>> -- >>>> QUALCOMM INDIA, 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 >>> 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
On 7 December 2012 13:15, Subhash Jadavani <subhashj@codeaurora.org> wrote: > On 12/7/2012 3:40 AM, Ulf Hansson wrote: >> >> On 6 December 2012 16:25, Subhash Jadavani <subhashj@codeaurora.org> >> wrote: >>> >>> On 12/6/2012 4:03 PM, Ulf Hansson wrote: >>>> >>>> On 4 December 2012 12:36, Subhash Jadavani <subhashj@codeaurora.org> >>>> wrote: >>>>> >>>>> If SDIO keep power flag (MMC_PM_KEEP_POWER) is not set, card would >>>>> be reinitialized during resume but as we are not resetting >>>>> (CMD52 reset) the SDIO card during this reinitialization, card may >>>>> fail to respond back to subsequent commands (CMD5 etc...). >>>>> >>>>> This change resets the card before the reinitialization of card >>>>> during resume. >>>>> >>>>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> >>>>> --- >>>>> drivers/mmc/core/sdio.c | 6 ++++-- >>>>> 1 files changed, 4 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >>>>> index 2273ce6..34ad4c8 100644 >>>>> --- a/drivers/mmc/core/sdio.c >>>>> +++ b/drivers/mmc/core/sdio.c >>>>> @@ -937,10 +937,12 @@ static int mmc_sdio_resume(struct mmc_host *host) >>>>> mmc_claim_host(host); >>>>> >>>>> /* No need to reinitialize powered-resumed nonremovable cards >>>>> */ >>>>> - if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) >>>>> + if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) >>>>> { >>>>> + sdio_reset(host); >>>>> + mmc_go_idle(host); >>>> >>>> If the card has lost power, why is this needed? Could it be that the >>>> host is not capable of cutting the power to card? >>> >>> >>> Yes, the regulator powering the card is always on in this case which >>> means >>> SDIO VDD is not powered off during the suspend. >>> >> So in principle the host should be able to notify the protocol layer >> about that mmc_card_keep_power will always be set somehow. >> That could be a more proper way of solving this maybe? What do you think? > > Ok. > Basically this would be the suspend/resume sequence with Keep Power flag > cleared. I mean the exact opposite. :-) mmc_keep_power should be _set_ during suspend/resume sequence. That will mean at suspend; sdio_disable_4bit_bus will be called and mmc_power_off will not. At resume, sdio_enable_4bit_bus will be called, without doing mmc_power_up and mmc_sdio_init_card. Do note that there is no API for the setting mmc_keep_power from a host. It exist only in the SDIO API. (sdio_set_host_pm_flags). So I guess we should invent this API for the host to use. > > Suspend: > 1. Execute Function driver suspend > 2. mmc_power_off() > 2.1 As this is regulator powering card SDIO VDD line is always on > regulator, it will stay on. > > Resume: > 1. mmc_power_up() > 1.1 Regulator was anyway on so nothing change here. > 2. mmc_sdio_init_card() - reinitialize the card > 2.1 CMD5 gets the commands response timeout which means card doesn't > respond back. > > My understanding is that behaviour seen in 2.1 (resume) is because during > suspend, function driver suspend might have done something with the SDIO > card (as keep power flag was disabled) and that's the reason during resume, > card fails to respond back to CMD5 (unless we send the CMD52 to reset the > card). It's an athros SDIO wifi card here. > > >>> So in principle the host should be able to notify the protocol layer >>> about that mmc_card_keep_power will always be set somehow. > Given the above issue, i am not sure how is this suggestion going to help in > this case? > > Regards, > Subhash > > >> >>>> It might be more safe to do this but I would like to understand more, >>>> before you get my ack on this patch. >>>> >>>>> err = mmc_sdio_init_card(host, host->ocr, host->card, >>>>> mmc_card_keep_power(host)); >>>>> - else if (mmc_card_keep_power(host) && >>>>> mmc_card_wake_sdio_irq(host)) { >>>>> + } else if (mmc_card_keep_power(host) && >>>>> mmc_card_wake_sdio_irq(host)) { >>>>> /* We may have switched to 1-bit mode during suspend >>>>> */ >>>>> err = sdio_enable_4bit_bus(host->card); >>>>> if (err > 0) { >>>>> -- >>>>> -- >>>>> QUALCOMM INDIA, 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 >>>> >>>> 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
On 12/7/2012 8:21 PM, Ulf Hansson wrote: > On 7 December 2012 13:15, Subhash Jadavani <subhashj@codeaurora.org> wrote: >> On 12/7/2012 3:40 AM, Ulf Hansson wrote: >>> On 6 December 2012 16:25, Subhash Jadavani <subhashj@codeaurora.org> >>> wrote: >>>> On 12/6/2012 4:03 PM, Ulf Hansson wrote: >>>>> On 4 December 2012 12:36, Subhash Jadavani <subhashj@codeaurora.org> >>>>> wrote: >>>>>> If SDIO keep power flag (MMC_PM_KEEP_POWER) is not set, card would >>>>>> be reinitialized during resume but as we are not resetting >>>>>> (CMD52 reset) the SDIO card during this reinitialization, card may >>>>>> fail to respond back to subsequent commands (CMD5 etc...). >>>>>> >>>>>> This change resets the card before the reinitialization of card >>>>>> during resume. >>>>>> >>>>>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> >>>>>> --- >>>>>> drivers/mmc/core/sdio.c | 6 ++++-- >>>>>> 1 files changed, 4 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >>>>>> index 2273ce6..34ad4c8 100644 >>>>>> --- a/drivers/mmc/core/sdio.c >>>>>> +++ b/drivers/mmc/core/sdio.c >>>>>> @@ -937,10 +937,12 @@ static int mmc_sdio_resume(struct mmc_host *host) >>>>>> mmc_claim_host(host); >>>>>> >>>>>> /* No need to reinitialize powered-resumed nonremovable cards >>>>>> */ >>>>>> - if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) >>>>>> + if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) >>>>>> { >>>>>> + sdio_reset(host); >>>>>> + mmc_go_idle(host); >>>>> If the card has lost power, why is this needed? Could it be that the >>>>> host is not capable of cutting the power to card? >>>> >>>> Yes, the regulator powering the card is always on in this case which >>>> means >>>> SDIO VDD is not powered off during the suspend. >>>> >>> So in principle the host should be able to notify the protocol layer >>> about that mmc_card_keep_power will always be set somehow. >>> That could be a more proper way of solving this maybe? What do you think? >> Ok. >> Basically this would be the suspend/resume sequence with Keep Power flag >> cleared. > I mean the exact opposite. :-) > > mmc_keep_power should be _set_ during suspend/resume sequence. > > That will mean at suspend; sdio_disable_4bit_bus will be called and > mmc_power_off will not. > At resume, sdio_enable_4bit_bus will be called, without doing > mmc_power_up and mmc_sdio_init_card. > > Do note that there is no API for the setting mmc_keep_power from a > host. It exist only in the SDIO API. (sdio_set_host_pm_flags). > So I guess we should invent this API for the host to use. Thanks Ulf. I got your point. Will try out your suggestion and will update (with patch & result) soon. On the other hand, Do you see it's worth to have this current patch as well? I guess it's safe to do sdio_reset() before SDIO card init and we are already playing safe even in mmc_rescan*() by calling sdio_reset() before card detection. Regards, Subhash > >> Suspend: >> 1. Execute Function driver suspend >> 2. mmc_power_off() >> 2.1 As this is regulator powering card SDIO VDD line is always on >> regulator, it will stay on. >> >> Resume: >> 1. mmc_power_up() >> 1.1 Regulator was anyway on so nothing change here. >> 2. mmc_sdio_init_card() - reinitialize the card >> 2.1 CMD5 gets the commands response timeout which means card doesn't >> respond back. >> >> My understanding is that behaviour seen in 2.1 (resume) is because during >> suspend, function driver suspend might have done something with the SDIO >> card (as keep power flag was disabled) and that's the reason during resume, >> card fails to respond back to CMD5 (unless we send the CMD52 to reset the >> card). It's an athros SDIO wifi card here. >> >> >>>> So in principle the host should be able to notify the protocol layer >>>> about that mmc_card_keep_power will always be set somehow. >> Given the above issue, i am not sure how is this suggestion going to help in >> this case? >> >> Regards, >> Subhash >> >> >>>>> It might be more safe to do this but I would like to understand more, >>>>> before you get my ack on this patch. >>>>> >>>>>> err = mmc_sdio_init_card(host, host->ocr, host->card, >>>>>> mmc_card_keep_power(host)); >>>>>> - else if (mmc_card_keep_power(host) && >>>>>> mmc_card_wake_sdio_irq(host)) { >>>>>> + } else if (mmc_card_keep_power(host) && >>>>>> mmc_card_wake_sdio_irq(host)) { >>>>>> /* We may have switched to 1-bit mode during suspend >>>>>> */ >>>>>> err = sdio_enable_4bit_bus(host->card); >>>>>> if (err > 0) { >>>>>> -- >>>>>> -- >>>>>> QUALCOMM INDIA, 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 >>>>> 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
On 8 December 2012 06:55, Subhash Jadavani <subhashj@codeaurora.org> wrote: > On 12/7/2012 8:21 PM, Ulf Hansson wrote: >> >> On 7 December 2012 13:15, Subhash Jadavani <subhashj@codeaurora.org> >> wrote: >>> >>> On 12/7/2012 3:40 AM, Ulf Hansson wrote: >>>> >>>> On 6 December 2012 16:25, Subhash Jadavani <subhashj@codeaurora.org> >>>> wrote: >>>>> >>>>> On 12/6/2012 4:03 PM, Ulf Hansson wrote: >>>>>> >>>>>> On 4 December 2012 12:36, Subhash Jadavani <subhashj@codeaurora.org> >>>>>> wrote: >>>>>>> >>>>>>> If SDIO keep power flag (MMC_PM_KEEP_POWER) is not set, card would >>>>>>> be reinitialized during resume but as we are not resetting >>>>>>> (CMD52 reset) the SDIO card during this reinitialization, card may >>>>>>> fail to respond back to subsequent commands (CMD5 etc...). >>>>>>> >>>>>>> This change resets the card before the reinitialization of card >>>>>>> during resume. >>>>>>> >>>>>>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> >>>>>>> --- >>>>>>> drivers/mmc/core/sdio.c | 6 ++++-- >>>>>>> 1 files changed, 4 insertions(+), 2 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c >>>>>>> index 2273ce6..34ad4c8 100644 >>>>>>> --- a/drivers/mmc/core/sdio.c >>>>>>> +++ b/drivers/mmc/core/sdio.c >>>>>>> @@ -937,10 +937,12 @@ static int mmc_sdio_resume(struct mmc_host >>>>>>> *host) >>>>>>> mmc_claim_host(host); >>>>>>> >>>>>>> /* No need to reinitialize powered-resumed nonremovable >>>>>>> cards >>>>>>> */ >>>>>>> - if (mmc_card_is_removable(host) || >>>>>>> !mmc_card_keep_power(host)) >>>>>>> + if (mmc_card_is_removable(host) || >>>>>>> !mmc_card_keep_power(host)) >>>>>>> { >>>>>>> + sdio_reset(host); >>>>>>> + mmc_go_idle(host); >>>>>> >>>>>> If the card has lost power, why is this needed? Could it be that the >>>>>> host is not capable of cutting the power to card? >>>>> >>>>> >>>>> Yes, the regulator powering the card is always on in this case which >>>>> means >>>>> SDIO VDD is not powered off during the suspend. >>>>> >>>> So in principle the host should be able to notify the protocol layer >>>> about that mmc_card_keep_power will always be set somehow. >>>> That could be a more proper way of solving this maybe? What do you >>>> think? >>> >>> Ok. >>> Basically this would be the suspend/resume sequence with Keep Power flag >>> cleared. >> >> I mean the exact opposite. :-) >> >> mmc_keep_power should be _set_ during suspend/resume sequence. >> >> That will mean at suspend; sdio_disable_4bit_bus will be called and >> mmc_power_off will not. >> At resume, sdio_enable_4bit_bus will be called, without doing >> mmc_power_up and mmc_sdio_init_card. >> >> Do note that there is no API for the setting mmc_keep_power from a >> host. It exist only in the SDIO API. (sdio_set_host_pm_flags). >> So I guess we should invent this API for the host to use. > > > Thanks Ulf. I got your point. Will try out your suggestion and will update > (with patch & result) soon. OK, great. You may keep me on cc. > On the other hand, > Do you see it's worth to have this current patch as well? I guess it's safe > to do sdio_reset() before SDIO card init and we are already playing safe > even in mmc_rescan*() by calling sdio_reset() before card detection. > That is a valid point. So you have my Ack for this patch! > Regards, > Subhash > > >> >>> Suspend: >>> 1. Execute Function driver suspend >>> 2. mmc_power_off() >>> 2.1 As this is regulator powering card SDIO VDD line is always on >>> regulator, it will stay on. >>> >>> Resume: >>> 1. mmc_power_up() >>> 1.1 Regulator was anyway on so nothing change here. >>> 2. mmc_sdio_init_card() - reinitialize the card >>> 2.1 CMD5 gets the commands response timeout which means card >>> doesn't >>> respond back. >>> >>> My understanding is that behaviour seen in 2.1 (resume) is because during >>> suspend, function driver suspend might have done something with the SDIO >>> card (as keep power flag was disabled) and that's the reason during >>> resume, >>> card fails to respond back to CMD5 (unless we send the CMD52 to reset the >>> card). It's an athros SDIO wifi card here. >>> >>> >>>>> So in principle the host should be able to notify the protocol layer >>>>> about that mmc_card_keep_power will always be set somehow. >>> >>> Given the above issue, i am not sure how is this suggestion going to help >>> in >>> this case? >>> >>> Regards, >>> Subhash >>> >>> >>>>>> It might be more safe to do this but I would like to understand more, >>>>>> before you get my ack on this patch. >>>>>> >>>>>>> err = mmc_sdio_init_card(host, host->ocr, >>>>>>> host->card, >>>>>>> >>>>>>> mmc_card_keep_power(host)); >>>>>>> - else if (mmc_card_keep_power(host) && >>>>>>> mmc_card_wake_sdio_irq(host)) { >>>>>>> + } else if (mmc_card_keep_power(host) && >>>>>>> mmc_card_wake_sdio_irq(host)) { >>>>>>> /* We may have switched to 1-bit mode during >>>>>>> suspend >>>>>>> */ >>>>>>> err = sdio_enable_4bit_bus(host->card); >>>>>>> if (err > 0) { >>>>>>> -- >>>>>>> -- >>>>>>> QUALCOMM INDIA, 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 >>>>>> >>>>>> Kind regards >>>>>> Ulf Hansson >>>>> >>>>> > Acked-by: Ulf Hansson <ulf.hansson@linaro.org> 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
diff --git a/drivers/mmc/core/sdio.c b/drivers/mmc/core/sdio.c index 2273ce6..34ad4c8 100644 --- a/drivers/mmc/core/sdio.c +++ b/drivers/mmc/core/sdio.c @@ -937,10 +937,12 @@ static int mmc_sdio_resume(struct mmc_host *host) mmc_claim_host(host); /* No need to reinitialize powered-resumed nonremovable cards */ - if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) + if (mmc_card_is_removable(host) || !mmc_card_keep_power(host)) { + sdio_reset(host); + mmc_go_idle(host); err = mmc_sdio_init_card(host, host->ocr, host->card, mmc_card_keep_power(host)); - else if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) { + } else if (mmc_card_keep_power(host) && mmc_card_wake_sdio_irq(host)) { /* We may have switched to 1-bit mode during suspend */ err = sdio_enable_4bit_bus(host->card); if (err > 0) {
If SDIO keep power flag (MMC_PM_KEEP_POWER) is not set, card would be reinitialized during resume but as we are not resetting (CMD52 reset) the SDIO card during this reinitialization, card may fail to respond back to subsequent commands (CMD5 etc...). This change resets the card before the reinitialization of card during resume. Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org> --- drivers/mmc/core/sdio.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-)