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

[Kever Yang]

Hi Philipp, Andreas,


On 08/25/2018 12:00 AM, Dr. Philipp Tomsich wrote:
>> 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…
'pre-reloc' is used in SPL and U-Boot before relocate, I think this
should be
clear to be as simple as possible, but now it's going to much like U-Boot
proper. It have 2 problem: code size and boot time．
Every feature will increase the code size, in Rockchip platform, we run SPL
in sram which have size limit, and the common code of upstream feature
update often break the size limit. In this case I want to add TPL for
all Rockchip
SoCs and used for ddr init only so that SPL will not have the size limit.
For boot time, you need to understand that the module tag as 'pre-reloc'
will
be binded twice in U-Boot proper and also twice in SPL. I hope we only
do things
we need for SPL and U-Boot proper, but not too much overhead by framework.
The API like clk_set_defaults() is able to used in every module, better
to use in
U-Boot Proper while the SPL should have a clear white list modules.

For example, we get everything from DTS for ddr node, but not using the
clk_set_defaults().

What's the sram size in your SoC?

Thanks,
- Kever
> 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.
>
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> https://lists.denx.de/listinfo/u-boot