Vlado Cibic has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33328 )
Change subject: mainboard: Add support for ASUS P8Z77-M PRO desktop mainboard ......................................................................
Patch Set 22: -Code-Review
(10 comments)
https://review.coreboot.org/#/c/33328/22/src/mainboard/asus/p8z77-m_pro/cmos... File src/mainboard/asus/p8z77-m_pro/cmos.layout:
https://review.coreboot.org/#/c/33328/22/src/mainboard/asus/p8z77-m_pro/cmos... PS22, Line 162: 7 16 1024M
Every setting larger than 512M is reserved according to the datasheet, so please remove. […]
I just copied the settings from the original Bios, but that's interesting! I'm gonna try to add values in 32mb increment and test if I can reach 1024M with this board with multiple OS! thx!
https://review.coreboot.org/#/c/33328/22/src/mainboard/asus/p8z77-m_pro/devi... File src/mainboard/asus/p8z77-m_pro/devicetree.cb:
https://review.coreboot.org/#/c/33328/22/src/mainboard/asus/p8z77-m_pro/devi... PS22, Line 96: device pnp 2e.2 off end # UART A
there is a COM port on the board?
Yep, but saly the one I bought has bended pins so I can't test it 8(
https://review.coreboot.org/#/c/33328/22/src/mainboard/asus/p8z77-m_pro/devi... PS22, Line 100: drq 0x30 = 1 # Enable device
When a LDN is set to 'on' it will set the right bit in 0x030.
Ooops, you're right!
https://review.coreboot.org/#/c/33328/22/src/mainboard/asus/p8z77-m_pro/devi... PS22, Line 103: drq 0x70 = 1 # Keyboard IRQ : drq 0x72 = 12 # Mouse IRQ
Those are irq's
Yep, I should change that to "irq" and not "drq".
https://review.coreboot.org/#/c/33328/22/src/mainboard/asus/p8z77-m_pro/devi... PS22, Line 127: 4e.0
most often 0c31 is set here as a dummy because PNP0C31 ACPI ID.
Interesting.
https://review.coreboot.org/#/c/33328/22/src/mainboard/asus/p8z77-m_pro/hda_... File src/mainboard/asus/p8z77-m_pro/hda_verb.c:
https://review.coreboot.org/#/c/33328/22/src/mainboard/asus/p8z77-m_pro/hda_... PS22, Line 30: : /* NID 0x11. */
those comments are pretty redundant. […]
Yep, autoport placed it there. I'll remove it.
https://review.coreboot.org/#/c/33328/22/src/mainboard/asus/p8z77-m_pro/roms... File src/mainboard/asus/p8z77-m_pro/romstage.c:
https://review.coreboot.org/#/c/33328/22/src/mainboard/asus/p8z77-m_pro/roms... PS22, Line 65: mainboard_config_superio
You probably want to enable serial on the COM port here.
Can't, I'm afraid my board's COM pins are bended and can't test well...
https://review.coreboot.org/#/c/33328/22/src/mainboard/asus/p8z77-m_pro/roms... PS22, Line 81: /* Setup "power on after fail" via NVRAM option */ : uint8_t acp; : int powerOnAfterFail = 0; : get_option(&powerOnAfterFail, "power_on_after_fail"); : : switch (powerOnAfterFail) { : case 2: : /* keep state before power off */ : acp = 0x40; /* 01000000b=0x40 */ : break; : case 1: : /* enable: turn computer on after computer fail */ : acp = 0x20; /* 00100000b=0x20 */ : break; : case 0: : default: : /* disable: turn off computer */ : acp = 0; /* bits 6-5 to zero */ : break; : }
This is not mainboard specific. Move it to the superio code.
Yep, better. thx.
https://review.coreboot.org/#/c/33328/22/src/mainboard/asus/p8z77-m_pro/roms... PS22, Line 101: : /* Set "3VSBSW# enable bit" to keep RAM powered on S3 suspend */ : acp |= 0x10; : pnp_write_config(ACPI_DEV, 0xe4, acp);
Setting this in the devicetree (irq 0xe4 = ...) should work too.
Ack
https://review.coreboot.org/#/c/33328/22/src/mainboard/asus/p8z77-m_pro/roms... PS22, Line 110: #if CONFIG(USE_NATIVE_RAMINIT)
Remove
Oh, yep. Both paths(NRI+MRC) can work without that CPP.