[coreboot-gerrit] Change in coreboot[master]: mainboard: add support for lenovo x1 carbon gen 1

[Alexander Couzens (Code Review)]

Alexander Couzens has posted comments on this change. ( https://review.coreboot.org/16994 )

Change subject: mainboard: add support for lenovo x1 carbon gen 1
......................................................................


Patch Set 4:

(4 comments)

https://review.coreboot.org/#/c/16994/4//COMMIT_MSG
Commit Message:

PS4, Line 22: check why device consumes more power (+ 3W while idle)
> Maybe c-state related? check if linux uses cpuidle (Using ACPI) or intel_id
cat /sys/devices/system/cpu/cpuidle/current_driver
intel_idle


https://review.coreboot.org/#/c/16994/4/src/mainboard/lenovo/x1_carbon_gen1/gpio.c
File src/mainboard/lenovo/x1_carbon_gen1/gpio.c:

PS4, Line 67: .gpio9  = GPIO_DIR_INPUT,
> Not meaningful when mode is native. You could declare it static and leave t
this is basicly copy-pasted from x230. I would like to create this file out of the vendor bios or look it up from the schematics.


PS4, Line 127: const struct pch_gpio_set1 pch_gpio_set1_invert = {
> Should this not be declared as static so empty field are initialised?
isn't const doing the same?


https://review.coreboot.org/#/c/16994/4/src/mainboard/lenovo/x1_carbon_gen1/spd/elpida.hex
File src/mainboard/lenovo/x1_carbon_gen1/spd/elpida.hex:

Line 1: 92 11 0b 03 04 00 00 01 03 52 01 08 0c 00 20 00 
> Out of pure interest: how do you come by this?
inteltool can export these when running the vendor uefi/bios.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I97c8e01a3ce0577d7dc9e8df7d33db3b155fe3d6
Gerrit-PatchSet: 4
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Alexander Couzens <lynxis at fe80.eu>
Gerrit-Reviewer: Alexander Couzens <lynxis at fe80.eu>
Gerrit-Reviewer: Arthur Heymans <arthur at aheymans.xyz>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins)
Gerrit-HasComments: Yes