Re: [PATCH] mmc: dw_mmc: rewrite CLKDIV computation

[Grant Grundler]

On Wed, Mar 27, 2013 at 5:25 AM, Seungwon Jeon <tgih.jun@xxxxxxxxxxx> wrote:
> On Wednesday, March 27, 2013, Grant Grundler wrote:
>> Last year Seungwon Jeon (Samsung) fixed a bug in CLKDIV computation.
> For easily identifying, it would be good to point the commit id and subject.

commit id will be different for different git trees and branches - not
as useful as one might think.

I will include the following and update the patch:
   Author: Seungwon Jeon <tgih.jun@xxxxxxxxxxx>
   Date:   Tue May 22 13:01:21 2012 +0900
   mmc: dw_mmc: correct the calculation for CLKDIV


>> But when debugging a related issue (http://crbug.com/221828) I found
>
> It is not easy to catch up issue in your site. What problem is bothering you?
> Could you describe the problem and conditions you have found?

The bug is not relevant to this patch - it's a "related bug". I
mentioned the bug only to explain my motivation for looking at this
code. I will move the bug reference out of the commit message.

To summarize, I was trying to backport "mmc: dw_mmc: correct the
calculation for CLKDIV" patch to 3.4 kernel to support faster eMMC
parts since the original computation in 3.4 kernel was returning wrong
CLKDIV value. But then ran into other problems specific to one
firmware combination breaking the eMMC clock settings.

cheers,
grant

>
> Thanks,
> Seungwon Jeon
>
>> the code unreadable.  This rewrite simplifies the computation and
>> explains each step.
>>
>> Signed-off-by: Grant Grundler <grundler@xxxxxxxxxxxx>
>> ---
>> Tested on Samsung Series 3 Chromebook (exynos 5250 chipset) using
>> ChromeOS 3.4 kernel (not 3.9-rc3 which this patch is based against).
>>
>> I've written a test program to confirm this patch generates the
>> same correct values and will share that separately.
>>
>>  drivers/mmc/host/dw_mmc.c | 22 +++++++++++++++-------
>>  1 file changed, 15 insertions(+), 7 deletions(-)
>>
>>
>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>> index 9834221..6fdf55b 100644
>> --- a/drivers/mmc/host/dw_mmc.c
>> +++ b/drivers/mmc/host/dw_mmc.c
>> @@ -631,14 +631,22 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot, bool force_clkinit)
>>
>>       if (slot->clock != host->current_speed || force_clkinit) {
>>               div = host->bus_hz / slot->clock;
>> -             if (host->bus_hz % slot->clock && host->bus_hz > slot->clock)
>> -                     /*
>> -                      * move the + 1 after the divide to prevent
>> -                      * over-clocking the card.
>> +             if (host->bus_hz > slot->clock) {
>> +                     /* don't overclock due to integer math losses */
>> +                     if ((div * slot->clock) < host->bus_hz)
>> +                             div++;
>> +
>> +                     /* don't overclock due to resolution of HW */
>> +                     if (div & 1)
>> +                             div++;
>> +
>> +                     /* See 6.2.3 CLKDIV in "Mobile Storage Host Databook"
>> +                      * Look for dwc_mobile_storage_db.pdf from Synopsys.
>> +                      * CLKDIV value 0 means divisor 1, val 1 -> 2, ...
>>                        */
>> -                     div += 1;
>> -
>> -             div = (host->bus_hz != slot->clock) ? DIV_ROUND_UP(div, 2) : 0;
>> +                     div /= 2;
>> +             } else
>> +                     div = 0;        /* use bus_hz */
>>
>>               dev_info(&slot->mmc->class_dev,
>>                        "Bus speed (slot %d) = %dHz (slot req %dHz, actual %dHZ"
>> --
>> 1.8.1.3
>>
>> --
>> 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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/