Felix Held 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 7: Code-Review+2
(4 comments)
since I don't have the datasheet, I can't verify the default values and the registers being present in the chip in this patch; what I could verify looked good to me in this patchset
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 reser […]
Done
https://review.coreboot.org/c/coreboot/+/34888/5/util/superiotool/ite.c@1068 PS5, Line 1068: {NOLDN, "Chip ID", : {0x20,0x21, EOT}, : {0x89,0x87, EOT}},
might be a bit redundant, since it's used to detect the chip. […]
Done
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}},
Which explains why other models usually leave these out...I'll fix and push shortly.
Done
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
Done