Re: [RFC] MMC-4.5 Power OFF Notify rework

[Saugata Das <saugata.das@xxxxxxxxxx>]

On 2 April 2012 21:26, Linus Walleij <linus.walleij@xxxxxxxxxx> wrote:
> On Mon, Apr 2, 2012 at 10:47 AM, Saugata Das <saugata.das@xxxxxxxxxx> wrote:
>
>>>> +       host->power_notify_type = MMC_HOST_PW_NOTIFY_LONG;
>>>
>>> This looks new, can you explain in the code as comments
>>> or in the commit message when SHORT and LONG notifications are
>>> used and why?
>>>
>>
>> The mmc_poweroff_notify with MMC_HOST_PW_NOTIFY_LONG will take longer
>> time  to complete than MMC_HOST_PW_NOTIFY_SHORT. So, the idea is that
>> if we use mmc_poweroff_notify on suspend path, then we should use
>> MMC_HOST_PW_NOTIFY_SHORT and if we use mmc_poweroff_notify from the
>> shutdown path, then we should use MMC_HOST_PW_NOTIFY_LONG.
>>
>> We will add this in the description.
>
> OK sounds good.
>
>>>> @@ -1338,12 +1338,18 @@ static int mmc_suspend(struct mmc_host *host)
>>>>        BUG_ON(!host->card);
>>>>
>>>>        mmc_claim_host(host);
>>>> -       if (mmc_card_can_sleep(host)) {
>>>> -               err = mmc_card_sleep(host);
>>>> +       if (host->caps2 & MMC_CAP2_POWER_OFF_VCCQ_DURING_SUSPEND) {
>>>> +               err = mmc_poweroff_notify(host);
>>>>                if (!err)
>>>> -                       mmc_card_set_sleep(host->card);
>>>> -       } else if (!mmc_host_is_spi(host))
>>>> +                       goto out;
>>>> +       }
>>>> +
>>>> +       if (mmc_card_can_sleep(host))
>>>> +               err = mmc_card_sleep(host);
>>>> +       else if (!mmc_host_is_spi(host))
>>>>                mmc_deselect_cards(host);
>>>
>>> Are you sure you should not deselect the card on an SPI host also if
>>> you power off? (I'm just confused, better to ask...)
>>>
>>
>> eMMC does not support SPI mode. So, the POWER OFF NOTIFY, which is an
>> eMMC feature, can not be used on SPI mode. The above code (which you
>> are referring) puts the eMMC to low power "standby" state with
>> mmc_deselect_cards if sleep is not allowed. This logic has not been
>> modified by the POWER OFF NOTIFY patch.
>
> OK I would add a small comment above the else if (!mmc_host_is_spi(host))
> such as /* SPI mode is only used external cards */ or so, it helsps when
> reading the code.
>

I understand but power off notify patch did not introduce this. May
be, we can add this comment in a different patch.

If no other comments, then we will resubmit this patch with
modification of the description.


> Yours,
> Linus Walleij
--
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