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

[Simon Glass]

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?

[...]

>>> +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.

Regards,
Simon