[PATCH 2/2 V2] Powerpc/eSDHC: Add limit to data and erase timeout value calculation

[PATCH 2/2 V2] Powerpc/eSDHC: Add limit to data and erase timeout value calculation

[Haijun Zhang]

Some cards apply too larg timeout value for host to response,
So limit the maximum data transfer timeout value and maximum erase
timeout value to aviod timeout issue.

Signed-off-by: Haijun Zhang <Haijun.Zhang@freescale.com>
Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
CC: Anton Vorontsov <cbouatmailru@gmail.com>
---
changes for v2:
        - Add limit to data and erase timeout value calculation
        - split the V1 patch into two V2 patchs.

 drivers/mmc/core/core.c |   27 ++++++++++++++++-----------
 1 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index c241fc1..c90b791 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -721,6 +721,10 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
 				data->timeout_ns =  100000000;	/* 100ms */
 		}
 	}
+
+	if (card->host->max_discard_to &&
+		card->host->max_discard_to < div_u64(data->timeout_ns, 1000000))
+		data->timeout_ns = (u64)card->host->max_discard_to * 1000000;
 }
 EXPORT_SYMBOL(mmc_set_data_timeout);
 
@@ -1880,7 +1884,7 @@ static unsigned int mmc_do_calc_max_discard(struct mmc_card *card,
 		return 0;
 
 	if (qty == 1)
-		return 1;
+		return 1 << card->erase_shift;
 
 	/* Convert qty to sectors */
 	if (card->erase_shift)
@@ -1898,16 +1902,17 @@ unsigned int mmc_calc_max_discard(struct mmc_card *card)
 	struct mmc_host *host = card->host;
 	unsigned int max_discard, max_trim;
 
-	if (!host->max_discard_to)
-		return UINT_MAX;
-
-	/*
-	 * Without erase_group_def set, MMC erase timeout depends on clock
-	 * frequence which can change.  In that case, the best choice is
-	 * just the preferred erase size.
-	 */
-	if (mmc_card_mmc(card) && !(card->ext_csd.erase_group_def & 1))
-		return card->pref_erase;
+	if (!host->max_discard_to) {
+		if (mmc_card_sd(card))
+			return UINT_MAX;
+		/*
+		 * Without erase_group_def set, MMC erase timeout depends on
+		 * clock frequence which can change.  In that case, the best
+		 * choice is just the preferred erase size.
+		 */
+		if (mmc_card_mmc(card) && !(card->ext_csd.erase_group_def & 1))
+			return card->pref_erase;
+	}
 
 	max_discard = mmc_do_calc_max_discard(card, MMC_ERASE_ARG);
 	if (mmc_can_trim(card)) {
-- 
1.7.0.4

[PATCH 1/2 V2] Powerpc/eSDHC: Calculate the applicable mmc erase timeout value

[Haijun Zhang]

As large area erase needs long time usually a few minutes,
which the host can't wait will bring about timeout error.
So we need to split the large area to small sections which
only need short erase time to avoid timeout error.

Signed-off-by: Haijun Zhang <Haijun.Zhang@freescale.com>
Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
CC: Anton Vorontsov <cbouatmailru@gmail.com>
---
changes for v2:
        - Recompute the timeout value and max_discard_to for mmc erase
	- split the V1 patch into two V2 patchs.

 drivers/mmc/host/sdhci-esdhc.h    |    1 -
 drivers/mmc/host/sdhci-of-esdhc.c |   13 +++++++++++++
 2 files changed, 13 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
index d25f9ab..bb6d664 100644
--- a/drivers/mmc/host/sdhci-esdhc.h
+++ b/drivers/mmc/host/sdhci-esdhc.h
@@ -21,7 +21,6 @@
 #define ESDHC_DEFAULT_QUIRKS	(SDHCI_QUIRK_FORCE_BLK_SZ_2048 | \
 				SDHCI_QUIRK_NO_BUSY_IRQ | \
 				SDHCI_QUIRK_NONSTANDARD_CLOCK | \
-				SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \
 				SDHCI_QUIRK_PIO_NEEDS_DELAY | \
 				SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
 
diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
index 63d219f..a09ea67 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -154,6 +154,18 @@ static void esdhc_of_set_clock(struct sdhci_host *host, unsigned int clock)
 	/* Set the clock */
 	esdhc_set_clock(host, clock);
 }
+/*
+ * As host dosn't supply us the method to calculate the timeout value,
+ * we assigned one for high speed SDHC card. So we can use this to calculate
+ * the max discard timeout value to limit the max discard sectors to avoid the
+ * timeout issue during large area erase.
+ */
+static unsigned int esdhc_of_get_timeout_clock(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+
+	return pltfm_host->clock / 1000 / 32;
+}
 
 #ifdef CONFIG_PM
 static u32 esdhc_proctl;
@@ -190,6 +202,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {
 	.enable_dma = esdhc_of_enable_dma,
 	.get_max_clock = esdhc_of_get_max_clock,
 	.get_min_clock = esdhc_of_get_min_clock,
+	.get_timeout_clock = esdhc_of_get_timeout_clock,
 	.platform_init = esdhc_of_platform_init,
 #ifdef CONFIG_PM
 	.platform_suspend = esdhc_of_suspend,
-- 
1.7.0.4

Re: [PATCH 1/2 V2] Powerpc/eSDHC: Calculate the applicable mmc erase timeout value

[Anton Vorontsov]

On Tue, Nov 13, 2012 at 05:09:22PM +0800, Haijun Zhang wrote:
> As large area erase needs long time usually a few minutes,
> which the host can't wait will bring about timeout error.
> So we need to split the large area to small sections which
> only need short erase time to avoid timeout error.
> 
> Signed-off-by: Haijun Zhang <Haijun.Zhang@freescale.com>
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> CC: Anton Vorontsov <cbouatmailru@gmail.com>
> ---
> changes for v2:
>         - Recompute the timeout value and max_discard_to for mmc erase
> 	- split the V1 patch into two V2 patchs.

Oh, I see. A new version! :)

>  drivers/mmc/host/sdhci-esdhc.h    |    1 -
>  drivers/mmc/host/sdhci-of-esdhc.c |   13 +++++++++++++
>  2 files changed, 13 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/mmc/host/sdhci-esdhc.h b/drivers/mmc/host/sdhci-esdhc.h
> index d25f9ab..bb6d664 100644
> --- a/drivers/mmc/host/sdhci-esdhc.h
> +++ b/drivers/mmc/host/sdhci-esdhc.h
> @@ -21,7 +21,6 @@
>  #define ESDHC_DEFAULT_QUIRKS	(SDHCI_QUIRK_FORCE_BLK_SZ_2048 | \
>  				SDHCI_QUIRK_NO_BUSY_IRQ | \
>  				SDHCI_QUIRK_NONSTANDARD_CLOCK | \
> -				SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \
>  				SDHCI_QUIRK_PIO_NEEDS_DELAY | \
>  				SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
>  
> diff --git a/drivers/mmc/host/sdhci-of-esdhc.c b/drivers/mmc/host/sdhci-of-esdhc.c
> index 63d219f..a09ea67 100644
> --- a/drivers/mmc/host/sdhci-of-esdhc.c
> +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> @@ -154,6 +154,18 @@ static void esdhc_of_set_clock(struct sdhci_host *host, unsigned int clock)
>  	/* Set the clock */
>  	esdhc_set_clock(host, clock);
>  }
> +/*

An empty line is missing after the closing curly brace.

Otherwise, it looks good

Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>

> + * As host dosn't supply us the method to calculate the timeout value,
> + * we assigned one for high speed SDHC card. So we can use this to calculate
> + * the max discard timeout value to limit the max discard sectors to avoid the
> + * timeout issue during large area erase.
> + */
> +static unsigned int esdhc_of_get_timeout_clock(struct sdhci_host *host)
> +{
> +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> +
> +	return pltfm_host->clock / 1000 / 32;
> +}
>  
>  #ifdef CONFIG_PM
>  static u32 esdhc_proctl;
> @@ -190,6 +202,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {
>  	.enable_dma = esdhc_of_enable_dma,
>  	.get_max_clock = esdhc_of_get_max_clock,
>  	.get_min_clock = esdhc_of_get_min_clock,
> +	.get_timeout_clock = esdhc_of_get_timeout_clock,
>  	.platform_init = esdhc_of_platform_init,
>  #ifdef CONFIG_PM
>  	.platform_suspend = esdhc_of_suspend,
> -- 
> 1.7.0.4

Re: [PATCH 2/2 V2] Powerpc/eSDHC: Add limit to data and erase timeout value calculation

[Anton Vorontsov]

On Tue, Nov 13, 2012 at 05:09:21PM +0800, Haijun Zhang wrote:
> Some cards apply too larg timeout value for host to response,
> So limit the maximum data transfer timeout value and maximum erase
> timeout value to aviod timeout issue.
> 
> Signed-off-by: Haijun Zhang <Haijun.Zhang@freescale.com>
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> CC: Anton Vorontsov <cbouatmailru@gmail.com>
> ---
> changes for v2:
>         - Add limit to data and erase timeout value calculation
>         - split the V1 patch into two V2 patchs.
> 
>  drivers/mmc/core/core.c |   27 ++++++++++++++++-----------
>  1 files changed, 16 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index c241fc1..c90b791 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -721,6 +721,10 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
>  				data->timeout_ns =  100000000;	/* 100ms */
>  		}
>  	}
> +
> +	if (card->host->max_discard_to &&
> +		card->host->max_discard_to < div_u64(data->timeout_ns, 1000000))

Incorrect indentation, please add one more tab.

Otherwise, the patch looks OK to me.

Reviewed-by: Anton Vorontsov <cbouatmailru@gmail.com>

> +		data->timeout_ns = (u64)card->host->max_discard_to * 1000000;
>  }
>  EXPORT_SYMBOL(mmc_set_data_timeout);
>  
> @@ -1880,7 +1884,7 @@ static unsigned int mmc_do_calc_max_discard(struct mmc_card *card,
>  		return 0;
>  
>  	if (qty == 1)
> -		return 1;
> +		return 1 << card->erase_shift;
>  
>  	/* Convert qty to sectors */
>  	if (card->erase_shift)
> @@ -1898,16 +1902,17 @@ unsigned int mmc_calc_max_discard(struct mmc_card *card)
>  	struct mmc_host *host = card->host;
>  	unsigned int max_discard, max_trim;
>  
> -	if (!host->max_discard_to)
> -		return UINT_MAX;
> -
> -	/*
> -	 * Without erase_group_def set, MMC erase timeout depends on clock
> -	 * frequence which can change.  In that case, the best choice is
> -	 * just the preferred erase size.
> -	 */
> -	if (mmc_card_mmc(card) && !(card->ext_csd.erase_group_def & 1))
> -		return card->pref_erase;
> +	if (!host->max_discard_to) {
> +		if (mmc_card_sd(card))
> +			return UINT_MAX;
> +		/*
> +		 * Without erase_group_def set, MMC erase timeout depends on
> +		 * clock frequence which can change.  In that case, the best
> +		 * choice is just the preferred erase size.
> +		 */
> +		if (mmc_card_mmc(card) && !(card->ext_csd.erase_group_def & 1))
> +			return card->pref_erase;
> +	}
>  
>  	max_discard = mmc_do_calc_max_discard(card, MMC_ERASE_ARG);
>  	if (mmc_can_trim(card)) {
> -- 
> 1.7.0.4

RE: [PATCH 1/2 V2] Powerpc/eSDHC: Calculate the applicable mmc erase timeout value

[Zhang Haijun-B42677]

Thanks a lot.
I had corrected according to your suggestion. 
Three new patch had send.

Regards
Haijun.

> -----Original Message-----
> From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> owner@vger.kernel.org] On Behalf Of Anton Vorontsov
> Sent: Monday, November 19, 2012 8:43 AM
> To: Zhang Haijun-B42677
> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093
> Subject: Re: [PATCH 1/2 V2] Powerpc/eSDHC: Calculate the applicable mmc
> erase timeout value
> 
> On Tue, Nov 13, 2012 at 05:09:22PM +0800, Haijun Zhang wrote:
> > As large area erase needs long time usually a few minutes, which the
> > host can't wait will bring about timeout error.
> > So we need to split the large area to small sections which only need
> > short erase time to avoid timeout error.
> >
> > Signed-off-by: Haijun Zhang <Haijun.Zhang@freescale.com>
> > Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> > CC: Anton Vorontsov <cbouatmailru@gmail.com>
> > ---
> > changes for v2:
> >         - Recompute the timeout value and max_discard_to for mmc erase
> > 	- split the V1 patch into two V2 patchs.
> 
> Oh, I see. A new version! :)
> 
> >  drivers/mmc/host/sdhci-esdhc.h    |    1 -
> >  drivers/mmc/host/sdhci-of-esdhc.c |   13 +++++++++++++
> >  2 files changed, 13 insertions(+), 1 deletions(-)
> >
> > diff --git a/drivers/mmc/host/sdhci-esdhc.h
> > b/drivers/mmc/host/sdhci-esdhc.h index d25f9ab..bb6d664 100644
> > --- a/drivers/mmc/host/sdhci-esdhc.h
> > +++ b/drivers/mmc/host/sdhci-esdhc.h
> > @@ -21,7 +21,6 @@
> >  #define ESDHC_DEFAULT_QUIRKS	(SDHCI_QUIRK_FORCE_BLK_SZ_2048 | \
> >  				SDHCI_QUIRK_NO_BUSY_IRQ | \
> >  				SDHCI_QUIRK_NONSTANDARD_CLOCK | \
> > -				SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK | \
> >  				SDHCI_QUIRK_PIO_NEEDS_DELAY | \
> >  				SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET)
> >
> > diff --git a/drivers/mmc/host/sdhci-of-esdhc.c
> > b/drivers/mmc/host/sdhci-of-esdhc.c
> > index 63d219f..a09ea67 100644
> > --- a/drivers/mmc/host/sdhci-of-esdhc.c
> > +++ b/drivers/mmc/host/sdhci-of-esdhc.c
> > @@ -154,6 +154,18 @@ static void esdhc_of_set_clock(struct sdhci_host
> *host, unsigned int clock)
> >  	/* Set the clock */
> >  	esdhc_set_clock(host, clock);
> >  }
> > +/*
> 
> An empty line is missing after the closing curly brace.
> 
> Otherwise, it looks good
> 
> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
> 
> > + * As host dosn't supply us the method to calculate the timeout
> > +value,
> > + * we assigned one for high speed SDHC card. So we can use this to
> > +calculate
> > + * the max discard timeout value to limit the max discard sectors to
> > +avoid the
> > + * timeout issue during large area erase.
> > + */
> > +static unsigned int esdhc_of_get_timeout_clock(struct sdhci_host
> > +*host) {
> > +	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> > +
> > +	return pltfm_host->clock / 1000 / 32; }
> >
> >  #ifdef CONFIG_PM
> >  static u32 esdhc_proctl;
> > @@ -190,6 +202,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {
> >  	.enable_dma = esdhc_of_enable_dma,
> >  	.get_max_clock = esdhc_of_get_max_clock,
> >  	.get_min_clock = esdhc_of_get_min_clock,
> > +	.get_timeout_clock = esdhc_of_get_timeout_clock,
> >  	.platform_init = esdhc_of_platform_init,  #ifdef CONFIG_PM
> >  	.platform_suspend = esdhc_of_suspend,
> > --
> > 1.7.0.4
> --
> 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