Kevin Cody has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/37995 )
Change subject: mainboard/asus/p10s-series: Initial port to ASUS P10S-I ......................................................................
Patch Set 9:
(8 comments)
Apologies, I hadn't seen that my replies were still drafts.
https://review.coreboot.org/c/coreboot/+/37995/3//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/37995/3//COMMIT_MSG@10 PS3, Line 10: P10S-M
I would not add this board until you have boot-tested it. […]
Probably not. At the moment, I've got the key CONFIG value commented out so it won't offer it as a build target. It just seemed to make sense to maintain the variant/ setup from the Supermicro board I'm basing this on.
Okay, I've dropped the variants/p10s-m directory and changed the wording in the latest commit. For now, p10s-i only, but it's set up so maybe someday others in the same family can be done.
https://review.coreboot.org/c/coreboot/+/37995/3/src/mainboard/asus/p10s-ser... File src/mainboard/asus/p10s-series/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/37995/3/src/mainboard/asus/p10s-ser... PS3, Line 3: Enable
I am not sure
Neither am I. Sure does look like it actually disables all that.
Changed in the latest push.
https://review.coreboot.org/c/coreboot/+/37995/3/src/mainboard/asus/p10s-ser... File src/mainboard/asus/p10s-series/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/37995/3/src/mainboard/asus/p10s-ser... PS3, Line 33: // CPU
I would drop this comment
Dropped.
https://review.coreboot.org/c/coreboot/+/37995/3/src/mainboard/asus/p10s-ser... PS3, Line 35: : Scope (_SB) { : Device (PCI0)
Device (_SB. […]
Changed thusly.
https://review.coreboot.org/c/coreboot/+/37995/3/src/mainboard/asus/p10s-ser... PS3, Line 44: // Chipset specific sleep states
I just dropped these with CB:37855
Comment dropped.
https://review.coreboot.org/c/coreboot/+/37995/3/src/mainboard/asus/p10s-ser... File src/mainboard/asus/p10s-series/include/mainboard.h:
https://review.coreboot.org/c/coreboot/+/37995/3/src/mainboard/asus/p10s-ser... PS3, Line 21: #endif /* _OARD_ASUS_P10S_SERIES_H */
OARD?
Fixed.
https://review.coreboot.org/c/coreboot/+/37995/3/src/mainboard/asus/p10s-ser... File src/mainboard/asus/p10s-series/variants/p10s-i/include/variant/gpio.h:
PS3:
How did you generate this?
I didn't. This is inherited verbatim from the Supermicro board I'm basing this on.
Replaced with values generated by a perl script I wrote that parses the output of inteltool -g and spits out the proper high-level macros. The new values still aren't quite exactly right but they're closer.
https://review.coreboot.org/c/coreboot/+/37995/3/src/mainboard/asus/p10s-ser... File src/mainboard/asus/p10s-series/variants/p10s-m/include/variant/gpio.h:
PS3:
How did you generate this?
I didn't. This is inherited verbatim from the Supermicro board I'm basing this on.
File dropped.