August 2024 - flashrom-gerrit - mail.coreboot.org

[S] Change in flashrom[main]: Provide an option to use PCI_ACCESS_ECAM to access pci register
by Kane Chen (Code Review) 16 Aug '24

16 Aug '24

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen, Kane Chen, Peter Marheine, Subrata Banik. Hello Hsuan Ting Chen, Hsuan-ting Chen, Peter Marheine, Subrata Banik, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/83896?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Code-Review+1 by Subrata Banik Change subject: Provide an option to use PCI_ACCESS_ECAM to access pci register ...................................................................... Provide an option to use PCI_ACCESS_ECAM to access pci register In the latest pciutils(v3.13.0), it supports accessing pci registers by ecam. This patch adds use_libpci_ecam option and libpci version check to decide whether flashrom calls libpci and use 0xcf8/0xcfc or ecam to access pci registers BUG=b:359813524 TEST=with use_libpci_ecam and libpci >= 3.13.0, flashrom is working with ECAM access Change-Id: I4549f87c8b01da0a1d3d8ce0b3b75c1f5fa2cbab Signed-off-by: Kane Chen <kane.chen(a)intel.com> --- M chipset_enable.c M meson.build 2 files changed, 13 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/96/83896/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/83896?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: I4549f87c8b01da0a1d3d8ce0b3b75c1f5fa2cbab Gerrit-Change-Number: 83896 Gerrit-PatchSet: 4 Gerrit-Owner: Kane Chen <kane.chen(a)intel.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-CC: Kane Chen <kane.chen(a)intel.corp-partner.google.com> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Subrata Banik <subratabanik(a)google.com> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Kane Chen <kane.chen(a)intel.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org>

1 0

[S] Change in flashrom[main]: Provide an option to use PCI_ACCESS_ECAM to access pci register
by Kane Chen (Code Review) 16 Aug '24

16 Aug '24

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen, Kane Chen, Peter Marheine. Kane Chen has posted comments on this change by Kane Chen. ( https://review.coreboot.org/c/flashrom/+/83896?usp=email ) Change subject: Provide an option to use PCI_ACCESS_ECAM to access pci register ...................................................................... Patch Set 3: (1 comment) File meson_options.txt: https://review.coreboot.org/c/flashrom/+/83896/comment/d341e2b2_0241c83c?us… : PS3, Line 9: option('use_libpci_ecam', type : 'boolean', value : false) > It looks like this is only used for 100-series PCHs which presumably always support ECAM, so is this […] My original thought is to avoid any potential impact since i only have chromebooks to verify. if it's ok to use ECAM directly, i can remove this option. thank you -- To view, visit https://review.coreboot.org/c/flashrom/+/83896?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: I4549f87c8b01da0a1d3d8ce0b3b75c1f5fa2cbab Gerrit-Change-Number: 83896 Gerrit-PatchSet: 3 Gerrit-Owner: Kane Chen <kane.chen(a)intel.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-CC: Kane Chen <kane.chen(a)intel.corp-partner.google.com> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Kane Chen <kane.chen(a)intel.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Fri, 16 Aug 2024 05:12:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Peter Marheine <pmarheine(a)chromium.org>

1 0

[S] Change in flashrom[main]: Provide an option to use PCI_ACCESS_ECAM to access pci register
by Peter Marheine (Code Review) 16 Aug '24

16 Aug '24

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen, Kane Chen. Peter Marheine has posted comments on this change by Kane Chen. ( https://review.coreboot.org/c/flashrom/+/83896?usp=email ) Change subject: Provide an option to use PCI_ACCESS_ECAM to access pci register ...................................................................... Patch Set 3: (1 comment) File meson_options.txt: https://review.coreboot.org/c/flashrom/+/83896/comment/e7d5df65_be4f737e?us… : PS3, Line 9: option('use_libpci_ecam', type : 'boolean', value : false) It looks like this is only used for 100-series PCHs which presumably always support ECAM, so is this option needed? It seems like if we can assume ECAM is always available on the relevant systems and it's less prone to bugs caused by libpci interacting poorly with OS-level drivers accessing the same hardware, then we should use ECAM as long as it's supported by the version of libpci in use and don't need this option. -- To view, visit https://review.coreboot.org/c/flashrom/+/83896?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: I4549f87c8b01da0a1d3d8ce0b3b75c1f5fa2cbab Gerrit-Change-Number: 83896 Gerrit-PatchSet: 3 Gerrit-Owner: Kane Chen <kane.chen(a)intel.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-CC: Kane Chen <kane.chen(a)intel.corp-partner.google.com> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Kane Chen <kane.chen(a)intel.com> Gerrit-Comment-Date: Fri, 16 Aug 2024 03:42:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[XS] Change in flashrom[main]: flashchips: add GD25LB512ME
by Anastasia Klimchuk (Code Review) 15 Aug '24

15 Aug '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/83795?usp=email ) Change subject: flashchips: add GD25LB512ME ...................................................................... flashchips: add GD25LB512ME Added GD25LB512ME to Flashchips.C added Sames as GD25LB512ME to GIGADEVICE_GD25LR512ME to flashchips.h GD25LB512ME is 1.8V 512Mbit, Quad enabled when shipped. https://www.gigadevice.com.cn/Public/Uploads/uploadfile/files/20230627/DS-0… Tested on ch347 with erase, program, read, and protection. Change-Id: I04103814f901478098c1a989f4239792b64073ec Signed-off-by: Victor Lim <vlim(a)gigadevice.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/83795 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, 3 insertions(+), 3 deletions(-) Approvals: build bot (Jenkins): Verified Anastasia Klimchuk: Looks good to me, approved diff --git a/flashchips.c b/flashchips.c index 60fbb1b..d566001 100644 --- a/flashchips.c +++ b/flashchips.c @@ -7068,7 +7068,7 @@ { .vendor = "GigaDevice", - .name = "GD25LR512ME", + .name = "GD25LB512ME/GD25LR512ME", .bustype = BUS_SPI, .manufacture_id = GIGADEVICE_ID, .model_id = GIGADEVICE_GD25LR512ME, @@ -7076,7 +7076,7 @@ .page_size = 256, /* OTP: 4096B total; read 0x48; write 0x42, erase 0x44 */ .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_QPI | FEATURE_4BA, - .tested = TEST_OK_PREW, + .tested = TEST_OK_PREWB, .probe = PROBE_SPI_RDID, .probe_timing = TIMING_ZERO, .block_erasers = diff --git a/include/flashchips.h b/include/flashchips.h index d8146ec..f309a55 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -407,7 +407,7 @@ #define GIGADEVICE_GD25LF128E 0x6318 #define GIGADEVICE_GD25LF256F 0x6319 #define GIGADEVICE_GD25LR256E 0x6719 -#define GIGADEVICE_GD25LR512ME 0x671A +#define GIGADEVICE_GD25LR512ME 0x671A /* Same as GD25LB512ME */ #define GIGADEVICE_GD25WQ80E 0x6514 #define GIGADEVICE_GD29GL064CAB 0x7E0601 -- To view, visit https://review.coreboot.org/c/flashrom/+/83795?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: I04103814f901478098c1a989f4239792b64073ec Gerrit-Change-Number: 83795 Gerrit-PatchSet: 4 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.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>

1 0

[XS] Change in flashrom[main]: [stlinkv3_spi]Mark STLinkV3-Mini not working
by Miklós Márton (Code Review) 15 Aug '24

15 Aug '24

Miklós Márton has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/83921?usp=email ) Change subject: [stlinkv3_spi]Mark STLinkV3-Mini not working ...................................................................... [stlinkv3_spi]Mark STLinkV3-Mini not working The STLinkV3 Mini does not support the bridge API, it return LIBUSB_IO_ERROR when querying the bridge version. The official ST updater does not lists the bridge version in the info screen. Due to it's construction (additional connector on the bottom) it is likely that ST disabled the bridge functions on the castellated pads. Change-Id: Ic1ea4ca7acedca5d374cff8fcef450c18e55a9e8 Signed-off-by: Miklos Marton <martonmiklosqdev(a)gmail.com> --- M stlinkv3_spi.c 1 file changed, 7 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/21/83921/1 diff --git a/stlinkv3_spi.c b/stlinkv3_spi.c index f9046df..d1de1dd 100644 --- a/stlinkv3_spi.c +++ b/stlinkv3_spi.c @@ -115,7 +115,7 @@ #define USB_TIMEOUT_IN_MS 5000 static const struct dev_entry devs_stlinkv3_spi[] = { - {0x0483, 0x374E, NT, "STMicroelectronics", "STLINK-V3E"}, + {0x0483, 0x374E, BAD, "STMicroelectronics", "STLINK-V3E"}, {0x0483, 0x374F, OK, "STMicroelectronics", "STLINK-V3S"}, {0x0483, 0x3753, OK, "STMicroelectronics", "STLINK-V3 dual VCP"}, {0x0483, 0x3754, NT, "STMicroelectronics", "STLINK-V3 no MSD"}, @@ -498,8 +498,13 @@ devs_stlinkv3_spi[devIndex].vendor_id, devs_stlinkv3_spi[devIndex].device_id, param_str); - if (stlinkv3_handle) + if (stlinkv3_handle) { + if (devs_stlinkv3_spi[devIndex].status == BAD) { + msg_perr("The STLINK-V3 Mini/MiniE does not support the bridge interface\n"); + goto init_err_exit; + } break; + } devIndex++; } -- To view, visit https://review.coreboot.org/c/flashrom/+/83921?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: Ic1ea4ca7acedca5d374cff8fcef450c18e55a9e8 Gerrit-Change-Number: 83921 Gerrit-PatchSet: 1 Gerrit-Owner: Miklós Márton <martonmiklosqdev(a)gmail.com>

1 0

[S] Change in flashrom[main]: Provide an option to use PCI_ACCESS_ECAM to access pci register
by Subrata Banik (Code Review) 15 Aug '24

15 Aug '24

Attention is currently required from: Hsuan Ting Chen, Hsuan-ting Chen, Kane Chen, Peter Marheine. Subrata Banik has posted comments on this change by Kane Chen. ( https://review.coreboot.org/c/flashrom/+/83896?usp=email ) Change subject: Provide an option to use PCI_ACCESS_ECAM to access pci register ...................................................................... Patch Set 3: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/83896?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: I4549f87c8b01da0a1d3d8ce0b3b75c1f5fa2cbab Gerrit-Change-Number: 83896 Gerrit-PatchSet: 3 Gerrit-Owner: Kane Chen <kane.chen(a)intel.com> Gerrit-Reviewer: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Reviewer: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Subrata Banik <subratabanik(a)google.com> Gerrit-CC: Kane Chen <kane.chen(a)intel.corp-partner.google.com> Gerrit-Attention: Hsuan Ting Chen <roccochen(a)chromium.org> Gerrit-Attention: Hsuan-ting Chen <roccochen(a)google.com> Gerrit-Attention: Kane Chen <kane.chen(a)intel.com> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 15 Aug 2024 06:35:45 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in flashrom[main]: Provide an option to use PCI_ACCESS_ECAM to access pci register
by Kane Chen (Code Review) 15 Aug '24

15 Aug '24

Kane Chen has posted comments on this change by Kane Chen. ( https://review.coreboot.org/c/flashrom/+/83896?usp=email ) Change subject: Provide an option to use PCI_ACCESS_ECAM to access pci register ...................................................................... Patch Set 3: This change is ready for review. -- To view, visit https://review.coreboot.org/c/flashrom/+/83896?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: I4549f87c8b01da0a1d3d8ce0b3b75c1f5fa2cbab Gerrit-Change-Number: 83896 Gerrit-PatchSet: 3 Gerrit-Owner: Kane Chen <kane.chen(a)intel.com> Gerrit-CC: Kane Chen <kane.chen(a)intel.corp-partner.google.com> Gerrit-Comment-Date: Thu, 15 Aug 2024 05:51:04 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No

1 0

[S] Change in flashrom[main]: flashrom.c: Rename {erase|write}_by_layout_new as the only one
by Anastasia Klimchuk (Code Review) 15 Aug '24

15 Aug '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/83852?usp=email ) Change subject: flashrom.c: Rename {erase|write}_by_layout_new as the only one ...................................................................... flashrom.c: Rename {erase|write}_by_layout_new as the only one We used to have two code paths for erase and write, so we had {erase|write}_by_layout in two variants: *_new and *_legacy. Now that legacy is removed, *_new can be renamed without *_new suffix. Change-Id: Ib21bf29e1993c4fc0516e76fde2ad283eedb50d2 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/83852 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Aarya <aarya.chaumal(a)gmail.com> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M flashrom.c 1 file changed, 2 insertions(+), 14 deletions(-) Approvals: Aarya: Looks good to me, approved build bot (Jenkins): Verified Peter Marheine: Looks good to me, approved diff --git a/flashrom.c b/flashrom.c index 453a2bf..01a41c4 100644 --- a/flashrom.c +++ b/flashrom.c @@ -1327,7 +1327,7 @@ return ret; } -static int erase_by_layout_new(struct flashctx *const flashctx) +static int erase_by_layout(struct flashctx *const flashctx) { bool all_skipped = true; const uint32_t flash_size = flashctx->chip->total_size * 1024; @@ -1370,12 +1370,7 @@ return ret; } -static int erase_by_layout(struct flashctx *const flashctx) -{ - return erase_by_layout_new(flashctx); -} - -static int write_by_layout_new(struct flashctx *const flashctx, +static int write_by_layout(struct flashctx *const flashctx, void *const curcontents, const void *const newcontents, bool *all_skipped) { @@ -1410,13 +1405,6 @@ return ret; } -static int write_by_layout(struct flashctx *const flashctx, - uint8_t *const curcontents, const uint8_t *const newcontents, - bool *all_skipped) -{ - return write_by_layout_new(flashctx, curcontents, newcontents, all_skipped); -} - /** * @brief Compares the included layout regions with content from a buffer. * -- To view, visit https://review.coreboot.org/c/flashrom/+/83852?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: Ib21bf29e1993c4fc0516e76fde2ad283eedb50d2 Gerrit-Change-Number: 83852 Gerrit-PatchSet: 2 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(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>

1 0

[L] Change in flashrom[main]: flashrom.c: Delete legacy erase and write logic
by Anastasia Klimchuk (Code Review) 15 Aug '24

15 Aug '24

Anastasia Klimchuk has submitted this change. ( https://review.coreboot.org/c/flashrom/+/83846?usp=email ) Change subject: flashrom.c: Delete legacy erase and write logic ...................................................................... flashrom.c: Delete legacy erase and write logic Current code path for erase and write has been enabled in the tree since May 2023, which is more than 1 year ago (15 months ago), and legacy path has been disabled since the same time. Current logic has been officially released in v1.4.0 in July 2024. Change-Id: I08fd686fecf6a5313eea2d66b368661c664f4800 Signed-off-by: Anastasia Klimchuk <aklm(a)flashrom.org> Reviewed-on: https://review.coreboot.org/c/flashrom/+/83846 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Aarya <aarya.chaumal(a)gmail.com> Reviewed-by: Peter Marheine <pmarheine(a)chromium.org> --- M erasure_layout.c M flashrom.c 2 files changed, 1 insertion(+), 369 deletions(-) Approvals: Aarya: Looks good to me, approved build bot (Jenkins): Verified Peter Marheine: Looks good to me, approved diff --git a/erasure_layout.c b/erasure_layout.c index ab54c12..a7eaa2d 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -151,7 +151,7 @@ * @param region_start pointer to start address of the region to align * @param region_end pointer to end address of the region to align * - * This function aligns start and end address of the region (in struct walk_info) + * This function aligns start and end address of the region * to some erase sector boundaries and modify the region start and end addresses * to match nearest erase sector boundaries. This function will be used in the * new algorithm for erase function selection. diff --git a/flashrom.c b/flashrom.c index dbf06cf..453a2bf 100644 --- a/flashrom.c +++ b/flashrom.c @@ -36,8 +36,6 @@ #include "chipdrivers.h" #include "erasure_layout.h" -static bool use_legacy_erase_path = false; - const char flashrom_version[] = FLASHROM_VERSION; static const struct programmer_entry *programmer = NULL; @@ -1329,250 +1327,6 @@ return ret; } -typedef int (*erasefn_t)(struct flashctx *, unsigned int addr, unsigned int len); -/** - * @private - * - * For read-erase-write, `curcontents` and `newcontents` shall point - * to buffers of the chip's size. Both are supposed to be prefilled - * with at least the included layout regions of the current flash - * contents (`curcontents`) and the data to be written to the flash - * (`newcontents`). - * - * For erase, `curcontents` and `newcontents` shall be NULL-pointers. - * - * The `chipoff_t` values are used internally by `walk_by_layout()`. - */ -struct walk_info { - uint8_t *curcontents; - const uint8_t *newcontents; - chipoff_t region_start; - chipoff_t region_end; - chipoff_t erase_start; - chipoff_t erase_end; -}; -/* returns 0 on success, 1 to retry with another erase function, 2 for immediate abort */ -typedef int (*per_blockfn_t)(struct flashctx *, const struct walk_info *, erasefn_t, bool *); - -static int walk_eraseblocks(struct flashctx *const flashctx, - struct walk_info *const info, - const size_t erasefunction, const per_blockfn_t per_blockfn, - bool *all_skipped) -{ - int ret; - size_t i, j; - bool first = true; - struct block_eraser *const eraser = &flashctx->chip->block_erasers[erasefunction]; - - info->erase_start = 0; - for (i = 0; i < NUM_ERASEREGIONS; ++i) { - /* count==0 for all automatically initialized array - members so the loop below won't be executed for them. */ - for (j = 0; j < eraser->eraseblocks[i].count; ++j, info->erase_start = info->erase_end + 1) { - info->erase_end = info->erase_start + eraser->eraseblocks[i].size - 1; - - /* Skip any eraseblock that is completely outside the current region. */ - if (info->erase_end < info->region_start) - continue; - if (info->region_end < info->erase_start) - break; - - /* Print this for every block except the first one. */ - if (first) - first = false; - else - msg_cdbg(", "); - msg_cdbg("0x%06"PRIx32"-0x%06"PRIx32":", info->erase_start, info->erase_end); - - erasefunc_t *erase_func = lookup_erase_func_ptr(eraser); - ret = per_blockfn(flashctx, info, erase_func, all_skipped); - if (ret) - return ret; - } - if (info->region_end < info->erase_start) - break; - } - msg_cdbg("\n"); - return 0; -} - -static int walk_by_layout(struct flashctx *const flashctx, struct walk_info *const info, - const per_blockfn_t per_blockfn, bool *all_skipped) -{ - const struct flashrom_layout *const layout = get_layout(flashctx); - const struct romentry *entry = NULL; - - *all_skipped = true; - msg_cinfo("Erasing and writing flash chip... "); - - while ((entry = layout_next_included(layout, entry))) { - const struct flash_region *region = &entry->region; - info->region_start = region->start; - info->region_end = region->end; - - size_t j; - int error = 1; /* retry as long as it's 1 */ - for (j = 0; j < NUM_ERASEFUNCTIONS; ++j) { - if (j != 0) - msg_cinfo("Looking for another erase function.\n"); - msg_cdbg("Trying erase function %zi... ", j); - if (check_block_eraser(flashctx, j, 1)) - continue; - - error = walk_eraseblocks(flashctx, info, j, per_blockfn, all_skipped); - if (error != 1) - break; - - if (info->curcontents) { - msg_cinfo("Reading current flash chip contents... "); - if (read_by_layout(flashctx, info->curcontents)) { - /* Now we are truly screwed. Read failed as well. */ - msg_cerr("Can't read anymore! Aborting.\n"); - /* We have no idea about the flash chip contents, so - retrying with another erase function is pointless. */ - error = 2; - break; - } - msg_cinfo("done. "); - } - } - if (error == 1) - msg_cinfo("No usable erase functions left.\n"); - if (error) { - msg_cerr("FAILED!\n"); - return 1; - } - } - if (*all_skipped) - msg_cinfo("\nWarning: Chip content is identical to the requested image.\n"); - msg_cinfo("Erase/write done.\n"); - return 0; -} - -static int erase_block(struct flashctx *const flashctx, - const struct walk_info *const info, const erasefn_t erasefn, - bool *all_skipped) -{ - const unsigned int erase_len = info->erase_end + 1 - info->erase_start; - const bool region_unaligned = info->region_start > info->erase_start || - info->erase_end > info->region_end; - uint8_t *backup_contents = NULL, *erased_contents = NULL; - int ret = 2; - - /* - * If the region is not erase-block aligned, merge current flash con- - * tents into a new buffer `backup_contents`. - */ - if (region_unaligned) { - backup_contents = malloc(erase_len); - erased_contents = malloc(erase_len); - if (!backup_contents || !erased_contents) { - msg_cerr("Out of memory!\n"); - ret = 1; - goto _free_ret; - } - memset(backup_contents, ERASED_VALUE(flashctx), erase_len); - memset(erased_contents, ERASED_VALUE(flashctx), erase_len); - - msg_cdbg("R"); - /* Merge data preceding the current region. */ - if (info->region_start > info->erase_start) { - const chipoff_t start = info->erase_start; - const chipsize_t len = info->region_start - info->erase_start; - if (read_flash(flashctx, backup_contents, start, len)) { - msg_cerr("Can't read! Aborting.\n"); - goto _free_ret; - } - } - /* Merge data following the current region. */ - if (info->erase_end > info->region_end) { - const chipoff_t start = info->region_end + 1; - const chipoff_t rel_start = start - info->erase_start; /* within this erase block */ - const chipsize_t len = info->erase_end - info->region_end; - if (read_flash(flashctx, backup_contents + rel_start, start, len)) { - msg_cerr("Can't read! Aborting.\n"); - goto _free_ret; - } - } - } - - ret = 1; - *all_skipped = false; - - msg_cdbg("E"); - - if (!flashctx->flags.skip_unwritable_regions) { - if (check_for_unwritable_regions(flashctx, info->erase_start, erase_len)) - goto _free_ret; - } - - unsigned int len; - for (unsigned int addr = info->erase_start; addr < info->erase_start + erase_len; addr += len) { - struct flash_region region; - get_flash_region(flashctx, addr, &region); - - len = min(info->erase_start + erase_len, region.end + 1) - addr; - - if (region.write_prot) { - msg_gdbg("%s: cannot erase inside %s region (%#08"PRIx32"..%#08"PRIx32"), skipping range (%#08x..%#08x).\n", - __func__, region.name, region.start, region.end, addr, addr + len - 1); - free(region.name); - continue; - } - - msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is writable, erasing range (%#08x..%#08x).\n", - __func__, region.name, region.start, region.end, addr, addr + len - 1); - free(region.name); - - if (erasefn(flashctx, addr, len)) - goto _free_ret; - if (check_erased_range(flashctx, addr, len)) { - msg_cerr("ERASE FAILED!\n"); - goto _free_ret; - } - } - - - if (region_unaligned) { - unsigned int starthere = 0, lenhere = 0, writecount = 0; - /* get_next_write() sets starthere to a new value after the call. */ - while ((lenhere = get_next_write(erased_contents + starthere, backup_contents + starthere, - erase_len - starthere, &starthere, flashctx->chip->gran))) { - if (!writecount++) - msg_cdbg("W"); - /* Needs the partial write function signature. */ - if (write_flash(flashctx, backup_contents + starthere, - info->erase_start + starthere, lenhere)) - goto _free_ret; - starthere += lenhere; - } - } - - ret = 0; - -_free_ret: - free(erased_contents); - free(backup_contents); - return ret; -} - -/** - * @brief Erases the included layout regions. - * - * If there is no layout set in the given flash context, the whole chip will - * be erased. - * - * @param flashctx Flash context to be used. - * @return 0 on success, - * 1 if all available erase functions failed. - */ -static int erase_by_layout_legacy(struct flashctx *const flashctx) -{ - struct walk_info info = { 0 }; - bool all_skipped = true; - return walk_by_layout(flashctx, &info, &erase_block, &all_skipped); -} - static int erase_by_layout_new(struct flashctx *const flashctx) { bool all_skipped = true; @@ -1618,129 +1372,9 @@ static int erase_by_layout(struct flashctx *const flashctx) { - if (use_legacy_erase_path) - return erase_by_layout_legacy(flashctx); return erase_by_layout_new(flashctx); } -static int read_erase_write_block(struct flashctx *const flashctx, - const struct walk_info *const info, const erasefn_t erasefn, - bool *all_skipped) -{ - const chipsize_t erase_len = info->erase_end + 1 - info->erase_start; - const bool region_unaligned = info->region_start > info->erase_start || - info->erase_end > info->region_end; - const uint8_t *newcontents = NULL; - int ret = 2; - - /* - * If the region is not erase-block aligned, merge current flash con- - * tents into `info->curcontents` and a new buffer `newc`. The former - * is necessary since we have no guarantee that the full erase block - * was already read into `info->curcontents`. For the latter a new - * buffer is used since `info->newcontents` might contain data for - * other unaligned regions that touch this erase block too. - */ - if (region_unaligned) { - msg_cdbg("R"); - uint8_t *const newc = malloc(erase_len); - if (!newc) { - msg_cerr("Out of memory!\n"); - return 1; - } - memcpy(newc, info->newcontents + info->erase_start, erase_len); - - /* Merge data preceding the current region. */ - if (info->region_start > info->erase_start) { - const chipoff_t start = info->erase_start; - const chipsize_t len = info->region_start - info->erase_start; - if (read_flash(flashctx, newc, start, len)) { - msg_cerr("Can't read! Aborting.\n"); - goto _free_ret; - } - memcpy(info->curcontents + start, newc, len); - } - /* Merge data following the current region. */ - if (info->erase_end > info->region_end) { - const chipoff_t start = info->region_end + 1; - const chipoff_t rel_start = start - info->erase_start; /* within this erase block */ - const chipsize_t len = info->erase_end - info->region_end; - if (read_flash(flashctx, newc + rel_start, start, len)) { - msg_cerr("Can't read! Aborting.\n"); - goto _free_ret; - } - memcpy(info->curcontents + start, newc + rel_start, len); - } - - newcontents = newc; - } else { - newcontents = info->newcontents + info->erase_start; - } - - ret = 1; - bool skipped = true; - uint8_t *const curcontents = info->curcontents + info->erase_start; - const uint8_t erased_value = ERASED_VALUE(flashctx); - if (!(flashctx->chip->feature_bits & FEATURE_NO_ERASE) && - need_erase(curcontents, newcontents, erase_len, flashctx->chip->gran, erased_value)) { - if (erase_block(flashctx, info, erasefn, all_skipped)) - goto _free_ret; - /* Erase was successful. Adjust curcontents. */ - memset(curcontents, erased_value, erase_len); - skipped = false; - } - - unsigned int starthere = 0, lenhere = 0, writecount = 0; - /* get_next_write() sets starthere to a new value after the call. */ - while ((lenhere = get_next_write(curcontents + starthere, newcontents + starthere, - erase_len - starthere, &starthere, flashctx->chip->gran))) { - if (!writecount++) - msg_cdbg("W"); - /* Needs the partial write function signature. */ - if (write_flash(flashctx, newcontents + starthere, - info->erase_start + starthere, lenhere)) - goto _free_ret; - starthere += lenhere; - skipped = false; - } - if (skipped) - msg_cdbg("S"); - else - *all_skipped = false; - - /* Update curcontents, other regions with overlapping erase blocks - might rely on this. */ - memcpy(curcontents, newcontents, erase_len); - ret = 0; - -_free_ret: - if (region_unaligned) - free((void *)newcontents); - return ret; -} - -/** - * @brief Writes the included layout regions from a given image. - * - * If there is no layout set in the given flash context, the whole image - * will be written. - * - * @param flashctx Flash context to be used. - * @param curcontents A buffer of full chip size with current chip contents of included regions. - * @param newcontents The new image to be written. - * @return 0 on success, - * 1 if anything has gone wrong. - */ -static int write_by_layout_legacy(struct flashctx *const flashctx, - void *const curcontents, const void *const newcontents, - bool *all_skipped) -{ - struct walk_info info; - info.curcontents = curcontents; - info.newcontents = newcontents; - return walk_by_layout(flashctx, &info, read_erase_write_block, all_skipped); -} - static int write_by_layout_new(struct flashctx *const flashctx, void *const curcontents, const void *const newcontents, bool *all_skipped) @@ -1780,8 +1414,6 @@ uint8_t *const curcontents, const uint8_t *const newcontents, bool *all_skipped) { - if (use_legacy_erase_path) - return write_by_layout_legacy(flashctx, curcontents, newcontents, all_skipped); return write_by_layout_new(flashctx, curcontents, newcontents, all_skipped); } -- To view, visit https://review.coreboot.org/c/flashrom/+/83846?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: I08fd686fecf6a5313eea2d66b368661c664f4800 Gerrit-Change-Number: 83846 Gerrit-PatchSet: 4 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(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>

1 0

[M] Change in flashrom[main]: flashchips: add GD25LF512MF model
by Victor Lim (Code Review) 14 Aug '24

14 Aug '24

Victor Lim has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/83912?usp=email ) Change subject: flashchips: add GD25LF512MF model ...................................................................... flashchips: add GD25LF512MF model GD25LF512MF: 1.8V 512Mbit high performance, quad IO enabled. I will have to send you the datasheet through email. Tested on ch347 with erase, write, read, and protection Change-Id: I3d202f5afcc9c33a4040f8186dc6fef1878ba79a Signed-off-by: Victor Lim <vlim(a)gigadevice.com> --- M flashchips.c M include/flashchips.h 2 files changed, 57 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/12/83912/1 diff --git a/flashchips.c b/flashchips.c index 2617162..5b8069c 100644 --- a/flashchips.c +++ b/flashchips.c @@ -6644,6 +6644,62 @@ { .vendor = "GigaDevice", + .name = "GD25LF512MF", + .bustype = BUS_SPI, + .manufacture_id = GIGADEVICE_ID, + .model_id = GIGADEVICE_GD25LF512MF, + .total_size = 65536, + .page_size = 256, + .feature_bits = FEATURE_WRSR_WREN | FEATURE_OTP | FEATURE_WRSR_EXT2 | FEATURE_4BA, + .tested = TEST_OK_PREWB, + .probe = PROBE_SPI_RDID, + .probe_timing = TIMING_ZERO, + .block_erasers = + { + { + .eraseblocks = { {4 * 1024, 16384} }, + .block_erase = SPI_BLOCK_ERASE_21, + }, { + .eraseblocks = { {4 * 1024, 16384} }, + .block_erase = SPI_BLOCK_ERASE_20, + }, { + .eraseblocks = { {32 * 1024, 2048} }, + .block_erase = SPI_BLOCK_ERASE_5C, + }, { + .eraseblocks = { {32 * 1024, 2048} }, + .block_erase = SPI_BLOCK_ERASE_52, + }, { + .eraseblocks = { {64 * 1024, 1024} }, + .block_erase = SPI_BLOCK_ERASE_DC, + }, { + .eraseblocks = { {64 * 1024, 1024} }, + .block_erase = SPI_BLOCK_ERASE_D8, + }, { + .eraseblocks = { {64 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_60, + }, { + .eraseblocks = { {64 * 1024 * 1024, 1} }, + .block_erase = SPI_BLOCK_ERASE_C7, + } + }, + .printlock = SPI_PRETTYPRINT_STATUS_REGISTER_BP4_SRWD, /* TODO: 2nd status reg (read with 0x35) */ + .unlock = SPI_DISABLE_BLOCKPROTECT_BP4_SRWD, + .write = SPI_CHIP_WRITE256, + .read = SPI_CHIP_READ, + .voltage = {1695, 1950}, + .reg_bits = + { + .srp = {STATUS1, 7, RW}, + .bp = {{STATUS1, 2, RW}, {STATUS1, 3, RW}, {STATUS1, 4, RW}, {STATUS1, 5, RW}}, + .tb = {STATUS1, 6, RW}, /* Called BP4 in datasheet, acts like TB */ + .srl = {STATUS2, 0, RW}, + .cmp = {STATUS2, 6, RW}, + }, + .decode_range = DECODE_RANGE_SPI25, + }, + + { + .vendor = "GigaDevice", .name = "GD25LQ128E/GD25LB128E/GD25LR128E/GD25LQ128D/GD25LQ128C", .bustype = BUS_SPI, .manufacture_id = GIGADEVICE_ID, diff --git a/include/flashchips.h b/include/flashchips.h index 250ade4..20f799e 100644 --- a/include/flashchips.h +++ b/include/flashchips.h @@ -407,6 +407,7 @@ #define GIGADEVICE_GD25LB512MF 0x601A /* Same as GD25LR512MF */ #define GIGADEVICE_GD25LF128E 0x6318 #define GIGADEVICE_GD25LF256F 0x6319 +#define GIGADEVICE_GD25LF512MF 0x631A #define GIGADEVICE_GD25LR256E 0x6719 #define GIGADEVICE_GD25LR512ME 0x671A /* Same as GD25LB512ME */ #define GIGADEVICE_GD25WQ80E 0x6514 -- To view, visit https://review.coreboot.org/c/flashrom/+/83912?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: I3d202f5afcc9c33a4040f8186dc6fef1878ba79a Gerrit-Change-Number: 83912 Gerrit-PatchSet: 1 Gerrit-Owner: Victor Lim <vlim(a)gigadevice.com>

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit August 2024