[U-Boot] [PATCH] rockchip: clk: rk3368: always run rkclk_init when driver probe

[Kever Yang]

Hi Philipp,


On 08/31/2017 06:25 PM, Andy Yan wrote:
> Hi Philipp:
>
>
> On 2017年08月28日 04:21, Dr. Philipp Tomsich wrote:
>>> On 23 Aug 2017, at 09:26, Andy Yan <andy.yan at rock-chips.com> wrote:
>>>
>>> commit 4bebf94e8544("rockchip: clk: rk3368: do not change
>>> CPLL/GPLL before returning to BROM") limits the pll can only
>>> be setup in SPL stage, but there are still some rk3368 based
>>> boards have not use SPL yet, so they need run rkclk_init to
>>> setup the pll in full u-boot stage, otherwise the clk_set_rate
>>> function will run into wrong logic, because it assume that all
>>> the pll have been set to the desired frequency.
>> If SPL is enabled, rkclk_init should not be called from the full
>> U-Boot, though.  I.e. we’ll need an appropriate conditional
>> test to check whether SPL is enabled or not...
>
>     Do you have any ideas here?

There should have some memory could be pass from SPL (optionally via 
ATF/OPTEE)
to U-Boot, may including information like this,
- hardware module info/status already known in previous stage, like dram 
cap in sysreg,
- the boot device, boot rom boot from SD card, then U-Boot prefer to 
boot from SD card,
- Android O, there is a A/B slot select, if we detect it in SPL, then we 
can pass it to U-Boot
and etc.

Maybe we can have a vendor defined structure locate in a reserved memory 
like boot0hook?

Thanks,
- Kever
>
>
>>
>>> Signed-off-by: Andy Yan <andy.yan at rock-chips.com>
>>> ---
>>>
>>> drivers/clk/rockchip/clk_rk3368.c | 10 ++--------
>>> 1 file changed, 2 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/clk/rockchip/clk_rk3368.c 
>>> b/drivers/clk/rockchip/clk_rk3368.c
>>> index 2be1f57..ee754f0 100644
>>> --- a/drivers/clk/rockchip/clk_rk3368.c
>>> +++ b/drivers/clk/rockchip/clk_rk3368.c
>>> @@ -47,14 +47,12 @@ struct pll_div {
>>>                (_nr * _no) == hz, #hz "Hz cannot be hit with PLL " \
>>>                "divisors on line " __stringify(__LINE__));
>>>
>>> -#if IS_ENABLED(CONFIG_SPL_BUILD) || IS_ENABLED(CONFIG_TPL_BUILD)
>>> static const struct pll_div apll_l_init_cfg = 
>>> PLL_DIVISORS(APLL_L_HZ, 12, 2);
>>> static const struct pll_div apll_b_init_cfg = 
>>> PLL_DIVISORS(APLL_B_HZ, 1, 2);
>>> #if !defined(CONFIG_TPL_BUILD)
>>> static const struct pll_div gpll_init_cfg = PLL_DIVISORS(GPLL_HZ, 1, 
>>> 2);
>>> static const struct pll_div cpll_init_cfg = PLL_DIVISORS(CPLL_HZ, 1, 
>>> 6);
>>> #endif
>>> -#endif
>>>
>>> static ulong rk3368_clk_get_rate(struct clk *clk);
>>>
>>> @@ -85,7 +83,6 @@ static uint32_t rkclk_pll_get_rate(struct 
>>> rk3368_cru *cru,
>>>     }
>>> }
>>>
>>> -#if IS_ENABLED(CONFIG_SPL_BUILD) || IS_ENABLED(CONFIG_TPL_BUILD)
>>> static int rkclk_set_pll(struct rk3368_cru *cru, enum rk3368_pll_id 
>>> pll_id,
>>>              const struct pll_div *div)
>>> {
>>> @@ -125,9 +122,7 @@ static int rkclk_set_pll(struct rk3368_cru *cru, 
>>> enum rk3368_pll_id pll_id,
>>> seamless
>>>     return 0;
>>> }
>>> -#endif
>>>
>>> -#if IS_ENABLED(CONFIG_SPL_BUILD) || IS_ENABLED(CONFIG_TPL_BUILD)
>>> static void rkclk_init(struct rk3368_cru *cru)
>>> {
>>>     u32 apllb, aplll, dpll, cpll, gpll;
>>> @@ -152,7 +147,7 @@ static void rkclk_init(struct rk3368_cru *cru)
>>>     debug("%s apllb(%d) apll(%d) dpll(%d) cpll(%d) gpll(%d)\n",
>>>            __func__, apllb, aplll, dpll, cpll, gpll);
>>> }
>>> -#endif
>>> +
>>>
>>> #if !IS_ENABLED(CONFIG_SPL_BUILD) || CONFIG_IS_ENABLED(MMC_SUPPORT)
>>> static ulong rk3368_mmc_get_clk(struct rk3368_cru *cru, uint clk_id)
>>> @@ -473,9 +468,8 @@ static int rk3368_clk_probe(struct udevice *dev)
>>>
>>>     priv->cru = map_sysmem(plat->dtd.reg[1], plat->dtd.reg[3]);
>>> #endif
>>> -#if IS_ENABLED(CONFIG_SPL_BUILD) || IS_ENABLED(CONFIG_TPL_BUILD)
>>> +
>>>     rkclk_init(priv->cru);
>>> -#endif
>>>
>>>     return 0;
>>> }
>>> -- 
>>> 2.7.4
>>>
>>>
>>
>>
>>
>
>