[PATCH V3 0/6] mmc: sdhci: a few fixes on timeout and max_discard_to

[PATCH V3 0/6] mmc: sdhci: a few fixes on timeout and max_discard_to

[Dong Aisheng]

This series mainly is a rebase version due to it's somehow missed.
See the original one at here:
http://marc.info/?l=linux-mmc&m=138726995528304&w=2
The issue is still exist on latest kernel.

Comparing to V2, the old patch 5 is removed due to someone already pushed one
simalar patch in the tree.
Patch 5 and 6 in this series is newly added for a few clean up and make
timeout_clk calculate more common for other platforms.

Details patch description is as follows:
Patch 1~4 mainly fixes the issue that the max timeout counter for uSDHC is
1 << 28 rather than 1 << 27. 1~2 fix getting the max timeout counter
while 3~4 fix setting the max timeout.
Thus it introduces two new platform hook: get_max_timeout_count and set_timeout
for those platform which have different timeout setting.

This issue is firstly reported here by Ed Sutter:
http://www.spinics.net/lists/linux-mmc/msg23375.html
The root cause is the max_discard_to got from uSDHC is too small, only 677ms,
which cause the max_discard_bytes for eMMC is only 512, then the discard operation
of mkfs.ext3 for an eMMC card is too slow, just like dead.
With above patches, the issue can be fixed.

Originally the max_discard_to for a high speed sdhc card may be:
mmc1: new high speed SDHC card at address aaaa
mmc1: calculated max. discard sectors 49152 for timeout 1355 ms
After fix:
mmc1: new high speed SDHC card at address aaaa
mmc1: calculated max. discard sectors 712704 for timeout 5422 ms

It also improves the card discard performance a lot due to max_discard_sectors
increase a lot.

ChangeLog:
v2->v3:
 1. rebase to Ulf's latest mmc tree
 2. make timeout_clk calcuation common for SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK

ChangeLog:
v1->v2:
 1. change .get_max_timeout to .get_max_timeout_count to reuse some code
 2. some other minor changes based on Shawn's comment.
 3. patch 6 in v1 is dropped since not need anymore after change 1

Dong Aisheng (6):
  mmc: sdhci: add platfrom get_max_timeout_count hook
  mmc: sdhci-esdhc-imx: fix incorrect max timeout cout for uSDHC
  mmc: sdhci: add platform set_timeout hook
  mmc: sdhci-esdhc-imx: set the correct max timeout value for uSDHC
  mmc: sdhci: calculate timeout_clk conditionally in sdhci_add_host
  mmc: sdhci: move timeout_clk dynamically calculation code into common
    code

 drivers/mmc/host/sdhci-esdhc-imx.c |   20 ++++++++++
 drivers/mmc/host/sdhci.c           |   74 ++++++++++++++++++++++--------------
 drivers/mmc/host/sdhci.h           |    3 +
 3 files changed, 68 insertions(+), 29 deletions(-)

-- 
1.7.8

[PATCH V3 1/6] mmc: sdhci: add platfrom get_max_timeout_count hook

[Dong Aisheng]

Currently the max timeout count is hardcode to 1 << 27 for calcuate
the max_busy_timeout, however, for some platforms the max timeout
count may not be 1 << 27, e.g. i.MX uSDHC is 1 << 28.
Thus 1 << 27 is not correct for such platform.

It is also possible that other platforms may have different values.
To be flexible, we add a get_max_timeout_count hook to get the correct
maximum timeout value for these platforms.

Reviewed-by: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
 drivers/mmc/host/sdhci.c |   11 ++++++++---
 drivers/mmc/host/sdhci.h |    1 +
 2 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 4cc2ff9..d633f288 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1196,8 +1196,11 @@ clock_set:
 		host->mmc->actual_clock = (host->max_clk * clk_mul) / real_div;
 		if (host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK) {
 			host->timeout_clk = host->mmc->actual_clock / 1000;
-			host->mmc->max_busy_timeout =
-				(1 << 27) / host->timeout_clk;
+			mmc->max_busy_timeout =
+					host->ops->get_max_timeout_count ?
+					host->ops->get_max_timeout_count(host) :
+					1 << 27;
+			mmc->max_busy_timeout /= host->timeout_clk;
 		}
 	}
 
@@ -2937,7 +2940,9 @@ int sdhci_add_host(struct sdhci_host *host)
 	if (host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK)
 		host->timeout_clk = mmc->f_max / 1000;
 
-	mmc->max_busy_timeout = (1 << 27) / host->timeout_clk;
+	mmc->max_busy_timeout = host->ops->get_max_timeout_count ?
+			host->ops->get_max_timeout_count(host) : 1 << 27;
+	mmc->max_busy_timeout /= host->timeout_clk;
 
 	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
 	mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 4a5cd5e..b130509 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -281,6 +281,7 @@ struct sdhci_ops {
 	unsigned int	(*get_max_clock)(struct sdhci_host *host);
 	unsigned int	(*get_min_clock)(struct sdhci_host *host);
 	unsigned int	(*get_timeout_clock)(struct sdhci_host *host);
+	unsigned int	(*get_max_timeout_count)(struct sdhci_host *host);
 	void		(*set_bus_width)(struct sdhci_host *host, int width);
 	void (*platform_send_init_74_clocks)(struct sdhci_host *host,
 					     u8 power_mode);
-- 
1.7.8

[PATCH V3 2/6] mmc: sdhci-esdhc-imx: fix incorrect max timeout cout for uSDHC

[Dong Aisheng]

The default sdhci code use the 1 << 27 as the max timeout counter to
to calculate the max_busy_timeout, however it's not correct for uSDHC
since its the max counter is 1 << 28.
Implement esdhc_get_max_timeout_cout to handle it correctly.

Reviewed-by: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 46677f0..546ed02 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -880,6 +880,14 @@ static void esdhc_reset(struct sdhci_host *host, u8 mask)
 	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
 }
 
+static unsigned int esdhc_get_max_timeout_count(struct sdhci_host *host)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data = pltfm_host->priv;
+
+	return esdhc_is_usdhc(imx_data) ? 1 << 28 : 1 << 27;
+}
+
 static struct sdhci_ops sdhci_esdhc_ops = {
 	.read_l = esdhc_readl_le,
 	.read_w = esdhc_readw_le,
@@ -889,6 +897,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {
 	.set_clock = esdhc_pltfm_set_clock,
 	.get_max_clock = esdhc_pltfm_get_max_clock,
 	.get_min_clock = esdhc_pltfm_get_min_clock,
+	.get_max_timeout_count = esdhc_get_max_timeout_count,
 	.get_ro = esdhc_pltfm_get_ro,
 	.set_bus_width = esdhc_pltfm_set_bus_width,
 	.set_uhs_signaling = esdhc_set_uhs_signaling,
-- 
1.7.8

[PATCH V3 3/6] mmc: sdhci: add platform set_timeout hook

[Dong Aisheng]

Currently the common code assume 0xE is the maximum timeout counter
value and use it to write into the timeout counter register.
However, it's fairly possible that some other SoCs may have different
max timeout register value. That means 0xE may be incorrect and
becomes meaningless.

It's also possible that other platforms has different timeout
calculation algorithm. To be flexible, this patch provides a .set_timeout
hook for those platforms to set the timeout on their way if they need.

Reviewed-by: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
 drivers/mmc/host/sdhci.c |   19 ++++++++++++++-----
 drivers/mmc/host/sdhci.h |    2 ++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index d633f288..881a7a0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -707,19 +707,28 @@ static void sdhci_set_transfer_irqs(struct sdhci_host *host)
 	sdhci_writel(host, host->ier, SDHCI_SIGNAL_ENABLE);
 }
 
-static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
+static void sdhci_set_timeout(struct sdhci_host *host, struct mmc_command *cmd)
 {
 	u8 count;
+
+	if (host->ops->set_timeout) {
+		host->ops->set_timeout(host, cmd);
+	} else {
+		count = sdhci_calc_timeout(host, cmd);
+		sdhci_writeb(host, count, SDHCI_TIMEOUT_CONTROL);
+	}
+}
+
+static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_command *cmd)
+{
 	u8 ctrl;
 	struct mmc_data *data = cmd->data;
 	int ret;
 
 	WARN_ON(host->data);
 
-	if (data || (cmd->flags & MMC_RSP_BUSY)) {
-		count = sdhci_calc_timeout(host, cmd);
-		sdhci_writeb(host, count, SDHCI_TIMEOUT_CONTROL);
-	}
+	if (data || (cmd->flags & MMC_RSP_BUSY))
+		sdhci_set_timeout(host, cmd);
 
 	if (!data)
 		return;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index b130509..9828ff8 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -282,6 +282,8 @@ struct sdhci_ops {
 	unsigned int	(*get_min_clock)(struct sdhci_host *host);
 	unsigned int	(*get_timeout_clock)(struct sdhci_host *host);
 	unsigned int	(*get_max_timeout_count)(struct sdhci_host *host);
+	void		(*set_timeout)(struct sdhci_host *host,
+				       struct mmc_command *cmd);
 	void		(*set_bus_width)(struct sdhci_host *host, int width);
 	void (*platform_send_init_74_clocks)(struct sdhci_host *host,
 					     u8 power_mode);
-- 
1.7.8

[PATCH V3 4/6] mmc: sdhci-esdhc-imx: set the correct max timeout value for uSDHC

[Dong Aisheng]

The default sdhci driver write 0xE into timeout counter register to
set the maximum timeout. The value is not correct for uSDHC since the
max counter value for uSDHC is 0xF.
Instead of using common timeout code in sdhci, we implement esdhc_set_timeout
to handle the difference between eSDHC and uSDHC.

Currently we simply set the max timeout value as before.
But in the future, we probably may implement IMX specific timeout
setting algorithm and use suitable timeout for different CMDs.

Reviewed-by: Shawn Guo <shawn.guo@linaro.org>
Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
 drivers/mmc/host/sdhci-esdhc-imx.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci-esdhc-imx.c b/drivers/mmc/host/sdhci-esdhc-imx.c
index 546ed02..587ee0e 100644
--- a/drivers/mmc/host/sdhci-esdhc-imx.c
+++ b/drivers/mmc/host/sdhci-esdhc-imx.c
@@ -888,6 +888,16 @@ static unsigned int esdhc_get_max_timeout_count(struct sdhci_host *host)
 	return esdhc_is_usdhc(imx_data) ? 1 << 28 : 1 << 27;
 }
 
+static void esdhc_set_timeout(struct sdhci_host *host, struct mmc_command *cmd)
+{
+	struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+	struct pltfm_imx_data *imx_data = pltfm_host->priv;
+
+	/* use maximum timeout counter */
+	sdhci_writeb(host, esdhc_is_usdhc(imx_data) ? 0xF : 0xE,
+			SDHCI_TIMEOUT_CONTROL);
+}
+
 static struct sdhci_ops sdhci_esdhc_ops = {
 	.read_l = esdhc_readl_le,
 	.read_w = esdhc_readw_le,
@@ -899,6 +909,7 @@ static struct sdhci_ops sdhci_esdhc_ops = {
 	.get_min_clock = esdhc_pltfm_get_min_clock,
 	.get_max_timeout_count = esdhc_get_max_timeout_count,
 	.get_ro = esdhc_pltfm_get_ro,
+	.set_timeout = esdhc_set_timeout,
 	.set_bus_width = esdhc_pltfm_set_bus_width,
 	.set_uhs_signaling = esdhc_set_uhs_signaling,
 	.reset = esdhc_reset,
-- 
1.7.8

[PATCH V3 5/6] mmc: sdhci: calculate timeout_clk conditionally in sdhci_add_host

[Dong Aisheng]

The timeout_clk calculation code in sdhci_add_host is meaningless for
SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK.
So only execute them with no SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK set.

Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
 drivers/mmc/host/sdhci.c |   34 +++++++++++++++++-----------------
 1 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 881a7a0..0253eb1 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -2931,27 +2931,27 @@ int sdhci_add_host(struct sdhci_host *host)
 	} else
 		mmc->f_min = host->max_clk / SDHCI_MAX_DIV_SPEC_200;
 
-	host->timeout_clk =
-		(caps[0] & SDHCI_TIMEOUT_CLK_MASK) >> SDHCI_TIMEOUT_CLK_SHIFT;
-	if (host->timeout_clk == 0) {
-		if (host->ops->get_timeout_clock) {
-			host->timeout_clk = host->ops->get_timeout_clock(host);
-		} else if (!(host->quirks &
-				SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK)) {
-			pr_err("%s: Hardware doesn't specify timeout clock "
-			       "frequency.\n", mmc_hostname(mmc));
-			return -ENODEV;
+	if (!(host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK)) {
+		host->timeout_clk = (caps[0] & SDHCI_TIMEOUT_CLK_MASK) >>
+					SDHCI_TIMEOUT_CLK_SHIFT;
+		if (host->timeout_clk == 0) {
+			if (host->ops->get_timeout_clock) {
+				host->timeout_clk =
+					host->ops->get_timeout_clock(host);
+			} else {
+				pr_err("%s: Hardware doesn't specify timeout clock frequency.\n",
+					mmc_hostname(mmc));
+				return -ENODEV;
+			}
 		}
-	}
-	if (caps[0] & SDHCI_TIMEOUT_CLK_UNIT)
-		host->timeout_clk *= 1000;
 
-	if (host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK)
-		host->timeout_clk = mmc->f_max / 1000;
+		if (caps[0] & SDHCI_TIMEOUT_CLK_UNIT)
+			host->timeout_clk *= 1000;
 
-	mmc->max_busy_timeout = host->ops->get_max_timeout_count ?
+		mmc->max_busy_timeout = host->ops->get_max_timeout_count ?
 			host->ops->get_max_timeout_count(host) : 1 << 27;
-	mmc->max_busy_timeout /= host->timeout_clk;
+		mmc->max_busy_timeout /= host->timeout_clk;
+	}
 
 	mmc->caps |= MMC_CAP_SDIO_IRQ | MMC_CAP_ERASE | MMC_CAP_CMD23;
 	mmc->caps2 |= MMC_CAP2_SDIO_IRQ_NOTHREAD;
-- 
1.7.8

[PATCH V3 6/6] mmc: sdhci: move timeout_clk dynamically calculation code into common code

[Dong Aisheng]

The timeout_clk calculation code for SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK case
is common and could be moved into common sdhci_do_set_ios, then platform code
which is not using sdhci_set_clock does not need to write the same code again.

Signed-off-by: Dong Aisheng <b29396@freescale.com>
---
 drivers/mmc/host/sdhci.c |   24 +++++++++++++-----------
 1 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0253eb1..f6a683b 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1201,18 +1201,8 @@ void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
 	}
 
 clock_set:
-	if (real_div) {
+	if (real_div)
 		host->mmc->actual_clock = (host->max_clk * clk_mul) / real_div;
-		if (host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK) {
-			host->timeout_clk = host->mmc->actual_clock / 1000;
-			mmc->max_busy_timeout =
-					host->ops->get_max_timeout_count ?
-					host->ops->get_max_timeout_count(host) :
-					1 << 27;
-			mmc->max_busy_timeout /= host->timeout_clk;
-		}
-	}
-
 	clk |= (div & SDHCI_DIV_MASK) << SDHCI_DIVIDER_SHIFT;
 	clk |= ((div & SDHCI_DIV_HI_MASK) >> SDHCI_DIV_MASK_LEN)
 		<< SDHCI_DIVIDER_HI_SHIFT;
@@ -1489,6 +1479,18 @@ static void sdhci_do_set_ios(struct sdhci_host *host, struct mmc_ios *ios)
 	if (!ios->clock || ios->clock != host->clock) {
 		host->ops->set_clock(host, ios->clock);
 		host->clock = ios->clock;
+
+		if (host->quirks & SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK &&
+		    host->clock) {
+			host->timeout_clk = host->mmc->actual_clock ?
+						host->mmc->actual_clock / 1000 :
+						host->clock / 1000;
+			host->mmc->max_busy_timeout =
+				host->ops->get_max_timeout_count ?
+				host->ops->get_max_timeout_count(host) :
+				1 << 27;
+			host->mmc->max_busy_timeout /= host->timeout_clk;
+		}
 	}
 
 	sdhci_set_power(host, ios->power_mode, ios->vdd);
-- 
1.7.8

Re: [PATCH V3 0/6] mmc: sdhci: a few fixes on timeout and max_discard_to

[Ulf Hansson]

On 27 August 2014 09:26, Dong Aisheng <b29396@freescale.com> wrote:
> This series mainly is a rebase version due to it's somehow missed.
> See the original one at here:
> http://marc.info/?l=linux-mmc&m=138726995528304&w=2
> The issue is still exist on latest kernel.
>
> Comparing to V2, the old patch 5 is removed due to someone already pushed one
> simalar patch in the tree.
> Patch 5 and 6 in this series is newly added for a few clean up and make
> timeout_clk calculate more common for other platforms.
>
> Details patch description is as follows:
> Patch 1~4 mainly fixes the issue that the max timeout counter for uSDHC is
> 1 << 28 rather than 1 << 27. 1~2 fix getting the max timeout counter
> while 3~4 fix setting the max timeout.
> Thus it introduces two new platform hook: get_max_timeout_count and set_timeout
> for those platform which have different timeout setting.
>
> This issue is firstly reported here by Ed Sutter:
> http://www.spinics.net/lists/linux-mmc/msg23375.html
> The root cause is the max_discard_to got from uSDHC is too small, only 677ms,
> which cause the max_discard_bytes for eMMC is only 512, then the discard operation
> of mkfs.ext3 for an eMMC card is too slow, just like dead.
> With above patches, the issue can be fixed.
>
> Originally the max_discard_to for a high speed sdhc card may be:
> mmc1: new high speed SDHC card at address aaaa
> mmc1: calculated max. discard sectors 49152 for timeout 1355 ms
> After fix:
> mmc1: new high speed SDHC card at address aaaa
> mmc1: calculated max. discard sectors 712704 for timeout 5422 ms
>
> It also improves the card discard performance a lot due to max_discard_sectors
> increase a lot.
>
> ChangeLog:
> v2->v3:
>  1. rebase to Ulf's latest mmc tree
>  2. make timeout_clk calcuation common for SDHCI_QUIRK_DATA_TIMEOUT_USES_SDCLK
>
> ChangeLog:
> v1->v2:
>  1. change .get_max_timeout to .get_max_timeout_count to reuse some code
>  2. some other minor changes based on Shawn's comment.
>  3. patch 6 in v1 is dropped since not need anymore after change 1
>
> Dong Aisheng (6):
>   mmc: sdhci: add platfrom get_max_timeout_count hook
>   mmc: sdhci-esdhc-imx: fix incorrect max timeout cout for uSDHC
>   mmc: sdhci: add platform set_timeout hook
>   mmc: sdhci-esdhc-imx: set the correct max timeout value for uSDHC
>   mmc: sdhci: calculate timeout_clk conditionally in sdhci_add_host
>   mmc: sdhci: move timeout_clk dynamically calculation code into common
>     code
>
>  drivers/mmc/host/sdhci-esdhc-imx.c |   20 ++++++++++
>  drivers/mmc/host/sdhci.c           |   74 ++++++++++++++++++++++--------------
>  drivers/mmc/host/sdhci.h           |    3 +
>  3 files changed, 68 insertions(+), 29 deletions(-)
>
> --
> 1.7.8
>

Thanks! I have applied them all for next.

Kind regards
Uffe