Re: [PATCH v5 4/9] mmc: dw_mmc: lookup for optional biu and ciu clocks

[Thomas Abraham <thomas.abraham@linaro.org>]

On 31 August 2012 13:29, Jaehoon Chung <jh80.chung@samsung.com> wrote:
>>>>>> +
>>>>>> +     host->ciu_clk = clk_get(host->dev, "ciu");
>>>>>> +     if (IS_ERR(host->ciu_clk))
>>>>>> +             dev_dbg(host->dev, "ciu clock not available\n");
>>>>>> +     else
>>>>>> +             clk_prepare_enable(host->ciu_clk);
>>>>>> +
>>>>>> +     if (IS_ERR(host->ciu_clk))
>>>>>> +             host->bus_hz = host->pdata->bus_hz;
>>>>>> +     else
>>>>>> +             host->bus_hz = clk_get_rate(host->ciu_clk);
>>>>> if clk_get_rate() is incorrect value(ex,400MHz),
>>>>> then mmc->f_min value is too high.
>>>>> because mmc->f_min is assigned to DIV_ROUND_UP(host->bus_hz, 510) into dw_mc_init_slot.
>>>>> Do you have any opinion for solving this?
>>>>
>>>> One option on Exynos5250 is to use the clock divider in the CLKSEL
>>>> register to divide the ciu clock to a lower value. For Exynos4, since
>>>> there is no clock divider in CLKSEL register, the platform code should
>>>> ensure that the ciu clock has a valid range.
>>> I know that can use div-ratio filed at the clksel register.
>>> On Exynos5, i known that is used the div-ratio at CLKSEL register.
>>> If ciu-clock is 400MHz, host->bus_hz is assigned to 400MHz.
>>> 1) host->bus_hz -> 400MHz (at dw-mmc-pltfm.c)
>>> 2) mmc->f_min = DIV_ROUND_UP(host->bus_hz, 510) then mmc->f_min is set to 784KHz.
>>> 3) then host->bus_hz is re-assigned to value that is divided to div-ratio at CLKSEL register.
>>>    at this time, host->bus_hz = 100MHz...
>>>
>>> I think this sequence is something wrong.
>>> (Is 784KHz too high for init card?)
>>>
>>> It's just my thinking..if my understanding is wrong, let me know plz.
>>
>> You have listed the steps 1 to 3 correctly. So, as per step 3, 100Mhz
>> / 510 ~= 196KHz. Which is well within 400KHz. So do you still see a
>> problem here?
> How do you think about this?
> [ 4.620000] mmc_host mmc0: Bus speed (slot 0) = 100000000Hz (slot req 784314Hz, actual 781250HZ div = 64)

You are right, the sequence is wrong. The host->bus_hz is set to the
correct value of 100Mhz in set_ios callback of exynos. But much prior
to that, the dw_mci_init_slot is called and that initializes
mmc->f_min to host->bus_hz / 510 = 400Mhz / 510 = 768KHz.

To fix this, I am planning to add another implementation specific
callback function that will adjust the host->bus_clk as per
implementation specific extensions. This callback will be called right
from dw_mci_probe right after the host->bus_hz is set using the
clk_get_rate() call.

Sorry, I was not paying proper attention to the log messages. Thanks
for your correction.

Regards,
Thomas.