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

Change in flashrom[master]: Add initial J-Link SPI programmer
by Marc Schink (Code Review) 21 Aug '18

21 Aug '18

Hello Paul Menzel, build bot (Jenkins), Nico Huber, I'd like you to reexamine a change. Please visit https://review.coreboot.org/28087 to look at the new patch set (#5). Change subject: Add initial J-Link SPI programmer ...................................................................... Add initial J-Link SPI programmer Tested with SEGGER J-Link EDU, Flasher ARM and flash chip W25Q16.V. Change-Id: Ie03a054a75457ec9e1cab36ea124bb53b10e8d7e Signed-off-by: Marc Schink <flashrom-dev(a)marcschink.de> --- M Makefile M README M flashrom.8.tmpl M flashrom.c A jlink_spi.c M programmer.h 6 files changed, 592 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/87/28087/5 -- To view, visit https://review.coreboot.org/28087 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: Ie03a054a75457ec9e1cab36ea124bb53b10e8d7e Gerrit-Change-Number: 28087 Gerrit-PatchSet: 5 Gerrit-Owner: Marc Schink <flashrom-dev(a)marcschink.de> Gerrit-Reviewer: Marc Schink <flashrom-dev(a)marcschink.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>

1 0

Change in flashrom[master]: chipset_enable.c: Mark Intel HM55 as DEP
by Nico Huber (Code Review) 21 Aug '18

21 Aug '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/28253 ) Change subject: chipset_enable.c: Mark Intel HM55 as DEP ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/28253 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaedd5bdc34dfff9b8588a3f4e1ad46460077fdf9 Gerrit-Change-Number: 28253 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 21 Aug 2018 17:57:17 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: chipset_enable.c: Mark Intel HM55 as DEP
by Paul Menzel (Code Review) 21 Aug '18

21 Aug '18

Paul Menzel has posted comments on this change. ( https://review.coreboot.org/28253 ) Change subject: chipset_enable.c: Mark Intel HM55 as DEP ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/28253 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Iaedd5bdc34dfff9b8588a3f4e1ad46460077fdf9 Gerrit-Change-Number: 28253 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 21 Aug 2018 16:59:58 +0000 Gerrit-HasComments: No Gerrit-HasLabels: Yes

1 0

Change in flashrom[master]: chipset_enable.c: Mark Intel HM55 as DEP
by Angel Pons (Code Review) 21 Aug '18

21 Aug '18

Angel Pons has uploaded this change for review. ( https://review.coreboot.org/28253 Change subject: chipset_enable.c: Mark Intel HM55 as DEP ...................................................................... chipset_enable.c: Mark Intel HM55 as DEP Tested reading, writing and erasing the internal flash chip using an HP 630 laptop with an Intel HM55. However, since all ME-enabled chipsets are marked as DEP instead of OK, this one shall follow suit as well. Change-Id: Iaedd5bdc34dfff9b8588a3f4e1ad46460077fdf9 Signed-off-by: Angel Pons <th3fanbus(a)gmail.com> --- M chipset_enable.c 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/53/28253/1 diff --git a/chipset_enable.c b/chipset_enable.c index 42f6728..566b1fb 100644 --- a/chipset_enable.c +++ b/chipset_enable.c @@ -1805,7 +1805,7 @@ {0x8086, 0x3b06, DEP, "Intel", "H55", enable_flash_pch5}, {0x8086, 0x3b07, DEP, "Intel", "QM57", enable_flash_pch5}, {0x8086, 0x3b08, NT, "Intel", "H57", enable_flash_pch5}, - {0x8086, 0x3b09, NT, "Intel", "HM55", enable_flash_pch5}, + {0x8086, 0x3b09, DEP, "Intel", "HM55", enable_flash_pch5}, {0x8086, 0x3b0a, NT, "Intel", "Q57", enable_flash_pch5}, {0x8086, 0x3b0b, NT, "Intel", "HM57", enable_flash_pch5}, {0x8086, 0x3b0d, NT, "Intel", "3400 Mobile SFF", enable_flash_pch5}, -- To view, visit https://review.coreboot.org/28253 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: Iaedd5bdc34dfff9b8588a3f4e1ad46460077fdf9 Gerrit-Change-Number: 28253 Gerrit-PatchSet: 1 Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>

1 0

Change in flashrom[master]: Add support for National Instruments USB-845x devices
by Márton Miklós (Code Review) 21 Aug '18

21 Aug '18

Márton Miklós has posted comments on this change. ( https://review.coreboot.org/25683 ) Change subject: Add support for National Instruments USB-845x devices ...................................................................... Patch Set 10: (16 comments) Review comments fixed, expect for the code copied from dediprog: I will wait for the authors feedback on the relicensing to GPL v2+. https://review.coreboot.org/#/c/25683/8/Makefile File Makefile: https://review.coreboot.org/#/c/25683/8/Makefile@967 PS8, Line 967: NI845X_LIBS += -L'${PROGRAMFILES(x86)}\National Instruments\NI-845x\MS Visual C' > Please try the following: […] Done https://review.coreboot.org/#/c/25683/8/flashrom.8.tmpl File flashrom.8.tmpl: https://review.coreboot.org/#/c/25683/8/flashrom.8.tmpl@a1094 PS8, Line 1094: : > it seems you dropped this from the digilent_spi description Done https://review.coreboot.org/#/c/25683/8/ni845x_spi.c File ni845x_spi.c: https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@15 PS8, Line 15: * : */ : : #include <ctype.h> > Please remove this paragraph. Done https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@20 PS8, Line 20: #include <string.h> > remove, please. Makefile already takes care of that. Done https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@34 PS8, Line 34: > No CamelCase please, same for all other new identifiers. Done https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@43 PS8, Line 43: static char error_string_buffer[1024]; > This is not supposed to be global, as you added it as an […] Done https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@50 PS8, Line 50: kNi845x33Volts, > obviously Done https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@52 PS8, Line 52: > no space after asterisk Done https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@55 PS8, Line 55: }; > should be `static const` Done https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@117 PS8, Line 117: ial_as_numbe > should be `const char` Done https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@117 PS8, Line 117: > no space after asterisk Done https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@119 PS8, Line 119: > Why is it called `handle`? isn't it rather `resource_value`? Done https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@136 PS8, Line 136: alformed reso > should use the `serial` function parameter instead Done https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@160 PS8, Line 160: if (tmp) { > else (if != 0)? Done https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@166 PS8, Line 166: > You can use `sizeof()` instead, as we know it's a char array. Done https://review.coreboot.org/#/c/25683/8/ni845x_spi.c@169 PS8, Line 169: * @brief ni845x_spi_open_resource > This whole block is very confusing. I don't see bigger problems, […] Done -- To view, visit https://review.coreboot.org/25683 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9477b6f0193bfdf20bbe63421a7fb97b597ec549 Gerrit-Change-Number: 25683 Gerrit-PatchSet: 10 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Elyes HAOUAS <ehaouas(a)noos.fr> Gerrit-CC: Márton Miklós <martonmiklosqdev(a)gmail.com> Gerrit-Comment-Date: Tue, 21 Aug 2018 13:16:17 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add support for National Instruments USB-845x devices
by Miklós Márton (Code Review) 21 Aug '18

21 Aug '18

Miklós Márton has uploaded a new patch set (#10) to the change originally created by David Hendricks. ( https://review.coreboot.org/25683 ) Change subject: Add support for National Instruments USB-845x devices ...................................................................... Add support for National Instruments USB-845x devices Change-Id: I9477b6f0193bfdf20bbe63421a7fb97b597ec549 Signed-off-by: Miklós Márton <martonmiklosqdev(a)gmail.com> --- M Makefile M flashrom.8.tmpl M flashrom.c A ni845x_spi.c M programmer.h 5 files changed, 629 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/25683/10 -- To view, visit https://review.coreboot.org/25683 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9477b6f0193bfdf20bbe63421a7fb97b597ec549 Gerrit-Change-Number: 25683 Gerrit-PatchSet: 10 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Elyes HAOUAS <ehaouas(a)noos.fr> Gerrit-CC: Márton Miklós <martonmiklosqdev(a)gmail.com>

1 0

Change in flashrom[master]: Add support for National Instruments USB-845x devices
by Miklós Márton (Code Review) 21 Aug '18

21 Aug '18

Miklós Márton has uploaded a new patch set (#9) to the change originally created by David Hendricks. ( https://review.coreboot.org/25683 ) Change subject: Add support for National Instruments USB-845x devices ...................................................................... Add support for National Instruments USB-845x devices Change-Id: I9477b6f0193bfdf20bbe63421a7fb97b597ec549 Signed-off-by: Miklós Márton <martonmiklosqdev(a)gmail.com> --- M Makefile M flashrom.8.tmpl M flashrom.c A ni845x_spi.c M programmer.h 5 files changed, 640 insertions(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/83/25683/9 -- To view, visit https://review.coreboot.org/25683 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I9477b6f0193bfdf20bbe63421a7fb97b597ec549 Gerrit-Change-Number: 25683 Gerrit-PatchSet: 9 Gerrit-Owner: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Elyes HAOUAS <ehaouas(a)noos.fr> Gerrit-CC: Márton Miklós <martonmiklosqdev(a)gmail.com>

1 0

Change in flashrom[master]: Add support to get layout from fmap (e.g. coreboot rom)
by David Hendricks (Code Review) 21 Aug '18

21 Aug '18

David Hendricks has posted comments on this change. ( https://review.coreboot.org/23203 ) Change subject: Add support to get layout from fmap (e.g. coreboot rom) ...................................................................... Patch Set 20: (39 comments) Thanks for the thorough review - great suggestions as usual. I finally had some time to look through this all. Some of the proposed changes required substantial refactoring, so the code looks quite a bit different than the previous patch. It looks less like the code in cbfstool and elsewhere, but by the same token should look more like flashrom code. I re-tested --fmap with ROM images that had were bsearch-able and ones that required lsearch. --fmap-file was simplified to only use lsearch since bsearch doesn't really matter there anyway. LMK what you think. https://review.coreboot.org/#/c/23203/19/cli_classic.c File cli_classic.c: https://review.coreboot.org/#/c/23203/19/cli_classic.c@58 PS19, Line 58: fmap in <fmapfile> > s/fmap contained in ? […] Dropped the word "binary" to make it <80. https://review.coreboot.org/#/c/23203/19/cli_classic.c@619 PS19, Line 619: > NB. […] *nod* What should we call it? FLASH_SIZE_BYTES()? Or maybe flash_size_bytes() as a static inline in flash.h? https://review.coreboot.org/#/c/23203/19/fmap.h File fmap.h: https://review.coreboot.org/#/c/23203/19/fmap.h@33 PS19, Line 33: * GNU General Public License ("GPL") version 2 as published by the Free > Is the above license not GPL compatible? It's dual-licensed, as the last paragraph states. Also, in general BSD licensed code can be used in GPLv2 projects. Or maybe I'm missing something? https://review.coreboot.org/#/c/23203/19/fmap.h@44 PS19, Line 44: #define FMAP_VER_MAJOR 1 /* this header's FMAP minor version */ > If we only support v1.1 but there were lower versions, please […] Done https://review.coreboot.org/#/c/23203/19/fmap.h@55 PS19, Line 55: > writing it in big endian seems weird agreed https://review.coreboot.org/#/c/23203/19/fmap.c File fmap.c: PS19: > The mixed usage of `size_t`, `unsigned int` and `unsigned long […] OK, I went ahead and updated that in a few places. Some functions return <0 to indicate failure and >=0 if successful e.g. to indicate offset. To avoid mixing types they could be updated to take an off_t buffer as an output and return 0 or 1 to indicate failure or success. But that would make the API inconsistent with how it's used elsewhere, and probably not worth it for now. https://review.coreboot.org/#/c/23203/19/fmap.c@40 PS19, Line 40: #include "flash.h" > mmh? […] Yes. https://review.coreboot.org/#/c/23203/19/fmap.c@51 PS19, Line 51: > this is `fmap_size(fmap)`? […] Done https://review.coreboot.org/#/c/23203/19/fmap.c@57 PS19, Line 57: if (fmap->ver_major != FMAP_VER_MAJOR) > seems weird to not use a `for` loop Usually I'd agree, but I think the author may have been trying to clarify how the conditions within the loop affect the flow. I don't have a strong preference either way, so I went ahead and updated it. https://review.coreboot.org/#/c/23203/19/fmap.c@62 PS19, Line 62: > seems weird to check this inside the loop I suspect the intention here is to say that if we reach FMAP_STRLEN - 1 without quitting due to the above if-statement, then then this should return failure. Another way to write this would be: if ((i == FMAP_STRLEN - 1) && (fmap->name[i] != 0) Or perhaps these two if-statements could be combined into: if (!isgraph(fmap->name[i]) && (fmap->name[i] != 0) Anyway, even if it seems a bit awkward to you or I it might be best to leave it as is unless there is a real problem to address. https://review.coreboot.org/#/c/23203/19/fmap.c@88 PS19, Line 88: bool fmap_found = > `size_t`? Done https://review.coreboot.org/#/c/23203/19/fmap.c@89 PS19, Line 89: > `bool`? Meh. Done. https://review.coreboot.org/#/c/23203/19/fmap.c@91 PS19, Line 91: c > <= ? Yes, it should be inclusive. Done. https://review.coreboot.org/#/c/23203/19/fmap.c@91 PS19, Line 91: *)&image[offset])) { > `sizeof(struct fmap)` Yep, done. https://review.coreboot.org/#/c/23203/19/fmap.c@92 PS19, Line 92: ap at > casting to `const` is unnecessary Done https://review.coreboot.org/#/c/23203/19/fmap.c@120 PS19, Line 120: msg_gerr("Out of memory.\n"); : return > should be below the inner loop for enhanced readability Gratuitous, but will make the change anyway. https://review.coreboot.org/#/c/23203/19/fmap.c@124 PS19, Line 124: p, fmap_size(fmap)); same as above where this should be inclusive and subtract the sizeof an fmap struct https://review.coreboot.org/#/c/23203/19/fmap.c@126 PS19, Line 126: > so `&image[0]` is always checked? that doesn't seem to make much sense True, I guess it was never a problem earlier, but of course now that we might have a slow programmers it's important to not waste cycles... Anyway, I tried a couple of things and I think the simplest/cleanest solution to address that corner case is to have a variable that tracks whether we've checked offset 0. https://review.coreboot.org/#/c/23203/19/fmap.c@129 PS19, Line 129: struct flashctx *const flashctx, size_t rom_offset, size_t len) > no need to break the line Done https://review.coreboot.org/#/c/23203/19/fmap.c@158 PS19, Line 158: malloc > initialization seems never used Done. https://review.coreboot.org/#/c/23203/19/fmap.c@166 PS19, Line 166: _free_ret: > I don't want to overoptimize things, but I have the feeling that […] After thinking about this a bit, I think we should just get rid of fmap_bsearch in this file. We really only care about the bsearch if we're reading directly from ROM, but in this codepath we're only going to read from memory. https://review.coreboot.org/#/c/23203/19/libflashrom.h File libflashrom.h: https://review.coreboot.org/#/c/23203/19/libflashrom.h@64 PS19, Line 64: , > qualification has no effect here and the name shouldn't be […] OK. https://review.coreboot.org/#/c/23203/19/libflashrom.h@67 PS19, Line 67: struct flashrom_flashctx *, const uint8_t *const buf, size_t len); > The library was intentionally designed to be free from file i/o. […] Sure. It took a bit of refactoring, but done. https://review.coreboot.org/#/c/23203/19/libflashrom.c File libflashrom.c: https://review.coreboot.org/#/c/23203/19/libflashrom.c@428 PS19, Line 428: 1 o > `!*fmap_out` Done https://review.coreboot.org/#/c/23203/19/libflashrom.c@442 PS19, Line 442: msg_gdbg("Adding fmap layout to global layout.\n"); > please break this line Done https://review.coreboot.org/#/c/23203/19/libflashrom.c@466 PS19, Line 466: c > <= ? Yep, this should be inclusive. Done. https://review.coreboot.org/#/c/23203/19/libflashrom.c@466 PS19, Line 466: t flashctx *const flashctx, const uint8_t *cons > could be asserted at the beginning of the function, right? Done. I had a check in the calling function, but it's probably clearer to put it at the beginning of this function. https://review.coreboot.org/#/c/23203/19/libflashrom.c@468 PS19, Line 468: > both conditions don't make much sense, imho. it should be […] Yes - I've added a variable to track whether or not we have checked offset 0. It's a bit of a kludge, but I think it addresses this corner case in the simplest manner. This function always reads from flash so beginning at rom_offset is the most intuitive way. https://review.coreboot.org/#/c/23203/19/libflashrom.c@473 PS19, Line 473: > Since this does not bail out on the stride loop we might as well use continue instead of break? if w […] Yes, that sounds like a good idea. While we're at it, I demoted this debug print to msg_cdbg(). If there's one failure due to a locked region, there will likely be many dozens or hundreds of similar benign failures. https://review.coreboot.org/#/c/23203/19/libflashrom.c@479 PS19, Line 479: } > seems impossible by loop condition Agreed, removed. https://review.coreboot.org/#/c/23203/19/libflashrom.c@485 PS19, Line 485: goto _free_ret; > missing check for NULL Done https://review.coreboot.org/#/c/23203/19/libflashrom.c@486 PS19, Line 486: } > we could skip reading the signature again Done https://review.coreboot.org/#/c/23203/19/libflashrom.c@491 PS19, Line 491: > I'm sure I've seen a function for this. Done https://review.coreboot.org/#/c/23203/19/libflashrom.c@492 PS19, Line 492: return ret; > missing check for NULL Done https://review.coreboot.org/#/c/23203/19/libflashrom.c@494 PS19, Line 494: > we could skip reading the header again Done https://review.coreboot.org/#/c/23203/19/libflashrom.c@499 PS19, Line 499: */ > IIRC, this can be checked on the header alone Done https://review.coreboot.org/#/c/23203/19/libflashrom.c@515 PS19, Line 515: * @param layout Layout to bet set. > potentially leaking `fmap` if we didn't succeed Done https://review.coreboot.org/#/c/23203/19/libflashrom.c@442 PS19, Line 442: msg_gdbg("Adding fmap layout to global layout.\n"); : if (flashrom_layout_parse_fmap(layout, flashctx, fmap)) { : msg_gerr("Failed to add fmap regions to layout.\n"); : ret = 1; : } : : free(fmap); : return ret; : } : : /** : * @brief Read a layout by searching buffer for fmap. : * : * @param[out] layout Points to a struct flashrom_layout pointer that : * gets set if the fmap is read and parsed successfully. : * @param[in] flashctx Flash context : * @param[in] buffer Buffer to search in : * @param[in] size Size of buffer to search : * : * @return 0 on success, : * 2 if the fmap couldn't be read, : * 1 on any other error. : */ : int flashrom_layout_read_fmap_from_buffer(struct flashrom_layout **const layout, : struct flashctx *const flashctx, const uint8_t *const buf, size_t size) : { : struct fmap *fmap = NULL; : int ret = 1; : : if (!buf || !size) : goto _ret; : : msg_gdbg("Attempting to read fmap from buffer.\n"); : if (fmap_read_from_buffer(&fmap, buf, size)) { : msg_gerr("Failed to read fmap from buffer.\n"); : ret = 1; : goto _ret; : } : : msg_gdbg("Adding fmap layout to global layout.\n"); : if (flashrom_layout_parse_fmap(layout, flashctx, fmap)) { : msg_gerr("Failed to add fmap regions to layout.\n"); : ret = 1; : goto _free_ret; : } : : ret = 0; : _free_ret: : free(fmap); : _ret: : return ret; : } : : /** : * @brief Free a layout. : * : * @param layout Layout to free. : */ : void flashrom_layout_release(struct flashrom_layout *const layout) : { : if (layout == get_global_layout()) : return; : : free(layout); : } : : /** : * @brief Set the active layout for a flash context. : * : * Note: This just sets a pointer. The caller must not release the layout : * as long as he uses it through the given flash context. : * : * @param flashctx Flash context whose layout will be set. : * @param layout Layout to bet set. : This whole thing should go in fmap.c. https://review.coreboot.org/#/c/23203/19/libflashrom.c@534 PS19, Line 534: > We should either allocate it or add a check below to not overflow the […] Done -- To view, visit https://review.coreboot.org/23203 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Gerrit-Change-Number: 23203 Gerrit-PatchSet: 20 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Comment-Date: Tue, 21 Aug 2018 05:23:37 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

Change in flashrom[master]: Add support to get layout from fmap (e.g. coreboot rom)
by David Hendricks (Code Review) 21 Aug '18

21 Aug '18

David Hendricks has uploaded a new patch set (#20) to the change originally created by Arthur Heymans. ( https://review.coreboot.org/23203 ) Change subject: Add support to get layout from fmap (e.g. coreboot rom) ...................................................................... Add support to get layout from fmap (e.g. coreboot rom) Flashmap, or simply fmap, is a binary data format for describing region offsets, sizes, and certain attributes and is widely used by coreboot. This patch adds support for the fmap data format version 1.1 and adds --fmap and --fmap-file arguments. Using --fmap will make flashrom to search the ROM content for fmap data. Using --fmap-file will make flashrom search a supplied file for fmap data. An example of how to update the COREBOOT region of a ROM: flashrom -p programmer --fmap -w coreboot.rom -i COREBOOT flashrom -p programmer --fmap-file coreboot.rom -w coreboot.rom -i COREBOOT The fmap functions are mostly copied from cbfstool. Currently it is made mutually exclusive with --ifd while still allowing --layout until we are more clever about this input. Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> Signed-off-by: David Hendricks <dhendricks(a)fb.com> --- M Makefile M cli_classic.c M flashrom.8.tmpl A fmap.c A fmap.h M libflashrom.c M libflashrom.h 7 files changed, 662 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/03/23203/20 -- To view, visit https://review.coreboot.org/23203 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I0e7fad38ed79a84d41358e1f175c36d255786c12 Gerrit-Change-Number: 23203 Gerrit-PatchSet: 20 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Werner Zeh <werner.zeh(a)siemens.com>

1 0

Change in flashrom[master]: Add initial J-Link SPI programmer
by Nico Huber (Code Review) 20 Aug '18

20 Aug '18

Nico Huber has posted comments on this change. ( https://review.coreboot.org/28087 ) Change subject: Add initial J-Link SPI programmer ...................................................................... Patch Set 4: (3 comments) https://review.coreboot.org/#/c/28087/2/jlink_spi.c File jlink_spi.c: https://review.coreboot.org/#/c/28087/2/jlink_spi.c@344 PS2, Line 344: > No, the device instance is not free'd because we still have a reference on it (jaylink_open()) Ack, I checked their code... not that they cared to document this side effect of jaylink_open(). https://review.coreboot.org/#/c/28087/4/jlink_spi.c File jlink_spi.c: https://review.coreboot.org/#/c/28087/4/jlink_spi.c@214 PS4, Line 214: && strlen(arg) > 0 same here... https://review.coreboot.org/#/c/28087/4/jlink_spi.c@237 PS4, Line 237: && strlen(arg) > 0 and there? -- To view, visit https://review.coreboot.org/28087 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie03a054a75457ec9e1cab36ea124bb53b10e8d7e Gerrit-Change-Number: 28087 Gerrit-PatchSet: 4 Gerrit-Owner: Marc Schink <flashrom-dev(a)marcschink.de> Gerrit-Reviewer: Marc Schink <flashrom-dev(a)marcschink.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Mon, 20 Aug 2018 21:29:01 +0000 Gerrit-HasComments: Yes Gerrit-HasLabels: No

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit August 2018