On 06/15/2012 05:49 AM, Saugata Das wrote:
On 15 June 2012 00:36, Per Forlin<per.lkml@xxxxxxxxx> wrote:
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
I shall sent it out today.
2. Is the bus_ops for poweroff_notify really necessary since only mmc
use it?
This was recommended in the review from Ulf. If it is not adding to a
bug, I propose to keep it this way. Otherwise, we will be going in
circles :-)
Moreover it seems close related to sleep, which is implemented with
bus_ops. So I would say, keep as is. We might change later, then both
sleep and poweroff_notify together.
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.
May be for MMC-4.41 cards this approach can be taken. For MMC-4.5, it
has to be power OFF notify when the power is removed. Lets do it for
another patch, since the intention of this patch is to fix the issues
around power OFF notify.
I agree with you Saugata, the exact same sequence as in suspend can not
be used. The reason is simply that we do not want to consider
MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND for doing poweroff_notify here,
as we want in suspend.
Leave this to be fixed in a separate patch instead.
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?
The problem in sending CMD0 without power OFF notify is possibility of
some data loss in MMC-4.5 devices.
I don't see the problem here. You will be sending power OFF notify when
you can. The only difference is when you "wake" the device from sleep
mode. Instead of using CMD5, which may work is some cases and in some
cases not (without restoring ios). So using CMD0 as common way of waking
up from suspend should be fine. Unless I missed something of course. :-)
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
