[U-Boot] [PATCH 2/2] mmc: rockchip: add clock init

[Kever Yang]

Hi Jaehoon Chung,

     Thanks for your review comment.

On 07/25/2016 01:09 PM, Jaehoon Chung wrote:
> Hi Kever,
>
> On 07/25/2016 01:50 PM, Kever Yang wrote:
>> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
>> ---
>>
>>   drivers/mmc/rockchip_sdhci.c    | 10 +++++++++-
>>   include/configs/rk3399_common.h |  2 +-
>>   2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mmc/rockchip_sdhci.c b/drivers/mmc/rockchip_sdhci.c
>> index 023c29b..a761a86 100644
>> --- a/drivers/mmc/rockchip_sdhci.c
>> +++ b/drivers/mmc/rockchip_sdhci.c
>> @@ -7,6 +7,7 @@
>>    */
>>   
>>   #include <common.h>
>> +#include <clk.h>
>>   #include <dm.h>
>>   #include <fdtdec.h>
>>   #include <libfdt.h>
>> @@ -32,15 +33,22 @@ static int arasan_sdhci_probe(struct udevice *dev)
>>   	struct rockchip_sdhc_plat *plat = dev_get_platdata(dev);
>>   	struct rockchip_sdhc *prv = dev_get_priv(dev);
>>   	struct sdhci_host *host = &prv->host;
>> +	struct clk clk;
>>   	int ret;
>>   	u32 caps;
>>   
>>   	host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
>>   	host->quirks = SDHCI_QUIRK_WAIT_SEND_CMD;
>>   
>> +	ret = clk_get_by_index(dev, 0, &clk);
>> +	if (ret < 0)
>> +		debug("%s get clock fail\n", __func__);
>> +
>> +	ret = clk_set_rate(&clk, CONFIG_ROCKCHIP_SDHCI_MAX_FREQ);
> Even if failed to get the clk, clk_set_rate() is called...right?
>

How about using a else branch for get clock return value?

>> +
>>   	caps = sdhci_readl(host, SDHCI_CAPABILITIES);
>>   	ret = sdhci_setup_cfg(&plat->cfg, dev->name, host->bus_width,
>> -			caps, CONFIG_ROCKCHIP_SDHCI_MAX_FREQ, EMMC_MIN_FREQ,
>> +			caps, ret, EMMC_MIN_FREQ,
> Don't use "ret"..use the meaningful variable for maximum clock.

  OK, will use another variate like max_freq.

>
>>   			host->version, host->quirks, 0);
>>   
>>   	host->mmc = &plat->mmc;
>> diff --git a/include/configs/rk3399_common.h b/include/configs/rk3399_common.h
>> index 6ce1aa7..eacbe4b 100644
>> --- a/include/configs/rk3399_common.h
>> +++ b/include/configs/rk3399_common.h
>> @@ -32,7 +32,7 @@
>>   #define CONFIG_GENERIC_MMC
>>   #define CONFIG_SDHCI
>>   #define CONFIG_BOUNCE_BUFFER
>> -#define CONFIG_ROCKCHIP_SDHCI_MAX_FREQ	200000000
>> +#define CONFIG_ROCKCHIP_SDHCI_MAX_FREQ	(198*1000000)
> Why change from 200MHz to 198MHz?

We are using a source for sdhci which clock rate is 594MHz, so we actually
get 198MHz in this sdhci driver clk_set_rate().
This change is not a must, but it may help people know the real frequency?


Thanks,
- Kever
>
> Best Regards,
> Jaehoon Chung
>
>>   
>>   #define CONFIG_FAT_WRITE
>>   
>>
>
>
>