- flashrom-gerrit - mail.coreboot.org

[L] Change in flashrom[main]: erasure_layout: Fix get_flash_region bug
by Anastasia Klimchuk (Code Review) 03 Jun '24

03 Jun '24

Attention is currently required from: Aarya, Alexander Goncharov, Nikolai Artemiev, Peter Marheine. Anastasia Klimchuk has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82393?usp=email ) Change subject: erasure_layout: Fix get_flash_region bug ...................................................................... Patch Set 16: (1 comment) Patchset: PS16: I am also wondering, whether maybe to post to mailing list in case someone interested to help testing, or even look at the patch -- To view, visit https://review.coreboot.org/c/flashrom/+/82393?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: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Gerrit-Change-Number: 82393 Gerrit-PatchSet: 16 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Mon, 03 Jun 2024 03:35:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in flashrom[main]: erasure_layout: Fix get_flash_region bug
by Anastasia Klimchuk (Code Review) 03 Jun '24

03 Jun '24

Attention is currently required from: Aarya, Alexander Goncharov, Nikolai Artemiev, Peter Marheine. Anastasia Klimchuk has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82393?usp=email ) The change is no longer submittable: All-Comments-Resolved is unsatisfied now. Change subject: erasure_layout: Fix get_flash_region bug ...................................................................... Patch Set 16: Code-Review+2 (2 comments) Patchset: PS16: This is going great (one tiny comment that I found). And, we have one more Co-developer! Nikolai, it would be great if you can review the end result of what we have done :) Thank you so much! File tests/erase_func_algo.c: https://review.coreboot.org/c/flashrom/+/82393/comment/d8bb0b26_fa98d91d?us… : PS16, Line 89: printf("Eraser called with blockaddr=0x%x, blocklen=0x%x\n", blockaddr, blocklen); I only now noticed, perhaps we can log erase_func here too (it is logged in `block_erase_chip_with_protected_region`) -- To view, visit https://review.coreboot.org/c/flashrom/+/82393?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: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Gerrit-Change-Number: 82393 Gerrit-PatchSet: 16 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Mon, 03 Jun 2024 03:28:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[L] Change in flashrom[main]: erasure_layout: Fix get_flash_region bug
by Peter Marheine (Code Review) 03 Jun '24

03 Jun '24

Attention is currently required from: Aarya, Alexander Goncharov, Anastasia Klimchuk, Nikolai Artemiev. Peter Marheine has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82393?usp=email ) Change subject: erasure_layout: Fix get_flash_region bug ...................................................................... Patch Set 16: Code-Review+2 (2 comments) Patchset: PS15: > So given that I added a piece of code, maybe you can review it :) That looks fine to me! I also verified that the tests still fail if the code fix is reverted, which they do. File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/82393/comment/50d2e09e_99aa2048?us… : PS13, Line 368: for (unsigned int addr = region_start; addr <= region_end; addr += len) { > I think this specific issue is outside of scope of the patch: mainly because it is not relevant to p […] Yeah, that can be addressed separately. This change fixes an actual bug where we destroy data we don't intend to, whereas the other issue is just that some operations will take longer than they need to. -- To view, visit https://review.coreboot.org/c/flashrom/+/82393?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: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Gerrit-Change-Number: 82393 Gerrit-PatchSet: 16 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sun, 02 Jun 2024 23:38:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[M] Change in flashrom[main]: ch347_spi: Add driver support for CH347F packaging
by Anastasia Klimchuk (Code Review) 02 Jun '24

02 Jun '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82193?usp=email ) Change subject: ch347_spi: Add driver support for CH347F packaging ...................................................................... ch347_spi: Add driver support for CH347F packaging CH347F can work simultaneously with SPI, I2C and other signals. CH347 introduce is available at the following URL: https://www.wch-ic.com/products/CH347.html Change-Id: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd Signed-off-by: ZhiYuanNJ Liu <871238103(a)qq.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/82193 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Nicholas Chin <nic.c3.14(a)gmail.com> --- M ch347_spi.c 1 file changed, 29 insertions(+), 21 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved Nicholas Chin: Looks good to me, approved diff --git a/ch347_spi.c b/ch347_spi.c index 570e25b..21dcabb 100644 --- a/ch347_spi.c +++ b/ch347_spi.c @@ -36,8 +36,8 @@ #define WRITE_EP 0x06 #define READ_EP 0x86 -#define MODE_1_IFACE 2 -#define MODE_2_IFACE 1 +#define CH347T_IFACE 2 +#define CH347F_IFACE 4 /* The USB descriptor says the max transfer size is 512 bytes, but the * vendor driver only seems to transfer a maximum of 510 bytes at once, @@ -48,25 +48,29 @@ struct ch347_spi_data { struct libusb_device_handle *handle; + int interface; }; /* TODO: Add support for HID mode */ static const struct dev_entry devs_ch347_spi[] = { - {0x1A86, 0x55DB, OK, "QinHeng Electronics", "USB To UART+SPI+I2C"}, + {0x1A86, 0x55DB, OK, "QinHeng Electronics", "USB To UART+SPI+I2C"}, /* CH347T */ + {0x1A86, 0x55DE, OK, "QinHeng Electronics", "USB To UART+SPI+I2C"}, /* CH347F */ {0} }; +static int ch347_interface[] = { + CH347T_IFACE, + CH347F_IFACE, +}; + static int ch347_spi_shutdown(void *data) { struct ch347_spi_data *ch347_data = data; - - /* TODO: Set this depending on the mode */ - int spi_interface = MODE_1_IFACE; + int spi_interface = ch347_data->interface; libusb_release_interface(ch347_data->handle, spi_interface); libusb_attach_kernel_driver(ch347_data->handle, spi_interface); libusb_close(ch347_data->handle); libusb_exit(NULL); - free(data); return 0; } @@ -262,6 +266,9 @@ /* Largely copied from ch341a_spi.c */ static int ch347_spi_init(const struct programmer_cfg *cfg) { + uint16_t vid = devs_ch347_spi[0].vendor_id; + uint16_t pid = 0; + int index = 0; struct ch347_spi_data *ch347_data = calloc(1, sizeof(*ch347_data)); if (!ch347_data) { msg_perr("Could not allocate space for SPI data\n"); @@ -274,35 +281,36 @@ free(ch347_data); return 1; } - /* Enable information, warning, and error messages (only). */ #if LIBUSB_API_VERSION < 0x01000106 libusb_set_debug(NULL, 3); #else libusb_set_option(NULL, LIBUSB_OPTION_LOG_LEVEL, LIBUSB_LOG_LEVEL_INFO); #endif - - uint16_t vid = devs_ch347_spi[0].vendor_id; - uint16_t pid = devs_ch347_spi[0].device_id; - ch347_data->handle = libusb_open_device_with_vid_pid(NULL, vid, pid); - if (ch347_data->handle == NULL) { - msg_perr("Couldn't open device %04x:%04x.\n", vid, pid); + while (devs_ch347_spi[index].vendor_id != 0) { + vid = devs_ch347_spi[index].vendor_id; + pid = devs_ch347_spi[index].device_id; + ch347_data->handle = libusb_open_device_with_vid_pid(NULL, vid, pid); + if (ch347_data->handle) { + ch347_data->interface = ch347_interface[index]; + break; + } + index++; + } + if (!ch347_data->handle) { + msg_perr("Couldn't find CH347.\n"); free(ch347_data); return 1; } - /* TODO: set based on mode */ - /* Mode 1 uses interface 2 for the SPI interface */ - int spi_interface = MODE_1_IFACE; - - ret = libusb_detach_kernel_driver(ch347_data->handle, spi_interface); + ret = libusb_detach_kernel_driver(ch347_data->handle, ch347_data->interface); if (ret != 0 && ret != LIBUSB_ERROR_NOT_FOUND) msg_pwarn("Cannot detach the existing USB driver. Claiming the interface may fail. %s\n", libusb_error_name(ret)); - ret = libusb_claim_interface(ch347_data->handle, spi_interface); + ret = libusb_claim_interface(ch347_data->handle, ch347_data->interface); if (ret != 0) { - msg_perr("Failed to claim interface 2: '%s'\n", libusb_error_name(ret)); + msg_perr("Failed to claim interface %d: '%s'\n", ch347_data->interface, libusb_error_name(ret)); goto error_exit; } -- To view, visit https://review.coreboot.org/c/flashrom/+/82193?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: I693baf1a0d9dc20757f56fba626b5f5ad20f71dd Gerrit-Change-Number: 82193 Gerrit-PatchSet: 14 Gerrit-Owner: ZhiYuanNJ Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>

1 0

[S] Change in flashrom[main]: flashchips: Add support for MXIC MX25R4035F
by Anastasia Klimchuk (Code Review) 02 Jun '24

02 Jun '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/81838?usp=email ) Change subject: flashchips: Add support for MXIC MX25R4035F ...................................................................... flashchips: Add support for MXIC MX25R4035F The MX25R4035F has been tested by ch341a programmer : read, write, erase and wp. We have tested --wp-enable, --wp-disable, --wp-list and --wp-range commands for write-protect feature. MX25R4035F datasheet is available at the following URL: https://www.macronix.com/Lists/Datasheet/Attachments/8671/MX25R4035F,%20Wid… Change-Id: I91dbc4735bf232e0b1dce72c7f06be967d35ebfb Signed-off-by: DanielZhang <danielzhang(a)mxic.com.cn> Reviewed-on: https://review.coreboot.org/c/flashrom/+/81838 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashchips.c M include/flashchips.h 2 files changed, 48 insertions(+), 0 deletions(-) Approvals: Anastasia Klimchuk: Looks good to me, approved build bot (Jenkins): Verified diff --git a/flashchips.c b/flashchips.c index 6fdc2f0..a5d9ec2 100644 --- a/flashchips.c +++ b/flashchips.c @@ -10669,6 +10669,53 @@ { .vendor = "Macronix", + .name = "MX25R4035F", + .bustype = BUS_SPI, + .manufacture_id = MACRONIX_ID, + .model_id = MACRONIX_MX25R4035F, + .total_size = 512, + .page_size = 256, + /* OTP: 1024B total; enter 0xB1, exit 0xC1 */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_CFGR, + .tested = TEST_OK_PREWB, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 128} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 16} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 8} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {1 * 512 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { {1 * 512 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_C7, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP3_SRWD, /* bit 6 is quad enable */ + .unlock = SPI_DISABLE_BLOCKPROTECT_BP3_SRWD, + .write = SPI_CHIP_WRITE256, + .read = SPI_CHIP_READ, /* Fast read (0x0B) and multi I/O supported */ + .voltage = {1650, 3600}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {CONFIG, 3, OTP} + }, + + .decode_range = DECODE_RANGE_SPI25, + }, + + { + .vendor = "Macronix", .name = "MX25R6435F", .bustype = BUS_SPI, .manufacture_id = MACRONIX_ID, diff --git a/include/flashchips.h b/include/flashchips.h index cb226fa..c264b04 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -546,6 +546,7 @@ #define MACRONIX_MX25R3235F 0x2816 #define MACRONIX_MX25R6435F 0x2817 #define MACRONIX_MX25R2035F 0x2812 +#define MACRONIX_MX25R4035F 0x2813 #define MACRONIX_MX25R8035F 0x2814 #define MACRONIX_MX29F001B 0x19 -- To view, visit https://review.coreboot.org/c/flashrom/+/81838?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: I91dbc4735bf232e0b1dce72c7f06be967d35ebfb Gerrit-Change-Number: 81838 Gerrit-PatchSet: 8 Gerrit-Owner: DZ <danielzhang(a)mxic.com.cn> 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>

1 0

[L] Change in flashrom[main]: erasure_layout: Fix get_flash_region bug
by Anastasia Klimchuk (Code Review) 01 Jun '24

01 Jun '24

Attention is currently required from: Aarya, Alexander Goncharov, Nikolai Artemiev, Peter Marheine. Anastasia Klimchuk has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82393?usp=email ) Change subject: erasure_layout: Fix get_flash_region bug ...................................................................... Patch Set 16: (1 comment) Patchset: PS15: > I added tests to run write operation over the chip with protected region(s). […] So given that I added a piece of code, maybe you can review it :) -- To view, visit https://review.coreboot.org/c/flashrom/+/82393?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: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Gerrit-Change-Number: 82393 Gerrit-PatchSet: 16 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sat, 01 Jun 2024 15:36:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[L] Change in flashrom[main]: erasure_layout: Fix get_flash_region bug
by Anastasia Klimchuk (Code Review) 01 Jun '24

01 Jun '24

Attention is currently required from: Aarya, Alexander Goncharov, Nikolai Artemiev, Peter Marheine. Anastasia Klimchuk has uploaded a new patch set (#16) to the change originally created by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82393?usp=email ) The following approvals got outdated and were removed: Verified+1 by build bot (Jenkins) Change subject: erasure_layout: Fix get_flash_region bug ...................................................................... erasure_layout: Fix get_flash_region bug When flash regions are protected, erase could incorrectly erase regions which were meant to be protected by requesting the correct size but using an erase opcode with coarser granularity than desired (for instance using a 16-byte erase command while attempting to erase only 8 bytes). This fixes that by exchanging the nesting of the loops over erase blocks and flash regions. Old: - Select erasefns - Loop over blocks to erase for each selected erasefn - Loop over programmer flash regions within erase block - Erase regions (may fail since selected erasefn will be too big if flash region is smaller than erase block) New: - Loop over programmer flash regions within erase block - Select erasefns within programer flash region - Loop over blocks to erase for each selected erasefn - Erase regions Eraser selection and erasing has also been factored out into a helper function to manage nesting depth. TEST=New test cases pass, whereas some of them fail without the changes to erasure_layout.c BUG=https://ticket.coreboot.org/issues/525 Change-Id: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Co-authored-by: Nikolai Artemiev <nartemiev(a)google.com> Co-authored-by: Anastasia Klimchuk <aklm(a)flashrom.org> Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> --- M erasure_layout.c M tests/erase_func_algo.c M tests/tests.c M tests/tests.h 4 files changed, 602 insertions(+), 96 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/93/82393/16 -- To view, visit https://review.coreboot.org/c/flashrom/+/82393?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: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Gerrit-Change-Number: 82393 Gerrit-PatchSet: 16 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>

1 0

[L] Change in flashrom[main]: erasure_layout: Fix get_flash_region bug
by Anastasia Klimchuk (Code Review) 01 Jun '24

01 Jun '24

Attention is currently required from: Aarya, Alexander Goncharov, Nikolai Artemiev, Peter Marheine. Anastasia Klimchuk has posted comments on this change by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82393?usp=email ) Change subject: erasure_layout: Fix get_flash_region bug ...................................................................... Patch Set 15: (2 comments) Patchset: PS15: I added tests to run write operation over the chip with protected region(s). Few things to note: 1) FLASHROM_FLAG_SKIP_UNREADABLE_REGIONS needed to be set for write operation, otherwise write failed. I am glad to discover that flags work, but now I want one more test, or actually two tests which set write and read flags false and make sure write fails Not necessarily in this patch 2) Now we need one more buf, I called it `written_protected_buf`, the reason is, in case of protected regions the content which is asked to be written is not what the end state of the chip is: because protected regions are not touched. File erasure_layout.c: https://review.coreboot.org/c/flashrom/+/82393/comment/145e00d1_d89b8974?us… : PS13, Line 368: for (unsigned int addr = region_start; addr <= region_end; addr += len) { > I made CB:82723 to illustrate, the issue repro on head […] I think this specific issue is outside of scope of the patch: mainly because it is not relevant to protected regions. Also, it seems to be an optimisation issue: the memory state is correct, and eraseblocks are good but not perfect. Since I have a repro case on the test case without protected regions in CB:82723 , I think we can move the discussion there. This patch should not be blocked by it. Do you agree? -- To view, visit https://review.coreboot.org/c/flashrom/+/82393?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: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Gerrit-Change-Number: 82393 Gerrit-PatchSet: 15 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Comment-Date: Sat, 01 Jun 2024 15:23:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org>

1 0

[L] Change in flashrom[main]: erasure_layout: Fix get_flash_region bug
by Anastasia Klimchuk (Code Review) 01 Jun '24

01 Jun '24

Attention is currently required from: Aarya, Alexander Goncharov, Anastasia Klimchuk, Nikolai Artemiev, Peter Marheine. Anastasia Klimchuk has uploaded a new patch set (#15) to the change originally created by Peter Marheine. ( https://review.coreboot.org/c/flashrom/+/82393?usp=email ) The following approvals got outdated and were removed: Verified-1 by build bot (Jenkins) Change subject: erasure_layout: Fix get_flash_region bug ...................................................................... erasure_layout: Fix get_flash_region bug When flash regions are protected, erase could incorrectly erase regions which were meant to be protected by requesting the correct size but using an erase opcode with coarser granularity than desired (for instance using a 16-byte erase command while attempting to erase only 8 bytes). This fixes that by exchanging the nesting of the loops over erase blocks and flash regions. Old: - Select erasefns - Loop over blocks to erase for each selected erasefn - Loop over programmer flash regions within erase block - Erase regions (may fail since selected erasefn will be too big if flash region is smaller than erase block) New: - Loop over programmer flash regions within erase block - Select erasefns within programer flash region - Loop over blocks to erase for each selected erasefn - Erase regions Eraser selection and erasing has also been factored out into a helper function to manage nesting depth. TEST=New test cases pass, whereas some of them fail without the changes to erasure_layout.c BUG=https://ticket.coreboot.org/issues/525 Change-Id: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Co-authored-by: Nikolai Artemiev <nartemiev(a)google.com> Co-authored-by: Anastasia Klimchuk <aklm(a)flashrom.org> Signed-off-by: Nikolai Artemiev <nartemiev(a)google.com> Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Signed-off-by: Peter Marheine <pmarheine(a)chromium.org> --- M erasure_layout.c M tests/erase_func_algo.c M tests/tests.c M tests/tests.h 4 files changed, 603 insertions(+), 97 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/93/82393/15 -- To view, visit https://review.coreboot.org/c/flashrom/+/82393?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: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9 Gerrit-Change-Number: 82393 Gerrit-PatchSet: 15 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Alexander Goncharov <chat(a)joursoir.net> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Attention: Alexander Goncharov <chat(a)joursoir.net>

1 0

[S] Change in flashrom[main]: flashchips.c: Add support for XM25RU256C
by Anastasia Klimchuk (Code Review) 01 Jun '24

01 Jun '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/82101?usp=email ) Change subject: flashchips.c: Add support for XM25RU256C ...................................................................... flashchips.c: Add support for XM25RU256C Tested read, write and erase with ch341a_spi programmer (and 1.8V adapter). Test log: https://paste.flashrom.org/view.php?id=3729 Change-Id: I431474a662304d09438e274706d3fc9cfbbe0bd6 Signed-off-by: Georg Gottleuber <ggo(a)tuxedocomputers.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/82101 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M flashchips.c M include/flashchips.h 2 files changed, 43 insertions(+), 0 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index 34e610a..6fdc2f0 100644 --- a/flashchips.c +++ b/flashchips.c @@ -22103,6 +22103,48 @@ }, { + .vendor = "XMC", + .name = "XM25RU256C", + .bustype = BUS_SPI, + .manufacture_id = ST_ID, + .model_id = XMC_XM25RU256C, + .total_size = 32768, + .page_size = 256, + /* supports SFDP */ + /* OTP: 1024B total, 256B reserved; read 0x48; write 0x42, erase 0x44, read ID 0x4B */ + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_4BA_ENTER_WREN | + FEATURE_4BA_EAR_C5C8 | FEATURE_4BA_READ | FEATURE_4BA_FAST_READ | + FEATURE_4BA_WRITE, + .tested = TEST_OK_PREW, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 8192} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 1024} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 512} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {32 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { {32 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_C7, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_PLAIN, /* TODO: improve */ + .unlock = SPI_DISABLE_BLOCKPROTECT, + .write = SPI_CHIP_WRITE256, + .read = SPI_CHIP_READ, + .voltage = {1650, 1950}, + }, + + { .vendor = "XTX Technology Limited", .name = "XT25F02E", .bustype = BUS_SPI, diff --git a/include/flashchips.h b/include/flashchips.h index d8e8bf1..cb226fa 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -851,6 +851,7 @@ #define XMC_XM25QU128C 0x4118 #define XMC_XM25QH256C 0x4019 #define XMC_XM25QU256C 0x4119 +#define XMC_XM25RU256C 0x4419 #define ST_M25PX80 0x7114 #define ST_M25PX16 0x7115 #define ST_M25PX32 0x7116 -- To view, visit https://review.coreboot.org/c/flashrom/+/82101?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: I431474a662304d09438e274706d3fc9cfbbe0bd6 Gerrit-Change-Number: 82101 Gerrit-PatchSet: 5 Gerrit-Owner: Georg Gottleuber <ggo(a)tuxedo.de> 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>

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit