mmc: sdhci-s3c: Fix base clock source management
Subsystems (4)
cover mmc: sdhci-s3c: Fix base clock source management
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(-)
Signature · 1 line
1.8.5.2
PATCH 1/6 mmc: sdhci-s3c: Use shifts to divide by powers of two
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.
--- drivers/mmc/host/sdhci-s3c.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
drivers/mmc/host/sdhci-s3c.c +5 -5
@@ -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); }/**
Signature · 1 line
1.8.5.2
PATCH 2/6 mmc: sdhci-s3c: Cache bus clock rates
In reply to Tomasz FigaTo 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.
--- drivers/mmc/host/sdhci-s3c.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)
drivers/mmc/host/sdhci-s3c.c +6 -3
@@ -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 ) {
Signature · 1 line
1.8.5.2
PATCH 3/6 mmc: sdhci-s3c: Use correct condition to check for clock presence
In reply to Tomasz FigaIS_ERR() must be used to make sure that not a valid clock was returned by clk_get() and company.
--- drivers/mmc/host/sdhci-s3c.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
drivers/mmc/host/sdhci-s3c.c +3 -5
@@ -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
Signature · 1 line
1.8.5.2
PATCH 4/6 mmc: sdhci-s3c: Simplify min/max clock calculation
In reply to Tomasz FigaThis 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.
--- drivers/mmc/host/sdhci-s3c.c | 60 +++++++++++++++++++++++++++++--------------- 1 file changed, 40 insertions(+), 20 deletions(-)
drivers/mmc/host/sdhci-s3c.c +40 -20
@@ -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.*/
Signature · 1 line
1.8.5.2
PATCH 5/6 mmc: sdhci-s3c: Fix handling of bus clock switching
In reply to Tomasz FigaCurrently 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().
--- drivers/mmc/host/sdhci-s3c.c | 78 ++++++++++---------------------------------- 1 file changed, 17 insertions(+), 61 deletions(-)
drivers/mmc/host/sdhci-s3c.c +17 -61
@@ -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; }
Signature · 1 line
1.8.5.2
PATCH 6/6 mmc: sdhci-s3c: Do not allow frequencies higher than requested
In reply to Tomasz FigaThis 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.
--- drivers/mmc/host/sdhci-s3c.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/mmc/host/sdhci-s3c.c +8 -1
@@ -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);
Signature · 1 line
1.8.5.2
cover mmc: sdhci-s3c: Fix base clock source management
In reply to Tomasz FigaAm Samstag, 11. Januar 2014, 22:39:00 schrieb Tomasz Figa:
Quoted 10 lines
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
Quoted 7 lines
* 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:
Thanks for the added bandwidth :-)
Heiko
Jaehoon Chung
maintainercover mmc: sdhci-s3c: Fix base clock source management
Hi, All.
Quoted 9 lines · Heiko Stübner
Am Samstag, 11. Januar 2014, 22:39:00 schrieb Tomasz Figa:
Quoted 8 lines
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@tiny6410:~# hdparm -t /dev/mmcblk0
Quoted 7 lines
Quoted 2 lines
/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
Quoted 7 lines
Quoted 7 lines
* After this series (48 MHz EPLL clock selected, leading to 48 MHz card clock):
root@tiny6410:~# hdparm -t /dev/mmcblk0
Quoted 7 lines
Quoted 2 lines
/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
Quoted 5 lines
So on a s3c2416:
Tested-by Heiko Stuebner <[email protected]>Acked-by Heiko Stuebner <[email protected]>
Tested with exynos4 board.
Acked-by; Jaehoon Chung <[email protected]>
Best Regards,
Jaehoon Chung
Quoted 11 lines
Thanks for the added bandwidth :-)
Heiko_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
cover mmc: sdhci-s3c: Fix base clock source management
Hi Jaehoon, Heiko,
Quoted 41 lines · Jaehoon Chung
Hi, All.
Quoted 9 lines · Heiko St?bner
Am Samstag, 11. Januar 2014, 22:39:00 schrieb Tomasz Figa:
Quoted 8 lines
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/mmcblk0Quoted 7 lines
Quoted 2 lines
/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/secQuoted 7 lines
Quoted 7 lines
* After this series (48 MHz EPLL clock selected, leading to 48 MHz card clock):
root at tiny6410:~# hdparm -t
/dev/mmcblk0Quoted 7 lines
Quoted 2 lines
/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/secQuoted 2 lines
Quoted 8 lines
Quoted 3 lines
So on a s3c2416:
Tested-by Heiko Stuebner <[email protected]>Acked-by Heiko Stuebner <[email protected]>Tested with exynos4 board.
Tested-by Jaehoon Chung <[email protected]>Acked-by; Jaehoon Chung <[email protected]>
Thank you both for testing and acking.
Best regards,
Tomasz
cover mmc: sdhci-s3c: Fix base clock source management
In reply to Tomasz FigaHi Chris,
Quoted 52 lines · 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
Quoted 3 lines
drivers/mmc/host/sdhci-s3c.c | 170 ++++++++++++++++++++----------------------- 1 file changed, 77 insertions(+), 93 deletions(-)
What do you think about this series?
Best regards,
Tomasz
cover mmc: sdhci-s3c: Fix base clock source management
Hi Chris,
Quoted 55 lines · Tomasz Figa
Hi Chris,
Quoted 52 lines · 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
Quoted 6 lines
Quoted 4 lines
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
cover mmc: sdhci-s3c: Fix base clock source management
Quoted 59 lines · Tomasz Figa
Hi Chris,
Quoted 56 lines · Tomasz Figa
Hi Chris,
Quoted 53 lines · 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
Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
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
cover mmc: sdhci-s3c: Fix base clock source management
Hi,
Quoted 3 lines · Tomasz Figa
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.
Signature · 1 line
Chris Ball <[email protected]> <http://printf.net/>
cover mmc: sdhci-s3c: Fix base clock source management
Quoted 8 lines · Chris Ball
Hi,
Quoted 3 lines · Tomasz Figa
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