Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/39219 )
Change subject: tablet etabpro intel glk based ......................................................................
Patch Set 4:
(26 comments)
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... File src/mainboard/microtech/etabpro/Kconfig:
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 3: BOARD_INTEL_BASEBOARD_GLKRVP BOARD_SPECIFIC_OPTIONS
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 4: def_bool n Enable this:
def_bool y
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 12: select MAINBOARD_HAS_CHROMEOS No ChromeOS for now.
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 14: select DRIVERS_GENERIC_MAX98357A : select DRIVERS_I2C_DA7219 These are for audio chips on chromebooks. Not needed
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 16: select SOC_ESPI That might not be true.
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 20: # config for GLK CHROME EC : select EC_GOOGLE_CHROMEEC : select EC_GOOGLE_CHROMEEC_ESPI : select VBOOT_LID_SWITCH : select EC_GOOGLE_CHROMEEC_SWITCHES No ChromeEC here
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 43: config VBOOT : select HAS_RECOVERY_MRC_CACHE : select MRC_CLEAR_NORMAL_CACHE_ON_RECOVERY_RETRAIN Remove this for now.
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 51: config CHROMEOS : bool : default y : select GBB_FLAG_DISABLE_EC_SOFTWARE_SYNC No ChromeOS
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 60: config VARIANT_DIR : string : default "glkrvp" Shouldn't be needed
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 64: config DEVICETREE : string : default "devicetree.cb" Is the default value, this can be removed
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 74: Intel_Glkrvp Hmmmmmmmmmm...
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 83: : config IS_GLK_RVP_1 : bool "Is this RVP1?" : default n Not applicable
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 91: : config INCLUDE_NHLT_BLOBS : bool "Include blobs for audio" : select NHLT_DMIC_2CH_16B : select NHLT_DMIC_4CH_16B : select NHLT_MAX98357 Not applicable either.
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... File src/mainboard/microtech/etabpro/Kconfig.name:
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 3: select BOARD_INTEL_BASEBOARD_GLKRVP : select BASEBOARD_GLKRVP_LAPTOP These can go away
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... File src/mainboard/microtech/etabpro/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 4: romstage-$(CONFIG_CHROMEOS) += chromeos.c ChromeOS!
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... File src/mainboard/microtech/etabpro/boardid.c:
PS4: This shouldn't be needed.
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... File src/mainboard/microtech/etabpro/bootblock.c:
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 31: mainboard_ec_init(); This isn't needed for now.
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... File src/mainboard/microtech/etabpro/chromeos.c:
PS4: Not going to use chromeos
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... File src/mainboard/microtech/etabpro/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 32: /* CPU */ I would remove this comment, it is of no use.
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 44: /* Chrome OS specific */ : #include <vendorcode/google/chromeos/acpi/chromeos.asl> : : #include <southbridge/intel/common/acpi/sleepstates.asl> : : /* 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> : } Not going to need this either
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... File src/mainboard/microtech/etabpro/ec.h:
PS4: That's specific to chromeec, should also go away
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... File src/mainboard/microtech/etabpro/ec.c:
PS4: Same here, chromeec specific
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... File src/mainboard/microtech/etabpro/mainboard.c:
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... PS4, Line 23: : #include <soc/nhlt.h> : #include <vendorcode/google/chromeos/chromeos.h> : #include "ec.h" : #include "gpio.h" More Chromebook-specific code here.
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... File src/mainboard/microtech/etabpro/nhlt.c:
PS4: Not needed for now.
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... File src/mainboard/microtech/etabpro/smihandler.c:
PS4: This should not be needed either.
https://review.coreboot.org/c/coreboot/+/39219/4/src/mainboard/microtech/eta... File src/mainboard/microtech/etabpro/variants.h:
PS4: This should not be needed either.