[U-Boot] [U-Boot, 36/36] rockchip: add common board file for rockchip platform

[Dr. Philipp Tomsich]

Kever,

> On 9 Apr 2018, at 00:35, Tom Rini <trini at konsulko.com> wrote:
> 
> On Sun, Apr 08, 2018 at 09:45:22AM +0800, Kever Yang wrote:
>> Philipp,
>> 
>> 
>> On 04/02/2018 05:28 AM, Philipp Tomsich wrote:
>>> 
>>> 
>>> On Tue, 27 Mar 2018, Kever Yang wrote:
>>> 
>>>> We use common board/spl/tpl file for all rockchip SoCs,
>>>> - all the SoC spec setting should move into SoC file like rk3288.c;
>>>> - tpl is option and only purpose to init DRAM, clock, uart(option);
>>>> - spl do secure relate one time init, boot device select, boot into
>>>>  U-Boot or trust or OS in falcon mode;
>>>> - board do boot mode detect, enable regulator, usb init and so on.
>>> 
>>> There's too much going on in a single commit/single series.
>>> This needs to be split up into multiple, independent steps (e.g. one
>>> for the timer changes, another one for the UART changes)...
>> 
>> I understand review the patches piece by piece is much more comfortable,
>> and this patch including "too much" things. And I never expect this
>> patch set
>> can be merge quickly, but we have to do this ASAP before more SoC coming.
>> I have do a lot of test and re-work in my local branch and at last make
>> it landed in
>> rockchip vendor U-Boot, with testing in most of SoCs(not including
>> rk3066/rk3188).
>> Well, I do try to split it into pieces, but I found that actually not
>> help very much
>> except waste much more time:
>> - The target is(very clear) to make rockchip soc board file in a good
>> shape with common files,
>>     instead of copy-paste for each soc(more than 10 of them now)
>> - then we need to identify what's common and what should go to soc and
>> board;
>> - remove using common rockchip timer and use arm generic timer instead
>> for armv7
>>     SoCs(rk3066 and rk3188 need still using rockchip timer)
>> - most soc need to do uart init, boot order select, and some
>> arch_cpu_init().
>> - don't break the boards already working, so I still leave some code
>> which not so common
>>     in board file, but I would like to remove or move them into right
>> place if I got a board
>>     to verify;

Having a clean commit-history and the ability to bisect and revert individual
patches is an important requirement for the overall project.

I thought to have already provided you the needed guidance on how to get
most fo this merged and highlighted where architectural discussions will be
needed.

So to get most of this merged quickly, you will need to break this up into
series that are more manageable (this is likely to not be a full list):
 * the changes for split out the UART configs
 * the timer changes
 * adding a common board-file and switching boards over
For the common board-file, you should add this first and then start moving
SoCs over onto this new file.  I believe (my memory may be wrong) to have
commented so in some of the individual reviews.

Finally, there’s a few architectural issues to discuss:
 1.	You are merging the “board”-files, but these are now merged across
	multiple SoCs and across multiple boards per SoC.  This is already
	causing some fraying at the edges (e.g. the number of rk_* hooks and
	the weak functions added).  We should very carefully consider how
	this will affect adding boards (which may have a specific market
	segment in mind and may—for lack of a better example—not want
	to have rockusb included) in the future.
 2.	The new weak-functions are causing a major headache: they are at
	odds with us trying to move to DM across the entire tree. I strongly
	believe that these weak functions will cause added debug issues in
	the short term and that they will eventually be replaced with more
	DM-like model in the long term.

Thanks,
Philipp.

>> @Simon, @Tom,
>> This patch set is to remove some common files and add some other common
>> files for
>> all Rockchip SoCs, I have to make sure the whole patch set can running
>> good for all SoCs,
>> but it's really hard to make every patch to build and work perfect for
>> all SoCs, is there
>> any mandatory rules for this?
> 
> So you mean possibly breaking some existing platforms?  I don't like the
> idea of doing that...
> 
> -- 
> Tom