Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/25509 )
Change subject: Add i945G based mainboard ......................................................................
Patch Set 32:
(11 comments)
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... File src/mainboard/nec/945g-m4/Kconfig:
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... PS32, Line 16: #select MAINBOARD_HAS_LPC_TPM ?
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... File src/mainboard/nec/945g-m4/acpi/ich7_pci_irqs.asl:
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... PS32, Line 23: Package() { 0x0000ffff, 0, 0, 16}, : : Package() { 0x0001ffff, 0, 0, 20}, : Package() { 0x0001ffff, 1, 0, 21}, : Package() { 0x0001ffff, 2, 0, 22}, : Package() { 0x0001ffff, 3, 0, 23}, : : Package() { 0x0002ffff, 0, 0, 21}, : Package() { 0x0002ffff, 1, 0, 22}, : Package() { 0x0002ffff, 2, 0, 23}, : Package() { 0x0002ffff, 3, 0, 20}, : : Package() { 0x0003ffff, 0, 0, 22}, : Package() { 0x0003ffff, 1, 0, 23}, : Package() { 0x0003ffff, 2, 0, 20}, : Package() { 0x0003ffff, 3, 0, 21}, : : Package() { 0x0004ffff, 0, 0, 23}, : Package() { 0x0004ffff, 1, 0, 20}, : Package() { 0x0004ffff, 2, 0, 21}, : Package() { 0x0004ffff, 3, 0, 22}, : : Package() { 0x0005ffff, 0, 0, 19}, : Package() { 0x0005ffff, 1, 0, 18}, : Package() { 0x0005ffff, 2, 0, 17}, : Package() { 0x0005ffff, 3, 0, 16}, : : Package() { 0x0006ffff, 0, 0, 18}, : Package() { 0x0006ffff, 1, 0, 17}, : Package() { 0x0006ffff, 2, 0, 16}, : Package() { 0x0006ffff, 3, 0, 19}, : : Package() { 0x0009ffff, 0, 0, 21}, : Package() { 0x0009ffff, 1, 0, 22}, : Package() { 0x0009ffff, 2, 0, 23}, : Package() { 0x0009ffff, 3, 0, 20}, Is this correct? You can get this information from vendor DSDT. Look for the _PRT method for device 0x1e0000
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... File src/mainboard/nec/945g-m4/acpi/platform.asl:
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... PS32, Line 2: * This file is part of the coreboot project. I think this whole file can be dropped. It's in southbridge/common/acpi/platform.asl
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... File src/mainboard/nec/945g-m4/acpi/thermal.asl:
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... PS32, Line 18: This file seems to be a no-op? remove?
https://review.coreboot.org/c/coreboot/+/25509/23/src/mainboard/nec/945g-m4/... File src/mainboard/nec/945g-m4/acpi/video.asl:
https://review.coreboot.org/c/coreboot/+/25509/23/src/mainboard/nec/945g-m4/... PS23, Line 16: : // Brightness write : Method (BRTW, 1, Serialized) : { : // TODO : } : : // Hot Key Display Switch : Method (HKDS, 1, Serialized) : { : // TODO : } : : // Lid Switch Display Switch : Method (LSDS, 1, Serialized) : { : // TODO : } : : // Brightness Notification : Method(BRTN,1,Serialized) : { : // TODO (no displays defined yet) : } This file is not even included it seems...
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... File src/mainboard/nec/945g-m4/acpi/video.asl:
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... PS32, Line 2: * This file is part of the coreboot project. Is this file needed?
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... File src/mainboard/nec/945g-m4/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... PS32, Line 92: #io 0x60 = 0x201 remove?
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... File src/mainboard/nec/945g-m4/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... PS32, Line 27: // #include "acpi/platform.asl" remove
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... File src/mainboard/nec/945g-m4/early_init.c:
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... PS32, Line 87: RCBA16(D31IR) = 0x0132; : RCBA16(D30IR) = 0x3241; : RCBA16(D29IR) = 0x0237; You can drop this if you want.
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... PS32, Line 95: // pci_write_config8(PCI_DEV(0, 0x1e, 2), 0x41, 1); : // pci_write_config32(PCI_DEV(0, 0x1e, 2), 0x10, 0x1800); : // pci_write_config32(PCI_DEV(0, 0x1e, 2), 0x14, 0x1900); : // pci_write_config8(PCI_DEV(0, 0x1e, 2), 0x4, 1); : // pci_write_config32(PCI_DEV(0, 0x1e, 3), 0x10, 0x1a00); : // pci_write_config32(PCI_DEV(0, 0x1e, 3), 0x14, 0x1b00); : // pci_write_config8(PCI_DEV(0, 0x1e, 3), 0x4, 1); remove?
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... File src/mainboard/nec/945g-m4/mainboard.c:
https://review.coreboot.org/c/coreboot/+/25509/32/src/mainboard/nec/945g-m4/... PS32, Line 24: wm_setup(); remove if not working?