Maxim Polyakov has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/31603 )
Change subject: mainboard: Add ASRock H110M-DVS ......................................................................
Patch Set 7:
(13 comments)
Hi all! I'm sorry, maybe I did not understand, Review completed? I just started committing here recently and may not know all rules. I fixed everything. Do you have any notes on my patch?
I can add to my work a coreboot-log for this board:
https://drive.yadro.com/s/QmHGXD2gFDaPqT5
Regards, Maxim Polyakov
https://review.coreboot.org/#/c/31603/2//COMMIT_MSG Commit Message:
https://review.coreboot.org/#/c/31603/2//COMMIT_MSG@13 PS2, Line 13: Intel FSP2.0
I do not know much about that, but is there a revision or something similar?
In Intel FSP repository it has Specification Version 2.0 * from https://github.com/IntelFsp/FSP :
|========================================|===================|===============| | | | FSP | | FSP Project Name | Directory Name | Specification | | | | Version | |========================================|===================|===============| | 7th Generation Intel® Core™ processors | KabylakeFspBinPkg | v2.0 * | | and chipsets (formerly Kaby Lake) | | | |========================================|===================|===============|
But in addition to this, FSP has revision 3.6.0 . I saw this revision in the last commit in the "Kabelake" branch: https://github.com/IntelFsp/FSP/commit/324ffc02523bf23a907a3ff305b43b5047adf...
I added "FSP 3.6.0" to the commit message.
https://review.coreboot.org/#/c/31603/2//COMMIT_MSG@17 PS2, Line 17: - Integrated graphics (only DVI port);
1920x1080 […]
I added this information to the commit message.
https://review.coreboot.org/#/c/31603/2/src/mainboard/asrock/h110m/Kconfig File src/mainboard/asrock/h110m/Kconfig:
https://review.coreboot.org/#/c/31603/2/src/mainboard/asrock/h110m/Kconfig@4... PS2, Line 45: : config VARIANT_DIR : string : default "dvs" if BOARD_ASROCK_H110M_DVS
remove until you add variants.
Done
https://review.coreboot.org/#/c/31603/2/src/mainboard/asrock/h110m/Kconfig@6... PS2, Line 62: config DEVICETREE : string : default "variants/baseboard/devicetree.cb" : : config OVERRIDE_DEVICETREE : string : default "variants/$(CONFIG_VARIANT_DIR)/overridetree.cb"
I reworked the patch to remove the "variants" directory. […]
Done
https://review.coreboot.org/#/c/31603/2/src/mainboard/asrock/h110m/Kconfig@7... PS2, Line 73: "3rdparty/blobs/mainboard/$(CONFIG_MAINBOARD_DIR)/descriptor.dvs.bin"
I removed it
Done
https://review.coreboot.org/#/c/31603/5/src/mainboard/asrock/h110m/include/h... File src/mainboard/asrock/h110m/include/hda_verb.h:
https://review.coreboot.org/#/c/31603/5/src/mainboard/asrock/h110m/include/h... PS5, Line 4: * Copyright (C) 2017 Intel Corporation
if you don't use variants, the file contents can be moved to hda_verb. […]
Done
https://review.coreboot.org/#/c/31603/5/src/mainboard/asrock/h110m/spd/spd_u... File src/mainboard/asrock/h110m/spd/spd_util.c:
https://review.coreboot.org/#/c/31603/5/src/mainboard/asrock/h110m/spd/spd_u... PS5, Line 4: * Copyright (C) 2016 Intel Corporation.
move everything to romstage.c and make it static […]
Done
https://review.coreboot.org/#/c/31603/2/src/mainboard/asrock/h110m/variants/... File src/mainboard/asrock/h110m/variants/baseboard/devicetree.cb:
https://review.coreboot.org/#/c/31603/2/src/mainboard/asrock/h110m/variants/... PS2, Line 38: # EC host command ranges are in 0x800-0x8ff & 0x200-0x20f : register "gen1_dec" = "0x00fc0801
I removed it
Done
https://review.coreboot.org/#/c/31603/2/src/mainboard/asrock/h110m/variants/... PS2, Line 54: register "SkipExtGfxScan" = "1"
Yes, I think it will be true if "SkipExtGfxScan" is set to 0 so that the system would scan external […]
Done
https://review.coreboot.org/#/c/31603/2/src/mainboard/asrock/h110m/variants/... PS2, Line 156: register "pirqa_routing" = "0x0b" : register "pirqb_routing" = "0x0a" : register "pirqc_routing" = "0x0b" : register "pirqd_routing" = "0x0b" : register "pirqe_routing" = "0x0b" : register "pirqf_routing" = "0x0b" : register "pirqg_routing" = "0x0b" : register "pirqh_routing" = "0x0b"
I removed it
Done
https://review.coreboot.org/#/c/31603/2/src/mainboard/asrock/h110m/variants/... PS2, Line 224: device pci 1c.0 off end # PCI Express Port 1 : device pci 1c.1 off end # PCI Express Port 2 : device pci 1c.2 off end # PCI Express Port 3 : device pci 1c.3 off end # PCI Express Port 4 : device pci 1c.4 off end # PCI Express Port 5 : device pci 1c.5 off end # PCI Express Port 6 : device pci 1c.6 off end # PCI Express Port 7 : device pci 1c.7 off end # PCI Express Port 8 : device pci 1d.0 off end # PCI Express Port 9 : device pci 1d.1 off end # PCI Express Port 10 : device pci 1d.2 off end # PCI Express Port 11 : device pci 1d.3 off end # PCI Express Port 12
I used to enable ports in overridetree. […]
Done
https://review.coreboot.org/#/c/31603/2/src/mainboard/asrock/h110m/variants/... PS2, Line 243: off
Should always be on
Done
https://review.coreboot.org/#/c/31603/2/src/mainboard/asrock/h110m/variants/... PS2, Line 246: off
I enabled hda […]
Done