December 2021 - flashrom-gerrit

Change in flashrom[master]: physmap: renoame to hwaccess_physmap, create own header
by Nico Huber (Code Review) 21 Dec '21

21 Dec '21

Attention is currently required from: Thomas Heijligen, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60113 ) Change subject: physmap: renoame to hwaccess_physmap, create own header ...................................................................... Patch Set 4: Code-Review+1 (3 comments) Commit Message: https://review.coreboot.org/c/flashrom/+/60113/comment/436d8cbb_816addcd PS4, Line 9: Line up physmap with the other hwaccess related code. Oh, we could make it a hwaccess/ subdir of src/. No wait, there is still no src/. Patchset: PS4: Just one question. File flashrom.c: https://review.coreboot.org/c/flashrom/+/60113/comment/eede6cf9_963d730a PS4, Line 41: #if CONFIG_INTERNAL == 1 Why the guard and why only here? It's just function prototypes, they shouldn't need guarding. Or do I miss something? -- To view, visit https://review.coreboot.org/c/flashrom/+/60113 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ieba6f4e94cfc3e668fcb8b3c978de5908aed2592 Gerrit-Change-Number: 60113 Gerrit-PatchSet: 4 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: Felix Singer <felixsinger(a)posteo.net> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 21 Dec 2021 17:30:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makefile: cleanup
by Nico Huber (Code Review) 21 Dec '21

21 Dec '21

Attention is currently required from: Thomas Heijligen, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59230 ) Change subject: Makefile: cleanup ...................................................................... Patch Set 3: Code-Review+1 (2 comments) Patchset: PS3: Lol, found an old draft message that I never sent :) seems still valid. Also, the commit message needs an update. File Makefile: https://review.coreboot.org/c/flashrom/+/59230/comment/c54a765f_357683df PS1, Line 217: LDFLAGS += -lgetopt Don't these need an `override` too? -- To view, visit https://review.coreboot.org/c/flashrom/+/59230 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1c9e869377677d624469af1ee9ece9a28fc3b559 Gerrit-Change-Number: 59230 Gerrit-PatchSet: 3 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 21 Dec 2021 17:20:06 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess: move x86 port I/O related code into own files
by Nico Huber (Code Review) 21 Dec '21

21 Dec '21

Attention is currently required from: Thomas Heijligen, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60110 ) Change subject: hwaccess: move x86 port I/O related code into own files ...................................................................... Patch Set 5: (1 comment) File Makefile: https://review.coreboot.org/c/flashrom/+/60110/comment/d0ddef62_640a0b43 PS5, Line 786: x86 > I would touch this later in an other commit. This is just about separating the code parts. My reasoning is that it shouldn't hurt to do it now, and then it would be correct from the beginning and we wouldn't need to touch this again. IOW, even though the code is old, you are adding a new, x86-specific file to the build, so it should be guarded? -- To view, visit https://review.coreboot.org/c/flashrom/+/60110 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I372b4a409f036da766c42bc406b596bc41b0f75a Gerrit-Change-Number: 60110 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 21 Dec 2021 17:18:31 +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]: hwaccess physmap: move x86 msr related code into own files
by Nico Huber (Code Review) 21 Dec '21

21 Dec '21

Attention is currently required from: Thomas Heijligen, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60111 ) Change subject: hwaccess physmap: move x86 msr related code into own files ...................................................................... Patch Set 4: Code-Review+1 (1 comment) File Makefile: https://review.coreboot.org/c/flashrom/+/60111/comment/dc5568ae_4f74c4bb PS4, Line 786: x86 Same here, if it has x86 in the name, guard it? -- To view, visit https://review.coreboot.org/c/flashrom/+/60111 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Idc9ce9df3ea1e291ad469de59467646b294119c4 Gerrit-Change-Number: 60111 Gerrit-PatchSet: 4 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 21 Dec 2021 17:13:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: hwaccess: move x86 port I/O related code into own files
by Thomas Heijligen (Code Review) 21 Dec '21

21 Dec '21

Attention is currently required from: Nico Huber, Angel Pons. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60110 ) Change subject: hwaccess: move x86 port I/O related code into own files ...................................................................... Patch Set 5: (1 comment) File Makefile: https://review.coreboot.org/c/flashrom/+/60110/comment/3c64a3b9_c55f9404 PS5, Line 786: x86 > Should we guard this by ARCH,x86 right away? Then we wouldn't need […] I would touch this later in an other commit. This is just about separating the code parts. -- To view, visit https://review.coreboot.org/c/flashrom/+/60110 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I372b4a409f036da766c42bc406b596bc41b0f75a Gerrit-Change-Number: 60110 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 21 Dec 2021 17:09:15 +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]: hwaccess: move x86 port I/O related code into own files
by Nico Huber (Code Review) 21 Dec '21

21 Dec '21

Attention is currently required from: Thomas Heijligen, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60110 ) Change subject: hwaccess: move x86 port I/O related code into own files ...................................................................... Patch Set 5: Code-Review+1 (1 comment) File Makefile: https://review.coreboot.org/c/flashrom/+/60110/comment/4ffd3f35_d89a2fa8 PS5, Line 786: x86 Should we guard this by ARCH,x86 right away? Then we wouldn't need the x86 checks inside the file, which would better match the name. (removing the checks could be a follow-up) -- To view, visit https://review.coreboot.org/c/flashrom/+/60110 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I372b4a409f036da766c42bc406b596bc41b0f75a Gerrit-Change-Number: 60110 Gerrit-PatchSet: 5 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Anastasia Klimchuk <aklm(a)chromium.org> 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 21 Dec 2021 17:05:10 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

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

21 Dec '21

Attention is currently required from: Nico Huber. Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60107 ) Change subject: Makefile: merge compiler, hwlibs, features targets into config target ...................................................................... Patch Set 2: (3 comments) File Makefile: https://review.coreboot.org/c/flashrom/+/60107/comment/58b56aaa_94339182 PS2, Line 918: %.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) > This looks like it all just moved without changes. Could it be done in a […] Ack https://review.coreboot.org/c/flashrom/+/60107/comment/6a85e796_a6d177db PS2, Line 948: # to define test programs we use verbatim variables, which get exported : # to environment variables and are referenced with $$<varname> later > Is this still true? I think this is obsolete by now. https://review.coreboot.org/c/flashrom/+/60107/comment/894c72a9_d6a2839c PS2, Line 953: > Also, two empty lines should be enough :) Ack -- 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: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Nico Huber <nico.h(a)gmx.de> Gerrit-Comment-Date: Tue, 21 Dec 2021 17:01:40 +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]: pci.h: move include into own wrapper
by Nico Huber (Code Review) 21 Dec '21

21 Dec '21

Attention is currently required from: Thomas Heijligen, Angel Pons. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/58883 ) Change subject: pci.h: move include into own wrapper ...................................................................... Patch Set 4: Code-Review+2 (1 comment) File hwaccess.h: https://review.coreboot.org/c/flashrom/+/58883/comment/0f799c1d_e1fa2523 PS1, Line 36: #define pci_mmio_writeb mmio_le_writeb : #define pci_mmio_writew mmio_le_writew : #define pci_mmio_writel mmio_le_writel : #define pci_mmio_readb mmio_le_readb : #define pci_mmio_readw mmio_le_readw : #define pci_mmio_readl mmio_le_readl > Ok, I'll keep the pci_*** names for now. Ack -- To view, visit https://review.coreboot.org/c/flashrom/+/58883 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: Ibf00356f0ef5cc92e0ec99f8fe5cdda56f47b166 Gerrit-Change-Number: 58883 Gerrit-PatchSet: 4 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: Thomas Heijligen <src(a)posteo.de> Gerrit-Attention: Angel Pons <th3fanbus(a)gmail.com> Gerrit-Comment-Date: Tue, 21 Dec 2021 16:58:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes 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 Nico Huber (Code Review) 21 Dec '21

21 Dec '21

Attention is currently required from: Thomas Heijligen. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60107 ) Change subject: Makefile: merge compiler, hwlibs, features targets into config target ...................................................................... Patch Set 2: Code-Review+1 (4 comments) Patchset: PS2: Looks good but would probably be easier to verify as separate commits. File Makefile: https://review.coreboot.org/c/flashrom/+/60107/comment/88a727d7_71828992 PS2, Line 918: %.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) This looks like it all just moved without changes. Could it be done in a separate commit? https://review.coreboot.org/c/flashrom/+/60107/comment/9ccff5f4_8b4fbdba PS2, Line 948: # to define test programs we use verbatim variables, which get exported : # to environment variables and are referenced with $$<varname> later Is this still true? https://review.coreboot.org/c/flashrom/+/60107/comment/e71bf83f_b28076d6 PS2, Line 953: Also, two empty lines should be enough :) -- 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: 2 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Attention: Thomas Heijligen <src(a)posteo.de> Gerrit-Comment-Date: Tue, 21 Dec 2021 16:54:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in flashrom[master]: Makefile: cleanup
by Thomas Heijligen (Code Review) 21 Dec '21

21 Dec '21

Thomas Heijligen has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/59230 ) Change subject: Makefile: cleanup ...................................................................... Set Ready For Review -- To view, visit https://review.coreboot.org/c/flashrom/+/59230 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: flashrom Gerrit-Branch: master Gerrit-Change-Id: I1c9e869377677d624469af1ee9ece9a28fc3b559 Gerrit-Change-Number: 59230 Gerrit-PatchSet: 3 Gerrit-Owner: Thomas Heijligen <src(a)posteo.de> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)mailbox.org> Gerrit-Comment-Date: Tue, 21 Dec 2021 16:53:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

2024

2023

2022

2021

2020

2019

2018

2017

flashrom-gerrit December 2021