Attention is currently required from: Xi Chen, Martin Roth, Paul Menzel, Yu-Ping Wu. Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/50294 )
Change subject: Add mediatek mt8192 dram vendor code ......................................................................
Patch Set 5:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/50294/comment/2b76280b_15cc5708 PS5, Line 11:
Please elaborate, why this is a good idea. […]
xixi's comments and the way he merged patches may lead to confusion. The real goal we want to achieve is "use the reference (official) memory initialization code from MTK's official firmware".
The code in vendor directory actually came from their official dram init lib. It is actually self-contained and can be compiled as a blob. But it's also not following coreboot coding style.
Without that, we have to keep refactoring between different coding styles between Coreboot and MTK's internal dram repo and maintaining multiple branches, which can easily go wrong and not able to apply the fixes from MTK upstream.
Patchset:
PS5:
@hung-te: Prefer to use one change instead of two changes: one for src/soc, one for src/vendorcode, […]
No, you have to make it clear "here's a drop from MTK's official release". Even better if it has some README and version like "Mediatek DRAM calibration for MT8192, version 1.0, released 2021/2/27". Merging into two will create confusion, and that's why you saw Paul's concern.
You should have one "vendor: Add Mediatek DRAM calibration reference code".
-
Hung-Te Lin (Code Review)