Attention is currently required from: Hsuan-ting Chen.
Yu-Ping Wu has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/75330?usp=email )
Change subject: lib: Support localized text of memory_training_desc in ux_locales.c ......................................................................
Patch Set 9:
(6 comments)
Commit Message:
https://review.coreboot.org/c/coreboot/+/75330/comment/7b22c9ce_0bbcf950 : PS9, Line 13: The preram_locales file follows the format: I have a few questions about the format.
1. From the parsing algorithm, there will be a problem if (a) `locale_id_1 == string_name_2`: Okay, this is prevented by an extra check in the bmpblk/build.py script (b) `localized_string_1 == string_name_2`: Do we want to prevent that? How?
To make bmpblk/build.py simpler and the parsing more robust, I wonder if there can be a different delimiter between strings:
``` string_name_1 \x00 locale_id_1 \x00 localized_string_1 \x00 ... \x01 string_name_2 \x00 locale_id_2 \x00 localized_string_2 \x00 ... \x01 string_name_3 \x00 locale_id_3 \x00 localized_string_3 \x00 ... ```
2. Since the file format is designed specifically for this use case, I wonder if we should add a version byte (or several bytes, or even a small header struct) in case there's a breaking change in the format in the future?
File src/lib/ux_locales.c:
https://review.coreboot.org/c/coreboot/+/75330/comment/24a16741_f7664c36 : PS9, Line 109: BIOS_INFO Shouldn't this be an error?
https://review.coreboot.org/c/coreboot/+/75330/comment/bd8f044c_ee7ac1e1 : PS9, Line 118: BIOS_INFO BIOS_WARNING
https://review.coreboot.org/c/coreboot/+/75330/comment/efd63c42_1ec3c073 : PS9, Line 128: offset Just pass 0 here (and remove the previous line)?
https://review.coreboot.org/c/coreboot/+/75330/comment/7d5b64d8_c578480b : PS9, Line 130: BIOS_INFO BIOS_ERR. Same for 2 instances below.
https://review.coreboot.org/c/coreboot/+/75330/comment/a783eb03_56359409 : PS9, Line 154: Extra space.