From mboxrd@z Thu Jan  1 00:00:00 1970
From: Simon Glass <sjg@chromium.org>
Date: Mon, 13 Feb 2017 22:31:38 -0700
Subject: [U-Boot] of-platdata address map decode for 64-bits
In-Reply-To: <58A16A94.2030406@rock-chips.com>
References: <587F0D35.6070608@rock-chips.com>
 <CAPnjgZ1ukVVQ1Zg_M14hkwfs2qc5y=Ja5NMSYAhuWL3vxp+JzA@mail.gmail.com>
 <5896723D.3040909@rock-chips.com>
 <CAPnjgZ2=BTLB6zitjsixeDPP8GOkN6GYL1t3DRL_n-RgEsQR4g@mail.gmail.com>
 <58A16A94.2030406@rock-chips.com>
Message-ID: <CAPnjgZ1SEWejVLXqdkNttUem3_ypZNB4q5Bt+Zp3Mzx98OEsTg@mail.gmail.com>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

Hi Kever,

On 13 February 2017 at 01:13, Kever Yang <kever.yang@rock-chips.com> wrote:
> Hi Simon,
>
> On 02/08/2017 01:10 PM, Simon Glass wrote:
>>
>> Hi Kever,
>>
>> On 4 February 2017 at 16:30, Kever Yang <kever.yang@rock-chips.com> wrote:
>>>
>>> Hi Simon,
>>>
>>>      For rk3399, the data for sdram driver in dts is big, I don't want to
>>> do
>>> the copy for it,
>>
>> Are you referring to the call to fdtdec_get_int_array()? Don't you
>> have heaps of SRAM though? So why does it matter? If you are really
>> worried about it you could loop through the data word by word instead.
>
>
> I don't understand how to "loop through the data word by word".

u32 *ptr;
u32 *reg = <address of memory controller>

ptr = fdt_getprop(...., &len);
len /= sizeof(u32);
for (i = 0; i < len)
   writel(fdt32_to_cpu(ptr[i]), reg[i]);


>>
>> But why?
>>
>>> so I think use of-platdata is a good method, and for image size, its
>>> smaller, then it's sooner to load.
>>
>> Yes it reduces the image size, but only by a small amount. I have not
>> checked it on ARM64 but it should be around 5KB. I don't think it is
>> very important.
>
>
> Here is why I insist on no more extra operation and extra code in SPL,
> - The CPU in bootrom usually running in pretty low frequency, the copy may
> be slow;
> - The eMMC in bootrom is running in pretty low frequency, the read is slow;
> - Not like kernel or U-Boot, I don't think the dts values much for SPL,
>     Most of driver used in SPL is one time init, and may totally different
> in
> different SoC, including clock/cru driver, pinctrl/grf driver, dram driver,
> only
> storage driver is shared in different SoC and different boot stage.
> IMO, the SPL suppose to be simple, fast to init dram and load next stage
> image(if
> we do not go back to bootrom).
>
> For production, I have do many times to speed up the boot duration in
> different SoCs,
> so I hope the upstream U-Boot for Rockchip SoC is not only for developers.
> We should
> care about the size even if the size is enough for boot, the size design is
> not only consider
> the boot, but also for some application in system suspend which dram
> suspend.

I am very much in agreement about your focus on boot time. I have no
interest in having a rockchip upstream U-Boot that is anything less
than the best in terms of boot time.

However, does it really matter? Can you benchmark the boot time with
and without of-platdata and show me that the difference is
significant?

So often I see optimisation of things that don't really matter.
Remember that of-platdata is going to be a pretty significant burden
on your development, so it needs to be worth it.

That said, in the meantime, let's continue with this path
(of-platdata). We can decide whether to enable it or disable it based
on the results.

Regards,
Simon


>
> Thanks,
> - Kever
>
>>
>> Regards,
>> Simon
>>
>>
>>> Thanks,
>>> - Kever
>>>
>>> On 01/26/2017 10:22 PM, Simon Glass wrote:
>>>>
>>>> Hi Kever,
>>>>
>>>> On 17 January 2017 at 23:37, Kever Yang <kever.yang@rock-chips.com>
>>>> wrote:
>>>>>
>>>>> Hi Simon,
>>>>>
>>>>>       For my rk3399(64bit) dts, the address is described as below:
>>>>> #address-cells = <2>;
>>>>> reg = <0x0 0xfe320000 0x0 0x4000>;
>>>>>
>>>>> not like 32-bit soc which reg address describe like:
>>>>> reg = <0xfe320000 0x4000>;
>>>>>
>>>>>       There should have some update for this dts decode, right?
>>>>> like:
>>>>> map_sysmem()
>>>>> regmap_init_mem_platdata()
>>>>>
>>>>>       Do you have a idea to update all these kind of function call?
>>>>> It will be very great if you can send a patch for it.
>>>>>
>>>>> Thanks,
>>>>> - Kever
>>>>>
>>>> Yes I think so.
>>>>
>>>> But first I'd like to understand why we need to use of-platdata on
>>>> this very modern SoC. Can you tell me the maximum SPL size you can
>>>> support on this SoC? I thought you had removed the 30KB limit?
>>>>
>>>> Using of-platdata only saves 3-4KB.
>>>>
>>>> Regards,
>>>> Simon
>>>>
>>>>
>>>>
>>>
>>
>>
>
>