Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/35031 )
Change subject: soc/intel/skylake: Add GPIOs layout for Lewisburg PCH ......................................................................
Patch Set 12:
(9 comments)
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/Kcon... File src/soc/intel/skylake/Kconfig:
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/Kcon... PS12, Line 178: WELISBURG_SOC_PCH_H Ah, so "Welisburg" is supposed to be Lewisburg!
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/Make... File src/soc/intel/skylake/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/Make... PS12, Line 87: # FIXME: it is required to add real microcode : cpu_microcode_bins += 3rdparty/intel-microcode/intel-ucode/06-5e-03 : cpu_microcode_bins += 3rdparty/intel-microcode/intel-ucode/06-9e-09 Aren't there any microcode updates for SKX? I would add the microcode options in a later patch, since the CPUs would use the microcode.
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/acpi... File src/soc/intel/skylake/acpi/gpio.asl:
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/acpi... PS12, Line 81: WELISBURG_SOC_PCH_H Welisburg? I guess you mean Wellsburg? But that's the older PCH, isn't the Skylake PCH called Lewisburg?
/me is confused.
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/acpi... PS12, Line 144: 3 Add a space after the '3' ?
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/gpio... File src/soc/intel/skylake/gpio.c:
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/gpio... PS12, Line 243: This space before the 'if' looks wrong
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/gpio... PS12, Line 246: This space before the 'endif' also looks wrong
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/incl... File src/soc/intel/skylake/include/soc/gpio_defs.h:
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/incl... PS12, Line 39: This space before the 'define' also looks wrong
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/incl... PS12, Line 40: Same
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/incl... File src/soc/intel/skylake/include/soc/gpio_wlb_pch_defs.h:
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/incl... PS12, Line 37: These look like spaces, could you use tabs please?