Eloy has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32346 )
Change subject: [WIP]mb/fujitsu: add Esprimo E900 ......................................................................
Patch Set 2:
(14 comments)
Mostly small fixes in the devicetree
https://review.coreboot.org/#/c/32346/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/32346/1//COMMIT_MSG@10 PS1, Line 10: USB is tested and works, PS/2 does not work yet.
Make this a list/enumeration.
Done
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/d... File src/mainboard/fujitsu/esprimo_e900/devicetree.cb:
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/d... PS1, Line 3: register "gfx.link_frequency_270_mhz" = "0"
Can be removed
Done
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/d... PS1, Line 5: register "gfx.use_spread_spectrum_clock" = "0" : register "gpu_cpu_backlight" = "0x00000000" : register "gpu_dp_b_hotplug" = "4" : register "gpu_dp_c_hotplug" = "4" : register "gpu_dp_d_hotplug" = "4" : register "gpu_panel_port_select" = "0" : register "gpu_panel_power_backlight_off_delay" = "0" : register "gpu_panel_power_backlight_on_delay" = "0" : register "gpu_panel_power_cycle_delay" = "4" : register "gpu_panel_power_down_delay" = "0" : register "gpu_panel_power_up_delay" = "0" : register "gpu_pch_backlight" = "0x00000000"
Can be removed
Done
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/d... PS1, Line 25: device lapic 0x0 on : end : device lapic 0xacac off : end
Please put the "end" on the same line
Done
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/d... PS1, Line 31: :
Add this line here: […]
Done
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/d... PS1, Line 37: register "gen3_dec" = "0x00000000" : register "gen4_dec" = "0x00000000"
Can be removed
Done
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/d... PS1, Line 40: register "pcie_hotplug_map" = "{ 0, 0, 0, 0, 0, 0, 0, 0 }"
Can be removed
Done
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/d... PS1, Line 49: device pci 16.1 off # Management Engine Interface 2 : end
Please move the "end" to the previous line
Done
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/d... PS1, Line 59: subsystemid 0x1734 0x11ba
After doing what the comment on line 32 says, lines like this one with the same subsystemid can be r […]
Done
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/d... PS1, Line 64: device pci 1c.0 off # PCIe Port #1 : end : device pci 1c.1 off # PCIe Port #2 : end : device pci 1c.2 off # PCIe Port #3 : end : device pci 1c.3 off # PCIe Port #4 : end : device pci 1c.4 off # PCIe Port #5 : end : device pci 1c.5 off # PCIe Port #6 : end : device pci 1c.6 off # PCIe Port #7 : end : device pci 1c.7 off # PCIe Port #8 : end
Does that board have any PCIe ports? It's weird none are enabled.
It does have two PCIe x16 slots and one PCIe x1 slot.
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/d... PS1, Line 100: Host bridge
Duplicated comment
Done
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/d... PS1, Line 100: device pci 00.0 on # Host bridge Host bridge : subsystemid 0x1734 0x11b9 : end : device pci 01.0 off # PCIe Bridge for discrete graphics : end : device pci 02.0 on # Internal graphics VGA controller : subsystemid 0x1734 0x11b9 : end
This block can be moved above the "chip southbridge/intel/bd82x6x" line
Done
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/d... File src/mainboard/fujitsu/esprimo_e900/dsdt.asl:
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/d... PS1, Line 15: /* Some generic macros */
Please remove this comment.
Done
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/r... File src/mainboard/fujitsu/esprimo_e900/romstage.c:
https://review.coreboot.org/#/c/32346/1/src/mainboard/fujitsu/esprimo_e900/r... PS1, Line 73: /* FIXME: Put proper SPD map here. */
How many RAM slots does this board have?
It has 4 slots