[coreboot-gerrit] Change in coreboot[master]: nb/amd/amdk8: Link raminit_f.c

Martin Roth (Code Review) gerrit at coreboot.org
Sat Apr 8 05:08:45 CEST 2017


Martin Roth has posted comments on this change. ( https://review.coreboot.org/19030 )

Change subject: nb/amd/amdk8: Link raminit_f.c
......................................................................


Patch Set 9:

(8 comments)

https://review.coreboot.org/#/c/19030/9/src/cpu/amd/model_fxx/init_cpus.c
File src/cpu/amd/model_fxx/init_cpus.c:

Line 332: 		#include <northbridge/amd/amdk8/f.h>
Why not put the include at the top of the file?  A bunch of work was done a while back to remove the guards around the includes of header files - is this an issue again here?


https://review.coreboot.org/#/c/19030/9/src/mainboard/asus/a8v-e_deluxe/romstage.c
File src/mainboard/asus/a8v-e_deluxe/romstage.c:

PS9, Line 41:  /* After vt8237r/early_smbus.c! */
Should this be removed too? And in the next few files...


https://review.coreboot.org/#/c/19030/9/src/northbridge/amd/amdk8/debug.h
File src/northbridge/amd/amdk8/debug.h:

PS9, Line 4: Copyright (C) 2016 Damien Zammit <damien at zamaudio.com>
Why Damien's copyright?


https://review.coreboot.org/#/c/19030/9/src/northbridge/amd/amdk8/raminit.h
File src/northbridge/amd/amdk8/raminit.h:

PS9, Line 5:  
tab here?


PS9, Line 5: 	
space here?


https://review.coreboot.org/#/c/19030/9/src/southbridge/amd/sb700/early_setup.c
File src/southbridge/amd/sb700/early_setup.c:

PS9, Line 33: #if CONFIG_K8_REV_F_SUPPORT
#if IS_ENABLED(CONFIG_K8_REV_F_SUPPORT)

But does it really need the guard?


PS9, Line 817: CONFIG_K8_REV_F_SUPPORT
IS_ENABLED()


https://review.coreboot.org/#/c/19030/9/src/southbridge/via/k8t890/early_car.c
File src/southbridge/via/k8t890/early_car.c:

Line 27: #if CONFIG_K8_REV_F_SUPPORT
same comments as the sb700 file


-- 
To view, visit https://review.coreboot.org/19030
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I9cd1ffff2c39021693fe1d5d3f90ec5f70891f57
Gerrit-PatchSet: 9
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki at gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes



More information about the coreboot-gerrit mailing list