I've attached the test program I wrote to compare the different
flavors of CLKDIV computation: old (3.4 kernel), current upstream, and
my rewrite.
thanks
grant
On Tue, Mar 26, 2013 at 3:50 PM, Grant Grundler <grundler@xxxxxxxxxxxx> wrote:
> Last year Seungwon Jeon (Samsung) fixed a bug in CLKDIV computation.
> But when debugging a related issue (http://crbug.com/221828) I found
> 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
>
#include <stdio.h>
/* from include/linux/kernel.h */
#define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
unsigned int original(unsigned int bus_hz, unsigned int clock)
{
unsigned int div;
if (bus_hz % clock)
/*
* move the + 1 after the divide to prevent
* over-clocking the card.
*/
div = ((bus_hz / clock) >> 1) + 1;
else
div = (bus_hz / clock) >> 1;
return div;
}
unsigned int upstream(unsigned int bus_hz, unsigned int clock)
{
unsigned int div;
div = bus_hz / clock;
if (bus_hz % clock && bus_hz > clock)
/*
* move the + 1 after the divide to prevent
* over-clocking the card.
*/
div += 1;
div = (bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0;
return div;
}
unsigned int ggg(unsigned int bus_hz, unsigned int clock)
{
unsigned int div;
div = bus_hz / clock;
if (bus_hz > clock) {
/* don't overclock due to integer math losses */
if ((div * clock) < 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, value 1 -> 2, val 2 -> 4 etc.
*/
div /= 2;
} else
div = 0; /* use bus_hz */
return div;
}
unsigned int bus_hz_tbl[] = { 13, 17, 19, 20, 50000000, 100000000};
unsigned int slot_hz_tbl[] = { 10, 13, 21, 40, 57, 400000, 784314, 52000000};
static unsigned int div_to_hz(unsigned int bus_hz, unsigned int d)
{
return d ? (bus_hz/(d*2)) : bus_hz;
}
static void verify_hz(unsigned int bus_hz, unsigned int requested_hz,
unsigned int divided_hz, const char *algo_name)
{
if (divided_hz > bus_hz)
printf(" [FAIL: %s > bus hz!]", algo_name);
if (divided_hz > requested_hz)
printf(" [FAIL: %s > slot hz!]", algo_name);
}
void main(void)
{
unsigned int i, j;
printf("bus/slot hz Original Upstream GGG\n");
for(i=0; i < sizeof(bus_hz_tbl)/sizeof(unsigned int); i++) {
unsigned int bus_hz = bus_hz_tbl[i];
for (j=0; j < sizeof(slot_hz_tbl)/sizeof(unsigned int); j++) {
unsigned int slot_hz = slot_hz_tbl[j];
unsigned int x = original(bus_hz, slot_hz);
unsigned int y = upstream(bus_hz, slot_hz);
unsigned int z = ggg(bus_hz, slot_hz);
unsigned int hz_x, hz_y, hz_z;
hz_x = div_to_hz(bus_hz, x);
hz_y = div_to_hz(bus_hz, y);
hz_z = div_to_hz(bus_hz, z);
printf("%8d/%6d %7d %6d %7d %6d %7d %6d",
bus_hz, slot_hz, x, hz_x, y, hz_y, z, hz_z);
verify_hz(bus_hz, slot_hz, hz_x, "Original");
verify_hz(bus_hz, slot_hz, hz_y, "Upstream");
verify_hz(bus_hz, slot_hz, hz_z, "GGG");
if (x != y)
#if 0
/* We know original (3.4 kernel) version was buggy.
* Don't have to be quite so loud about it.
*/
printf("DEBUG: Orig != Upstream\n",
bus_hz, slot_hz);
#else
printf(" [Orig Fixed]");
#endif
if (z != y)
printf(" WARN: GGG != Upstream");
printf("\n");
}
}
}
