February 2023 - flashrom-gerrit

[M] Change in flashrom[master]: asm106x: add programmer for ASM106x SATA controllers
by Thomas Heijligen (Code Review) 15 Feb '23

15 Feb '23

Attention is currently required from: Felix Singer, Edward O'Callaghan, Alex Badea. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73037 ) Change subject: asm106x: add programmer for ASM106x SATA controllers ...................................................................... Patch Set 5: Code-Review+2 (6 comments) Patchset: PS5: Thanks for the detailed answers. From my side it looks nearly fine. I've added a few nits. Beside that I've ordered one of those controllers and will test it in the next days. Edward, do you see something missing? File asm106x.c: https://review.coreboot.org/c/flashrom/+/73037/comment/b11e9d7b_379b19ef PS3, Line 35: {PCI_VENDOR_ID_ASMEDIA, 0x0612, OK, "ASMedia", "ASM106x"}, > Ack Ok, I'm fine with it. If someone has a chip with the other PCI-Id we can clarify it then. File asm106x.c: https://review.coreboot.org/c/flashrom/+/73037/comment/ceda25b2_76854e05 PS5, Line 47: x Please use `inttype.h` macros here for fixed length integer. We have problems with those on some compilers and a proper syntax for them comes first with c23. So for uint8_t as hex it gets ``` msg_pdbg2("asm106x status %#02"PRIx8" tries %d\n", val, timeout); ``` https://review.coreboot.org/c/flashrom/+/73037/comment/0bbecbdf_161cd28d PS5, Line 57: msg_pdbg("asm106x timed out, ctrl 0x%02x\n", val); Same here `... ctrl %#02"PRIx8"\n"` https://review.coreboot.org/c/flashrom/+/73037/comment/e9288783_f380cfe1 PS5, Line 81: msg_pdbg2("asm106x write 0x%08x chunk %u\n", val, chunk); `write %#08"PRIx32" chunk` https://review.coreboot.org/c/flashrom/+/73037/comment/838c006a_c83343a9 PS5, Line 99: msg_pdbg2("asm106x read 0x%08x chunk %u\n", val, chunk); `read %#08"PRIx32" chunk` -- To view, visit https://review.coreboot.org/c/flashrom/+/73037 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I591b117be911bdb8249247c20530c1cf70f6e70d Gerrit-Change-Number: 73037 Gerrit-PatchSet: 5 Gerrit-Owner: Alex Badea <vamposdecampos(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Alex Badea <vamposdecampos(a)gmail.com> Gerrit-Comment-Date: Wed, 15 Feb 2023 10:49:22 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Alex Badea <vamposdecampos(a)gmail.com> Gerrit-MessageType: comment

1 0

[S] Change in flashrom[master]: jedec.c: Fold up dst into toggle_ready_jedec()
by Edward O'Callaghan (Code Review) 15 Feb '23

15 Feb '23

Attention is currently required from: Stefan Reinauer, Anastasia Klimchuk. Edward O'Callaghan has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72691 ) Change subject: jedec.c: Fold up dst into toggle_ready_jedec() ...................................................................... Patch Set 1: (1 comment) File jedec.c: https://review.coreboot.org/c/flashrom/+/72691/comment/71954a72_fddba77f PS1, Line 72: const chipaddr dst = flash->virtual_memory; > Just wondering, I see that all existing callsites have dst as `flash->virtual_memory` but is this al […] Not according to the current code. Either way it already has flashctx as a operand to the function. -- To view, visit https://review.coreboot.org/c/flashrom/+/72691 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib7a3fdbc6e0a888093dc8da6f5567a7301ec5040 Gerrit-Change-Number: 72691 Gerrit-PatchSet: 1 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Stefan Reinauer <stefan.reinauer(a)coreboot.org> Gerrit-Attention: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Comment-Date: Wed, 15 Feb 2023 06:34:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-MessageType: comment

1 0

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

15 Feb '23

Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/71834 ) ( 2 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. )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> Reviewed-on: https://review.coreboot.org/c/flashrom/+/71834 Reviewed-by: Anastasia Klimchuk <aklm(a)chromium.org> Reviewed-by: Thomas Heijligen <src(a)posteo.de> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M Makefile M include/programmer.h M internal.c A internal_par.c M meson.build 5 files changed, 105 insertions(+), 60 deletions(-) Approvals: build bot (Jenkins): Verified Thomas Heijligen: Looks good to me, approved Anastasia Klimchuk: Looks good to me, approved diff --git a/Makefile b/Makefile index b37e807..16af214 100644 --- a/Makefile +++ b/Makefile @@ -584,14 +584,14 @@ 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 superio.o amd_imc.o wbsio_spi.o mcp6x_spi.o \ + internal.o internal_par.o it87spi.o sb600spi.o superio.o amd_imc.o wbsio_spi.o mcp6x_spi.o \ ichspi.o dmi.o known_boards.o ACTIVE_PROGRAMMERS += internal 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 ACTIVE_PROGRAMMERS += internal endif endif diff --git a/include/programmer.h b/include/programmer.h index db32b2c..2a4ca76 100644 --- a/include/programmer.h +++ b/include/programmer.h @@ -387,7 +387,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 07126bf..c75b472 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__) @@ -34,60 +34,6 @@ enum chipbustype internal_buses_supported = BUS_NONE; -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, @@ -309,8 +255,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. */ report_nonwl_laptop_detected(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 2575fe1..d78aff9 100644 --- a/meson.build +++ b/meson.build @@ -246,6 +246,7 @@ 'board_enable.c', 'cbtable.c', 'internal.c', + 'internal_par.c', 'it87spi.c', 'sb600spi.c', 'superio.c', @@ -261,6 +262,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: 5 Gerrit-Owner: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-MessageType: merged

1 0

[M] Change in flashrom[master]: asm106x: add programmer for ASM106x SATA controllers
by Alex Badea (Code Review) 15 Feb '23

15 Feb '23

Attention is currently required from: Felix Singer, Thomas Heijligen. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/73037 to look at the new patch set (#5). Change subject: asm106x: add programmer for ASM106x SATA controllers ...................................................................... asm106x: add programmer for ASM106x SATA controllers The ASMedia ASM106x series is a PCIe-SATA controller chip. It supports an attached SPI flash chip that can contain configuration and PCI option ROM. The interface is a simple shifter accessed via PCI config space, up to 4 bytes at a time. Add a programmer driver for it. Tested on a G536PCE1061V11 IO-PCE1061-V1.1 PCIe card, and a MPCE2ST-A01 VER006S mini-PCIe card, both with chips marked ASM1061, both enumerate as: 01:00.0 SATA controller [0106]: ASMedia Technology Inc. ASM1062 Serial ATA Controller [1b21:0612] (rev 02) (prog-if 01 [AHCI 1.0]) Subsystem: ASMedia Technology Inc. ASM1062 Serial ATA Controller [1b21:1060] Change-Id: I591b117be911bdb8249247c20530c1cf70f6e70d Signed-off-by: Alex Badea <vamposdecampos(a)gmail.com> --- M Makefile A asm106x.c M flashrom.8.tmpl M include/programmer.h M meson.build M meson_options.txt M programmer_table.c M test_build.sh 8 files changed, 213 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/73037/5 -- To view, visit https://review.coreboot.org/c/flashrom/+/73037 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I591b117be911bdb8249247c20530c1cf70f6e70d Gerrit-Change-Number: 73037 Gerrit-PatchSet: 5 Gerrit-Owner: Alex Badea <vamposdecampos(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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: asm106x: add programmer for ASM106x SATA controllers
by Alex Badea (Code Review) 15 Feb '23

15 Feb '23

Attention is currently required from: Felix Singer, Thomas Heijligen. Alex Badea has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73037 ) Change subject: asm106x: add programmer for ASM106x SATA controllers ...................................................................... Patch Set 4: (1 comment) File asm106x.c: https://review.coreboot.org/c/flashrom/+/73037/comment/bd6e9a0f_a9dca2a8 PS3, Line 35: {PCI_VENDOR_ID_ASMEDIA, 0x0612, OK, "ASMedia", "ASM106x"}, > Well, it's interesting. […] Ack -- To view, visit https://review.coreboot.org/c/flashrom/+/73037 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I591b117be911bdb8249247c20530c1cf70f6e70d Gerrit-Change-Number: 73037 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Badea <vamposdecampos(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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 15 Feb 2023 02:49:19 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Alex Badea <vamposdecampos(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: asm106x: add programmer for ASM106x SATA controllers
by Alex Badea (Code Review) 15 Feb '23

15 Feb '23

Attention is currently required from: Felix Singer, Thomas Heijligen. Alex Badea has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73037 ) Change subject: asm106x: add programmer for ASM106x SATA controllers ...................................................................... Patch Set 4: (1 comment) Patchset: PS3: > Ack Not sure if product URLs should go in commit message. For posterity, these two: https://www.ebay.com/itm/204081736637 https://www.amazon.com/gp/product/B09B257HW6 -- To view, visit https://review.coreboot.org/c/flashrom/+/73037 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I591b117be911bdb8249247c20530c1cf70f6e70d Gerrit-Change-Number: 73037 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Badea <vamposdecampos(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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 15 Feb 2023 02:48:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Comment-In-Reply-To: Alex Badea <vamposdecampos(a)gmail.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: asm106x: add programmer for ASM106x SATA controllers
by Alex Badea (Code Review) 15 Feb '23

15 Feb '23

Attention is currently required from: Felix Singer, Thomas Heijligen. Alex Badea has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/73037 ) Change subject: asm106x: add programmer for ASM106x SATA controllers ...................................................................... Patch Set 3: (7 comments) Patchset: PS3: > Hi Alex, thank you for the contribution! […] Ack File asm106x.c: https://review.coreboot.org/c/flashrom/+/73037/comment/d300626b_1b94e31e PS3, Line 35: {PCI_VENDOR_ID_ASMEDIA, 0x0612, OK, "ASMedia", "ASM106x"}, > This PCI-Id is assigned to the ASM1062 Serial ATA Controller, […] Well, it's interesting. From driver blobs[1] linked from forums[2] it looks like this: $ grep -rh VEN.*DeviceDesc.*\" . | sort -u PCI\VEN_1B21&DEV_0601.DeviceDesc ="Asmedia 106x SATA Controller" PCI\VEN_1B21&DEV_0602.DeviceDesc ="Asmedia 106x SATA Controller" PCI\VEN_1B21&DEV_0611.DeviceDesc ="Asmedia 106x SATA Controller" PCI\VEN_1B21&DEV_0612.DeviceDesc ="Asmedia 106x SATA Controller" PCI\VEN_1B21&DEV_0614.DeviceDesc ="Asmedia SATA Express Controller" PCI\VEN_1B21&DEV_0615.DeviceDesc ="Asmedia SATA Express Controller" PCI\VEN_1B21&DEV_0620.DeviceDesc ="Asmedia 106x SATA Controller" PCI\VEN_1B21&DEV_0621.DeviceDesc ="Asmedia 106x SATA/RAID Controller" PCI\VEN_1B21&DEV_0622.DeviceDesc ="Asmedia 106x SATA/RAID Controller" PCI\VEN_1B21&DEV_0624.DeviceDesc ="Asmedia 106x SATA/RAID Controller" PCI\VEN_1B21&DEV_0625.DeviceDesc ="Asmedia 106x SATA/RAID Controller" This kernel patch[3] suggests 0x0611 and 0x0612 are both ASM1061. Either way, I'm not sure if driver compatibility translates into SPI interface compatibility as well. My boards have ASM1061 chips on them and they both report 0x0612, which is what I've declared here. Let me know if I should add the others too (as "NT"). [1] http://ableconn.com/support_1.php?gid=120 [2] https://www.techpowerup.com/forums/threads/latest-driver-and-firmware-for-a… [3] https://patchwork.kernel.org/project/linux-pci/patch/1315453426-8796-1-git-… https://review.coreboot.org/c/flashrom/+/73037/comment/7de74237_a053f99b PS3, Line 51: programmer_delay(flash, 10); > `programmer_delay()` should be used outside of the programmer. […] Done https://review.coreboot.org/c/flashrom/+/73037/comment/40a74070_b8aac08b PS3, Line 59: if (pval) > You could move this 'if, assignment, return' where the break is. […] Done https://review.coreboot.org/c/flashrom/+/73037/comment/f442be14_de7d0d7d PS3, Line 74: writecnt, readcnt); > This can be moved in the line above. Done https://review.coreboot.org/c/flashrom/+/73037/comment/fea7da31_c0a735e9 PS3, Line 115: ctrl | ASM106X_CTRL_CSN); > This function call can be in one line Done File meson.build: https://review.coreboot.org/c/flashrom/+/73037/comment/f8b28cb2_e1d6fbbb PS3, Line 152: 'cpu_families' : cpus_port_io, > This line should be removed. […] Done -- To view, visit https://review.coreboot.org/c/flashrom/+/73037 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I591b117be911bdb8249247c20530c1cf70f6e70d Gerrit-Change-Number: 73037 Gerrit-PatchSet: 3 Gerrit-Owner: Alex Badea <vamposdecampos(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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Wed, 15 Feb 2023 02:47:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: asm106x: add programmer for ASM106x SATA controllers
by Alex Badea (Code Review) 15 Feb '23

15 Feb '23

Attention is currently required from: Felix Singer, Alex Badea. Hello Felix Singer, build bot (Jenkins), Thomas Heijligen, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/73037 to look at the new patch set (#4). Change subject: asm106x: add programmer for ASM106x SATA controllers ...................................................................... asm106x: add programmer for ASM106x SATA controllers The ASMedia ASM106x series is a PCIe-SATA controller chip. It supports an attached SPI flash chip that can contain configuration and PCI option ROM. The interface is a simple shifter accessed via PCI config space, up to 4 bytes at a time. Add a programmer driver for it. Tested on a PCIe and a mini-PCIe card, both with chips marked ASM1061, both enumerate as: 01:00.0 SATA controller [0106]: ASMedia Technology Inc. ASM1062 Serial ATA Controller [1b21:0612] (rev 02) (prog-if 01 [AHCI 1.0]) Subsystem: ASMedia Technology Inc. ASM1062 Serial ATA Controller [1b21:1060] Change-Id: I591b117be911bdb8249247c20530c1cf70f6e70d Signed-off-by: Alex Badea <vamposdecampos(a)gmail.com> --- M Makefile A asm106x.c M flashrom.8.tmpl M include/programmer.h M meson.build M meson_options.txt M programmer_table.c M test_build.sh 8 files changed, 212 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/37/73037/4 -- To view, visit https://review.coreboot.org/c/flashrom/+/73037 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I591b117be911bdb8249247c20530c1cf70f6e70d Gerrit-Change-Number: 73037 Gerrit-PatchSet: 4 Gerrit-Owner: Alex Badea <vamposdecampos(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: Felix Singer <felixsinger(a)posteo.net> Gerrit-Attention: Alex Badea <vamposdecampos(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[M] Change in flashrom[master]: writeprotect: Add function to get register values and WP bit masks
by Nikolai Artemiev (Code Review) 14 Feb '23

14 Feb '23

Attention is currently required from: Edward O'Callaghan, Anastasia Klimchuk, Sergii Dmytruk. Nikolai Artemiev has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/69847 ) Change subject: writeprotect: Add function to get register values and WP bit masks ...................................................................... Patch Set 8: (3 comments) This change is ready for review. Commit Message: https://review.coreboot.org/c/flashrom/+/69847/comment/9b51163e_7656cb26 PS5, Line 7: writeprotect: Add function to get register values and WP bit masks > Is this to allow updating WP registers via some non-standard method? Would be nice to have outline […] It will allow us to more thoroughly qualify flash chips once flashrom_tester is able to query the register values during testing. I've uploaded a WIP commit to add a libflashrom interface as well in CB:73053. File tests/chip_wp.c: https://review.coreboot.org/c/flashrom/+/69847/comment/4e669625_91f5162f PS5, Line 363: strdup( > you do not need a heap allocation, just a `const char *` Done https://review.coreboot.org/c/flashrom/+/69847/comment/1020316e_60eb7497 PS5, Line 396: free(param_dup); > drop Done -- To view, visit https://review.coreboot.org/c/flashrom/+/69847 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ib2a47153b230c9f82bb4eca357c335f2abbacc20 Gerrit-Change-Number: 69847 Gerrit-PatchSet: 8 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> 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-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 14 Feb 2023 23:50:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Edward O'Callaghan <quasisec(a)chromium.org> Comment-In-Reply-To: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-MessageType: comment

1 0

[M] Change in flashrom[master]: Add support for VIA VL805 USB Controller flashing
by ChrisEric1 CECL (Code Review) 14 Feb '23

14 Feb '23

Attention is currently required from: Thomas Heijligen, Nicholas Chin. ChrisEric1 CECL has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/72057 ) Change subject: Add support for VIA VL805 USB Controller flashing ...................................................................... Patch Set 15: (1 comment) File vl805.c: https://review.coreboot.org/c/flashrom/+/72057/comment/42f67230_98f40e3c PS15, Line 43: > Should this be static struct instead? "No, as this is just a type declaration and thus static doesn't have an effect If it were struct vl805_spi_data { struct pci_dev *dev; } vl805_data; then that would define a variable called vl805_data of type struct vl805_spi_data (similar to something like int i = 1; and then the static keyword would make a difference. But vl805_data would be global which we're trying to avoid" -- To view, visit https://review.coreboot.org/c/flashrom/+/72057 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I71435afcacdf97e14d627e35bce3d29de9657f38 Gerrit-Change-Number: 72057 Gerrit-PatchSet: 15 Gerrit-Owner: ChrisEric1 CECL <christopherericlentocha(a)gmail.com> Gerrit-Reviewer: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Nicholas Chin <nic.c3.14(a)gmail.com> Gerrit-Comment-Date: Tue, 14 Feb 2023 19:46:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: ChrisEric1 CECL <christopherericlentocha(a)gmail.com> Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit February 2023