a
Patch set 11:Code-Review +1
18 comments:
Patch Set #1, Line 7: New board
Add Sandybridge board Asus P8Z77-M
Done
Patch Set #7, Line 7: mainboard: New ivybridge board ASUS P8Z77-M
Please make it a statement by adding a verb (in imperative mood): […]
Done
Patch Set #7, Line 13: - SeaBIOS boot
What version, and what OS?
Done. I expect SeaBIOS version to be the currently-default version.
Patch Set #7, Line 18: - Hardware monitoring under Linux
What version?
Done (stated above)
Patch Set #7, Line 24: - 2ch sound playback, Linux and Windows
What versions?
Done (stated above)
nit: this board also supports Sandy Bridge CPUs
With which CPU did you test this port?
File src/mainboard/asus/p8z77-m/acpi/platform.asl:
Patch Set #7, Line 6: Return(Package(){0,0})
Add spaces (`util/autoport/ec_none.go`).
Ack (IMHO, we should auto-format ASL)
File src/mainboard/asus/p8z77-m/cmos.layout:
# -----------------------------------------------------------------
# Status Register A
# -----------------------------------------------------------------
# Status Register B
# -----------------------------------------------------------------
# Status Register C
#96 4 r 0 status_c_rsvd
#100 1 r 0 uf_flag
#101 1 r 0 af_flag
#102 1 r 0 pf_flag
#103 1 r 0 irqf_flag
# -----------------------------------------------------------------
# Status Register D
#104 7 r 0 status_d_rsvd
#111 1 r 0 valid_cmos_ram
# -----------------------------------------------------------------
# Diagnostic Status Register
#112 8 r 0 diag_rsvd1
We can get rid of this I think
Done
File src/mainboard/asus/p8z77-m/devicetree.cb:
Patch Set #10, Line 21: Series 6 Cougar Point PCH
Series 7 Panther Point PCH
Done
Please add a 2nd space after the "on" words, so that the "end" words are all aligned
Done
I think there's mixed tabs/spaces right before the comments
Done
Patch Set #10, Line 40: slots
Slots, in plural? There are two PCIe slots, the one closest to the CPU is on `device pci 01. […]
Done
File src/mainboard/asus/p8z77-m/dsdt.asl:
Patch Set #6, Line 25: #include <drivers/intel/gma/acpi/default_brightness_levels.asl>
Not needed anymore
*poke*
File src/mainboard/asus/p8z77-m/early_init.c:
unsigned int
*poke*
Patch Set #10, Line 108: for (i = 0; i < max; i += 2)
Difficult question how to optimize between code size and readability. […]
IMHO, it's well-commented code, so I don't mind keeping it like this. I'd add the rationale as a comment though. (See current patchset)
File src/mainboard/asus/p8z77-m/early_init.c:
Patch Set #11, Line 48: static const u8 register_values[] = {
I'd strongly recommend adding some comment here explaining this is done to save some bytes. Otherwise, people may be tempted to change this to align it with other boards.
File src/mainboard/asus/p8z77-m/gpio.c:
Patch Set #10, Line 2: /* This file is part of the coreboot project. */
We got rid of these comments
Done
To view, visit change 38988. To unsubscribe, or for help writing mail filters, visit settings.