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

Change in flashrom[master]: linux_mtd: Free param right after its last usage
by Angel Pons (Code Review) 05 Aug '21

05 Aug '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56822 ) Change subject: linux_mtd: Free param right after its last usage ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File linux_mtd.c: https://review.coreboot.org/c/flashrom/+/56822/comment/4a96e689_b4c8808b PS1, Line 401: if (param) : msg_perr("%s does not exist\n", sysfs_path); : else : msg_pdbg("%s does not exist\n", sysfs_path); suspicious, both branches are equal? -- To view, visit https://review.coreboot.org/c/flashrom/+/56822 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I126a8d06297ef4d42bc93a73f7067ccc1352d1e9 Gerrit-Change-Number: 56822 Gerrit-PatchSet: 1 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: 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 05 Aug 2021 14:35:39 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: nicintel_eeprom: Check UNPROG_DEVICE in 82580 shutdown
by Angel Pons (Code Review) 05 Aug '21

05 Aug '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56821 ) Change subject: nicintel_eeprom: Check UNPROG_DEVICE in 82580 shutdown ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://review.coreboot.org/c/flashrom/+/56821 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5c729a3a63d0106e65525a6a77b2f9104c96847f Gerrit-Change-Number: 56821 Gerrit-PatchSet: 1 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: 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 05 Aug 2021 14:34:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: nicintel_eeprom: Delete extra parentheses from if condition
by Angel Pons (Code Review) 05 Aug '21

05 Aug '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Anastasia Klimchuk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56820 ) Change subject: nicintel_eeprom: Delete extra parentheses from if condition ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56820 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I068bd227668a79f14e38e703a5a49db18d5de8c7 Gerrit-Change-Number: 56820 Gerrit-PatchSet: 1 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: 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 05 Aug 2021 14:33:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: realtek_mst_i2c_spi: don't always fail init
by Angel Pons (Code Review) 05 Aug '21

05 Aug '21

Attention is currently required from: Paul Menzel, Peter Marheine. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56799 ) Change subject: realtek_mst_i2c_spi: don't always fail init ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/56799 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0596014fc9b76b4d11de230f9f5c414c1321dff1 Gerrit-Change-Number: 56799 Gerrit-PatchSet: 2 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Comment-Date: Thu, 05 Aug 2021 14:32:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: libflashrom: clear layout on flashrom_layout_release
by Peter Marheine (Code Review) 05 Aug '21

05 Aug '21

Peter Marheine has abandoned this change. ( https://review.coreboot.org/c/flashrom/+/54147 ) Change subject: libflashrom: clear layout on flashrom_layout_release ...................................................................... Abandoned Fixed more comprehensively by removing the global layout. -- To view, visit https://review.coreboot.org/c/flashrom/+/54147 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8ba4b356f2a40ccbfd4e91f81af01d5f6a6de515 Gerrit-Change-Number: 54147 Gerrit-PatchSet: 3 Gerrit-Owner: Peter Marheine <pmarheine(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Daniel Campello <campello(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-MessageType: abandon

1 0

Change in flashrom[master]: opaque_master: Use new API to register shutdown function
by Anastasia Klimchuk (Code Review) 05 Aug '21

05 Aug '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Angel Pons. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/56825 ) Change subject: opaque_master: Use new API to register shutdown function ...................................................................... Patch Set 1: (1 comment) File nicintel_eeprom.c: https://review.coreboot.org/c/flashrom/+/56825/comment/a98ffe4a_6e88d6b2 PS1, Line 497: : return register_opaque_master(&opaque_master_nicintel_ee_82580, eecp); The piece of data, eecp, is only used in shutdown function, and not used as opaque_master data (previously there was NULL passed as opaque_master data). I don't think it is a problem, but please correct me if I am wrong. This doesn't make any change to shutdown flow, because eecp was used as data before and same now. As for other operations (read/write etc), they do not expect any data in flashctx, so they will just ignore eecp. For future, ideally global state will be removed from this programmer (there is global state here at the moment, I am not changing this). Then, piece of data in opaque_master will actually be used, and eecp will become one of the members of data struct. -- To view, visit https://review.coreboot.org/c/flashrom/+/56825 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id8471a117556edcbf9694752fabe05cf4501ce70 Gerrit-Change-Number: 56825 Gerrit-PatchSet: 1 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: 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: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 05 Aug 2021 05:45:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: opaque_master: Use new API to register shutdown function
by Anastasia Klimchuk (Code Review) 05 Aug '21

05 Aug '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/56825 ) Change subject: opaque_master: Use new API to register shutdown function ...................................................................... opaque_master: Use new API to register shutdown function TEST ME ON DEVICE This allows opaque masters to register shutdown function in opaque_master struct, which means there is no need to call register_shutdown in init function, since this call is now a part of register_opaque_master. BUG=b:185191942 TEST=builds and ninja test Change-Id: Id8471a117556edcbf9694752fabe05cf4501ce70 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M linux_mtd.c M nicintel_eeprom.c 2 files changed, 5 insertions(+), 15 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/25/56825/1 diff --git a/linux_mtd.c b/linux_mtd.c index 42d569d..04eccde 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -314,6 +314,7 @@ .read = linux_mtd_read, .write = linux_mtd_write, .erase = linux_mtd_erase, + .shutdown = linux_mtd_shutdown, }; /* Returns 0 if setup is successful, non-zero to indicate error */ @@ -418,14 +419,7 @@ return 1; } - if (register_shutdown(linux_mtd_shutdown, (void *)data)) { - free(data); - return 1; - } - - register_opaque_master(&linux_mtd_opaque_master, data); - - return 0; + return register_opaque_master(&linux_mtd_opaque_master, data); linux_mtd_init_exit: free(param); diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index fd90250..5887073 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -446,6 +446,7 @@ .read = nicintel_ee_read, .write = nicintel_ee_write_82580, .erase = nicintel_ee_erase_82580, + .shutdown = nicintel_ee_shutdown_82580, }; static const struct opaque_master opaque_master_nicintel_ee_i210 = { @@ -453,6 +454,7 @@ .read = nicintel_ee_read, .write = nicintel_ee_write_i210, .erase = nicintel_ee_erase_i210, + .shutdown = nicintel_ee_shutdown_i210, }; static int nicintel_ee_init(void) @@ -493,19 +495,13 @@ *eecp = eec; } - if (register_shutdown(nicintel_ee_shutdown_82580, eecp)) - return 1; - - return register_opaque_master(&opaque_master_nicintel_ee_82580, NULL); + return register_opaque_master(&opaque_master_nicintel_ee_82580, eecp); } else { nicintel_eebar = rphysmap("Intel i210 NIC w/ emulated EEPROM", io_base_addr + 0x12000, MEMMAP_SIZE); if (!nicintel_eebar) return 1; - if (register_shutdown(nicintel_ee_shutdown_i210, NULL)) - return 1; - return register_opaque_master(&opaque_master_nicintel_ee_i210, NULL); } -- To view, visit https://review.coreboot.org/c/flashrom/+/56825 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id8471a117556edcbf9694752fabe05cf4501ce70 Gerrit-Change-Number: 56825 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: opaque_master: Move shutdown function above opaque_master struct
by Anastasia Klimchuk (Code Review) 05 Aug '21

05 Aug '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/56824 ) Change subject: opaque_master: Move shutdown function above opaque_master struct ...................................................................... opaque_master: Move shutdown function above opaque_master struct This patch prepares opaque masters to use new API which allows to register shutdown function in opaque_master struct. See also later patch in this chain, where opaque masters are converted to new API. BUG=b:185191942 TEST=builds and ninja test Comparing flashrom binary before and after the patch, make clean && make CONFIG_EVERYTHING=yes VERSION=none binary is the same Change-Id: I5000cfceeba859a76177a17c1cb7d1c1e9fc03fe Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M linux_mtd.c M nicintel_eeprom.c 2 files changed, 25 insertions(+), 25 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/24/56824/1 diff --git a/linux_mtd.c b/linux_mtd.c index 7780feb..42d569d 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -295,6 +295,17 @@ return 0; } +static int linux_mtd_shutdown(void *data) +{ + struct linux_mtd_data *mtd_data = data; + if (mtd_data->dev_fp != NULL) { + fclose(mtd_data->dev_fp); + } + free(data); + + return 0; +} + static const struct opaque_master linux_mtd_opaque_master = { /* max_data_{read,write} don't have any effect for this programmer */ .max_data_read = MAX_DATA_UNSPECIFIED, @@ -356,17 +367,6 @@ return ret; } -static int linux_mtd_shutdown(void *data) -{ - struct linux_mtd_data *mtd_data = data; - if (mtd_data->dev_fp != NULL) { - fclose(mtd_data->dev_fp); - } - free(data); - - return 0; -} - static int linux_mtd_init(void) { char *param; diff --git a/nicintel_eeprom.c b/nicintel_eeprom.c index ec0f04e..fd90250 100644 --- a/nicintel_eeprom.c +++ b/nicintel_eeprom.c @@ -395,20 +395,6 @@ return nicintel_ee_write_82580(flash, NULL, addr, len); } -static const struct opaque_master opaque_master_nicintel_ee_82580 = { - .probe = nicintel_ee_probe_82580, - .read = nicintel_ee_read, - .write = nicintel_ee_write_82580, - .erase = nicintel_ee_erase_82580, -}; - -static const struct opaque_master opaque_master_nicintel_ee_i210 = { - .probe = nicintel_ee_probe_i210, - .read = nicintel_ee_read, - .write = nicintel_ee_write_i210, - .erase = nicintel_ee_erase_i210, -}; - static int nicintel_ee_shutdown_i210(void *arg) { if (!done_i20_write) @@ -455,6 +441,20 @@ return ret; } +static const struct opaque_master opaque_master_nicintel_ee_82580 = { + .probe = nicintel_ee_probe_82580, + .read = nicintel_ee_read, + .write = nicintel_ee_write_82580, + .erase = nicintel_ee_erase_82580, +}; + +static const struct opaque_master opaque_master_nicintel_ee_i210 = { + .probe = nicintel_ee_probe_i210, + .read = nicintel_ee_read, + .write = nicintel_ee_write_i210, + .erase = nicintel_ee_erase_i210, +}; + static int nicintel_ee_init(void) { if (rget_io_perms()) -- To view, visit https://review.coreboot.org/c/flashrom/+/56824 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I5000cfceeba859a76177a17c1cb7d1c1e9fc03fe Gerrit-Change-Number: 56824 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: opaque_master: Add shutdown function in opaque_master struct
by Anastasia Klimchuk (Code Review) 05 Aug '21

05 Aug '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/56823 ) Change subject: opaque_master: Add shutdown function in opaque_master struct ...................................................................... opaque_master: Add shutdown function in opaque_master struct With this, register_opaque_master can take care of register_shutdown as well, and every opaque master only needs to call register_opaque_master instead of calling both register_opaque_master and register_shutdown. Next patches in the chain convert opaque masters to use new API. BUG=b:185191942 TEST=builds and ninja test from CB:56413 Change-Id: I34183e6bafc787eec54ee4a26b73a40803f3ce99 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M opaque.c M programmer.h 2 files changed, 8 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/56823/1 diff --git a/opaque.c b/opaque.c index f098ffc..7704ec7 100644 --- a/opaque.c +++ b/opaque.c @@ -50,6 +50,13 @@ { struct registered_master rmst = {0}; + if (mst->shutdown) { + if (register_shutdown(mst->shutdown, data)) { + mst->shutdown(data); /* cleanup */ + return 1; + } + } + if (!mst->probe || !mst->read || !mst->write || !mst->erase) { msg_perr("%s called with incomplete master definition. " "Please report a bug at flashrom(a)flashrom.org\n", diff --git a/programmer.h b/programmer.h index 95e2cda..074dd32 100644 --- a/programmer.h +++ b/programmer.h @@ -442,6 +442,7 @@ int (*read) (struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len); int (*write) (struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len); int (*erase) (struct flashctx *flash, unsigned int blockaddr, unsigned int blocklen); + int (*shutdown)(void *data); void *data; }; int register_opaque_master(const struct opaque_master *mst, void *data); -- To view, visit https://review.coreboot.org/c/flashrom/+/56823 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I34183e6bafc787eec54ee4a26b73a40803f3ce99 Gerrit-Change-Number: 56823 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: linux_mtd: Free param right after its last usage
by Anastasia Klimchuk (Code Review) 05 Aug '21

05 Aug '21

Anastasia Klimchuk has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/56822 ) Change subject: linux_mtd: Free param right after its last usage ...................................................................... linux_mtd: Free param right after its last usage Param is only used in the first half of init function, and it is local, so there is no need to keep it until the end. This makes handling error paths in the second half of init function shorter, because those paths can just return 1 instead of going to a label. BUG=b:185191942 TEST=builds and ninja test from CB:56413 Change-Id: I126a8d06297ef4d42bc93a73f7067ccc1352d1e9 Signed-off-by: Anastasia Klimchuk <aklm(a)chromium.org> --- M linux_mtd.c 1 file changed, 6 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/22/56822/1 diff --git a/linux_mtd.c b/linux_mtd.c index 284cde4..7780feb 100644 --- a/linux_mtd.c +++ b/linux_mtd.c @@ -404,27 +404,29 @@ msg_pdbg("%s does not exist\n", sysfs_path); goto linux_mtd_init_exit; } + free(param); data = calloc(1, sizeof(*data)); if (!data) { msg_perr("Unable to allocate memory for linux_mtd_data\n"); - goto linux_mtd_init_exit; + return 1; } /* Get MTD info and store it in `data` */ if (linux_mtd_setup(dev_num, data)) { free(data); - goto linux_mtd_init_exit; + return 1; } if (register_shutdown(linux_mtd_shutdown, (void *)data)) { free(data); - goto linux_mtd_init_exit; + return 1; } register_opaque_master(&linux_mtd_opaque_master, data); - ret = 0; + return 0; + linux_mtd_init_exit: free(param); return ret; -- To view, visit https://review.coreboot.org/c/flashrom/+/56822 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I126a8d06297ef4d42bc93a73f7067ccc1352d1e9 Gerrit-Change-Number: 56822 Gerrit-PatchSet: 1 Gerrit-Owner: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newchange

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit August 2021