On 8 December 2012 06:55, Subhash Jadavani <subhashj@xxxxxxxxxxxxxx> wrote: > On 12/7/2012 8:21 PM, Ulf Hansson wrote: >> >> On 7 December 2012 13:15, Subhash Jadavani <subhashj@xxxxxxxxxxxxxx> >> wrote: >>> >>> On 12/7/2012 3:40 AM, Ulf Hansson wrote: >>>> >>>> On 6 December 2012 16:25, Subhash Jadavani <subhashj@xxxxxxxxxxxxxx> >>>> wrote: >>>>> >>>>> On 12/6/2012 4:03 PM, Ulf Hansson wrote: >>>>>> >>>>>> On 4 December 2012 12:36, Subhash Jadavani <subhashj@xxxxxxxxxxxxxx> >>>>>> 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@xxxxxxxxxxxxxx> >>>>>>> --- >>>>>>> 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@xxxxxxxxxxxxxxx >>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>>> >>>>>> Kind regards >>>>>> Ulf Hansson >>>>> >>>>> > Acked-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx> Kind regards Ulf Hansson -- To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html