[U-Boot] [PATCH] spi: mxc_spi: Fix pre and post divider calculation

Dirk Behme dirk.behme at gmail.com
Sat May 4 12:06:43 CEST 2013


Am 03.05.2013 22:47, schrieb Troy Kisky:
> On 5/2/2013 10:58 PM, Dirk Behme wrote:
>> Do you want to say you propose
>>
>> post_div = pre_div / 16;
>> pre_div = 16;
>>
>> ?
> yes, that's what I said
>>
>> If so:
>>
>> First, I agree that we have to use the same dividers in both lines.
>>
>> But, second, this would mean that you use /16 as max pre_div. For
>> the i.MX6 case where clk_src is 60MHz this would result in a
>> pre-divided clock of 3.75Mhz (instead of 4MHz with /15).
>
> That does sound better for i.MX6, what about other processors using
> this file?
>
>>
>> So using /15 or /16 is just a decision of which end clocks most
>> probably are needed.
>>
>> If you want to be able to configure 4MHz, 2MHz, 1MHz, 500kHz etc
>> then /15 is the better choice.
>>
>> If you want to be able to configure 3.75Mhz, 1.875MHz, 937.5kHz,
>> 468.75kHz etc then /16 is the better choice.
>>
>> I vote for /15 as done by my patch.
>
> Thanks for explaining. The downside of using /15 is that you can't get
> the slowest clock possible.

Yes. I was looking for the _highest_ clock possible, though ;) And 
this isn't correctly done by the recent code. This is why I was 
looking into it ...

> How about restructuring the code to improve both. Calculate post_div
> first.
>
> pre_div = DIV_ROUND_UP(clk_src, max_hz);
> /* fls(1) = 1, fls(0x80000000) = 32, fls(16) = 5 */
> post_div = fls(pre_div - 1);
> if (post_div > 4)
>      post_div -= 4;
> else
>      post_div = 0;
>
> if (post_div >= 16) {
>             printf("Error: no divider for the freq: %d\n",
>                                          max_hz);
>             return -1;
> }
> pre_div = (pre_div + (1 << post_div) - 1) >> post_div;

Using my test code gives the correct values using this algorithm. So 
yes, sounds good.

Just a small note: Wouldn't it be better to put the printf and the 
last line with the pre_div calculation into the if(post_div > 4) part? 
I.e.

pre_div = DIV_ROUND_UP(clk_src, max_hz);
/* fls(1) = 1, fls(0x80000000) = 32, fls(16) = 5 */
post_div = fls(pre_div - 1);
if (post_div > 4) {
     post_div -= 4;

     if (post_div >= 16) {
            printf("Error: no divider for the freq: %d\n",
                                         max_hz);
            return -1;
     }
     pre_div = (pre_div + (1 << post_div) - 1) >> post_div;

} else
     post_div = 0;

?

In case we agree on this, I'm thinking about doing 2 patches to make 
clear what we are doing:

1. Re-doing my initial patch with

post_div = pre_div / 16;
pre_div = 16;

This would be the "fix the issues in the existing (non-optimal) 
algorithm but keep that" patch.

2. Replace the existing algorithm with your above version. This would 
be the "improve the algorithm" patch.

What do you think?

Best regards

Dirk







More information about the U-Boot mailing list