Re: [PATCH v2] mxs-mmc: fix clock rate setting

2011-07-15 Thread Koen Beel
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


[PATCH v2] mxs-mmc: fix clock rate setting

2011-07-11 Thread Koen Beel
Hi,

(Sorry, patch in attachment as I still don't have a decent mail setup
at my company)

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.

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

Regards,
Koen


0001-mxs-mmc-fix-clock-rate-setting.patch
Description: Binary data


Re: [PATCH] mxs-mmc: fix clock rate setting

2011-07-06 Thread Koen Beel
On Wed, Jul 6, 2011 at 11:38 AM, Wolfram Sang w.s...@pengutronix.de wrote:
 On Mon, Jul 04, 2011 at 12:59:29PM +0200, Koen Beel wrote:
 On Mon, Jul 4, 2011 at 12:34 PM, Wolfram Sang w.s...@pengutronix.de wrote:
 
 Well, maybe not. My colleague complained and I think he is right 
 that we are
 mapping div2 from the range 0 to 256 (inclusive!) to an 8-bit 
 range. This must
 be wrong for one value per se.

If you look at the context of the patch, you will find it's 'div2 - 1'
than 'div2' gets written into register.
  
   Exactly. The '- 1' is why Koen changed the upper limit from  256 to = 
   256.
   The lower limit fix is currently 'if (div2 == 0) div2 == 1', which is a 
   2:1
   mapping. Not good, or?
  
  So you are saying the patch is a right fix but not the most optimal
  one?  In that case, it does not concern me.  I acked it as an valid
  fix.
 
  The patches fixes a few things, but for div2, it is curing the symptoms,
  not the cause, I think. If you look at the formula in the datasheet:
 
  rate = ssp / (clock_divide * (1 + clock_rate));
 
  In the code, the calculation of div2 is equal to '1 + clock_rate' (the 1
  gets subtracted when the value is written to the register which is a bit
  unfortunate; doing it earlier would reduce confusion IMO). So that can
  never be 0, it is a divisor. We should really use DIV_ROUND_UP here.
  Even when not dealing with 0, this seems needed. Assume:
 
  ssp = 5760, wanted_rate = 2500, div1 = 2
 
  will give
 
  div2 = int(1.152) = 1 (meaning 0 + 1, because div2 - 1 will be written)
 
  The rate will thus be:
 
  actual_rate = 5760 / 2 * (0 + 1)
             = 2880
 
  - too fast!
 
  Or did I get something wrong?
 
  Regards,
 
    Wolfram

 Well, right now the clock freq. is set to the minimum clock value
 reaching the requested rate.

 Sorry, it gets a bit confusing: what do you mean with right now?

I mean: in current mainline the actual clock is set to be at least the
requested clock rate.


 In current implementation, the rate will
 be higher in lot's of cases (all cases where the requested clock freq.
 cannot exactly be made).

 Yes.

This is just the consequence of the fact that the actual clock is set
to be at least the requested clock rate.


 But the thing is, the driver now enters dev_err, and returns without
 changing anything when the clock cannot be made as low as requested.
 In that case you will almost certainly end up with a clock being even
 higher then the lowest possible. So that not good I think.
 I might be better to set the clock as low as possible not matter what
 you to after that.

 Yes, might be a bit academic (who would want 4kHz ;)), but still
 correct. Shawn, do you agree?

 About the rounding. I don't think rounding is good. I think it would

 We do rounding already (int conversion). Just in the wrong direction.

 be better to choose between having at least the requested rate (as it
 is now; will result is somewhat higher clock then requested in many
 cases)

 You want a rate faster than what the card could do?

Correct, you don't want the clock to be faster then the requested clock.
So the rounding is indeed in the wrong direction now.


, or having at maximum the requested clock rate. Both have there
 problems.

 If I understood you correctly, this is my DIV_ROUND_UP suggestion. Which
 problems does it have?

I think I misunderstood this suggestion previously. Using DIV_ROUND_UP
would do the rounding in the correct direction.
This should result in: the actual clock is as high as possible
without being higher then the requested clock.


 Regards,

   Wolfram

 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.10 (GNU/Linux)

 iEYEARECAAYFAk4ULSgACgkQD27XaX1/VRsorQCfVRVm9Vpv4YSsfiMqIFctTKG9
 7qkAnicSrjSzwObzBjyISDnXz6+Sze2s
 =5LIq
 -END PGP SIGNATURE-


--
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


Re: [PATCH] mxs-mmc: fix clock rate setting

2011-07-04 Thread Koen Beel
On Sun, Jul 3, 2011 at 1:54 PM, Shawn Guo shawn@freescale.com wrote:
 On Sun, Jul 03, 2011 at 12:31:24PM +0200, Wolfram Sang wrote:
 On Sun, Jul 03, 2011 at 05:28:52PM +0800, Shawn Guo wrote:
  On Sat, Jul 02, 2011 at 03:12:25PM +0200, Wolfram Sang wrote:
   On Fri, Jul 01, 2011 at 11:46:29PM +0800, Shawn Guo wrote:
On Fri, Jul 01, 2011 at 04:44:09PM +0200, Wolfram Sang wrote:
 Hi,

 On Fri, Jul 01, 2011 at 03:26:39PM +0200, Koen Beel wrote:
  I send the patch as attachment for now.

 Fine with me in this case...

  But I'll have to look into another way of doing this. Corporate 
  mail
  system is adding stupid disclaimers, gmail web ui is not working ok
  and corporate firewalls avoid using a different smtp server...

 Good luck with that!

 About the patch itself: I didn't verify the formulas, but it does 
 solve one
 special problem here. Thanks a lot! So:

 Tested-by: Wolfram Sang w.s...@pengutronix.de

 @chris: If Shawn also likes the patch, I think this is a stable 
 candidate.

Thanks for the fixing, Koen.
   
Acked-by: Shawn Guo shawn@freescale.com
  
   Well, maybe not. My colleague complained and I think he is right that we 
   are
   mapping div2 from the range 0 to 256 (inclusive!) to an 8-bit range. 
   This must
   be wrong for one value per se.
  
  If you look at the context of the patch, you will find it's 'div2 - 1'
  than 'div2' gets written into register.

 Exactly. The '- 1' is why Koen changed the upper limit from  256 to = 256.
 The lower limit fix is currently 'if (div2 == 0) div2 == 1', which is a 2:1
 mapping. Not good, or?

 So you are saying the patch is a right fix but not the most optimal
 one?  In that case, it does not concern me.  I acked it as an valid
 fix.

According to the mx23/28 datasheet:
- actual mmc/sdio clock = ssp_sck = (sspclk) / ( clock_divide * (1+clock_rate) )
- clock_divide ranges from 2 to 254, by steps of 2
- clock_rate ranges from 0 to 255, so div2 ranges from 1 to 256
- if div2 is the range [0..1[, the only thing we can do is setting it
on 1. It just can't be lower. The actual clock rate will be lower then
requested.
- div2 will only be  1 if div1 is 2. This is because div1 is tested
for a good value starting at the lowest possible value (2). Making
div2 higher will not solve the issue, it will only result in an even
lower actual clock
- you could choose the give dev_err (cannot set clock to ...) but i
would not do that. Default requested clock for high speed sdio is 50
MHz and the default configuration for mx23 does not support this, so
you will also get an error. And having a lower clock then requested is
not that bad as having a higher clock.

In the code (as before):
- div1 = clock_divide (just written in hw)
- div2 = 1+clock_rate (clock_rate = div2 - 1 is written in hw)

So i think its correct. It work here in all tested condition.

Regards,
Koen


 --
 Regards,
 Shawn


--
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


Re: [PATCH] mxs-mmc: fix clock rate setting

2011-07-04 Thread Koen Beel
On Mon, Jul 4, 2011 at 12:34 PM, Wolfram Sang w.s...@pengutronix.de wrote:

Well, maybe not. My colleague complained and I think he is right that 
we are
mapping div2 from the range 0 to 256 (inclusive!) to an 8-bit range. 
This must
be wrong for one value per se.
   
   If you look at the context of the patch, you will find it's 'div2 - 1'
   than 'div2' gets written into register.
 
  Exactly. The '- 1' is why Koen changed the upper limit from  256 to = 
  256.
  The lower limit fix is currently 'if (div2 == 0) div2 == 1', which is a 2:1
  mapping. Not good, or?
 
 So you are saying the patch is a right fix but not the most optimal
 one?  In that case, it does not concern me.  I acked it as an valid
 fix.

 The patches fixes a few things, but for div2, it is curing the symptoms,
 not the cause, I think. If you look at the formula in the datasheet:

 rate = ssp / (clock_divide * (1 + clock_rate));

 In the code, the calculation of div2 is equal to '1 + clock_rate' (the 1
 gets subtracted when the value is written to the register which is a bit
 unfortunate; doing it earlier would reduce confusion IMO). So that can
 never be 0, it is a divisor. We should really use DIV_ROUND_UP here.
 Even when not dealing with 0, this seems needed. Assume:

 ssp = 5760, wanted_rate = 2500, div1 = 2

 will give

 div2 = int(1.152) = 1 (meaning 0 + 1, because div2 - 1 will be written)

 The rate will thus be:

 actual_rate = 5760 / 2 * (0 + 1)
            = 2880

 - too fast!

 Or did I get something wrong?

 Regards,

   Wolfram

Well, right now the clock freq. is set to the minimum clock value
reaching the requested rate. In current implementation, the rate will
be higher in lot's of cases (all cases where the requested clock freq.
cannot exactly be made).

But the thing is, the driver now enters dev_err, and returns without
changing anything when the clock cannot be made as low as requested.
In that case you will almost certainly end up with a clock being even
higher then the lowest possible. So that not good I think.
I might be better to set the clock as low as possible not matter what
you to after that.

About the rounding. I don't think rounding is good. I think it would
be better to choose between having at least the requested rate (as it
is now; will result is somewhat higher clock then requested in many
cases), or having at maximum the requested clock rate. Both have there
problems.





 --
 Pengutronix e.K.                           | Wolfram Sang                |
 Industrial Linux Solutions                 | http://www.pengutronix.de/  |

 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.10 (GNU/Linux)

 iEYEARECAAYFAk4RlxoACgkQD27XaX1/VRuf7ACgqQ4PvBf9d0am2ButvDT0ZHy1
 CQwAn1iHNXcl6t46IW7L/l3UkW7J2nxb
 =kbX8
 -END PGP SIGNATURE-


--
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


Re: [PATCH] mxs-mmc: fix clock rate setting

2011-07-04 Thread Koen Beel
On Fri, Jul 1, 2011 at 4:44 PM, Wolfram Sang w.s...@pengutronix.de wrote:
 Hi,

 On Fri, Jul 01, 2011 at 03:26:39PM +0200, Koen Beel wrote:
 I send the patch as attachment for now.

 Fine with me in this case...

 But I'll have to look into another way of doing this. Corporate mail
 system is adding stupid disclaimers, gmail web ui is not working ok
 and corporate firewalls avoid using a different smtp server...

 Good luck with that!

 About the patch itself: I didn't verify the formulas, but it does solve one
 special problem here. Thanks a lot! So:

Does it solve any other special problem except for the wrong clock rate setting?



 Tested-by: Wolfram Sang w.s...@pengutronix.de

 @chris: If Shawn also likes the patch, I think this is a stable candidate.

 Very minor nits:

 From ab610437fade5b30fdeacd9e959ddf95bc42587a Mon Sep 17 00:00:00 2001
 From: Koen Beel koen.b...@barco.com
 Date: Thu, 30 Jun 2011 12:00:19 +0200
 Subject: [PATCH] mxs-mmc: fix clock rate setting

 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.

 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.

 Commit msg should linebreak at 72.


 Signed-off-by: Koen beel koen.b...@barco.com

 Capital 'B'?

Indeed, it should have been:
Signed-off-by: Koen Beel koen.b...@barco.com


 Regards,

   Wolfram

 --
 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


Re: [PATCH] mxs-mmc: fix clock rate setting

2011-07-01 Thread Koen Beel
Hi,

Think the tabs problem was due to the gmail web ui.
Hope it's better now.


Signed-off-by: Koen Beel koen.beel at barco.com
---
 drivers/mmc/host/mxs-mmc.c |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index 99d39a6..3575330 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -570,22 +570,22 @@ static void mxs_mmc_set_clk_rate(struct
mxs_mmc_host *host, unsigned int rate)
ssp_rate = clk_get_rate(host-clk);
-   for (div1 = 2; div1  254; div1 += 2) {
+   for (div1 = 2; div1 = 254; div1 += 2) {
div2 = ssp_rate / rate / div1;
-   if (div2  0x100)
+   if (div2 = 256)
break;
}
-   if (div1 = 254) {
+   if (div1  254) {
dev_err(mmc_dev(host-mmc),
%s: cannot set clock to %d\n, __func__, rate);
return;
}
if (div2 == 0)
-   bit_rate = ssp_rate / div1;
-   else
-   bit_rate = ssp_rate / div1 / div2;
+   div2 = 1;
+
+   bit_rate = ssp_rate / div1 / div2;
val = readl(host-base + HW_SSP_TIMING);
val = ~(BM_SSP_TIMING_CLOCK_DIVIDE | BM_SSP_TIMING_CLOCK_RATE);
--
1.7.4.1

On Fri, Jul 1, 2011 at 11:17 AM, Wolfram Sang w.s...@pengutronix.de wrote:
 On Thu, Jun 30, 2011 at 04:55:07PM +0200, Wolfram Sang wrote:
 On Thu, Jun 30, 2011 at 12:13:34PM +0200, Koen Beel wrote:
  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.
 
  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.
 
  Signed-off-by: Koen Beel koen.beel.barco at gmail.com

 Looks promising, but your tabs are garbled (0xa0 0x20 here?)

 Can you repost a patch which applies? I'd like to test it.

 --
 Pengutronix e.K.                           | Wolfram Sang                |
 Industrial Linux Solutions                 | http://www.pengutronix.de/  |

 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.10 (GNU/Linux)

 iEUEARECAAYFAk4NkMEACgkQD27XaX1/VRvw0QCeP4F3oeKe1Ge3SLohJICLxZre
 LKYAmJ2sEztaKIVw4NsZMYNqCUbbwFQ=
 =C7kg
 -END PGP SIGNATURE-


--
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


Re: [PATCH] mxs-mmc: fix clock rate setting

2011-07-01 Thread Koen Beel
I send the patch as attachment for now.

But I'll have to look into another way of doing this. Corporate mail
system is adding stupid disclaimers, gmail web ui is not working ok
and corporate firewalls avoid using a different smtp server...



On Fri, Jul 1, 2011 at 2:27 PM, Wolfram Sang w.s...@pengutronix.de wrote:

 Think the tabs problem was due to the gmail web ui.
 Hope it's better now.

 No 0xa0 anymore, but still no tabs. Oh, and lines are wrapped.

 Documentation/email-clients.txt says

 ===

 Gmail (Web GUI)

 Does not work for sending patches.

 Gmail web client converts tabs to spaces automatically.

 At the same time it wraps lines every 78 chars with CRLF style line
 breaks although tab2space problem can be solved with external editor.

 Another problem is that Gmail will base64-encode any message that has a
 non-ASCII character. That includes things like European names.

 ===

 I don't know if git-send-email can work directly with gmail, but it
 seems you need some kind of alternative approach.

 Regards,

   Wolfram

 --
 Pengutronix e.K.                           | Wolfram Sang                |
 Industrial Linux Solutions                 | http://www.pengutronix.de/  |

 -BEGIN PGP SIGNATURE-
 Version: GnuPG v1.4.10 (GNU/Linux)

 iEYEARECAAYFAk4NvR0ACgkQD27XaX1/VRtjmACfQzU2j5+awQ78Ey7Ir+39uhxn
 j70AoJQG0D6tK3rYh4YeYsiyFAsLieC5
 =DqDV
 -END PGP SIGNATURE-


From ab610437fade5b30fdeacd9e959ddf95bc42587a Mon Sep 17 00:00:00 2001
From: Koen Beel koen.b...@barco.com
Date: Thu, 30 Jun 2011 12:00:19 +0200
Subject: [PATCH] mxs-mmc: fix clock rate setting

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.

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.

Signed-off-by: Koen beel koen.b...@barco.com
---
 drivers/mmc/host/mxs-mmc.c |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index 99d39a6..3575330 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -570,22 +570,22 @@ static void mxs_mmc_set_clk_rate(struct mxs_mmc_host *host, unsigned int rate)
 
 	ssp_rate = clk_get_rate(host-clk);
 
-	for (div1 = 2; div1  254; div1 += 2) {
+	for (div1 = 2; div1 = 254; div1 += 2) {
 		div2 = ssp_rate / rate / div1;
-		if (div2  0x100)
+		if (div2 = 256)
 			break;
 	}
 
-	if (div1 = 254) {
+	if (div1  254) {
 		dev_err(mmc_dev(host-mmc),
 			%s: cannot set clock to %d\n, __func__, rate);
 		return;
 	}
 
 	if (div2 == 0)
-		bit_rate = ssp_rate / div1;
-	else
-		bit_rate = ssp_rate / div1 / div2;
+		div2 = 1;
+
+	bit_rate = ssp_rate / div1 / div2;
 
 	val = readl(host-base + HW_SSP_TIMING);
 	val = ~(BM_SSP_TIMING_CLOCK_DIVIDE | BM_SSP_TIMING_CLOCK_RATE);
-- 
1.7.4.1



[PATCH] mxs-mmc: fix clock rate setting

2011-06-30 Thread Koen Beel
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.

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.

Signed-off-by: Koen Beel koen.beel.barco at gmail.com
---
 drivers/mmc/host/mxs-mmc.c |   12 ++--
 1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/mxs-mmc.c b/drivers/mmc/host/mxs-mmc.c
index 99d39a6..3575330 100644
--- a/drivers/mmc/host/mxs-mmc.c
+++ b/drivers/mmc/host/mxs-mmc.c
@@ -570,22 +570,22 @@ static void mxs_mmc_set_clk_rate(struct
mxs_mmc_host *host, unsigned int rate)
    ssp_rate = clk_get_rate(host-clk);
-   for (div1 = 2; div1  254; div1 += 2) {
+   for (div1 = 2; div1 = 254; div1 += 2) {
        div2 = ssp_rate / rate / div1;
-       if (div2  0x100)
+       if (div2 = 256)
            break;
    }
-   if (div1 = 254) {
+   if (div1  254) {
        dev_err(mmc_dev(host-mmc),
            %s: cannot set clock to %d\n, __func__, rate);
        return;
    }
    if (div2 == 0)
-       bit_rate = ssp_rate / div1;
-   else
-       bit_rate = ssp_rate / div1 / div2;
+       div2 = 1;
+
+   bit_rate = ssp_rate / div1 / div2;
    val = readl(host-base + HW_SSP_TIMING);
    val = ~(BM_SSP_TIMING_CLOCK_DIVIDE | BM_SSP_TIMING_CLOCK_RATE);
--
1.7.4.1
--
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