On 11/02/2018 03:19 AM, Masahiro Yamada wrote:
> On Thu, Nov 1, 2018 at 8:38 PM Masahiro Yamada
> <yamada.masah...@socionext.com> wrote:
>>
>> On Thu, Nov 1, 2018 at 6:39 PM Marek Vasut <marek.va...@gmail.com> wrote:
>>>
>>> Switch the driver to using clk_get_rate()/clk_set_rate() instead of
>>> caching the mclk frequency in it's private data. This is required on
>>> the SDHI variant of the controller, where the upstream mclk need to
>>> be adjusted when using UHS modes.
>>
>> No.
>>
>> What you need to do is move tmio_sd_set_clk_rate()
>> to a platform hook.
>>
>> See Linux commit 0196c8db8363f7627df6f78615271ae0ba430500
>>
>>
>>
>>
>>> Platforms which do not support clock framework or do not support it
>>> in eg. SPL default to 100 MHz clock.
>>
>> This is bad.
>> You never know whether the default clock is 100 MHz or not.
>>
>>
>>
>>> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
>>> Cc: Masahiro Yamada <yamada.masah...@socionext.com>
>>> ---
>>> V2: Fix build on certain platforms using SPL without clock framework
>>> ---
>>> drivers/mmc/renesas-sdhi.c | 14 ++++++--------
>>> drivers/mmc/tmio-common.c | 21 ++++++++++++++++++---
>>> drivers/mmc/tmio-common.h | 2 +-
>>> drivers/mmc/uniphier-sd.c | 13 +++++--------
>>> 4 files changed, 30 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/mmc/renesas-sdhi.c b/drivers/mmc/renesas-sdhi.c
>>> index f8dc5f57ce..1590f496d7 100644
>>> --- a/drivers/mmc/renesas-sdhi.c
>>> +++ b/drivers/mmc/renesas-sdhi.c
>>> @@ -333,7 +333,6 @@ static int renesas_sdhi_probe(struct udevice *dev)
>>> struct tmio_sd_priv *priv = dev_get_priv(dev);
>>> u32 quirks = dev_get_driver_data(dev);
>>> struct fdt_resource reg_res;
>>> - struct clk clk;
>>> DECLARE_GLOBAL_DATA_PTR;
>>> int ret;
>>>
>>> @@ -350,22 +349,21 @@ static int renesas_sdhi_probe(struct udevice *dev)
>>> quirks |= TMIO_SD_CAP_16BIT;
>>> }
>>>
>>> - ret = clk_get_by_index(dev, 0, &clk);
>>> + ret = clk_get_by_index(dev, 0, &priv->clk);
>>> if (ret < 0) {
>>> dev_err(dev, "failed to get host clock\n");
>>> return ret;
>>> }
>>>
>>> /* set to max rate */
>>> - priv->mclk = clk_set_rate(&clk, ULONG_MAX);
>>> - if (IS_ERR_VALUE(priv->mclk)) {
>>> + ret = clk_set_rate(&priv->clk, 200000000);
>>> + if (ret < 0) {
>>> dev_err(dev, "failed to set rate for host clock\n");
>>> - clk_free(&clk);
>>> - return priv->mclk;
>>> + clk_free(&priv->clk);
>>> + return ret;
>>> }
>>>
>>> - ret = clk_enable(&clk);
>>> - clk_free(&clk);
>>> + ret = clk_enable(&priv->clk);
>>> if (ret) {
>>> dev_err(dev, "failed to enable host clock\n");
>>> return ret;
>>> diff --git a/drivers/mmc/tmio-common.c b/drivers/mmc/tmio-common.c
>>> index 5f927c6150..611fc5fdbc 100644
>>> --- a/drivers/mmc/tmio-common.c
>>> +++ b/drivers/mmc/tmio-common.c
>>> @@ -551,16 +551,28 @@ static void tmio_sd_set_ddr_mode(struct tmio_sd_priv
>>> *priv,
>>> tmio_sd_writel(priv, tmp, TMIO_SD_IF_MODE);
>>> }
>>>
>>> +static ulong tmio_sd_clk_get_rate(struct tmio_sd_priv *priv)
>>> +{
>>> +#if CONFIG_IS_ENABLED(CLK)
>>> + return clk_get_rate(&priv->clk);
>>> +#else
>>> + return 100000000;
>>> +#endif
>>> +}
>>> static void tmio_sd_set_clk_rate(struct tmio_sd_priv *priv,
>>> struct mmc *mmc)
>>> {
>>> unsigned int divisor;
>>> u32 val, tmp;
>>> + ulong mclk;
>>>
>>> if (!mmc->clock)
>>> return;
>>>
>>> - divisor = DIV_ROUND_UP(priv->mclk, mmc->clock);
>>> + mclk = tmio_sd_clk_get_rate(priv);
>>> +
>>> + divisor = DIV_ROUND_UP(mclk, mmc->clock);
>>>
>>> if (divisor <= 1)
>>> val = (priv->caps & TMIO_SD_CAP_RCAR) ?
>>> @@ -704,6 +716,7 @@ int tmio_sd_probe(struct udevice *dev, u32 quirks)
>>> struct tmio_sd_priv *priv = dev_get_priv(dev);
>>> struct mmc_uclass_priv *upriv = dev_get_uclass_priv(dev);
>>> fdt_addr_t base;
>>> + ulong mclk;
>>> int ret;
>>>
>>> base = devfdt_get_addr(dev);
>>> @@ -744,10 +757,12 @@ int tmio_sd_probe(struct udevice *dev, u32 quirks)
>>>
>>> tmio_sd_host_init(priv);
>>>
>>> + mclk = tmio_sd_clk_get_rate(priv);
>>> +
>>> plat->cfg.voltages = MMC_VDD_165_195 | MMC_VDD_32_33 |
>>> MMC_VDD_33_34;
>>> - plat->cfg.f_min = priv->mclk /
>>> + plat->cfg.f_min = mclk /
>>> (priv->caps & TMIO_SD_CAP_DIV1024 ? 1024 : 512);
>>> - plat->cfg.f_max = priv->mclk;
>>> + plat->cfg.f_max = mclk;
>>> plat->cfg.b_max = U32_MAX; /* max value of TMIO_SD_SECCNT */
>>>
>>> upriv->mmc = &plat->mmc;
>>> diff --git a/drivers/mmc/tmio-common.h b/drivers/mmc/tmio-common.h
>>> index 792b1ba5ae..fcdee4df57 100644
>>> --- a/drivers/mmc/tmio-common.h
>>> +++ b/drivers/mmc/tmio-common.h
>>> @@ -117,7 +117,7 @@ struct tmio_sd_plat {
>>>
>>> struct tmio_sd_priv {
>>> void __iomem *regbase;
>>> - unsigned long mclk;
>>> + struct clk clk;
>>> unsigned int version;
>>> u32 caps;
>>> #define TMIO_SD_CAP_NONREMOVABLE BIT(0) /* Nonremovable e.g. eMMC */
>>> diff --git a/drivers/mmc/uniphier-sd.c b/drivers/mmc/uniphier-sd.c
>>> index 813c28494c..870d1c9929 100644
>>> --- a/drivers/mmc/uniphier-sd.c
>>> +++ b/drivers/mmc/uniphier-sd.c
>>> @@ -38,28 +38,25 @@ static int uniphier_sd_probe(struct udevice *dev)
>>> struct clk clk;
>>> int ret;
>>>
>>> - ret = clk_get_by_index(dev, 0, &clk);
>>> + ret = clk_get_by_index(dev, 0, &priv->clk);
>>> if (ret < 0) {
>>> dev_err(dev, "failed to get host clock\n");
>>> return ret;
>>> }
>>>
>>> /* set to max rate */
>>> - priv->mclk = clk_set_rate(&clk, ULONG_MAX);
>>> - if (IS_ERR_VALUE(priv->mclk)) {
>>> + ret = clk_set_rate(&priv->clk, ULONG_MAX);
>>> + if (ret < 0) {
>>> dev_err(dev, "failed to set rate for host clock\n");
>>> - clk_free(&clk);
>>> - return priv->mclk;
>>> + clk_free(&priv->clk);
>>> + return ret;
>>> }
>>>
>>> ret = clk_enable(&clk);
>>> - clk_free(&clk);
>>> if (ret) {
>>> dev_err(dev, "failed to enable host clock\n");
>>> return ret;
>>> }
>>> -#else
>>> - priv->mclk = 100000000;
>>> #endif
>>>
>>> return tmio_sd_probe(dev, 0);
>
>
> Finally, I figure out why this patch breaks my boards.
>
>
>
>
> diff --git a/drivers/mmc/uniphier-sd.c b/drivers/mmc/uniphier-sd.c
> index 870d1c9..d8eca30 100644
> --- a/drivers/mmc/uniphier-sd.c
> +++ b/drivers/mmc/uniphier-sd.c
> @@ -35,7 +35,6 @@ static int uniphier_sd_probe(struct udevice *dev)
> {
> struct tmio_sd_priv *priv = dev_get_priv(dev);
> #ifndef CONFIG_SPL_BUILD
> - struct clk clk;
> int ret;
>
> ret = clk_get_by_index(dev, 0, &priv->clk);
> @@ -52,7 +51,7 @@ static int uniphier_sd_probe(struct udevice *dev)
> return ret;
> }
>
> - ret = clk_enable(&clk);
> + ret = clk_enable(&priv->clk);
> if (ret) {
> dev_err(dev, "failed to enable host clock\n");
> return ret;
Thanks, added.
I'll also turn the clk_get_rate() into a callback and implement it
differently on socionext and renesas platforms to avoid polluting the
core code.
--
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
