[coreboot-gerrit] Change in coreboot[master]: util/superiotool/fintek.c: Add support for F71889A

[Felix Held (Code Review)]

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


-- 
To view, visit https://review.coreboot.org/17442
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ifc5035ebe2c1c66213069567da757886923e5596
Gerrit-PatchSet: 9
Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Owner: Marty Plummer <ntzrmtthihu777 at gmail.com>
Gerrit-Reviewer: Felix Held <felix-coreboot at felixheld.de>
Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki at gmail.com>
Gerrit-Reviewer: Marty Plummer <ntzrmtthihu777 at gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h at gmx.de>
Gerrit-Reviewer: Paul Menzel <paulepanter at users.sourceforge.net>
Gerrit-Reviewer: build bot (Jenkins) <no-reply at coreboot.org>
Gerrit-HasComments: Yes