December 2022 - flashrom-gerrit

[M] Change in flashrom[master]: spi.c: Add AT45 & SF25F erasefn opcode mapping
by Edward O'Callaghan (Code Review) 04 Dec '22

04 Dec '22

Attention is currently required from: Simon Buhrow, Thomas Heijligen, Aarya. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67717 ) Change subject: spi.c: Add AT45 & SF25F erasefn opcode mapping ...................................................................... Patch Set 6: (3 comments) Patchset: PS6: I would just squash this and the two previous patches together. There is no benefit to having them separate and actually introduces the possibility of subtle things leaking though review like that NULL. File spi.c: https://review.coreboot.org/c/flashrom/+/67717/comment/e2e3c4d2_66e1fe1c PS6, Line 229: Assuming 0x00 .. https://review.coreboot.org/c/flashrom/+/67717/comment/2adc1ada_10f71bcd PS6, Line 229: 0x00 `NULL` -- To view, visit https://review.coreboot.org/c/flashrom/+/67717 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I798a91f1e20b63662715c68e6d43d03fc6005d51 Gerrit-Change-Number: 67717 Gerrit-PatchSet: 6 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Simon Buhrow Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Sun, 04 Dec 2022 23:18:30 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: layout: Check return values for strdup in register_include_arg
by Felix Singer (Code Review) 03 Dec '22

03 Dec '22

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70006 ) Change subject: layout: Check return values for strdup in register_include_arg ...................................................................... Patch Set 3: (4 comments) File layout.c: https://review.coreboot.org/c/flashrom/+/70006/comment/cfe0ec42_17bcabf6 PS2, Line 139: || file == NULL > Yes I forgot that indeed file is optional parameter. […] Line 144 causes a memory leak now. If a colon is found but the second parameter (the file name) isn't given, then `name` doesn't get freed because line 144 just returns. That's why I suggested to leave that code where it is and to just extend that if-block, so that we error out early as possible. You can either replace that return with `goto error` or you move lines 140-152 below line 131, where they were before. I prefer the latter. File layout.c: https://review.coreboot.org/c/flashrom/+/70006/comment/1511c23a_63309098 PS3, Line 135: if (name == NULL) { nit: `!name` https://review.coreboot.org/c/flashrom/+/70006/comment/9ff374eb_419a66a6 PS3, Line 142: if(!colon[1]) { mising space `if (` https://review.coreboot.org/c/flashrom/+/70006/comment/b94b0165_679e6b6f PS3, Line 148: if (file == NULL) { nit: `!file` -- To view, visit https://review.coreboot.org/c/flashrom/+/70006 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6c22196be6847a8c9704f1de936604a51b4b8a28 Gerrit-Change-Number: 70006 Gerrit-PatchSet: 3 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sat, 03 Dec 2022 15:09:59 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: spi.c: Add erasefn and opcodes for AT45 and S25F to lookup list
by Aarya (Code Review) 03 Dec '22

03 Dec '22

Aarya has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/67354 ) Change subject: spi.c: Add erasefn and opcodes for AT45 and S25F to lookup list ...................................................................... Abandoned CB:67717 does the job -- To view, visit https://review.coreboot.org/c/flashrom/+/67354 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ief25932120f940dea0ffe161a79219deecb2e8ab Gerrit-Change-Number: 67354 Gerrit-PatchSet: 12 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: abandon

1 0

[M] Change in flashrom[master]: spi.c: Move spi_get_erasefn_from_opcode to spi.c
by Aarya (Code Review) 03 Dec '22

03 Dec '22

Aarya has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/67368 ) Change subject: spi.c: Move spi_get_erasefn_from_opcode to spi.c ...................................................................... Abandoned CB:67715 does the job -- To view, visit https://review.coreboot.org/c/flashrom/+/67368 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3263b7642c35e7fd1da0002175123e5560d94752 Gerrit-Change-Number: 67368 Gerrit-PatchSet: 9 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Simon Buhrow Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: abandon

1 0

[S] Change in flashrom[master]: spi.c: Make parameter (struct flashctx *) const
by Aarya (Code Review) 03 Dec '22

03 Dec '22

Aarya has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/70306 ) Change subject: spi.c: Make parameter (struct flashctx *) const ...................................................................... spi.c: Make parameter (struct flashctx *) const Make (struct flashctx *) -> (const struct flashctx *) in probe_opcode functions. Change-Id: Ied840dbbcbe50e44cabd32eea37b7257b5e9c957 Signed-off-by: Aarya Chaumal <aarya.chaumal(a)gmail.com> --- M dummyflasher.c M ichspi.c M include/programmer.h M spi.c 4 files changed, 18 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/70306/1 diff --git a/dummyflasher.c b/dummyflasher.c index 35cdfef..ca744f8 100644 --- a/dummyflasher.c +++ b/dummyflasher.c @@ -122,7 +122,7 @@ emu_data->spi_write_256_chunksize); } -static bool dummy_spi_probe_opcode(struct flashctx *flash, uint8_t opcode) +static bool dummy_spi_probe_opcode(const struct flashctx *flash, uint8_t opcode) { size_t i; struct emu_data *emu_data = flash->mst->spi.data; diff --git a/ichspi.c b/ichspi.c index 62d1799..4588502 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1661,7 +1661,7 @@ return ret; } -static bool ich_spi_probe_opcode(struct flashctx *flash, uint8_t opcode) +static bool ich_spi_probe_opcode(const struct flashctx *flash, uint8_t opcode) { return find_opcode(curopcodes, opcode) >= 0; } diff --git a/include/programmer.h b/include/programmer.h index 55e300a..16448ea 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -312,7 +312,7 @@ int (*write_256)(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int (*write_aai)(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int (*shutdown)(void *data); - bool (*probe_opcode)(struct flashctx *flash, uint8_t opcode); + bool (*probe_opcode)(const struct flashctx *flash, uint8_t opcode); void *data; }; @@ -322,7 +322,7 @@ int default_spi_read(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int default_spi_write_256(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int default_spi_write_aai(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); -bool default_spi_probe_opcode(struct flashctx *flash, uint8_t opcode); +bool default_spi_probe_opcode(const struct flashctx *flash, uint8_t opcode); int register_spi_master(const struct spi_master *mst, void *data); /* The following enum is needed by ich_descriptor_tool and ich* code as well as in chipset_enable.c. */ diff --git a/spi.c b/spi.c index 26be8c7..93ef1af 100644 --- a/spi.c +++ b/spi.c @@ -134,7 +134,7 @@ return flash->mst->spi.write_aai(flash, buf, start, len); } -bool default_spi_probe_opcode(struct flashctx *flash, uint8_t opcode) +bool default_spi_probe_opcode(const struct flashctx *flash, uint8_t opcode) { return true; } -- To view, visit https://review.coreboot.org/c/flashrom/+/70306 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ied840dbbcbe50e44cabd32eea37b7257b5e9c957 Gerrit-Change-Number: 70306 Gerrit-PatchSet: 1 Gerrit-Owner: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newchange

1 0

[S] Change in flashrom[master]: layout: Check return values for strdup in register_include_arg
by Anastasia Klimchuk (Code Review) 03 Dec '22

03 Dec '22

Attention is currently required from: Felix Singer, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70006 ) Change subject: layout: Check return values for strdup in register_include_arg ...................................................................... Patch Set 3: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/70006/comment/b4a9e04f_3de5b2c2 PS2, Line 11: > Explain the issues and the new code changes. Done File layout.c: https://review.coreboot.org/c/flashrom/+/70006/comment/c801d890_332d18c9 PS2, Line 139: || file == NULL > Just rethinking.. […] Yes I forgot that indeed file is optional parameter. Please have a look at my latest version, whether it covers what we want? I read all your comments carefully. -- To view, visit https://review.coreboot.org/c/flashrom/+/70006 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6c22196be6847a8c9704f1de936604a51b4b8a28 Gerrit-Change-Number: 70006 Gerrit-PatchSet: 3 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Sat, 03 Dec 2022 09:32:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: layout: Check return values for strdup in register_include_arg
by Anastasia Klimchuk (Code Review) 03 Dec '22

03 Dec '22

Attention is currently required from: Edward O'Callaghan, Angel Pons, Anastasia Klimchuk. Hello Felix Singer, build bot (Jenkins), Edward O'Callaghan, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70006 to look at the new patch set (#3). Change subject: layout: Check return values for strdup in register_include_arg ...................................................................... layout: Check return values for strdup in register_include_arg strdup return values should be checked for NULL to catch the potential error case of out of memory. Follow up on commit 45d50a101e8073191e6d88143990ed91d3bfe815 Ticket: https://ticket.coreboot.org/issues/372 Change-Id: I6c22196be6847a8c9704f1de936604a51b4b8a28 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M layout.c 1 file changed, 39 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/06/70006/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/70006 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I6c22196be6847a8c9704f1de936604a51b4b8a28 Gerrit-Change-Number: 70006 Gerrit-PatchSet: 3 Gerrit-Owner: 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: spi: Make 'default_spi_write_aai' the default unless defined
by Edward O'Callaghan (Code Review) 03 Dec '22

03 Dec '22

Attention is currently required from: Thomas Heijligen. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/67479 ) Change subject: spi: Make 'default_spi_write_aai' the default unless defined ...................................................................... Patch Set 4: (2 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/67479/comment/fb901c4e_6c4057e1 PS1, Line 9: Drop the explicit need to specify the default 'default_spi_write_aai' > Please mention the struct where you perform the changes on Done Patchset: PS1: > Two things: […] Addressed the first part in the commit message by making clear the mathematical constraint here in precise terms. Regarding the second part. I am not sure renaming everything in the tree with a prefix or suffix of optimised. For one that seems like a different patch to this one that is semantic and secondly that line of reasoning means everything non-NULL is a 'optimised/optional' version which I am not sure makes sense either - 'custom' would be a closer term to the reality but even then makes no sense to use in the function identifier. All we are saying here is to make function pointers in the struct RAII where a default construction of the type into a initial value is valid. To address the concerns of the second part a alternative compromise would perhaps just be a code comment above spi.c:spi_aai_write(). -- To view, visit https://review.coreboot.org/c/flashrom/+/67479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7f14aaea0edcf0c08cea0e9cd27d58152707fb2a Gerrit-Change-Number: 67479 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Sat, 03 Dec 2022 03:31:40 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: spi: Make 'default_spi_write_aai' the default unless defined
by Edward O'Callaghan (Code Review) 03 Dec '22

03 Dec '22

Attention is currently required from: Edward O'Callaghan. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Anastasia Klimchuk, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67479 to look at the new patch set (#4). Change subject: spi: Make 'default_spi_write_aai' the default unless defined ...................................................................... spi: Make 'default_spi_write_aai' the default unless defined A NULL func pointer is necessary and sufficient for the condition `NULL func pointer => default_spi_write_aai' as to not need this explicit specification of 'default'. Therefore Drop the explicit need to specify the 'default_spi_write_aai' callback function pointer in the spi_master struct. This is a reasonable default for every other driver in the tree with only a few exceptions. This simplifies the code and driver development. Change-Id: I7f14aaea0edcf0c08cea0e9cd27d58152707fb2a Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M bitbang_spi.c M buspirate_spi.c M ch341a_spi.c M digilent_spi.c M dummyflasher.c M ft2232_spi.c M ichspi.c M jlink_spi.c M linux_spi.c M mediatek_i2c_spi.c M mstarddc_spi.c M ni845x_spi.c M pickit2_spi.c M raiden_debug_spi.c M sb600spi.c M serprog.c M spi.c M stlinkv3_spi.c M usbblaster_spi.c 19 files changed, 23 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/67479/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/67479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7f14aaea0edcf0c08cea0e9cd27d58152707fb2a Gerrit-Change-Number: 67479 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(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-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: spi: Make 'default_spi_write_aai' the default unless defined
by Edward O'Callaghan (Code Review) 03 Dec '22

03 Dec '22

Attention is currently required from: Edward O'Callaghan. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Anastasia Klimchuk, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/67479 to look at the new patch set (#3). Change subject: spi: Make 'default_spi_write_aai' the default unless defined ...................................................................... spi: Make 'default_spi_write_aai' the default unless defined Drop the explicit need to specify the default 'default_spi_write_aai' callback function pointer in the spi_master struct. This is a reasonable default for every other driver in the tree with only a few exceptions. This simplifies the code and driver development. Change-Id: I7f14aaea0edcf0c08cea0e9cd27d58152707fb2a Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M bitbang_spi.c M buspirate_spi.c M ch341a_spi.c M digilent_spi.c M dummyflasher.c M ft2232_spi.c M ichspi.c M jlink_spi.c M linux_spi.c M mediatek_i2c_spi.c M mstarddc_spi.c M ni845x_spi.c M pickit2_spi.c M raiden_debug_spi.c M sb600spi.c M serprog.c M spi.c M stlinkv3_spi.c M usbblaster_spi.c 19 files changed, 21 insertions(+), 24 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/79/67479/3 -- To view, visit https://review.coreboot.org/c/flashrom/+/67479 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I7f14aaea0edcf0c08cea0e9cd27d58152707fb2a Gerrit-Change-Number: 67479 Gerrit-PatchSet: 3 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Felix Singer <felixsinger(a)posteo.net> Gerrit-Reviewer: Peter Marheine <pmarheine(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-MessageType: newpatchset

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit December 2022