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 18:
(15 comments)
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... File src/mainboard/lenovo/t410/acpi/platform.asl:
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 17: /* These come from the dynamically created CPU SSDT */ : External(PDC0) : External(PDC1)
I think we got rid of that...
Done
https://review.coreboot.org/c/coreboot/+/11791/4/src/mainboard/lenovo/t410/c... File src/mainboard/lenovo/t410/cmos.layout:
https://review.coreboot.org/c/coreboot/+/11791/4/src/mainboard/lenovo/t410/c... PS4, Line 115: 10 3 128M
any reason to skip 256M ?
The x201 doesn't have it either
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... File src/mainboard/lenovo/t410/gpio.c:
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 63: .gpio9 = GPIO_DIR_INPUT,
Not meaningful when GPIO_MODE_NATIVE. […]
Done
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 135: .gpio2 = GPIO_NO_INVERT,
maybe hide the NO_INVERT ones?
Done
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... File src/mainboard/lenovo/t410/romstage.c:
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 53: LPC_GEN1_DEC
Outside scope of this patch, but it would be nice to move that to SB code like bd82x6x
Ack
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 141: sizeof(rcba_dump3)
That is 0 right?
no
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 146: : static inline void write_acpi32(u32 addr, u32 val) : { : outl(val, DEFAULT_PMBASE | addr); : } : : static inline void write_acpi16(u32 addr, u16 val) : { : outw(val, DEFAULT_PMBASE | addr); : } : : static inline u32 read_acpi32(u32 addr) : { : return inl(DEFAULT_PMBASE | addr); : }
read/write_pmbase_x
Done
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 225: Read PM1_CNT
sidenote: clearing this bit makes raminit fail, so take care or add a comment
Done
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 251: write_acpi16(0x2, 0x0);
clear PM1_EN
Done
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 252: 0x28
GPE0_EN
Done
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 253: 0x2c
GPE0_EN + 4 (8byte reg)
Done
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 258: 0x900
hmm bit12 is undocumented...
Done
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 259: 0xffff7ffe
clear GPE0_STS
Done
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 259: 0x20
GPE0_STS
Done
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 260: 0x34
SMI_STS
Done