Attention is currently required from: Arthur Heymans, Maulik V Vaghela, Paul Menzel, Angel Pons, Subrata Banik, Michael Niewöhner, Patrick Rudolph, Nico Huber, Mario Scheithauer, Lean Sheng Tan, Werner Zeh, Felix Held. Martin Roth has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/55367 )
Change subject: soc/intel/elkhartlake: Introduce Intel PSE ......................................................................
Patch Set 52:
(2 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/55367/comment/0fa1fcbb_f57ff5e4 PS52, Line 9: The Intel® Programmable Services Engine (Intel® PSE) is a : dedicated offload engine for IoT functions powered by an ARM : Cortex-M7 microcontroller. It provides independent, low-DMIPS : computing and low-speed I/Os for IoT applications, plus : dedicated services for real-time computing and time-sensitive : synchronization. : : The PSE hosts new functions, including remote out-of-band : device management, network proxy, embedded controller lite : and sensor hub. : : This CL enables the user to provide the base address of the : PSE FW blob which will then be loaded by the FSP-S onto the : ARM controller. PSE FW will do the initialization work of : PSE controller and it's peripherals. The loading of PSE FW : should have negligible impact on boot time unless PSE : controller could not locate PSE FW and FSP will attempt to : redo PSE FW loading and wait for PSE handshake until it times : out. Once PSE controller locate PSE FW, it will do initialization : concurrently by itself with coreboot booting. : : It also adds PSE related FSP-S UPD settings which enables the : setup of peripheral ownership (assigned to the PSE or x86 : subsystem) and interrupts. These assignments need to take : place at a given point in the boot process and cannot be : changed later. : : To verify if PSE FW is loaded properly, the user could enable : PchPseShellEnabled flag and the log will be printed at PSE UART 2. : : For further info please refer to doc #611825 (for HW overview) : and #614110 (for PSE EDS).
Isn't the limit 72 characters per line?
Yes, 72 characters is the preferred length.
https://doc.coreboot.org/getting_started/writing_documentation.html#basic-an...
File src/soc/intel/elkhartlake/Makefile.inc:
https://review.coreboot.org/c/coreboot/+/55367/comment/582e7ec8_16318701 PS2, Line 58: pse.bin-align := 0x1000
Looks like FSP copies the PSE FW to RAM, so the alignment of the file in CBFS shouldn't matter. […]
I agree with Angel here. Adding an artificial alignment requirement will tend to waste space in CBFS. It's not a big deal, but since it's actually being decompressed, I don't see any reason for the alignment requirement. Maybe at one point it wasn't being compressed and was being loaded directly from the rom space?
-
Martin Roth (Code Review)