Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/11791 )
Change subject: mainboard/lenovo/t410: Add new port ......................................................................
Patch Set 23:
(8 comments)
https://review.coreboot.org/c/coreboot/+/11791/21/src/mainboard/lenovo/t410/... File src/mainboard/lenovo/t410/acpi/ec.asl:
https://review.coreboot.org/c/coreboot/+/11791/21/src/mainboard/lenovo/t410/... PS21, Line 19: Scope(_SB.PCI0.LPCB.EC) : { : }
Maybe put the dock. […]
removed
https://review.coreboot.org/c/coreboot/+/11791/23/src/mainboard/lenovo/t410/... File src/mainboard/lenovo/t410/devicetree.cb:
https://review.coreboot.org/c/coreboot/+/11791/23/src/mainboard/lenovo/t410/... PS23, Line 120:
remove tab and fix alignment of things below.
Done
https://review.coreboot.org/c/coreboot/+/11791/21/src/mainboard/lenovo/t410/... File src/mainboard/lenovo/t410/hda_verb.c:
https://review.coreboot.org/c/coreboot/+/11791/21/src/mainboard/lenovo/t410/... PS21, Line 25: /
I think you can remove all of these comments
Done
https://review.coreboot.org/c/coreboot/+/11791/10/src/mainboard/lenovo/t410/... File src/mainboard/lenovo/t410/mainboard.c:
https://review.coreboot.org/c/coreboot/+/11791/10/src/mainboard/lenovo/t410/... PS10, Line 99: outl(0, pmbase + SMI_EN);
write_pmbase32(SMI_EN, 0); […]
removed
https://review.coreboot.org/c/coreboot/+/11791/10/src/mainboard/lenovo/t410/... PS10, Line 108: /* If we're resuming from suspend, blink suspend LED */
Copied as is from x201/mainboard.c. I personally have no idea what the original author meant here.
removed
https://review.coreboot.org/c/coreboot/+/11791/21/src/mainboard/lenovo/t410/... File src/mainboard/lenovo/t410/romstage.c:
https://review.coreboot.org/c/coreboot/+/11791/21/src/mainboard/lenovo/t410/... PS21, Line 54: : pci_write_config32(PCH_LPC_DEV, LPC_GEN1_DEC, 0x7c1601); : pci_write_config32(PCH_LPC_DEV, LPC_GEN2_DEC, 0xc15e1); : pci_write_config32(PCH_LPC_DEV, LPC_GEN3_DEC, 0x1c1681); : pci_write_config32(PCH_LPC_DEV, LPC_GEN4_DEC, (0x68 & ~3) | 0x00040001);
Any idea if you need those during romstage as there is ramstage code to set those up based on dt (co […]
15e0 is used by pmh7_* functions. Sounds like a good plan to move that to romstage, like done on bd82x6x
https://review.coreboot.org/c/coreboot/+/11791/23/src/mainboard/lenovo/t410/... File src/mainboard/lenovo/t410/romstage.c:
https://review.coreboot.org/c/coreboot/+/11791/23/src/mainboard/lenovo/t410/... PS23, Line 28: : /* Seems copied from Lenovo Thinkpad x201, might be wrong */
fix?
Done
https://review.coreboot.org/c/coreboot/+/11791/18/src/mainboard/lenovo/t410/... File src/mainboard/lenovo/t410/thermal.h:
https://review.coreboot.org/c/coreboot/+/11791/18/src/mainboard/lenovo/t410/... PS18, Line 4: * Copyright (C) 2008-2009 coresystems GmbH : * Copyright (C) 2011 The Chromium OS Authors. All rights reserved. : * Copyright (C) 2014 Vladimir Serbinenko : * Copyright (C) 2016 Patrick Rudolph siro@das-labor.org : * Copyright (C) 2017 James Ye jye836@gmail.com : *
makes little sense here?
Done
-
Patrick Rudolph (Code Review)