- flashrom-gerrit - mail.coreboot.org

[M] Change in flashrom[master]: tests/chip: Add non-aligned write within a region unit-test
by Sam McNally (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Evan Benn. Sam McNally has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71659 ) Change subject: tests/chip: Add non-aligned write within a region unit-test ...................................................................... Patch Set 5: Code-Review+2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71659 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Gerrit-Change-Number: 71659 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Thu, 12 Jan 2023 03:26:03 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: dmi.c: Pass is_laptop by ref into dmi
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Felix Singer, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71622 ) Change subject: dmi.c: Pass is_laptop by ref into dmi ...................................................................... Patch Set 2: (2 comments) File dmi.c: https://review.coreboot.org/c/flashrom/+/71622/comment/a9ea2717_0d92df76 PS1, Line 158: *is_laptop = 2; > For another patch: This should be an enum. I improved upon the patch to at least make it a return value. Thanks for the review. https://review.coreboot.org/c/flashrom/+/71622/comment/c6b4986d_89ed23d7 PS1, Line 402: is_laptop > nit: update comment too? Done -- To view, visit https://review.coreboot.org/c/flashrom/+/71622 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d5ad6c0623269492d775a99a947fd6fe26c5f91 Gerrit-Change-Number: 71622 Gerrit-PatchSet: 2 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-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 12 Jan 2023 03:19:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: dmi.c: Pass is_laptop by ref into dmi
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Felix Singer, Edward O'Callaghan. Hello Felix Singer, build bot (Jenkins), I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71622 to look at the new patch set (#2). Change subject: dmi.c: Pass is_laptop by ref into dmi ...................................................................... dmi.c: Pass is_laptop by ref into dmi Prefix the remaining global cases with `g_` to avoid shadowing issues and for easy greping. Change-Id: I3d5ad6c0623269492d775a99a947fd6fe26c5f91 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M board_enable.c M dmi.c M include/programmer.h M internal.c 4 files changed, 45 insertions(+), 30 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/22/71622/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71622 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I3d5ad6c0623269492d775a99a947fd6fe26c5f91 Gerrit-Change-Number: 71622 Gerrit-PatchSet: 2 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-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Attention: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[S] Change in flashrom[master]: programmer: Add pre-init hook
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71579 ) Change subject: programmer: Add pre-init hook ...................................................................... Patch Set 1: (2 comments) File flashrom.c: https://review.coreboot.org/c/flashrom/+/71579/comment/be9e351b_4a6ca29d PS1, Line 164: Initialising > Very pedantic, buuut... […] It would be really nice not to erase my language and history with Americanisation of the British language, I prefer not to use a z. flashrom needs a lot of grammatical work in any case. File include/programmer.h: https://review.coreboot.org/c/flashrom/+/71579/comment/5c1f3cb9_663b57e8 PS1, Line 51: struct programmer_cfg *cfg > The lack of `const` is intentional, right? It is yes. I am going to mark these patches as WIP and put them in a topic. I was trying to deal with how internal requires board init first that mutates state so I was thinking the mutating part can be done in the pre phase. It is quite tricky how the code is. -- To view, visit https://review.coreboot.org/c/flashrom/+/71579 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Icb9ba758650fbe3731021d6b8f95c46ea0d24af1 Gerrit-Change-Number: 71579 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 12 Jan 2023 03:03:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: internal.c: Factor out laptop alerts into helper func
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Felix Singer, Angel Pons. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71578 ) Change subject: internal.c: Factor out laptop alerts into helper func ...................................................................... Patch Set 1: (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/71578/comment/fd86f654_1052ce47 PS1, Line 9: unfortant > Did you mean `unfortunate`? Done. Thanks for the review. File internal.c: https://review.coreboot.org/c/flashrom/+/71578/comment/7a299383_879178a4 PS1, Line 167: > Add "from parameters", at first it seemed like this is about the function's name. […] Done https://review.coreboot.org/c/flashrom/+/71578/comment/60a258db_62565432 PS1, Line 170: /* Report if a non-whitelisted laptop is detected that likely uses a legacy bus. */ > Maybe keep this comment on the call site Done -- To view, visit https://review.coreboot.org/c/flashrom/+/71578 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8eea91012e6539b4fdf5d49a75a9cb48bb8a57ca Gerrit-Change-Number: 71578 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Thu, 12 Jan 2023 02:57:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: internal.c: Factor out laptop alerts into helper func
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Felix Singer, Edward O'Callaghan. Hello Felix Singer, build bot (Jenkins), Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71578 to look at the new patch set (#2). Change subject: internal.c: Factor out laptop alerts into helper func ...................................................................... internal.c: Factor out laptop alerts into helper func Minor however a unfortunate '_' suffix is temporarily needed to skirt around global variable shadowing. Change-Id: I8eea91012e6539b4fdf5d49a75a9cb48bb8a57ca Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M internal.c 1 file changed, 44 insertions(+), 25 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/78/71578/2 -- To view, visit https://review.coreboot.org/c/flashrom/+/71578 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I8eea91012e6539b4fdf5d49a75a9cb48bb8a57ca Gerrit-Change-Number: 71578 Gerrit-PatchSet: 2 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: internal: Move parallel logic into internal_par implementation
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/71834 ) Change subject: internal: Move parallel logic into internal_par implementation ...................................................................... internal: Move parallel logic into internal_par implementation The parallel internal programmer is its own implementation. Move it and call into it from the top-level internal.c programmer implementation. Change-Id: Idabeceb59a36680f5fbb45d3ee4bd5dbf837373b Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M Makefile M include/programmer.h M internal.c A internal_par.c M meson.build 5 files changed, 101 insertions(+), 61 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/34/71834/1 diff --git a/Makefile b/Makefile index 425b58c..93fd615 100644 --- a/Makefile +++ b/Makefile @@ -583,13 +583,13 @@ ifeq ($(CONFIG_INTERNAL) $(CONFIG_INTERNAL_X86), yes yes) FEATURE_FLAGS += -D'CONFIG_INTERNAL=1' PROGRAMMER_OBJS += processor_enable.o chipset_enable.o board_enable.o cbtable.o \ - internal.o it87spi.o sb600spi.o amd_imc.o wbsio_spi.o mcp6x_spi.o \ + internal.o internal_par.o it87spi.o sb600spi.o amd_imc.o wbsio_spi.o mcp6x_spi.o \ ichspi.o dmi.o known_boards.o endif else ifeq ($(CONFIG_INTERNAL), yes) FEATURE_FLAGS += -D'CONFIG_INTERNAL=1' -PROGRAMMER_OBJS += processor_enable.o chipset_enable.o board_enable.o cbtable.o internal.o known_boards.o +PROGRAMMER_OBJS += processor_enable.o chipset_enable.o board_enable.o cbtable.o internal.o internal_par.o known_boards.o endif endif diff --git a/include/programmer.h b/include/programmer.h index 281a4f6..ca43de0 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -388,7 +388,8 @@ /* mcp6x_spi.c */ int mcp6x_spi_init(int want_spi); - +/* internal_par.c */ +void internal_par_init(enum chipbustype buses); /* sb600spi.c */ int sb600_probe_spi(const struct programmer_cfg *cfg, struct pci_dev *dev); diff --git a/internal.c b/internal.c index f21c4fc..cb0c19c 100644 --- a/internal.c +++ b/internal.c @@ -18,9 +18,9 @@ #include <string.h> #include <stdbool.h> #include <stdlib.h> + #include "flash.h" #include "programmer.h" -#include "hwaccess_physmap.h" #include "platform/pci.h" #if defined(__i386__) || defined(__x86_64__) @@ -63,61 +63,6 @@ #endif -static void internal_chip_writeb(const struct flashctx *flash, uint8_t val, - chipaddr addr) -{ - mmio_writeb(val, (void *) addr); -} - -static void internal_chip_writew(const struct flashctx *flash, uint16_t val, - chipaddr addr) -{ - mmio_writew(val, (void *) addr); -} - -static void internal_chip_writel(const struct flashctx *flash, uint32_t val, - chipaddr addr) -{ - mmio_writel(val, (void *) addr); -} - -static uint8_t internal_chip_readb(const struct flashctx *flash, - const chipaddr addr) -{ - return mmio_readb((void *) addr); -} - -static uint16_t internal_chip_readw(const struct flashctx *flash, - const chipaddr addr) -{ - return mmio_readw((void *) addr); -} - -static uint32_t internal_chip_readl(const struct flashctx *flash, - const chipaddr addr) -{ - return mmio_readl((void *) addr); -} - -static void internal_chip_readn(const struct flashctx *flash, uint8_t *buf, - const chipaddr addr, size_t len) -{ - mmio_readn((void *)addr, buf, len); - return; -} - -static const struct par_master par_master_internal = { - .map_flash_region = physmap, - .unmap_flash_region = physunmap, - .chip_readb = internal_chip_readb, - .chip_readw = internal_chip_readw, - .chip_readl = internal_chip_readl, - .chip_readn = internal_chip_readn, - .chip_writeb = internal_chip_writeb, - .chip_writew = internal_chip_writew, - .chip_writel = internal_chip_writel, -}; - static int get_params(const struct programmer_cfg *cfg, bool *boardenable, bool *boardmismatch, bool *force_laptop, bool *not_a_laptop, @@ -307,8 +252,7 @@ } #endif - if (internal_buses_supported & BUS_NONSPI) - register_par_master(&par_master_internal, internal_buses_supported, NULL); + internal_par_init(internal_buses_supported); /* Report if a non-whitelisted laptop is detected that likely uses a legacy bus. */ if (is_laptop && !laptop_ok) { diff --git a/internal_par.c b/internal_par.c new file mode 100644 index 0000000..e8e387c --- /dev/null +++ b/internal_par.c @@ -0,0 +1,79 @@ +/* + * This file is part of the flashrom project. + * + * Copyright (C) 2009 Carl-Daniel Hailfinger + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include "programmer.h" +#include "hwaccess_physmap.h" + +static void internal_chip_writeb(const struct flashctx *flash, uint8_t val, + chipaddr addr) +{ + mmio_writeb(val, (void *) addr); +} + +static void internal_chip_writew(const struct flashctx *flash, uint16_t val, + chipaddr addr) +{ + mmio_writew(val, (void *) addr); +} + +static void internal_chip_writel(const struct flashctx *flash, uint32_t val, + chipaddr addr) +{ + mmio_writel(val, (void *) addr); +} + +static uint8_t internal_chip_readb(const struct flashctx *flash, + const chipaddr addr) +{ + return mmio_readb((void *) addr); +} + +static uint16_t internal_chip_readw(const struct flashctx *flash, + const chipaddr addr) +{ + return mmio_readw((void *) addr); +} + +static uint32_t internal_chip_readl(const struct flashctx *flash, + const chipaddr addr) +{ + return mmio_readl((void *) addr); +} + +static void internal_chip_readn(const struct flashctx *flash, uint8_t *buf, + const chipaddr addr, size_t len) +{ + mmio_readn((void *)addr, buf, len); + return; +} + +static const struct par_master par_master_internal = { + .map_flash_region = physmap, + .unmap_flash_region = physunmap, + .chip_readb = internal_chip_readb, + .chip_readw = internal_chip_readw, + .chip_readl = internal_chip_readl, + .chip_readn = internal_chip_readn, + .chip_writeb = internal_chip_writeb, + .chip_writew = internal_chip_writew, + .chip_writel = internal_chip_writel, +}; + +void internal_par_init(enum chipbustype buses) +{ + if (buses & BUS_NONSPI) + register_par_master(&par_master_internal, internal_buses_supported, NULL); +} diff --git a/meson.build b/meson.build index 27a284e..a673b81 100644 --- a/meson.build +++ b/meson.build @@ -245,6 +245,7 @@ 'board_enable.c', 'cbtable.c', 'internal.c', + 'internal_par.c', 'it87spi.c', 'sb600spi.c', 'amd_imc.c', @@ -259,6 +260,7 @@ 'cbtable.c', 'chipset_enable.c', 'internal.c', + 'internal_par.c', 'processor_enable.c', 'pcidev.c', 'known_boards.c', -- To view, visit https://review.coreboot.org/c/flashrom/+/71834 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idabeceb59a36680f5fbb45d3ee4bd5dbf837373b Gerrit-Change-Number: 71834 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: newchange

1 0

[M] Change in flashrom[master]: tests/chip: Add non-aligned write within a region unit-test
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Sam McNally, Anastasia Klimchuk, Evan Benn. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71659 ) Change subject: tests/chip: Add non-aligned write within a region unit-test ...................................................................... Patch Set 5: (1 comment) Commit Message: https://review.coreboot.org/c/flashrom/+/71659/comment/2ce4f567_3a9e82c8 PS2, Line 7: tests/: Add subregion alignment unit-test > What does non-aligned mean? The commit message can match the test name https://github.com/flashrom/flashrom/blob/master/flashrom.c#L1426 everything renamed to be consistent with the term "non-aligned" -- To view, visit https://review.coreboot.org/c/flashrom/+/71659 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Gerrit-Change-Number: 71659 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Thu, 12 Jan 2023 02:08:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests/chip: Add non-aligned write within a region unit-test
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Sam McNally, Anastasia Klimchuk, Evan Benn. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/71659 ) Change subject: tests/chip: Add non-aligned write within a region unit-test ...................................................................... Patch Set 5: (6 comments) File tests/chip.c: https://review.coreboot.org/c/flashrom/+/71659/comment/170e0c22_0910e6ef PS2, Line 469: * {MOCK_CHIP_SUBREGION_CONTENTS} [..] {MOCK_CHIP_SUBREGION_CONTENTS}. > I dont understand this: […] Done File tests/chip.c: https://review.coreboot.org/c/flashrom/+/71659/comment/93405606_c3190b1c PS4, Line 443: /* FIXME: MOCK_CHIP_CONTENT is buggy within setup_chip, it should also > should this comment be here? if its buggy does the test work? what bug? yes the test works, its a bug to do with setup_chip(). I don't know why, so I just left a comment here. https://review.coreboot.org/c/flashrom/+/71659/comment/aa83030b_02d88d6b PS4, Line 457: /* Expect to verify the write completed successfuly within the region. */ > is the presence of these flags important for catching the bug? […] Done https://review.coreboot.org/c/flashrom/+/71659/comment/77c0e8b0_f8e2e7e1 PS4, Line 469: * Create subregion smaller than erase granularity of chip. > subregion -> region whole file replace please Done https://review.coreboot.org/c/flashrom/+/71659/comment/ab5c0463_0d6e8f4b PS4, Line 498: /* .. */ > remove Done https://review.coreboot.org/c/flashrom/+/71659/comment/b3b4eece_232192e4 PS4, Line 500: /* Expect a verification pass that content within the region however > this doesnt make sense Done -- To view, visit https://review.coreboot.org/c/flashrom/+/71659 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Gerrit-Change-Number: 71659 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Evan Benn <evanbenn(a)google.com> Gerrit-Comment-Date: Thu, 12 Jan 2023 02:03:47 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Evan Benn <evanbenn(a)google.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tests/chip: Add non-aligned write within a region unit-test
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Sam McNally, Edward O'Callaghan, Anastasia Klimchuk. Hello Sam McNally, build bot (Jenkins), Evan Benn, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/71659 to look at the new patch set (#5). Change subject: tests/chip: Add non-aligned write within a region unit-test ...................................................................... tests/chip: Add non-aligned write within a region unit-test A written region that is sized below that of the erasure granularity can result in a incorrectly read region that does not include prior content within the region before the write op. This was dealt with in ChromeOS downstream by expanding out the read to match the erase granularity however does not seem to impact upstream. Add a unit-test to avoid regression as this is important behaviour to cover. Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M tests/chip.c M tests/tests.c M tests/tests.h 3 files changed, 110 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/59/71659/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/71659 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f Gerrit-Change-Number: 71659 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Evan Benn <evanbenn(a)google.com> Gerrit-Reviewer: Sam McNally <sammc(a)google.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Attention: Sam McNally <sammc(a)google.com> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: newpatchset

1 0

2025

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit