November 2023 - flashrom-gerrit

[S] Change in flashrom[main]: flashchips: change print lock status funcs for Winbond chips
by Anastasia Klimchuk (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Alexander Goncharov, Edward O'Callaghan, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75877?usp=email ) Change subject: flashchips: change print lock status funcs for Winbond chips ...................................................................... Patch Set 3: Code-Review+1 (1 comment) Patchset: PS3: I think it's all good, let's also ask Nikolai. -- To view, visit https://review.coreboot.org/c/flashrom/+/75877?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I5066863b514825aee0dffe496492514ac99b6e49 Gerrit-Change-Number: 75877 Gerrit-PatchSet: 3 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 03 Nov 2023 08:17:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: spi25_statusreg: rename amic_a25l032 print to a generic name
by Anastasia Klimchuk (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Alexander Goncharov, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78874?usp=email ) Change subject: spi25_statusreg: rename amic_a25l032 print to a generic name ...................................................................... Patch Set 1: Code-Review+2 (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/78874/comment/1424623c_acc1dae6 : PS1, Line 1278: /* bit5: T/B, bit6: prot size */ Is this comment useful? if yes, you can rename the function/enum but keep the comment -- To view, visit https://review.coreboot.org/c/flashrom/+/78874?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I7169a2312698343e1065cdca91a3985e00cb3804 Gerrit-Change-Number: 78874 Gerrit-PatchSet: 1 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Fri, 03 Nov 2023 08:09:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: flashchips: Split GD25Q127C/GD25Q128C and add GD25Q128E
by Anastasia Klimchuk (Code Review) 03 Nov '23

03 Nov '23

Attention is currently required from: Hsuan-ting Chen, Nikolai Artemiev, Stefan Reinauer. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/78348?usp=email ) Change subject: flashchips: Split GD25Q127C/GD25Q128C and add GD25Q128E ...................................................................... Patch Set 3: Code-Review+1 (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/78348/comment/df20e8e9_659bc854 : PS3, Line 15: I also found datasheet for GD25Q127C https://www.gigadevice.com.cn/Public/Uploads/uploadfile/files/20220714/DS-0… could you please add to commit message? I couldn't find GD25Q128C datasheet, but if you have a link to it, would be great to add it too. Thank you! File flashchips.c: https://review.coreboot.org/c/flashrom/+/78348/comment/5fa6f57b_1fe46b10 : PS3, Line 6846: .name = "GD25Q127C/GD25Q128E", I think this one should go before previous, alphabetically. It starts with GD25Q127C and the previous one is GD25Q128C -- To view, visit https://review.coreboot.org/c/flashrom/+/78348?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I3300671b1cf74b3ea0469b9c5a833489ab4914f5 Gerrit-Change-Number: 78348 Gerrit-PatchSet: 3 Gerrit-Owner: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Fri, 03 Nov 2023 07:45:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: raiden: Support target index with generic REQ_ENABLE
by Anastasia Klimchuk (Code Review) 03 Nov '23

03 Nov '23

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/77999?usp=email ) Change subject: raiden: Support target index with generic REQ_ENABLE ...................................................................... raiden: Support target index with generic REQ_ENABLE Some devices such as the GSC knows how it is wired to AP and EC flash chips, and can be told which specific device to talk to. Other devices such as Servo Micro and HyperDebug are generic, and do not know how they are wired, the caller is responsible for first configure the appropriate MUXes or buffers, and then tell the debugger which port to use (Servo Micro has just one SPI port, HyperDebug is the first that has multiple). The Raiden protocol allows both the cases of USB devices knowing their wiring and not. If I were to declare the protocol in Rust, this is how the information of the Raiden protocol "enable request" would be encoded: ``` enum { EnableGeneric(u8), EnableAp, EnableEc, ... } ``` The first label `EnableGeneric(u8)` is to be used with HyperDebug that does not know how its ports are wired, and allow access by index. The other labels `EnableAp` and `EnableEc` are to be used with the GSC. The actual transmission of the enum above uses the bRequest and low byte of wValue of a USB control request, but that is a detail and not conceptually important. Until now, `-p raiden_debug_spi:target=AP` or `...:target=EC` could be used to make flashrom use `EnableAp` or `EnableEc`, and if neither was given, it would default to `EnableGeneric`, which now that wValue is used means `EnableGeneric(0)`. I find it rather straight-forward, that `-p raiden_debug_spi:target=1`, `...:target=2`, etc. should translate to `EnableGeneric(1)`, etc. This patchset achieves this, by adding a second 16-bit parameter value, next to request_enable. I have tested that flashrom can detect the same Winbond flash chip "W25Q128.V..M" with two different Raiden USB devices as below. TEST=flashrom -p raiden_debug_spi:serial=0701B044-91AC3132,target=AP TEST=flashrom -p raiden_debug_spi:serial=205635783236,target=1 Signed-off-by: Jes B. Klinke <jbk(a)chromium.org> Change-Id: I03bf4f3210186fb5937b42e298761907b03e08b7 Reviewed-on: https://review.coreboot.org/c/flashrom/+/77999 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M doc/classic_cli_manpage.rst M raiden_debug_spi.c M tests/raiden_debug_spi.c M tests/tests.c M tests/tests.h 5 files changed, 132 insertions(+), 19 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/doc/classic_cli_manpage.rst b/doc/classic_cli_manpage.rst index 9a455b0..5d6b7ef 100644 --- a/doc/classic_cli_manpage.rst +++ b/doc/classic_cli_manpage.rst @@ -865,11 +865,15 @@ raiden_debug_spi programmer ^^^^^^^^^^^^^^^^^^^^^^^^^^^ -The target of the SPI flashing mux must be specified with the ``target`` parameter with the:: +Some devices such as the GSC knows how it is wired to AP and EC flash chips, and can be told which specific device to talk to using the ``target`` parameter:: - flashrom -p raiden_debug_spi:target=chip + flashrom -p raiden_debug_spi:target={ap,ec} -syntax, where ``chip`` is either the ``ap`` or ``ec`` to flash, otherwise a unspecified target terminates at the end-point. +Other devices such as Servo Micro and HyperDebug are generic, and do not know how they are wired, the caller is responsible for first configure the appropriate MUXes or buffers, and then tell the debugger which port to use (Servo Micro has just one SPI port, HyperDebug is the first of this kind to have multiple):: + + flashrom -p raiden_debug_spi:target=N + +where ``N`` is an non-negative integer (default ``0``). The default is to use the first available servo. You can use the optional ``serial`` parameter to specify the servo USB device serial number to use specifically with:: diff --git a/raiden_debug_spi.c b/raiden_debug_spi.c index c5642ff..8fd5f74 100644 --- a/raiden_debug_spi.c +++ b/raiden_debug_spi.c @@ -1450,29 +1450,43 @@ return ap_request; } -static int get_target(const struct programmer_cfg *cfg) +static int decode_programmer_param(const struct programmer_cfg *cfg, uint8_t *request, + uint16_t *request_parameter) { /** * REQ_ENABLE doesn't specify a target bus, and will be rejected * by adapters that support more than one target. */ - int request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE; + uint8_t request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE; + uint16_t parameter = 0; + int ret = 0; char *target_str = extract_programmer_param_str(cfg, "target"); + printf("FISK: %s\n", target_str); + if (target_str) { - if (!strcasecmp(target_str, "ap")) + char *endptr; + int index = strtol(target_str, &endptr, 0); + if (*target_str && !*endptr && index >= 0 && index < 256) { + request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE; + parameter = index; + } else if (!strcasecmp(target_str, "ap")) request_enable = get_ap_request_type(cfg); else if (!strcasecmp(target_str, "ec")) request_enable = RAIDEN_DEBUG_SPI_REQ_ENABLE_EC; else { msg_perr("Invalid target: %s\n", target_str); - request_enable = -1; + ret = 1; } } free(target_str); - msg_pinfo("Raiden target: %d\n", request_enable); + if (ret == 0) { + msg_pinfo("Raiden target: %d,%d\n", request_enable, parameter); - return request_enable; + *request = request_enable; + *request_parameter = parameter; + } + return ret; } static void free_dev_list(struct usb_device **dev_lst) @@ -1493,10 +1507,12 @@ bool found = false; int ret; - int request_enable = get_target(cfg); - if (request_enable < 0) { + uint8_t request_enable; + uint16_t request_parameter; + ret = decode_programmer_param(cfg, &request_enable, &request_parameter); + if (ret != 0) { free(serial); - return 1; + return ret; } usb_match_init(cfg, &match); @@ -1588,7 +1604,7 @@ LIBUSB_REQUEST_TYPE_VENDOR | LIBUSB_RECIPIENT_INTERFACE, request_enable, - 0, + request_parameter, device->interface_descriptor->bInterfaceNumber, NULL, 0, diff --git a/tests/raiden_debug_spi.c b/tests/raiden_debug_spi.c index 5c79a90..4f0bdd3 100644 --- a/tests/raiden_debug_spi.c +++ b/tests/raiden_debug_spi.c @@ -104,15 +104,100 @@ .fallback_open_state = &raiden_debug_fallback_open_state, }; - /* - * 12 is the length of programmer param string for 3-digit address. - * Address can be max 3-digit because it needs to fit into uint8_t. - */ - char raiden_debug_param[12]; - snprintf(raiden_debug_param, 12, "address=%d", USB_DEVICE_ADDRESS); + char raiden_debug_param[32]; + snprintf(raiden_debug_param, sizeof(raiden_debug_param), + "address=%d", USB_DEVICE_ADDRESS); + run_basic_lifecycle(state, &raiden_debug_io, &programmer_raiden_debug_spi, raiden_debug_param); +} + +void raiden_debug_targetAP_basic_lifecycle_test_success(void **state) +{ + struct io_mock_fallback_open_state raiden_debug_fallback_open_state = { + .noc = 0, + .paths = { NULL }, + }; + const struct io_mock raiden_debug_io = { + .libusb_get_device_list = raiden_debug_libusb_get_device_list, + .libusb_free_device_list = raiden_debug_libusb_free_device_list, + .libusb_get_device_descriptor = raiden_debug_libusb_get_device_descriptor, + .libusb_get_config_descriptor = raiden_debug_libusb_get_config_descriptor, + .libusb_free_config_descriptor = raiden_debug_libusb_free_config_descriptor, + .fallback_open_state = &raiden_debug_fallback_open_state, + }; + + char raiden_debug_param[32]; + snprintf(raiden_debug_param, sizeof(raiden_debug_param), + "address=%d,target=AP", USB_DEVICE_ADDRESS); + run_basic_lifecycle(state, &raiden_debug_io, &programmer_raiden_debug_spi, raiden_debug_param); +} + +void raiden_debug_targetEC_basic_lifecycle_test_success(void **state) +{ + struct io_mock_fallback_open_state raiden_debug_fallback_open_state = { + .noc = 0, + .paths = { NULL }, + }; + const struct io_mock raiden_debug_io = { + .libusb_get_device_list = raiden_debug_libusb_get_device_list, + .libusb_free_device_list = raiden_debug_libusb_free_device_list, + .libusb_get_device_descriptor = raiden_debug_libusb_get_device_descriptor, + .libusb_get_config_descriptor = raiden_debug_libusb_get_config_descriptor, + .libusb_free_config_descriptor = raiden_debug_libusb_free_config_descriptor, + .fallback_open_state = &raiden_debug_fallback_open_state, + }; + + char raiden_debug_param[32]; + snprintf(raiden_debug_param, sizeof(raiden_debug_param), + "address=%d,target=ec", USB_DEVICE_ADDRESS); + run_basic_lifecycle(state, &raiden_debug_io, &programmer_raiden_debug_spi, raiden_debug_param); +} + +void raiden_debug_target0_basic_lifecycle_test_success(void **state) +{ + struct io_mock_fallback_open_state raiden_debug_fallback_open_state = { + .noc = 0, + .paths = { NULL }, + }; + const struct io_mock raiden_debug_io = { + .libusb_get_device_list = raiden_debug_libusb_get_device_list, + .libusb_free_device_list = raiden_debug_libusb_free_device_list, + .libusb_get_device_descriptor = raiden_debug_libusb_get_device_descriptor, + .libusb_get_config_descriptor = raiden_debug_libusb_get_config_descriptor, + .libusb_free_config_descriptor = raiden_debug_libusb_free_config_descriptor, + .fallback_open_state = &raiden_debug_fallback_open_state, + }; + + char raiden_debug_param[32]; + snprintf(raiden_debug_param, sizeof(raiden_debug_param), + "address=%d,target=0", USB_DEVICE_ADDRESS); + run_basic_lifecycle(state, &raiden_debug_io, &programmer_raiden_debug_spi, raiden_debug_param); +} + +void raiden_debug_target1_basic_lifecycle_test_success(void **state) +{ + struct io_mock_fallback_open_state raiden_debug_fallback_open_state = { + .noc = 0, + .paths = { NULL }, + }; + const struct io_mock raiden_debug_io = { + .libusb_get_device_list = raiden_debug_libusb_get_device_list, + .libusb_free_device_list = raiden_debug_libusb_free_device_list, + .libusb_get_device_descriptor = raiden_debug_libusb_get_device_descriptor, + .libusb_get_config_descriptor = raiden_debug_libusb_get_config_descriptor, + .libusb_free_config_descriptor = raiden_debug_libusb_free_config_descriptor, + .fallback_open_state = &raiden_debug_fallback_open_state, + }; + + char raiden_debug_param[32]; + snprintf(raiden_debug_param, sizeof(raiden_debug_param), + "address=%d,target=1", USB_DEVICE_ADDRESS); run_basic_lifecycle(state, &raiden_debug_io, &programmer_raiden_debug_spi, raiden_debug_param); } #else SKIP_TEST(raiden_debug_basic_lifecycle_test_success) + SKIP_TEST(raiden_debug_targetAP_basic_lifecycle_test_success) + SKIP_TEST(raiden_debug_targetEC_basic_lifecycle_test_success) + SKIP_TEST(raiden_debug_target0_basic_lifecycle_test_success) + SKIP_TEST(raiden_debug_target1_basic_lifecycle_test_success) #endif /* CONFIG_RAIDEN_DEBUG_SPI */ diff --git a/tests/tests.c b/tests/tests.c index 8b4ad03..35bd599 100644 --- a/tests/tests.c +++ b/tests/tests.c @@ -463,6 +463,10 @@ cmocka_unit_test(dummy_all_buses_test_success), cmocka_unit_test(nicrealtek_basic_lifecycle_test_success), cmocka_unit_test(raiden_debug_basic_lifecycle_test_success), + cmocka_unit_test(raiden_debug_targetAP_basic_lifecycle_test_success), + cmocka_unit_test(raiden_debug_targetEC_basic_lifecycle_test_success), + cmocka_unit_test(raiden_debug_target0_basic_lifecycle_test_success), + cmocka_unit_test(raiden_debug_target1_basic_lifecycle_test_success), cmocka_unit_test(dediprog_basic_lifecycle_test_success), cmocka_unit_test(linux_mtd_probe_lifecycle_test_success), cmocka_unit_test(linux_spi_probe_lifecycle_test_success), diff --git a/tests/tests.h b/tests/tests.h index e273e1d..3841d20 100644 --- a/tests/tests.h +++ b/tests/tests.h @@ -54,6 +54,10 @@ void dummy_all_buses_test_success(void **state); void nicrealtek_basic_lifecycle_test_success(void **state); void raiden_debug_basic_lifecycle_test_success(void **state); +void raiden_debug_targetAP_basic_lifecycle_test_success(void **state); +void raiden_debug_targetEC_basic_lifecycle_test_success(void **state); +void raiden_debug_target0_basic_lifecycle_test_success(void **state); +void raiden_debug_target1_basic_lifecycle_test_success(void **state); void dediprog_basic_lifecycle_test_success(void **state); void linux_mtd_probe_lifecycle_test_success(void **state); void linux_spi_probe_lifecycle_test_success(void **state); -- To view, visit https://review.coreboot.org/c/flashrom/+/77999?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I03bf4f3210186fb5937b42e298761907b03e08b7 Gerrit-Change-Number: 77999 Gerrit-PatchSet: 7 Gerrit-Owner: Jes Klinke <jbk(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-MessageType: merged

1 0

[S] Change in flashrom[main]: flashchips: change print lock status funcs for Winbond chips
by Alexander Goncharov (Code Review) 02 Nov '23

02 Nov '23

Attention is currently required from: Anastasia Klimchuk, Edward O'Callaghan, Nikolai Artemiev, Stefan Reinauer. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/75877?usp=email ) Change subject: flashchips: change print lock status funcs for Winbond chips ...................................................................... Patch Set 3: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/75877/comment/32920e51_48adc4df : PS2, Line 17877: SPI_PRETTYPRINT_STATUS_REGISTER_AMIC_A25L032 > I agree with renaming, these names are long already so long name is fine. […] CB:78874 is in charge of renaming this printlock -- To view, visit https://review.coreboot.org/c/flashrom/+/75877?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I5066863b514825aee0dffe496492514ac99b6e49 Gerrit-Change-Number: 75877 Gerrit-PatchSet: 3 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Comment-Date: Thu, 02 Nov 2023 08:04:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Comment-In-Reply-To: Nikolai Artemiev <nartemiev(a)google.com> Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[main]: flashchips: change print lock status funcs for Winbond chips
by Alexander Goncharov (Code Review) 02 Nov '23

02 Nov '23

Attention is currently required from: Alexander Goncharov, Edward O'Callaghan, Nikolai Artemiev. Hello Anastasia Klimchuk, Edward O'Callaghan, Nikolai Artemiev, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/75877?usp=email to look at the new patch set (#3). Change subject: flashchips: change print lock status funcs for Winbond chips ...................................................................... flashchips: change print lock status funcs for Winbond chips Decode status register bits for user friendly output. Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Change-Id: I5066863b514825aee0dffe496492514ac99b6e49 --- M flashchips.c 1 file changed, 21 insertions(+), 21 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/77/75877/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/75877?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I5066863b514825aee0dffe496492514ac99b6e49 Gerrit-Change-Number: 75877 Gerrit-PatchSet: 3 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[main]: spi25_statusreg: rename amic_a25l032 print to a generic name
by Alexander Goncharov (Code Review) 02 Nov '23

02 Nov '23

Alexander Goncharov has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/78874?usp=email ) Change subject: spi25_statusreg: rename amic_a25l032 print to a generic name ...................................................................... spi25_statusreg: rename amic_a25l032 print to a generic name Other chips (at least Winbond) will benefit from this change. Also, drop the FIXME comment, as it can be misleading. The "pretty print" functions should only display values from the Status Register, so using an inappropriate function might only confuse user. Signed-off-by: Alexander Goncharov <chat(a)joursoir.net> Change-Id: I7169a2312698343e1065cdca91a3985e00cb3804 --- M flashchips.c M include/flash.h M spi25_statusreg.c 3 files changed, 6 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/74/78874/1 diff --git a/flashchips.c b/flashchips.c index 9210de2..0096b44 100644 --- a/flashchips.c +++ b/flashchips.c @@ -745,7 +745,7 @@ .block_erase = SPI_BLOCK_ERASE_C7, } }, - .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_AMIC_A25L032, /* bit5: T/B, bit6: prot size */ + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_SRWD_SEC_TB_BP2_WELWIP, .unlock = SPI_DISABLE_BLOCKPROTECT_BP2_SRWD, /* TODO: 2nd status reg (read with 0x35) */ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, @@ -1275,7 +1275,7 @@ .block_erase = SPI_BLOCK_ERASE_C7, } }, - .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_AMIC_A25L032, /* bit5: T/B, bit6: prot size */ + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_SRWD_SEC_TB_BP2_WELWIP, .unlock = SPI_DISABLE_BLOCKPROTECT_BP2_SRWD, /* TODO: 2nd status reg (read with 0x35) */ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, @@ -1315,7 +1315,7 @@ .block_erase = SPI_BLOCK_ERASE_C7, } }, - .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_AMIC_A25L032, /* bit5: T/B, bit6: prot size */ + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_SRWD_SEC_TB_BP2_WELWIP, .unlock = SPI_DISABLE_BLOCKPROTECT_BP2_SRWD, /* TODO: 2nd status reg (read with 0x35) */ .write = SPI_CHIP_WRITE256, .read = SPI_CHIP_READ, diff --git a/include/flash.h b/include/flash.h index 4c08549..284461e 100644 --- a/include/flash.h +++ b/include/flash.h @@ -369,7 +369,6 @@ PRINTLOCK_W39V080A, PRINTLOCK_W39V080FA, PRINTLOCK_W39V080FA_DUAL, - SPI_PRETTYPRINT_STATUS_REGISTER_AMIC_A25L032, SPI_PRETTYPRINT_STATUS_REGISTER_AT25DF, SPI_PRETTYPRINT_STATUS_REGISTER_AT25DF_SEC, SPI_PRETTYPRINT_STATUS_REGISTER_AT25F, @@ -385,6 +384,7 @@ SPI_PRETTYPRINT_STATUS_REGISTER_BP2_EP_SRWD, SPI_PRETTYPRINT_STATUS_REGISTER_BP2_SRWD, SPI_PRETTYPRINT_STATUS_REGISTER_BP2_TB_BPL, + SPI_PRETTYPRINT_STATUS_REGISTER_SRWD_SEC_TB_BP2_WELWIP, SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, SPI_PRETTYPRINT_STATUS_REGISTER_BP4_SRWD, SPI_PRETTYPRINT_STATUS_REGISTER_DEFAULT_WELWIP, diff --git a/spi25_statusreg.c b/spi25_statusreg.c index bb56ee1..98988af 100644 --- a/spi25_statusreg.c +++ b/spi25_statusreg.c @@ -601,16 +601,7 @@ return 0; } -/* === Amic === - * FIXME: spi_disable_blockprotect is incorrect but works fine for chips using - * spi_prettyprint_status_register_bp1_srwd or - * spi_prettyprint_status_register_bp2_srwd. - * FIXME: spi_disable_blockprotect is incorrect and will fail for chips using - * spi_prettyprint_status_register_amic_a25l032 if those have locks controlled - * by the second status register. - */ - -static int spi_prettyprint_status_register_amic_a25l032(struct flashctx *flash) +static int spi_prettyprint_status_register_srwd_sec_tb_bp2_welwip(struct flashctx *flash) { uint8_t status; int ret = spi_read_register(flash, STATUS1, &status); @@ -1056,7 +1047,6 @@ case PRINTLOCK_W39V080A: return &printlock_w39v080a; case PRINTLOCK_W39V080FA: return &printlock_w39v080fa; case PRINTLOCK_W39V080FA_DUAL: return &printlock_w39v080fa_dual; - case SPI_PRETTYPRINT_STATUS_REGISTER_AMIC_A25L032: return &spi_prettyprint_status_register_amic_a25l032; case SPI_PRETTYPRINT_STATUS_REGISTER_AT25DF: return &spi_prettyprint_status_register_at25df; case SPI_PRETTYPRINT_STATUS_REGISTER_AT25DF_SEC: return &spi_prettyprint_status_register_at25df_sec; case SPI_PRETTYPRINT_STATUS_REGISTER_AT25F: return &spi_prettyprint_status_register_at25f; @@ -1072,6 +1062,7 @@ case SPI_PRETTYPRINT_STATUS_REGISTER_BP2_EP_SRWD: return &spi_prettyprint_status_register_bp2_ep_srwd; case SPI_PRETTYPRINT_STATUS_REGISTER_BP2_SRWD: return &spi_prettyprint_status_register_bp2_srwd; case SPI_PRETTYPRINT_STATUS_REGISTER_BP2_TB_BPL: return &spi_prettyprint_status_register_bp2_tb_bpl; + case SPI_PRETTYPRINT_STATUS_REGISTER_SRWD_SEC_TB_BP2_WELWIP: return &spi_prettyprint_status_register_srwd_sec_tb_bp2_welwip; case SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD: return &spi_prettyprint_status_register_bp3_srwd; case SPI_PRETTYPRINT_STATUS_REGISTER_BP4_SRWD: return &spi_prettyprint_status_register_bp4_srwd; case SPI_PRETTYPRINT_STATUS_REGISTER_DEFAULT_WELWIP: return &spi_prettyprint_status_register_default_welwip; -- To view, visit https://review.coreboot.org/c/flashrom/+/78874?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I7169a2312698343e1065cdca91a3985e00cb3804 Gerrit-Change-Number: 78874 Gerrit-PatchSet: 1 Gerrit-Owner: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: newchange

1 0

[XS] Change in flashrom[main]: doc: Add meson test command to TLDR for meson instructions
by Alexander Goncharov (Code Review) 02 Nov '23

02 Nov '23

Alexander Goncharov has submitted this change. ( https://review.coreboot.org/c/flashrom/+/78689?usp=email ) Change subject: doc: Add meson test command to TLDR for meson instructions ...................................................................... doc: Add meson test command to TLDR for meson instructions Running tests is one of the regular things to do in dev process, same as compile, so it should be highlighted in the TLDR section of the document. The patch adds test command to TLDR version in README file, and in dedicated meson instructions doc. Change-Id: I67d5f4decdac15e6a72f4372135dab7d44396594 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/78689 Reviewed-by: Jes Klinke <jbk(a)chromium.org> Reviewed-by: Alexander Goncharov <chat(a)joursoir.net> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M README.rst M doc/dev_guide/building_from_source.rst 2 files changed, 2 insertions(+), 0 deletions(-) Approvals: Peter Marheine: Looks good to me, approved build bot (Jenkins): Verified Alexander Goncharov: Looks good to me, approved Jes Klinke: Looks good to me, but someone else must approve diff --git a/README.rst b/README.rst index 0a0ea08..371fd61 100644 --- a/README.rst +++ b/README.rst @@ -32,6 +32,7 @@ meson setup builddir meson compile -C builddir + meson test -C builddir meson install -C builddir For full detailed instructions, follow the information in diff --git a/doc/dev_guide/building_from_source.rst b/doc/dev_guide/building_from_source.rst index 64250dd..4e50753 100644 --- a/doc/dev_guide/building_from_source.rst +++ b/doc/dev_guide/building_from_source.rst @@ -33,6 +33,7 @@ meson setup builddir meson compile -C builddir + meson test -C builddir meson install -C builddir -- To view, visit https://review.coreboot.org/c/flashrom/+/78689?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: I67d5f4decdac15e6a72f4372135dab7d44396594 Gerrit-Change-Number: 78689 Gerrit-PatchSet: 3 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Jes Klinke <jbk(a)chromium.org> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[main]: flashchips: Add Puya P25Q21H/11H/06H
by Alexander Goncharov (Code Review) 02 Nov '23

02 Nov '23

Attention is currently required from: Anastasia Klimchuk, Angel Pons, Nikolai Artemiev, WereCatf. Alexander Goncharov has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58134?usp=email ) Change subject: flashchips: Add Puya P25Q21H/11H/06H ...................................................................... Patch Set 7: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/58134?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Idd43145c72607837cb7afa1b007e68eb8e63ebd9 Gerrit-Change-Number: 58134 Gerrit-PatchSet: 7 Gerrit-Owner: WereCatf <werecatf(a)outlook.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: WereCatf <werecatf(a)outlook.com> Gerrit-Comment-Date: Thu, 02 Nov 2023 06:51:10 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[main]: flashchips: Add Puya P25Q21H/11H/06H
by Anastasia Klimchuk (Code Review) 01 Nov '23

01 Nov '23

Attention is currently required from: Alexander Goncharov, Angel Pons, Nikolai Artemiev, WereCatf. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58134?usp=email ) Change subject: flashchips: Add Puya P25Q21H/11H/06H ...................................................................... Patch Set 7: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/58134/comment/68eda0f4_a06e6625 : PS6, Line 10: https://datasheet.lcsc.com/szlcsc/1908281813_PUYA-P25Q21H-SSH-IT_C414061.pdf > I cannot access this _part_ of the site from Switzerland / Russia. […] I can't access that old link anymore! So I think they just removed the file, country doesn't matter. It's funny because I could open it few weeks ago. I replaced the link in commit message with your new link, thank you so much! File flashchips.c: https://review.coreboot.org/c/flashrom/+/58134/comment/795da11a_fe7b70a9 : PS6, Line 14141: 256 > The document that I used says: "3*512-Byte Security Registers With OTP Lock". […] I can't check since old doc is not accessible anymore, but the datasheet looks like exactly the same. So I think this was a mistake and I corrected to 3 x 512 for all three chips. -- To view, visit https://review.coreboot.org/c/flashrom/+/58134?usp=email To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: main Gerrit-Change-Id: Idd43145c72607837cb7afa1b007e68eb8e63ebd9 Gerrit-Change-Number: 58134 Gerrit-PatchSet: 7 Gerrit-Owner: WereCatf <werecatf(a)outlook.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-CC: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-CC: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: WereCatf <werecatf(a)outlook.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Wed, 01 Nov 2023 01:19:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Alexander Goncharov <chat(a)joursoir.net> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2023