Felix Held has posted comments on this change. ( https://review.coreboot.org/17442 )
Change subject: util/superiotool/fintek.c: Add support for F71889A ......................................................................
Patch Set 9:
(6 comments)
https://review.coreboot.org/#/c/17442/9/util/superiotool/fintek.c File util/superiotool/fintek.c:
PS9, Line 475: 0x00 one bit of the register 0x28 is determined by some hardware strap, probably NANA would be better here
PS9, Line 499: NANA the value for 0xa6 should probably be 0x00
PS9, Line 491: {0x06, "GPIO", : {0xf0, 0xf1, 0xf2, 0xf3, 0xfe, 0xff, 0xe0, 0xe1, 0xe2, : 0xe3, 0xd0, 0xd1, 0xd2, 0xd3, 0xc0, 0xc1, 0xc2, 0xc3, : 0xb0, 0xb1, 0xb2, 0xa0, 0xa1, 0xa2, 0xa4, 0xa5, 0xa6, : 0xab, 0xac, 0xad, 0xae, 0xaf, 0x90, 0x91, 0x92, 0x93, : 0x80, 0x81, 0x82, 0x83, EOT}, : {0x00, 0x7f, NANA, 0x00, 0x00, 0x00, 0x00, 0x7f, NANA, : 0x00, 0x00, 0xe0, NANA, 0x00, 0x00, 0xff, NANA, 0x00, : 0x00, 0xff, NANA, 0x00, 0x1f, NANA, 0x00, 0x00, NANA, : 0x00, 0x00, 0x00, 0xe0, 0x00, 0x00, 0x00, NANA, 0x00, : 0x00, 0xff, NANA, 0x00, EOT} }, ordering these so that the addresses increase and are not in a slightly random order would probably be a good idea
PS9, Line 505: MISC I'm unsure about this byte; might also be 0x00, but it isn't really documented
PS9, Line 517: 0x07 0x07 or 0x00 that's the question. the datasheet has both values for this register...
PS9, Line 512: {0x0a, "PME, ACPI, and ERP", : {0x30, 0xf0, 0xf1, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7, : 0xfa, 0xfc, 0xfe, 0xe0, 0xe1, 0xe2, 0xe3, 0xe4, 0xe5, : 0xe6, 0xe7, 0xe8, 0xe9, 0xec, 0xed, 0xee, EOT}, : {0x00, 0x00, NANA, 0x00, NANA, 0x26, 0x04, 0x07, 0x08, : 0x00, 0x07, 0x00, 0x80, 0xcc, NANA, 0x13, 0x09, 0xc7, : 0x09, 0x63, 0x10, 0xff, NANA, 0x00, 0x00, EOT} }, maybe order the address/value pairs. address 0xfd with data 0x00 missing