On Sun, Jul 3, 2011 at 1:54 PM, Shawn Guo <shawn.guo@xxxxxxxxxxxxx> 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.sang@xxxxxxxxxxxxxx>
>> > > > >
>> > > > > @chris: If Shawn also likes the patch, I think this is a stable candidate.
>> > > > >
>> > > > Thanks for the fixing, Koen.
>> > > >
>> > > > Acked-by: Shawn Guo <shawn.guo@xxxxxxxxxxxxx>
>> > >
>> > > 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
