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

[Kever Yang]

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

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

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;
>>>>   
>>>
>>>
>>
>
>
>