Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32076 )
Change subject: mb/protectli/vault: Add FW2B and FW4B Braswell based boards support ......................................................................
Patch Set 16:
(14 comments)
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... File src/mainboard/protectli/vault_bsw/acpi/superio.asl:
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... PS15, Line 13: Method (_STA, 0, NotSerialized) {
Should '{' be placed on newline?
Done
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... File src/mainboard/protectli/vault_bsw/acpi_tables.c:
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... PS15, Line 12: #include <arch/smp/mpspec.h>
Are al these include files required?
Cleaned up from obsolete includes
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... File src/mainboard/protectli/vault_bsw/cmos.layout:
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... PS15, Line 60: 388 4 h 0 reboot_counter
Compare with FB FBG1701: […]
Removed option table as it was unused.
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... PS15, Line 70: 400 1 e 2 hyper_threading
unused
Removed option table as it was unused.
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... PS15, Line 80: 416 128 r 0 vbnv
ChromeOS and SandyBridge MRC not used!
Removed option table as it was unused.
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... File src/mainboard/protectli/vault_bsw/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... PS15, Line 77: # Disable use of ACPI mode
Comment what is done. […]
Done
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... PS15, Line 125: device pci 1f.0 on # 8086 229c - LPC bridge
TAB missing for alignment
Done
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... File src/mainboard/protectli/vault_bsw/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... PS15, Line 13: 0x02, // DSDT revision: ACPI v2.0 and up
One line comment use '/*'
Done
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... PS15, Line 27: Scope (_SB) { : Device (PCI0)
Um, that brace on the same line as Scope looks weird... […]
Done
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... File src/mainboard/protectli/vault_bsw/fadt.c:
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... PS15, Line 23: header->asl_compiler_revision = 1;
special reason not using = asl_revision?
Done
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... File src/mainboard/protectli/vault_bsw/gpio.c:
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... PS15, Line 10: #include <soc/gpio.h>
Is soc/gpio. […]
Removed obsolete includes
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... PS15, Line 249:
Remove empty line
Done
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... File src/mainboard/protectli/vault_bsw/ramstage.c:
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... PS15, Line 10: #include <boardid.h>
Is boardid. […]
Actually it is not. Removed.
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... File src/mainboard/protectli/vault_bsw/romstage.c:
https://review.coreboot.org/c/coreboot/+/32076/15/src/mainboard/protectli/va... PS15, Line 10: #include <fsp/car.h>
Is this car. […]
No, removed.