Attention is currently required from: Furquan Shaikh, Tim Wawrzynczak, Subrata Banik, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/52895 )
Change subject: include/console: Fix duplicate entry of postcode 0x79 ......................................................................
Patch Set 3:
(3 comments)
Patchset:
PS3: I only use post codes about once every 3 years, so I don't mind any changes. I didn't notice that the other definition is unused, but agree to what Angel says.
I left two random thoughts anyway :)
File src/arch/x86/c_start.S:
https://review.coreboot.org/c/coreboot/+/52895/comment/e558fcb2_a5629a1a PS3, Line 92: post_code(POST_PRE_HARDWAREMAIN) /* post 6e */ NB. I once worked on another project with similar defines for status codes and comments with their values. This is generally hard to main- tain and very confusing if updating a comment was missed. The solution I came up with was to make the codes part of the identifier, e.g.
#define POST_PRE_HARDWAREMAIN_6e 0x6e
This way no comment is needed, and the compiler would notice if some place isn't updated. What do you think?
File src/include/console/post_codes.h:
https://review.coreboot.org/c/coreboot/+/52895/comment/8508bd4e_644c0f43 PS3, Line 120: #define POST_ENTRY_RAMSTAGE 0x6f When I first read the patch, I thought this is out of order. "entry ramstage" made me think this is the real entry (in assembler). Maybe it could be renamed to `POST_ENTRY_HARDWAREMAIN`?