December 2021 - flashrom-gerrit

Change in flashrom[master]: Makefile: Make pkg-config mandatory to find libftdi1
by Thomas Heijligen (Code Review) 14 Dec '21

14 Dec '21

Attention is currently required from: Nico Huber. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58623 ) Change subject: Makefile: Make pkg-config mandatory to find libftdi1 ...................................................................... Patch Set 9: (1 comment) File Makefile.include: https://review.coreboot.org/c/flashrom/+/58623/comment/4422683f_4bc8ef9a PS5, Line 40: 2>/dev/null > she $(debug_shell will work until we move all this in a target. […] I've replaced $(shell ...) by $(call debug_shell, ...) to get this upstream. But I'm not a fan of this. -- To view, visit https://review.coreboot.org/c/flashrom/+/58623 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I41f5186d9f3e063c12c8c6eea888d0b0bf534259 Gerrit-Change-Number: 58623 Gerrit-PatchSet: 9 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Tue, 14 Dec 2021 12:51:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Comment-In-Reply-To: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makefile: merge compiler, hwlibs, features targets into config target
by Thomas Heijligen (Code Review) 14 Dec '21

14 Dec '21

Thomas Heijligen has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/60107 ) Change subject: Makefile: merge compiler, hwlibs, features targets into config target ...................................................................... Makefile: merge compiler, hwlibs, features targets into config target Change-Id: Ic76f923bca2beb6f95b8ea0cced4569b07e9b9ba Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile 1 file changed, 47 insertions(+), 52 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/07/60107/1 diff --git a/Makefile b/Makefile index 183a66d..2f51782 100644 --- a/Makefile +++ b/Makefile @@ -854,43 +854,12 @@ LIBFLASHROM_OBJS = $(CHIP_OBJS) $(PROGRAMMER_OBJS) $(LIB_OBJS) OBJS = $(CLI_OBJS) $(LIBFLASHROM_OBJS) -all: hwlibs features $(PROGRAM)$(EXEC_SUFFIX) $(PROGRAM).8 +all: config $(PROGRAM)$(EXEC_SUFFIX) $(PROGRAM).8 ifeq ($(ARCH), x86) @+$(MAKE) -C util/ich_descriptors_tool/ HOST_OS=$(HOST_OS) TARGET_OS=$(TARGET_OS) endif -$(PROGRAM)$(EXEC_SUFFIX): $(OBJS) - $(CC) -o $(PROGRAM)$(EXEC_SUFFIX) $(OBJS) $(LDFLAGS) - -libflashrom.a: $(LIBFLASHROM_OBJS) - $(AR) rcs $@ $^ - $(RANLIB) $@ - -# TAROPTIONS reduces information leakage from the packager's system. -# If other tar programs support command line arguments for setting uid/gid of -# stored files, they can be handled here as well. -TAROPTIONS = $(shell LC_ALL=C tar --version|grep -q GNU && echo "--owner=root --group=root") - -%.o: %.c features - $(CC) -MMD $(CFLAGS) $(CPPFLAGS) $(FLASHROM_CFLAGS) $(FEATURE_CFLAGS) $(SCMDEF) -o $@ -c $< - -# Make sure to add all names of generated binaries here. -# This includes all frontends and libflashrom. -# We don't use EXEC_SUFFIX here because we want to clean everything. -clean: - rm -f $(PROGRAM) $(PROGRAM).exe libflashrom.a $(filter-out Makefile.d, $(wildcard *.d *.o)) $(PROGRAM).8 $(PROGRAM).8.html $(BUILD_DETAILS_FILE) - @+$(MAKE) -C util/ich_descriptors_tool/ clean - -distclean: clean - rm -f .libdeps - -strip: $(PROGRAM)$(EXEC_SUFFIX) - $(STRIP) $(STRIP_ARGS) $(PROGRAM)$(EXEC_SUFFIX) - -# to define test programs we use verbatim variables, which get exported -# to environment variables and are referenced with $$<varname> later - -compiler: featuresavailable +config: @echo -n "C compiler found: " @if [ $(CC_WORKING) = yes ]; \ then $(CC) --version 2>/dev/null | head -1; \ @@ -904,8 +873,6 @@ echo " This might work but usually does not, please beware."; fi @echo "Target endian: $(ENDIAN)" @if [ $(ENDIAN) = unknown ]; then echo Aborting.; exit 1; fi - -hwlibs: compiler @echo Dependency libpci found: $(HAS_LIBPCI) @if [ $(HAS_LIBPCI) = yes ]; then \ echo " Checking for old \"pci_get_dev()\": $(HAS_PCI_OLD_GET_DEV)";\ @@ -933,28 +900,56 @@ echo " CFLAGS: $(CONFIG_LIBFTDI1_CFLAGS)"; \ echo " LDFLAGS: $(CONFIG_LIBFTDI1_LDFLAGS)"; \ fi - -# If a user does not explicitly request a non-working feature, we should -# silently disable it. However, if a non-working (does not compile) feature -# is explicitly requested, we should bail out with a descriptive error message. -# We also have to check that at least one programmer driver is enabled. -featuresavailable: -ifeq ($(PROGRAMMER_OBJS),) - @echo "You have to enable at least one programmer driver!" - @false -endif -ifneq ($(UNSUPPORTED_FEATURES), ) - @echo "The following features are unavailable on your machine: $(UNSUPPORTED_FEATURES)" - @false -endif - -features: hwlibs @echo "Checking for header \"mtd/mtd-user.h\": $(HAS_LINUX_MTD)" @echo "Checking for header \"linux/spi/spidev.h\": $(HAS_LINUX_SPI)" @echo "Checking for header \"linux/i2c-dev.h\": $(HAS_LINUX_I2C)" @echo "Checking for header \"linux/i2c.h\": $(HAS_LINUX_I2C)" @echo "Checking for header \"sys/utsname.h\": $(HAS_UTSNAME)" @echo "Checking for function \"clock_gettime\": $(HAS_CLOCK_GETTIME)" + @if ! [ "$(PROGRAMMER_OBJS)" ]; then \ + echo "You have to enable at least one programmer driver!"; \ + exit 1; \ + fi + @if [ "$(UNSUPPORTED_FEATURES)" ]; then \ + echo "The following features are unavailable on your machine: $(UNSUPPORTED_FEATURES)" \ + exit 1; \ + fi + +%.o: %.c config + $(CC) -MMD $(CFLAGS) $(CPPFLAGS) $(FLASHROM_CFLAGS) $(FEATURE_CFLAGS) $(SCMDEF) -o $@ -c $< + + +$(PROGRAM)$(EXEC_SUFFIX): $(OBJS) + $(CC) -o $(PROGRAM)$(EXEC_SUFFIX) $(OBJS) $(LDFLAGS) + +libflashrom.a: $(LIBFLASHROM_OBJS) + $(AR) rcs $@ $^ + $(RANLIB) $@ + +# TAROPTIONS reduces information leakage from the packager's system. +# If other tar programs support command line arguments for setting uid/gid of +# stored files, they can be handled here as well. +TAROPTIONS = $(shell LC_ALL=C tar --version|grep -q GNU && echo "--owner=root --group=root") + + +# Make sure to add all names of generated binaries here. +# This includes all frontends and libflashrom. +# We don't use EXEC_SUFFIX here because we want to clean everything. +clean: + rm -f $(PROGRAM) $(PROGRAM).exe libflashrom.a $(filter-out Makefile.d, $(wildcard *.d *.o)) $(PROGRAM).8 $(PROGRAM).8.html $(BUILD_DETAILS_FILE) + @+$(MAKE) -C util/ich_descriptors_tool/ clean + +distclean: clean + rm -f .libdeps + +strip: $(PROGRAM)$(EXEC_SUFFIX) + $(STRIP) $(STRIP_ARGS) $(PROGRAM)$(EXEC_SUFFIX) + +# to define test programs we use verbatim variables, which get exported +# to environment variables and are referenced with $$<varname> later + + + $(PROGRAM).8.html: $(PROGRAM).8 @groff -mandoc -Thtml $< >$@ @@ -1007,7 +1002,7 @@ libpayload: clean make CC="CC=i386-elf-gcc lpgcc" AR=i386-elf-ar RANLIB=i386-elf-ranlib -.PHONY: all install clean distclean compiler hwlibs features _export export tarball featuresavailable libpayload +.PHONY: all install clean distclean config _export export tarball libpayload # Disable implicit suffixes and built-in rules (for performance and profit) .SUFFIXES: -- To view, visit https://review.coreboot.org/c/flashrom/+/60107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ic76f923bca2beb6f95b8ea0cced4569b07e9b9ba Gerrit-Change-Number: 60107 Gerrit-PatchSet: 1 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-MessageType: newchange

1 0

Change in flashrom[master]: Makefile: Rework NI-845x detection
by Thomas Heijligen (Code Review) 14 Dec '21

14 Dec '21

Attention is currently required from: Miklós Márton, Thomas Heijligen, Angel Pons. Hello build bot (Jenkins), Nico Huber, Miklós Márton, Miklós Márton, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/59047 to look at the new patch set (#12). Change subject: Makefile: Rework NI-845x detection ...................................................................... Makefile: Rework NI-845x detection Since the NI-845x is a Windows only proprietary library, disable it by default. Use `HAS_LIB_NI845X=yes` to enable it. The default search path is `${PROGRAMFILES}\National Instruments\NI-845x\MS Visual C` and can be overwritten by `CONFIG_NI845X_LIBRARY_PATH`. Use `CONFIG_LIB_NI845X_CFLAGS` and `CONFIG_LIB_NI845X_LDFLAGS` for setting the cflags and ld flags manually. Change-Id: I918c3605a5ac168708a6a10fd92ee2a1aae9729b Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile M Makefile.include 2 files changed, 36 insertions(+), 65 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/47/59047/12 -- To view, visit https://review.coreboot.org/c/flashrom/+/59047 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I918c3605a5ac168708a6a10fd92ee2a1aae9729b Gerrit-Change-Number: 59047 Gerrit-PatchSet: 12 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Attention: Miklós Márton <martonmiklosqdev(a)gmail.com> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: Makefile: Make pkg-config mandatory to find libftdi1
by Thomas Heijligen (Code Review) 14 Dec '21

14 Dec '21

Attention is currently required from: Nico Huber. Hello build bot (Jenkins), Nico Huber, Angel Pons, I'd like you to reexamine a change. Please visit https://review.coreboot.org/c/flashrom/+/58623 to look at the new patch set (#9). Change subject: Makefile: Make pkg-config mandatory to find libftdi1 ...................................................................... Makefile: Make pkg-config mandatory to find libftdi1 Use `make HAS_LIBFTDI1=yes/no` to override the pkg-config detection and `CONFIG_LIBFTDI1_CFLAGS` and `CONFIG_LIBFTDI1_LDFLAGS` to set cflags and ldflags manually. Change-Id: I41f5186d9f3e063c12c8c6eea888d0b0bf534259 Signed-off-by: Thomas Heijligen <thomas.heijligen(a)secunet.com> --- M Makefile A Makefile.d/ft232h_test.c M Makefile.include 3 files changed, 57 insertions(+), 59 deletions(-) git pull ssh://review.coreboot.org:29418/flashrom refs/changes/23/58623/9 -- To view, visit https://review.coreboot.org/c/flashrom/+/58623 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I41f5186d9f3e063c12c8c6eea888d0b0bf534259 Gerrit-Change-Number: 58623 Gerrit-PatchSet: 9 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: newpatchset

1 0

Change in flashrom[master]: Makefile: Make pkg-config mandatory to find libpci
by Thomas Heijligen (Code Review) 14 Dec '21

14 Dec '21

Attention is currently required from: Nico Huber. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59050 ) Change subject: Makefile: Make pkg-config mandatory to find libpci ...................................................................... Patch Set 12: (2 comments) Patchset: PS12: > > I'm not 100% sure it's a move into the right direction. […] I've an idea how to implement this. I would do this in a follow up patch. Makefile: ifeq ($(HAS_LIBPCI),no) $(call mark_unsupported, $(DEPENDS_ON_LIBPCI)) endif HAS_LIBPCI := $(call find_dependency, libpci) ifeq ($(HAS_LIBPCI),no) $(call disable, $(DEPENDS_ON_LIBPCI)) endif File Makefile: https://review.coreboot.org/c/flashrom/+/59050/comment/987be984_7b93e8a0 PS7, Line 772: # This is a dirty hack, but it saves us from checking all PCI drivers and all platforms manually. : # libpci may need raw memory, MSR or PCI port I/O on some platforms. : # Individual drivers might have the same needs as well. > Doesn't this comment stay valid? I guess we should at some point untangle our […] We should split pci programmers to DEPENDS_ON_IO_PORTS, DEPENDS_ON_RAW_MEM_ACCESS, DEPENDS_ON_X86_MSR -- To view, visit https://review.coreboot.org/c/flashrom/+/59050 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I21b4a261b34b7e688635fc6e20b7beebfa64c7ed Gerrit-Change-Number: 59050 Gerrit-PatchSet: 12 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Tue, 14 Dec 2021 12:47:42 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Nico Huber <nico.h(a)gmx.de> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: spi25_statusreg: make register read/write functions generic
by Angel Pons (Code Review) 14 Dec '21

14 Dec '21

Attention is currently required from: Nico Huber, Nikolai Artemiev, Sergii Dmytruk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58475 ) Change subject: spi25_statusreg: make register read/write functions generic ...................................................................... Patch Set 14: (1 comment) File flash.h: https://review.coreboot.org/c/flashrom/+/58475/comment/8f2d67fe_fd3976b4 PS14, Line 169: enum flash_reg { : INVALID_REG = 0, : STATUS1, : STATUS2, : CONFIG1, : }; : #define MAX_REGISTERS 4 > Does this need to be public, or can it remain private inside `spi25_statusreg. […] Also, this is specific to SPI flash chips. I'm not sure if it should be here. -- To view, visit https://review.coreboot.org/c/flashrom/+/58475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a3951bbf993f2d8d830143b29d3ce16cc6901d7 Gerrit-Change-Number: 58475 Gerrit-PatchSet: 14 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 14 Dec 2021 10:36:16 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Angel Pons <th3fanbus(a)gmail.com> Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flash: add structure to represent chip wp configuration
by Angel Pons (Code Review) 14 Dec '21

14 Dec '21

Attention is currently required from: Nico Huber, Nikolai Artemiev, Sergii Dmytruk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58478 ) Change subject: flash: add structure to represent chip wp configuration ...................................................................... Patch Set 21: (1 comment) File flash.h: https://review.coreboot.org/c/flashrom/+/58478/comment/88e6d732_d1891d3e PS21, Line 201: /* Complete description of a chip's write protection configuration */ : struct flashrom_wp_chip_config { : size_t srp_bit_count; : uint8_t srp[MAX_SRP_BITS]; : : bool cmp_bit_present; : uint8_t cmp; : : bool sec_bit_present; : uint8_t sec; : : bool tb_bit_present; : uint8_t tb; : : size_t bp_bit_count; : uint8_t bp[MAX_BP_BITS]; : }; This is specific to SPI flash chips. -- To view, visit https://review.coreboot.org/c/flashrom/+/58478 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I17dee630248ce7b51e624a6e46d7097d5d0de809 Gerrit-Change-Number: 58478 Gerrit-PatchSet: 21 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 14 Dec 2021 10:35:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: flashchips: add writeprotect bit layout map to chips
by Angel Pons (Code Review) 14 Dec '21

14 Dec '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Nikolai Artemiev, Sergii Dmytruk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58477 ) Change subject: flashchips: add writeprotect bit layout map to chips ...................................................................... Patch Set 20: (1 comment) File flashchips.c: https://review.coreboot.org/c/flashrom/+/58477/comment/32ff0615_36e77b49 PS20, Line 6353: .tb = {STATUS1, 5, RW}, /* Called BP3 in datasheet, acts like TB */ : .sec = {STATUS1, 6, RW}, /* Called BP4 in datasheet, acts like SEC */ Huh, I would've expected BP3 and BP4 to work like the other BPx bits. -- To view, visit https://review.coreboot.org/c/flashrom/+/58477 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Id08d77e6d4ca5109c0d698271146d026dbc21284 Gerrit-Change-Number: 58477 Gerrit-PatchSet: 20 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 14 Dec 2021 10:34:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

Change in flashrom[master]: spi25_statusreg,flashchips: add SR2 read/write support
by Angel Pons (Code Review) 14 Dec '21

14 Dec '21

Attention is currently required from: Nico Huber, Edward O'Callaghan, Nikolai Artemiev, Sergii Dmytruk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58570 ) Change subject: spi25_statusreg,flashchips: add SR2 read/write support ...................................................................... Patch Set 17: Code-Review+1 (1 comment) File spi25_statusreg.c: https://review.coreboot.org/c/flashrom/+/58570/comment/e748d629_9a1f7bb6 PS17, Line 109: if (spi_read_register(flash, STATUS1, &sr1)) : return 1; Would it make sense to print an error here? -- To view, visit https://review.coreboot.org/c/flashrom/+/58570 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I34a503b0958e8f2f22a2a993a6ea529eb46b41db Gerrit-Change-Number: 58570 Gerrit-PatchSet: 17 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 14 Dec 2021 10:31:53 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: spi25_statusreg: make register read/write functions generic
by Angel Pons (Code Review) 14 Dec '21

14 Dec '21

Attention is currently required from: Nico Huber, Nikolai Artemiev, Sergii Dmytruk. Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58475 ) Change subject: spi25_statusreg: make register read/write functions generic ...................................................................... Patch Set 14: (8 comments) File flash.h: https://review.coreboot.org/c/flashrom/+/58475/comment/8eb6e671_1eebe37f PS14, Line 169: enum flash_reg { : INVALID_REG = 0, : STATUS1, : STATUS2, : CONFIG1, : }; : #define MAX_REGISTERS 4 Does this need to be public, or can it remain private inside `spi25_statusreg.c`? File spi25_statusreg.c: https://review.coreboot.org/c/flashrom/+/58475/comment/d2825c1b_b1a5c042 PS14, Line 51: int result = spi_send_multicommand(flash, cmds); Would be good to check with manibuilder if this causes problems with unusual toolchains (e.g. DJGPP). https://review.coreboot.org/c/flashrom/+/58475/comment/62c990d2_3cb2aef2 PS14, Line 126: should be a tab https://review.coreboot.org/c/flashrom/+/58475/comment/79cf7549_f8bfb225 PS14, Line 134: nit: double blank line https://review.coreboot.org/c/flashrom/+/58475/comment/b196aedc_5366d628 PS14, Line 138: /* FIXME: We should propagate the error. */ Ah, this is why you want to drop `spi_read_status_register()` https://review.coreboot.org/c/flashrom/+/58475/comment/0fcb07c5_f39ff12c PS14, Line 149: nit: double blank line https://review.coreboot.org/c/flashrom/+/58475/comment/87fb8ad1_df5ffccd PS14, Line 194: spi_write_register failed. Would be good to indicate which register failed: Could not write status register 1. Or similar. https://review.coreboot.org/c/flashrom/+/58475/comment/3868e53e_88d303bc PS14, Line 207: spi_write_register failed. Ditto -- To view, visit https://review.coreboot.org/c/flashrom/+/58475 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I0a3951bbf993f2d8d830143b29d3ce16cc6901d7 Gerrit-Change-Number: 58475 Gerrit-PatchSet: 14 Gerrit-Owner: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> Gerrit-Reviewer: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Reviewer: Edward O'Callaghan <quasisec(a)chromium.org> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Attention: Nikolai Artemiev <nartemiev(a)google.com> Gerrit-Attention: Sergii Dmytruk <sergii.dmytruk(a)3mdeb.com> Gerrit-Comment-Date: Tue, 14 Dec 2021 10:29:27 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit December 2021