Attention is currently required from: Michał Żygowski, Kyösti Mälkki. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52934 )
Change subject: nb/amd/agesa/family14/northbridge.c: Use generic allocation functions ......................................................................
Patch Set 2: Code-Review-1
(3 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/52934/comment/5f67fd07_6f10c83f PS2, Line 9: Remove obsolete resource assigning functions. Why are they obsolete?
Patchset:
PS2: Unless I missed something, this is a mess. The commit message is almost empty. Why can the code be dropped? and if we can drop the assignments, why do we allocate the resources in the first place?
It would probably be a good idea to separate NB and domain code changes into individual commits. And also not to mix cosmetical changes in.
It hurts me to see how deep in debts this code is and that the community has to constantly pay interest for it. Please do not make things worse by making the review harder than necessary. Every change should be reasoned about in the commit message before review starts. If reviewers constantly have to ask, why this and that, that's a waste of time. If the code should be kept upstream in the long run, unexplained changes and shallow reviews only add additional debts. If nobody wants to invest into the code, better make plans to maintain it on another branch.
File src/northbridge/amd/agesa/family14/northbridge.c:
https://review.coreboot.org/c/coreboot/+/52934/comment/5612e507_6f3760ca PS2, Line 448: set_resource(dev, res, nodeid); If we can drop this, why does the respective code in nb_read_resources() stay?