Ufe,
I have been side-tracked but will get back on it. I will be the worst
case scenario (lot of random write before suspend for different eMMC).
I also need to rework the wait for DAT0 to deassert instead of the
busy line as usual.
Gwendal.
On Fri, Dec 15, 2017 at 1:24 AM, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
> On 29 November 2017 at 15:14, Ulf Hansson <ulf.hansson@xxxxxxxxxx> wrote:
>> [...]
>>
>>>> static int mmc_poweroff_notify(struct mmc_card *card, unsigned int notify_type)
>>>> {
>>>> - unsigned int timeout = card->ext_csd.generic_cmd6_time;
>>>> + unsigned int timeout;
>>>> + bool use_busy_signal = true;
>>>> int err;
>>>>
>>>> - /* Use EXT_CSD_POWER_OFF_SHORT as default notification type. */
>>>> - if (notify_type == EXT_CSD_POWER_OFF_LONG)
>>>> + switch (notify_type) {
>>>> + case EXT_CSD_POWER_OFF_LONG:
>>>> timeout = card->ext_csd.power_off_longtime;
>>>> + break;
>>>> + case EXT_CSD_SLEEP_NOTIFICATION:
>>>> + timeout = card->ext_csd.sleep_notification_time;
>>>> + use_busy_signal = false;
>>>
>>> This is wrong.
>>>
>>> If you set use_busy_signal to false, it means that we don't care about
>>> waiting for the card to stop signaling busy on DAT0 after setting
>>> EXT_CSD_SLEEP_NOTIFICATION. This then becomes a violation of the eMMC
>>> spec, because we must not issue a CMD5 before the card have stopped
>>> signaling busy.
>>>
>>> I realize that for those devices that don't support
>>> MMC_CAP_WAIT_WHILE_BUSY or have the ->card_busy() callback
>>> implemented, the consequence may be that mmc_poll_for_busy() may call
>>> mmc_delay() with a very big timeout. This could be a big problem, I
>>> guess.
>>
>> Well, it may also be a big problem even if the host supports
>> ->card_busy() and/or MMC_CAP_WAIT_WHILE_BUSY (but maybe not as big),
>> simply because we can't be waiting here for several seconds to allow
>> the card to deal with background operations.
>>
>> I guess we need to try and see what happens. Perhaps you can share
>> some data about the timeouts you get?
>>
>>>
>>>> + break;
>>>> + default:
>>>> + /* Use EXT_CSD_POWER_OFF_SHORT as default notification type. */
>>>> + timeout = card->ext_csd.generic_cmd6_time;
>>>> + }
>>>>
>>>> err = __mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>>> EXT_CSD_POWER_OFF_NOTIFICATION,
>>>> - notify_type, timeout, 0, true, false, false);
>>>> + notify_type, timeout, 0, use_busy_signal, false, false);
>>>> if (err)
>>>> pr_err("%s: Power Off Notification timed out, %u\n",
>>>> mmc_hostname(card->host), timeout);
>>
>> [...]
>>
>> Kind regards
>> Uffe
>
> Gwendal, any news on this?
>
> As changes for the SLEEP_NOTIFICATION keeps being posted, clearly
> there is a need for us to deal with it in some way. I have been
> thinking of a fall-back solution, if we can't get it it work, but
> anyway I am eager to find a solution so we finally can sort this out.
>
> Kind regards
> Uffe
--
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
