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

[Thomas Abraham]

On 31 August 2012 11:32, Jaehoon Chung <jh80.chung at samsung.com> wrote:
> On 08/31/2012 02:29 PM, Thomas Abraham wrote:
>> On 30 August 2012 14:18, Jaehoon Chung <jh80.chung at samsung.com> wrote:
>>> Hi Thomas,
>>>
>>> On 08/29/2012 07:48 PM, Thomas Abraham wrote:
>>>> Some platforms allow for clock gating and control of bus interface unit clock
>>>> and card interface unit clock. Add support for clock lookup of optional biu
>>>> and ciu clocks for clock gating and clock speed determination.
>>>>
>>>> Signed-off-by: Abhilash Kesavan <a.kesavan at samsung.com>
>>>> Signed-off-by: Thomas Abraham <thomas.abraham at linaro.org>
>>>> Acked-by: Will Newton <will.newton at imgtec.com>
>>>> ---
>>>>  drivers/mmc/host/dw_mmc.c  |   42 +++++++++++++++++++++++++++++++++++++++---
>>>>  include/linux/mmc/dw_mmc.h |    4 ++++
>>>>  2 files changed, 43 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
>>>> index 227c42e..90c7c7b 100644
>>>> --- a/drivers/mmc/host/dw_mmc.c
>>>> +++ b/drivers/mmc/host/dw_mmc.c
>>>> @@ -1960,18 +1960,38 @@ int dw_mci_probe(struct dw_mci *host)
>>>>               return -ENODEV;
>>>>       }
>>>>
>>>> -     if (!host->pdata->bus_hz) {
>>>> +     host->biu_clk = clk_get(host->dev, "biu");
>>>> +     if (IS_ERR(host->biu_clk))
>>>> +             dev_dbg(host->dev, "biu clock not available\n");
>>>> +     else
>>>> +             clk_prepare_enable(host->biu_clk);
>>> biu is clock for bus interface?
>>> if didn't get "biu_clk" or didn't clk_prepare_enable(), then can we initialize the card?
>>
>> Hi Jaehoon,
>>
>> Yes, the biu clock is for bus interface. The biu and ciu clock lookup
>> and enable here is optional in the above change. If a platform does
>> not define these clocks, then the platform code is responsible for
>> enabling these clocks.
> If biu_clk is presented...
> Is there no probability that clk_prepare_enable is failed?

Yes, clk_prepare_enable can fail. I will fix this by setting biu_clk
and ciu_clk to NULL, in case the clk_prepare_enable fails.

>>
>>>> +
>>>> +     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?

Thanks,
Thomas.