[PATCH] mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP

[PATCH] mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP

[Thierry Escande]

From: zhaojohn <john.zhao@intel.com>

Hynix eMMC devices sometimes take 50% longer to resume from sleep.
Based on a recommendation from Hynix, send a Power-Off Notification
before going to S3 to restore a resume time consistently within spec.

Signed-off-by: zhaojohn <john.zhao@intel.com>
Signed-off-by: Arindam Roy <arindam.roy@intel.com>
Tested-by: Freddy Paul <freddy.paul@intel.com>
Reviewed-by: Icarus W Sparry <icarus.w.sparry@intel.com>
Reviewed-by: Marc Herbert <marc.herbert@intel.com>
Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
Signed-off-by: Brian Norris <briannorris@chromium.org>
Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
---
 drivers/mmc/card/block.c | 8 ++++++++
 drivers/mmc/core/mmc.c   | 8 +++++++-
 include/linux/mmc/card.h | 2 ++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 2206d44..cbc2d97 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -2572,6 +2572,14 @@ static const struct mmc_fixup blk_fixups[] =
 	MMC_FIXUP("V10016", CID_MANFID_KINGSTON, CID_OEMID_ANY, add_quirk_mmc,
 		  MMC_QUIRK_TRIM_BROKEN),
 
+	/*
+	 * Hynix eMMC devices sometimes take 50% longer to resume from sleep.
+	 * Based on a recommendation from Hynix, send a Power-Off Notification
+	 * before going to S3 to restore a resume time consistently within spec.
+	 */
+	MMC_FIXUP(CID_NAME_ANY, CID_MANFID_HYNIX, CID_OEMID_ANY, add_quirk_mmc,
+		  MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP),
+
 	END_FIXUP
 };
 
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index f2d185c..46a4562 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1925,8 +1925,14 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
 	if (mmc_can_poweroff_notify(host->card) &&
 		((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
 		err = mmc_poweroff_notify(host->card, notify_type);
-	else if (mmc_can_sleep(host->card))
+	else if (mmc_can_sleep(host->card)) {
+		if (host->card->quirks & MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP) {
+			err = mmc_poweroff_notify(host->card, notify_type);
+			if (err)
+				goto out;
+		}
 		err = mmc_sleep(host);
+	}
 	else if (!mmc_host_is_spi(host))
 		err = mmc_deselect_cards(host);
 
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index d8673ca..30e34b0 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -281,6 +281,8 @@ struct mmc_card {
 #define MMC_QUIRK_BROKEN_IRQ_POLLING	(1<<11)	/* Polling SDIO_CCCR_INTx could create a fake interrupt */
 #define MMC_QUIRK_TRIM_BROKEN	(1<<12)		/* Skip trim */
 #define MMC_QUIRK_BROKEN_HPI	(1<<13)		/* Disable broken HPI support */
+#define MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP \
+				(1<<14)		/* Poweroff notification*/
 
 
 	unsigned int		erase_size;	/* erase size in sectors */
-- 
2.7.4

Re: [PATCH] mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP

[Ulf Hansson]

On 3 October 2016 at 16:19, Thierry Escande
<thierry.escande@collabora.com> wrote:
> From: zhaojohn <john.zhao@intel.com>
>
> Hynix eMMC devices sometimes take 50% longer to resume from sleep.
> Based on a recommendation from Hynix, send a Power-Off Notification
> before going to S3 to restore a resume time consistently within spec.

Could you also share what mmc controller and SoC you get this results from?

More precisely, are you using MMC_CAP_WAIT_WHILE_BUSY?

>
> Signed-off-by: zhaojohn <john.zhao@intel.com>
> Signed-off-by: Arindam Roy <arindam.roy@intel.com>
> Tested-by: Freddy Paul <freddy.paul@intel.com>
> Reviewed-by: Icarus W Sparry <icarus.w.sparry@intel.com>
> Reviewed-by: Marc Herbert <marc.herbert@intel.com>
> Reviewed-by: Eric Caruso <ejcaruso@chromium.org>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Signed-off-by: Thierry Escande <thierry.escande@collabora.com>
> ---
>  drivers/mmc/card/block.c | 8 ++++++++
>  drivers/mmc/core/mmc.c   | 8 +++++++-
>  include/linux/mmc/card.h | 2 ++
>  3 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 2206d44..cbc2d97 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -2572,6 +2572,14 @@ static const struct mmc_fixup blk_fixups[] =
>         MMC_FIXUP("V10016", CID_MANFID_KINGSTON, CID_OEMID_ANY, add_quirk_mmc,
>                   MMC_QUIRK_TRIM_BROKEN),
>
> +       /*
> +        * Hynix eMMC devices sometimes take 50% longer to resume from sleep.
> +        * Based on a recommendation from Hynix, send a Power-Off Notification
> +        * before going to S3 to restore a resume time consistently within spec.
> +        */
> +       MMC_FIXUP(CID_NAME_ANY, CID_MANFID_HYNIX, CID_OEMID_ANY, add_quirk_mmc,
> +                 MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP),
> +
>         END_FIXUP
>  };
>
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index f2d185c..46a4562 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1925,8 +1925,14 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
>         if (mmc_can_poweroff_notify(host->card) &&
>                 ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
>                 err = mmc_poweroff_notify(host->card, notify_type);
> -       else if (mmc_can_sleep(host->card))
> +       else if (mmc_can_sleep(host->card)) {
> +               if (host->card->quirks & MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP) {
> +                       err = mmc_poweroff_notify(host->card, notify_type);
> +                       if (err)
> +                               goto out;
> +               }

So, I am curious to know from a power management point of view; how
does the card behave comparing the sleep and power off notification
command?

Is the card in a low power state after the power off notification has
been received? If so, did you manage to do some measurement for that
or perhaps the data-sheet tells about this? It would be interesting to
know if there were any differences between sleep and power off
notification in this regards.

[...]

Kind regards
Uffe

Re: [PATCH] mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP

[Thierry Escande]

Hi Ulf,

On 25/10/2016 12:03, Ulf Hansson wrote:
> On 3 October 2016 at 16:19, Thierry Escande
> <thierry.escande@collabora.com> wrote:
>> From: zhaojohn <john.zhao@intel.com>
>>
>> Hynix eMMC devices sometimes take 50% longer to resume from sleep.
>> Based on a recommendation from Hynix, send a Power-Off Notification
>> before going to S3 to restore a resume time consistently within spec.
>
> Could you also share what mmc controller and SoC you get this results from?
>
> More precisely, are you using MMC_CAP_WAIT_WHILE_BUSY?
This occurs on a braswell based chromebook, using the acpi sdhci 
controller. So yes, using MMC_CAP_WAIT_WHILE_BUSY.

[...]
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index f2d185c..46a4562 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1925,8 +1925,14 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
>>         if (mmc_can_poweroff_notify(host->card) &&
>>                 ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
>>                 err = mmc_poweroff_notify(host->card, notify_type);
>> -       else if (mmc_can_sleep(host->card))
>> +       else if (mmc_can_sleep(host->card)) {
>> +               if (host->card->quirks & MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP) {
>> +                       err = mmc_poweroff_notify(host->card, notify_type);
>> +                       if (err)
>> +                               goto out;
>> +               }
>
> So, I am curious to know from a power management point of view; how
> does the card behave comparing the sleep and power off notification
> command?
>
> Is the card in a low power state after the power off notification has
> been received? If so, did you manage to do some measurement for that
> or perhaps the data-sheet tells about this? It would be interesting to
> know if there were any differences between sleep and power off
> notification in this regards.
I do not have any clue about that. It appears only with Hynix emmc and 
the fix has been approved by Hynix engineers... It seems that if not 
powered off, the firmware does some garbage collection when resuming and 
it takes more time...

Regards,
  Thierry

Re: [PATCH] mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP

[Ulf Hansson]

On 27 October 2016 at 17:06, Thierry Escande
<thierry.escande@collabora.com> wrote:
> Hi Ulf,
>
> On 25/10/2016 12:03, Ulf Hansson wrote:
>>
>> On 3 October 2016 at 16:19, Thierry Escande
>> <thierry.escande@collabora.com> wrote:
>>>
>>> From: zhaojohn <john.zhao@intel.com>
>>>
>>> Hynix eMMC devices sometimes take 50% longer to resume from sleep.
>>> Based on a recommendation from Hynix, send a Power-Off Notification
>>> before going to S3 to restore a resume time consistently within spec.
>>
>>
>> Could you also share what mmc controller and SoC you get this results
>> from?
>>
>> More precisely, are you using MMC_CAP_WAIT_WHILE_BUSY?
>
> This occurs on a braswell based chromebook, using the acpi sdhci controller.
> So yes, using MMC_CAP_WAIT_WHILE_BUSY.
>
> [...]
>>>
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index f2d185c..46a4562 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -1925,8 +1925,14 @@ static int _mmc_suspend(struct mmc_host *host,
>>> bool is_suspend)
>>>         if (mmc_can_poweroff_notify(host->card) &&
>>>                 ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
>>>                 err = mmc_poweroff_notify(host->card, notify_type);
>>> -       else if (mmc_can_sleep(host->card))
>>> +       else if (mmc_can_sleep(host->card)) {
>>> +               if (host->card->quirks &
>>> MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP) {
>>> +                       err = mmc_poweroff_notify(host->card,
>>> notify_type);
>>> +                       if (err)
>>> +                               goto out;
>>> +               }
>>
>>
>> So, I am curious to know from a power management point of view; how
>> does the card behave comparing the sleep and power off notification
>> command?
>>
>> Is the card in a low power state after the power off notification has
>> been received? If so, did you manage to do some measurement for that
>> or perhaps the data-sheet tells about this? It would be interesting to
>> know if there were any differences between sleep and power off
>> notification in this regards.
>
> I do not have any clue about that. It appears only with Hynix emmc and the
> fix has been approved by Hynix engineers... It seems that if not powered
> off, the firmware does some garbage collection when resuming and it takes
> more time...

Okay, I see. So before I continue reviewing, can you please also tell
what regulators to the card that is being cut while powering off in
this path.

VMMC, VQMMC?

Kind regards
Uffe

Re: [PATCH] mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP

[Alex Lemberg]

Hi,

By the eMMC5.0 spec, before sending Sleep command (CMD5), hosts may set the
POWER_OFF_NOTIFICATION byte to SLEEP_NOTIFICATION (0x04).
Isn’t this a case in this patch?
If yes, why not sending SLEEP_NOTIFICATION instead of POWER_OFF_SHORT?

In the past we had a discussion on this topic, but the solution became more complicated because 
of possibly long timeout (max 83 seconds) on SLEEP_NOTIFICATION.
https://marc.info/?t=143374696600002&r=1&w=1

But still, in case we want to support POWER_OFF_NOTIFICATION before Sleep command,
and in case we want to be eMMC spec aligned, I believe the right thing to do
is to support SLEEP_NOTIFICATION…

Thanks,
Alex

On 10/27/16, 10:49 PM, "linux-mmc-owner@vger.kernel.org on behalf of Ulf Hansson" <linux-mmc-owner@vger.kernel.org on behalf of ulf.hansson@linaro.org> wrote:

>On 27 October 2016 at 17:06, Thierry Escande
><thierry.escande@collabora.com> wrote:
>> Hi Ulf,
>>
>> On 25/10/2016 12:03, Ulf Hansson wrote:
>>>
>>> On 3 October 2016 at 16:19, Thierry Escande
>>> <thierry.escande@collabora.com> wrote:
>>>>
>>>> From: zhaojohn <john.zhao@intel.com>
>>>>
>>>> Hynix eMMC devices sometimes take 50% longer to resume from sleep.
>>>> Based on a recommendation from Hynix, send a Power-Off Notification
>>>> before going to S3 to restore a resume time consistently within spec.
>>>
>>>
>>> Could you also share what mmc controller and SoC you get this results
>>> from?
>>>
>>> More precisely, are you using MMC_CAP_WAIT_WHILE_BUSY?
>>
>> This occurs on a braswell based chromebook, using the acpi sdhci controller.
>> So yes, using MMC_CAP_WAIT_WHILE_BUSY.
>>
>> [...]
>>>>
>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>> index f2d185c..46a4562 100644
>>>> --- a/drivers/mmc/core/mmc.c
>>>> +++ b/drivers/mmc/core/mmc.c
>>>> @@ -1925,8 +1925,14 @@ static int _mmc_suspend(struct mmc_host *host,
>>>> bool is_suspend)
>>>>         if (mmc_can_poweroff_notify(host->card) &&
>>>>                 ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
>>>>                 err = mmc_poweroff_notify(host->card, notify_type);
>>>> -       else if (mmc_can_sleep(host->card))
>>>> +       else if (mmc_can_sleep(host->card)) {
>>>> +               if (host->card->quirks &
>>>> MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP) {
>>>> +                       err = mmc_poweroff_notify(host->card,
>>>> notify_type);
>>>> +                       if (err)
>>>> +                               goto out;
>>>> +               }
>>>
>>>
>>> So, I am curious to know from a power management point of view; how
>>> does the card behave comparing the sleep and power off notification
>>> command?
>>>
>>> Is the card in a low power state after the power off notification has
>>> been received? If so, did you manage to do some measurement for that
>>> or perhaps the data-sheet tells about this? It would be interesting to
>>> know if there were any differences between sleep and power off
>>> notification in this regards.
>>
>> I do not have any clue about that. It appears only with Hynix emmc and the
>> fix has been approved by Hynix engineers... It seems that if not powered
>> off, the firmware does some garbage collection when resuming and it takes
>> more time...
>
>Okay, I see. So before I continue reviewing, can you please also tell
>what regulators to the card that is being cut while powering off in
>this path.
>
>VMMC, VQMMC?
>
>Kind regards
>Uffe
>--
>To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP

[Thierry Escande]

Hi,

On 31/10/2016 11:14, Alex Lemberg wrote:
> Hi,
>
> By the eMMC5.0 spec, before sending Sleep command (CMD5), hosts may set the
> POWER_OFF_NOTIFICATION byte to SLEEP_NOTIFICATION (0x04).
> Isn’t this a case in this patch?
> If yes, why not sending SLEEP_NOTIFICATION instead of POWER_OFF_SHORT?
I cannot tell if this was really what the Hynix engineers meant here.

> In the past we had a discussion on this topic, but the solution became more complicated because
> of possibly long timeout (max 83 seconds) on SLEEP_NOTIFICATION.
> https://marc.info/?t=143374696600002&r=1&w=1
>
> But still, in case we want to support POWER_OFF_NOTIFICATION before Sleep command,
> and in case we want to be eMMC spec aligned, I believe the right thing to do
> is to support SLEEP_NOTIFICATION…
I can give a try to this patchset to see if it solves the issue and get 
back to you afterward.

Regards,
  Thierry

>
> Thanks,
> Alex
>
> On 10/27/16, 10:49 PM, "linux-mmc-owner@vger.kernel.org on behalf of Ulf Hansson" <linux-mmc-owner@vger.kernel.org on behalf of ulf.hansson@linaro.org> wrote:
>
>> On 27 October 2016 at 17:06, Thierry Escande
>> <thierry.escande@collabora.com> wrote:
>>> Hi Ulf,
>>>
>>> On 25/10/2016 12:03, Ulf Hansson wrote:
>>>>
>>>> On 3 October 2016 at 16:19, Thierry Escande
>>>> <thierry.escande@collabora.com> wrote:
>>>>>
>>>>> From: zhaojohn <john.zhao@intel.com>
>>>>>
>>>>> Hynix eMMC devices sometimes take 50% longer to resume from sleep.
>>>>> Based on a recommendation from Hynix, send a Power-Off Notification
>>>>> before going to S3 to restore a resume time consistently within spec.
>>>>
>>>>
>>>> Could you also share what mmc controller and SoC you get this results
>>>> from?
>>>>
>>>> More precisely, are you using MMC_CAP_WAIT_WHILE_BUSY?
>>>
>>> This occurs on a braswell based chromebook, using the acpi sdhci controller.
>>> So yes, using MMC_CAP_WAIT_WHILE_BUSY.
>>>
>>> [...]
>>>>>
>>>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>>>> index f2d185c..46a4562 100644
>>>>> --- a/drivers/mmc/core/mmc.c
>>>>> +++ b/drivers/mmc/core/mmc.c
>>>>> @@ -1925,8 +1925,14 @@ static int _mmc_suspend(struct mmc_host *host,
>>>>> bool is_suspend)
>>>>>         if (mmc_can_poweroff_notify(host->card) &&
>>>>>                 ((host->caps2 & MMC_CAP2_FULL_PWR_CYCLE) || !is_suspend))
>>>>>                 err = mmc_poweroff_notify(host->card, notify_type);
>>>>> -       else if (mmc_can_sleep(host->card))
>>>>> +       else if (mmc_can_sleep(host->card)) {
>>>>> +               if (host->card->quirks &
>>>>> MMC_QUIRK_NOTIFY_POWEROFF_ON_SLEEP) {
>>>>> +                       err = mmc_poweroff_notify(host->card,
>>>>> notify_type);
>>>>> +                       if (err)
>>>>> +                               goto out;
>>>>> +               }
>>>>
>>>>
>>>> So, I am curious to know from a power management point of view; how
>>>> does the card behave comparing the sleep and power off notification
>>>> command?
>>>>
>>>> Is the card in a low power state after the power off notification has
>>>> been received? If so, did you manage to do some measurement for that
>>>> or perhaps the data-sheet tells about this? It would be interesting to
>>>> know if there were any differences between sleep and power off
>>>> notification in this regards.
>>>
>>> I do not have any clue about that. It appears only with Hynix emmc and the
>>> fix has been approved by Hynix engineers... It seems that if not powered
>>> off, the firmware does some garbage collection when resuming and it takes
>>> more time...
>>
>> Okay, I see. So before I continue reviewing, can you please also tell
>> what regulators to the card that is being cut while powering off in
>> this path.
>>
>> VMMC, VQMMC?
>>
>> Kind regards
>> Uffe
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>