Patrick Rudolph has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/23135 )
Change subject: superio: Add ASpeed AST2400 ......................................................................
Patch Set 28:
(6 comments)
https://review.coreboot.org/#/c/23135/23/src/superio/aspeed/ast2400/superio.... File src/superio/aspeed/ast2400/superio.c:
https://review.coreboot.org/#/c/23135/23/src/superio/aspeed/ast2400/superio.... PS23, Line 49: &ops
these should be replaced with NULL, since all LDNs use &ops passed to pnp_enable_devices below and n […]
Still applies
https://review.coreboot.org/#/c/23135/23/src/superio/aspeed/ast2400/superio.... PS23, Line 55: { &ops, AST2400_GPIO, PNP_IRQ0, },
the GPIO LDN doesn't have an IO region?
no IO region, please add a comment as well
https://review.coreboot.org/#/c/23135/28/src/superio/aspeed/common/Kconfig File src/superio/aspeed/common/Kconfig:
https://review.coreboot.org/#/c/23135/28/src/superio/aspeed/common/Kconfig@1... PS28, Line 18: # Generic Aspeed romstage driver - Just enough UART initialisation code for preram
https://review.coreboot.org/#/c/23135/27/src/superio/aspeed/common/early_ser... File src/superio/aspeed/common/early_serial.c:
https://review.coreboot.org/#/c/23135/27/src/superio/aspeed/common/early_ser... PS27, Line 19: * A generic romstage (pre-ram) driver for Aspeed variant Super I/O chips. preram, no need to mention romstage
https://review.coreboot.org/#/c/23135/27/src/superio/aspeed/common/early_ser... PS27, Line 38: #include <device/pnp.h> obsolete
https://review.coreboot.org/#/c/23135/27/src/superio/aspeed/common/early_ser... PS27, Line 40: #include <device/pnp_type.h> obsolete
-
Patrick Rudolph (Code Review)