Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33640 )
Change subject: superio/nuvoton/nct6791d: Add symbol to select COM port ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/#/c/33640/1/src/superio/nuvoton/common/early_ser... File src/superio/nuvoton/common/early_serial.c:
https://review.coreboot.org/#/c/33640/1/src/superio/nuvoton/common/early_ser... PS1, Line 73: Route GPIO8 pin group to COM A
isn't it the other way around? COM A gets routed to the pins of that GPIO group
I understand this as "Route the pins which correspond to the GPIO8 group to the COM A function." Sure, the wording in that comment might be imprecise.
https://review.coreboot.org/#/c/33640/1/src/superio/nuvoton/common/early_ser... PS1, Line 74: pnp_write_config(dev, 0x2a, 0x00);
hmm, would it be better to only modify the bits corresponding to the serial port pinmux?
I thought so, but has anybody initialized the superio at such an early stage? the other bits aren't changed from the reset default so it doesn't matter much I think.
-
Angel Pons (Code Review)