Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/38143 )
Change subject: ASUS P5Q support and menu entries added ......................................................................
Patch Set 1:
(5 comments)
Hi, welcome to coreboot!
I've got a few suggestions for you. If you have any questions, feel free to ask me anything :-)
https://review.coreboot.org/c/coreboot/+/38143/1//COMMIT_MSG Commit Message:
PS1: Does this work? I would recommend listing in the commit message:
- What is tested and works - What does not work - What is untested
https://review.coreboot.org/c/coreboot/+/38143/1//COMMIT_MSG@7 PS1, Line 7: ASUS P5Q support and menu entries added Commit messages should use imperative tense, as well as the scope of the change as a prefix. For example:
mb/asus/p5qc: Add ASUS P5Q as a variant
https://review.coreboot.org/c/coreboot/+/38143/1//COMMIT_MSG@10 PS1, Line 10: There's no `Signed-off` line, which is probably why Jenkins marked the change as failed.
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/Kco... File src/mainboard/asus/p5qc/Kconfig:
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/Kco... PS1, Line 55: config GPIO_C The GPIO settings might be different for the Asus P5Q. I would suggest using util/inteltool to dump the GPIO registers, running the stock Asus BIOS. The meaning of the registers can be found in the ICH10 datasheet.
You can also use this to generate a gpio.c:
https://github.com/Th3Fanbus/gpio-scripts/blob/master/gengpio.c
I know, it's an ugly hack of a program. To use it, you need to paste the values from inteltool into the corresponding variables:
GPIO_USE_SEL GP_IO_SEL GP_LVL GPI_INV GPIO_USE_SEL2 GP_IO_SEL2 GP_LVL2
Note that there's an additional GPIO bank. This only exists on Sandybridge and newer, so you should trim the resulting gpio.c
In any case, if you show me the register values with the stock Asus BIOS, I can check if the GPIO configuration is correct.
https://review.coreboot.org/c/coreboot/+/38143/1/src/mainboard/asus/p5qc/var... File src/mainboard/asus/p5qc/variants/p5q/devicetree.cb:
PS1: This looks very similar to the existing devicetrees. Has anything been changed?