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 :-)
> 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.
>
> 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.
>
> 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
