[U-Boot] [PATCH 11/25] clk: Allow clock defaults to be set also during re-reloc state

[Dr. Philipp Tomsich]

> On 24 Aug 2018, at 17:54, Andreas Dannenberg <dannenberg at ti.com> wrote:
> 
> Philipp,
> 
> On Fri, Aug 24, 2018 at 04:42:15PM +0200, Dr. Philipp Tomsich wrote:
>> +Kever
>> 
>>> On 24 Aug 2018, at 16:12, Tom Rini <trini at konsulko.com> wrote:
>>> 
>>> On Tue, Aug 21, 2018 at 08:01:49PM +0530, Lokesh Vutla wrote:
>>> 
>>>> From: Andreas Dannenberg <dannenberg at ti.com>
>>>> 
>>>> The earlier commit f4fcba5c5ba ("clk: implement clk_set_defaults()")
>>>> which introduced the functionality for setting clock defaults such as
>>>> rates and parents will skip the processing when executing in a re-reloc
>>>> state. This for example can prevent the assigning of clock parents
>>>> when running in SPL code. Go ahead and remove this limitation.
>>>> 
>>>> Signed-off-by: Andreas Dannenberg <dannenberg at ti.com>
>>>> Signed-off-by: Lokesh Vutla <lokeshvutla at ti.com>
>>>> ---
>>>> drivers/clk/clk-uclass.c | 4 ----
>>>> 1 file changed, 4 deletions(-)
>>>> 
>>>> diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c
>>>> index 2b15978e14..04b369aa5a 100644
>>>> --- a/drivers/clk/clk-uclass.c
>>>> +++ b/drivers/clk/clk-uclass.c
>>>> @@ -243,10 +243,6 @@ int clk_set_defaults(struct udevice *dev)
>>>> {
>>>> 	int ret;
>>>> 
>>>> -	/* If this is running pre-reloc state, don't take any action. */
>>>> -	if (!(gd->flags & GD_FLG_RELOC))
>>>> -		return 0;
>>>> -
>>>> 	debug("%s(%s)\n", __func__, dev_read_name(dev));
>>>> 
>>>> 	ret = clk_set_default_parents(dev);
>>> 
>>> Philipp? David?  Comments?  Thanks!
>> 
>> If I remember correctly, David had a concern regarding an increase in
>> boottime if we ran this twice… adding Kever, as he was also involved
>> in the discussion.
>> 
>> I settled on skipping it for pre-reloc, but it’s an imperfect solution: the
>> boottime increase comes from the fact that some devices have a large
>> number of assigned-clocks, that the device-tree processing has a cost,
>> and that we don’t have a way of synchronising between SPL and full
>> U-Boot to avoid redoing the complete init-flow.
> 
> Good to know some of the background; when I did this patch initially it
> was not really clear why this was removed and it obviously was an issue
> for what I was doing that I had to overcome and re-adding this was the
> simpliest thing to do at that time.
> 
>> Maybe we should have a SPL-specific property for the assigned-clocks
>> to be set pre-reloc?
> 
> Need to think about this some more. Generally we probably want to do as
> little as possible before relocation. Unfortunately for the K3 family of
> SoCs much is dependent on loading/installing the system firmware (SYSFW)
> image including to get DDR operational which itself requires us to use a
> lot of DT/DM stuff pre-reloc. So a little bit of a chicken and egg
> problem…

If you need someone to throw your thoughts at, feel free to share them with
me.  With the newer Rockchip devices that I am focused on, we also do a lot
DT/DM stuff pre-reloc (I have a 100+KB SPL stage on the RK3399 at the 
moment) and this trend is likely to increase … e.g. I may split the RK3399
to use TPL for DRAM-init and will then have the SPL stage running from
DRAM which will remove the last remaining size constraints.

--Philipp.