Martin Roth has posted comments on this change. ( https://review.coreboot.org/19604 )
Change subject: mainboard/intel/glkrvp: Add support for GLKRVP ......................................................................
Patch Set 26:
(7 comments)
https://review.coreboot.org/#/c/19604/26/src/mainboard/intel/glkrvp/Kconfig File src/mainboard/intel/glkrvp/Kconfig:
PS26, Line 23: help : This option allows you to select the on board EC to use. : Select whether the board has Intel EC or Chrome EC Indent the lines after the 'help' keyword by two additional spaces. This will fix the lint errors.
help This option allows you to select the on board EC to use. Select whether the board has Intel EC or Chrome EC
PS26, Line 27: change tab to space
PS26, Line 28: bool "Chrome EC" : select EC_GOOGLE_CHROMEEC : select EC_GOOGLE_CHROMEEC_LPC outdent by one tab
PS26, Line 32: config GLK_INTEL_EC : bool "Intel EC" : select EC_ACPI same as above comments.
https://review.coreboot.org/#/c/19604/26/src/mainboard/intel/glkrvp/acpi_tab... File src/mainboard/intel/glkrvp/acpi_tables.c:
PS26, Line 1: /* : * This program is free software; you can redistribute it and/or modify : * it under the terms of the GNU General Public License as published by : * the Free Software Foundation; version 2 of the License. : * : * This program is distributed in the hope that it will be useful, : * but WITHOUT ANY WARRANTY; without even the implied warranty of : * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the : * GNU General Public License for more details. : */ Please remove the license from the empty file.
https://review.coreboot.org/#/c/19604/26/src/mainboard/intel/glkrvp/bootbloc... File src/mainboard/intel/glkrvp/bootblock.c:
PS26, Line 22: void mainboard_ec_init(void); Why does this need to be here? Should it go into variant/ec.h instead?
https://review.coreboot.org/#/c/19604/26/src/mainboard/intel/glkrvp/dsdt.asl File src/mainboard/intel/glkrvp/dsdt.asl:
PS26, Line 51: #if 0 : /* LID and Power button. */ : Scope (_SB) : { : Device (LID0) : { : Name (_HID, EisaId ("PNP0C0D")) : Method (_LID, 0) : { : Return (_SB.PCI0.LPCB.EC0.LIDS) : } : Name (_PRW, Package () { GPE_EC_WAKE, 0x3 }) : } : Device (PWRB) : { : Name (_HID, EisaId ("PNP0C0C")) : } : } : /* Chrome OS Embedded Controller */ : Scope (_SB.PCI0.LPCB) : { : /* ACPI code for EC SuperIO functions */ : #include <ec/google/chromeec/acpi/superio.asl> : /* ACPI code for EC functions */ : #include <ec/google/chromeec/acpi/ec.asl> : } : : /* Dynamic Platform Thermal Framework */ : Scope (_SB) : { : /* Per board variant specific definitions. */ : #include <variant/acpi/dptf.asl> : /* Include soc specific DPTF changes */ : #include <soc/intel/apollolake/acpi/dptf.asl> : /* Include common dptf ASL files */ : #include <soc/intel/common/acpi/dptf/dptf.asl> : } : #endif remove?