Michał Żygowski has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/40351 )
Change subject: mb/dell/optiplex_9010: Add Dell OptiPlex 9010 SFF support ......................................................................
Patch Set 9:
(2 comments)
https://review.coreboot.org/c/coreboot/+/40351/9/src/mainboard/dell/optiplex... File src/mainboard/dell/optiplex_9010/romstage.c:
https://review.coreboot.org/c/coreboot/+/40351/9/src/mainboard/dell/optiplex... PS9, Line 17: uint16_t ec_fw_version; : : sch5545_emi_init(0x2e); : if (sch5545_emi_get_int_mask_high()) : printk(BIOS_SPEW, "EC interrupt mask MSB is not 0\n"); : : sch5545_emi_disable_interrupts(); : sch5545_ec_early_init(); : sch5545_ec_hwm_early_init(); : : if (!s3resume) { : ec_fw_version = sch5545_get_ec_fw_version(); : printk(BIOS_DEBUG, "SCH5545 EC firmware version %04x\n", ec_fw_version); : sch5545_update_ec_firmware(ec_fw_version); : } : printk(BIOS_DEBUG, "EC early init complete.\n");
Is there a reason to do this during the romstage?
IMO bootblock should do only basic initialization of CAR and Super I/O. Putting more to the bootblock makes no sense. Also the initialization and firmware update must be done quite early, because EC awaits for communication from the host. If it doesn't receive the required initialization sequence after few seconds from system reset, it enters kind of error path. The error path means the fans will be spinned up to high speed and EC does not respond to any messages sent by host anymore. So it is not good to do it later (i.e. ramstage) if EC already entered error path.
https://review.coreboot.org/c/coreboot/+/40351/9/src/mainboard/dell/optiplex... PS9, Line 39: : if (CONFIG(CONSOLE_SERIAL)) : sch5545_enable_uart(0x2e, 0);
This is also done in the bootblock?
Yes, I am replicating it here because of an issue described above. It gives a chance to catch some debug on serial after power failure. I need more experiments with EC initialization to catch the moment where UART IO base can be programmed. For now I do not use serial port in favor of USB debug. This is something I would like to fix in the nearest future, but I don't want to hold off with the patches anymore (my initial work started with coreboot ~4.10).