From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kever Yang Date: Mon, 25 Jul 2016 15:34:02 +0800 Subject: [U-Boot] [PATCH 2/2] mmc: rockchip: add clock init In-Reply-To: <57959F18.1090609@samsung.com> References: <1469422259-16952-1-git-send-email-kever.yang@rock-chips.com> <1469422259-16952-3-git-send-email-kever.yang@rock-chips.com> <57959F18.1090609@samsung.com> Message-ID: <5795C0EA.2060704@rock-chips.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de 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 >> --- >> >> 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 >> +#include >> #include >> #include >> #include >> @@ -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 >> >> > > >