Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33842 )
Change subject: src/superio/nuvoton: Add support for NCT5539D ......................................................................
Patch Set 8:
(5 comments)
https://review.coreboot.org/#/c/33842/8/src/superio/nuvoton/common/early_ser... File src/superio/nuvoton/common/early_serial.c:
https://review.coreboot.org/#/c/33842/8/src/superio/nuvoton/common/early_ser... PS8, Line 70: 0x40 0x00
https://review.coreboot.org/#/c/33842/8/src/superio/nuvoton/nct5539d/nct5539... File src/superio/nuvoton/nct5539d/nct5539d.h:
https://review.coreboot.org/#/c/33842/8/src/superio/nuvoton/nct5539d/nct5539... PS8, Line 29: BCLK bclk?
https://review.coreboot.org/#/c/33842/8/src/superio/nuvoton/nct5539d/nct5539... PS8, Line 50: #define NCT5539D_PCHDSW ((3 << 8) | NCT5539D_DS) : #define NCT5539D_DSWWOPT ((4 << 8) | NCT5539D_DS) : #define NCT5539D_DS3OPT ((5 << 8) | NCT5539D_DS) : #define NCT5539D_DSDSS ((6 << 8) | NCT5539D_DS) : #define NCT5539D_DSPU ((7 << 8) | NCT5539D_DS) haven't seen those in the datasheet
https://review.coreboot.org/#/c/33842/8/src/superio/nuvoton/nct5539d/superio... File src/superio/nuvoton/nct5539d/superio.c:
https://review.coreboot.org/#/c/33842/8/src/superio/nuvoton/nct5539d/superio... PS8, Line 60: PNP_IO1 I don't see a corresponding resource in the datasheet
https://review.coreboot.org/#/c/33842/8/src/superio/nuvoton/nct5539d/superio... PS8, Line 82: { NULL, NCT5539D_PCHDSW}, : { NULL, NCT5539D_DSWWOPT}, : { NULL, NCT5539D_DS3OPT}, : { NULL, NCT5539D_DSDSS}, : { NULL, NCT5539D_DSPU}, see the comment in the header file