Dear coreboot folks,
it is great to see patches adding new boards to our repository. Lately this was AMD Olive Hill [1], ASRock IMB-A180 [2] and just now the Micro Passion Kratos-X1 [3].
The Olive Hill took a AMD Family 15 board as a template, the IMB-A180 was derived from the Olive Hill and, I believe, the Kratos-X1 is also derived from one of these two. (Derived means, an existing board’s directory was copied and then adapted.)
With the current approach it is difficult to see the actual changes to the template board. Especially in Gerrit. Therefore these board support patches are hard to review.
Do you know, how to avoid this problem? Currently, I came only up with adding a “copy-commit” with just the Kconfig name changes and then the actual changes in a separate commit on top of it. Jens Rottmann did the same when adding the LiPPERT (now ADLINK) boards [4][5].
Thanks,
Paul
[1] http://review.coreboot.org/3784 [2] http://review.coreboot.org/3880 [3] http://review.coreboot.org/3951 [4] http://review.coreboot.org/2552 [5] http://review.coreboot.org/2553
it's never easy to come up with rules that work for all occasions. Usually, one should start with a clean Kconfig, devicetree.cb, Makefile, and maybe a mainboard.c. The danger in copying all the files without change is that one ends up with a bunch of devices in the new board that are not really supported (this has happened for many boards over the years).
So the cleanest is to start with a totally minimal setup, and add as needed.
ron
Am 29.09.2013 23:16, schrieb Paul Menzel:
With the current approach it is difficult to see the actual changes to the template board. Especially in Gerrit. Therefore these board support patches are hard to review.
By fixing the tools. My main gripe with git still is its lack of serious file rename/copy tracking (though that can be worked around manually locally). While I can't speak for the git/gerrit communities, I'd guess they accept patches.
As for what we can do with our own tools, and in particular on AMD boards: cut down on the duplication. There's lots of stuff in mainboards that - to me - looks it really belongs into chipset code. That should simplify reviews even more than hacks to work around git's metadata tracking limitations.
Patrick
Am Sonntag, den 29.09.2013, 23:31 +0200 schrieb Patrick Georgi:
Am 29.09.2013 23:16, schrieb Paul Menzel:
With the current approach it is difficult to see the actual changes to the template board. Especially in Gerrit. Therefore these board support patches are hard to review.
By fixing the tools. My main gripe with git still is its lack of serious file rename/copy tracking (though that can be worked around manually locally). While I can't speak for the git/gerrit communities, I'd guess they accept patches.
Sure, though unrealistic in my opinion. But until git is fixed, the problem of carefully and easily reviewing such patches persists.
As for what we can do with our own tools, and in particular on AMD boards: cut down on the duplication. There's lots of stuff in mainboards that - to me - looks it really belongs into chipset code. That should simplify reviews even more than hacks to work around git's metadata tracking limitations.
Agreed. But nobody has done this either and have not heard of somebody working on this.
While I agree, that the above is the better solution, I think it is unrealistic to assume, that this is going to happen in the foreseeable future. In my opinion, the duplication is not going to be removed, until patches adding them are accepted and pushed. And that seems unfeasible too.
Thanks,
Paul
maybe we just accept that it is a hard problem with no good general solution?
:-)
ron
-
Patrick Georgi -
Paul Menzel -
ron minnich