Attention is currently required from: Máté Kukri (mkukri), Felix Held. Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55250 )
Change subject: [WIP] src/superio/smsc: Add support for the SCH5555 ......................................................................
Patch Set 1:
(11 comments)
File src/superio/smsc/sch5555/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/55250/comment/498bba9d_fac235d9 PS1, Line 4: sch5555_ramstage.c does not exist -> remove
File src/superio/smsc/sch5555/sch5555_bootblock.c:
https://review.coreboot.org/c/coreboot/+/55250/comment/50101d17_b99db070 PS1, Line 2: - no need to repeat the name in the filename - usually this file would be called early_init.c
https://review.coreboot.org/c/coreboot/+/55250/comment/c02345fc_3e34ef25 PS1, Line 12: smsc_conf_enter(); pnp_enter_conf_state; look at src/superio/smsc/sch5545/sch5545_early_init.c for an example
https://review.coreboot.org/c/coreboot/+/55250/comment/5c74117a_817f45ef PS1, Line 15: smsc_write8(LDN_SELECT, LDN_GLOBAL); use pnp_set_logical_device
https://review.coreboot.org/c/coreboot/+/55250/comment/24e8fbf5_f85a31e5 PS1, Line 19: smsc_write8(LDN_SELECT, LDN_LPCI); as above
https://review.coreboot.org/c/coreboot/+/55250/comment/21672dbe_22982f6f PS1, Line 23: smsc_conf_exit pnp_exit_conf_mode
https://review.coreboot.org/c/coreboot/+/55250/comment/b0f4eb0b_ed8408d5 PS1, Line 31: smsc_write8(LDN_SELECT, LDN_LPCI); : : switch (uart_no) { : case SCH5555_UART1: : smsc_write32(LPCI_UART1_BAR, serial_iobase << 16 | 0x8707); : smsc_write8(LDN_SELECT, LDN_UART1); : break; : case SCH5555_UART2: : smsc_write32(LPCI_UART2_BAR, serial_iobase << 16 | 0x8707); : smsc_write8(LDN_SELECT, LDN_UART2); : break; : } : : // Setup UART's configuration registers : smsc_write8(LDN_ACTIVE, 0x01); // Activate : smsc_write8(0x0f, 0x02); // Config select you could avoid this by letting the board pass the dev instead of just the uart_no. have a look at src/superio/aspeed/common/early_serial.c and src/mainboard/ocp/deltalake/bootblock.c for an example
https://review.coreboot.org/c/coreboot/+/55250/comment/9225791e_64ed5cfe PS1, Line 48: smsc_conf_exit as above
File src/superio/smsc/sch5555/sch5555_common.c:
https://review.coreboot.org/c/coreboot/+/55250/comment/442abe01_8f5af392 PS1, Line 6: // : // Super I/O register access : // nit: We usually use C89 style (C99 is allowed, though)
``` /* Super I/O register access */ ```
or
``` /* * Super I/O register access */ ```
https://review.coreboot.org/c/coreboot/+/55250/comment/69aa5135_9a81f27a PS1, Line 10: oid smsc_conf_enter(void) : { : outb(0x55, SMSC_INDEX); : } : : void smsc_conf_exit(void) : { : outb(0xaa, SMSC_INDEX); : } not sio-specific,make use of pnp_enter_conf_mode, pnp_exit_conf_mode instead (see src/superio/common,src/device/pnp_device.c)
https://review.coreboot.org/c/coreboot/+/55250/comment/8c5f8d8c_516bfe03 PS1, Line 52: sc_write8(offset, value & 0xff); : smsc_writ unsure, but might outw work?
-
Michael Niewöhner (Code Review)