Hi Saugata,
I can have a go and test it. But first I would like to bring up 3
concerns that I have with this patch.
1. This patch should be sent to cc-stable in order to repair the bug
introduced in 3.4
2. Is the bus_ops for poweroff_notify really necessary since only mmc
use it? There are already bus_ops for suspend/resume,
power_save/power_restore and remove. It feels like it would be
possible to address poweroff_notify internally from mmc.c from theses
bus_ops. I would be nice to add this feature without having to touch
core.c.
For instance. Call mmc_suspend() from mmc_remove()
+++ b/drivers/mmc/core/mmc.c
@@ -1302,7 +1302,7 @@ static void mmc_remove(struct mmc_host *host)
+ __mmc_suspend(host, true);
mmc_remove_card(host->card);
@@ -1347,7 +1347,7 @@ static void mmc_detect(struct mmc_host *host)
-static int mmc_suspend(struct mmc_host *host)
+static int __mmc_suspend(struct mmc_host *host, bool remove)
@@ -1356,7 +1356,8 @@ static int mmc_suspend(struct mmc_host *host)
mmc_claim_host(host);
if (mmc_can_poweroff_notify(host->card) &&
- (host->caps2 & MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND)) {
+ (host->caps2 & MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND ||
+ remove)) {
err = mmc_poweroff_notify(host, MMC_PW_OFF_NOTIFY_SHORT);
} else {
if (mmc_card_can_sleep(host))
@@ -1372,6 +1373,11 @@ static int mmc_suspend(struct mmc_host *host)
+static int mmc_suspend(struct mmc_host *host)
+{
+ return __mmc_suspend(host, false);
+}
+
Calling mmc_suspend from mmc_remove() has another advantage since it
may issue SLEEP (CMD5) before the card is removed and power off. This
is recommended by eMMC Vendors in order to shutdown the eMMC safely to
prevent data corruption. When the platform shuts down the power to the
eMMC will be turned off no matter what.
3. About the sleep and awake issue. This is not really related to
poweroff_notify() as I see it. I would recommend to use CMD 0 to
re-init the card safely after sleep in this patch. Then you could send
out a sleep/awake patch that address this separately. This would also
make #1 easier, send patch to cc-stable. Adding save/restore IOS is a
new feature and should not be sent to the cc-stable list. What do you
think?
BR
/Per
On Thu, Jun 14, 2012 at 5:15 PM, Saugata Das <saugata.das@xxxxxxxxxx> wrote:
> On 14 June 2012 20:20, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>> Hi Girish,
>>
>> On 14 June 2012 15:21, Girish K S <girish.shivananjappa@xxxxxxxxxx> wrote:
>>> On 14 June 2012 18:43, Per Forlin <per.lkml@xxxxxxxxx> wrote:
>>>> Hi Girish and Suagata,
>>>>
>>>> I have run some regression tests with this patch on our board (ux500
>>>> family) running suspend and resume of the eMMC 4.41 device.
>>>>
>>>> The two patches I have looked at are:
>>>> 1. "mmc: core: Fix PowerOff Notify suspend/resume" (merged)
>>>> 2. " MMC-4.5 Power OFF Notify Rework"
>>>>
>>>> With only patch #1 the eMMC doesn't power up after in resume() after
>>>> being suspended. The eMMC doesn't respond at all after suspend. It's
>>>> not powered up.
>>>> Running tests with #1 and #2, the card is powered up but it doesn't
>>>> wake up after CMD5. Commands that arrive are after resume/CMD5
>>>> timeouts. I even tried by increasing the awake timeout to 5 seconds
>>>> but i didn't help.
>>>>
>>>> The eMMC on my board successfully suspends and resumes with patch #1
>>>> and #2 if waking up the card using CMD0 (mmc_card_init()) instead of
>>>> CMD5.
>>>>
>>>> Have anyone else seen the same issue?
>>>> Have this patch been verified on a board together with eMMC 4.41 that
>>>> supports card power off.
>>> This rework patch is still under progress. we are modifying it. In our
>>> earlier discussions subhash has posted the
>>> same issue and a solution for this. we should save ios context before
>>> sleep and restore ios before awake. soon rework patch will be
>>> posted with the above recomenedded solution.
>>>
>>
>> I think the best solution is to always do mmc_card_init when doing
>> resume, it will be nice a simple.
>
> Note that, with power OFF notify (MMC-4.5), there will be some pending
> operation with the MMC controller. If we do mmc_card_init after
> suspend, then there could be some data loss.
>
> I have passed to Per the latest patch (Subhash reported that it is
> working). I shall forward to you as well. Lets solve the issue. If you
> can work around, without mmc_card_init after suspend, then you are
> most welcome to update the patch :-)
>
>
>> Otherwise it will be somewhat tricky to keep track of what state we
>> are in, and if the ios should be restored or not.
>>
>> Finally, I would be glad to help out in posting an updated version of
>> this patch, if that OK with you?
>>
>> Kind regards
>> Ulf Hansson
--
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
