Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/34888 )
Change subject: util/superiotool: add IT8987 detection and register support ......................................................................
Patch Set 5:
(5 comments)
https://review.coreboot.org/c/coreboot/+/34888/5/util/superiotool/ite.c File util/superiotool/ite.c:
https://review.coreboot.org/c/coreboot/+/34888/5/util/superiotool/ite.c@1067 PS5, Line 1067: {0x8987, "IT8987", { Minor: Maybe add a comment noting that global registers 0x24, 0x27, 0x28, 0x29, 0x2a, 0x2b are reserved (if that's not implied already by their absence)
https://review.coreboot.org/c/coreboot/+/34888/5/util/superiotool/ite.c@1092 PS5, Line 1092: {NOLDN, "Logical Device Activate Register (LDA)", : {0x30,EOT}, : {0x00,EOT}}, : {NOLDN, "I/O Port Base Address for Descriptor 0 (IOBAD0)", : {0x60,0x61,EOT}, : {NANA,NANA,EOT}}, : {NOLDN, "I/O Port Base Address for Descriptor 1 (IOBAD1)", : {0x62,0x63,EOT}, : {NANA,NANA,EOT}}, : {NOLDN, "Interrupt Request Number and Wake-Up on IRQ Enable (IRQNUMX)", : {0x70,EOT}, : {NANA,EOT}}, : {NOLDN, "Interrupt Request Type Select (IRQTP)", : {0x71,EOT}, : {NANA,EOT}}, : {NOLDN, "DMA Channel Select 0 (DMAS0)", : {0x74,EOT}, : {0x04,EOT}}, : {NOLDN, "DMA Channel Select 1 (DMAS1)", : {0x75,EOT}, : {0x04,EOT}},
this is all specific to the different LDNs. […]
For context: These are specified in the datasheet as the common fields of the LDN-specific registers. So they're still LDN-specific, but you know that 0x71 means "IRQ number" everywhere (as opposed to the 0xf* registers, whose meaning is LDN-specific)
The idea is that the first registers are like a "header" and the registers for each LDN (0x30 onwards) are on different pages. You can choose between different pages using register 0x07. To get an idea of what this looks like check: Figure 6-1. Host View Register Map via Index-Data Pair
https://review.coreboot.org/c/coreboot/+/34888/5/util/superiotool/ite.c@1131 PS5, Line 1131: {0x11, "Power Management I/F Channel 1 (PMC1)", Note for reviewers: PMC1 does lack the 0x64, 0x65, 0xf0 registers that other PMCs have.
https://review.coreboot.org/c/coreboot/+/34888/5/util/superiotool/ite.c@1143 PS5, Line 1143: {0x17, "Power Management I/F Channel 3 (PMC3)", Same note as PMC1
https://review.coreboot.org/c/coreboot/+/34888/5/util/superiotool/ite.c@1149 PS5, Line 1149: 4 (PMC4 Copypasta went wrong? s/4/5/g