Hi,
On Mon, Jul 11, 2011 at 10:26 PM, Wolfram Sang w.s...@pengutronix.de wrote:
Hi Koen,
thanks for v2.
On Mon, Jul 11, 2011 at 09:52:01PM +0200, Koen Beel wrote:
(Sorry, patch in attachment as I still don't have a decent mail setup
at my company)
That's bad :(
Fix clock rate setting on mxs-mmc driver.
Previously, if div2 was zero the value for TIMING_CLOCK_RATE would
have been 255 instead of 0.
Also the limits for div1 (TIMING_CLOCK_DIVIDE) and div2
(TIMING_CLOCK_RATE + 1) where not correctly defined.
Very minor nit: No paragraphs I'd say.
Can easily be reproduced on mx23evk: default clock for high speed sdio
cards is 50 MHz. With a SSP_CLK of 28.8 MHz default), this resulted in
an actual clock rate of about 56 kHz.
Tested on mx23evk.
Changes in V2 patch:
- use DIV_ROUND_UP to make sure the actual clock rate is not higher
then the requested clock rate.
- rename variables to reflect naming in datasheet and to make things
more clear
Changelogs are good, but should go below ---
Signed-off-by: Koen Beel koen.b...@barco.com
---
drivers/mmc/host/mxs-mmc.c | 30 ++
1 files changed, 14 insertions(+), 16 deletions(-)
diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index 99d39a6..d513d47 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -564,40 +564,38 @@ static void mxs_mmc_request(struct mmc_host *mmc,
struct mmc_request *mrq)
static void mxs_mmc_set_clk_rate(struct mxs_mmc_host *host, unsigned int
rate)
{
- unsigned int ssp_rate, bit_rate;
- u32 div1, div2;
+ unsigned int ssp_clk, ssp_sck;
ssp_sck is not really needed? Value could directly go to host-clk_rate.
+ u32 clock_divide, clock_rate;
u32 val;
- ssp_rate = clk_get_rate(host-clk);
+ ssp_clk = clk_get_rate(host-clk);
- for (div1 = 2; div1 254; div1 += 2) {
- div2 = ssp_rate / rate / div1;
- if (div2 0x100)
+ for (clock_divide = 2; clock_divide = 254; clock_divide += 2) {
+ clock_rate = DIV_ROUND_UP(ssp_clk, rate * clock_divide);
+ clock_rate = (clock_rate 0) ? clock_rate - 1 : 0;
+ if (clock_rate = 255)
break;
}
- if (div1 = 254) {
+ if (clock_divide 254) {
dev_err(mmc_dev(host-mmc),
%s: cannot set clock to %d\n, __func__, rate);
Didn't you want to set some minimal values instead of just returning?
Just wondering, could be a seperate patch in my book...
return;
}
- if (div2 == 0)
- bit_rate = ssp_rate / div1;
- else
- bit_rate = ssp_rate / div1 / div2;
+ ssp_sck = ssp_clk / clock_divide / (1 + clock_rate);
val = readl(host-base + HW_SSP_TIMING);
val = ~(BM_SSP_TIMING_CLOCK_DIVIDE | BM_SSP_TIMING_CLOCK_RATE);
- val |= BF_SSP(div1, TIMING_CLOCK_DIVIDE);
- val |= BF_SSP(div2 - 1, TIMING_CLOCK_RATE);
+ val |= BF_SSP(clock_divide, TIMING_CLOCK_DIVIDE);
+ val |= BF_SSP(clock_rate, TIMING_CLOCK_RATE);
writel(val, host-base + HW_SSP_TIMING);
- host-clk_rate = bit_rate;
+ host-clk_rate = ssp_sck;
dev_dbg(mmc_dev(host-mmc),
- %s: div1 %d, div2 %d, ssp %d, bit %d, rate %d\n,
- __func__, div1, div2, ssp_rate, bit_rate, rate);
+ %s: clock_divide %d, clock_rate %d, ssp_clk %d, rate_actual
%d, rate_requested %d\n,
+ __func__, clock_divide, clock_rate, ssp_clk, ssp_sck, rate);
}
Rest looks good to me, and test was also successful.
Chris, if you want to take this version already:
Still need me to do something on this or is it ok?
Koen
Reviewed-by: Wolfram Sang w.s...@pengutronix.de
--
Pengutronix e.K. | Wolfram Sang |
Industrial Linux Solutions | http://www.pengutronix.de/ |
--
To unsubscribe from this list: send the line unsubscribe linux-mmc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
