March 2025 - flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[main]: libflashrom: Update the API for Logger Callback
by Dmitry Zhadinets (Code Review) 30 Mar '25

30 Mar '25

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Hello Anastasia Klimchuk, Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/86875?usp=email to look at the new patch set (#10). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: libflashrom: Update the API for Logger Callback ...................................................................... libflashrom: Update the API for Logger Callback The initial implementation does not account for user_data, requiring the calling application to use a global scope. This may lead to issues related to object lifecycle management and other architectural concerns. This patch adds user_data to the user’s log callback. Moreover, it performs message formatting, so the application only needs to pass the formatted string to the selected output. The change does not break the existing logging API but extends it. A new API version is introduced with the v2 suffix. Testing: Both unit tests and CLI tools serve as libflashrom clients. All unit tests run successfully. Change-Id: Iea738bd371fa3d69b9cf222c89ee67490d30af39 Signed-off-by: Dmitry Zhadinets <dzhadinets(a)gmail.com> --- M include/flash.h M include/libflashrom.h M libflashrom.c M libflashrom.map M meson.build M meson_options.txt A tests/libflashrom.c M tests/meson.build M tests/tests.c M tests/tests.h 10 files changed, 157 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/86875/10 -- To view, visit https://review.coreboot.org/c/flashrom/+/86875?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Iea738bd371fa3d69b9cf222c89ee67490d30af39 Gerrit-Change-Number: 86875 Gerrit-PatchSet: 10 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>

1 0

[M] Change in flashrom[main]: libflashrom: Update the API for Logger Callback
by Dmitry Zhadinets (Code Review) 30 Mar '25

30 Mar '25

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Dmitry Zhadinets has posted comments on this change by Dmitry Zhadinets. ( https://review.coreboot.org/c/flashrom/+/86875?usp=email ) Change subject: libflashrom: Update the API for Logger Callback ...................................................................... Patch Set 9: (1 comment) File libflashrom.c: https://review.coreboot.org/c/flashrom/+/86875/comment/cd1fad7d_64ad61c1?us… : PS4, Line 59: /* Dynamic messages and double formatting may not be ideal, : as they are slow. It would be better to limit the message : size to a fixed value (e.g., char message[1024] = {0};). */ > Returning an error seems like an okay start, but in practical terms that's nearly the same as silent […] I did. I agree that truncation is better then nothing but .... If the user will really want to handle long messages he will not be ably to do it. Only recompilation or using the old approach. because the user does not control it. -- To view, visit https://review.coreboot.org/c/flashrom/+/86875?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Iea738bd371fa3d69b9cf222c89ee67490d30af39 Gerrit-Change-Number: 86875 Gerrit-PatchSet: 9 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Sun, 30 Mar 2025 23:43:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Dmitry Zhadinets <dzhadinets(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0

[M] Change in flashrom[main]: libflashrom: Update the API for Logger Callback
by Dmitry Zhadinets (Code Review) 30 Mar '25

30 Mar '25

Attention is currently required from: Anastasia Klimchuk, Dmitry Zhadinets. Hello Anastasia Klimchuk, Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/86875?usp=email to look at the new patch set (#9). The following approvals got outdated and were removed: Code-Review+2 by Anastasia Klimchuk, Verified+1 by build bot (Jenkins) Change subject: libflashrom: Update the API for Logger Callback ...................................................................... libflashrom: Update the API for Logger Callback The initial implementation does not account for user_data, requiring the calling application to use a global scope. This may lead to issues related to object lifecycle management and other architectural concerns. This patch adds user_data to the user’s log callback. Moreover, it performs message formatting, so the application only needs to pass the formatted string to the selected output. The change does not break the existing logging API but extends it. A new API version is introduced with the v2 suffix. Testing: Both unit tests and CLI tools serve as libflashrom clients. All unit tests run successfully. Change-Id: Iea738bd371fa3d69b9cf222c89ee67490d30af39 Signed-off-by: Dmitry Zhadinets <dzhadinets(a)gmail.com> --- M include/flash.h M include/libflashrom.h M libflashrom.c M libflashrom.map M meson.build M meson_options.txt A tests/libflashrom.c M tests/meson.build M tests/tests.c M tests/tests.h 10 files changed, 154 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/86875/9 -- To view, visit https://review.coreboot.org/c/flashrom/+/86875?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Iea738bd371fa3d69b9cf222c89ee67490d30af39 Gerrit-Change-Number: 86875 Gerrit-PatchSet: 9 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: libflashrom: Update the API for Logger Callback
by Peter Marheine (Code Review) 30 Mar '25

30 Mar '25

Attention is currently required from: Dmitry Zhadinets. Peter Marheine has posted comments on this change by Dmitry Zhadinets. ( https://review.coreboot.org/c/flashrom/+/86875?usp=email ) Change subject: libflashrom: Update the API for Logger Callback ...................................................................... Patch Set 8: (1 comment) File libflashrom.c: https://review.coreboot.org/c/flashrom/+/86875/comment/c9766ae2_273eecd4?us… : PS4, Line 59: /* Dynamic messages and double formatting may not be ideal, : as they are slow. It would be better to limit the message : size to a fixed value (e.g., char message[1024] = {0};). */ > Peter, I wanted to check, do you agree with how this ended up looking like? No dynamic allocation, h […] Returning an error seems like an okay start, but in practical terms that's nearly the same as silently dropping the message because most uses of logging don't bother to check the return value (and I think that's reasonable because checking it would lead to much messier code). As a compromise, I'd suggest truncating the message and if it does get truncated then also log a note that the previous message was truncated. To give users a knob to twiddle in case they really want to be able to handle long messages, I would also make the buffer size a compile-time option. ``` char message[LOG_MESSAGE_LENGTH_LIMIT]; int actual_len = vsnprintf(message, sizeof(message), format, args); global_log_callback_v2(level, message, global_log_user_data); if (actual_len >= sizeof(message)) { snprintf(message, sizeof(message), "%d characters were truncated from the previous log message", actual_len - sizeof(message) + 1); global_log_callback_v2(FLASHROM_MSG_WARN, message, global_log_user_data); } ``` -- To view, visit https://review.coreboot.org/c/flashrom/+/86875?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Iea738bd371fa3d69b9cf222c89ee67490d30af39 Gerrit-Change-Number: 86875 Gerrit-PatchSet: 8 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Comment-Date: Sun, 30 Mar 2025 22:49:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Dmitry Zhadinets <dzhadinets(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0

[M] Change in flashrom[main]: libflashrom: Update the API for Logger Callback
by Anastasia Klimchuk (Code Review) 30 Mar '25

30 Mar '25

Attention is currently required from: Dmitry Zhadinets, Peter Marheine. Anastasia Klimchuk has posted comments on this change by Dmitry Zhadinets. ( https://review.coreboot.org/c/flashrom/+/86875?usp=email ) Change subject: libflashrom: Update the API for Logger Callback ...................................................................... Patch Set 8: Code-Review+2 (1 comment) File libflashrom.c: https://review.coreboot.org/c/flashrom/+/86875/comment/3668c62c_79a7c0a7?us… : PS4, Line 59: /* Dynamic messages and double formatting may not be ideal, : as they are slow. It would be better to limit the message : size to a fixed value (e.g., char message[1024] = {0};). */ > Done Peter, I wanted to check, do you agree with how this ended up looking like? No dynamic allocation, however no silent dropping either (an error is returned) -- To view, visit https://review.coreboot.org/c/flashrom/+/86875?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Iea738bd371fa3d69b9cf222c89ee67490d30af39 Gerrit-Change-Number: 86875 Gerrit-PatchSet: 8 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Sun, 30 Mar 2025 12:33:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Dmitry Zhadinets <dzhadinets(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0

[M] Change in flashrom[main]: libflashrom: Add set log level functionality
by NyeonWoo Kim (Code Review) 30 Mar '25

30 Mar '25

Attention is currently required from: Anastasia Klimchuk, Dmitry Zhadinets. NyeonWoo Kim has posted comments on this change by Dmitry Zhadinets. ( https://review.coreboot.org/c/flashrom/+/87047?usp=email ) Change subject: libflashrom: Add set log level functionality ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/87047?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I095d48b8feb5fbc950a36eb17bed0d7cb8d9df64 Gerrit-Change-Number: 87047 Gerrit-PatchSet: 1 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: NyeonWoo Kim Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sun, 30 Mar 2025 06:14:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in flashrom[main]: libflashrom: Add set log level functionality
by Dmitry Zhadinets (Code Review) 29 Mar '25

29 Mar '25

Dmitry Zhadinets has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/87047?usp=email ) Change subject: libflashrom: Add set log level functionality ...................................................................... libflashrom: Add set log level functionality Before this commit, any message from Flashrom would trigger the user's callback. This could lead to additional delays and slow down overall Flashrom performance. This patch adds the ability to configure the log level for messages from Flashrom. It sets the default log level to INFO Testing: Both unit tests and CLI tools serve as libflashrom clients. All unit tests run successfully. Change-Id: I095d48b8feb5fbc950a36eb17bed0d7cb8d9df64 Signed-off-by: Dmitry Zhadinets <dzhadinets(a)gmail.com> --- M include/libflashrom.h M libflashrom.c M tests/libflashrom.c M tests/tests.c M tests/tests.h 5 files changed, 59 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/87047/1 diff --git a/include/libflashrom.h b/include/libflashrom.h index 96479ed..b9b0d17 100644 --- a/include/libflashrom.h +++ b/include/libflashrom.h @@ -57,6 +57,20 @@ FLASHROM_MSG_SPEW = 5, }; typedef int(flashrom_log_callback)(enum flashrom_log_level, const char *format, va_list); + +/** + * @brief Set the log level. + * + * Set a log level for messages from libflashrom. + * The print callback will be invoked only for messages + * with a log level less than or equal to the configured value. + * The default log level is FLASHROM_MSG_INFO. + * + * @param level The log level to be set. + */ + + void flashrom_set_log_level(enum flashrom_log_level level); + /** * @brief Set the log callback function. * diff --git a/libflashrom.c b/libflashrom.c index 506aeb3..c059ba2 100644 --- a/libflashrom.c +++ b/libflashrom.c @@ -33,6 +33,7 @@ static flashrom_log_callback *global_log_callback = NULL; static flashrom_log_callback_v2 *global_log_callback_v2 = NULL; static void *global_log_user_data = NULL; +enum flashrom_log_level global_log_level = FLASHROM_MSG_INFO; int flashrom_init(const int perform_selfcheck) { @@ -46,8 +47,11 @@ return 0; /* TODO: nothing to do? */ } -/* TODO: flashrom_set_loglevel()? do we need it? - For now, let the user decide in their callback. */ +void flashrom_set_log_level(enum flashrom_log_level level) +{ + global_log_level = level; +} + void flashrom_set_log_callback(flashrom_log_callback *const log_callback) { global_log_callback = log_callback; @@ -98,7 +102,7 @@ /** @private */ int print(const enum flashrom_log_level level, const char *const fmt, ...) { - if (global_log_callback) { + if (global_log_callback && level <= global_log_level) { int ret; va_list args; va_start(args, fmt); diff --git a/tests/libflashrom.c b/tests/libflashrom.c index 42a2da0..562f2f3 100644 --- a/tests/libflashrom.c +++ b/tests/libflashrom.c @@ -23,26 +23,22 @@ static int test_log_callback(enum flashrom_log_level level, const char *format, va_list vargs) { - /* check that loglevel has passed corectly */ - assert_int_equal(level, FLASHROM_MSG_INFO); char message[3] = {0}; vsnprintf(message, 3, format, vargs); assert_string_equal(message, "1\n"); - return 0x666 + 1; + return 0x666 + (int)level; } static void test_log_callback_v2(enum flashrom_log_level level, const char *message, void *user_data) { - /* check that loglevel has passed corectly */ - assert_int_equal(level, FLASHROM_MSG_ERROR); /* check that user dta has passed */ assert_ptr_not_equal(user_data, 0); /* check that user_data is correct */ assert_int_equal(*(int *)(user_data), 100500); /* check that format is working correctly */ assert_string_equal(message, "2\n"); - *(int*)user_data = 0x666 + 2; + *(int*)user_data = 0x666 + (int)level; } void flashrom_set_log_callback_test_success(void **state) @@ -50,7 +46,7 @@ (void)state; /* unused */ flashrom_set_log_callback(test_log_callback); /* check that callback is called */ - assert_int_equal(print(FLASHROM_MSG_INFO, "1%s", "\n"), 0x666 + 1); + assert_int_equal(print(FLASHROM_MSG_INFO, "1%s", "\n"), 0x666 + (int)FLASHROM_MSG_INFO); flashrom_set_log_callback(NULL); } @@ -61,6 +57,38 @@ flashrom_set_log_callback_v2(test_log_callback_v2, &user_data); print(FLASHROM_MSG_ERROR, "2%s", "\n"); /* check that callback is called */ - assert_int_equal(user_data, 0x666 + 2); + assert_int_equal(user_data, 0x666 + (int)FLASHROM_MSG_ERROR); flashrom_set_log_callback_v2(NULL, NULL); } + +void flashrom_set_log_level_test_success(void **state) +{ + (void)state; /* unused */ + int user_data = 100500; + flashrom_set_log_callback_v2(test_log_callback_v2, &user_data); + flashrom_set_log_level(FLASHROM_MSG_WARN); + print(FLASHROM_MSG_DEBUG, "2%s", "\n"); + /* check that callback is not called */ + assert_int_equal(user_data, 100500); + print(FLASHROM_MSG_ERROR, "2%s", "\n"); + /* check that callback is called for less */ + assert_int_equal(user_data, 0x666 + (int)FLASHROM_MSG_ERROR); + user_data = 100500; + print(FLASHROM_MSG_WARN, "2%s", "\n"); + /* check that callback is called for equal */ + assert_int_equal(user_data, 0x666 + (int)FLASHROM_MSG_WARN); + + /* v1 API check */ + flashrom_set_log_callback(test_log_callback); + /* check that callback is not called */ + assert_int_equal(print(FLASHROM_MSG_INFO, "1%s", "\n"), 0); + /* check that callback is called for equal */ + assert_int_equal(print(FLASHROM_MSG_WARN, "1%s", "\n"), 0x666 + (int)FLASHROM_MSG_WARN); + /* check that callback is called for less*/ + assert_int_equal(print(FLASHROM_MSG_ERROR, "1%s", "\n"), 0x666 + (int)FLASHROM_MSG_ERROR); + + flashrom_set_log_level(FLASHROM_MSG_INFO); + /* check that callback is called after the change*/ + assert_int_equal(print(FLASHROM_MSG_INFO, "1%s", "\n"), 0x666 + (int)FLASHROM_MSG_INFO); + flashrom_set_log_callback(NULL); +} \ No newline at end of file diff --git a/tests/tests.c b/tests/tests.c index 4a48068..7f6efe6 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -441,6 +441,7 @@ const struct CMUnitTest libflashrom_tests[] = { cmocka_unit_test(flashrom_set_log_callback_test_success), cmocka_unit_test(flashrom_set_log_callback_v2_test_success), + cmocka_unit_test(flashrom_set_log_level_test_success), }; ret |= cmocka_run_group_tests_name("libflashrom.c tests", libflashrom_tests, NULL, NULL); diff --git a/tests/tests.h b/tests/tests.h index 6b34805..49d08c3 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -34,6 +34,7 @@ /* libflashrom.c */ void flashrom_set_log_callback_test_success(void **state); void flashrom_set_log_callback_v2_test_success(void **state); +void flashrom_set_log_level_test_success(void **state); /* spi25.c */ void spi_write_enable_test_success(void **state); -- To view, visit https://review.coreboot.org/c/flashrom/+/87047?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I095d48b8feb5fbc950a36eb17bed0d7cb8d9df64 Gerrit-Change-Number: 87047 Gerrit-PatchSet: 1 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(a)gmail.com>

1 0

[M] Change in flashrom[main]: libflashrom: Update the API for Logger Callback
by Dmitry Zhadinets (Code Review) 29 Mar '25

29 Mar '25

Attention is currently required from: Anastasia Klimchuk, Peter Marheine. Dmitry Zhadinets has posted comments on this change by Dmitry Zhadinets. ( https://review.coreboot.org/c/flashrom/+/86875?usp=email ) Change subject: libflashrom: Update the API for Logger Callback ...................................................................... Patch Set 7: (2 comments) File libflashrom.c: https://review.coreboot.org/c/flashrom/+/86875/comment/93e4c07f_b8ed4089?us… : PS7, Line 75: (unsigned)size+1 > in here, you need spaces between +, […] Done https://review.coreboot.org/c/flashrom/+/86875/comment/c4b7e52a_8e48b5cc?us… : PS7, Line 75: if (((unsigned)size+1) > sizeof(message)) : return -ERANGE; > Thank you so much for writing in that much details, this is helpful! I read everything and did lots […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/86875?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Iea738bd371fa3d69b9cf222c89ee67490d30af39 Gerrit-Change-Number: 86875 Gerrit-PatchSet: 7 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Sat, 29 Mar 2025 22:21:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Dmitry Zhadinets <dzhadinets(a)gmail.com> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: libflashrom: Update the API for Logger Callback
by Dmitry Zhadinets (Code Review) 29 Mar '25

29 Mar '25

Attention is currently required from: Dmitry Zhadinets, Peter Marheine. Hello Anastasia Klimchuk, Peter Marheine, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/86875?usp=email to look at the new patch set (#8). The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: libflashrom: Update the API for Logger Callback ...................................................................... libflashrom: Update the API for Logger Callback The initial implementation does not account for user_data, requiring the calling application to use a global scope. This may lead to issues related to object lifecycle management and other architectural concerns. This patch adds user_data to the user’s log callback. Moreover, it performs message formatting, so the application only needs to pass the formatted string to the selected output. The change does not break the existing logging API but extends it. A new API version is introduced with the v2 suffix. Testing: Both unit tests and CLI tools serve as libflashrom clients. All unit tests run successfully. Change-Id: Iea738bd371fa3d69b9cf222c89ee67490d30af39 Signed-off-by: Dmitry Zhadinets <dzhadinets(a)gmail.com> --- M include/flash.h M include/libflashrom.h M libflashrom.c M libflashrom.map A tests/libflashrom.c M tests/meson.build M tests/tests.c M tests/tests.h 8 files changed, 149 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/75/86875/8 -- To view, visit https://review.coreboot.org/c/flashrom/+/86875?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Iea738bd371fa3d69b9cf222c89ee67490d30af39 Gerrit-Change-Number: 86875 Gerrit-PatchSet: 8 Gerrit-Owner: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Dmitry Zhadinets <dzhadinets(a)gmail.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>

1 0

[S] Change in flashrom[main]: flashchips: Add XMC XM25QH64A
by Anastasia Klimchuk (Code Review) 29 Mar '25

29 Mar '25

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/86990?usp=email ) Change subject: flashchips: Add XMC XM25QH64A ...................................................................... flashchips: Add XMC XM25QH64A Based off of the now-abandoned GitHub pull request here: https://github.com/flashrom/flashrom/pull/239 Datasheet: https://www.micros.com.pl/mediaserver/PFXM25QH64AHIG_0001.pdf This commit applies the changes on top of the refactor where the flash chip declarations were separated by vendor. Change-Id: I5b11e30f0a5357a6cbb32ddb93f450de5364c60b Co-authored-by: Ayushman Dutta <ayushman999(a)gmail.com> Co-authored-by: "aiyion.prime" <git(a)aiyionpri.me> Co-authored-by: Eric Park <me(a)ericswpark.com> Signed-off-by: Eric Park <me(a)ericswpark.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/86990 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M flashchips/xmc.c M include/flashchips.h 2 files changed, 42 insertions(+), 0 deletions(-) Approvals: Anastasia Klimchuk: Looks good to me, approved build bot (Jenkins): Verified Eric Park: Looks good to me, but someone else must approve diff --git a/flashchips/xmc.c b/flashchips/xmc.c index 4ed0225..a4e730e 100644 --- a/flashchips/xmc.c +++ b/flashchips/xmc.c @@ -324,6 +324,47 @@ { .vendor = "XMC", + .name = "XM25QH64A", + .bustype = BUS_SPI, + .manufacture_id = ST_ID, + .model_id = XMC_XM25QH64A, + .total_size = 8192, + .page_size = 256, + /* supports SFDP */ + /* OTP: 512B total; enter 0x3A */ + /* QPI enable 0x38, disable 0xFF */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI, + .tested = TEST_OK_PREW, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 2048} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 256} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 128} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { { 8192 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { { 8192 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_C7, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, /* bit6 is quad enable */ + .unlock = SPI_DISABLE_BLOCKPROTECT_BP3_SRWD, + .write = SPI_CHIP_WRITE256, + .read = SPI_CHIP_READ, + .voltage = {2700, 3600}, + }, + + { + .vendor = "XMC", .name = "XM25QH128A", .bustype = BUS_SPI, .manufacture_id = ST_ID, diff --git a/include/flashchips.h b/include/flashchips.h index 556729c..a1a471c 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -864,6 +864,7 @@ #define XMC_XM25QU16C 0x5015 #define XMC_XM25QH32C 0x4016 /* Same as XM25QH32D */ #define XMC_XM25QU32C 0x5016 +#define XMC_XM25QH64A 0x7017 #define XMC_XM25QH128A 0x7018 #define XMC_XM25QH128C 0x4018 /* Same as XM25QH128D */ #define XMC_XM25QU128C 0x4118 /* Same as XM25QU128D */ -- To view, visit https://review.coreboot.org/c/flashrom/+/86990?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I5b11e30f0a5357a6cbb32ddb93f450de5364c60b Gerrit-Change-Number: 86990 Gerrit-PatchSet: 8 Gerrit-Owner: Eric Park <me(a)ericswpark.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Eric Park <me(a)ericswpark.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit March 2025