Attention is currently required from: Nico Huber, Thomas Heijligen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52946 )
Change subject: programmer_table: move each entry to the associated programmer source
......................................................................
Patch Set 4: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/52946
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3d02bd789f0299e936eb86819b3b15b5ea2bb921
Gerrit-Change-Number: 52946
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Xiang W <wxjstz(a)126.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Mon, 07 Jun 2021 21:26:39 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Nico Huber, Thomas Heijligen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52946 )
Change subject: programmer_table: move each entry to the associated programmer source
......................................................................
Patch Set 4:
(2 comments)
Patchset:
PS4:
Thomas this is very cool, thank you!!
File programmer.h:
https://review.coreboot.org/c/flashrom/+/52946/comment/3648704a_a3cb7188
PS4, Line 402: #if CONFIG_DUMMY == 1
> Please just drop all these `#if`. The only difference it makes is […]
I am trying to understand, why it has to be an error (compile-time or link-time)? If some CONFIG_xxx is disabled, then one line with programmer_entry is not included, why an error?
--
To view, visit https://review.coreboot.org/c/flashrom/+/52946
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I3d02bd789f0299e936eb86819b3b15b5ea2bb921
Gerrit-Change-Number: 52946
Gerrit-PatchSet: 4
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Xiang W <wxjstz(a)126.com>
Gerrit-Attention: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Mon, 07 Jun 2021 21:25:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Angel Pons.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52945 )
Change subject: programmer_table: convert entries to pointers
......................................................................
Patch Set 3:
(1 comment)
File flashrom.c:
https://review.coreboot.org/c/flashrom/+/52945/comment/aecb42f5_4c8daf53
PS3, Line 1716: programmer_table_size != PROGRAMMER_INVALID
How this works after PROGRAMMER_INVALID has been removed from the table in CB:55115 ?
--
To view, visit https://review.coreboot.org/c/flashrom/+/52945
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ifbb0ee4db5a85b1cd2afeafe4dca838579f79878
Gerrit-Change-Number: 52945
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
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: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Xiang W <wxjstz(a)126.com>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Comment-Date: Mon, 07 Jun 2021 20:55:44 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: Thomas Heijligen.
Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/52944 )
Change subject: programmer_table: move array content to an own file
......................................................................
Patch Set 3: Code-Review+1
--
To view, visit https://review.coreboot.org/c/flashrom/+/52944
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I8e6d704e845ee4152c8676dd19dff0934fff007b
Gerrit-Change-Number: 52944
Gerrit-PatchSet: 3
Gerrit-Owner: Thomas Heijligen <src(a)posteo.de>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-CC: Xiang W <wxjstz(a)126.com>
Gerrit-Attention: Thomas Heijligen <src(a)posteo.de>
Gerrit-Comment-Date: Mon, 07 Jun 2021 20:43:21 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: Edward O'Callaghan, Angel Pons, Arthur Heymans, Anastasia Klimchuk.
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/55269 )
Change subject: treewide: Use calloc() where applicable
......................................................................
Patch Set 1:
(5 comments)
Patchset:
PS1:
I'm not convinced about unnecessary initialization. Want to have it
discussed at least once...
File jlink_spi.c:
https://review.coreboot.org/c/flashrom/+/55269/comment/796f3f29_e13f22fd
PS1, Line 128: memset(buffer + writecnt, 0x00, readcnt);
I wonder why this is done at all.
File libflashrom.c:
https://review.coreboot.org/c/flashrom/+/55269/comment/cf3f41c7_d207abf8
PS1, Line 123: const char **supported_programmers = calloc(PROGRAMMER_INVALID + 1, sizeof(char *));
This is probably what the original author intended. IIRC, I left a comment
in Gerrit about this API (which is completely broken).
https://review.coreboot.org/c/flashrom/+/55269/comment/0cd9cdb4_71590a0a
PS1, Line 144: calloc(flashchips_size, sizeof(*supported_flashchips));
Hmmm, I'm not convinced to initialize things that are supposed to be completely
overwritten later. I doubt that it's the case for calloc() but I know that some
static analysis tools complain about explicit but unnecessary initialization.
Failure to override all fields could be avoided, for instance, by using a
struct initializer below, e.g.
const struct flashrom_flashchip_info supported_flashchip = {
.vendor = flashchips[i].vendor,
/* ... */
};
supported_flashchips[i] = supported_flashchip;
File mstarddc_spi.c:
https://review.coreboot.org/c/flashrom/+/55269/comment/e63a6177_c38b5157
PS1, Line 83: uint8_t *cmd = malloc((writecnt + 1) * sizeof(uint8_t));
This is just `malloc(writecnt + 1);` AIUI.
--
To view, visit https://review.coreboot.org/c/flashrom/+/55269
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Id6ef0d6d02c7b63baabc9a0087275cab22a48736
Gerrit-Change-Number: 55269
Gerrit-PatchSet: 1
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Attention: Arthur Heymans <arthur(a)aheymans.xyz>
Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org>
Gerrit-Comment-Date: Mon, 07 Jun 2021 14:14:47 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/54964 )
Change subject: platform: Fix endianness detection for Apple Silicon Macs
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
> I was able to successfully (and repeatedly) read and write W25X40 SPI flash with CH341A-based progra […]
That tells us that libusb is working and probably a lot of USB programmers with it :)
--
To view, visit https://review.coreboot.org/c/flashrom/+/54964
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: Ifdb1523ee2c7023e657cfd7b823b091d5deef513
Gerrit-Change-Number: 54964
Gerrit-PatchSet: 3
Gerrit-Owner: Ivan V <root(a)pcm720.me>
Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org>
Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org>
Gerrit-Comment-Date: Mon, 07 Jun 2021 13:06:43 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Ivan V <root(a)pcm720.me>
Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de>
Gerrit-MessageType: comment
Attention is currently required from: Angel Pons.
Hello Nico Huber,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/55267
to look at the new patch set (#2).
Change subject: treewide: Drop unnecessary uses of memset/memcpy
......................................................................
treewide: Drop unnecessary uses of memset/memcpy
Simply provide an initialiser or use a direct assignment instead.
Change-Id: I07385375cd8eec8a95874001b402b2c17ec09e09
Signed-off-by: Angel Pons <th3fanbus(a)gmail.com>
---
M flashrom.c
M ichspi.c
M jlink_spi.c
M linux_mtd.c
M physmap.c
M raiden_debug_spi.c
M stlinkv3_spi.c
M usbblaster_spi.c
8 files changed, 21 insertions(+), 49 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/67/55267/2
--
To view, visit https://review.coreboot.org/c/flashrom/+/55267
To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings
Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-Change-Id: I07385375cd8eec8a95874001b402b2c17ec09e09
Gerrit-Change-Number: 55267
Gerrit-PatchSet: 2
Gerrit-Owner: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de>
Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com>
Gerrit-MessageType: newpatchset