diff mbox

mmc: core: Remove MMC_CAP2_HC_ERASE_SZ

Message ID 1497257366-4223-1-git-send-email-ulf.hansson@linaro.org
State Superseded
Headers show

Commit Message

Ulf Hansson June 12, 2017, 8:49 a.m. UTC 
The MMC_CAP2_HC_ERASE_SZ is used only by a few mmc host drivers. Its intent
is to enable eMMC's high-capacity erase size, as to improve the behaviour
of the erase operations.

However, for eMMCs supporting trim/discard, the MMC_CAP2_HC_ERASE_SZ
becomes somewhat redundant. This because trim/discard operates on sectors
and these commands takes precedence over erase commands, when supported.

In addition, we should really avoid software configuration options that
isn't necessary, but instead strive to deploy common behaviours. For these
reasons, let's remove the capability bit for MMC_CAP2_HC_ERASE_SZ and the
corresponding behaviour.

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

---
 drivers/mmc/core/mmc.c            | 3 +--
 drivers/mmc/host/sdhci-acpi.c     | 1 -
 drivers/mmc/host/sdhci-brcmstb.c  | 3 ---
 drivers/mmc/host/sdhci-pci-core.c | 4 +---
 include/linux/mmc/host.h          | 1 -
 5 files changed, 2 insertions(+), 10 deletions(-)

-- 
2.7.4

Comments

Adrian Hunter June 12, 2017, 9:26 a.m. UTC | #1 
On 12/06/17 11:49, Ulf Hansson wrote:
> The MMC_CAP2_HC_ERASE_SZ is used only by a few mmc host drivers. Its intent

> is to enable eMMC's high-capacity erase size, as to improve the behaviour

> of the erase operations.

> 

> However, for eMMCs supporting trim/discard, the MMC_CAP2_HC_ERASE_SZ

> becomes somewhat redundant. This because trim/discard operates on sectors

> and these commands takes precedence over erase commands, when supported.

> 

> In addition, we should really avoid software configuration options that

> isn't necessary, but instead strive to deploy common behaviours. For these

> reasons, let's remove the capability bit for MMC_CAP2_HC_ERASE_SZ and the

> corresponding behaviour.

> 

> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

> ---

>  drivers/mmc/core/mmc.c            | 3 +--

>  drivers/mmc/host/sdhci-acpi.c     | 1 -

>  drivers/mmc/host/sdhci-brcmstb.c  | 3 ---

>  drivers/mmc/host/sdhci-pci-core.c | 4 +---

>  include/linux/mmc/host.h          | 1 -

>  5 files changed, 2 insertions(+), 10 deletions(-)

> 

> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c

> index e504b66..d1a6d74 100644

> --- a/drivers/mmc/core/mmc.c

> +++ b/drivers/mmc/core/mmc.c

> @@ -1655,8 +1655,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,

>  	 * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF

>  	 * bit.  This bit will be lost every time after a reset or power off.

>  	 */

> -	if (card->ext_csd.partition_setting_completed ||

> -	    (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {

> +	if (card->ext_csd.partition_setting_completed) {


PARTITION_SETTING_COMPLETED was added in eMMC v4.4 but ERASE_GROUP_DEF was
added in eMMC v4.3, so this doesn't work for v4.3 devices.

Why not enable it always? i.e.

	if (card->ext_csd.rev >= 3) {

>  		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,

>  				 EXT_CSD_ERASE_GROUP_DEF, 1,

>  				 card->ext_csd.generic_cmd6_time);

> diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c

> index 89d9a8c..cf66a3d 100644

> --- a/drivers/mmc/host/sdhci-acpi.c

> +++ b/drivers/mmc/host/sdhci-acpi.c

> @@ -274,7 +274,6 @@ static const struct sdhci_acpi_slot sdhci_acpi_slot_int_emmc = {

>  	.caps    = MMC_CAP_8_BIT_DATA | MMC_CAP_NONREMOVABLE |

>  		   MMC_CAP_HW_RESET | MMC_CAP_1_8V_DDR |

>  		   MMC_CAP_CMD_DURING_TFR | MMC_CAP_WAIT_WHILE_BUSY,

> -	.caps2   = MMC_CAP2_HC_ERASE_SZ,

>  	.flags   = SDHCI_ACPI_RUNTIME_PM,

>  	.quirks  = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,

>  	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |

> diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c

> index 242c5dc..e2f6383 100644

> --- a/drivers/mmc/host/sdhci-brcmstb.c

> +++ b/drivers/mmc/host/sdhci-brcmstb.c

> @@ -89,9 +89,6 @@ static int sdhci_brcmstb_probe(struct platform_device *pdev)

>  		goto err_clk;

>  	}

>  

> -	/* Enable MMC_CAP2_HC_ERASE_SZ for better max discard calculations */

> -	host->mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ;

> -

>  	sdhci_get_of_property(pdev);

>  	mmc_of_parse(host->mmc);

>  

> diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c

> index 8fa84a0..227a5cb 100644

> --- a/drivers/mmc/host/sdhci-pci-core.c

> +++ b/drivers/mmc/host/sdhci-pci-core.c

> @@ -347,8 +347,7 @@ static inline void sdhci_pci_remove_own_cd(struct sdhci_pci_slot *slot)

>  static int mfd_emmc_probe_slot(struct sdhci_pci_slot *slot)

>  {

>  	slot->host->mmc->caps |= MMC_CAP_8_BIT_DATA | MMC_CAP_NONREMOVABLE;

> -	slot->host->mmc->caps2 |= MMC_CAP2_BOOTPART_NOACC |

> -				  MMC_CAP2_HC_ERASE_SZ;

> +	slot->host->mmc->caps2 |= MMC_CAP2_BOOTPART_NOACC;

>  	return 0;

>  }

>  

> @@ -587,7 +586,6 @@ static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)

>  				 MMC_CAP_HW_RESET | MMC_CAP_1_8V_DDR |

>  				 MMC_CAP_CMD_DURING_TFR |

>  				 MMC_CAP_WAIT_WHILE_BUSY;

> -	slot->host->mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ;

>  	slot->hw_reset = sdhci_pci_int_hw_reset;

>  	if (slot->chip->pdev->device == PCI_DEVICE_ID_INTEL_BSW_EMMC)

>  		slot->host->timeout_clk = 1000; /* 1000 kHz i.e. 1 MHz */

> diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h

> index 9209f95..c81380a 100644

> --- a/include/linux/mmc/host.h

> +++ b/include/linux/mmc/host.h

> @@ -287,7 +287,6 @@ struct mmc_host {

>  #define MMC_CAP2_HS200_1_2V_SDR	(1 << 6)        /* can support */

>  #define MMC_CAP2_HS200		(MMC_CAP2_HS200_1_8V_SDR | \

>  				 MMC_CAP2_HS200_1_2V_SDR)

> -#define MMC_CAP2_HC_ERASE_SZ	(1 << 9)	/* High-capacity erase size */

>  #define MMC_CAP2_CD_ACTIVE_HIGH	(1 << 10)	/* Card-detect signal active high */

>  #define MMC_CAP2_RO_ACTIVE_HIGH	(1 << 11)	/* Write-protect signal active high */

>  #define MMC_CAP2_PACKED_RD	(1 << 12)	/* Allow packed read */

> 


--
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 June 12, 2017, 12:08 p.m. UTC | #2 
On 12 June 2017 at 11:26, Adrian Hunter <adrian.hunter@intel.com> wrote:
> On 12/06/17 11:49, Ulf Hansson wrote:

>> The MMC_CAP2_HC_ERASE_SZ is used only by a few mmc host drivers. Its intent

>> is to enable eMMC's high-capacity erase size, as to improve the behaviour

>> of the erase operations.

>>

>> However, for eMMCs supporting trim/discard, the MMC_CAP2_HC_ERASE_SZ

>> becomes somewhat redundant. This because trim/discard operates on sectors

>> and these commands takes precedence over erase commands, when supported.

>>

>> In addition, we should really avoid software configuration options that

>> isn't necessary, but instead strive to deploy common behaviours. For these

>> reasons, let's remove the capability bit for MMC_CAP2_HC_ERASE_SZ and the

>> corresponding behaviour.

>>

>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

>> ---

>>  drivers/mmc/core/mmc.c            | 3 +--

>>  drivers/mmc/host/sdhci-acpi.c     | 1 -

>>  drivers/mmc/host/sdhci-brcmstb.c  | 3 ---

>>  drivers/mmc/host/sdhci-pci-core.c | 4 +---

>>  include/linux/mmc/host.h          | 1 -

>>  5 files changed, 2 insertions(+), 10 deletions(-)

>>

>> diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c

>> index e504b66..d1a6d74 100644

>> --- a/drivers/mmc/core/mmc.c

>> +++ b/drivers/mmc/core/mmc.c

>> @@ -1655,8 +1655,7 @@ static int mmc_init_card(struct mmc_host *host, u32 ocr,

>>        * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF

>>        * bit.  This bit will be lost every time after a reset or power off.

>>        */

>> -     if (card->ext_csd.partition_setting_completed ||

>> -         (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {

>> +     if (card->ext_csd.partition_setting_completed) {

>

> PARTITION_SETTING_COMPLETED was added in eMMC v4.4 but ERASE_GROUP_DEF was

> added in eMMC v4.3, so this doesn't work for v4.3 devices.

>

> Why not enable it always? i.e.

>

>         if (card->ext_csd.rev >= 3) {


Yes, that makes sense! Thanks for spotting this!

However, that actually means this didn't work as expected for hosts
that didn't specify MMC_CAP2_HC_ERASE_SZ.

[...]

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
diff mbox

Patch

diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c
index e504b66..d1a6d74 100644
--- a/drivers/mmc/core/mmc.c
+++ b/drivers/mmc/core/mmc.c
@@ -1655,8 +1655,7 @@  static int mmc_init_card(struct mmc_host *host, u32 ocr,
 	 * If enhanced_area_en is TRUE, host needs to enable ERASE_GRP_DEF
 	 * bit.  This bit will be lost every time after a reset or power off.
 	 */
-	if (card->ext_csd.partition_setting_completed ||
-	    (card->ext_csd.rev >= 3 && (host->caps2 & MMC_CAP2_HC_ERASE_SZ))) {
+	if (card->ext_csd.partition_setting_completed) {
 		err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
 				 EXT_CSD_ERASE_GROUP_DEF, 1,
 				 card->ext_csd.generic_cmd6_time);
diff --git a/drivers/mmc/host/sdhci-acpi.c b/drivers/mmc/host/sdhci-acpi.c
index 89d9a8c..cf66a3d 100644
--- a/drivers/mmc/host/sdhci-acpi.c
+++ b/drivers/mmc/host/sdhci-acpi.c
@@ -274,7 +274,6 @@  static const struct sdhci_acpi_slot sdhci_acpi_slot_int_emmc = {
 	.caps    = MMC_CAP_8_BIT_DATA | MMC_CAP_NONREMOVABLE |
 		   MMC_CAP_HW_RESET | MMC_CAP_1_8V_DDR |
 		   MMC_CAP_CMD_DURING_TFR | MMC_CAP_WAIT_WHILE_BUSY,
-	.caps2   = MMC_CAP2_HC_ERASE_SZ,
 	.flags   = SDHCI_ACPI_RUNTIME_PM,
 	.quirks  = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC,
 	.quirks2 = SDHCI_QUIRK2_PRESET_VALUE_BROKEN |
diff --git a/drivers/mmc/host/sdhci-brcmstb.c b/drivers/mmc/host/sdhci-brcmstb.c
index 242c5dc..e2f6383 100644
--- a/drivers/mmc/host/sdhci-brcmstb.c
+++ b/drivers/mmc/host/sdhci-brcmstb.c
@@ -89,9 +89,6 @@  static int sdhci_brcmstb_probe(struct platform_device *pdev)
 		goto err_clk;
 	}
 
-	/* Enable MMC_CAP2_HC_ERASE_SZ for better max discard calculations */
-	host->mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ;
-
 	sdhci_get_of_property(pdev);
 	mmc_of_parse(host->mmc);
 
diff --git a/drivers/mmc/host/sdhci-pci-core.c b/drivers/mmc/host/sdhci-pci-core.c
index 8fa84a0..227a5cb 100644
--- a/drivers/mmc/host/sdhci-pci-core.c
+++ b/drivers/mmc/host/sdhci-pci-core.c
@@ -347,8 +347,7 @@  static inline void sdhci_pci_remove_own_cd(struct sdhci_pci_slot *slot)
 static int mfd_emmc_probe_slot(struct sdhci_pci_slot *slot)
 {
 	slot->host->mmc->caps |= MMC_CAP_8_BIT_DATA | MMC_CAP_NONREMOVABLE;
-	slot->host->mmc->caps2 |= MMC_CAP2_BOOTPART_NOACC |
-				  MMC_CAP2_HC_ERASE_SZ;
+	slot->host->mmc->caps2 |= MMC_CAP2_BOOTPART_NOACC;
 	return 0;
 }
 
@@ -587,7 +586,6 @@  static int byt_emmc_probe_slot(struct sdhci_pci_slot *slot)
 				 MMC_CAP_HW_RESET | MMC_CAP_1_8V_DDR |
 				 MMC_CAP_CMD_DURING_TFR |
 				 MMC_CAP_WAIT_WHILE_BUSY;
-	slot->host->mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ;
 	slot->hw_reset = sdhci_pci_int_hw_reset;
 	if (slot->chip->pdev->device == PCI_DEVICE_ID_INTEL_BSW_EMMC)
 		slot->host->timeout_clk = 1000; /* 1000 kHz i.e. 1 MHz */
diff --git a/include/linux/mmc/host.h b/include/linux/mmc/host.h
index 9209f95..c81380a 100644
--- a/include/linux/mmc/host.h
+++ b/include/linux/mmc/host.h
@@ -287,7 +287,6 @@  struct mmc_host {
 #define MMC_CAP2_HS200_1_2V_SDR	(1 << 6)        /* can support */
 #define MMC_CAP2_HS200		(MMC_CAP2_HS200_1_8V_SDR | \
 				 MMC_CAP2_HS200_1_2V_SDR)
-#define MMC_CAP2_HC_ERASE_SZ	(1 << 9)	/* High-capacity erase size */
 #define MMC_CAP2_CD_ACTIVE_HIGH	(1 << 10)	/* Card-detect signal active high */
 #define MMC_CAP2_RO_ACTIVE_HIGH	(1 << 11)	/* Write-protect signal active high */
 #define MMC_CAP2_PACKED_RD	(1 << 12)	/* Allow packed read */