Kever,
On 9 Apr 2018, at 00:35, Tom Rini trini@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