diff mbox

[v3,1/1] mmc: Hynix: add QUIRK_NOTIFY_POWEROFF_ON_SLEEP

Message ID 1480429488-22289-2-git-send-email-thierry.escande@collabora.com (mailing list archive)
State New, archived
Headers show

Commit Message

Thierry Escande Nov. 29, 2016, 2:24 p.m. UTC 
From: zhaojohn <john.zhao@intel.com>

Hynix eMMC devices sometimes take 50% longer to resume from sleep. This
occurs on Braswell based chromebook with acpi sdhci controller.

Based on a recommendation from Hynix engineers, this patch sends a
Power-Off Notification using mmc_poweroff_notify() before going to S3 to
have a resume time consistently within spec. No voltage regulator are
being cut through this notification but merely tells the eMMC firmware
that we're about to do so and get it to behave better on resume.

Signed-off-by: zhaojohn <john.zhao@intel.com>
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(-)

Comments

Jaehoon Chung Nov. 30, 2016, 8:46 a.m. UTC | #1 
On 11/29/2016 11:24 PM, Thierry Escande wrote:
> From: zhaojohn <john.zhao@intel.com>
> 
> Hynix eMMC devices sometimes take 50% longer to resume from sleep. This
> occurs on Braswell based chromebook with acpi sdhci controller.
> 
> Based on a recommendation from Hynix engineers, this patch sends a
> Power-Off Notification using mmc_poweroff_notify() before going to S3 to
> have a resume time consistently within spec. No voltage regulator are
> being cut through this notification but merely tells the eMMC firmware
> that we're about to do so and get it to behave better on resume.
> 
> Signed-off-by: zhaojohn <john.zhao@intel.com>
> 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 709a872..0066a66 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -2573,6 +2573,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("HBG4e\005", CID_MANFID_HYNIX, 0x014a, 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 df19777..774d478 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1941,8 +1941,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);

After running mmc_poweroff_notify(), also doing mmc_sleep()? Is it right?

> +	}
>  	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 73fad83..e4940f4 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 */
> 

--
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
Thierry Escande Nov. 30, 2016, 9:10 a.m. UTC | #2 
Hi Jaehoon,

On 30/11/2016 09:46, Jaehoon Chung wrote:
> On 11/29/2016 11:24 PM, Thierry Escande wrote:
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index df19777..774d478 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1941,8 +1941,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);
>
> After running mmc_poweroff_notify(), also doing mmc_sleep()? Is it right?

Yes, that's what the quirk is intended for. This firmware resumes from 
sleep faster if it has been notified for power-off. This is a 
recommendation from Hynix.

Regards,
  Thierry
--
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
Ulf Hansson Nov. 30, 2016, 10:29 a.m. UTC | #3 
+Alex

On 30 November 2016 at 10:10, Thierry Escande
<thierry.escande@collabora.com> wrote:
> Hi Jaehoon,
>
> On 30/11/2016 09:46, Jaehoon Chung wrote:
>>
>> On 11/29/2016 11:24 PM, Thierry Escande wrote:
>>>
>>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>>> index df19777..774d478 100644
>>> --- a/drivers/mmc/core/mmc.c
>>> +++ b/drivers/mmc/core/mmc.c
>>> @@ -1941,8 +1941,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);
>>
>>
>> After running mmc_poweroff_notify(), also doing mmc_sleep()? Is it right?
>
>
> Yes, that's what the quirk is intended for. This firmware resumes from sleep
> faster if it has been notified for power-off. This is a recommendation from
> Hynix.

I think we need to discuss this change in the context of eMMC sleep
notification - as a generic solution and conforming to the eMMC spec.

This was also as pointed out by Alex [1]. Perhaps you can respond to
his earlier reply?

>
> Regards,
>  Thierry

Kind regards
Uffe

[1]
http://www.spinics.net/lists/linux-mmc/msg39892.html
--
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
diff mbox

Patch

diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
index 709a872..0066a66 100644
--- a/drivers/mmc/card/block.c
+++ b/drivers/mmc/card/block.c
@@ -2573,6 +2573,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("HBG4e\005", CID_MANFID_HYNIX, 0x014a, 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 df19777..774d478 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1941,8 +1941,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 73fad83..e4940f4 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 */