Marty Plummer has posted comments on this change. ( https://review.coreboot.org/17669 )
Change subject: superio/fintek: Add support for Fintek F71889A. ......................................................................
Patch Set 9:
(2 comments)
is there any board actually using this superio chip? if no mainboard selects this chip, the code won't get build-tested...
No, as of right now no board makes use of this superio (afaik). I'm working on it because my current board makes use of it, but there is a lot of groundwork to be dealt with (mostly am3+ ddr3 init and the glue code that makes all the things work together) before there will be a board that works.
https://review.coreboot.org/#/c/17669/6/src/superio/fintek/f71889a/superio.c File src/superio/fintek/f71889a/superio.c:
PS6, Line 57: 0x07f8
this reserves only 1 byte of io space; the device also uses the byte at pos
Could you explain this a bit? I based this code largely (read: exact same with a bit of tweaking to match datasheets to the best of my understanding) on the f71869ab code and both the f71889a and it are very similar for the KBC register, so I can't see how the f71869ad uses one byte and the f71889a would use two. (or for that matter, why 0x07ff signifies one byte)
https://review.coreboot.org/#/c/17669/7/src/superio/fintek/f71889a/superio.c File src/superio/fintek/f71889a/superio.c:
Line 60: { &ops, F71889A_CIR, PNP_IO0 | PNP_IRQ0, {0x7f8, 0}, },
the io mask is missing here
Ah, so it is! Unsure here, so I assume a single byte is enough. It has been added.