Attention is currently required from: Máté Kukri, Paul Menzel, Angel Pons, Michael Niewöhner, Felix Held. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55250 )
Change subject: superio/smsc: Add support for the SCH555x series ......................................................................
Patch Set 12:
(7 comments)
File src/superio/smsc/sch5555/sch5555.h:
https://review.coreboot.org/c/coreboot/+/55250/comment/1ffaad86_72719742 PS1, Line 51: #define EMI_IOBASE 0xa00 : #define RUNTIME_IOBASE 0xa40
Alright, I'll revisit if I get bored enough
Can't we set them as io resources in the devicetree? (and then read that in C code)
File src/superio/smsc/sch555x/bootblock.c:
https://review.coreboot.org/c/coreboot/+/55250/comment/35cfde8c_9e461541 PS12, Line 35: // Enable IRQs Why?
(also, mixing comment styles is a little irritating)
File src/superio/smsc/sch555x/ramstage.c:
https://review.coreboot.org/c/coreboot/+/55250/comment/811b7014_9afea6c6 PS12, Line 1: ;; w-what? :)
https://review.coreboot.org/c/coreboot/+/55250/comment/b47c79b5_64ca8bb6 PS12, Line 13: pc_keyboard_init(NO_AUX_DEVICE); Why add untested code?
https://review.coreboot.org/c/coreboot/+/55250/comment/46a24cbb_29c37c2e PS12, Line 74: 0x70 PNP_IDX_IRQ0
https://review.coreboot.org/c/coreboot/+/55250/comment/fbfd46a1_5d4c96f6 PS12, Line 77: 0x72 PNP_IDX_IRQ1
File src/superio/smsc/sch555x/sch555x.h:
PS12: This file seems to mix internals and the mainboard API.
-
Nico Huber (Code Review)