February 2023 - flashrom-gerrit

[S] Change in flashrom[master]: programmer_table.c: Allow name_to_entry() to return default when set
by Thomas Heijligen (Code Review) 14 Feb '23

14 Feb '23

Attention is currently required from: Edward O'Callaghan. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72821 ) Change subject: programmer_table.c: Allow name_to_entry() to return default when set ...................................................................... Patch Set 1: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/72821/comment/9b449354_e1bedde5 PS1, Line 8: When CONFIG_DEFAULT_PROGRAMMER_NAME is set by the build system, programmer_name_to_entry() returns this if an empty string is given as name. -- To view, visit https://review.coreboot.org/c/flashrom/+/72821 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I75fd24a76d0f6f25072428a8d992a1977c4ad1f8 Gerrit-Change-Number: 72821 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 14 Feb 2023 16:40:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: programmer_table.c: Allow name_to_entry() to return default when set
by Thomas Heijligen (Code Review) 14 Feb '23

14 Feb '23

Attention is currently required from: Edward O'Callaghan. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72821 ) Change subject: programmer_table.c: Allow name_to_entry() to return default when set ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Patchset: PS1: You know my intentions for CONFIG_DEFAULT_PROGRAMMER..., but as long as we keep it this is a good move -- To view, visit https://review.coreboot.org/c/flashrom/+/72821 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I75fd24a76d0f6f25072428a8d992a1977c4ad1f8 Gerrit-Change-Number: 72821 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 14 Feb 2023 16:37:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: programmer_table.c: Move name to entry to own function
by Thomas Heijligen (Code Review) 14 Feb '23

14 Feb '23

Attention is currently required from: Edward O'Callaghan. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72820 ) Change subject: programmer_table.c: Move name to entry to own function ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/72820/comment/dcad95ef_f8db1a67 PS1, Line 7: programmer_table.c: Move name to entry to own function ...: Move programmer name to entry ... -- To view, visit https://review.coreboot.org/c/flashrom/+/72820 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5da3861b47d0d0b527e7cde5dd61bb696de6fedb Gerrit-Change-Number: 72820 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 14 Feb 2023 16:34:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: printf: Use inttype.h macros for format strings
by Thomas Heijligen (Code Review) 14 Feb '23

14 Feb '23

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/73040 ) Change subject: printf: Use inttype.h macros for format strings ...................................................................... printf: Use inttype.h macros for format strings DJGPP for compiling DOS has other sizes for the normal int types and therefore throwing errors when when using %i %d or %x with uint32_t. Fix these warnings by using the macros created for it and provided in inttypes.h. Change-Id: Ia75b6df981ce60c891161fe553c7ceab8570178d Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M dummyflasher.c M erasure_layout.c M flashrom.c M ich_descriptors.c M jedec.c M layout.c M s25f.c M sfdp.c M spi25.c M spi95.c 10 files changed, 82 insertions(+), 66 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/40/73040/1 diff --git a/dummyflasher.c b/dummyflasher.c index 2bdd10c..cafcfde 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -211,7 +211,7 @@ static void dummy_chip_writel(const struct flashctx *flash, uint32_t val, chipaddr addr) { - msg_pspew("%s: addr=0x%" PRIxPTR ", val=0x%08x\n", __func__, addr, val); + msg_pspew("%s: addr=0x%" PRIxPTR ", val=0x%08"PRIx32"\n", __func__, addr, val); } static void dummy_chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len) diff --git a/erasure_layout.c b/erasure_layout.c index 05376de..675af1c 100644 --- a/erasure_layout.c +++ b/erasure_layout.c @@ -314,7 +314,7 @@ if (region.write_prot) { msg_gdbg("%s: cannot erase inside %s " - "region (%#08x..%#08x), skipping range (%#08x..%#08x).\n", + "region (%#08"PRIx32"..%#08"PRIx32"), skipping range (%#08x..%#08x).\n", __func__, region.name, region.start, region.end - 1, addr, addr + len - 1); @@ -322,7 +322,7 @@ continue; } - msg_gdbg("%s: %s region (%#08x..%#08x) is " + msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is " "writable, erasing range (%#08x..%#08x).\n", __func__, region.name, region.start, region.end - 1, @@ -340,7 +340,7 @@ ret = erasefn(flashctx, start_addr, block_len); if (ret) { msg_cerr("Failed to execute erase command " - "for offset %#x to %#x.\n", + "for offset %#"PRIx32" to %#"PRIx32".\n", start_addr, start_addr + block_len); ret = -1; goto _end; @@ -350,11 +350,11 @@ memset(curcontents+start_addr, erased_value, block_len); // after erase make it unselected again erase_layout[i].layout_list[j].selected = false; - msg_cdbg("E(%x:%x)", start_addr, start_addr + block_len - 1); + msg_cdbg("E(%"PRIx32":%"PRIx32")", start_addr, start_addr + block_len - 1); // verify erase ret = check_erased_range(flashctx, start_addr, block_len); if (ret) { - msg_cerr("Verifying flash. Erase failed for range %#x : %#x, Abort.\n", + msg_cerr("Verifying flash. Erase failed for range %#"PRIx32" : %#"PRIx32", Abort.\n", start_addr, start_addr + block_len - 1); goto _end; } @@ -382,14 +382,14 @@ // adjust curcontents memcpy(curcontents + region_start + start_here, newcontents + region_start + start_here, len_here); - msg_cdbg("W(%x:%x)", region_start + start_here, region_start + start_here + len_here - 1); + msg_cdbg("W(%"PRIx32":%"PRIx32")", region_start + start_here, region_start + start_here + len_here - 1); *all_skipped = false; } // verify write ret = verify_range(flashctx, newcontents + region_start, region_start, region_end - region_start); if (ret) { - msg_cerr("Verifying flash. Write failed for range %#x : %#x, Abort.\n", + msg_cerr("Verifying flash. Write failed for range %#"PRIx32" : %#"PRIx32", Abort.\n", region_start, region_end); goto _end; } @@ -401,6 +401,6 @@ free(old_start_buf); free(old_end_buf); - msg_cinfo("Erase/write done from %x to %x\n", region_start, region_end); + msg_cinfo("Erase/write done from %"PRIx32" to %"PRIx32"\n", region_start, region_end); return ret; } diff --git a/flashrom.c b/flashrom.c index d17e04b..6023065 100644 --- a/flashrom.c +++ b/flashrom.c @@ -384,7 +384,7 @@ get_flash_region(flash, addr, &region); if (region.write_prot) { - msg_gerr("%s: cannot write/erase inside %s region (%#08x..%#08x).\n", + msg_gerr("%s: cannot write/erase inside %s region (%#08"PRIx32"..%#08"PRIx32").\n", __func__, region.name, region.start, region.end - 1); free(region.name); return -1; @@ -580,7 +580,7 @@ if (region.read_prot) { if (flash->flags.skip_unreadable_regions) { - msg_gdbg("%s: cannot read inside %s region (%#08x..%#08x), " + msg_gdbg("%s: cannot read inside %s region (%#08"PRIx32"..%#08"PRIx32"), " "filling (%#08x..%#08x) with erased value instead.\n", __func__, region.name, region.start, region.end - 1, addr, addr + read_len - 1); @@ -590,12 +590,12 @@ continue; } - msg_gerr("%s: cannot read inside %s region (%#08x..%#08x).\n", + msg_gerr("%s: cannot read inside %s region (%#08"PRIx32"..%#08"PRIx32").\n", __func__, region.name, region.start, region.end - 1); free(region.name); return -1; } - msg_gdbg("%s: %s region (%#08x..%#08x) is readable, reading range (%#08x..%#08x).\n", + msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is readable, reading range (%#08x..%#08x).\n", __func__, region.name, region.start, region.end - 1, addr, addr + read_len - 1); free(region.name); @@ -648,20 +648,20 @@ if ((region.write_prot && flash->flags.skip_unwritable_regions) || (region.read_prot && flash->flags.skip_unreadable_regions)) { - msg_gdbg("%s: Skipping verification of %s region (%#08x..%#08x)\n", + msg_gdbg("%s: Skipping verification of %s region (%#08"PRIx32"..%#08"PRIx32")\n", __func__, region.name, region.start, region.end - 1); free(region.name); continue; } if (region.read_prot) { - msg_gerr("%s: Verification imposible because %s region (%#08x..%#08x) is unreadable.\n", + msg_gerr("%s: Verification imposible because %s region (%#08"PRIx32"..%#08"PRIx32") is unreadable.\n", __func__, region.name, region.start, region.end - 1); free(region.name); goto out_free; } - msg_gdbg("%s: Verifying %s region (%#08x..%#08x)\n", + msg_gdbg("%s: Verifying %s region (%#08"PRIx32"..%#08"PRIx32")\n", __func__, region.name, region.start, region.end - 1); free(region.name); @@ -1016,13 +1016,13 @@ const uint8_t *rbuf = buf + addr - start; if (region.write_prot) { - msg_gdbg("%s: cannot write inside %s region (%#08x..%#08x), skipping (%#08x..%#08x).\n", + msg_gdbg("%s: cannot write inside %s region (%#08"PRIx32"..%#08"PRIx32"), skipping (%#08x..%#08x).\n", __func__, region.name, region.start, region.end - 1, addr, addr + write_len - 1); free(region.name); continue; } - msg_gdbg("%s: %s region (%#08x..%#08x) is writable, writing range (%#08x..%#08x).\n", + msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is writable, writing range (%#08x..%#08x).\n", __func__, region.name, region.start, region.end - 1, addr, addr + write_len - 1); write_func_t *write_func = lookup_write_func_ptr(flash->chip); @@ -1347,7 +1347,7 @@ first = false; else msg_cdbg(", "); - msg_cdbg("0x%06x-0x%06x:", info->erase_start, info->erase_end); + 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); @@ -1479,13 +1479,13 @@ len = min(info->erase_start + erase_len, region.end) - addr; if (region.write_prot) { - msg_gdbg("%s: cannot erase inside %s region (%#08x..%#08x), skipping range (%#08x..%#08x).\n", + msg_gdbg("%s: cannot erase inside %s region (%#08"PRIx32"..%#08"PRIx32"), skipping range (%#08x..%#08x).\n", __func__, region.name, region.start, region.end - 1, addr, addr + len - 1); free(region.name); continue; } - msg_gdbg("%s: %s region (%#08x..%#08x) is writable, erasing range (%#08x..%#08x).\n", + msg_gdbg("%s: %s region (%#08"PRIx32"..%#08"PRIx32") is writable, erasing range (%#08x..%#08x).\n", __func__, region.name, region.start, region.end - 1, addr, addr + len - 1); free(region.name); diff --git a/ich_descriptors.c b/ich_descriptors.c index b37ce6d..a2a99bd 100644 --- a/ich_descriptors.c +++ b/ich_descriptors.c @@ -94,13 +94,13 @@ void prettyprint_ich_reg_vscc(uint32_t reg_val, int verbosity, bool print_vcl) { - print(verbosity, "BES=0x%x, ", (reg_val & VSCC_BES) >> VSCC_BES_OFF); - print(verbosity, "WG=%d, ", (reg_val & VSCC_WG) >> VSCC_WG_OFF); - print(verbosity, "WSR=%d, ", (reg_val & VSCC_WSR) >> VSCC_WSR_OFF); - print(verbosity, "WEWS=%d, ", (reg_val & VSCC_WEWS) >> VSCC_WEWS_OFF); - print(verbosity, "EO=0x%x", (reg_val & VSCC_EO) >> VSCC_EO_OFF); + print(verbosity, "BES=0x%"PRIx32", ", (reg_val & VSCC_BES) >> VSCC_BES_OFF); + print(verbosity, "WG=%"PRId32", ", (reg_val & VSCC_WG) >> VSCC_WG_OFF); + print(verbosity, "WSR=%"PRId32", ", (reg_val & VSCC_WSR) >> VSCC_WSR_OFF); + print(verbosity, "WEWS=%"PRId32", ", (reg_val & VSCC_WEWS) >> VSCC_WEWS_OFF); + print(verbosity, "EO=0x%"PRIx32"", (reg_val & VSCC_EO) >> VSCC_EO_OFF); if (print_vcl) - print(verbosity, ", VCL=%d", (reg_val & VSCC_VCL) >> VSCC_VCL_OFF); + print(verbosity, ", VCL=%"PRId32"", (reg_val & VSCC_VCL) >> VSCC_VCL_OFF); print(verbosity, "\n"); } @@ -145,23 +145,23 @@ void prettyprint_ich_descriptor_content(enum ich_chipset cs, const struct ich_desc_content *cont) { msg_pdbg2("=== Content Section ===\n"); - msg_pdbg2("FLVALSIG 0x%08x\n", cont->FLVALSIG); - msg_pdbg2("FLMAP0 0x%08x\n", cont->FLMAP0); - msg_pdbg2("FLMAP1 0x%08x\n", cont->FLMAP1); - msg_pdbg2("FLMAP2 0x%08x\n", cont->FLMAP2); + msg_pdbg2("FLVALSIG 0x%08"PRIx32"\n", cont->FLVALSIG); + msg_pdbg2("FLMAP0 0x%08"PRIx32"\n", cont->FLMAP0); + msg_pdbg2("FLMAP1 0x%08"PRIx32"\n", cont->FLMAP1); + msg_pdbg2("FLMAP2 0x%08"PRIx32"\n", cont->FLMAP2); msg_pdbg2("\n"); msg_pdbg2("--- Details ---\n"); msg_pdbg2("NR (Number of Regions): %5zd\n", ich_number_of_regions(cs, cont)); - msg_pdbg2("FRBA (Flash Region Base Address): 0x%03x\n", getFRBA(cont)); + msg_pdbg2("FRBA (Flash Region Base Address): 0x%03"PRIx32"\n", getFRBA(cont)); msg_pdbg2("NC (Number of Components): %5d\n", cont->NC + 1); - msg_pdbg2("FCBA (Flash Component Base Address): 0x%03x\n", getFCBA(cont)); + msg_pdbg2("FCBA (Flash Component Base Address): 0x%03"PRIx32"\n", getFCBA(cont)); msg_pdbg2("ISL (ICH/PCH/SoC Strap Length): %5d\n", cont->ISL); - msg_pdbg2("FISBA/FPSBA (Flash ICH/PCH/SoC Strap Base Addr): 0x%03x\n", getFISBA(cont)); + msg_pdbg2("FISBA/FPSBA (Flash ICH/PCH/SoC Strap Base Addr): 0x%03"PRIx32"\n", getFISBA(cont)); msg_pdbg2("NM (Number of Masters): %5zd\n", ich_number_of_masters(cs, cont)); - msg_pdbg2("FMBA (Flash Master Base Address): 0x%03x\n", getFMBA(cont)); + msg_pdbg2("FMBA (Flash Master Base Address): 0x%03"PRIx32"\n", getFMBA(cont)); msg_pdbg2("MSL/PSL (MCH/PROC Strap Length): %5d\n", cont->MSL); - msg_pdbg2("FMSBA (Flash MCH/PROC Strap Base Address): 0x%03x\n", getFMSBA(cont)); + msg_pdbg2("FMSBA (Flash MCH/PROC Strap Base Address): 0x%03"PRIx32"\n", getFMSBA(cont)); msg_pdbg2("\n"); } @@ -374,10 +374,10 @@ } msg_pdbg2("=== Component Section ===\n"); - msg_pdbg2("FLCOMP 0x%08x\n", desc->component.FLCOMP); - msg_pdbg2("FLILL 0x%08x\n", desc->component.FLILL ); + msg_pdbg2("FLCOMP 0x%08"PRIx32"\n", desc->component.FLCOMP); + msg_pdbg2("FLILL 0x%08"PRIx32"\n", desc->component.FLILL ); if (has_flill1) - msg_pdbg2("FLILL1 0x%08x\n", desc->component.FLILL1); + msg_pdbg2("FLILL1 0x%08"PRIx32"\n", desc->component.FLILL1); msg_pdbg2("\n"); msg_pdbg2("--- Details ---\n"); @@ -442,11 +442,11 @@ } uint32_t base = ICH_FREG_BASE(reg->FLREGs[i]); uint32_t limit = ICH_FREG_LIMIT(reg->FLREGs[i]); - msg_pdbg2("Region %d (%-7s) ", i, region_names[i]); + msg_pdbg2("Region %"PRId32" (%-7s) ", i, region_names[i]); if (base > limit) msg_pdbg2("is unused.\n"); else - msg_pdbg2("0x%08x - 0x%08x\n", base, limit); + msg_pdbg2("0x%08"PRIx32" - 0x%08"PRIx32"\n", base, limit); } void prettyprint_ich_descriptor_region(const enum ich_chipset cs, const struct ich_descriptors *const desc) @@ -460,7 +460,7 @@ return; } for (i = 0; i < nr; i++) - msg_pdbg2("FLREG%zd 0x%08x\n", i, desc->region.FLREGs[i]); + msg_pdbg2("FLREG%zd 0x%08"PRIx32"\n", i, desc->region.FLREGs[i]); msg_pdbg2("\n"); msg_pdbg2("--- Details ---\n"); @@ -480,7 +480,7 @@ return; } for (i = 0; i < nm; i++) - msg_pdbg2("FLMSTR%zd 0x%08x\n", i + 1, desc->master.FLMSTRs[i]); + msg_pdbg2("FLMSTR%zd 0x%08"PRIx32"\n", i + 1, desc->master.FLMSTRs[i]); msg_pdbg2("\n"); msg_pdbg2("--- Details ---\n"); @@ -912,7 +912,7 @@ msg_pdbg2("--- North/MCH/PROC (%d entries) ---\n", max_count); for (i = 0; i < max_count; i++) - msg_pdbg2("STRP%-2d = 0x%08x\n", i, desc->north.STRPs[i]); + msg_pdbg2("STRP%-2d = 0x%08"PRIx32"\n", i, desc->north.STRPs[i]); msg_pdbg2("\n"); max_count = MIN(ARRAY_SIZE(desc->south.STRPs), desc->content.ISL); @@ -924,7 +924,7 @@ msg_pdbg2("--- South/ICH/PCH (%d entries) ---\n", max_count); for (i = 0; i < max_count; i++) - msg_pdbg2("STRP%-2d = 0x%08x\n", i, desc->south.STRPs[i]); + msg_pdbg2("STRP%-2d = 0x%08"PRIx32"\n", i, desc->south.STRPs[i]); msg_pdbg2("\n"); switch (cs) { @@ -970,20 +970,20 @@ { int i; msg_pdbg2("=== Upper Map Section ===\n"); - msg_pdbg2("FLUMAP1 0x%08x\n", umap->FLUMAP1); + msg_pdbg2("FLUMAP1 0x%08"PRIx32"\n", umap->FLUMAP1); msg_pdbg2("\n"); msg_pdbg2("--- Details ---\n"); msg_pdbg2("VTL (length in DWORDS) = %d\n", umap->VTL); - msg_pdbg2("VTBA (base address) = 0x%6.6x\n", getVTBA(umap)); + msg_pdbg2("VTBA (base address) = 0x%6.6"PRIx32"\n", getVTBA(umap)); msg_pdbg2("\n"); msg_pdbg2("VSCC Table: %d entries\n", umap->VTL/2); for (i = 0; i < umap->VTL/2; i++) { uint32_t jid = umap->vscc_table[i].JID; uint32_t vscc = umap->vscc_table[i].VSCC; - msg_pdbg2(" JID%d = 0x%08x\n", i, jid); - msg_pdbg2(" VSCC%d = 0x%08x\n", i, vscc); + msg_pdbg2(" JID%d = 0x%08"PRIx32"\n", i, jid); + msg_pdbg2(" VSCC%d = 0x%08"PRIx32"\n", i, vscc); msg_pdbg2(" "); /* indentation */ prettyprint_rdid(jid); msg_pdbg2(" "); /* indentation */ diff --git a/jedec.c b/jedec.c index e97468e..0890b95 100644 --- a/jedec.c +++ b/jedec.c @@ -143,7 +143,7 @@ /* Issue JEDEC Product ID Exit command */ chip_writeb(flash, 0xF0, bios + (0x5555 & mask)); - msg_cdbg("%s: man_id 0x%02x, dev_id 0x%06x", __func__, man_id, dev_id); + msg_cdbg("%s: man_id 0x%02"PRIx32", dev_id 0x%06"PRIx32"", __func__, man_id, dev_id); if (!oddparity(man_id)) msg_cdbg(", man_id parity violation"); @@ -250,7 +250,7 @@ if (probe_timing_exit) programmer_delay(flash, probe_timing_exit); - msg_cdbg("%s: id1 0x%02x, id2 0x%02x", __func__, largeid1, largeid2); + msg_cdbg("%s: id1 0x%02"PRIx32", id2 0x%02"PRIx32"", __func__, largeid1, largeid2); if (!oddparity(id1)) msg_cdbg(", id1 parity violation"); diff --git a/layout.c b/layout.c index 4f02be6..4be965e 100644 --- a/layout.c +++ b/layout.c @@ -292,7 +292,7 @@ if (lhsr->end < rhsr->start) continue; - msg_gwarn("Regions %s [0x%08x-0x%08x] and %s [0x%08x-0x%08x] overlap\n", + msg_gwarn("Regions %s [0x%08"PRIx32"-0x%08"PRIx32"] and %s [0x%08"PRIx32"-0x%08"PRIx32"] overlap\n", lhsr->name, lhsr->start, lhsr->end, rhsr->name, rhsr->start, rhsr->end); overlap_detected = 1; } diff --git a/s25f.c b/s25f.c index 549883b..dd15efc 100644 --- a/s25f.c +++ b/s25f.c @@ -184,7 +184,7 @@ int result = spi_send_command(flash, sizeof(read_cr_cmd), 1, read_cr_cmd, &cfg); if (result) { - msg_cerr("%s failed during command execution at address 0x%x\n", + msg_cerr("%s failed during command execution at address 0x%"PRIx32"\n", __func__, addr); return -1; } @@ -222,7 +222,7 @@ int result = spi_send_multicommand(flash, cmds); if (result) { - msg_cerr("%s failed during command execution at address 0x%x\n", + msg_cerr("%s failed during command execution at address 0x%"PRIx32"\n", __func__, addr); return -1; } diff --git a/sfdp.c b/sfdp.c index 5eddb21..8683779 100644 --- a/sfdp.c +++ b/sfdp.c @@ -35,7 +35,7 @@ */ 0 }; - msg_cspew("%s: addr=0x%x, len=%d, data:\n", __func__, address, len); + msg_cspew("%s: addr=0x%"PRIx32", len=%d, data:\n", __func__, address, len); newbuf = malloc(len + 1); if (!newbuf) return SPI_PROGRAMMER_ERROR; @@ -96,7 +96,7 @@ if (eraser->eraseblocks[0].size == block_size && eraser->block_erase == erasefn) { msg_cdbg2(" Tried to add a duplicate block eraser: " - "%d x %d B with opcode 0x%02x.\n", + "%"PRId32" x %"PRId32" B with opcode 0x%02x.\n", total_size/block_size, block_size, opcode); return 1; } @@ -110,7 +110,7 @@ eraser->block_erase = erasefn; eraser->eraseblocks[0].size = block_size; eraser->eraseblocks[0].count = total_size/block_size; - msg_cdbg2(" Block eraser %d: %d x %d B with opcode " + msg_cdbg2(" Block eraser %d: %"PRId32" x %"PRId32" B with opcode " "0x%02x\n", i, total_size/block_size, block_size, opcode); return 0; @@ -274,7 +274,7 @@ tmp32 |= ((unsigned int)buf[3]) << 24; if (tmp32 != 0x50444653) { - msg_cdbg2("Signature = 0x%08x (should be 0x50444653)\n", tmp32); + msg_cdbg2("Signature = 0x%08"PRIx32" (should be 0x50444653)\n", tmp32); msg_cdbg("No SFDP signature found.\n"); return 0; } @@ -320,7 +320,7 @@ hdrs[i].v_major, hdrs[i].v_minor); len = hdrs[i].len * 4; tmp32 = hdrs[i].ptp; - msg_cdbg2(" Length %d B, Parameter Table Pointer 0x%06x\n", + msg_cdbg2(" Length %d B, Parameter Table Pointer 0x%06"PRIx32"\n", len, tmp32); if (tmp32 + len >= (1 << 24)) { @@ -345,7 +345,7 @@ msg_cspew(" Parameter table contents:\n"); for (tmp32 = 0; tmp32 < len; tmp32++) { if ((tmp32 % 8) == 0) { - msg_cspew(" 0x%04x: ", tmp32); + msg_cspew(" 0x%04"PRIx32": ", tmp32); } msg_cspew(" %02x", tbuf[tmp32]); if ((tmp32 % 8) == 7) { diff --git a/spi25.c b/spi25.c index f54e4c8..6a6ee75 100644 --- a/spi25.c +++ b/spi25.c @@ -141,7 +141,7 @@ { const struct flashchip *chip = flash->chip; - msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2); + msg_cdbg("%s: id1 0x%02"PRIx32", id2 0x%02"PRIx32"\n", __func__, id1, id2); if (id1 == chip->manufacture_id && id2 == chip->model_id) return 1; @@ -232,7 +232,7 @@ id2 = readarr[0]; - msg_cdbg("%s: id 0x%x\n", __func__, id2); + msg_cdbg("%s: id 0x%"PRIx32"\n", __func__, id2); if (id2 != flash->chip->model_id) return 0; @@ -252,7 +252,7 @@ id1 = id_cache[RES2].bytes[0]; id2 = id_cache[RES2].bytes[1]; - msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2); + msg_cdbg("%s: id1 0x%"PRIx32", id2 0x%"PRIx32"\n", __func__, id1, id2); if (id1 != flash->chip->manufacture_id || id2 != flash->chip->model_id) return 0; @@ -272,7 +272,7 @@ id1 = (id_cache[RES3].bytes[0] << 8) | id_cache[RES3].bytes[1]; id2 = id_cache[RES3].bytes[3]; - msg_cdbg("%s: id1 0x%x, id2 0x%x\n", __func__, id1, id2); + msg_cdbg("%s: id1 0x%"PRIx32", id2 0x%"PRIx32"\n", __func__, id1, id2); if (id1 != flash->chip->manufacture_id || id2 != flash->chip->model_id) return 0; @@ -294,7 +294,7 @@ id1 = readarr[0]; id2 = readarr[1]; - msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2); + msg_cdbg("%s: id1 0x%02"PRIx32", id2 0x%02"PRIx32"\n", __func__, id1, id2); if (id1 == flash->chip->manufacture_id && id2 == flash->chip->model_id) return 1; diff --git a/spi95.c b/spi95.c index 976f99a..d1f0ba1 100644 --- a/spi95.c +++ b/spi95.c @@ -46,7 +46,7 @@ id1 = readarr[0]; // manufacture id id2 = (readarr[1] << 8) | readarr[2]; // SPI family code + model id - msg_cdbg("%s: id1 0x%02x, id2 0x%02x\n", __func__, id1, id2); + msg_cdbg("%s: id1 0x%02"PRIx32", id2 0x%02"PRIx32"\n", __func__, id1, id2); if (id1 == flash->chip->manufacture_id && id2 == flash->chip->model_id) return 1; -- To view, visit https://review.coreboot.org/c/flashrom/+/73040 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia75b6df981ce60c891161fe553c7ceab8570178d Gerrit-Change-Number: 73040 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: meson: use cfg_data for to evaluate cflags
by Thomas Heijligen (Code Review) 14 Feb '23

14 Feb '23

Attention is currently required from: Edward O'Callaghan. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72656 ) Change subject: meson: use cfg_data for to evaluate cflags ...................................................................... Patch Set 1: (1 comment) File meson.build: https://review.coreboot.org/c/flashrom/+/72656/comment/d7d2df39_9e7cd33a PS1, Line 481: : config.set('CONFIG_DEFAULT_PROGRAMMER_NAME', get_option('default_programmer_name') != '' ? '&programmer_' + get_option('default_programmer_name') : 'NULL') > not sure I like the embedded ternary operator use in this case, could it be assigned to a intermedia […] You know, I would like to remove the `CONFIG_DEFAULT_PROGRAMMER`. It accesses parts which should not be accessible by the cli and also alters the default behavior based on compile time decisions. In an automated job the programmer should given in the same way as the operation to perform. I don't hang on the syntax and can change it. -- To view, visit https://review.coreboot.org/c/flashrom/+/72656 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie91b4e1377b9d6d347511d4727c0a26a314b1e57 Gerrit-Change-Number: 72656 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Tue, 14 Feb 2023 12:03:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: internal.c: Factor out laptop alerts into helper func
by Edward O'Callaghan (Code Review) 14 Feb '23

14 Feb '23

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/71578 ) Change subject: internal.c: Factor out laptop alerts into helper func ...................................................................... internal.c: Factor out laptop alerts into helper func Minor however a unfortunate '_' suffix is temporarily needed to skirt around global variable shadowing. Change-Id: I8eea91012e6539b4fdf5d49a75a9cb48bb8a57ca Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/71578 Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Alexander Goncharov <chat(a)joursoir.net> Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Reviewed-by: Thomas Heijligen <src(a)posteo.de> --- M internal.c 1 file changed, 49 insertions(+), 25 deletions(-) Approvals: build bot (Jenkins): Verified Thomas Heijligen: Looks good to me, approved Angel Pons: Looks good to me, approved Alexander Goncharov: Looks good to me, but someone else must approve diff --git a/internal.c b/internal.c index c5fdb74..07126bf 100644 --- a/internal.c +++ b/internal.c @@ -162,6 +162,36 @@ return 0; } +// FIXME: remove '_' suffix from parameters once global shadowing is fixed. +static void report_nonwl_laptop_detected(int is_laptop_, bool laptop_ok_) +{ + if (is_laptop_ && !laptop_ok_) { + msg_pinfo("========================================================================\n"); + if (is_laptop_ == 1) { + msg_pinfo("You seem to be running flashrom on an unknown laptop. Some\n" + "internal buses have been disabled for safety reasons.\n\n"); + } else { + msg_pinfo("You may be running flashrom on an unknown laptop. We could not\n" + "detect this for sure because your vendor has not set up the SMBIOS\n" + "tables correctly. Some internal buses have been disabled for\n" + "safety reasons. You can enforce using all buses by adding\n" + " -p internal:laptop=this_is_not_a_laptop\n" + "to the command line, but please read the following warning if you\n" + "are not sure.\n\n"); + } + msg_perr("Laptops, notebooks and netbooks are difficult to support and we\n" + "recommend to use the vendor flashing utility. The embedded controller\n" + "(EC) in these machines often interacts badly with flashing.\n" + "See the manpage and https://flashrom.org/Laptops for details.\n\n" + "If flash is shared with the EC, erase is guaranteed to brick your laptop\n" + "and write may brick your laptop.\n" + "Read and probe may irritate your EC and cause fan failure, backlight\n" + "failure and sudden poweroff.\n" + "You have been warned.\n" + "========================================================================\n"); + } +} + static int internal_init(const struct programmer_cfg *cfg) { int ret = 0; @@ -283,31 +313,7 @@ register_par_master(&par_master_internal, internal_buses_supported, NULL); /* Report if a non-whitelisted laptop is detected that likely uses a legacy bus. */ - if (is_laptop && !laptop_ok) { - msg_pinfo("========================================================================\n"); - if (is_laptop == 1) { - msg_pinfo("You seem to be running flashrom on an unknown laptop. Some\n" - "internal buses have been disabled for safety reasons.\n\n"); - } else { - msg_pinfo("You may be running flashrom on an unknown laptop. We could not\n" - "detect this for sure because your vendor has not set up the SMBIOS\n" - "tables correctly. Some internal buses have been disabled for\n" - "safety reasons. You can enforce using all buses by adding\n" - " -p internal:laptop=this_is_not_a_laptop\n" - "to the command line, but please read the following warning if you\n" - "are not sure.\n\n"); - } - msg_perr("Laptops, notebooks and netbooks are difficult to support and we\n" - "recommend to use the vendor flashing utility. The embedded controller\n" - "(EC) in these machines often interacts badly with flashing.\n" - "See the manpage and https://flashrom.org/Laptops for details.\n\n" - "If flash is shared with the EC, erase is guaranteed to brick your laptop\n" - "and write may brick your laptop.\n" - "Read and probe may irritate your EC and cause fan failure, backlight\n" - "failure and sudden poweroff.\n" - "You have been warned.\n" - "========================================================================\n"); - } + report_nonwl_laptop_detected(is_laptop, laptop_ok); ret = 0; -- To view, visit https://review.coreboot.org/c/flashrom/+/71578 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8eea91012e6539b4fdf5d49a75a9cb48bb8a57ca Gerrit-Change-Number: 71578 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> 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: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> 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[master]: meson: use cfg_data for to evaluate cflags
by Edward O'Callaghan (Code Review) 14 Feb '23

14 Feb '23

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72656 ) Change subject: meson: use cfg_data for to evaluate cflags ...................................................................... Patch Set 1: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/72656/comment/da765064_703cab26 PS1, Line 7: use cfg_data for to evaluate cflags there is a bit more going on here like the version switching from the meson project to something decent so maybe just elaborate a bit. File meson.build: https://review.coreboot.org/c/flashrom/+/72656/comment/b9b6137e_269b66bc PS1, Line 481: : config.set('CONFIG_DEFAULT_PROGRAMMER_NAME', get_option('default_programmer_name') != '' ? '&programmer_' + get_option('default_programmer_name') : 'NULL') not sure I like the embedded ternary operator use in this case, could it be assigned to a intermediate for clarity? But whatever, it isn't a big deal. -- To view, visit https://review.coreboot.org/c/flashrom/+/72656 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie91b4e1377b9d6d347511d4727c0a26a314b1e57 Gerrit-Change-Number: 72656 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Tue, 14 Feb 2023 11:01:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: meson: use cfg_data for to evaluate cflags
by Edward O'Callaghan (Code Review) 14 Feb '23

14 Feb '23

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72656 ) Change subject: meson: use cfg_data for to evaluate cflags ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/72656 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie91b4e1377b9d6d347511d4727c0a26a314b1e57 Gerrit-Change-Number: 72656 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Tue, 14 Feb 2023 10:58:37 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: Add support for VIA VL805 USB 3 XHCI flashing
by Thomas Heijligen (Code Review) 14 Feb '23

14 Feb '23

Attention is currently required from: ChrisEric1 CECL, Nicholas Chin. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72057 ) Change subject: Add support for VIA VL805 USB 3 XHCI flashing ...................................................................... Patch Set 11: (1 comment) File vl805.c: https://review.coreboot.org/c/flashrom/+/72057/comment/fc88988c_7078d5c6 PS3, Line 38: static struct pci_dev *dev = NULL; > Here is my broken progress: https://gist.githubusercontent. […] Yes, you have to define an custom data struct which is passed along. Have a look at this patch https://review.coreboot.org/c/flashrom/+/73037/3/asm106x.c In line 30 is a data structure defined. This get initialized in the init function from line 138ff, transfered to the allocated data structure and then registered in line 154 together with the spi_master. The `...spi_send_command` function has access to this data. There you've done it almost right. The data gets then passed on to all internal functions of the programmer. `asm106x_wait_ready()` in case of the link above and the `vl805_setregval()` etc functions in your case. So they will be extended to take a `struct pci_dev*` as argument instead of using the global. It will become `static void vl805_setregval(const struct pci_dev *dev, int reg, uint32_t val)` I hope this helps you 😊 -- To view, visit https://review.coreboot.org/c/flashrom/+/72057 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38 Gerrit-Change-Number: 72057 Gerrit-PatchSet: 11 Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: ChrisEric1 CECL <christopherericlentocha(a)gmail.com> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Tue, 14 Feb 2023 10:39:54 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: ChrisEric1 CECL <christopherericlentocha(a)gmail.com> Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: asm106x: add programmer for ASM106x SATA controllers
by Thomas Heijligen (Code Review) 14 Feb '23

14 Feb '23

Attention is currently required from: Felix Singer, Alex Badea. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73037 ) Change subject: asm106x: add programmer for ASM106x SATA controllers ...................................................................... Patch Set 3: (7 comments) Patchset: PS3: Hi Alex, thank you for the contribution! The code looks already quite good. Anyway I have a questions and some minor annotations. On which device have you tested this driver? Can you mention it in the commit message. File asm106x.c: https://review.coreboot.org/c/flashrom/+/73037/comment/ba48e766_d6e120a3 PS3, Line 35: {PCI_VENDOR_ID_ASMEDIA, 0x0612, OK, "ASMedia", "ASM106x"}, This PCI-Id is assigned to the ASM1062 Serial ATA Controller, the PCI-Id 0x625 is assigned to '106x SATA/RAID Controller'. Beside that there is a PCI-Id 0x0611 for 'ASM1061 SATA IDE Controller'. For which one is this driver? Or does it work on all of them? PCI-Ids from pciutils (https://pci-ids.ucw.cz/v2.2/pci.ids) https://review.coreboot.org/c/flashrom/+/73037/comment/9db02c32_d99bd84a PS3, Line 51: programmer_delay(flash, 10); `programmer_delay()` should be used outside of the programmer. It is there to determine if an programmer specific delay function should be called or the default one. Here you can use `void default_delay(unsigned int usec)`. With it you can also pass only the `struct pci_dev *pci` to this function. ``` static int asm106x_wait_ready(const struct pci_dev *pci, uint8_t *pval) ``` https://review.coreboot.org/c/flashrom/+/73037/comment/b1ebaf4f_82ac445b PS3, Line 59: if (pval) You could move this 'if, assignment, return' where the break is. Then everything below the loop is a timeout. https://review.coreboot.org/c/flashrom/+/73037/comment/5522f531_3ac722ae PS3, Line 74: writecnt, readcnt); This can be moved in the line above. https://review.coreboot.org/c/flashrom/+/73037/comment/6156e402_58c4eb14 PS3, Line 115: ctrl | ASM106X_CTRL_CSN); This function call can be in one line File meson.build: https://review.coreboot.org/c/flashrom/+/73037/comment/7143d1a9_afdb5109 PS3, Line 152: 'cpu_families' : cpus_port_io, This line should be removed. The driver uses only PCI register to communicate with the flash, not I/O ports. -- To view, visit https://review.coreboot.org/c/flashrom/+/73037 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I591b117be911bdb8249247c20530c1cf70f6e70d Gerrit-Change-Number: 73037 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Badea <vamposdecampos(a)gmail.com> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Alex Badea <vamposdecampos(a)gmail.com> Gerrit-Comment-Date: Tue, 14 Feb 2023 10:20:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit February 2023