Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/11791 )
Change subject: mainboard/lenovo/t410: Add new port ......................................................................
Patch Set 17:
(19 comments)
So is this tested? Skipping the romstage RCBA replay makes me wonder... (I documented quite a few of these and am in the process of cleaning it up)
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... File src/mainboard/lenovo/t410/Kconfig:
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 34: MMCONF_BASE_ADDRESS Care to send a separate patch to move this to nb/nehalem/kconfig?
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 42: config MAX_CPUS : int : default 4 : : config CPU_ADDR_BITS : int : default 36 To me it makes more sense to put these in cpu/intel/model_2056x.
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...
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 ?
https://review.coreboot.org/c/coreboot/+/11791/4/src/mainboard/lenovo/t410/g... File src/mainboard/lenovo/t410/gpio.c:
https://review.coreboot.org/c/coreboot/+/11791/4/src/mainboard/lenovo/t410/g... PS4, Line 61: .gpio9 = GPIO_DIR_INPUT, not meaningful here. You can generally hack autoport (add PCI ID) to have it generate this file for you.
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. if you hook up the LPC PCI id in autoport you should be able to generate this file. I have a poorly written python script (but works) that can output this file if you desire it.
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?
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
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 74: static const u32 rcba_dump3[] = { : /* 3310 */ 0x02060100, 0x0000000f, 0x01020000, 0x80000000, : /* 3320 */ 0x00000000, 0x04000000, 0x00000000, 0x00000000, : /* 3330 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3340 */ 0x000fffff, 0x00000000, 0x00000000, 0x00000000, : /* 3350 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3360 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3370 */ 0x00000000, 0x00000000, 0x7f8fdfff, 0x00000000, : /* 3380 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3390 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 33a0 */ 0x00003900, 0x00000000, 0x00000000, 0x00000000, : /* 33b0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 33c0 */ 0x00010000, 0x00000000, 0x00000000, 0x0001004b, : /* 33d0 */ 0x06000008, 0x00010000, 0x00000000, 0x00000000, : /* 33e0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 33f0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3400 */ 0x0000001c, 0x00000080, 0x00000000, 0x00000000, : /* 3410 */ 0x00000c61, 0x00000000, 0x16e41fe1, 0xbf4f001f, : /* 3420 */ 0x00000000, 0x00060010, 0x0000000d, 0x00000000, : /* 3430 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3440 */ 0xdeaddeed, 0x00000000, 0x00000000, 0x00000000, : /* 3450 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3460 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3470 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3480 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3490 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 34a0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 34b0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 34c0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 34d0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 34e0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 34f0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3500 */ 0x2000055f, 0x2000055f, 0x2000074b, 0x2000014b, : /* 3510 */ 0x2000014b, 0x2000074b, 0x2000074b, 0x2000074b, : /* 3520 */ 0x2000055f, 0x2000055f, 0x2000074b, 0x2000074b, : /* 3530 */ 0x20000557, 0x2000055f, 0x00000000, 0x00000000, : /* 3540 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3550 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3560 */ 0x00000001, 0x000026a3, 0x00040002, 0x01000052, : /* 3570 */ 0x02000772, 0x16000f8f, 0x1800ff4f, 0x0001d630, : /* 3580 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3590 */ 0x00000003, 0x00000000, 0x00000000, 0x00000000, : /* 35a0 */ 0xfc000201, 0x3c000201, 0x00000000, 0x00000000, : /* 35b0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 35c0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 35d0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 35e0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 35f0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3600 */ 0x0a001f00, 0x00000000, 0x00000000, 0x00000001, : /* 3610 */ 0x00010000, 0x00000000, 0x00000000, 0x00000000, : /* 3620 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3630 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3640 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3650 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3660 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3670 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3680 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3690 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 36a0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 36b0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 36c0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 36d0 */ 0x00000000, 0x089c0018, 0x00000000, 0x00000000, : /* 36e0 */ 0x11111111, 0x00000000, 0x00000000, 0x00000000, : /* 36f0 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : /* 3700 */ 0x00000000, 0x00000000, 0x00000000, 0x00000000, : }; Does it work without this?
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 141: sizeof(rcba_dump3) That is 0 right?
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
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
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 251: write_acpi16(0x2, 0x0); clear PM1_EN
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 252: 0x28 GPE0_EN
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 253: 0x2c GPE0_EN + 4 (8byte reg)
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 258: 0x900 hmm bit12 is undocumented...
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 259: 0xffff7ffe clear GPE0_STS
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 259: 0x20 GPE0_STS
https://review.coreboot.org/c/coreboot/+/11791/17/src/mainboard/lenovo/t410/... PS17, Line 260: 0x34 SMI_STS