[PATCH 0/6] mmc: sdhci-s3c: Fix base clock source management

[PATCH 0/6] mmc: sdhci-s3c: Fix base clock source management

[Tomasz Figa]

On platforms prior to Exynos the SDHCI block used internal clock
divider controlled by SELFREQ field of CLKCON register to divide base
clock selected from several external clocks fed to the block by
SELBASECLK bitfield of CONTROL2 register. Depending on wanted clock
frequency, different external clock may be the best choice and so
the driver needs to switch the SELBASECLK mux on the fly.

However the selection logic has been broken for quite some time leaving
the controller using always clock 0, which is not always the right
source and leading to suboptimal performance of the SDHCI block on
affected platforms.

This series intends to fix the problems mentioned above and also clean-up
clock management code slightly.

Tested on S3C6410-based Mini6410 board, with following performance
figures:

* Before this series (133 MHz HCLK always selected, leading to at most
  33 MHz card clock):

root at tiny6410:~# hdparm -t /dev/mmcblk0

/dev/mmcblk0:
 Timing buffered disk reads:  42 MB in  3.10 seconds =  13.54 MB/sec
root at tiny6410:~# hdparm -t /dev/mmcblk0

/dev/mmcblk0:
 Timing buffered disk reads:  44 MB in  3.11 seconds =  14.14 MB/sec
root at tiny6410:~#

* After this series (48 MHz EPLL clock selected, leading to 48 MHz card
  clock):

root at tiny6410:~# hdparm -t /dev/mmcblk0 

/dev/mmcblk0:
 Timing buffered disk reads:  56 MB in  3.04 seconds =  18.41 MB/sec
root at tiny6410:~# hdparm -t /dev/mmcblk0

/dev/mmcblk0:
 Timing buffered disk reads:  60 MB in  3.06 seconds =  19.63 MB/sec
root at tiny6410:~#

Tomasz Figa (6):
  mmc: sdhci-s3c: Use shifts to divide by powers of two
  mmc: sdhci-s3c: Cache bus clock rates
  mmc: sdhci-s3c: Use correct condition to check for clock presence
  mmc: sdhci-s3c: Simplify min/max clock calculation
  mmc: sdhci-s3c: Fix handling of bus clock switching
  mmc: sdhci-s3c: Do not allow frequencies higher than requested

 drivers/mmc/host/sdhci-s3c.c | 170 ++++++++++++++++++++-----------------------
 1 file changed, 77 insertions(+), 93 deletions(-)

-- 
1.8.5.2

[PATCH 1/6] mmc: sdhci-s3c: Use shifts to divide by powers of two

[Tomasz Figa]

Current implementation of sdhci_s3c_consider_clock() is highly
inefficient due to multiple integer divisions by variable performed in a
loop. Since only divisors that are powers of two are considered, this
patch replaces them with respective shifts, removing all the integer
divisions.

Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>
---
 drivers/mmc/host/sdhci-s3c.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 6debda9..52770d5 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -144,7 +144,7 @@ static unsigned int sdhci_s3c_consider_clock(struct sdhci_s3c *ourhost,
 {
 	unsigned long rate;
 	struct clk *clksrc = ourhost->clk_bus[src];
-	int div;
+	int shift;
 
 	if (!clksrc)
 		return UINT_MAX;
@@ -160,15 +160,15 @@ static unsigned int sdhci_s3c_consider_clock(struct sdhci_s3c *ourhost,
 
 	rate = clk_get_rate(clksrc);
 
-	for (div = 1; div < 256; div *= 2) {
-		if ((rate / div) <= wanted)
+	for (shift = 0; shift < 8; ++shift) {
+		if ((rate >> shift) <= wanted)
 			break;
 	}
 
 	dev_dbg(&ourhost->pdev->dev, "clk %d: rate %ld, want %d, got %ld\n",
-		src, rate, wanted, rate / div);
+		src, rate, wanted, rate >> shift);
 
-	return wanted - (rate / div);
+	return wanted - (rate >> shift);
 }
 
 /**
-- 
1.8.5.2

[PATCH 2/6] mmc: sdhci-s3c: Cache bus clock rates

[Tomasz Figa]

To fix scheduling while atomic happening in sdhci_s3c_set_clock() caused
by calling clk_get_rate() that might sleep, this patch modifies the
driver to cache rates of all bus clocks at probe time and then only use
those cache values.

Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>
---
 drivers/mmc/host/sdhci-s3c.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 52770d5..9b78391 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -57,6 +57,7 @@ struct sdhci_s3c {
 
 	struct clk		*clk_io;
 	struct clk		*clk_bus[MAX_BUS_CLK];
+	unsigned long		clk_rates[MAX_BUS_CLK];
 };
 
 /**
@@ -158,7 +159,7 @@ static unsigned int sdhci_s3c_consider_clock(struct sdhci_s3c *ourhost,
 		return wanted - rate;
 	}
 
-	rate = clk_get_rate(clksrc);
+	rate = ourhost->clk_rates[src];
 
 	for (shift = 0; shift < 8; ++shift) {
 		if ((rate >> shift) <= wanted)
@@ -215,7 +216,7 @@ static void sdhci_s3c_set_clock(struct sdhci_host *host, unsigned int clock)
 		writew(0, host->ioaddr + SDHCI_CLOCK_CONTROL);
 
 		ourhost->cur_clk = best_src;
-		host->max_clk = clk_get_rate(clk);
+		host->max_clk = ourhost->clk_rates[best_src];
 
 		ctrl = readl(host->ioaddr + S3C_SDHCI_CONTROL2);
 		ctrl &= ~S3C_SDHCI_CTRL2_SELBASECLK_MASK;
@@ -583,8 +584,10 @@ static int sdhci_s3c_probe(struct platform_device *pdev)
 		 */
 		sc->cur_clk = ptr;
 
+		sc->clk_rates[ptr] = clk_get_rate(sc->clk_bus[ptr]);
+
 		dev_info(dev, "clock source %d: %s (%ld Hz)\n",
-			 ptr, name, clk_get_rate(clk));
+				ptr, name, sc->clk_rates[ptr]);
 	}
 
 	if (clks == 0) {
-- 
1.8.5.2

[PATCH 3/6] mmc: sdhci-s3c: Use correct condition to check for clock presence

[Tomasz Figa]

IS_ERR() must be used to make sure that not a valid clock was returned
by clk_get() and company.

Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>
---
 drivers/mmc/host/sdhci-s3c.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 9b78391..7fde938 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -147,7 +147,7 @@ static unsigned int sdhci_s3c_consider_clock(struct sdhci_s3c *ourhost,
 	struct clk *clksrc = ourhost->clk_bus[src];
 	int shift;
 
-	if (!clksrc)
+	if (IS_ERR(clksrc))
 		return UINT_MAX;
 
 	/*
@@ -567,16 +567,14 @@ static int sdhci_s3c_probe(struct platform_device *pdev)
 	clk_prepare_enable(sc->clk_io);
 
 	for (clks = 0, ptr = 0; ptr < MAX_BUS_CLK; ptr++) {
-		struct clk *clk;
 		char name[14];
 
 		snprintf(name, 14, "mmc_busclk.%d", ptr);
-		clk = devm_clk_get(dev, name);
-		if (IS_ERR(clk))
+		sc->clk_bus[ptr] = devm_clk_get(dev, name);
+		if (IS_ERR(sc->clk_bus[ptr]))
 			continue;
 
 		clks++;
-		sc->clk_bus[ptr] = clk;
 
 		/*
 		 * save current clock index to know which clock bus
-- 
1.8.5.2

[PATCH 4/6] mmc: sdhci-s3c: Simplify min/max clock calculation

[Tomasz Figa]

This patch reimplements functions calculating minimum and maximum clock
rates to leverage clock rate cache introduced by previous patches.

In addition, the calculation is simplified to just comparing input
clock rates (max case) or input clock rates divided by maximum divisor
(min case), which is basically what the original code did, but with much
more unnecessary work.

Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>
---
 drivers/mmc/host/sdhci-s3c.c | 60 +++++++++++++++++++++++++++++---------------
 1 file changed, 40 insertions(+), 20 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 7fde938..7e14db0 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -112,20 +112,16 @@ static void sdhci_s3c_check_sclk(struct sdhci_host *host)
 static unsigned int sdhci_s3c_get_max_clk(struct sdhci_host *host)
 {
 	struct sdhci_s3c *ourhost = to_s3c(host);
-	struct clk *busclk;
-	unsigned int rate, max;
-	int clk;
+	unsigned long rate, max = 0;
+	int src;
 
 	/* note, a reset will reset the clock source */
 
 	sdhci_s3c_check_sclk(host);
 
-	for (max = 0, clk = 0; clk < MAX_BUS_CLK; clk++) {
-		busclk = ourhost->clk_bus[clk];
-		if (!busclk)
-			continue;
 
-		rate = clk_get_rate(busclk);
+	for (src = 0; src < MAX_BUS_CLK; src++) {
+		rate = ourhost->clk_rates[src];
 		if (rate > max)
 			max = rate;
 	}
@@ -255,17 +251,17 @@ static void sdhci_s3c_set_clock(struct sdhci_host *host, unsigned int clock)
 static unsigned int sdhci_s3c_get_min_clock(struct sdhci_host *host)
 {
 	struct sdhci_s3c *ourhost = to_s3c(host);
-	unsigned int delta, min = UINT_MAX;
+	unsigned long rate, min = ULONG_MAX;
 	int src;
 
 	for (src = 0; src < MAX_BUS_CLK; src++) {
-		delta = sdhci_s3c_consider_clock(ourhost, src, 0);
-		if (delta == UINT_MAX)
+		rate = ourhost->clk_rates[src] / 256;
+		if (!rate)
 			continue;
-		/* delta is a negative value in this case */
-		if (-delta < min)
-			min = -delta;
+		if (rate < min)
+			min = rate;
 	}
+
 	return min;
 }
 
@@ -273,20 +269,44 @@ static unsigned int sdhci_s3c_get_min_clock(struct sdhci_host *host)
 static unsigned int sdhci_cmu_get_max_clock(struct sdhci_host *host)
 {
 	struct sdhci_s3c *ourhost = to_s3c(host);
+	unsigned long rate, max = 0;
+	int src;
+
+	for (src = 0; src < MAX_BUS_CLK; src++) {
+		struct clk *clk;
+
+		clk = ourhost->clk_bus[src];
+		if (IS_ERR(clk))
+			continue;
+
+		rate = clk_round_rate(clk, ULONG_MAX);
+		if (rate > max)
+			max = rate;
+	}
 
-	return clk_round_rate(ourhost->clk_bus[ourhost->cur_clk], UINT_MAX);
+	return max;
 }
 
 /* sdhci_cmu_get_min_clock - callback to get minimal supported clock value. */
 static unsigned int sdhci_cmu_get_min_clock(struct sdhci_host *host)
 {
 	struct sdhci_s3c *ourhost = to_s3c(host);
+	unsigned long rate, min = ULONG_MAX;
+	int src;
 
-	/*
-	 * initial clock can be in the frequency range of
-	 * 100KHz-400KHz, so we set it as max value.
-	 */
-	return clk_round_rate(ourhost->clk_bus[ourhost->cur_clk], 400000);
+	for (src = 0; src < MAX_BUS_CLK; src++) {
+		struct clk *clk;
+
+		clk = ourhost->clk_bus[src];
+		if (IS_ERR(clk))
+			continue;
+
+		rate = clk_round_rate(clk, 0);
+		if (rate < min)
+			min = rate;
+	}
+
+	return min;
 }
 
 /* sdhci_cmu_set_clock - callback on clock change.*/
-- 
1.8.5.2

[PATCH 5/6] mmc: sdhci-s3c: Fix handling of bus clock switching

[Tomasz Figa]

Currently the driver assumes at probe that controller is configured for
last valid enumerated bus clock. This assumption is completely wrong, as
there is no way to ensure such configuration until the hardware gets
first configured (by calling sdhci_s3c_set_clock()).

This patch modifies the driver to set current clock at probe to unknown
state (represented by negative value) and make sure that the hardware
gets actually configured to selected clock in sdhci_s3c_set_clock().

Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>
---
 drivers/mmc/host/sdhci-s3c.c | 78 ++++++++++----------------------------------
 1 file changed, 17 insertions(+), 61 deletions(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index 7e14db0..bad0e00 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -51,7 +51,7 @@ struct sdhci_s3c {
 	struct platform_device	*pdev;
 	struct resource		*ioarea;
 	struct s3c_sdhci_platdata *pdata;
-	unsigned int		cur_clk;
+	int			cur_clk;
 	int			ext_cd_irq;
 	int			ext_cd_gpio;
 
@@ -78,32 +78,6 @@ static inline struct sdhci_s3c *to_s3c(struct sdhci_host *host)
 }
 
 /**
- * get_curclk - convert ctrl2 register to clock source number
- * @ctrl2: Control2 register value.
- */
-static u32 get_curclk(u32 ctrl2)
-{
-	ctrl2 &= S3C_SDHCI_CTRL2_SELBASECLK_MASK;
-	ctrl2 >>= S3C_SDHCI_CTRL2_SELBASECLK_SHIFT;
-
-	return ctrl2;
-}
-
-static void sdhci_s3c_check_sclk(struct sdhci_host *host)
-{
-	struct sdhci_s3c *ourhost = to_s3c(host);
-	u32 tmp = readl(host->ioaddr + S3C_SDHCI_CONTROL2);
-
-	if (get_curclk(tmp) != ourhost->cur_clk) {
-		dev_dbg(&ourhost->pdev->dev, "restored ctrl2 clock setting\n");
-
-		tmp &= ~S3C_SDHCI_CTRL2_SELBASECLK_MASK;
-		tmp |= ourhost->cur_clk << S3C_SDHCI_CTRL2_SELBASECLK_SHIFT;
-		writel(tmp, host->ioaddr + S3C_SDHCI_CONTROL2);
-	}
-}
-
-/**
  * sdhci_s3c_get_max_clk - callback to get maximum clock frequency.
  * @host: The SDHCI host instance.
  *
@@ -115,11 +89,6 @@ static unsigned int sdhci_s3c_get_max_clk(struct sdhci_host *host)
 	unsigned long rate, max = 0;
 	int src;
 
-	/* note, a reset will reset the clock source */
-
-	sdhci_s3c_check_sclk(host);
-
-
 	for (src = 0; src < MAX_BUS_CLK; src++) {
 		rate = ourhost->clk_rates[src];
 		if (rate > max)
@@ -206,20 +175,22 @@ static void sdhci_s3c_set_clock(struct sdhci_host *host, unsigned int clock)
 		struct clk *clk = ourhost->clk_bus[best_src];
 
 		clk_prepare_enable(clk);
-		clk_disable_unprepare(ourhost->clk_bus[ourhost->cur_clk]);
-
-		/* turn clock off to card before changing clock source */
-		writew(0, host->ioaddr + SDHCI_CLOCK_CONTROL);
+		if (ourhost->cur_clk >= 0)
+			clk_disable_unprepare(
+					ourhost->clk_bus[ourhost->cur_clk]);
 
 		ourhost->cur_clk = best_src;
 		host->max_clk = ourhost->clk_rates[best_src];
-
-		ctrl = readl(host->ioaddr + S3C_SDHCI_CONTROL2);
-		ctrl &= ~S3C_SDHCI_CTRL2_SELBASECLK_MASK;
-		ctrl |= best_src << S3C_SDHCI_CTRL2_SELBASECLK_SHIFT;
-		writel(ctrl, host->ioaddr + S3C_SDHCI_CONTROL2);
 	}
 
+	/* turn clock off to card before changing clock source */
+	writew(0, host->ioaddr + SDHCI_CLOCK_CONTROL);
+
+	ctrl = readl(host->ioaddr + S3C_SDHCI_CONTROL2);
+	ctrl &= ~S3C_SDHCI_CTRL2_SELBASECLK_MASK;
+	ctrl |= best_src << S3C_SDHCI_CTRL2_SELBASECLK_SHIFT;
+	writel(ctrl, host->ioaddr + S3C_SDHCI_CONTROL2);
+
 	/* reprogram default hardware configuration */
 	writel(S3C64XX_SDHCI_CONTROL4_DRIVE_9mA,
 		host->ioaddr + S3C64XX_SDHCI_CONTROL4);
@@ -573,6 +544,7 @@ static int sdhci_s3c_probe(struct platform_device *pdev)
 	sc->host = host;
 	sc->pdev = pdev;
 	sc->pdata = pdata;
+	sc->cur_clk = -1;
 
 	platform_set_drvdata(pdev, host);
 
@@ -595,13 +567,6 @@ static int sdhci_s3c_probe(struct platform_device *pdev)
 			continue;
 
 		clks++;
-
-		/*
-		 * save current clock index to know which clock bus
-		 * is used later in overriding functions.
-		 */
-		sc->cur_clk = ptr;
-
 		sc->clk_rates[ptr] = clk_get_rate(sc->clk_bus[ptr]);
 
 		dev_info(dev, "clock source %d: %s (%ld Hz)\n",
@@ -614,10 +579,6 @@ static int sdhci_s3c_probe(struct platform_device *pdev)
 		goto err_no_busclks;
 	}
 
-#ifndef CONFIG_PM_RUNTIME
-	clk_prepare_enable(sc->clk_bus[sc->cur_clk]);
-#endif
-
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	host->ioaddr = devm_ioremap_resource(&pdev->dev, res);
 	if (IS_ERR(host->ioaddr)) {
@@ -730,10 +691,6 @@ static int sdhci_s3c_probe(struct platform_device *pdev)
 	return 0;
 
  err_req_regs:
-#ifndef CONFIG_PM_RUNTIME
-	clk_disable_unprepare(sc->clk_bus[sc->cur_clk]);
-#endif
-
  err_no_busclks:
 	clk_disable_unprepare(sc->clk_io);
 
@@ -764,9 +721,6 @@ static int sdhci_s3c_remove(struct platform_device *pdev)
 	pm_runtime_dont_use_autosuspend(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);
 
-#ifndef CONFIG_PM_RUNTIME
-	clk_disable_unprepare(sc->clk_bus[sc->cur_clk]);
-#endif
 	clk_disable_unprepare(sc->clk_io);
 
 	sdhci_free_host(host);
@@ -800,7 +754,8 @@ static int sdhci_s3c_runtime_suspend(struct device *dev)
 
 	ret = sdhci_runtime_suspend_host(host);
 
-	clk_disable_unprepare(ourhost->clk_bus[ourhost->cur_clk]);
+	if (ourhost->cur_clk >= 0)
+		clk_disable_unprepare(ourhost->clk_bus[ourhost->cur_clk]);
 	clk_disable_unprepare(busclk);
 	return ret;
 }
@@ -813,7 +768,8 @@ static int sdhci_s3c_runtime_resume(struct device *dev)
 	int ret;
 
 	clk_prepare_enable(busclk);
-	clk_prepare_enable(ourhost->clk_bus[ourhost->cur_clk]);
+	if (ourhost->cur_clk >= 0)
+		clk_prepare_enable(ourhost->clk_bus[ourhost->cur_clk]);
 	ret = sdhci_runtime_resume_host(host);
 	return ret;
 }
-- 
1.8.5.2

[PATCH 6/6] mmc: sdhci-s3c: Do not allow frequencies higher than requested

[Tomasz Figa]

This patch modifies sdhci_s3c_consider_clock() to fail if bus clock
being considered can not provide frequency lower or equal requested,
instead of returning the lowest supported.

Signed-off-by: Tomasz Figa <tomasz.figa@gmail.com>
---
 drivers/mmc/host/sdhci-s3c.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/sdhci-s3c.c b/drivers/mmc/host/sdhci-s3c.c
index bad0e00..d61eb5a 100644
--- a/drivers/mmc/host/sdhci-s3c.c
+++ b/drivers/mmc/host/sdhci-s3c.c
@@ -126,11 +126,18 @@ static unsigned int sdhci_s3c_consider_clock(struct sdhci_s3c *ourhost,
 
 	rate = ourhost->clk_rates[src];
 
-	for (shift = 0; shift < 8; ++shift) {
+	for (shift = 0; shift <= 8; ++shift) {
 		if ((rate >> shift) <= wanted)
 			break;
 	}
 
+	if (shift > 8) {
+		dev_dbg(&ourhost->pdev->dev,
+			"clk %d: rate %ld, min rate %lu > wanted %u\n",
+			src, rate, rate / 256, wanted);
+		return UINT_MAX;
+	}
+
 	dev_dbg(&ourhost->pdev->dev, "clk %d: rate %ld, want %d, got %ld\n",
 		src, rate, wanted, rate >> shift);
 
-- 
1.8.5.2

[PATCH 0/6] mmc: sdhci-s3c: Fix base clock source management

[Heiko Stübner]

Am Samstag, 11. Januar 2014, 22:39:00 schrieb Tomasz Figa:
> Tested on S3C6410-based Mini6410 board, with following performance
> figures:
> 
> * Before this series (133 MHz HCLK always selected, leading to at most
>   33 MHz card clock):
>
> root at tiny6410:~# hdparm -t /dev/mmcblk0
> 
> /dev/mmcblk0:
>  Timing buffered disk reads:  42 MB in  3.10 seconds =  13.54 MB/sec

# hdparm -t /dev/mmcblk0

/dev/mmcblk0:
 Timing buffered disk reads: 44 MB in  3.12 seconds =  14.11 MB/sec


> * After this series (48 MHz EPLL clock selected, leading to 48 MHz card
>   clock):
>
> root at tiny6410:~# hdparm -t /dev/mmcblk0
> 
> /dev/mmcblk0:
>  Timing buffered disk reads:  56 MB in  3.04 seconds =  18.41 MB/sec

# hdparm -t /dev/mmcblk0

/dev/mmcblk0:
 Timing buffered disk reads: 48 MB in  3.01 seconds =  15.94 MB/sec


So on a s3c2416:
Tested-by: Heiko Stuebner <heiko@sntech.de>
Acked-by: Heiko Stuebner <heiko@sntech.de>


Thanks for the added bandwidth :-)
Heiko

[PATCH 0/6] mmc: sdhci-s3c: Fix base clock source management

[Jaehoon Chung]

Hi, All.

On 01/13/2014 06:18 AM, Heiko St?bner wrote:
> Am Samstag, 11. Januar 2014, 22:39:00 schrieb Tomasz Figa:
>> Tested on S3C6410-based Mini6410 board, with following performance
>> figures:
>>
>> * Before this series (133 MHz HCLK always selected, leading to at most
>>   33 MHz card clock):
>>
>> root at tiny6410:~# hdparm -t /dev/mmcblk0
>>
>> /dev/mmcblk0:
>>  Timing buffered disk reads:  42 MB in  3.10 seconds =  13.54 MB/sec
> 
> # hdparm -t /dev/mmcblk0
> 
> /dev/mmcblk0:
>  Timing buffered disk reads: 44 MB in  3.12 seconds =  14.11 MB/sec

/dev/mmcblk0:
 Timing buffered disk reads:  38 MB in  3.14 seconds =  12.12 MB/sec
> 
> 
>> * After this series (48 MHz EPLL clock selected, leading to 48 MHz card
>>   clock):
>>
>> root at tiny6410:~# hdparm -t /dev/mmcblk0
>>
>> /dev/mmcblk0:
>>  Timing buffered disk reads:  56 MB in  3.04 seconds =  18.41 MB/sec
> 
> # hdparm -t /dev/mmcblk0
> 
> /dev/mmcblk0:
>  Timing buffered disk reads: 48 MB in  3.01 seconds =  15.94 MB/sec

 /dev/mmcblk0:
 Timing buffered disk reads:  56 MB in  3.05 seconds =  18.35 MB/sec
> 
> 
> So on a s3c2416:
> Tested-by: Heiko Stuebner <heiko@sntech.de>
> Acked-by: Heiko Stuebner <heiko@sntech.de>

Tested with exynos4 board.

Tested-by: Jaehoon Chung <jh80.chung@samsung.com>
Acked-by; Jaehoon Chung <jh80.chung@samsung.com>

Best Regards,
Jaehoon Chung

> 
> 
> Thanks for the added bandwidth :-)
> Heiko
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH 0/6] mmc: sdhci-s3c: Fix base clock source management

[Tomasz Figa]

Hi Jaehoon, Heiko,

On 13.01.2014 06:47, Jaehoon Chung wrote:
> Hi, All.
>
> On 01/13/2014 06:18 AM, Heiko St?bner wrote:
>> Am Samstag, 11. Januar 2014, 22:39:00 schrieb Tomasz Figa:
>>> Tested on S3C6410-based Mini6410 board, with following performance
>>> figures:
>>>
>>> * Before this series (133 MHz HCLK always selected, leading to at most
>>>    33 MHz card clock):
>>>
>>> root at tiny6410:~# hdparm -t /dev/mmcblk0
>>>
>>> /dev/mmcblk0:
>>>   Timing buffered disk reads:  42 MB in  3.10 seconds =  13.54 MB/sec
>>
>> # hdparm -t /dev/mmcblk0
>>
>> /dev/mmcblk0:
>>   Timing buffered disk reads: 44 MB in  3.12 seconds =  14.11 MB/sec
>
> /dev/mmcblk0:
>   Timing buffered disk reads:  38 MB in  3.14 seconds =  12.12 MB/sec
>>
>>
>>> * After this series (48 MHz EPLL clock selected, leading to 48 MHz card
>>>    clock):
>>>
>>> root at tiny6410:~# hdparm -t /dev/mmcblk0
>>>
>>> /dev/mmcblk0:
>>>   Timing buffered disk reads:  56 MB in  3.04 seconds =  18.41 MB/sec
>>
>> # hdparm -t /dev/mmcblk0
>>
>> /dev/mmcblk0:
>>   Timing buffered disk reads: 48 MB in  3.01 seconds =  15.94 MB/sec
>
>   /dev/mmcblk0:
>   Timing buffered disk reads:  56 MB in  3.05 seconds =  18.35 MB/sec
>>
>>
>> So on a s3c2416:
>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>> Acked-by: Heiko Stuebner <heiko@sntech.de>
>
> Tested with exynos4 board.
>
> Tested-by: Jaehoon Chung <jh80.chung@samsung.com>
> Acked-by; Jaehoon Chung <jh80.chung@samsung.com>

Thank you both for testing and acking.

Best regards,
Tomasz

[PATCH 0/6] mmc: sdhci-s3c: Fix base clock source management

[Tomasz Figa]

Hi Chris,

On 11.01.2014 22:39, Tomasz Figa wrote:
> On platforms prior to Exynos the SDHCI block used internal clock
> divider controlled by SELFREQ field of CLKCON register to divide base
> clock selected from several external clocks fed to the block by
> SELBASECLK bitfield of CONTROL2 register. Depending on wanted clock
> frequency, different external clock may be the best choice and so
> the driver needs to switch the SELBASECLK mux on the fly.
>
> However the selection logic has been broken for quite some time leaving
> the controller using always clock 0, which is not always the right
> source and leading to suboptimal performance of the SDHCI block on
> affected platforms.
>
> This series intends to fix the problems mentioned above and also clean-up
> clock management code slightly.
>
> Tested on S3C6410-based Mini6410 board, with following performance
> figures:
>
> * Before this series (133 MHz HCLK always selected, leading to at most
>    33 MHz card clock):
>
> root at tiny6410:~# hdparm -t /dev/mmcblk0
>
> /dev/mmcblk0:
>   Timing buffered disk reads:  42 MB in  3.10 seconds =  13.54 MB/sec
> root at tiny6410:~# hdparm -t /dev/mmcblk0
>
> /dev/mmcblk0:
>   Timing buffered disk reads:  44 MB in  3.11 seconds =  14.14 MB/sec
> root at tiny6410:~#
>
> * After this series (48 MHz EPLL clock selected, leading to 48 MHz card
>    clock):
>
> root at tiny6410:~# hdparm -t /dev/mmcblk0
>
> /dev/mmcblk0:
>   Timing buffered disk reads:  56 MB in  3.04 seconds =  18.41 MB/sec
> root at tiny6410:~# hdparm -t /dev/mmcblk0
>
> /dev/mmcblk0:
>   Timing buffered disk reads:  60 MB in  3.06 seconds =  19.63 MB/sec
> root at tiny6410:~#
>
> Tomasz Figa (6):
>    mmc: sdhci-s3c: Use shifts to divide by powers of two
>    mmc: sdhci-s3c: Cache bus clock rates
>    mmc: sdhci-s3c: Use correct condition to check for clock presence
>    mmc: sdhci-s3c: Simplify min/max clock calculation
>    mmc: sdhci-s3c: Fix handling of bus clock switching
>    mmc: sdhci-s3c: Do not allow frequencies higher than requested
>
>   drivers/mmc/host/sdhci-s3c.c | 170 ++++++++++++++++++++-----------------------
>   1 file changed, 77 insertions(+), 93 deletions(-)
>

What do you think about this series?

Best regards,
Tomasz

[PATCH 0/6] mmc: sdhci-s3c: Fix base clock source management

[Tomasz Figa]

Hi Chris,

On 07.02.2014 10:58, Tomasz Figa wrote:
> Hi Chris,
>
> On 11.01.2014 22:39, Tomasz Figa wrote:
>> On platforms prior to Exynos the SDHCI block used internal clock
>> divider controlled by SELFREQ field of CLKCON register to divide base
>> clock selected from several external clocks fed to the block by
>> SELBASECLK bitfield of CONTROL2 register. Depending on wanted clock
>> frequency, different external clock may be the best choice and so
>> the driver needs to switch the SELBASECLK mux on the fly.
>>
>> However the selection logic has been broken for quite some time leaving
>> the controller using always clock 0, which is not always the right
>> source and leading to suboptimal performance of the SDHCI block on
>> affected platforms.
>>
>> This series intends to fix the problems mentioned above and also clean-up
>> clock management code slightly.
>>
>> Tested on S3C6410-based Mini6410 board, with following performance
>> figures:
>>
>> * Before this series (133 MHz HCLK always selected, leading to at most
>>    33 MHz card clock):
>>
>> root at tiny6410:~# hdparm -t /dev/mmcblk0
>>
>> /dev/mmcblk0:
>>   Timing buffered disk reads:  42 MB in  3.10 seconds =  13.54 MB/sec
>> root at tiny6410:~# hdparm -t /dev/mmcblk0
>>
>> /dev/mmcblk0:
>>   Timing buffered disk reads:  44 MB in  3.11 seconds =  14.14 MB/sec
>> root at tiny6410:~#
>>
>> * After this series (48 MHz EPLL clock selected, leading to 48 MHz card
>>    clock):
>>
>> root at tiny6410:~# hdparm -t /dev/mmcblk0
>>
>> /dev/mmcblk0:
>>   Timing buffered disk reads:  56 MB in  3.04 seconds =  18.41 MB/sec
>> root at tiny6410:~# hdparm -t /dev/mmcblk0
>>
>> /dev/mmcblk0:
>>   Timing buffered disk reads:  60 MB in  3.06 seconds =  19.63 MB/sec
>> root at tiny6410:~#
>>
>> Tomasz Figa (6):
>>    mmc: sdhci-s3c: Use shifts to divide by powers of two
>>    mmc: sdhci-s3c: Cache bus clock rates
>>    mmc: sdhci-s3c: Use correct condition to check for clock presence
>>    mmc: sdhci-s3c: Simplify min/max clock calculation
>>    mmc: sdhci-s3c: Fix handling of bus clock switching
>>    mmc: sdhci-s3c: Do not allow frequencies higher than requested
>>
>>   drivers/mmc/host/sdhci-s3c.c | 170
>> ++++++++++++++++++++-----------------------
>>   1 file changed, 77 insertions(+), 93 deletions(-)
>>
>
> What do you think about this series?

Could you take this series for 3.15?

Best regards,
Tomasz

[PATCH 0/6] mmc: sdhci-s3c: Fix base clock source management

[Tomasz Figa]

On 20.02.2014 20:26, Tomasz Figa wrote:
> Hi Chris,
>
> On 07.02.2014 10:58, Tomasz Figa wrote:
>> Hi Chris,
>>
>> On 11.01.2014 22:39, Tomasz Figa wrote:
>>> On platforms prior to Exynos the SDHCI block used internal clock
>>> divider controlled by SELFREQ field of CLKCON register to divide base
>>> clock selected from several external clocks fed to the block by
>>> SELBASECLK bitfield of CONTROL2 register. Depending on wanted clock
>>> frequency, different external clock may be the best choice and so
>>> the driver needs to switch the SELBASECLK mux on the fly.
>>>
>>> However the selection logic has been broken for quite some time leaving
>>> the controller using always clock 0, which is not always the right
>>> source and leading to suboptimal performance of the SDHCI block on
>>> affected platforms.
>>>
>>> This series intends to fix the problems mentioned above and also
>>> clean-up
>>> clock management code slightly.
>>>
>>> Tested on S3C6410-based Mini6410 board, with following performance
>>> figures:
>>>
>>> * Before this series (133 MHz HCLK always selected, leading to at most
>>>    33 MHz card clock):
>>>
>>> root at tiny6410:~# hdparm -t /dev/mmcblk0
>>>
>>> /dev/mmcblk0:
>>>   Timing buffered disk reads:  42 MB in  3.10 seconds =  13.54 MB/sec
>>> root at tiny6410:~# hdparm -t /dev/mmcblk0
>>>
>>> /dev/mmcblk0:
>>>   Timing buffered disk reads:  44 MB in  3.11 seconds =  14.14 MB/sec
>>> root at tiny6410:~#
>>>
>>> * After this series (48 MHz EPLL clock selected, leading to 48 MHz card
>>>    clock):
>>>
>>> root at tiny6410:~# hdparm -t /dev/mmcblk0
>>>
>>> /dev/mmcblk0:
>>>   Timing buffered disk reads:  56 MB in  3.04 seconds =  18.41 MB/sec
>>> root at tiny6410:~# hdparm -t /dev/mmcblk0
>>>
>>> /dev/mmcblk0:
>>>   Timing buffered disk reads:  60 MB in  3.06 seconds =  19.63 MB/sec
>>> root at tiny6410:~#
>>>
>>> Tomasz Figa (6):
>>>    mmc: sdhci-s3c: Use shifts to divide by powers of two
>>>    mmc: sdhci-s3c: Cache bus clock rates
>>>    mmc: sdhci-s3c: Use correct condition to check for clock presence
>>>    mmc: sdhci-s3c: Simplify min/max clock calculation
>>>    mmc: sdhci-s3c: Fix handling of bus clock switching
>>>    mmc: sdhci-s3c: Do not allow frequencies higher than requested
>>>
>>>   drivers/mmc/host/sdhci-s3c.c | 170
>>> ++++++++++++++++++++-----------------------
>>>   1 file changed, 77 insertions(+), 93 deletions(-)
>>>
>>
>> What do you think about this series?
>
> Could you take this series for 3.15?

Ping.

It's been almost two months since I posted this series, it's been 
already ACKed and it would be nice to have it applied for upcoming release.

Best regards,
Tomasz

[PATCH 0/6] mmc: sdhci-s3c: Fix base clock source management

[Chris Ball]

Hi,

On Mon, Mar 03 2014, Tomasz Figa wrote:
> It's been almost two months since I posted this series, it's been
> already ACKed and it would be nice to have it applied for upcoming
> release.

Sorry about this, Tomasz -- now pushed to mmc-next for 3.15, thanks.

- Chris.
-- 
Chris Ball   <chris@printf.net>   <http://printf.net/>

[PATCH 0/6] mmc: sdhci-s3c: Fix base clock source management

[Tomasz Figa]

On 03.03.2014 16:24, Chris Ball wrote:
> Hi,
>
> On Mon, Mar 03 2014, Tomasz Figa wrote:
>> It's been almost two months since I posted this series, it's been
>> already ACKed and it would be nice to have it applied for upcoming
>> release.
>
> Sorry about this, Tomasz -- now pushed to mmc-next for 3.15, thanks.

Thanks. \o/

Best regards,
Tomasz