[coreboot-gerrit] Change in coreboot[master]: mainboard/intel/glkrvp: Add support for GLKRVP

[Martin Roth (Code Review)]

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_tables.c
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/bootblock.c
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?


-- 
To view, visit https://review.coreboot.org/19604
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iab688aca6a4f5c5e32801215ba3a1a440e50fbef
Gerrit-PatchSet: 26
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Hannah Williams <hannah.williams at intel.com>
Gerrit-Reviewer: Aaron Durbin <adurbin at chromium.org>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov at gmail.com>
Gerrit-Reviewer: Bora Guvendik <bora.guvendik at intel.com>
Gerrit-Reviewer: Brandon Breitenstein <brandon.breitenstein at intel.com>
Gerrit-Reviewer: Brenton Dong <brenton.m.dong at intel.com>
Gerrit-Reviewer: Cole Nelson <colex.nelson at intel.com>
Gerrit-Reviewer: Han Lim Ng <nhlhanlim93 at gmail.com>
Gerrit-Reviewer: Martin Roth <martinroth at google.com>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: Ravishankar Sarawadi <ravishankar.sarawadi at intel.com>
Gerrit-Reviewer: Shaunak Saha <shaunak.saha at intel.com>
Gerrit-Reviewer: Subrata Banik <subrata.banik at intel.com>
Gerrit-Reviewer: Sumeet R Pawnikar <sumeet.r.pawnikar at intel.com>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-HasComments: Yes