Attention is currently required from: Alexander Couzens, Felix Singer.
Nicolas Provost has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75992?usp=email )
Change subject: new port for Lenovo L420 laptop ......................................................................
Patch Set 7:
(15 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/75992/comment/51eec87e_3b4f825a : PS3, Line 6:
Subject line should not end with a period.
Please fix.
Patchset:
PS4:
Hi Nicolas. Thanks for your contribution and welcome to coreboot! 😊 […]
Hi Felix. Thanks, I will check this.
File src/mainboard/lenovo/l420/Kconfig:
https://review.coreboot.org/c/coreboot/+/75992/comment/d2519e48_07ecfd7a : PS4, Line 24: # Workaround for EC/KBC.
remove the empty line and this comment
Done
https://review.coreboot.org/c/coreboot/+/75992/comment/a18f6a2c_e40aaa95 : PS4, Line 27: # Realtek 8168
remove the empty line and this comment
Done
https://review.coreboot.org/c/coreboot/+/75992/comment/12109366_4612134b : PS4, Line 55: int
no need to redefine the type, please remove
Done
https://review.coreboot.org/c/coreboot/+/75992/comment/d4202f80_dc94c304 : PS4, Line 59: int
no need to redefine the type, please remove
Acknowledged
https://review.coreboot.org/c/coreboot/+/75992/comment/25cc4573_d293a3e1 : PS4, Line 63: string
no need to redefine the type, please remove
Acknowledged
https://review.coreboot.org/c/coreboot/+/75992/comment/050a33b6_a0b5123f : PS4, Line 75: config WARNINGS_ARE_ERRORS : default n : : config PAYLOAD_CONFIGFILE : default
These shouldn't be in the mainboard Kconfig, please remove.
WARNINGS_ARE_ERRORS is needed while using a non-coreboot toolchain (for now I'm using the native toolchain on my x86 Linux). Anyway, I can remove it (but this option is not available by default).
File src/mainboard/lenovo/l420/board_info.txt:
https://review.coreboot.org/c/coreboot/+/75992/comment/e61b34f9_776b0692 : PS4, Line 5: Flashrom support: n
probably y?
Flashrom on internal (chipset) programmer will not work unless the rom is already unlocked (ME). But an external programmer works using flashrom..
File src/mainboard/lenovo/l420/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/75992/comment/e525b193_f84cff81 : PS4, Line 29: # IGD Displays (spread spectrum on) : register "gfx" = "GMA_STATIC_DISPLAYS(1)" : : # Enable DisplayPort Hotplug with 6ms pulse : register "gpu_dp_d_hotplug" = "0x06" : : # Enable Panel as LVDS and configure power delays : register "gpu_panel_port_select" = "PANEL_PORT_LVDS" : register "gpu_panel_power_cycle_delay" = "6" : register "gpu_panel_power_up_delay" = "300" # T1+T2: 30ms : register "gpu_panel_power_down_delay" = "300" # T5+T6: 30ms : register "gpu_panel_power_backlight_on_delay" = "2500" # T3: 200ms : register "gpu_panel_power_backlight_off_delay" = "2500" # T4: 200ms : register "gpu_cpu_backlight" = "0x1155" : r
Move these under the IGD controller
Done
https://review.coreboot.org/c/coreboot/+/75992/comment/91eea0df_4815f47b : PS4, Line 45: device domain 0 on
Sandybridge has a chipset device tree (at src/northbridge/intel/sandybridge/chipset. […]
Done
File src/mainboard/lenovo/l420/dsdt.asl:
https://review.coreboot.org/c/coreboot/+/75992/comment/88ace753_62de4233 : PS4, Line 24: // global NVS and variables
seems superfluous, remove
Done
https://review.coreboot.org/c/coreboot/+/75992/comment/74d13f5e_63608915 : PS4, Line 28: Scope (_SB) { : Device (PCI0) : { : #include <northbridge/intel/sandybridge/acpi/sandybridge.asl> : #include <southbridge/intel/bd82x6x/acpi/pch.asl> : #include <drivers/intel/gma/acpi/default_brightness_levels.asl> : } : }
Done
File src/mainboard/lenovo/l420/mainboard.c:
https://review.coreboot.org/c/coreboot/+/75992/comment/5271a1a6_4fff5345 : PS4, Line 9: // mainboard_enable is executed as first thing after : // enumerate_buses().
seems superfluous, remove
Done
File src/mainboard/lenovo/l420/smihandler.c:
https://review.coreboot.org/c/coreboot/+/75992/comment/5333a61e_2bba41e0 : PS4, Line 13: /* FIXME: check this */
checked?
Checking this, I remembered the ec on the L420 is not an H8 (as on my old Lenovo x220) but an IT8518. So more work to do..