[U-Boot] [RESEND PATCH v2] mmc: rockchip_sdhci: add clock init for mmc

[Jaehoon Chung]

On 12/28/2016 10:45 AM, Kever Yang wrote:
> Hi Jaehoon,
> 
> On 12/28/2016 09:34 AM, Jaehoon Chung wrote:
>> Hi Kever,
>>
>> On 12/28/2016 10:12 AM, Kever Yang wrote:
>>> Hi Jaehoon,
>>>
>>> On 12/28/2016 06:08 AM, Jaehoon Chung wrote:
>>>> Hi Kever,
>>>>
>>>> On 12/27/2016 10:09 PM, Kever Yang wrote:
>>>>> Init the clock rate to CONFIG_ROCKCHIP_SDHCI_MAX_FREQ with clock driver
>>>>> api.
>>>>>
>>>>> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
>>>>> ---
>>>>>
>>>>> Changes in v2:
>>>>> - using the return value
>>>>>
>>>>>    drivers/mmc/rockchip_sdhci.c | 11 +++++++++++
>>>>>    1 file changed, 11 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
>>>>> index c56e1a3..96049f3 100644
>>>>> --- a/drivers/mmc/rockchip_sdhci.c
>>>>> +++ b/drivers/mmc/rockchip_sdhci.c
>>>>> @@ -12,6 +12,7 @@
>>>>>    #include <libfdt.h>
>>>>>    #include <malloc.h>
>>>>>    #include <sdhci.h>
>>>>> +#include <clk.h>
>>>>>      /* 400KHz is max freq for card ID etc. Use that as min */
>>>>>    #define EMMC_MIN_FREQ    400000
>>>>> @@ -33,6 +34,16 @@ static int arasan_sdhci_probe(struct udevice *dev)
>>>>>        struct rockchip_sdhc *prv = dev_get_priv(dev);
>>>>>        struct sdhci_host *host = &prv->host;
>>>>>        int ret;
>>>>> +    struct clk clk;
>>>>> +
>>>>> +    ret = clk_get_by_index(dev, 0, &clk);
>>>>> +    if (!ret) {
>>>>> +        ret = clk_set_rate(&clk, CONFIG_ROCKCHIP_SDHCI_MAX_FREQ);
>>>> How about getting clock value from dt?
>>> SDHCI dts node is like below:
>>>          sdhci: sdhci at fe330000 {
>>>                  u-boot,dm-pre-reloc;
>>>                  compatible = "rockchip,rk3399-sdhci-5.1", "arasan,sdhci-5.1";
>>>                  reg = <0x0 0xfe330000 0x0 0x10000>;
>>>                  interrupts = <GIC_SPI 11 IRQ_TYPE_LEVEL_HIGH>;
>>>                  assigned-clocks = <&cru SCLK_EMMC>;
>>>                  assigned-clock-rates = <200000000>;
>>>                  clocks = <&cru SCLK_EMMC>, <&cru ACLK_EMMC>;
>>>                  clock-names = "clk_xin", "clk_ahb";
>>>                  phys = <&emmc_phy>;
>>>                  phy-names = "phy_arasan";
>>>                  status = "disabled";
>>>          };
>>>
>>> There is an assigned-clock-rates, which is parsed by clock driver in kernel,
>>> but the U-Boot clk driver do not have this feature, is it OK for us to parse it in sdhci driver?
>> Yes, Maybe it's possible to parse it in sdhci driver.
>> (assigned-clock-rate might be for assigning to initial source clock value.)
>> In Linux kernel, there is also similar property like "max-frequency".
>> MMC can have the difference maximum frequency for each SoC.
>>
>> I'm considering that implement the generic mmc parsing function.
>> Because some property can be used commonly.
>>
>> If you put the parser in sdhci driver, i will rework it into generic parsing function in future.
>> How about? :)
> 
> Look into another sdmmc dts node(dwmmc), I can found clk-freq-min-max, I think this is more generic for add the SDHCI driver, right?

Yes, likes them.

>         sdmmc: dwmmc at fe320000 {
>                 compatible = "rockchip,rk3399-dw-mshc",
>                              "rockchip,rk3288-dw-mshc";
>                 reg = <0x0 0xfe320000 0x0 0x4000>;
>                 interrupts = <GIC_SPI 65 IRQ_TYPE_LEVEL_HIGH>;
>                 clock-freq-min-max = <400000 150000000>;
>                 clocks = <&cru SCLK_SDMMC>, <&cru HCLK_SDMMC>,
>                          <&cru SCLK_SDMMC_DRV>, <&cru SCLK_SDMMC_SAMPLE>;
>                 clock-names = "ciu", "biu", "ciu-drive", "ciu-sample";
>                 pinctrl-names = "default";
>                 pinctrl-0 = <&sdmmc_clk>;
>                 fifo-depth = <0x100>;
>                 status = "disabled";
>         };
> 
> The U-Boot dwmmc driver will set the clock via system clock API, but SDHCI driver do not do that,
> that's why I have to init the clock in rockchip_sdhci.

Now, I think that it's enough to put the parsing code in your rockchip_sdhci.c, isn't?

arasan_sdhci_probe() {
..
parsing "max-frequency"
clk_get_by_index();
clk_set_rate();
..

}

Is it working?

> 
> I hope everything are working like kernel, you also working on kernel mmc driver, right?

Yes, i'm working on also kernel side. I also hope so. :)

Best Regards,
Jaehoon Chung

> 
> Thanks,
> - Kever
> 
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>> Thanks,
>>> - Kever
>>>
>>>> Best Regards,
>>>> Jaehoon Chung
>>>>
>>>>> +        if (IS_ERR_VALUE(ret))
>>>>> +            printf("%s clk set rate fail!\n", __func__);
>>>>> +    } else {
>>>>> +        printf("%s fail to get clk\n", __func__);
>>>>> +    }
>>>>>          host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;
>>>>>   
>>>>
>>>>
>>>
>>
>>
>>
> 
> 
> 
> 
>