Maxim Polyakov 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 13:
(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!
Yes :D fixed
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 Lewisb […]
:D :D Oh, sorry Ж) Maybe I need a vacation :D Renamed to LEWISBURG_SOC_PCH_H. Thx
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/acpi... PS12, Line 144: 3
Add a space after the '3' ?
Fixed
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
Fixed
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/gpio... PS12, Line 246:
This space before the 'endif' also looks wrong
Fixed
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 24: What about the space here? I think it is also unnecessary
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/incl... PS12, Line 39:
This space before the 'define' also looks wrong
Fixed
https://review.coreboot.org/c/coreboot/+/35031/12/src/soc/intel/skylake/incl... PS12, Line 40:
Same
Fixed
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?
Added tab