[U-Boot] [PATCH 6/6] arm64: rk3399: add SPL support

[Simon Glass]

Hi Kever,

On 4 February 2017 at 19:01, Kever Yang <kever.yang at rock-chips.com> wrote:
> Hi Simon,
>
>
> On 01/26/2017 10:23 PM, Simon Glass wrote:
>>
>> Hi Kever,
>>
>> On 18 January 2017 at 05:16, Kever Yang <kever.yang at rock-chips.com> wrote:
>>>
>>> Add spl support for rk3399, default with of-platdata enabled.
>>>
>>> Signed-off-by: Kever Yang <kever.yang at rock-chips.com>
>>> ---
>>>
>>>   arch/arm/Kconfig                              |   1 +
>>>   arch/arm/dts/rk3399-evb.dts                   |   2 +
>>>   arch/arm/dts/rk3399.dtsi                      |  44 +++++++
>>>   arch/arm/include/asm/arch-rockchip/clock.h    |   3 +
>>>   arch/arm/mach-rockchip/Kconfig                |   2 +
>>>   arch/arm/mach-rockchip/Makefile               |   1 +
>>>   arch/arm/mach-rockchip/rk3399-board-spl.c     | 158
>>> ++++++++++++++++++++++++++
>>>   arch/arm/mach-rockchip/rk3399/syscon_rk3399.c |  40 +++++++
>>>   configs/evb-rk3399_defconfig                  |  18 +++
>>>   include/configs/rk3399_common.h               |  11 ++
>>>   10 files changed, 280 insertions(+)
>>>   create mode 100644 arch/arm/mach-rockchip/rk3399-board-spl.c
>>
>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>
>> But there is too much in this patch. Please split out the patches. My
>> suggestion:
>>
>> - syscon additions
>> - dts/dtsi additions
>> - arch/arm/Kconfig and include/configs changes
>> - board-spl.c stuff
>> - defconfig changes to enable everything
>>
>> So it should be possible to enable/disable SPL just in the final patch.
>
>
> Cc Tom here.
> I have some confuse for patch split in U-Boot, last time I see a patch set
> to init support
> for other SoC, patches split very detail and almost one patch for one
> module(like your comment
> in this patch), then Tom's comment says there is no need for that detail,
> only one patch for SoC
> and one patch for board is OK.
>
> My understand(for U-Boot) is:
> - driver patch is very clear and should be split out,
> - other parts like dts/defconfig and soc/board for one new SoC support,
>    could be gather in one patch or two if there goes to the same maintainer
> and branch.
>
> The grf definition and clock driver has split out as your comment in my
> 'RFC' version,
> I can split this patch into 5 patches if you still required.

Well let's leave it for now, and keep it in mind for the future. It is
useful to split things up for easier review and also it allows us to
bisect / revert for problems more easily.

Regards,
Simon