January 2023 - flashrom-gerrit

[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

[M] Change in flashrom[master]: tree/: Drop default_spi_probe_opcode for NULL case
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Thomas Heijligen, Aarya, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70264 ) Change subject: tree/: Drop default_spi_probe_opcode for NULL case ...................................................................... Patch Set 5: (2 comments) Patchset: PS1: > I looked into remaining Aarya's patches, and it seems like there won't be conflicts, my understandin […] Correct. Patchset: PS4: > Needs to be rebased, otherwise looks good (hope my other comment is correct). Done. -- To view, visit https://review.coreboot.org/c/flashrom/+/70264 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id502c5d2596ad1db52faf05723083620e4c52c12 Gerrit-Change-Number: 70264 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> 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-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Thu, 12 Jan 2023 01:29:17 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: tree/: Drop default_spi_probe_opcode for NULL case
by Edward O'Callaghan (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Aarya. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, Aarya, Anastasia Klimchuk, Peter Marheine, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/70264 to look at the new patch set (#5). Change subject: tree/: Drop default_spi_probe_opcode for NULL case ...................................................................... tree/: Drop default_spi_probe_opcode for NULL case A NULL func pointer is necessary and sufficient for the condition `NULL func pointer => true' as to not need this boilerplate as it implies default behaviour of a supported opcode within the `check_block_eraser()` match supported loop. Ran; ``` $ find . -name '*.[c,h]' -exec sed -i '/.probe_opcode = default_spi_probe_opcode,/d' '{}' \; ``` Change-Id: Id502c5d2596ad1db52faf05723083620e4c52c12 Signed-off-by: Edward O'Callaghan <quasisec(a)google.com> --- M bitbang_spi.c M buspirate_spi.c M ch341a_spi.c M dediprog.c M digilent_spi.c M dirtyjtag_spi.c M flashrom.c M ft2232_spi.c M include/chipdrivers.h M include/programmer.h M it87spi.c M jlink_spi.c M linux_spi.c M mediatek_i2c_spi.c M mstarddc_spi.c M ni845x_spi.c M parade_lspcon.c M pickit2_spi.c M raiden_debug_spi.c M realtek_mst_i2c_spi.c M sb600spi.c M serprog.c M spi.c M spi25_statusreg.c M stlinkv3_spi.c M usbblaster_spi.c M wbsio_spi.c 27 files changed, 30 insertions(+), 32 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/64/70264/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/70264 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id502c5d2596ad1db52faf05723083620e4c52c12 Gerrit-Change-Number: 70264 Gerrit-PatchSet: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: tree/: Drop default_spi_probe_opcode for NULL case
by Anastasia Klimchuk (Code Review) 12 Jan '23

12 Jan '23

Attention is currently required from: Thomas Heijligen, Edward O'Callaghan, Aarya. Anastasia Klimchuk has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/70264 ) Change subject: tree/: Drop default_spi_probe_opcode for NULL case ...................................................................... Patch Set 4: Code-Review+2 (2 comments) Patchset: PS1: > rebased, it should be fine to merge now? I looked into remaining Aarya's patches, and it seems like there won't be conflicts, my understanding that the ones that had overlap in code has already been merged. Thomas/Edward, please correct me if I am wrong. Patchset: PS4: Needs to be rebased, otherwise looks good (hope my other comment is correct). -- To view, visit https://review.coreboot.org/c/flashrom/+/70264 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id502c5d2596ad1db52faf05723083620e4c52c12 Gerrit-Change-Number: 70264 Gerrit-PatchSet: 4 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Aarya <aarya.chaumal(a)gmail.com> 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-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Aarya <aarya.chaumal(a)gmail.com> Gerrit-Comment-Date: Thu, 12 Jan 2023 01:09:01 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit January 2023