Attention is currently required from: Hung-Te Lin, Rex-BC Chen, Paul Menzel, Angel Pons, Yu-Ping Wu. Jianjun Wang has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/63019 )
Change subject: soc/mediatek: Add mtk_early_init for passing data across sessions ......................................................................
Patch Set 2:
(8 comments)
File src/soc/mediatek/common/Kconfig:
https://review.coreboot.org/c/coreboot/+/63019/comment/0949e6d5_115dbd1c PS1, Line 53: config MEDIATEK_EARLY_INIT : bool : help : This option allows passing data across sessions.
But the region may not exist on other MTK SoCs?
So can we keep it here?
File src/soc/mediatek/common/early_init.c:
https://review.coreboot.org/c/coreboot/+/63019/comment/050f5e91_637e06d0 PS1, Line 11: assert
This could be _Static_assert.
As discussed, _Static_assert cannot build pass with REGION_SIZE().
https://review.coreboot.org/c/coreboot/+/63019/comment/262ab88c_45ebceef PS1, Line 25: void mtk_pcie_save_timestamp(void)
Absolutely. This function doesn't belong here.
Renamed to a common one
https://review.coreboot.org/c/coreboot/+/63019/comment/dd3a19a2_d142f3ca PS1, Line 27: timestamp
cur_time
Done
https://review.coreboot.org/c/coreboot/+/63019/comment/e7a3d911_f14beae3 PS1, Line 35: microseconds
We shouldn't access the field directly (see the comment in include/timer.h). […]
Done
https://review.coreboot.org/c/coreboot/+/63019/comment/714b1f76_fd4a2738 PS1, Line 48: return cur_time.microseconds - init->pcie_timestamp_us;
we have to add a special case here - if pcie_timestamp_us is 0 then we should return 0. […]
Done
File src/soc/mediatek/common/include/soc/early_init.h:
https://review.coreboot.org/c/coreboot/+/63019/comment/0d5e9a53_d90f5705 PS1, Line 10: mtk_early_init
If you plan to move things like DRAM size as well then I'd agree presistent_data may be a good regio […]
Done
https://review.coreboot.org/c/coreboot/+/63019/comment/143283ac_fe277740 PS1, Line 17: mtk_pcie_save_timestamp
This also works. Then we can keep these functions in this file.
That's really a good idea, thanks.