[PATCH 1/2] mmc: core: Use mmc_flush_cache() during mmc suspend

[PATCH 1/2] mmc: core: Use mmc_flush_cache() during mmc suspend

[Ulf Hansson]

Earlier we disabled the cache during suspend, which meant a flush was
internally at the eMMC performed as well.

To simplify code we can make use of the mmc_flush_cache(), during mmc
suspend, which makes the mmc_cache_ctrl() redundant so then we can
remove it.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/core.c  |   38 --------------------------------------
 drivers/mmc/core/mmc.c   |    2 +-
 include/linux/mmc/host.h |    2 --
 3 files changed, 1 insertion(+), 41 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 098374b..df591a9 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2602,44 +2602,6 @@ int mmc_flush_cache(struct mmc_card *card)
 }
 EXPORT_SYMBOL(mmc_flush_cache);
 
-/*
- * Turn the cache ON/OFF.
- * Turning the cache OFF shall trigger flushing of the data
- * to the non-volatile storage.
- * This function should be called with host claimed
- */
-int mmc_cache_ctrl(struct mmc_host *host, u8 enable)
-{
-	struct mmc_card *card = host->card;
-	unsigned int timeout;
-	int err = 0;
-
-	if (!(host->caps2 & MMC_CAP2_CACHE_CTRL) ||
-			mmc_card_is_removable(host))
-		return err;
-
-	if (card && mmc_card_mmc(card) &&
-			(card->ext_csd.cache_size > 0)) {
-		enable = !!enable;
-
-		if (card->ext_csd.cache_ctrl ^ enable) {
-			timeout = enable ? card->ext_csd.generic_cmd6_time : 0;
-			err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
-					EXT_CSD_CACHE_CTRL, enable, timeout);
-			if (err)
-				pr_err("%s: cache %s error %d\n",
-						mmc_hostname(card->host),
-						enable ? "on" : "off",
-						err);
-			else
-				card->ext_csd.cache_ctrl = enable;
-		}
-	}
-
-	return err;
-}
-EXPORT_SYMBOL(mmc_cache_ctrl);
-
 #ifdef CONFIG_PM
 
 /* Do the card removal on suspend if card is assumed removeable
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index 98e9eb0..ef1cc73 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1484,7 +1484,7 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
 			goto out;
 	}
 
-	err = mmc_cache_ctrl(host, 0);
+	err = mmc_flush_cache(host->card);
 	if (err)
 		goto out;
 
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 99f5709..f539bc7 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -388,8 +388,6 @@ int mmc_power_restore_host(struct mmc_host *host);
 void mmc_detect_change(struct mmc_host *, unsigned long delay);
 void mmc_request_done(struct mmc_host *, struct mmc_request *);
 
-int mmc_cache_ctrl(struct mmc_host *, u8);
-
 static inline void mmc_signal_sdio_irq(struct mmc_host *host)
 {
 	host->ops->enable_sdio_irq(host, 0);
-- 
1.7.9.5

[PATCH 2/2] mmc: core: Enable MMC_CAP2_CACHE_CTRL as default

[Ulf Hansson]

There are no reason to why the use of a non-volatile internal eMMC
cache should be controlled by a host cap. Instead let's just enable it
if the eMMC card supports it.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---
 drivers/mmc/core/core.c  |    3 ---
 drivers/mmc/core/mmc.c   |    3 +--
 include/linux/mmc/host.h |    1 -
 3 files changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index df591a9..66ec347 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -2585,9 +2585,6 @@ int mmc_flush_cache(struct mmc_card *card)
 	struct mmc_host *host = card->host;
 	int err = 0;
 
-	if (!(host->caps2 & MMC_CAP2_CACHE_CTRL))
-		return err;
-
 	if (mmc_card_mmc(card) &&
 			(card->ext_csd.cache_size > 0) &&
 			(card->ext_csd.cache_ctrl & 1)) {
diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index ef1cc73..7ab3e9c 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1287,8 +1287,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	 * If cache size is higher than 0, this indicates
 	 * the existence of cache and it can be turned on.
 	 */
-	if ((host->caps2 & MMC_CAP2_CACHE_CTRL) &&
-			card->ext_csd.cache_size > 0) {
+	if (card->ext_csd.cache_size > 0) {
 		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 				EXT_CSD_CACHE_CTRL, 1,
 				card->ext_csd.generic_cmd6_time);
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index f539bc7..8383e3f 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -264,7 +264,6 @@ struct mmc_host {
 	u32			caps2;		/* More host capabilities */
 
 #define MMC_CAP2_BOOTPART_NOACC	(1 << 0)	/* Boot partition no access */
-#define MMC_CAP2_CACHE_CTRL	(1 << 1)	/* Allow cache control */
 #define MMC_CAP2_FULL_PWR_CYCLE	(1 << 2)	/* Can do full power cycle */
 #define MMC_CAP2_NO_MULTI_READ	(1 << 3)	/* Multiblock reads don't work */
 #define MMC_CAP2_NO_SLEEP_CMD	(1 << 4)	/* Don't allow sleep command */
-- 
1.7.9.5

RE: [PATCH 2/2] mmc: core: Enable MMC_CAP2_CACHE_CTRL as default

[Seungwon Jeon]

On Wed, December 18, 2013 Ulf Hansson wrote: 
> There are no reason to why the use of a non-volatile internal eMMC
> cache should be controlled by a host cap. Instead let's just enable it
> if the eMMC card supports it.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

When cache feature was introduced for the first time, there was stability issue in some devices.
So host should have decided whether to select that feature.
Now, your change makes sense. But just check minor one below.

Acked-by: Seungwon Jeon <tgih.jun@samsung.com>

> ---
>  drivers/mmc/core/core.c  |    3 ---
>  drivers/mmc/core/mmc.c   |    3 +--
>  include/linux/mmc/host.h |    1 -
>  3 files changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index df591a9..66ec347 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2585,9 +2585,6 @@ int mmc_flush_cache(struct mmc_card *card)
>  	struct mmc_host *host = card->host;
Also, 'host' can be removed.  No use.

Thanks,
Seungwon Jeon

>  	int err = 0;
> 
> -	if (!(host->caps2 & MMC_CAP2_CACHE_CTRL))
> -		return err;
> -
>  	if (mmc_card_mmc(card) &&
>  			(card->ext_csd.cache_size > 0) &&
>  			(card->ext_csd.cache_ctrl & 1)) {
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index ef1cc73..7ab3e9c 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1287,8 +1287,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>  	 * If cache size is higher than 0, this indicates
>  	 * the existence of cache and it can be turned on.
>  	 */
> -	if ((host->caps2 & MMC_CAP2_CACHE_CTRL) &&
> -			card->ext_csd.cache_size > 0) {
> +	if (card->ext_csd.cache_size > 0) {
>  		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>  				EXT_CSD_CACHE_CTRL, 1,
>  				card->ext_csd.generic_cmd6_time);
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index f539bc7..8383e3f 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -264,7 +264,6 @@ struct mmc_host {
>  	u32			caps2;		/* More host capabilities */
> 
>  #define MMC_CAP2_BOOTPART_NOACC	(1 << 0)	/* Boot partition no access */
> -#define MMC_CAP2_CACHE_CTRL	(1 << 1)	/* Allow cache control */
>  #define MMC_CAP2_FULL_PWR_CYCLE	(1 << 2)	/* Can do full power cycle */
>  #define MMC_CAP2_NO_MULTI_READ	(1 << 3)	/* Multiblock reads don't work */
>  #define MMC_CAP2_NO_SLEEP_CMD	(1 << 4)	/* Don't allow sleep command */
> --
> 1.7.9.5
> 
> --
> 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 1/2] mmc: core: Use mmc_flush_cache() during mmc suspend

[Seungwon Jeon]

On Wed, December 18, 2013 Ulf Hansson wrote:
> Earlier we disabled the cache during suspend, which meant a flush was
> internally at the eMMC performed as well.
> 
> To simplify code we can make use of the mmc_flush_cache(), during mmc
> suspend, which makes the mmc_cache_ctrl() redundant so then we can
> remove it.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Acked-by: Seungwon Jeon <tgih.jun@samsung.com>

Thanks,
Seungwon Jeon

> ---
>  drivers/mmc/core/core.c  |   38 --------------------------------------
>  drivers/mmc/core/mmc.c   |    2 +-
>  include/linux/mmc/host.h |    2 --
>  3 files changed, 1 insertion(+), 41 deletions(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 098374b..df591a9 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -2602,44 +2602,6 @@ int mmc_flush_cache(struct mmc_card *card)
>  }
>  EXPORT_SYMBOL(mmc_flush_cache);
> 
> -/*
> - * Turn the cache ON/OFF.
> - * Turning the cache OFF shall trigger flushing of the data
> - * to the non-volatile storage.
> - * This function should be called with host claimed
> - */
> -int mmc_cache_ctrl(struct mmc_host *host, u8 enable)
> -{
> -	struct mmc_card *card = host->card;
> -	unsigned int timeout;
> -	int err = 0;
> -
> -	if (!(host->caps2 & MMC_CAP2_CACHE_CTRL) ||
> -			mmc_card_is_removable(host))
> -		return err;
> -
> -	if (card && mmc_card_mmc(card) &&
> -			(card->ext_csd.cache_size > 0)) {
> -		enable = !!enable;
> -
> -		if (card->ext_csd.cache_ctrl ^ enable) {
> -			timeout = enable ? card->ext_csd.generic_cmd6_time : 0;
> -			err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> -					EXT_CSD_CACHE_CTRL, enable, timeout);
> -			if (err)
> -				pr_err("%s: cache %s error %d\n",
> -						mmc_hostname(card->host),
> -						enable ? "on" : "off",
> -						err);
> -			else
> -				card->ext_csd.cache_ctrl = enable;
> -		}
> -	}
> -
> -	return err;
> -}
> -EXPORT_SYMBOL(mmc_cache_ctrl);
> -
>  #ifdef CONFIG_PM
> 
>  /* Do the card removal on suspend if card is assumed removeable
> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
> index 98e9eb0..ef1cc73 100644
> --- a/drivers/mmc/core/mmc.c
> +++ b/drivers/mmc/core/mmc.c
> @@ -1484,7 +1484,7 @@ static int _mmc_suspend(struct mmc_host *host, bool is_suspend)
>  			goto out;
>  	}
> 
> -	err = mmc_cache_ctrl(host, 0);
> +	err = mmc_flush_cache(host->card);
>  	if (err)
>  		goto out;
> 
> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
> index 99f5709..f539bc7 100644
> --- a/include/linux/mmc/host.h
> +++ b/include/linux/mmc/host.h
> @@ -388,8 +388,6 @@ int mmc_power_restore_host(struct mmc_host *host);
>  void mmc_detect_change(struct mmc_host *, unsigned long delay);
>  void mmc_request_done(struct mmc_host *, struct mmc_request *);
> 
> -int mmc_cache_ctrl(struct mmc_host *, u8);
> -
>  static inline void mmc_signal_sdio_irq(struct mmc_host *host)
>  {
>  	host->ops->enable_sdio_irq(host, 0);
> --
> 1.7.9.5
> 
> --
> 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 2/2] mmc: core: Enable MMC_CAP2_CACHE_CTRL as default

[Ulf Hansson]

On 19 December 2013 15:43, Seungwon Jeon <tgih.jun@samsung.com> wrote:
> On Wed, December 18, 2013 Ulf Hansson wrote:
>> There are no reason to why the use of a non-volatile internal eMMC
>> cache should be controlled by a host cap. Instead let's just enable it
>> if the eMMC card supports it.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>
> When cache feature was introduced for the first time, there was stability issue in some devices.
> So host should have decided whether to select that feature.
> Now, your change makes sense. But just check minor one below.
>
> Acked-by: Seungwon Jeon <tgih.jun@samsung.com>
>
>> ---
>>  drivers/mmc/core/core.c  |    3 ---
>>  drivers/mmc/core/mmc.c   |    3 +--
>>  include/linux/mmc/host.h |    1 -
>>  3 files changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>> index df591a9..66ec347 100644
>> --- a/drivers/mmc/core/core.c
>> +++ b/drivers/mmc/core/core.c
>> @@ -2585,9 +2585,6 @@ int mmc_flush_cache(struct mmc_card *card)
>>       struct mmc_host *host = card->host;
> Also, 'host' can be removed.  No use.

Thanks for your review! Will fix in v2 and add your ack.

Kind regards
Ulf Hansson

>
> Thanks,
> Seungwon Jeon
>
>>       int err = 0;
>>
>> -     if (!(host->caps2 & MMC_CAP2_CACHE_CTRL))
>> -             return err;
>> -
>>       if (mmc_card_mmc(card) &&
>>                       (card->ext_csd.cache_size > 0) &&
>>                       (card->ext_csd.cache_ctrl & 1)) {
>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
>> index ef1cc73..7ab3e9c 100644
>> --- a/drivers/mmc/core/mmc.c
>> +++ b/drivers/mmc/core/mmc.c
>> @@ -1287,8 +1287,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,
>>        * If cache size is higher than 0, this indicates
>>        * the existence of cache and it can be turned on.
>>        */
>> -     if ((host->caps2 & MMC_CAP2_CACHE_CTRL) &&
>> -                     card->ext_csd.cache_size > 0) {
>> +     if (card->ext_csd.cache_size > 0) {
>>               err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
>>                               EXT_CSD_CACHE_CTRL, 1,
>>                               card->ext_csd.generic_cmd6_time);
>> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
>> index f539bc7..8383e3f 100644
>> --- a/include/linux/mmc/host.h
>> +++ b/include/linux/mmc/host.h
>> @@ -264,7 +264,6 @@ struct mmc_host {
>>       u32                     caps2;          /* More host capabilities */
>>
>>  #define MMC_CAP2_BOOTPART_NOACC      (1 << 0)        /* Boot partition no access */
>> -#define MMC_CAP2_CACHE_CTRL  (1 << 1)        /* Allow cache control */
>>  #define MMC_CAP2_FULL_PWR_CYCLE      (1 << 2)        /* Can do full power cycle */
>>  #define MMC_CAP2_NO_MULTI_READ       (1 << 3)        /* Multiblock reads don't work */
>>  #define MMC_CAP2_NO_SLEEP_CMD        (1 << 4)        /* Don't allow sleep command */
>> --
>> 1.7.9.5
>>
>> --
>> 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
>