Change in coreboot[master]: soc/intel/common: Add support for CSE IOM/NPHY sub-parition update
Attention is currently required from: Tim Wawrzynczak, Julius Werner, Sridhar Siricilla. Krishna P Bhat D has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/59685 )
Change subject: soc/intel/common: Add support for CSE IOM/NPHY sub-parition update ......................................................................
Patch Set 14:
(9 comments)
Patchset:
PS11:
Well, what would you prefer: that you make this change now and test it on your hardware, or that I m […]
Agreed.
File src/soc/intel/common/block/cse/cse_lite.c:
https://review.coreboot.org/c/coreboot/+/59685/comment/eefc70c0_9407cd21 PS13, Line 406: __weak bool is_cse_sub_part_update_req(void)
This doesn't seem like a good use case for weak functions (see CB:58934 for some ongoing discussion […]
Done
https://review.coreboot.org/c/coreboot/+/59685/comment/f8e19b5a_5d4f8c66 PS13, Line 755: static ssize_t rdev_readat_offset(const struct region_device *rd, void *b, size_t offset,
Why are you redefining a well-known existing function with a different name, anyway? All this achiev […]
Done
https://review.coreboot.org/c/coreboot/+/59685/comment/2f194975_c11d81f9 PS13, Line 833: static const char *get_source_rdev_fmap_name(void)
Get rid of this function. Don't reimplement what vboot and the CBFS core are already doing for you.
Done
https://review.coreboot.org/c/coreboot/+/59685/comment/8e4708a9_d6d960b9 PS13, Line 854: static bool cse_sub_part_get_source_rdev(struct region_device *rdev, const char *name)
Get rid of this function. The new CBFS API intentionally abstracts away from raw rdevs. […]
Done
https://review.coreboot.org/c/coreboot/+/59685/comment/4d26e54f_5c4158ab PS13, Line 932: if (!cse_sub_part_get_source_rdev(&source_rdev, name))
If you want to get the `name` CBFS file from the currently active boot CBFS, just use […]
Was about to push this change in a subsequent patch. Making the changes in this patch now.
https://review.coreboot.org/c/coreboot/+/59685/comment/b5854614_7ddf7ee5 PS13, Line 936: if (!cse_get_sub_part_fw_version(type, &source_rdev, &source_fw_ver))
Just pass the `subpart_cbfs_rw` pointer returned from the above in here, and add the appropriate off […]
Ack
https://review.coreboot.org/c/coreboot/+/59685/comment/bb80f9ad_2704d488 PS13, Line 983: region_device_sz(&source_rdev)
Here you would just use the `size` variable that got filled out above by cbfs_map().
Done
https://review.coreboot.org/c/coreboot/+/59685/comment/fd82a35d_1a54d141 PS13, Line 989: rdev_munmap(&source_rdev, subpart_cbfs_rw);
...and here you would use […]
Done
-
Krishna P Bhat D (Code Review)