Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/46110 )
Change subject: mrc_cache: Add data_size parameter to mrc_cache_load_current ......................................................................
Patch Set 1:
(2 comments)
https://review.coreboot.org/c/coreboot/+/46110/1//COMMIT_MSG Commit Message:
https://review.coreboot.org/c/coreboot/+/46110/1//COMMIT_MSG@10 PS1, Line 10: This is needed for ARM devices like trogdor, which : need to know the size of the training data when populating the QcLib : interface table. You know, thinking about this some more, I think it is more appropriate to specify the full size of the available memory buffer after all. That's because it is an in/out buffer and I guess it's theoretically possible that the new training data it wants to write is larger than the previous one. QcLib uses the size value to determine how much it is allowed to write back, and that's probably the more important point. For the passed-in buffer the size is probably encoded somewhere inside the data anyway.
So, I don't think it's wrong to expand the API to allow returning this info in general, but we probably don't need it for Trogdor after all.
https://review.coreboot.org/c/coreboot/+/46110/1/src/drivers/mrc_cache/mrc_c... File src/drivers/mrc_cache/mrc_cache.c:
https://review.coreboot.org/c/coreboot/+/46110/1/src/drivers/mrc_cache/mrc_c... PS1, Line 273: *data_size = Let's make this optional, e.g.
if (data_size) *data_size = dsize;
so callers that don't care can just pass NULL. (Or go with Furquan's suggestion which is fine too.)