Bill XIE has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36093 )
Change subject: mb/lenovo/x200: Add ThinkPad X301 as a variant ......................................................................
Patch Set 4:
(9 comments)
Patch Set 3:
For x301 gpio.c: gpio19,22,60 should be GPI (general purpose input), it's pulled high on the board gpio32 is set as GPI, should be native - CLKRUN# (if this one fails I blame EC FW, TPM chipie and intel strongly encourages its use)
Unused native functions (all pulled high, so GPI): gpio19,22,23 Unused USB OC# (all pulled high, so GPI): gpio29,31,41,43,44,45,46,47 Unused legacy PCI: gpio50,52,54 should be GPI, and gpio51,53,55 GPO High
https://review.coreboot.org/c/coreboot/+/36093/3/Documentation/mainboard/len... File Documentation/mainboard/lenovo/x301.md:
https://review.coreboot.org/c/coreboot/+/36093/3/Documentation/mainboard/len... PS3, Line 7: x301_kb_removed
Add something more complete: "X301 with WSON8 chip replaces with SOIC8 chip"
done.
https://review.coreboot.org/c/coreboot/+/36093/3/Documentation/mainboard/len... PS3, Line 20: Otherwise, it may be recommended to replace it with a : SOIC-8 one.
You should mention that you might need to add the chip to the IFD VSCC list, https://review. […]
done.
https://review.coreboot.org/c/coreboot/+/36093/3/Documentation/mainboard/len... PS3, Line 39: : [T420 / T520 / X220 / T420s / W520 common]: xx20_series.md
Stale comment?
done.
https://review.coreboot.org/c/coreboot/+/36093/3/Documentation/mainboard/len... File Documentation/mainboard/lenovo/x301_kb_removed.jpg:
PS3:
Reduce image size to about 100KiB
Does it need to shrink more?
https://review.coreboot.org/c/coreboot/+/36093/4/src/mainboard/lenovo/x200/K... File src/mainboard/lenovo/x200/Kconfig:
https://review.coreboot.org/c/coreboot/+/36093/4/src/mainboard/lenovo/x200/K... PS4, Line 26: select H8_DOCK_EARLY_INIT if BOARD_LENOVO_X301 This is the "dirty hack" I use to fight an old dirty hack -- H8_DOCK_EARLY_INIT, which may need repurposing for devices using h8 but without a dock, like x301.
https://review.coreboot.org/c/coreboot/+/36093/3/src/mainboard/lenovo/x200/M... File src/mainboard/lenovo/x200/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/36093/3/src/mainboard/lenovo/x200/M... PS3, Line 16: variants/$(VARIANT_DIR)/dock.c
I think defining a Kconfig in the mainboard Kconfig file "HAS_DOCK_CONNECTOR" and conditionally incl […]
I agree with you, but now I may have to use dirty hack to fight another widely used dirty hack.
https://review.coreboot.org/c/coreboot/+/36093/3/src/mainboard/lenovo/x200/M... PS3, Line 18: blc.c
You might want to add your EDID string to this list to have a properly 'tuned' backlight pwm frequen […]
Done
https://review.coreboot.org/c/coreboot/+/36093/3/src/mainboard/lenovo/x200/b... File src/mainboard/lenovo/x200/board_info.txt:
https://review.coreboot.org/c/coreboot/+/36093/3/src/mainboard/lenovo/x200/b... PS3, Line 2: ThinkPad X200 baseboard
Thinkpad X200/X200T/X200S/X301
Done
https://review.coreboot.org/c/coreboot/+/36093/3/src/southbridge/intel/i8280... File src/southbridge/intel/i82801ix/nvs.h:
https://review.coreboot.org/c/coreboot/+/36093/3/src/southbridge/intel/i8280... PS3, Line 137: global_nvs_t
This reminds me that this has to go to the sb dir.
What should I do here?