November 2022 - flashrom-gerrit

[S] Change in flashrom[master]: chipdrivers: Rename s/erase_sector_stm50/stm50_sector_erase
by Edward O'Callaghan (Code Review) 13 Nov '22

13 Nov '22

Attention is currently required from: Felix Singer. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69047 ) Change subject: chipdrivers: Rename s/erase_sector_stm50/stm50_sector_erase ...................................................................... Patch Set 3: (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/69047/comment/63ba7f75_fae1c7ce PS3, Line 377: STM50_SECTOR_ERASE > I am wondering if we should go the other way, renaming this to ERASE_SECTOR_STM50, since this would […] No, that is just aesthetically making things 'look the same' not making things that have meaning to be consistent. The aim here is to have a `vendor_` || `protocol_vendor_` || `std_protocol_vendor_` prefix rather than a suffix. The op should be the suffix. So, `JEDEC_thing_erase` || `SPI_thing_erase` etc. The current names are all over the place. -- To view, visit https://review.coreboot.org/c/flashrom/+/69047 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia916b5782abf04499ab46a540820acbe4a171068 Gerrit-Change-Number: 69047 Gerrit-PatchSet: 3 Gerrit-Owner: 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-Comment-Date: Sun, 13 Nov 2022 01:48:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Felix Singer <felixsinger(a)posteo.net> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: programmer: Drop dead fallback_map() boilerplate
by Felix Singer (Code Review) 12 Nov '22

12 Nov '22

Felix Singer has submitted this change. ( https://review.coreboot.org/c/flashrom/+/68963 ) Change subject: programmer: Drop dead fallback_map() boilerplate ...................................................................... programmer: Drop dead fallback_map() boilerplate The fallback_{un}map() boilerplate code doesn't do anything, merely distracts away from otherwise linear control flow. Just drop it as anything in the future that could need such a thing is free to implement it when required. Change-Id: Ibb7760f807fae040416cef2797a7dbf6572f7df9 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> Reviewed-on: https://review.coreboot.org/c/flashrom/+/68963 Reviewed-by: Angel Pons <th3fanbus(a)gmail.com> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> Reviewed-by: Felix Singer <felixsinger(a)posteo.net> --- M flashrom.c M include/programmer.h M parallel.c M programmer.c 4 files changed, 27 insertions(+), 25 deletions(-) Approvals: build bot (Jenkins): Verified Felix Singer: Looks good to me, approved Angel Pons: Looks good to me, approved diff --git a/flashrom.c b/flashrom.c index 815f489..5483af3 100644 --- a/flashrom.c +++ b/flashrom.c @@ -215,11 +215,15 @@ else if (mst->buses_supported & BUS_NONSPI) map_flash_region = mst->par.map_flash_region; - void *ret; + /* A result of NULL causes mapped addresses to be chip physical + * addresses, assuming only a single region is mapped (the entire flash + * space). Chips with a second region (like a register map) require a + * real memory mapping to distinguish the different ranges. Those chips + * are FWH/LPC, so the bus master provides a real mapping. + */ + void *ret = NULL; if (map_flash_region) ret = map_flash_region(descr, phys_addr, len); - else - ret = fallback_map(descr, phys_addr, len); msg_gspew("%s: mapping %s from 0x%0*" PRIxPTR " to 0x%0*" PRIxPTR "\n", __func__, descr, PRIxPTR_WIDTH, phys_addr, PRIxPTR_WIDTH, (uintptr_t) ret); return ret; @@ -236,8 +240,6 @@ if (unmap_flash_region) unmap_flash_region(virt_addr, len); - else - fallback_unmap(virt_addr, len); msg_gspew("%s: unmapped 0x%0*" PRIxPTR "\n", __func__, PRIxPTR_WIDTH, (uintptr_t)virt_addr); } diff --git a/include/programmer.h b/include/programmer.h index 386e6ec..55e300a 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -439,8 +439,6 @@ int register_par_master(const struct par_master *mst, const enum chipbustype buses, void *data); /* programmer.c */ -void *fallback_map(const char *descr, uintptr_t phys_addr, size_t len); -void fallback_unmap(void *virt_addr, size_t len); void fallback_chip_writew(const struct flashctx *flash, uint16_t val, chipaddr addr); void fallback_chip_writel(const struct flashctx *flash, uint32_t val, chipaddr addr); void fallback_chip_writen(const struct flashctx *flash, const uint8_t *buf, chipaddr addr, size_t len); diff --git a/parallel.c b/parallel.c index e635088..cea9e7a 100644 --- a/parallel.c +++ b/parallel.c @@ -76,7 +76,7 @@ } } - /* Bus masters supporting FWH/LPC cannot use fallback_map(), distinct + /* Bus masters supporting FWH/LPC cannot use chip physical maps, distinct * mappings are needed to support chips with FEATURE_REGISTERMAP */ if ((buses & (BUS_FWH | BUS_LPC)) && !mst->map_flash_region) { diff --git a/programmer.c b/programmer.c index 939d8c2..d4f15a1 100644 --- a/programmer.c +++ b/programmer.c @@ -17,23 +17,6 @@ #include "flash.h" #include "programmer.h" -/* Fallback map() for programmers which don't need special handling */ -void *fallback_map(const char *descr, uintptr_t phys_addr, size_t len) -{ - /* A result of NULL causes mapped addresses to be chip physical - * addresses, assuming only a single region is mapped (the entire flash - * space). Chips with a second region (like a register map) require a - * real memory mapping to distinguish the different ranges. Those chips - * are FWH/LPC, so the bus master provides a real mapping. - */ - return NULL; -} - -/* No-op/fallback unmap() for programmers which don't need special handling */ -void fallback_unmap(void *virt_addr, size_t len) -{ -} - /* Little-endian fallback for drivers not supporting 16 bit accesses */ void fallback_chip_writew(const struct flashctx *flash, uint16_t val, chipaddr addr) -- To view, visit https://review.coreboot.org/c/flashrom/+/68963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibb7760f807fae040416cef2797a7dbf6572f7df9 Gerrit-Change-Number: 68963 Gerrit-PatchSet: 6 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(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-MessageType: merged

1 0

[S] Change in flashrom[master]: programmer: Drop dead fallback_map() boilerplate
by Felix Singer (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/68963 ) Change subject: programmer: Drop dead fallback_map() boilerplate ...................................................................... Patch Set 5: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/68963/comment/d2ee753f_6d984c34 PS4, Line 10: merly > nit: mer*e*ly Done -- To view, visit https://review.coreboot.org/c/flashrom/+/68963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibb7760f807fae040416cef2797a7dbf6572f7df9 Gerrit-Change-Number: 68963 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Comment-Date: Sat, 12 Nov 2022 16:39:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: programmer: Drop dead fallback_map() boilerplate
by Felix Singer (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan. Felix Singer has uploaded a new patch set (#5) to the change originally created by Edward O'Callaghan. ( https://review.coreboot.org/c/flashrom/+/68963 ) Change subject: programmer: Drop dead fallback_map() boilerplate ...................................................................... programmer: Drop dead fallback_map() boilerplate The fallback_{un}map() boilerplate code doesn't do anything, merely distracts away from otherwise linear control flow. Just drop it as anything in the future that could need such a thing is free to implement it when required. Change-Id: Ibb7760f807fae040416cef2797a7dbf6572f7df9 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M flashrom.c M include/programmer.h M parallel.c M programmer.c 4 files changed, 23 insertions(+), 25 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/63/68963/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/68963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibb7760f807fae040416cef2797a7dbf6572f7df9 Gerrit-Change-Number: 68963 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: flashrom.c: Replace 'exit(1)' leaks with return codes on err paths
by Felix Singer (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Edward O'Callaghan, Angel Pons. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69474 ) Change subject: flashrom.c: Replace 'exit(1)' leaks with return codes on err paths ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69474 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie4186a40071e9a7296d601582ff15ad7df09c70a Gerrit-Change-Number: 69474 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> 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-Comment-Date: Sat, 12 Nov 2022 16:35:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: Position heap alloc along side check in compare_range()
by Felix Singer (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Edward O'Callaghan. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69473 ) Change subject: flashrom.c: Position heap alloc along side check in compare_range() ...................................................................... Patch Set 1: Code-Review+1 (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/69473/comment/fc5da205_7fb177cd PS1, Line 451: uint8_t *cmpbuf = malloc(len); I would keep the declaration here and move the malloc down. -- To view, visit https://review.coreboot.org/c/flashrom/+/69473 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0386ac4c09a541cb9a659b2410ce49c3292ecc6e Gerrit-Change-Number: 69473 Gerrit-PatchSet: 1 Gerrit-Owner: 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-Comment-Date: Sat, 12 Nov 2022 16:22:46 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: tree/: Make heap alloc checks err msg consistent
by Felix Singer (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Edward O'Callaghan. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69472 ) Change subject: tree/: Make heap alloc checks err msg consistent ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Patchset: PS1: I am wondering if it makes sense to introduce an inline function or a define for these. -- To view, visit https://review.coreboot.org/c/flashrom/+/69472 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id84a9f15c33781efc494ed36a1c7cec82a0333d6 Gerrit-Change-Number: 69472 Gerrit-PatchSet: 1 Gerrit-Owner: 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-Comment-Date: Sat, 12 Nov 2022 16:17:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: flashrom.c: Drop redundant chip read validation in verify_range()
by Felix Singer (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69471 ) Change subject: flashrom.c: Drop redundant chip read validation in verify_range() ...................................................................... Patch Set 1: Code-Review+2 (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/69471/comment/63240269_cc89ccc7 PS1, Line 11: verfy_range typo: verify (missing i) -- To view, visit https://review.coreboot.org/c/flashrom/+/69471 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ifc57dd89715115e03d013691352463a8b3c0dc52 Gerrit-Change-Number: 69471 Gerrit-PatchSet: 1 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: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sat, 12 Nov 2022 16:13:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: chipdrivers: Rename s/erase_sector_stm50/stm50_sector_erase
by Felix Singer (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Edward O'Callaghan. Felix Singer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69047 ) Change subject: chipdrivers: Rename s/erase_sector_stm50/stm50_sector_erase ...................................................................... Patch Set 3: Code-Review+1 (1 comment) File flashrom.c: https://review.coreboot.org/c/flashrom/+/69047/comment/da9b7312_3978fb7b PS3, Line 377: STM50_SECTOR_ERASE I am wondering if we should go the other way, renaming this to ERASE_SECTOR_STM50, since this would align with the other names above. Same for the follow-up patches, e.g. erase_opaque. Related function names start with the operation type as prefix, as for example write_opaque or read_opaque. -- To view, visit https://review.coreboot.org/c/flashrom/+/69047 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ia916b5782abf04499ab46a540820acbe4a171068 Gerrit-Change-Number: 69047 Gerrit-PatchSet: 3 Gerrit-Owner: 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-Comment-Date: Sat, 12 Nov 2022 14:57:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: cli_classic.c: Be consistent with pointer types
by Arthur Heymans (Code Review) 12 Nov '22

12 Nov '22

Attention is currently required from: Nico Huber, Angel Pons, Nikolai Artemiev, Anastasia Klimchuk. Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69451 ) Change subject: cli_classic.c: Be consistent with pointer types ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/69451 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ie8f5bc0d9296f7c6b8f8a351b53052f5fe86b09d Gerrit-Change-Number: 69451 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: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Sat, 12 Nov 2022 08:48:05 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit November 2022