Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/28640 )
Change subject: mb/clevo/kbl-u: Add Clevo N130WU/N131WU ......................................................................
Patch Set 116:
(6 comments)
https://review.coreboot.org/c/coreboot/+/28640/114/Documentation/mainboard/c... File Documentation/mainboard/clevo/n130wu/index.md:
https://review.coreboot.org/c/coreboot/+/28640/114/Documentation/mainboard/c... PS114, Line 16: ITE 8587
ITE IT8587E
Done
https://review.coreboot.org/c/coreboot/+/28640/114/src/mainboard/clevo/Kconf... File src/mainboard/clevo/Kconfig:
https://review.coreboot.org/c/coreboot/+/28640/114/src/mainboard/clevo/Kconf... PS114, Line 2: #
drop the #
Done
https://review.coreboot.org/c/coreboot/+/28640/114/src/mainboard/clevo/kbl-u... File src/mainboard/clevo/kbl-u/Kconfig:
https://review.coreboot.org/c/coreboot/+/28640/114/src/mainboard/clevo/kbl-u... PS114, Line 80: config MAINBOARD_SUPPORTS_SKYLAKE_CPU : default n if BOARD_CLEVO_N13xWU : : config MAINBOARD_SUPPORTS_KABYLAKE_DUAL : default n if BOARD_CLEVO_N13xWU
Is this needed?
With these I make sure that unnecessary microcode doesn't get added.
https://review.coreboot.org/c/coreboot/+/28640/114/src/mainboard/clevo/kbl-u... File src/mainboard/clevo/kbl-u/variants/n13xwu/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/28640/114/src/mainboard/clevo/kbl-u... PS114, Line 5: register "deep_s3_enable_ac" = "0" : register "deep_s3_enable_dc" = "0" : register "deep_s5_enable_ac" = "0" : register "deep_s5_enable_dc" = "0" : register "deep_sx_config" = "DSX_EN_LAN_WAKE_PIN"
If Deep Sx is not enabled, drop this.
Done
https://review.coreboot.org/c/coreboot/+/28640/114/src/mainboard/clevo/kbl-u... PS114, Line 127: subsystemid 0x1558 0x1313
Inherit it?
Done
https://review.coreboot.org/c/coreboot/+/28640/114/src/mainboard/clevo/kbl-u... PS114, Line 170: device pci 00.0 on end # x4 TBT
Are these device nodes necessary?
I use them for documentation.