Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30977 )
Change subject: [NOTFORMERGE] intel/d945gclf board fork attempt ......................................................................
Patch Set 1:
(14 comments)
https://review.coreboot.org/#/c/30977/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/30977/1//COMMIT_MSG@11 PS1, Line 11: 1.make a folder advantech in mainboard . : : 2.change vendor name in Kconfig.name files accordingly. : : 3.copy folder intle/d945gclf in advantech folder. : rename folder with som4461 : : 4.In som4461/kconfig.name change board name to som4461 : : 5.In som4461/kconfig , change existing superio to select : SUPERIO_WINBOND_W83627DHG This looks good.
https://review.coreboot.org/#/c/30977/1//COMMIT_MSG@23 PS1, Line 23: 6.In som4461/devicetree.cb change existing chip to : : chip superio/winbond/w83627dhg Have the values on this devicetree section been changed according to superiotool?
https://review.coreboot.org/#/c/30977/1//COMMIT_MSG@29 PS1, Line 29: 8.Change romstage.c , include winbond.h and w83627dhg.h The SuperIO code in romstage.c is SuperIO-specific.
https://review.coreboot.org/#/c/30977/1//COMMIT_MSG@32 PS1, Line 32: >donot include microcode, Why?
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/Kcon... File src/mainboard/advantech/som4461/Kconfig:
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/Kcon... PS1, Line 26: select HAVE_PIRQ_TABLE This option refers to mainboard-specific code you have here. If you have not changed irq_tables.c, comment this entry out for now.
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/Kcon... PS1, Line 30: BOARD_ROMSIZE_KB_512 Is this size correct? I see 8 Mbit on a document, which would be 1024 KB.
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/acpi... File src/mainboard/advantech/som4461/acpi/ec.asl:
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/acpi... PS1, Line 1: /* Does your mainboard have an Embedded Controller?
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/acpi... File src/mainboard/advantech/som4461/acpi/superio.asl:
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/acpi... PS1, Line 15: : Check this file on src/mainboard/asus/p5qpl-am/acpi
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/boar... File src/mainboard/advantech/som4461/board_info.txt:
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/boar... PS1, Line 3: 2011 Really?
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/devi... File src/mainboard/advantech/som4461/devicetree.cb:
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/devi... PS1, Line 27: subsystemid 0x8086 0x464c inherit This should be the subsystemid of your mainboard. Check with `lspci -nnv`
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/devi... PS1, Line 48: 0x20000601 This comes from inteltool
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/devi... PS1, Line 57: device pci 1b.0 on end # High Definition Audio Check `lspci -nntv` to make sure you are enabling the devices your mainboard uses, if not done already.
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/mpta... File src/mainboard/advantech/som4461/mptable.c:
PS1: This is mainboard-specific, and probably not needed for now.
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/roms... File src/mainboard/advantech/som4461/romstage.c:
https://review.coreboot.org/#/c/30977/1/src/mainboard/advantech/som4461/roms... PS1, Line 30: #define SERIAL_DEV PNP_DEV(0x2e, W83627DHG_SP1) : #define GPIO_DEV PNP_DEV(0x2e, W83627DHG_GPIO2345_V) Please insert a newline before and after these lines