[PATCH] Powerpc eSDHC: Use u64 to calculate the timeout value to avoid overflow

[PATCH] Powerpc eSDHC: Use u64 to calculate the timeout value to avoid overflow

[Haijun Zhang]

As some mmc cards need large timeout value usually a few seconds,
so data timeout nanosecond will overflow with u32 variable and
give the wrong timeout value, so use u64 will be safe.

Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
Signed-off-by: Haijun Zhang <Haijun.Zhang@freescale.com>
Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---
 drivers/mmc/core/core.c  |   21 ++++++++-------------
 drivers/mmc/host/sdhci.c |    6 +++---
 include/linux/mmc/core.h |    2 +-
 3 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 06c42cf..893144e 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -208,10 +208,10 @@ mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 
 	if (mrq->data) {
 		pr_debug("%s:     blksz %d blocks %d flags %08x "
-			"tsac %d ms nsac %d\n",
+			"tsac %lld ms nsac %d\n",
 			mmc_hostname(host), mrq->data->blksz,
 			mrq->data->blocks, mrq->data->flags,
-			mrq->data->timeout_ns / 1000000,
+			div_u64(mrq->data->timeout_ns, 1000000),
 			mrq->data->timeout_clks);
 	}
 
@@ -659,16 +659,16 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
 	if (data->flags & MMC_DATA_WRITE)
 		mult <<= card->csd.r2w_factor;
 
-	data->timeout_ns = card->csd.tacc_ns * mult;
+	data->timeout_ns = (u64)card->csd.tacc_ns * mult;
 	data->timeout_clks = card->csd.tacc_clks * mult;
 
 	/*
 	 * SD cards also have an upper limit on the timeout.
 	 */
 	if (mmc_card_sd(card)) {
-		unsigned int timeout_us, limit_us;
+		u64 timeout_us, limit_us;
 
-		timeout_us = data->timeout_ns / 1000;
+		timeout_us = div_u64(data->timeout_ns, 1000);
 		if (mmc_host_clk_rate(card->host))
 			timeout_us += data->timeout_clks * 1000 /
 				(mmc_host_clk_rate(card->host) / 1000);
@@ -1545,14 +1545,9 @@ static unsigned int mmc_mmc_erase_timeout(struct mmc_card *card,
 		/* CSD Erase Group Size uses write timeout */
 		unsigned int mult = (10 << card->csd.r2w_factor);
 		unsigned int timeout_clks = card->csd.tacc_clks * mult;
-		unsigned int timeout_us;
-
-		/* Avoid overflow: e.g. tacc_ns=80000000 mult=1280 */
-		if (card->csd.tacc_ns < 1000000)
-			timeout_us = (card->csd.tacc_ns * mult) / 1000;
-		else
-			timeout_us = (card->csd.tacc_ns / 1000) * mult;
+		u64 timeout_us;
 
+		timeout_us = (u64)(card->csd.tacc_ns / 1000) * mult;
 		/*
 		 * ios.clock is only a target.  The real clock rate might be
 		 * less but not that much less, so fudge it by multiplying by 2.
@@ -1561,7 +1556,7 @@ static unsigned int mmc_mmc_erase_timeout(struct mmc_card *card,
 		timeout_us += (timeout_clks * 1000) /
 			      (mmc_host_clk_rate(card->host) / 1000);
 
-		erase_timeout = timeout_us / 1000;
+		erase_timeout = div_u64(timeout_us, 1000);
 
 		/*
 		 * Theoretically, the calculation could underflow so round up
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f05a377..a5adc9f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -651,7 +651,7 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 {
 	u8 count;
 	struct mmc_data *data = cmd->data;
-	unsigned target_timeout, current_timeout;
+	u64 target_timeout, current_timeout;
 
 	/*
 	 * If the host controller provides us with an incorrect timeout
@@ -668,9 +668,9 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 
 	/* timeout in us */
 	if (!data)
-		target_timeout = cmd->cmd_timeout_ms * 1000;
+		target_timeout = (u64)cmd->cmd_timeout_ms * 1000;
 	else {
-		target_timeout = data->timeout_ns / 1000;
+		target_timeout = div_u64(data->timeout_ns, 1000);
 		if (host->clock)
 			target_timeout += data->timeout_clks / host->clock;
 	}
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 9b9cdaf..a2ff2e9 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -99,7 +99,7 @@ struct mmc_command {
 };
 
 struct mmc_data {
-	unsigned int		timeout_ns;	/* data timeout (in ns, max 80ms) */
+	u64			timeout_ns;	/* data timeout (in ns) */
 	unsigned int		timeout_clks;	/* data timeout (in clocks) */
 	unsigned int		blksz;		/* data block size */
 	unsigned int		blocks;		/* number of blocks */
-- 
1.7.0.4

[PATCH 2/2] 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>
Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---
changes for v2:
	- Add limit to data and erase timeout value calculation

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

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 893144e..ed5744b 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -636,6 +636,7 @@ EXPORT_SYMBOL(mmc_read_bkops_status);
  */
 void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
 {
+	struct mmc_host *host = card->host;
 	unsigned int mult;
 
 	/*
@@ -721,6 +722,10 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
 				data->timeout_ns =  100000000;	/* 100ms */
 		}
 	}
+
+	if (host->max_discard_to &&
+		host->max_discard_to < div_u64(data->timeout_ns, 1000000))
+		data->timeout_ns = (u64)host->max_discard_to * 1000000;
 }
 EXPORT_SYMBOL(mmc_set_data_timeout);
 
@@ -1880,7 +1885,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 +1903,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 2/2] Powerpc/eSDHC: Add limit to data and erase timeout value calculation

[Anton Vorontsov]

On Mon, Nov 19, 2012 at 11:38:29AM +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>
> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>

Ditto about the sign-off, it's inappropriate in this case.

[...]
> @@ -721,6 +722,10 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
>  				data->timeout_ns =  100000000;	/* 100ms */
>  		}
>  	}
> +
> +	if (host->max_discard_to &&
> +		host->max_discard_to < div_u64(data->timeout_ns, 1000000))

Still wrong indentation.

> +		data->timeout_ns = (u64)host->max_discard_to * 1000000;
>  }
>  EXPORT_SYMBOL(mmc_set_data_timeout);

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

[Zhang Haijun-B42677]

Regards
Haijun.


> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
> Sent: Monday, November 19, 2012 1:05 PM
> To: Zhang Haijun-B42677
> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093
> Subject: Re: [PATCH 2/2] Powerpc/eSDHC: Add limit to data and erase
> timeout value calculation
> 
> On Mon, Nov 19, 2012 at 11:38:29AM +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>
> > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> 
> Ditto about the sign-off, it's inappropriate in this case.


[Haijun: ] Sorry.
So, replace signed-off-by with Acked-by?

> 
> [...]
> > @@ -721,6 +722,10 @@ void mmc_set_data_timeout(struct mmc_data *data,
> const struct mmc_card *card)
> >  				data->timeout_ns =  100000000;	/* 100ms */
> >  		}
> >  	}
> > +
> > +	if (host->max_discard_to &&
> > +		host->max_discard_to < div_u64(data->timeout_ns, 1000000))
> 
> Still wrong indentation.

[Haijun: ] Did you mean here need two tabs not one ?

> 
> > +		data->timeout_ns = (u64)host->max_discard_to * 1000000;
> >  }
> >  EXPORT_SYMBOL(mmc_set_data_timeout);

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

[Anton Vorontsov]

On Mon, Nov 19, 2012 at 05:23:21AM +0000, Zhang Haijun-B42677 wrote:
> > Subject: Re: [PATCH 2/2] Powerpc/eSDHC: Add limit to data and erase
> > timeout value calculation
> > 
> > On Mon, Nov 19, 2012 at 11:38:29AM +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>
> > > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> > 
> > Ditto about the sign-off, it's inappropriate in this case.
> 
> 
> [Haijun: ] Sorry.
> So, replace signed-off-by with Acked-by?

For patches that got my Acked-by, place the Acked-by tag.
For patches that got my Reviewed-by, place the Reviewed-by tag.

> > [...]
> > > @@ -721,6 +722,10 @@ void mmc_set_data_timeout(struct mmc_data *data,
> > const struct mmc_card *card)
> > >  				data->timeout_ns =  100000000;	/* 100ms */
> > >  		}
> > >  	}
> > > +
> > > +	if (host->max_discard_to &&
> > > +		host->max_discard_to < div_u64(data->timeout_ns, 1000000))
> > 
> > Still wrong indentation.
> 
> [Haijun: ] Did you mean here need two tabs not one ?

Yes.

> > 
> > > +		data->timeout_ns = (u64)host->max_discard_to * 1000000;
> > >  }
> > >  EXPORT_SYMBOL(mmc_set_data_timeout);

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

[Zhang Haijun-B42677]

All corrected, many thanks.


Regards
Haijun.


> -----Original Message-----
> From: Anton Vorontsov [mailto:cbouatmailru@gmail.com]
> Sent: Monday, November 19, 2012 1:34 PM
> To: Zhang Haijun-B42677
> Cc: linux-mmc@vger.kernel.org; Huang Changming-R66093
> Subject: Re: [PATCH 2/2] Powerpc/eSDHC: Add limit to data and erase
> timeout value calculation
> 
> On Mon, Nov 19, 2012 at 05:23:21AM +0000, Zhang Haijun-B42677 wrote:
> > > Subject: Re: [PATCH 2/2] Powerpc/eSDHC: Add limit to data and erase
> > > timeout value calculation
> > >
> > > On Mon, Nov 19, 2012 at 11:38:29AM +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>
> > > > Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
> > >
> > > Ditto about the sign-off, it's inappropriate in this case.
> >
> >
> > [Haijun: ] Sorry.
> > So, replace signed-off-by with Acked-by?
> 
> For patches that got my Acked-by, place the Acked-by tag.
> For patches that got my Reviewed-by, place the Reviewed-by tag.
> 
> > > [...]
> > > > @@ -721,6 +722,10 @@ void mmc_set_data_timeout(struct mmc_data
> *data,
> > > const struct mmc_card *card)
> > > >  				data->timeout_ns =  100000000;	/*
> 100ms */
> > > >  		}
> > > >  	}
> > > > +
> > > > +	if (host->max_discard_to &&
> > > > +		host->max_discard_to < div_u64(data->timeout_ns,
> 1000000))
> > >
> > > Still wrong indentation.
> >
> > [Haijun: ] Did you mean here need two tabs not one ?
> 
> Yes.
> 
> > >
> > > > +		data->timeout_ns = (u64)host->max_discard_to * 1000000;
> > > >  }
> > > >  EXPORT_SYMBOL(mmc_set_data_timeout);

[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>
Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>
---
changes for v2:
	- Recompute the timeout value and max_discard_to for mmc erase

 drivers/mmc/host/sdhci-esdhc.h    |    1 -
 drivers/mmc/host/sdhci-of-esdhc.c |   14 ++++++++++++++
 2 files changed, 14 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..4592010 100644
--- a/drivers/mmc/host/sdhci-of-esdhc.c
+++ b/drivers/mmc/host/sdhci-of-esdhc.c
@@ -154,6 +154,19 @@ 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 +203,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] Powerpc eSDHC: Use u64 to calculate the timeout value to avoid overflow

[Anton Vorontsov]

On Mon, Nov 19, 2012 at 11:38:28AM +0800, Haijun Zhang wrote:
> As some mmc cards need large timeout value usually a few seconds,
> so data timeout nanosecond will overflow with u32 variable and
> give the wrong timeout value, so use u64 will be safe.
> 
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> Signed-off-by: Haijun Zhang <Haijun.Zhang@freescale.com>
> Signed-off-by: Anton Vorontsov <cbouatmailru@gmail.com>

Nope, you should not put my Signed-off-by here, I didn't sign on the
patch, neither I'm in patch's delivery path.

Preserving my exact tags (Acked-by or Reviewed-by) is important, they all
have different meanings.

You can refer to Documentation/SubmittingPatches, the document describes
each tag in detail.

Thanks,
Anton.

[PATCH 2/2 v3] 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>
Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
---
changes for v3:
	- Add limit to data and erase timeout value calculation

 drivers/mmc/core/core.c |   35 +++++++++++++++++++++--------------
 1 files changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 893144e..edb2964 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -636,6 +636,7 @@ EXPORT_SYMBOL(mmc_read_bkops_status);
  */
 void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
 {
+	struct mmc_host *host = card->host;
 	unsigned int mult;
 
 	/*
@@ -669,9 +670,9 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
 		u64 timeout_us, limit_us;
 
 		timeout_us = div_u64(data->timeout_ns, 1000);
-		if (mmc_host_clk_rate(card->host))
+		if (mmc_host_clk_rate(host))
 			timeout_us += data->timeout_clks * 1000 /
-				(mmc_host_clk_rate(card->host) / 1000);
+				(mmc_host_clk_rate(host) / 1000);
 
 		if (data->flags & MMC_DATA_WRITE)
 			/*
@@ -712,7 +713,7 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
 	 * continuous stream of data until the internal logic
 	 * overflowed.
 	 */
-	if (mmc_host_is_spi(card->host)) {
+	if (mmc_host_is_spi(host)) {
 		if (data->flags & MMC_DATA_WRITE) {
 			if (data->timeout_ns < 1000000000)
 				data->timeout_ns = 1000000000;	/* 1s */
@@ -721,6 +722,11 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
 				data->timeout_ns =  100000000;	/* 100ms */
 		}
 	}
+
+	if (host->max_discard_to &&
+			(host->max_discard_to <
+			 div_u64(data->timeout_ns, 1000000)))
+		data->timeout_ns = (u64)host->max_discard_to * 1000000;
 }
 EXPORT_SYMBOL(mmc_set_data_timeout);
 
@@ -1880,7 +1886,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 +1904,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] Powerpc eSDHC: Use u64 to calculate the timeout value to avoid overflow

[Haijun Zhang]

As some mmc cards need large timeout value usually a few seconds,
so data timeout nanosecond will overflow with u32 variable and
give the wrong timeout value, so use u64 will be safe.

Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
Signed-off-by: Haijun Zhang <Haijun.Zhang@freescale.com>
Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>
---
 drivers/mmc/core/core.c  |   21 ++++++++-------------
 drivers/mmc/host/sdhci.c |    6 +++---
 include/linux/mmc/core.h |    2 +-
 3 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 06c42cf..893144e 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -208,10 +208,10 @@ mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 
 	if (mrq->data) {
 		pr_debug("%s:     blksz %d blocks %d flags %08x "
-			"tsac %d ms nsac %d\n",
+			"tsac %lld ms nsac %d\n",
 			mmc_hostname(host), mrq->data->blksz,
 			mrq->data->blocks, mrq->data->flags,
-			mrq->data->timeout_ns / 1000000,
+			div_u64(mrq->data->timeout_ns, 1000000),
 			mrq->data->timeout_clks);
 	}
 
@@ -659,16 +659,16 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
 	if (data->flags & MMC_DATA_WRITE)
 		mult <<= card->csd.r2w_factor;
 
-	data->timeout_ns = card->csd.tacc_ns * mult;
+	data->timeout_ns = (u64)card->csd.tacc_ns * mult;
 	data->timeout_clks = card->csd.tacc_clks * mult;
 
 	/*
 	 * SD cards also have an upper limit on the timeout.
 	 */
 	if (mmc_card_sd(card)) {
-		unsigned int timeout_us, limit_us;
+		u64 timeout_us, limit_us;
 
-		timeout_us = data->timeout_ns / 1000;
+		timeout_us = div_u64(data->timeout_ns, 1000);
 		if (mmc_host_clk_rate(card->host))
 			timeout_us += data->timeout_clks * 1000 /
 				(mmc_host_clk_rate(card->host) / 1000);
@@ -1545,14 +1545,9 @@ static unsigned int mmc_mmc_erase_timeout(struct mmc_card *card,
 		/* CSD Erase Group Size uses write timeout */
 		unsigned int mult = (10 << card->csd.r2w_factor);
 		unsigned int timeout_clks = card->csd.tacc_clks * mult;
-		unsigned int timeout_us;
-
-		/* Avoid overflow: e.g. tacc_ns=80000000 mult=1280 */
-		if (card->csd.tacc_ns < 1000000)
-			timeout_us = (card->csd.tacc_ns * mult) / 1000;
-		else
-			timeout_us = (card->csd.tacc_ns / 1000) * mult;
+		u64 timeout_us;
 
+		timeout_us = (u64)(card->csd.tacc_ns / 1000) * mult;
 		/*
 		 * ios.clock is only a target.  The real clock rate might be
 		 * less but not that much less, so fudge it by multiplying by 2.
@@ -1561,7 +1556,7 @@ static unsigned int mmc_mmc_erase_timeout(struct mmc_card *card,
 		timeout_us += (timeout_clks * 1000) /
 			      (mmc_host_clk_rate(card->host) / 1000);
 
-		erase_timeout = timeout_us / 1000;
+		erase_timeout = div_u64(timeout_us, 1000);
 
 		/*
 		 * Theoretically, the calculation could underflow so round up
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f05a377..a5adc9f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -651,7 +651,7 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 {
 	u8 count;
 	struct mmc_data *data = cmd->data;
-	unsigned target_timeout, current_timeout;
+	u64 target_timeout, current_timeout;
 
 	/*
 	 * If the host controller provides us with an incorrect timeout
@@ -668,9 +668,9 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 
 	/* timeout in us */
 	if (!data)
-		target_timeout = cmd->cmd_timeout_ms * 1000;
+		target_timeout = (u64)cmd->cmd_timeout_ms * 1000;
 	else {
-		target_timeout = data->timeout_ns / 1000;
+		target_timeout = div_u64(data->timeout_ns, 1000);
 		if (host->clock)
 			target_timeout += data->timeout_clks / host->clock;
 	}
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 9b9cdaf..a2ff2e9 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -99,7 +99,7 @@ struct mmc_command {
 };
 
 struct mmc_data {
-	unsigned int		timeout_ns;	/* data timeout (in ns, max 80ms) */
+	u64			timeout_ns;	/* data timeout (in ns) */
 	unsigned int		timeout_clks;	/* data timeout (in clocks) */
 	unsigned int		blksz;		/* data block size */
 	unsigned int		blocks;		/* number of blocks */
-- 
1.7.0.4

Re: [PATCH] Powerpc eSDHC: Use u64 to calculate the timeout value to avoid overflow

[Chris Ball]

Hi,

On Mon, Nov 19 2012, Haijun Zhang wrote:
> As some mmc cards need large timeout value usually a few seconds,
> so data timeout nanosecond will overflow with u32 variable and
> give the wrong timeout value, so use u64 will be safe.
>
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> Signed-off-by: Haijun Zhang <Haijun.Zhang@freescale.com>
> Acked-by: Anton Vorontsov <cbouatmailru@gmail.com>

Thanks, pushed to mmc-next for 3.8.

- Chris.
-- 
Chris Ball   <cjb@laptop.org>   <http://printf.net/>
One Laptop Per Child

[PATCH] Powerpc eSDHC: Use u64 to calculate the timeout value to avoid overflow

[Haijun Zhang]

As some mmc cards need large timeout value usually a few seconds,
so data timeout nanosecond will overflow with u32 variable and
give the wrong timeout value, so use u64 will be safe.

Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
Signed-off-by: Haijun Zhang <Haijun.Zhang@freescale.com>
CC: Anton Vorontsov <cbouatmailru@gmail.com>
---
 drivers/mmc/core/core.c  |   21 ++++++++-------------
 drivers/mmc/host/sdhci.c |    6 +++---
 include/linux/mmc/core.h |    2 +-
 3 files changed, 12 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
index 06c42cf..c241fc1 100644
--- a/drivers/mmc/core/core.c
+++ b/drivers/mmc/core/core.c
@@ -208,10 +208,10 @@ mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
 
 	if (mrq->data) {
 		pr_debug("%s:     blksz %d blocks %d flags %08x "
-			"tsac %d ms nsac %d\n",
+			"tsac %lld ms nsac %d\n",
 			mmc_hostname(host), mrq->data->blksz,
 			mrq->data->blocks, mrq->data->flags,
-			mrq->data->timeout_ns / 1000000,
+			div_u64(mrq->data->timeout_ns, 1000000),
 			mrq->data->timeout_clks);
 	}
 
@@ -659,16 +659,16 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
 	if (data->flags & MMC_DATA_WRITE)
 		mult <<= card->csd.r2w_factor;
 
-	data->timeout_ns = card->csd.tacc_ns * mult;
+	data->timeout_ns = (u64)card->csd.tacc_ns * (u64)mult;
 	data->timeout_clks = card->csd.tacc_clks * mult;
 
 	/*
 	 * SD cards also have an upper limit on the timeout.
 	 */
 	if (mmc_card_sd(card)) {
-		unsigned int timeout_us, limit_us;
+		u64 timeout_us, limit_us;
 
-		timeout_us = data->timeout_ns / 1000;
+		timeout_us = div_u64(data->timeout_ns, 1000);
 		if (mmc_host_clk_rate(card->host))
 			timeout_us += data->timeout_clks * 1000 /
 				(mmc_host_clk_rate(card->host) / 1000);
@@ -1545,14 +1545,9 @@ static unsigned int mmc_mmc_erase_timeout(struct mmc_card *card,
 		/* CSD Erase Group Size uses write timeout */
 		unsigned int mult = (10 << card->csd.r2w_factor);
 		unsigned int timeout_clks = card->csd.tacc_clks * mult;
-		unsigned int timeout_us;
-
-		/* Avoid overflow: e.g. tacc_ns=80000000 mult=1280 */
-		if (card->csd.tacc_ns < 1000000)
-			timeout_us = (card->csd.tacc_ns * mult) / 1000;
-		else
-			timeout_us = (card->csd.tacc_ns / 1000) * mult;
+		u64 timeout_us;
 
+		timeout_us = (u64)(card->csd.tacc_ns / 1000) * (u64)mult;
 		/*
 		 * ios.clock is only a target.  The real clock rate might be
 		 * less but not that much less, so fudge it by multiplying by 2.
@@ -1561,7 +1556,7 @@ static unsigned int mmc_mmc_erase_timeout(struct mmc_card *card,
 		timeout_us += (timeout_clks * 1000) /
 			      (mmc_host_clk_rate(card->host) / 1000);
 
-		erase_timeout = timeout_us / 1000;
+		erase_timeout = div_u64(timeout_us, 1000);
 
 		/*
 		 * Theoretically, the calculation could underflow so round up
diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index f05a377..a5adc9f 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -651,7 +651,7 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 {
 	u8 count;
 	struct mmc_data *data = cmd->data;
-	unsigned target_timeout, current_timeout;
+	u64 target_timeout, current_timeout;
 
 	/*
 	 * If the host controller provides us with an incorrect timeout
@@ -668,9 +668,9 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 
 	/* timeout in us */
 	if (!data)
-		target_timeout = cmd->cmd_timeout_ms * 1000;
+		target_timeout = (u64)cmd->cmd_timeout_ms * 1000;
 	else {
-		target_timeout = data->timeout_ns / 1000;
+		target_timeout = div_u64(data->timeout_ns, 1000);
 		if (host->clock)
 			target_timeout += data->timeout_clks / host->clock;
 	}
diff --git a/include/linux/mmc/core.h b/include/linux/mmc/core.h
index 9b9cdaf..a2ff2e9 100644
--- a/include/linux/mmc/core.h
+++ b/include/linux/mmc/core.h
@@ -99,7 +99,7 @@ struct mmc_command {
 };
 
 struct mmc_data {
-	unsigned int		timeout_ns;	/* data timeout (in ns, max 80ms) */
+	u64			timeout_ns;	/* data timeout (in ns) */
 	unsigned int		timeout_clks;	/* data timeout (in clocks) */
 	unsigned int		blksz;		/* data block size */
 	unsigned int		blocks;		/* number of blocks */
-- 
1.7.0.4

Re: [PATCH] Powerpc eSDHC: Use u64 to calculate the timeout value to avoid overflow

[Anton Vorontsov]

On Mon, Nov 12, 2012 at 12:45:28PM +0800, Haijun Zhang wrote:
> As some mmc cards need large timeout value usually a few seconds,
> so data timeout nanosecond will overflow with u32 variable and
> give the wrong timeout value, so use u64 will be safe.
> 
> Signed-off-by: Jerry Huang <Chang-Ming.Huang@freescale.com>
> Signed-off-by: Haijun Zhang <Haijun.Zhang@freescale.com>
> CC: Anton Vorontsov <cbouatmailru@gmail.com>
> ---
>  drivers/mmc/core/core.c  |   21 ++++++++-------------
>  drivers/mmc/host/sdhci.c |    6 +++---
>  include/linux/mmc/core.h |    2 +-
>  3 files changed, 12 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> index 06c42cf..c241fc1 100644
> --- a/drivers/mmc/core/core.c
> +++ b/drivers/mmc/core/core.c
> @@ -208,10 +208,10 @@ mmc_start_request(struct mmc_host *host, struct mmc_request *mrq)
>  
>  	if (mrq->data) {
>  		pr_debug("%s:     blksz %d blocks %d flags %08x "
> -			"tsac %d ms nsac %d\n",
> +			"tsac %lld ms nsac %d\n",
>  			mmc_hostname(host), mrq->data->blksz,
>  			mrq->data->blocks, mrq->data->flags,
> -			mrq->data->timeout_ns / 1000000,
> +			div_u64(mrq->data->timeout_ns, 1000000),
>  			mrq->data->timeout_clks);
>  	}
>  
> @@ -659,16 +659,16 @@ void mmc_set_data_timeout(struct mmc_data *data, const struct mmc_card *card)
>  	if (data->flags & MMC_DATA_WRITE)
>  		mult <<= card->csd.r2w_factor;
>  
> -	data->timeout_ns = card->csd.tacc_ns * mult;
> +	data->timeout_ns = (u64)card->csd.tacc_ns * (u64)mult;

one cast would be enough.

>  	data->timeout_clks = card->csd.tacc_clks * mult;
>  
>  	/*
>  	 * SD cards also have an upper limit on the timeout.
>  	 */
>  	if (mmc_card_sd(card)) {
> -		unsigned int timeout_us, limit_us;
> +		u64 timeout_us, limit_us;
>  
> -		timeout_us = data->timeout_ns / 1000;
> +		timeout_us = div_u64(data->timeout_ns, 1000);
>  		if (mmc_host_clk_rate(card->host))
>  			timeout_us += data->timeout_clks * 1000 /
>  				(mmc_host_clk_rate(card->host) / 1000);
> @@ -1545,14 +1545,9 @@ static unsigned int mmc_mmc_erase_timeout(struct mmc_card *card,
>  		/* CSD Erase Group Size uses write timeout */
>  		unsigned int mult = (10 << card->csd.r2w_factor);
>  		unsigned int timeout_clks = card->csd.tacc_clks * mult;
> -		unsigned int timeout_us;
> -
> -		/* Avoid overflow: e.g. tacc_ns=80000000 mult=1280 */
> -		if (card->csd.tacc_ns < 1000000)
> -			timeout_us = (card->csd.tacc_ns * mult) / 1000;
> -		else
> -			timeout_us = (card->csd.tacc_ns / 1000) * mult;
> +		u64 timeout_us;
>  
> +		timeout_us = (u64)(card->csd.tacc_ns / 1000) * (u64)mult;

Ditto.

Otherwise it looks good.

Thanks,
Anton.