[U-Boot] [PATCH 06/13] rockchip: rk3066: add core support

[Simon Glass]

Hi Pawel,

On 9 June 2017 at 07:03, Paweł Jarosz <paweljarosz3691 at gmail.com> wrote:
>
>
> W dniu 09.06.2017 o 14:27, Simon Glass pisze:
>
>> Hi Pawel,
>>
>> On 7 June 2017 at 09:22, Paweł Jarosz <paweljarosz3691 at gmail.com> wrote:
>>>
>>> Hi Simon
>>>
>>>
>>> W dniu 06.06.2017 o 23:10, Simon Glass pisze:
>>>>
>>>> Hi Pawel,
>>>>
>>>> On 6 June 2017 at 12:50, Paweł Jarosz <paweljarosz3691 at gmail.com> wrote:
>>>>>
>>>>> +#include <dm/test.h>
>>>>> +#include <dm/util.h>
>>>>> +#include <power/regulator.h>
>>>>> +#include <syscon.h>
>>>>
>>>> That should go below spi.h
>>>>
>>> Ok
>>>>>
>>>>> +
>>>>> +DECLARE_GLOBAL_DATA_PTR;
>>>>> +
>>>>> +u32 spl_boot_device(void)
>>>>> +{
>>>>
>>>> This function seems to be common. Can you please (in a separate patch)
>>>> move it into a separate shared file and delete all the copies?
>>>
>>> Ok
>>>>
>>>>
>>>>> +
>>>>> +       sdram_initialise();
>>>>
>>>> How come TPL is setting up the DRAM? Shouldn't that be done in SPL?
>>>>
>>> Due to size issues i needed to move spl load to sdram (not sram) and
>>> sdram
>>> settings
>>> should be done by code executed in SRAM or you get to hang a board.
>>
>> What is the size limit on SRAM?
>
>
> RK3066 has 64KB SRAM but bootrom only reads 1KB + 31KB.

That is not a very low limit. RK3288 uses SPL (and not TPL) and it has
the same limit. Can we not just drop the use of TPL?

>
>> [...]
>>
>>>>> +config SYS_SOC
>>>>> +        default "rockchip"
>>>>> +
>>>>> +config SYS_MALLOC_F_LEN
>>>>> +        default 0x0800
>>>>> +
>>>>> +config SPL_LIBCOMMON_SUPPORT
>>>>
>>>> You should be able to add these as 'imply' options now (to the
>>>> ROCKCHIP_RK3066 config)
>>>>
>>> What do you mean by 'imply' options?
>>
>> Things like:
>>
>> config ROCKCHIP_RK3288
>>     bool "Support Rockchip RK3288"
>>     select CPU_V7
>>     select SPL_BOARD_INIT if SPL
>>     select SUPPORT_SPL
>>     select SPL
>>     imply SPL_LIBCOMMON_SUPPORT
>>
>> Either select or imply cause that option to be enabled.
>>
>>>>> +#define CONFIG_SPL_FRAMEWORK           1
>>>>> +#define CONFIG_SPL_CLK                 1
>>>>> +#define CONFIG_SPL_PINCTRL             1
>>>>> +#define CONFIG_SPL_REGMAP              1
>>>>
>>>> Many of these should be set in Kconfig or the defconfig file.
>>>>
>>> Yes but some have gave me tpl build issues that's why i did that here
>>
>> We need to figure out if TPL is really needed. E.g. I don't think
>> kylin_rk3036 needs it. It produces an SPL that is about 4KB.
>
>
> RK3036 has a different bootrom load process so we shouldn't compare those
> two.
> Better is to compare RK3066 and RK3188 as those are similiar(RK3188 is
> without 200KB limit i think)

OK, still I don't understand why SPL cannot work.

>
> Paweł
>>
>> Regards,
>> Simon
>
>
Regards,
Simon